Skip to content

Commit

Permalink
Cache Nix system in EnsureNixInstalled; Remove InstallablePackageName…
Browse files Browse the repository at this point in the history
…s; and rename ConfigPackageNames to PackageNames; drop error return from ConfigPackages
  • Loading branch information
savil committed Aug 11, 2023
1 parent 9a3354a commit 9bbc3dc
Show file tree
Hide file tree
Showing 15 changed files with 57 additions and 116 deletions.
4 changes: 2 additions & 2 deletions internal/devconfig/packages.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,9 @@ func (p *Package) VersionedName() string {
return name
}

func (p *Package) IsEnabledOnPlatform() (bool, error) {
func (p *Package) IsEnabledOnPlatform() bool {
// TODO savil. Next PR will update this implementation
return true, nil
return true
}

func (p *Package) UnmarshalJSON(data []byte) error {
Expand Down
10 changes: 3 additions & 7 deletions internal/devpkg/package.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,16 +63,12 @@ func PackageFromStrings(rawNames []string, l lock.Locker) []*Package {
return packages
}

func PackagesFromConfig(config *devconfig.Config, l lock.Locker) ([]*Package, error) {
func PackagesFromConfig(config *devconfig.Config, l lock.Locker) []*Package {
result := []*Package{}
for _, pkg := range config.Packages.Collection {
isInstallable, err := pkg.IsEnabledOnPlatform()
if err != nil {
return nil, err
}
result = append(result, newPackage(pkg.VersionedName(), isInstallable, l))
result = append(result, newPackage(pkg.VersionedName(), pkg.IsEnabledOnPlatform(), l))
}
return result, nil
return result
}

// PackageFromString constructs Package from the raw name provided.
Expand Down
81 changes: 18 additions & 63 deletions internal/impl/devbox.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,14 +110,9 @@ func Open(opts *devopt.Opts) (*Devbox, error) {
)
box.lockfile = lock

hasDeprecated, err := box.HasDeprecatedPackages()
if err != nil {
return nil, err
}

if !opts.IgnoreWarnings &&
!legacyPackagesWarningHasBeenShown &&
hasDeprecated {
box.HasDeprecatedPackages() {
legacyPackagesWarningHasBeenShown = true
globalPath, err := GlobalDataPath()
if err != nil {
Expand All @@ -143,11 +138,7 @@ func (d *Devbox) Config() *devconfig.Config {
}

func (d *Devbox) ConfigHash() (string, error) {
pkgs, err := d.ConfigPackages()
if err != nil {
return "", err
}
pkgHashes := lo.Map(pkgs, func(i *devpkg.Package, _ int) string { return i.Hash() })
pkgHashes := lo.Map(d.configPackages(), func(i *devpkg.Package, _ int) string { return i.Hash() })
includeHashes := lo.Map(d.Includes(), func(i plugin.Includable, _ int) string { return i.Hash() })
h, err := d.cfg.Hash()
if err != nil {
Expand Down Expand Up @@ -411,7 +402,7 @@ func (d *Devbox) GenerateDevcontainer(ctx context.Context, force bool) error {
redact.Safe(filepath.Base(devContainerPath)), err)
}
// generate devcontainer.json
err = generate.CreateDevcontainer(ctx, devContainerPath, d.ConfigPackageNames())
err = generate.CreateDevcontainer(ctx, devContainerPath, d.PackageNames())
if err != nil {
return redact.Errorf("error generating devcontainer.json in <project>/%s: %w",
redact.Safe(filepath.Base(devContainerPath)), err)
Expand Down Expand Up @@ -492,12 +483,7 @@ func (d *Devbox) saveCfg() error {
}

func (d *Devbox) Services() (services.Services, error) {
packages, err := d.InstallablePackages()
if err != nil {
return nil, err
}

pluginSvcs, err := d.pluginManager.GetServices(packages, d.cfg.Include)
pluginSvcs, err := d.pluginManager.GetServices(d.InstallablePackages(), d.cfg.Include)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -835,11 +821,7 @@ func (d *Devbox) computeNixEnv(ctx context.Context, usePrintDevEnvCache bool) (m
// We still need to be able to add env variables to non-service binaries
// (e.g. ruby). This would involve understanding what binaries are associated
// to a given plugin.
installablePackages, err := d.InstallablePackages()
if err != nil {
return nil, err
}
pluginEnv, err := d.pluginManager.Env(installablePackages, d.cfg.Include, env)
pluginEnv, err := d.pluginManager.Env(d.InstallablePackages(), d.cfg.Include, env)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -942,42 +924,23 @@ func (d *Devbox) nixFlakesFilePath() string {
}

// ConfigPackageNames returns the package names as defined in devbox.json
func (d *Devbox) ConfigPackageNames() []string {
// TODO savil: centralize implementation by calling d.ConfigPackages and getting pkg.Raw
func (d *Devbox) PackageNames() []string {
// TODO savil: centralize implementation by calling d.configPackages and getting pkg.Raw
// Skipping for now to avoid propagating the error value.
return d.cfg.Packages.VersionedNames()
}

// InstallablePackageNames returns the names of packages that are to be installed
func (d *Devbox) InstallablePackageNames() ([]string, error) {
pkgs, err := d.InstallablePackages()
if err != nil {
return nil, err
}
return lo.Map(pkgs, func(pkg *devpkg.Package, _ int) string {
return pkg.Raw
}), nil
}

// ConfigPackages returns the packages that are defined in devbox.json
// configPackages returns the packages that are defined in devbox.json
// NOTE: the return type is different from devconfig.Packages
func (d *Devbox) ConfigPackages() ([]*devpkg.Package, error) {
pkgs, err := devpkg.PackagesFromConfig(d.cfg, d.lockfile)
if err != nil {
return nil, err
}
return pkgs, nil
func (d *Devbox) configPackages() []*devpkg.Package {
return devpkg.PackagesFromConfig(d.cfg, d.lockfile)
}

// InstallablePackages returns the packages that are to be installed
func (d *Devbox) InstallablePackages() ([]*devpkg.Package, error) {
pkgs, err := d.ConfigPackages()
if err != nil {
return nil, err
}
return lo.Filter(pkgs, func(pkg *devpkg.Package, _ int) bool {
func (d *Devbox) InstallablePackages() []*devpkg.Package {
return lo.Filter(d.configPackages(), func(pkg *devpkg.Package, _ int) bool {
return pkg.IsInstallable()
}), nil
})
}

func (d *Devbox) Includes() []plugin.Includable {
Expand All @@ -990,26 +953,18 @@ func (d *Devbox) Includes() []plugin.Includable {
return includes
}

func (d *Devbox) HasDeprecatedPackages() (bool, error) {
pkgs, err := d.ConfigPackages()
if err != nil {
return false, err
}
for _, pkg := range pkgs {
func (d *Devbox) HasDeprecatedPackages() bool {
for _, pkg := range d.configPackages() {
if pkg.IsLegacy() {
return true, nil
return true
}
}
return false, nil
return false
}

func (d *Devbox) findPackageByName(name string) (string, error) {
pkgs, err := d.ConfigPackages()
if err != nil {
return "", err
}
results := map[string]bool{}
for _, pkg := range pkgs {
for _, pkg := range d.configPackages() {
if pkg.String() == name || pkg.CanonicalName() == name {
results[pkg.String()] = true
}
Expand Down
2 changes: 1 addition & 1 deletion internal/impl/flakes.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ func (d *Devbox) getLocalFlakesDirs() []string {
localFlakeDirs := []string{}

// searching through installed packages to get location of local flakes
for _, pkg := range d.ConfigPackageNames() {
for _, pkg := range d.PackageNames() {
// filtering local flakes packages
if strings.HasPrefix(pkg, "path:") {
pkgDirAndName, _ := strings.CutPrefix(pkg, "path:")
Expand Down
2 changes: 1 addition & 1 deletion internal/impl/global.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (
const currentGlobalProfile = "default"

func (d *Devbox) PrintGlobalList() error {
for _, p := range d.ConfigPackageNames() {
for _, p := range d.PackageNames() {
fmt.Fprintf(d.writer, "* %s\n", p)
}
return nil
Expand Down
20 changes: 4 additions & 16 deletions internal/impl/packages.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func (d *Devbox) Add(ctx context.Context, pkgsNames ...string) error {
// names of added packages (even if they are already in config). We use this
// to know the exact name to mark as allowed insecure later on.
addedPackageNames := []string{}
existingPackageNames := d.ConfigPackageNames()
existingPackageNames := d.PackageNames()
for _, pkg := range pkgs {
// If exact versioned package is already in the config, skip.
if slices.Contains(existingPackageNames, pkg.Versioned()) {
Expand Down Expand Up @@ -258,11 +258,7 @@ func (d *Devbox) addPackagesToProfile(ctx context.Context, mode installMode) err
// If packages are in profile but nixpkgs has been purged, the experience
// will be poor when we try to run print-dev-env. So we ensure nixpkgs is
// prefetched for all relevant packages (those not in binary cache).
installablePkgs, err := d.InstallablePackages()
if err != nil {
return err
}
for _, input := range installablePkgs {
for _, input := range d.InstallablePackages() {
if err := input.EnsureNixpkgsPrefetched(d.writer); err != nil {
return err
}
Expand Down Expand Up @@ -385,11 +381,7 @@ func (d *Devbox) pendingPackagesForInstallation(ctx context.Context) ([]string,
if err != nil {
return nil, err
}
installablePkgs, err := d.InstallablePackages()
if err != nil {
return nil, err
}
for _, input := range installablePkgs {
for _, input := range d.InstallablePackages() {
_, err := nixprofile.ProfileListIndex(&nixprofile.ProfileListIndexArgs{
List: list,
Lockfile: d.lockfile,
Expand Down Expand Up @@ -424,11 +416,7 @@ func (d *Devbox) extraPackagesInProfile(ctx context.Context) ([]*nixprofile.NixP
if err != nil {
return nil, err
}
devboxInputs, err := d.InstallablePackages()
if err != nil {
return nil, err
}

devboxInputs := d.InstallablePackages()
if len(devboxInputs) == len(profileItems) {
// Optimization: skip comparison if number of packages are the same. This only works
// because we assume that all packages in `devbox.json` have just been added to the
Expand Down
2 changes: 1 addition & 1 deletion internal/impl/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func (d *Devbox) inputsToUpdate(pkgs ...string) ([]*devpkg.Package, error) {
pkgsToUpdate = append(pkgsToUpdate, found)
}
if len(pkgsToUpdate) == 0 {
pkgsToUpdate = d.ConfigPackageNames()
pkgsToUpdate = d.PackageNames()
}

return devpkg.PackageFromStrings(pkgsToUpdate, d.lockfile), nil
Expand Down
2 changes: 1 addition & 1 deletion internal/lock/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ package lock
type devboxProject interface {
ConfigHash() (string, error)
NixPkgsCommitHash() string
ConfigPackageNames() []string
PackageNames() []string
ProjectDir() string
}

Expand Down
2 changes: 1 addition & 1 deletion internal/lock/lockfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ func IsLegacyPackage(pkg string) bool {
// Tidy ensures that the lockfile has the set of packages corresponding to the devbox.json config.
// It gets rid of older packages that are no longer needed.
func (f *File) Tidy() {
f.Packages = lo.PickByKeys(f.Packages, f.devboxProject.ConfigPackageNames())
f.Packages = lo.PickByKeys(f.Packages, f.devboxProject.PackageNames())
}

// IsUpToDateAndInstalled returns true if the lockfile is up to date and the
Expand Down
15 changes: 11 additions & 4 deletions internal/nix/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,12 +95,19 @@ func isRoot() bool {
return os.Geteuid() == 0
}

func EnsureNixInstalled(writer io.Writer, withDaemonFunc func() *bool) error {
func EnsureNixInstalled(writer io.Writer, withDaemonFunc func() *bool) (err error) {
defer func() {
if err == nil {
// call System to ensure its value is internally cached so we can rely on MustGetSystem
_, err = System()
}
}()

if BinaryInstalled() {
return nil
}
if dirExists() {
if err := SourceNixEnv(); err != nil {
if err = SourceNixEnv(); err != nil {
return err
} else if BinaryInstalled() {
return nil
Expand All @@ -122,12 +129,12 @@ func EnsureNixInstalled(writer io.Writer, withDaemonFunc func() *bool) error {
fmt.Scanln()
}

if err := Install(writer, withDaemonFunc()); err != nil {
if err = Install(writer, withDaemonFunc()); err != nil {
return err
}

// Source again
if err := SourceNixEnv(); err != nil {
if err = SourceNixEnv(); err != nil {
return err
}

Expand Down
9 changes: 9 additions & 0 deletions internal/nix/nix.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,17 @@ func ExperimentalFlags() []string {
}
}

// TODO: rename to System
func MustGetSystem() string {
if cachedSystem == "" {
panic("MustGetSystem called before being initialized by System")
}
return cachedSystem
}

var cachedSystem string

// TODO: rename to ComputeSystem
func System() (string, error) {
// For Savil to debug "remove nixpkgs" feature. The Search api lacks x86-darwin info.
// So, I need to fake that I am x86-linux and inspect the output in generated devbox.lock
Expand Down
2 changes: 1 addition & 1 deletion internal/plugin/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ type Manager struct {
}

type devboxProject interface {
InstallablePackageNames() ([]string, error)
PackageNames() []string
ProjectDir() string
}

Expand Down
7 changes: 1 addition & 6 deletions internal/plugin/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,18 +157,13 @@ func (m *Manager) createFile(
urlForInput = pkg.URLForFlakeInput()
}

packages, err := m.InstallablePackageNames()
if err != nil {
return err
}

var buf bytes.Buffer
if err = tmpl.Execute(&buf, map[string]any{
"DevboxDir": filepath.Join(m.ProjectDir(), devboxDirName, name),
"DevboxDirRoot": filepath.Join(m.ProjectDir(), devboxDirName),
"DevboxProfileDefault": filepath.Join(m.ProjectDir(), nix.ProfilePath),
"PackageAttributePath": attributePath,
"Packages": packages,
"Packages": m.PackageNames(),
"System": system,
"URLForInput": urlForInput,
"Virtenv": filepath.Join(virtenvPath, name),
Expand Down
6 changes: 1 addition & 5 deletions internal/shellgen/flake_plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,7 @@ func newFlakePlan(ctx context.Context, devbox devboxer) (*flakePlan, error) {
ctx, task := trace.NewTask(ctx, "devboxFlakePlan")
defer task.End()

userPackages, err := devbox.InstallablePackages()
if err != nil {
return nil, err
}

userPackages := devbox.InstallablePackages()
// Create plugin directories first because inputs might depend on them
for _, pkg := range userPackages {
if err := devbox.PluginManager().Create(pkg); err != nil {
Expand Down
9 changes: 2 additions & 7 deletions internal/shellgen/scripts.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ const HooksFilename = ".hooks"
type devboxer interface {
Config() *devconfig.Config
Lockfile() *lock.File
InstallablePackages() ([]*devpkg.Package, error)
InstallablePackages() []*devpkg.Package
PluginManager() *plugin.Manager
ProjectDir() string
}
Expand All @@ -42,14 +42,9 @@ func WriteScriptsToFiles(devbox devboxer) error {
return errors.WithStack(err)
}

packages, err := devbox.InstallablePackages()
if err != nil {
return err
}

// Write all hooks to a file.
written := map[string]struct{}{} // set semantics; value is irrelevant
pluginHooks, err := plugin.InitHooks(packages, devbox.ProjectDir())
pluginHooks, err := plugin.InitHooks(devbox.InstallablePackages(), devbox.ProjectDir())
if err != nil {
return errors.WithStack(err)
}
Expand Down

0 comments on commit 9bbc3dc

Please sign in to comment.