From 08cdfb0dc20d997f6a0753a588e339c430a713ee Mon Sep 17 00:00:00 2001 From: Savil Srivastava <676452+savil@users.noreply.github.com> Date: Thu, 11 Jan 2024 16:01:06 -0800 Subject: [PATCH 01/11] [StorePathParts] move to nix package --- internal/devpkg/narinfo_cache.go | 38 ++----------------------- internal/devpkg/package_test.go | 43 ---------------------------- internal/nix/storepath.go | 40 ++++++++++++++++++++++++++ internal/nix/storepath_test.go | 48 ++++++++++++++++++++++++++++++++ 4 files changed, 90 insertions(+), 79 deletions(-) create mode 100644 internal/nix/storepath.go create mode 100644 internal/nix/storepath_test.go diff --git a/internal/devpkg/narinfo_cache.go b/internal/devpkg/narinfo_cache.go index ba8475bf320..170ff7e00cc 100644 --- a/internal/devpkg/narinfo_cache.go +++ b/internal/devpkg/narinfo_cache.go @@ -4,10 +4,8 @@ import ( "context" "io" "net/http" - "strings" "sync" "time" - "unicode" "github.com/pkg/errors" "go.jetpack.io/devbox/internal/boxcli/featureflag" @@ -110,8 +108,8 @@ func (p *Package) fetchNarInfoStatus() (bool, error) { ) } - pathParts := newStorePathParts(sysInfo.StorePath) - reqURL := BinaryCache + "/" + pathParts.hash + ".narinfo" + pathParts := nix.NewStorePathParts(sysInfo.StorePath) + reqURL := BinaryCache + "/" + pathParts.Hash + ".narinfo" ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() req, err := http.NewRequestWithContext(ctx, http.MethodHead, reqURL, nil) @@ -181,35 +179,3 @@ func (p *Package) sysInfoIfExists() (*lock.SystemInfo, error) { } return sysInfo, nil } - -// storePath are the constituent parts of -// /nix/store/-- -// -// This is a helper struct for analyzing the string representation -type storePathParts struct { - hash string - name string - version string -} - -// newStorePathParts splits a Nix store path into its hash, name and version -// components in the same way that Nix does. -// -// See https://nixos.org/manual/nix/stable/language/builtins.html#builtins-parseDrvName -func newStorePathParts(path string) storePathParts { - path = strings.TrimPrefix(path, "/nix/store/") - // path is now ---- +// +// This is a helper struct for analyzing the string representation +type StorePathParts struct { + Hash string + Name string + Version string +} + +// NewStorePathParts splits a Nix store path into its hash, name and version +// components in the same way that Nix does. +// +// See https://nixos.org/manual/nix/stable/language/builtins.html#builtins-parseDrvName +// +// TODO: store paths can also have `-{output}` suffixes, which need to be handled below. +func NewStorePathParts(path string) StorePathParts { + path = strings.TrimPrefix(path, "/nix/store/") + // path is now -- Date: Thu, 21 Dec 2023 16:52:59 -0800 Subject: [PATCH 02/11] nix profile from flake --- internal/devbox/nixprofile.go | 88 ++++++++++++++++++ internal/devbox/packages.go | 169 ++-------------------------------- 2 files changed, 96 insertions(+), 161 deletions(-) create mode 100644 internal/devbox/nixprofile.go diff --git a/internal/devbox/nixprofile.go b/internal/devbox/nixprofile.go new file mode 100644 index 00000000000..0410357f9a9 --- /dev/null +++ b/internal/devbox/nixprofile.go @@ -0,0 +1,88 @@ +package devbox + +import ( + "context" + "encoding/json" + "fmt" + "os/exec" + "slices" + + "go.jetpack.io/devbox/internal/nix" +) + +func syncFlakeToProfile(ctx context.Context, flakePath, profilePath string) error { + cmd := exec.CommandContext(ctx, "nix", "eval", "--json", flakePath+"#devShells."+nix.System()+".default.buildInputs") + b, err := cmd.Output() + if err != nil { + return fmt.Errorf("nix eval devShells: %v", err) + } + storePaths := []string{} + if err := json.Unmarshal(b, &storePaths); err != nil { + return fmt.Errorf("unmarshal store paths: %s: %v", b, err) + } + + listCmd := exec.CommandContext(ctx, "nix", "profile", "list", "--json", "--profile", profilePath) + b, err = listCmd.Output() + if err != nil { + return err + } + var profile struct { + Elements []struct { + StorePaths []string + } + } + if err := json.Unmarshal(b, &profile); err != nil { + return fmt.Errorf("unmarshal profile: %v", err) + } + got := make([]string, 0, len(profile.Elements)) + for _, e := range profile.Elements { + got = append(got, e.StorePaths...) + } + + add, remove := diffStorePaths(got, storePaths) + if len(remove) > 0 { + removeCmd := exec.CommandContext(ctx, "nix", "profile", "remove", "--profile", profilePath) + removeCmd.Args = append(removeCmd.Args, remove...) + if err := removeCmd.Run(); err != nil { + return err + } + } + if len(add) > 0 { + addCmd := exec.CommandContext(ctx, "nix", "profile", "install", "--profile", profilePath) + addCmd.Args = append(addCmd.Args, add...) + if err := addCmd.Run(); err != nil { + return err + } + } + return nil +} + +func diffStorePaths(got, want []string) (add, remove []string) { + slices.Sort(got) + slices.Sort(want) + + var g, w int + for { + if g >= len(got) { + add = append(add, want[w:]...) + break + } + if w >= len(want) { + remove = append(remove, got[g:]...) + break + } + + switch { + case got[g] == want[w]: + g++ + w++ + case got[g] < want[w]: + remove = append(remove, got[g]) + g++ + case got[g] > want[w]: + add = append(add, want[w]) + w++ + } + } + return add, remove +} diff --git a/internal/devbox/packages.go b/internal/devbox/packages.go index c750f4d0b89..2516b0074b1 100644 --- a/internal/devbox/packages.go +++ b/internal/devbox/packages.go @@ -18,7 +18,6 @@ import ( "go.jetpack.io/devbox/internal/devbox/devopt" "go.jetpack.io/devbox/internal/devpkg" "go.jetpack.io/devbox/internal/devpkg/pkgtype" - "go.jetpack.io/devbox/internal/nix/nixprofile" "go.jetpack.io/devbox/internal/shellgen" "go.jetpack.io/devbox/internal/boxcli/usererr" @@ -274,10 +273,6 @@ func (d *Devbox) ensureStateIsUpToDate(ctx context.Context, mode installMode) er } } - if err := d.syncPackagesToProfile(ctx, mode); err != nil { - return err - } - if err := d.InstallRunXPackages(ctx); err != nil { return err } @@ -297,6 +292,14 @@ func (d *Devbox) ensureStateIsUpToDate(ctx context.Context, mode installMode) er return err } + profile, err := d.profilePath() + if err != nil { + return err + } + if err := syncFlakeToProfile(ctx, d.flakeDir(), profile); err != nil { + return err + } + // Ensure we clean out packages that are no longer needed. d.lockfile.Tidy() @@ -331,162 +334,6 @@ func (d *Devbox) profilePath() (string, error) { return absPath, errors.WithStack(os.MkdirAll(filepath.Dir(absPath), 0o755)) } -// syncPackagesToProfile can ensure that all packages in devbox.json exist in the nix profile, -// and no more. However, it may skip some steps depending on the `mode`. -func (d *Devbox) syncPackagesToProfile(ctx context.Context, mode installMode) error { - defer debug.FunctionTimer().End() - defer trace.StartRegion(ctx, "syncPackagesToProfile").End() - - // First, fetch the profile items from the nix-profile, - // and get the installable packages - profileDir, err := d.profilePath() - if err != nil { - return err - } - profileItems, err := nixprofile.ProfileListItems(d.stderr, profileDir) - if err != nil { - return err - } - packages, err := d.AllInstallablePackages() - if err != nil { - return err - } - - // Remove non-nix packages from the list - packages = lo.Filter(packages, devpkg.IsNix) - - if err := devpkg.FillNarInfoCache(ctx, packages...); err != nil { - return err - } - - // Second, remove any packages from the nix-profile that are not in the config - itemsToKeep := profileItems - if mode != install { - itemsToKeep, err = d.removeExtraItemsFromProfile(ctx, profileDir, profileItems, packages) - if err != nil { - return err - } - } - - // we are done if mode is uninstall - if mode == uninstall { - return nil - } - - // Last, find the pending packages, and ensure they are added to the nix-profile - // Important to maintain the order of packages as specified by - // Devbox.InstallablePackages() (higher priority first). - // We also run nix profile upgrade on any virtenv flakes. This is a bit of a - // blunt approach, but ensured any plugin created flakes are up-to-date. - pending := []*devpkg.Package{} - for _, pkg := range packages { - idx, err := nixprofile.ProfileListIndex(&nixprofile.ProfileListIndexArgs{ - Items: itemsToKeep, - Lockfile: d.lockfile, - Writer: d.stderr, - Package: pkg, - ProfileDir: profileDir, - }) - if err != nil { - if !errors.Is(err, nix.ErrPackageNotFound) { - return err - } - pending = append(pending, pkg) - } else if f, err := pkg.FlakeInstallable(); err == nil && d.pluginManager.PathIsInVirtenv(f.Ref.Path) { - if err := nix.ProfileUpgrade(profileDir, idx); err != nil { - return err - } - } - } - - return d.addPackagesToProfile(ctx, pending) -} - -func (d *Devbox) removeExtraItemsFromProfile( - ctx context.Context, - profileDir string, - profileItems []*nixprofile.NixProfileListItem, - packages []*devpkg.Package, -) ([]*nixprofile.NixProfileListItem, error) { - defer debug.FunctionTimer().End() - defer trace.StartRegion(ctx, "removeExtraPackagesFromProfile").End() - - itemsToKeep := []*nixprofile.NixProfileListItem{} - extras := []*nixprofile.NixProfileListItem{} - // Note: because devpkg.Package uses memoization when normalizing attribute paths (slow operation), - // and since we're reusing the Package objects, this O(n*m) loop becomes O(n+m) wrt the slow operation. - for _, item := range profileItems { - found := false - for _, pkg := range packages { - if item.Matches(pkg, d.lockfile) { - itemsToKeep = append(itemsToKeep, item) - found = true - break - } - } - if !found { - extras = append(extras, item) - } - } - // Remove by index to avoid comparing nix.ProfileListItem <> nix.Inputs again. - if err := nixprofile.ProfileRemoveItems(profileDir, extras); err != nil { - return nil, err - } - return itemsToKeep, nil -} - -// addPackagesToProfile inspects the packages in devbox.json, checks which of them -// are missing from the nix profile, and then installs each package individually into the -// nix profile. -func (d *Devbox) addPackagesToProfile(ctx context.Context, pkgs []*devpkg.Package) error { - defer debug.FunctionTimer().End() - defer trace.StartRegion(ctx, "addPackagesToProfile").End() - - if len(pkgs) == 0 { - return nil - } - - // 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). - if err := devpkg.EnsureNixpkgsPrefetched(ctx, d.stderr, pkgs); err != nil { - return err - } - - var msg string - if len(pkgs) == 1 { - msg = fmt.Sprintf("Installing package: %s.", pkgs[0]) - } else { - pkgNames := lo.Map(pkgs, func(p *devpkg.Package, _ int) string { return p.Raw }) - msg = fmt.Sprintf("Installing %d packages: %s.", len(pkgs), strings.Join(pkgNames, ", ")) - } - fmt.Fprintf(d.stderr, "\n%s\n\n", msg) - - profileDir, err := d.profilePath() - if err != nil { - return fmt.Errorf("error getting profile path: %w", err) - } - - total := len(pkgs) - for idx, pkg := range pkgs { - stepNum := idx + 1 - - stepMsg := fmt.Sprintf("[%d/%d] %s", stepNum, total, pkg) - - if err = nixprofile.ProfileInstall(ctx, &nixprofile.ProfileInstallArgs{ - CustomStepMessage: stepMsg, - Lockfile: d.lockfile, - Package: pkg.Raw, - ProfilePath: profileDir, - Writer: d.stderr, - }); err != nil { - return fmt.Errorf("error installing package %s: %w", pkg, err) - } - } - - return nil -} - var resetCheckDone = false // resetProfileDirForFlakes ensures the profileDir directory is cleared of old From 54e0493663de3d9a19540e3ada59fba7f1de748a Mon Sep 17 00:00:00 2001 From: Savil Srivastava <676452+savil@users.noreply.github.com> Date: Thu, 21 Dec 2023 17:14:33 -0800 Subject: [PATCH 03/11] fixes: experimental flags, UX improvements --- internal/devbox/nixprofile.go | 107 ++++++++++++++++---------- internal/devbox/packages.go | 6 +- internal/devbox/util.go | 2 +- internal/devpkg/validation.go | 2 +- internal/nix/eval.go | 14 +++- internal/nix/nixprofile/item.go | 4 + internal/nix/nixprofile/profile.go | 71 ++++++++++------- internal/nix/nixprofile/store_path.go | 1 + internal/nix/nixprofile/upgrade.go | 8 +- internal/nix/profiles.go | 14 ++-- testscripts/languages/php.test.txt | 1 - testscripts/packages/unfree.test.txt | 2 +- 12 files changed, 142 insertions(+), 90 deletions(-) create mode 100644 internal/nix/nixprofile/store_path.go diff --git a/internal/devbox/nixprofile.go b/internal/devbox/nixprofile.go index 0410357f9a9..545badb480c 100644 --- a/internal/devbox/nixprofile.go +++ b/internal/devbox/nixprofile.go @@ -4,54 +4,79 @@ import ( "context" "encoding/json" "fmt" - "os/exec" "slices" + "strings" "go.jetpack.io/devbox/internal/nix" + "go.jetpack.io/devbox/internal/nix/nixprofile" ) -func syncFlakeToProfile(ctx context.Context, flakePath, profilePath string) error { - cmd := exec.CommandContext(ctx, "nix", "eval", "--json", flakePath+"#devShells."+nix.System()+".default.buildInputs") - b, err := cmd.Output() +func (d *Devbox) syncFlakeToProfile(ctx context.Context) error { + profilePath, err := d.profilePath() + if err != nil { + return err + } + + // Get the build inputs (i.e. store paths) from the generated flake's devShell. + buildInputPaths, err := nix.Eval( + ctx, + d.stderr, + d.flakeDir()+"#devShells."+nix.System()+".default.buildInputs", + "--json", + ) if err != nil { return fmt.Errorf("nix eval devShells: %v", err) } storePaths := []string{} - if err := json.Unmarshal(b, &storePaths); err != nil { - return fmt.Errorf("unmarshal store paths: %s: %v", b, err) + if err := json.Unmarshal(buildInputPaths, &storePaths); err != nil { + return fmt.Errorf("unmarshal store paths: %s: %v", buildInputPaths, err) } - listCmd := exec.CommandContext(ctx, "nix", "profile", "list", "--json", "--profile", profilePath) - b, err = listCmd.Output() + // Get the store-paths of the packages currently installed in the nix profile + items, err := nixprofile.ProfileListItems(ctx, d.stderr, profilePath) if err != nil { - return err - } - var profile struct { - Elements []struct { - StorePaths []string - } + return fmt.Errorf("nix profile list: %v", err) } - if err := json.Unmarshal(b, &profile); err != nil { - return fmt.Errorf("unmarshal profile: %v", err) - } - got := make([]string, 0, len(profile.Elements)) - for _, e := range profile.Elements { - got = append(got, e.StorePaths...) + got := make([]string, 0, len(items)) + for _, item := range items { + got = append(got, item.StorePaths()...) } + // Diff the store paths and install/remove packages as needed add, remove := diffStorePaths(got, storePaths) if len(remove) > 0 { - removeCmd := exec.CommandContext(ctx, "nix", "profile", "remove", "--profile", profilePath) - removeCmd.Args = append(removeCmd.Args, remove...) - if err := removeCmd.Run(); err != nil { + packagesToRemove := make([]string, 0, len(remove)) + for _, p := range remove { + storePath := nix.NewStorePathParts(p) + packagesToRemove = append(packagesToRemove, fmt.Sprintf("%s@%s", storePath.Name, storePath.Version)) + } + if len(packagesToRemove) == 1 { + fmt.Fprintf(d.stderr, "Removing %s\n", strings.Join(packagesToRemove, ", ")) + } else { + fmt.Fprintf(d.stderr, "Removing packages: %s\n", strings.Join(packagesToRemove, ", ")) + } + + if err := nix.ProfileRemove(profilePath, remove...); err != nil { return err } } if len(add) > 0 { - addCmd := exec.CommandContext(ctx, "nix", "profile", "install", "--profile", profilePath) - addCmd.Args = append(addCmd.Args, add...) - if err := addCmd.Run(); err != nil { - return err + total := len(add) + for idx, addPath := range add { + stepNum := idx + 1 + storePath := nix.NewStorePathParts(addPath) + nameAndVersion := fmt.Sprintf("%s@%s", storePath.Name, storePath.Version) + stepMsg := fmt.Sprintf("[%d/%d] %s", stepNum, total, nameAndVersion) + + if err = nixprofile.ProfileInstall(ctx, &nixprofile.ProfileInstallArgs{ + CustomStepMessage: stepMsg, + Installable: addPath, + PackageName: storePath.Name, + ProfilePath: profilePath, + Writer: d.stderr, + }); err != nil { + return fmt.Errorf("error installing package %s: %w", addPath, err) + } } } return nil @@ -61,27 +86,27 @@ func diffStorePaths(got, want []string) (add, remove []string) { slices.Sort(got) slices.Sort(want) - var g, w int + var gotIdx, wantIdx int for { - if g >= len(got) { - add = append(add, want[w:]...) + if gotIdx >= len(got) { + add = append(add, want[wantIdx:]...) break } - if w >= len(want) { - remove = append(remove, got[g:]...) + if wantIdx >= len(want) { + remove = append(remove, got[gotIdx:]...) break } switch { - case got[g] == want[w]: - g++ - w++ - case got[g] < want[w]: - remove = append(remove, got[g]) - g++ - case got[g] > want[w]: - add = append(add, want[w]) - w++ + case got[gotIdx] == want[wantIdx]: + gotIdx++ + wantIdx++ + case got[gotIdx] < want[wantIdx]: + remove = append(remove, got[gotIdx]) + gotIdx++ + case got[gotIdx] > want[wantIdx]: + add = append(add, want[wantIdx]) + wantIdx++ } } return add, remove diff --git a/internal/devbox/packages.go b/internal/devbox/packages.go index 2516b0074b1..904fbe2be49 100644 --- a/internal/devbox/packages.go +++ b/internal/devbox/packages.go @@ -292,11 +292,7 @@ func (d *Devbox) ensureStateIsUpToDate(ctx context.Context, mode installMode) er return err } - profile, err := d.profilePath() - if err != nil { - return err - } - if err := syncFlakeToProfile(ctx, d.flakeDir(), profile); err != nil { + if err := d.syncFlakeToProfile(ctx); err != nil { return err } diff --git a/internal/devbox/util.go b/internal/devbox/util.go index 64c5168a7cb..1771880df99 100644 --- a/internal/devbox/util.go +++ b/internal/devbox/util.go @@ -25,7 +25,7 @@ func (d *Devbox) addDevboxUtilityPackage(ctx context.Context, pkg string) error return err } - return nixprofile.ProfileInstall(ctx, &nixprofile.ProfileInstallArgs{ + return nixprofile.ProfileInstallPackage(ctx, &nixprofile.ProfileInstallPackageArgs{ Lockfile: d.lockfile, Package: pkg, ProfilePath: profilePath, diff --git a/internal/devpkg/validation.go b/internal/devpkg/validation.go index a43289c0082..04a60d6a183 100644 --- a/internal/devpkg/validation.go +++ b/internal/devpkg/validation.go @@ -38,7 +38,7 @@ func (p *Package) ValidateInstallsOnSystem() (bool, error) { if len(info) == 0 { return false, nil } - if out, err := nix.Eval(u); err != nil && + if out, err := nix.RawEval(u); err != nil && strings.Contains(string(out), "is not available on the requested hostPlatform") { return false, nil } diff --git a/internal/nix/eval.go b/internal/nix/eval.go index 94ad821a807..48b3a824786 100644 --- a/internal/nix/eval.go +++ b/internal/nix/eval.go @@ -1,7 +1,9 @@ package nix import ( + "context" "encoding/json" + "io" "os" "strconv" ) @@ -46,14 +48,22 @@ func PackageKnownVulnerabilities(path string) []string { return vulnerabilities } -// Eval is raw nix eval. Needs to be parsed. Useful for stuff like +// RawEval is nix eval. Needs to be parsed. Useful for stuff like // nix eval --raw nixpkgs/9ef09e06806e79e32e30d17aee6879d69c011037#fuse3 // to determine if a package if a package can be installed in system. -func Eval(path string) ([]byte, error) { +func RawEval(path string) ([]byte, error) { cmd := command("eval", "--raw", path) return cmd.CombinedOutput() } +func Eval(ctx context.Context, w io.Writer, installable string, options ...string) ([]byte, error) { + args := []string{"eval", installable} + args = append(args, options...) + cmd := commandContext(ctx, args...) + cmd.Stderr = w + return cmd.Output() +} + func AllowInsecurePackages() { os.Setenv("NIXPKGS_ALLOW_INSECURE", "1") } diff --git a/internal/nix/nixprofile/item.go b/internal/nix/nixprofile/item.go index b6759500ee9..af14ad0b4f0 100644 --- a/internal/nix/nixprofile/item.go +++ b/internal/nix/nixprofile/item.go @@ -83,3 +83,7 @@ func (i *NixProfileListItem) String() string { i.nixStorePaths, ) } + +func (i *NixProfileListItem) StorePaths() []string { + return i.nixStorePaths +} diff --git a/internal/nix/nixprofile/profile.go b/internal/nix/nixprofile/profile.go index dc9490f6c69..ff0af3c2629 100644 --- a/internal/nix/nixprofile/profile.go +++ b/internal/nix/nixprofile/profile.go @@ -23,15 +23,17 @@ import ( ) // ProfileListItems returns a list of the installed packages. +// TODO drop the Profile prefix from this function name. func ProfileListItems( + ctx context.Context, writer io.Writer, profileDir string, ) ([]*NixProfileListItem, error) { - output, err := nix.ProfileList(writer, profileDir, true /*useJSON*/) + output, err := nix.ProfileList(ctx, writer, profileDir, true /*useJSON*/) if err != nil { // fallback to legacy profile list // NOTE: maybe we should check the nix version first, instead of falling back on _any_ error. - return profileListLegacy(writer, profileDir) + return profileListLegacy(ctx, writer, profileDir) } type ProfileListElement struct { @@ -65,11 +67,13 @@ func ProfileListItems( } // profileListLegacy lists the items in a nix profile before nix 2.17.0 introduced --json. +// TODO drop the Profile prefix from this function name. func profileListLegacy( + ctx context.Context, writer io.Writer, profileDir string, ) ([]*NixProfileListItem, error) { - output, err := nix.ProfileList(writer, profileDir, false /*useJSON*/) + output, err := nix.ProfileList(ctx, writer, profileDir, false /*useJSON*/) if err != nil { return nil, errors.WithStack(err) } @@ -99,6 +103,7 @@ func profileListLegacy( return items, nil } +// TODO drop the Profile prefix from this name. type ProfileListIndexArgs struct { // For performance, you can reuse the same list in multiple operations if you // are confident index has not changed. @@ -111,11 +116,12 @@ type ProfileListIndexArgs struct { // ProfileListIndex returns the index of args.Package in the nix profile specified by args.ProfileDir, // or -1 if it's not found. Callers can pass in args.Items to avoid having to call `nix-profile list` again. -func ProfileListIndex(args *ProfileListIndexArgs) (int, error) { +// TODO drop the Profile prefix from this name. +func ProfileListIndex(ctx context.Context, args *ProfileListIndexArgs) (int, error) { var err error items := args.Items if items == nil { - items, err = ProfileListItems(args.Writer, args.ProfileDir) + items, err = ProfileListItems(ctx, args.Writer, args.ProfileDir) if err != nil { return -1, err } @@ -189,7 +195,8 @@ func parseNixProfileListItem(line string) (*NixProfileListItem, error) { }, nil } -type ProfileInstallArgs struct { +// TODO drop the Profile prefix from this name. +type ProfileInstallPackageArgs struct { CustomStepMessage string Lockfile *lock.File Package string @@ -197,8 +204,9 @@ type ProfileInstallArgs struct { Writer io.Writer } -// ProfileInstall calls nix profile install with default profile -func ProfileInstall(ctx context.Context, args *ProfileInstallArgs) error { +// ProfileInstallPackage installs a Devbox package into a profile. +// TODO drop the Profile prefix from this name. +func ProfileInstallPackage(ctx context.Context, args *ProfileInstallPackageArgs) error { input := devpkg.PackageFromStringWithDefaults(args.Package, args.Lockfile) inCache, err := input.IsInBinaryCache() @@ -226,7 +234,31 @@ func ProfileInstall(ctx context.Context, args *ProfileInstallArgs) error { ) } } - stepMsg := args.Package + + installable, err := input.Installable() + if err != nil { + return err + } + return ProfileInstall(ctx, &ProfileInstallArgs{ + CustomStepMessage: args.CustomStepMessage, + Installable: installable, + PackageName: args.Package, + ProfilePath: args.ProfilePath, + }) +} + +// TODO drop the Profile prefix from this name. +type ProfileInstallArgs struct { + CustomStepMessage string + Installable string + PackageName string + ProfilePath string + Writer io.Writer +} + +// TODO drop the Profile prefix from this name. +func ProfileInstall(ctx context.Context, args *ProfileInstallArgs) error { + stepMsg := args.PackageName if args.CustomStepMessage != "" { stepMsg = args.CustomStepMessage // Only print this first one if we have a custom message. Otherwise it feels @@ -234,12 +266,7 @@ func ProfileInstall(ctx context.Context, args *ProfileInstallArgs) error { fmt.Fprintf(args.Writer, "%s\n", stepMsg) } - installable, err := input.Installable() - if err != nil { - return err - } - - err = nix.ProfileInstall(args.Writer, args.ProfilePath, installable) + err := nix.ProfileInstall(ctx, args.Writer, args.ProfilePath, args.Installable) if err != nil { fmt.Fprintf(args.Writer, "%s: ", stepMsg) color.New(color.FgRed).Fprintf(args.Writer, "Fail\n") @@ -250,17 +277,3 @@ func ProfileInstall(ctx context.Context, args *ProfileInstallArgs) error { color.New(color.FgGreen).Fprintf(args.Writer, "Success\n") return nil } - -// ProfileRemoveItems removes the items from the profile, in a single call, using their indexes. -// It is up to the caller to ensure that the underlying profile has not changed since the items -// were queried. -func ProfileRemoveItems(profilePath string, items []*NixProfileListItem) error { - if len(items) == 0 { - return nil - } - indexes := []string{} - for _, item := range items { - indexes = append(indexes, strconv.Itoa(item.index)) - } - return nix.ProfileRemove(profilePath, indexes) -} diff --git a/internal/nix/nixprofile/store_path.go b/internal/nix/nixprofile/store_path.go new file mode 100644 index 00000000000..d691897e42e --- /dev/null +++ b/internal/nix/nixprofile/store_path.go @@ -0,0 +1 @@ +package nixprofile diff --git a/internal/nix/nixprofile/upgrade.go b/internal/nix/nixprofile/upgrade.go index 2200e56b911..bfd579913df 100644 --- a/internal/nix/nixprofile/upgrade.go +++ b/internal/nix/nixprofile/upgrade.go @@ -4,6 +4,7 @@ package nixprofile import ( + "context" "os" "go.jetpack.io/devbox/internal/devpkg" @@ -11,18 +12,19 @@ import ( "go.jetpack.io/devbox/internal/nix" ) -func ProfileUpgrade(ProfileDir string, pkg *devpkg.Package, lock *lock.File) error { +func ProfileUpgrade(profileDir string, pkg *devpkg.Package, lock *lock.File) error { idx, err := ProfileListIndex( + context.TODO(), &ProfileListIndexArgs{ Lockfile: lock, Writer: os.Stderr, Package: pkg, - ProfileDir: ProfileDir, + ProfileDir: profileDir, }, ) if err != nil { return err } - return nix.ProfileUpgrade(ProfileDir, idx) + return nix.ProfileUpgrade(profileDir, idx) } diff --git a/internal/nix/profiles.go b/internal/nix/profiles.go index e01516cc1c6..7ec9707d356 100644 --- a/internal/nix/profiles.go +++ b/internal/nix/profiles.go @@ -4,6 +4,7 @@ package nix import ( + "context" "encoding/json" "fmt" "io" @@ -17,8 +18,8 @@ import ( "go.jetpack.io/devbox/internal/redact" ) -func ProfileList(writer io.Writer, profilePath string, useJSON bool) (string, error) { - cmd := command("profile", "list", "--profile", profilePath) +func ProfileList(ctx context.Context, writer io.Writer, profilePath string, useJSON bool) (string, error) { + cmd := commandContext(ctx, "profile", "list", "--profile", profilePath) if useJSON { cmd.Args = append(cmd.Args, "--json") } @@ -29,7 +30,7 @@ func ProfileList(writer io.Writer, profilePath string, useJSON bool) (string, er return string(out), nil } -func ProfileInstall(writer io.Writer, profilePath, installable string) error { +func ProfileInstall(ctx context.Context, writer io.Writer, profilePath, installable string) error { if !IsInsecureAllowed() && PackageIsInsecure(installable) { knownVulnerabilities := PackageKnownVulnerabilities(installable) errString := fmt.Sprintf("Package %s is insecure. \n\n", installable) @@ -40,7 +41,8 @@ func ProfileInstall(writer io.Writer, profilePath, installable string) error { return usererr.New(errString) } - cmd := command( + cmd := commandContext( + ctx, "profile", "install", "--profile", profilePath, "--impure", // for NIXPKGS_ALLOW_UNFREE @@ -63,13 +65,13 @@ func ProfileInstall(writer io.Writer, profilePath, installable string) error { return cmd.Run() } -func ProfileRemove(profilePath string, indexes []string) error { +func ProfileRemove(profilePath string, elements ...string) error { cmd := command( append([]string{ "profile", "remove", "--profile", profilePath, "--impure", // for NIXPKGS_ALLOW_UNFREE - }, indexes...)..., + }, elements...)..., ) cmd.Env = allowUnfreeEnv(allowInsecureEnv(os.Environ())) diff --git a/testscripts/languages/php.test.txt b/testscripts/languages/php.test.txt index 3cd60353e21..f45ece1073f 100644 --- a/testscripts/languages/php.test.txt +++ b/testscripts/languages/php.test.txt @@ -18,7 +18,6 @@ stdout 'done\n' { "packages": [ "php81@latest", - "php81Packages.composer@latest", "php81Extensions.ds@latest" ] } diff --git a/testscripts/packages/unfree.test.txt b/testscripts/packages/unfree.test.txt index f8ae874f251..c2c107c0572 100644 --- a/testscripts/packages/unfree.test.txt +++ b/testscripts/packages/unfree.test.txt @@ -4,6 +4,6 @@ exec devbox init # we could test with slack and/or vscode. Using slack since it is lighter. exec devbox add slack -stderr 'Installing package: slack.' +stderr 'Adding package "slack@latest" to devbox.json' exec devbox rm slack From d782e6100382706c624a83da4f105c1984a6a564 Mon Sep 17 00:00:00 2001 From: Savil Srivastava <676452+savil@users.noreply.github.com> Date: Wed, 3 Jan 2024 09:38:24 -0800 Subject: [PATCH 04/11] install profile in offline mode --- internal/devbox/nixprofile.go | 9 ++++-- internal/nix/nixprofile/profile.go | 8 +++++- internal/nix/profiles.go | 28 +++++++++++++------ .../add/add_platforms_flakeref.test.txt | 10 +++++-- 4 files changed, 40 insertions(+), 15 deletions(-) diff --git a/internal/devbox/nixprofile.go b/internal/devbox/nixprofile.go index 545badb480c..751f1db9943 100644 --- a/internal/devbox/nixprofile.go +++ b/internal/devbox/nixprofile.go @@ -71,9 +71,12 @@ func (d *Devbox) syncFlakeToProfile(ctx context.Context) error { if err = nixprofile.ProfileInstall(ctx, &nixprofile.ProfileInstallArgs{ CustomStepMessage: stepMsg, Installable: addPath, - PackageName: storePath.Name, - ProfilePath: profilePath, - Writer: d.stderr, + // Install in offline mode for speed. We know we should have all the files + // locally in /nix/store since we have run `nix print-dev-env` prior to this. + Offline: true, + PackageName: storePath.Name, + ProfilePath: profilePath, + Writer: d.stderr, }); err != nil { return fmt.Errorf("error installing package %s: %w", addPath, err) } diff --git a/internal/nix/nixprofile/profile.go b/internal/nix/nixprofile/profile.go index ff0af3c2629..0e946cd1cda 100644 --- a/internal/nix/nixprofile/profile.go +++ b/internal/nix/nixprofile/profile.go @@ -251,6 +251,7 @@ func ProfileInstallPackage(ctx context.Context, args *ProfileInstallPackageArgs) type ProfileInstallArgs struct { CustomStepMessage string Installable string + Offline bool PackageName string ProfilePath string Writer io.Writer @@ -266,7 +267,12 @@ func ProfileInstall(ctx context.Context, args *ProfileInstallArgs) error { fmt.Fprintf(args.Writer, "%s\n", stepMsg) } - err := nix.ProfileInstall(ctx, args.Writer, args.ProfilePath, args.Installable) + err := nix.ProfileInstall(ctx, &nix.ProfileInstallArgs{ + Installable: args.Installable, + Offline: args.Offline, + ProfilePath: args.ProfilePath, + Writer: args.Writer, + }) if err != nil { fmt.Fprintf(args.Writer, "%s: ", stepMsg) color.New(color.FgRed).Fprintf(args.Writer, "Fail\n") diff --git a/internal/nix/profiles.go b/internal/nix/profiles.go index 7ec9707d356..072116722bf 100644 --- a/internal/nix/profiles.go +++ b/internal/nix/profiles.go @@ -30,10 +30,17 @@ func ProfileList(ctx context.Context, writer io.Writer, profilePath string, useJ return string(out), nil } -func ProfileInstall(ctx context.Context, writer io.Writer, profilePath, installable string) error { - if !IsInsecureAllowed() && PackageIsInsecure(installable) { - knownVulnerabilities := PackageKnownVulnerabilities(installable) - errString := fmt.Sprintf("Package %s is insecure. \n\n", installable) +type ProfileInstallArgs struct { + Installable string + Offline bool + ProfilePath string + Writer io.Writer +} + +func ProfileInstall(ctx context.Context, args *ProfileInstallArgs) error { + if !IsInsecureAllowed() && PackageIsInsecure(args.Installable) { + knownVulnerabilities := PackageKnownVulnerabilities(args.Installable) + errString := fmt.Sprintf("Package %s is insecure. \n\n", args.Installable) if len(knownVulnerabilities) > 0 { errString += fmt.Sprintf("Known vulnerabilities: %s \n\n", knownVulnerabilities) } @@ -44,22 +51,25 @@ func ProfileInstall(ctx context.Context, writer io.Writer, profilePath, installa cmd := commandContext( ctx, "profile", "install", - "--profile", profilePath, + "--profile", args.ProfilePath, "--impure", // for NIXPKGS_ALLOW_UNFREE // Using an arbitrary priority to avoid conflicts with other packages. // Note that this is not really the priority we care about, since we // use the flake.nix to specify the priority. - "--priority", nextPriority(profilePath), - installable, + "--priority", nextPriority(args.ProfilePath), ) + if args.Offline { + cmd.Args = append(cmd.Args, "--offline") + } + cmd.Args = append(cmd.Args, args.Installable) cmd.Env = allowUnfreeEnv(os.Environ()) // If nix profile install runs as tty, the output is much nicer. If we ever // need to change this to our own writers, consider that you may need // to implement your own nicer output. --print-build-logs flag may be useful. cmd.Stdin = os.Stdin - cmd.Stdout = writer - cmd.Stderr = writer + cmd.Stdout = args.Writer + cmd.Stderr = args.Writer debug.Log("running command: %s\n", cmd) return cmd.Run() diff --git a/testscripts/add/add_platforms_flakeref.test.txt b/testscripts/add/add_platforms_flakeref.test.txt index 345fd45e4b6..4006807c862 100644 --- a/testscripts/add/add_platforms_flakeref.test.txt +++ b/testscripts/add/add_platforms_flakeref.test.txt @@ -2,9 +2,15 @@ exec devbox install -exec devbox add github:F1bonacc1/process-compose/v0.40.2 --exclude-platform x86_64-darwin +# aside: choose armv7l-linux to verify that the add actually works on the +# current host that is unlikely to be armv7l-linux +exec devbox add github:F1bonacc1/process-compose/v0.40.2 --exclude-platform armv7l-linux json.superset devbox.json expected_devbox1.json +# verify that the package is installed on this platform +exec devbox run -- process-compose version +stdout '0.40.2' + -- devbox.json -- { "packages": [ @@ -19,7 +25,7 @@ json.superset devbox.json expected_devbox1.json "hello": "", "cowsay": "latest", "github:F1bonacc1/process-compose/v0.40.2": { - "excluded_platforms": ["x86_64-darwin"] + "excluded_platforms": ["armv7l-linux"] } } } From a859c19bc5f8193c02a6c65ed4025bedae602c93 Mon Sep 17 00:00:00 2001 From: Savil Srivastava <676452+savil@users.noreply.github.com> Date: Wed, 3 Jan 2024 10:05:20 -0800 Subject: [PATCH 05/11] ensure packages are installable, and cleanup, enable php testscript --- .github/workflows/cli-tests.yaml | 5 ++- internal/devbox/nixprofile.go | 40 +++++++++++------------ internal/devbox/packages.go | 51 +++++++++++++++++++++++++++--- internal/nix/nixprofile/profile.go | 33 ++----------------- testscripts/languages/php.test.txt | 18 ++++------- 5 files changed, 77 insertions(+), 70 deletions(-) diff --git a/.github/workflows/cli-tests.yaml b/.github/workflows/cli-tests.yaml index 21a0a2038ae..a4deb69eed5 100644 --- a/.github/workflows/cli-tests.yaml +++ b/.github/workflows/cli-tests.yaml @@ -129,9 +129,8 @@ jobs: run-project-tests: ["project-tests", "project-tests-off"] # Run tests on: # 1. the oldest supported nix version (which is 2.9.0? But determinate-systems installer has 2.12.0) - # 2. nix version 2.17.0 which introduces a new code path that minimizes nixpkgs downloads. - # 3. latest nix version - nix-version: ["2.12.0", "2.17.0", "2.19.2"] + # 2. latest nix version, must be > 2.17.0 which introduces a new code path that minimizes nixpkgs downloads. + nix-version: ["2.12.0", "2.19.2"] exclude: - is-main: "not-main" os: "${{ inputs.run-mac-tests && 'dummy' || 'macos-latest' }}" diff --git a/internal/devbox/nixprofile.go b/internal/devbox/nixprofile.go index 751f1db9943..9de41aa03af 100644 --- a/internal/devbox/nixprofile.go +++ b/internal/devbox/nixprofile.go @@ -4,13 +4,14 @@ import ( "context" "encoding/json" "fmt" - "slices" "strings" "go.jetpack.io/devbox/internal/nix" "go.jetpack.io/devbox/internal/nix/nixprofile" ) +// syncFlakeToProfile ensures the buildInputs from the flake's devShell are +// installed in the nix profile. func (d *Devbox) syncFlakeToProfile(ctx context.Context) error { profilePath, err := d.profilePath() if err != nil { @@ -73,6 +74,7 @@ func (d *Devbox) syncFlakeToProfile(ctx context.Context) error { Installable: addPath, // Install in offline mode for speed. We know we should have all the files // locally in /nix/store since we have run `nix print-dev-env` prior to this. + // Also avoids some "substituter not found for store-path" errors. Offline: true, PackageName: storePath.Name, ProfilePath: profilePath, @@ -86,30 +88,24 @@ func (d *Devbox) syncFlakeToProfile(ctx context.Context) error { } func diffStorePaths(got, want []string) (add, remove []string) { - slices.Sort(got) - slices.Sort(want) + gotSet := map[string]bool{} + for _, g := range got { + gotSet[g] = true + } + wantSet := map[string]bool{} + for _, w := range want { + wantSet[w] = true + } - var gotIdx, wantIdx int - for { - if gotIdx >= len(got) { - add = append(add, want[wantIdx:]...) - break - } - if wantIdx >= len(want) { - remove = append(remove, got[gotIdx:]...) - break + for _, g := range got { + if _, ok := wantSet[g]; !ok { + remove = append(remove, g) } + } - switch { - case got[gotIdx] == want[wantIdx]: - gotIdx++ - wantIdx++ - case got[gotIdx] < want[wantIdx]: - remove = append(remove, got[gotIdx]) - gotIdx++ - case got[gotIdx] > want[wantIdx]: - add = append(add, want[wantIdx]) - wantIdx++ + for _, w := range want { + if _, ok := gotSet[w]; !ok { + add = append(add, w) } } return add, remove diff --git a/internal/devbox/packages.go b/internal/devbox/packages.go index 904fbe2be49..b8ba2a3c529 100644 --- a/internal/devbox/packages.go +++ b/internal/devbox/packages.go @@ -266,6 +266,13 @@ func (d *Devbox) ensureStateIsUpToDate(ctx context.Context, mode installMode) er fmt.Fprintln(d.stderr, "Ensuring packages are installed.") } + // Validate packages. Must be run up-front and definitely prior to computeEnv + // and syncFlakeToProfile below that will evaluate the flake and may give + // inscrutable errors if the package is uninstallable. + if err := d.validatePackages(); err != nil { + return err + } + // Create plugin directories first because packages might need them for _, pkg := range d.InstallablePackages() { if err := d.PluginManager().Create(pkg); err != nil { @@ -285,13 +292,15 @@ func (d *Devbox) ensureStateIsUpToDate(ctx context.Context, mode installMode) er return err } - // Use the printDevEnvCache if we are adding or removing or updating any package, - // AND we are not in the shellenv-enabled environment of the current devbox-project. - usePrintDevEnvCache := mode != ensure && !d.IsEnvEnabled() - if _, err := d.computeEnv(ctx, usePrintDevEnvCache); err != nil { + // Skip the print-dev-env's cache if we are in a devbox-environment. If not, + // skip the cache if we're either installing packages or ensuring + // the project state is up-to-date. + skipPrintDevEnvCache := d.IsEnvEnabled() || (mode == ensure || mode == install) + if _, err := d.computeEnv(ctx, !skipPrintDevEnvCache /*usePrintDevEnvCache*/); err != nil { return err } + // Ensure the nix profile has the packages from the flake. if err := d.syncFlakeToProfile(ctx); err != nil { return err } @@ -379,3 +388,37 @@ func (d *Devbox) InstallRunXPackages(ctx context.Context) error { } return nil } + +// validatePackages will ensure that packages are available to be installed +// in the user's current system. +func (d *Devbox) validatePackages() error { + for _, pkg := range d.InstallablePackages() { + + inCache, err := pkg.IsInBinaryCache() + if err != nil { + return err + } + + if !inCache && nix.IsGithubNixpkgsURL(pkg.URLForFlakeInput()) { + if err := nix.EnsureNixpkgsPrefetched(d.stderr, pkg.HashFromNixPkgsURL()); err != nil { + return err + } + if exists, err := pkg.ValidateInstallsOnSystem(); err != nil { + return err + } else if !exists { + platform := nix.System() + return usererr.New( + "package %s cannot be installed on your platform %s.\n"+ + "If you know this package is incompatible with %[2]s, then "+ + "you could run `devbox add %[1]s --exclude-platform %[2]s` and re-try.\n"+ + "If you think this package should be compatible with %[2]s, then "+ + "it's possible this particular version is not available yet from the nix registry. "+ + "You could try `devbox add` with a different version for this package.\n", + pkg.Raw, + platform, + ) + } + } + } + return nil +} diff --git a/internal/nix/nixprofile/profile.go b/internal/nix/nixprofile/profile.go index 0e946cd1cda..e0b0814b46b 100644 --- a/internal/nix/nixprofile/profile.go +++ b/internal/nix/nixprofile/profile.go @@ -15,7 +15,6 @@ import ( "github.com/fatih/color" "github.com/pkg/errors" "github.com/samber/lo" - "go.jetpack.io/devbox/internal/boxcli/usererr" "go.jetpack.io/devbox/internal/devpkg" "go.jetpack.io/devbox/internal/lock" "go.jetpack.io/devbox/internal/nix" @@ -207,35 +206,8 @@ type ProfileInstallPackageArgs struct { // ProfileInstallPackage installs a Devbox package into a profile. // TODO drop the Profile prefix from this name. func ProfileInstallPackage(ctx context.Context, args *ProfileInstallPackageArgs) error { - input := devpkg.PackageFromStringWithDefaults(args.Package, args.Lockfile) - - inCache, err := input.IsInBinaryCache() - if err != nil { - return err - } - - if !inCache && nix.IsGithubNixpkgsURL(input.URLForFlakeInput()) { - if err := nix.EnsureNixpkgsPrefetched(args.Writer, input.HashFromNixPkgsURL()); err != nil { - return err - } - if exists, err := input.ValidateInstallsOnSystem(); err != nil { - return err - } else if !exists { - platform := nix.System() - return usererr.New( - "package %s cannot be installed on your platform %s.\n"+ - "If you know this package is incompatible with %[2]s, then "+ - "you could run `devbox add %[1]s --exclude-platform %[2]s` and re-try.\n"+ - "If you think this package should be compatible with %[2]s, then "+ - "it's possible this particular version is not available yet from the nix registry. "+ - "You could try `devbox add` with a different version for this package.\n", - input.Raw, - platform, - ) - } - } - - installable, err := input.Installable() + pkg := devpkg.PackageFromStringWithDefaults(args.Package, args.Lockfile) + installable, err := pkg.Installable() if err != nil { return err } @@ -244,6 +216,7 @@ func ProfileInstallPackage(ctx context.Context, args *ProfileInstallPackageArgs) Installable: installable, PackageName: args.Package, ProfilePath: args.ProfilePath, + Writer: args.Writer, }) } diff --git a/testscripts/languages/php.test.txt b/testscripts/languages/php.test.txt index f45ece1073f..16b09e97d49 100644 --- a/testscripts/languages/php.test.txt +++ b/testscripts/languages/php.test.txt @@ -1,23 +1,19 @@ exec devbox run php index.php stdout 'done\n' -# temporarily disable rm test until we fix. It requires ensuring that package -# in profile matches the flake we are installing. -# For non-flakes, this involves comparing versions. For flakes we need to compare -# the content (or hash) - -# exec devbox rm php81Extensions.ds -# exec devbox run php index.php -# stdout 'ds extension is not enabled' +exec devbox rm php81Extensions.ds +exec devbox run php index.php +stdout 'ds extension is not enabled' -# exec devbox add php81Extensions.ds -# exec devbox run php index.php -# stdout 'done\n' +exec devbox add php81Extensions.ds +exec devbox run php index.php +stdout 'done\n' -- devbox.json -- { "packages": [ "php81@latest", + "php81Packages.composer@latest", "php81Extensions.ds@latest" ] } From d312585aa0d7e772be4b586f2a0c5f7c0d7bb0cf Mon Sep 17 00:00:00 2001 From: Savil Srivastava <676452+savil@users.noreply.github.com> Date: Thu, 4 Jan 2024 12:06:56 -0800 Subject: [PATCH 06/11] remove ProfileInstallPackage --- internal/devbox/util.go | 17 +++++++++++------ internal/nix/nixprofile/profile.go | 28 ++-------------------------- 2 files changed, 13 insertions(+), 32 deletions(-) diff --git a/internal/devbox/util.go b/internal/devbox/util.go index 1771880df99..a79e89abaef 100644 --- a/internal/devbox/util.go +++ b/internal/devbox/util.go @@ -10,8 +10,9 @@ import ( "path/filepath" "github.com/pkg/errors" - "go.jetpack.io/devbox/internal/nix/nixprofile" + "go.jetpack.io/devbox/internal/devpkg" + "go.jetpack.io/devbox/internal/nix/nixprofile" "go.jetpack.io/devbox/internal/xdg" ) @@ -19,15 +20,19 @@ import ( // It's used to install applications devbox might need, like process-compose // This is an alternative to a global install which would modify a user's // environment. -func (d *Devbox) addDevboxUtilityPackage(ctx context.Context, pkg string) error { +func (d *Devbox) addDevboxUtilityPackage(ctx context.Context, pkgName string) error { + pkg := devpkg.PackageFromStringWithDefaults(pkgName, d.lockfile) + installable, err := pkg.Installable() + if err != nil { + return err + } profilePath, err := utilityNixProfilePath() if err != nil { return err } - - return nixprofile.ProfileInstallPackage(ctx, &nixprofile.ProfileInstallPackageArgs{ - Lockfile: d.lockfile, - Package: pkg, + return nixprofile.ProfileInstall(ctx, &nixprofile.ProfileInstallArgs{ + Installable: installable, + PackageName: pkgName, ProfilePath: profilePath, Writer: d.stderr, }) diff --git a/internal/nix/nixprofile/profile.go b/internal/nix/nixprofile/profile.go index e0b0814b46b..521f08495d9 100644 --- a/internal/nix/nixprofile/profile.go +++ b/internal/nix/nixprofile/profile.go @@ -194,32 +194,6 @@ func parseNixProfileListItem(line string) (*NixProfileListItem, error) { }, nil } -// TODO drop the Profile prefix from this name. -type ProfileInstallPackageArgs struct { - CustomStepMessage string - Lockfile *lock.File - Package string - ProfilePath string - Writer io.Writer -} - -// ProfileInstallPackage installs a Devbox package into a profile. -// TODO drop the Profile prefix from this name. -func ProfileInstallPackage(ctx context.Context, args *ProfileInstallPackageArgs) error { - pkg := devpkg.PackageFromStringWithDefaults(args.Package, args.Lockfile) - installable, err := pkg.Installable() - if err != nil { - return err - } - return ProfileInstall(ctx, &ProfileInstallArgs{ - CustomStepMessage: args.CustomStepMessage, - Installable: installable, - PackageName: args.Package, - ProfilePath: args.ProfilePath, - Writer: args.Writer, - }) -} - // TODO drop the Profile prefix from this name. type ProfileInstallArgs struct { CustomStepMessage string @@ -230,6 +204,8 @@ type ProfileInstallArgs struct { Writer io.Writer } +// ProfileInstall installs a package into a profile. Its a convenience wrapper around nix.ProfileInstall +// that can print nicer output when installing multiple packages. // TODO drop the Profile prefix from this name. func ProfileInstall(ctx context.Context, args *ProfileInstallArgs) error { stepMsg := args.PackageName From be2f7bdf9f518c9a682f78a2223ef18e8ce348ce Mon Sep 17 00:00:00 2001 From: Savil Srivastava <676452+savil@users.noreply.github.com> Date: Thu, 4 Jan 2024 13:42:58 -0800 Subject: [PATCH 07/11] debugging --- internal/devbox/packages.go | 3 ++- internal/nix/nixprofile/store_path.go | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) delete mode 100644 internal/nix/nixprofile/store_path.go diff --git a/internal/devbox/packages.go b/internal/devbox/packages.go index b8ba2a3c529..0f541879987 100644 --- a/internal/devbox/packages.go +++ b/internal/devbox/packages.go @@ -295,7 +295,8 @@ func (d *Devbox) ensureStateIsUpToDate(ctx context.Context, mode installMode) er // Skip the print-dev-env's cache if we are in a devbox-environment. If not, // skip the cache if we're either installing packages or ensuring // the project state is up-to-date. - skipPrintDevEnvCache := d.IsEnvEnabled() || (mode == ensure || mode == install) + skipPrintDevEnvCache := true + // skipPrintDevEnvCache := d.IsEnvEnabled() || (mode == ensure || mode == install) if _, err := d.computeEnv(ctx, !skipPrintDevEnvCache /*usePrintDevEnvCache*/); err != nil { return err } diff --git a/internal/nix/nixprofile/store_path.go b/internal/nix/nixprofile/store_path.go deleted file mode 100644 index d691897e42e..00000000000 --- a/internal/nix/nixprofile/store_path.go +++ /dev/null @@ -1 +0,0 @@ -package nixprofile From f9961219828249f14b2f29f88d8a4ee2ab18ba0e Mon Sep 17 00:00:00 2001 From: Savil Srivastava <676452+savil@users.noreply.github.com> Date: Thu, 4 Jan 2024 16:49:44 -0800 Subject: [PATCH 08/11] always recompute Devbox environment, and reuse buildInputs from print-dev-env output --- internal/devbox/nixprofile.go | 27 +++++++++------------------ internal/devbox/packages.go | 12 +++++------- internal/devpkg/validation.go | 2 +- internal/nix/eval.go | 14 ++------------ 4 files changed, 17 insertions(+), 38 deletions(-) diff --git a/internal/devbox/nixprofile.go b/internal/devbox/nixprofile.go index 9de41aa03af..cf096ebaa60 100644 --- a/internal/devbox/nixprofile.go +++ b/internal/devbox/nixprofile.go @@ -2,7 +2,6 @@ package devbox import ( "context" - "encoding/json" "fmt" "strings" @@ -12,25 +11,17 @@ import ( // syncFlakeToProfile ensures the buildInputs from the flake's devShell are // installed in the nix profile. -func (d *Devbox) syncFlakeToProfile(ctx context.Context) error { +// buildInputs is a space-separated list of store paths from the nix print-dev-env output's buildInputs. +func (d *Devbox) syncFlakeToProfile(ctx context.Context, buildInputs string) error { profilePath, err := d.profilePath() if err != nil { return err } - // Get the build inputs (i.e. store paths) from the generated flake's devShell. - buildInputPaths, err := nix.Eval( - ctx, - d.stderr, - d.flakeDir()+"#devShells."+nix.System()+".default.buildInputs", - "--json", - ) - if err != nil { - return fmt.Errorf("nix eval devShells: %v", err) - } - storePaths := []string{} - if err := json.Unmarshal(buildInputPaths, &storePaths); err != nil { - return fmt.Errorf("unmarshal store paths: %s: %v", buildInputPaths, err) + // Get the build inputs (i.e. store paths) from the generated flake's print-dev-env output + wantStorePaths := []string{} + if buildInputs != "" { // if buildInputs is empty, then we don't want wantStorePaths to be an array with a single "" entry + wantStorePaths = strings.Split(buildInputs, " ") } // Get the store-paths of the packages currently installed in the nix profile @@ -38,13 +29,13 @@ func (d *Devbox) syncFlakeToProfile(ctx context.Context) error { if err != nil { return fmt.Errorf("nix profile list: %v", err) } - got := make([]string, 0, len(items)) + gotStorePaths := make([]string, 0, len(items)) for _, item := range items { - got = append(got, item.StorePaths()...) + gotStorePaths = append(gotStorePaths, item.StorePaths()...) } // Diff the store paths and install/remove packages as needed - add, remove := diffStorePaths(got, storePaths) + add, remove := diffStorePaths(gotStorePaths, wantStorePaths) if len(remove) > 0 { packagesToRemove := make([]string, 0, len(remove)) for _, p := range remove { diff --git a/internal/devbox/packages.go b/internal/devbox/packages.go index 0f541879987..09947e1d93f 100644 --- a/internal/devbox/packages.go +++ b/internal/devbox/packages.go @@ -292,17 +292,15 @@ func (d *Devbox) ensureStateIsUpToDate(ctx context.Context, mode installMode) er return err } - // Skip the print-dev-env's cache if we are in a devbox-environment. If not, - // skip the cache if we're either installing packages or ensuring - // the project state is up-to-date. - skipPrintDevEnvCache := true - // skipPrintDevEnvCache := d.IsEnvEnabled() || (mode == ensure || mode == install) - if _, err := d.computeEnv(ctx, !skipPrintDevEnvCache /*usePrintDevEnvCache*/); err != nil { + // Always re-compute print-dev-env to ensure all packages are installed, and + // the correct set of packages are reflected in the nix-profile below. + env, err := d.computeEnv(ctx, false /*usePrintDevEnvCache*/) + if err != nil { return err } // Ensure the nix profile has the packages from the flake. - if err := d.syncFlakeToProfile(ctx); err != nil { + if err := d.syncFlakeToProfile(ctx, env["buildInputs"]); err != nil { return err } diff --git a/internal/devpkg/validation.go b/internal/devpkg/validation.go index 04a60d6a183..a43289c0082 100644 --- a/internal/devpkg/validation.go +++ b/internal/devpkg/validation.go @@ -38,7 +38,7 @@ func (p *Package) ValidateInstallsOnSystem() (bool, error) { if len(info) == 0 { return false, nil } - if out, err := nix.RawEval(u); err != nil && + if out, err := nix.Eval(u); err != nil && strings.Contains(string(out), "is not available on the requested hostPlatform") { return false, nil } diff --git a/internal/nix/eval.go b/internal/nix/eval.go index 48b3a824786..94ad821a807 100644 --- a/internal/nix/eval.go +++ b/internal/nix/eval.go @@ -1,9 +1,7 @@ package nix import ( - "context" "encoding/json" - "io" "os" "strconv" ) @@ -48,22 +46,14 @@ func PackageKnownVulnerabilities(path string) []string { return vulnerabilities } -// RawEval is nix eval. Needs to be parsed. Useful for stuff like +// Eval is raw nix eval. Needs to be parsed. Useful for stuff like // nix eval --raw nixpkgs/9ef09e06806e79e32e30d17aee6879d69c011037#fuse3 // to determine if a package if a package can be installed in system. -func RawEval(path string) ([]byte, error) { +func Eval(path string) ([]byte, error) { cmd := command("eval", "--raw", path) return cmd.CombinedOutput() } -func Eval(ctx context.Context, w io.Writer, installable string, options ...string) ([]byte, error) { - args := []string{"eval", installable} - args = append(args, options...) - cmd := commandContext(ctx, args...) - cmd.Stderr = w - return cmd.Output() -} - func AllowInsecurePackages() { os.Setenv("NIXPKGS_ALLOW_INSECURE", "1") } From 4fb3d1405873f1b171b43eec0157e78bc6be61b3 Mon Sep 17 00:00:00 2001 From: Savil Srivastava <676452+savil@users.noreply.github.com> Date: Fri, 5 Jan 2024 11:46:28 -0800 Subject: [PATCH 09/11] improve validatePackages to only operate on packages to be installed --- internal/devbox/nixprofile.go | 13 ++------ internal/devbox/packages.go | 57 +++++++++++++++++++++++++++++++---- 2 files changed, 54 insertions(+), 16 deletions(-) diff --git a/internal/devbox/nixprofile.go b/internal/devbox/nixprofile.go index cf096ebaa60..d6d7bc581ad 100644 --- a/internal/devbox/nixprofile.go +++ b/internal/devbox/nixprofile.go @@ -9,21 +9,14 @@ import ( "go.jetpack.io/devbox/internal/nix/nixprofile" ) -// syncFlakeToProfile ensures the buildInputs from the flake's devShell are -// installed in the nix profile. -// buildInputs is a space-separated list of store paths from the nix print-dev-env output's buildInputs. -func (d *Devbox) syncFlakeToProfile(ctx context.Context, buildInputs string) error { +// syncNixProfile ensures the nix profile has the packages specified in wantStorePaths. +// It also removes any packages from the nix profile that are not in wantStorePaths. +func (d *Devbox) syncNixProfile(ctx context.Context, wantStorePaths []string) error { profilePath, err := d.profilePath() if err != nil { return err } - // Get the build inputs (i.e. store paths) from the generated flake's print-dev-env output - wantStorePaths := []string{} - if buildInputs != "" { // if buildInputs is empty, then we don't want wantStorePaths to be an array with a single "" entry - wantStorePaths = strings.Split(buildInputs, " ") - } - // Get the store-paths of the packages currently installed in the nix profile items, err := nixprofile.ProfileListItems(ctx, d.stderr, profilePath) if err != nil { diff --git a/internal/devbox/packages.go b/internal/devbox/packages.go index 09947e1d93f..e78c5b10d50 100644 --- a/internal/devbox/packages.go +++ b/internal/devbox/packages.go @@ -18,6 +18,7 @@ import ( "go.jetpack.io/devbox/internal/devbox/devopt" "go.jetpack.io/devbox/internal/devpkg" "go.jetpack.io/devbox/internal/devpkg/pkgtype" + "go.jetpack.io/devbox/internal/nix/nixprofile" "go.jetpack.io/devbox/internal/shellgen" "go.jetpack.io/devbox/internal/boxcli/usererr" @@ -267,9 +268,9 @@ func (d *Devbox) ensureStateIsUpToDate(ctx context.Context, mode installMode) er } // Validate packages. Must be run up-front and definitely prior to computeEnv - // and syncFlakeToProfile below that will evaluate the flake and may give + // and syncNixProfile below that will evaluate the flake and may give // inscrutable errors if the package is uninstallable. - if err := d.validatePackages(); err != nil { + if err := d.validatePackagesToBeInstalled(ctx); err != nil { return err } @@ -300,7 +301,13 @@ func (d *Devbox) ensureStateIsUpToDate(ctx context.Context, mode installMode) er } // Ensure the nix profile has the packages from the flake. - if err := d.syncFlakeToProfile(ctx, env["buildInputs"]); err != nil { + buildInputs := []string{} + if env["buildInputs"] != "" { + // env["buildInputs"] can be empty string if there are no packages in the project + // if buildInputs is empty, then we don't want wantStorePaths to be an array with a single "" entry + buildInputs = strings.Split(env["buildInputs"], " ") + } + if err := d.syncNixProfile(ctx, buildInputs); err != nil { return err } @@ -388,11 +395,49 @@ func (d *Devbox) InstallRunXPackages(ctx context.Context) error { return nil } -// validatePackages will ensure that packages are available to be installed +// validatePackagesToBeInstalled will ensure that packages are available to be installed // in the user's current system. -func (d *Devbox) validatePackages() error { - for _, pkg := range d.InstallablePackages() { +func (d *Devbox) validatePackagesToBeInstalled(ctx context.Context) error { + // First, fetch the profile items from the nix-profile, + profileDir, err := d.profilePath() + if err != nil { + return err + } + profileItems, err := nixprofile.ProfileListItems(ctx, d.stderr, profileDir) + if err != nil { + return err + } + + // Second, get and prepare all the packages that must be installed in this project + packages, err := d.AllInstallablePackages() + if err != nil { + return err + } + packages = lo.Filter(packages, devpkg.IsNix) // Remove non-nix packages from the list + if err := devpkg.FillNarInfoCache(ctx, packages...); err != nil { + return err + } + + // Third, compute which packages need to be installed + packagesToInstall := []*devpkg.Package{} + // Note: because devpkg.Package uses memoization when normalizing attribute paths (slow operation), + // and since we're reusing the Package objects, this O(n*m) loop becomes O(n+m) wrt the slow operation. + for _, pkg := range packages { + found := false + for _, item := range profileItems { + if item.Matches(pkg, d.lockfile) { + found = true + break + } + } + if !found { + packagesToInstall = append(packagesToInstall, pkg) + } + } + // Last, validate that packages that need to be installed are in fact installable + // on the user's current system. + for _, pkg := range packagesToInstall { inCache, err := pkg.IsInBinaryCache() if err != nil { return err From 1f418d59d023ee05956fa537003a6e462d9e34fd Mon Sep 17 00:00:00 2001 From: Savil Srivastava <676452+savil@users.noreply.github.com> Date: Tue, 9 Jan 2024 16:58:20 -0800 Subject: [PATCH 10/11] install packages via nix build --- internal/devbox/packages.go | 90 ++++++++++++++++++++++++----- internal/lock/local.go | 9 +++ internal/lock/lockfile.go | 6 ++ internal/nix/build.go | 29 ++++++++++ testscripts/add/global_add.test.txt | 2 +- 5 files changed, 119 insertions(+), 17 deletions(-) create mode 100644 internal/nix/build.go diff --git a/internal/devbox/packages.go b/internal/devbox/packages.go index e78c5b10d50..e4fad6bb543 100644 --- a/internal/devbox/packages.go +++ b/internal/devbox/packages.go @@ -248,10 +248,23 @@ const ( // The `mode` is used for: // 1. Skipping certain operations that may not apply. // 2. User messaging to explain what operations are happening, because this function may take time to execute. -func (d *Devbox) ensureStateIsUpToDate(ctx context.Context, mode installMode) error { +// +// nolint:revive to turn off linter complaining about function complexity +func (d *Devbox) ensureStateIsUpToDate(ctx context.Context, mode installMode) error { //nolint:revive defer trace.StartRegion(ctx, "devboxEnsureStateIsUpToDate").End() defer debug.FunctionTimer().End() + if mode != ensure && !d.IsEnvEnabled() { + // if mode is install/uninstall/update and we are not in a devbox environment, + // then we skip some operations below for speed. + // Remove the local.lock file so that we re-compute the project state when + // we are in the devbox environment again. + fmt.Printf("mode is not ensure and env is not enabled, so will remove local.lock on exit\n") + + defer func() { _ = d.lockfile.RemoveLocal() }() + // return nil + } + // if mode is install or uninstall, then we need to update the nix-profile // and lockfile, so we must continue below. upToDate, err := d.lockfile.IsUpToDateAndInstalled() @@ -262,10 +275,12 @@ func (d *Devbox) ensureStateIsUpToDate(ctx context.Context, mode installMode) er if mode == ensure { // if mode is ensure and we are up to date, then we can skip the rest if upToDate { + fmt.Printf("state is up to date. Returning early\n") return nil } fmt.Fprintln(d.stderr, "Ensuring packages are installed.") } + fmt.Printf("state is not up to date. Continuing...\n") // Validate packages. Must be run up-front and definitely prior to computeEnv // and syncNixProfile below that will evaluate the flake and may give @@ -285,6 +300,9 @@ func (d *Devbox) ensureStateIsUpToDate(ctx context.Context, mode installMode) er return err } + fmt.Printf("mode is %s\n", mode) + fmt.Printf("env is enabled: %v\n", d.IsEnvEnabled()) + if err := shellgen.GenerateForPrintEnv(ctx, d); err != nil { return err } @@ -293,24 +311,37 @@ func (d *Devbox) ensureStateIsUpToDate(ctx context.Context, mode installMode) er return err } - // Always re-compute print-dev-env to ensure all packages are installed, and - // the correct set of packages are reflected in the nix-profile below. - env, err := d.computeEnv(ctx, false /*usePrintDevEnvCache*/) - if err != nil { - return err - } + // The steps contained in this if-block of computeEnv and syncNixProfile are a tad + // slow. So, we only do it if we are in a devbox environment, or if mode is ensure. + if mode == ensure || d.IsEnvEnabled() { + // Re-compute print-dev-env to ensure all packages are installed, and + // the correct set of packages are reflected in the nix-profile below. + env, err := d.computeEnv(ctx, false /*usePrintDevEnvCache*/) + if err != nil { + return err + } - // Ensure the nix profile has the packages from the flake. - buildInputs := []string{} - if env["buildInputs"] != "" { - // env["buildInputs"] can be empty string if there are no packages in the project - // if buildInputs is empty, then we don't want wantStorePaths to be an array with a single "" entry - buildInputs = strings.Split(env["buildInputs"], " ") - } - if err := d.syncNixProfile(ctx, buildInputs); err != nil { - return err + // Ensure the nix profile has the packages from the flake. + buildInputs := []string{} + if env["buildInputs"] != "" { + // env["buildInputs"] can be empty string if there are no packages in the project + // if buildInputs is empty, then we don't want wantStorePaths to be an array with a single "" entry + buildInputs = strings.Split(env["buildInputs"], " ") + } + if err := d.syncNixProfile(ctx, buildInputs); err != nil { + return err + } + + } else if mode == install || mode == update { + // Else: if we are not in a devbox environment, and we are installing or updating + // then we must ensure the new nix packages are in the nix store. This way, the + // next time we enter a devbox environment, we will have the packages available locally. + if err := d.installNixPackagesToStore(ctx); err != nil { + return err + } } + fmt.Printf("calling lockfile.tidy\n") // Ensure we clean out packages that are no longer needed. d.lockfile.Tidy() @@ -395,6 +426,33 @@ func (d *Devbox) InstallRunXPackages(ctx context.Context) error { return nil } +// installNixPackagesToStore will install all the packages in the nix store, if +// mode is install or update, and we're not in a devbox environment. +// This is done by running `nix build` on the flake +func (d *Devbox) installNixPackagesToStore(ctx context.Context) error { + packages, err := d.AllInstallablePackages() + if err != nil { + return err + } + packages = lo.Filter(packages, devpkg.IsNix) // Remove non-nix packages from the list + + installables := []string{} + for _, pkg := range packages { + i, err := pkg.Installable() + if err != nil { + return err + } + installables = append(installables, i) + } + + if len(installables) == 0 { + return nil + } + + // --no-link to avoid generating the result objects + return nix.Build(ctx, []string{"--no-link"}, installables...) +} + // validatePackagesToBeInstalled will ensure that packages are available to be installed // in the user's current system. func (d *Devbox) validatePackagesToBeInstalled(ctx context.Context) error { diff --git a/internal/lock/local.go b/internal/lock/local.go index 65b490eefc9..b89a7ce5a65 100644 --- a/internal/lock/local.go +++ b/internal/lock/local.go @@ -5,7 +5,9 @@ package lock import ( "errors" + "fmt" "io/fs" + "os" "path/filepath" "go.jetpack.io/devbox/internal/build" @@ -49,6 +51,8 @@ func isLocalUpToDate(project devboxProject) (bool, error) { } func updateLocal(project devboxProject) error { + fmt.Printf("updating local.lock\n") + // debug.PrintStack() l, err := readLocal(project) if err != nil { return err @@ -74,6 +78,11 @@ func readLocal(project devboxProject) (*localLockFile, error) { return lockFile, nil } +func removeLocal(project devboxProject) error { + // RemoveAll to avoid error in case file does not exist. + return os.RemoveAll(localLockFilePath(project)) +} + func forProject(project devboxProject) (*localLockFile, error) { configHash, err := project.ConfigHash() if err != nil { diff --git a/internal/lock/lockfile.go b/internal/lock/lockfile.go index f7d4fa7cfdf..86f6548fa53 100644 --- a/internal/lock/lockfile.go +++ b/internal/lock/lockfile.go @@ -55,6 +55,7 @@ func (f *File) Add(pkgs ...string) error { return err } } + fmt.Printf("calling lockfile.Add with pkgs: %v\n", pkgs) return f.Save() } @@ -176,6 +177,11 @@ func (f *File) isDirty() (bool, error) { return currentHash != filesystemHash, nil } +// RemoveLocal removes the local.lock file +func (f *File) RemoveLocal() error { + return removeLocal(f.devboxProject) +} + func lockFilePath(project devboxProject) string { return filepath.Join(project.ProjectDir(), "devbox.lock") } diff --git a/internal/nix/build.go b/internal/nix/build.go new file mode 100644 index 00000000000..4493ca6a40a --- /dev/null +++ b/internal/nix/build.go @@ -0,0 +1,29 @@ +package nix + +import ( + "context" + "fmt" + "os/exec" + + "github.com/pkg/errors" +) + +func Build(ctx context.Context, flags []string, installables ...string) error { + // --impure is required for allowUnfreeEnv to work. + cmd := commandContext(ctx, "build", "--impure") + cmd.Args = append(cmd.Args, flags...) + cmd.Args = append(cmd.Args, installables...) + // We need to allow Unfree packages to be installed. We choose to not also add os.Environ() to the environment + // to keep the command as pure as possible, even though we must pass --impure to nix build. + cmd.Env = allowUnfreeEnv([]string{}) // allowUnfreeEnv(os.Environ()) + + out, err := cmd.Output() + if err != nil { + if exitErr := (&exec.ExitError{}); errors.As(err, &exitErr) { + fmt.Printf("Exit code: %d, output: %s\n", exitErr.ExitCode(), exitErr.Stderr) + } + return err + } + fmt.Printf("Ran nix build, output: %s\n", out) + return nil +} diff --git a/testscripts/add/global_add.test.txt b/testscripts/add/global_add.test.txt index 0bb0ebc5fb8..373ebbde7b1 100644 --- a/testscripts/add/global_add.test.txt +++ b/testscripts/add/global_add.test.txt @@ -4,7 +4,7 @@ ! exec vim --version exec devbox global add ripgrep vim -exec devbox global shellenv +exec devbox global shellenv --recompute source.path exec rg --version exec vim --version From 8521fa47b4bfdb2a6cc8ec0130748c8c4523fc25 Mon Sep 17 00:00:00 2001 From: Savil Srivastava <676452+savil@users.noreply.github.com> Date: Wed, 10 Jan 2024 15:38:14 -0800 Subject: [PATCH 11/11] code review feedback fixes --- internal/devbox/nixprofile.go | 32 ++--------- internal/devbox/packages.go | 99 ++++++++++++++++++----------------- internal/lock/local.go | 3 -- internal/lock/lockfile.go | 1 - internal/nix/build.go | 10 ++-- 5 files changed, 62 insertions(+), 83 deletions(-) diff --git a/internal/devbox/nixprofile.go b/internal/devbox/nixprofile.go index d6d7bc581ad..5414051897e 100644 --- a/internal/devbox/nixprofile.go +++ b/internal/devbox/nixprofile.go @@ -5,8 +5,10 @@ import ( "fmt" "strings" + "github.com/samber/lo" "go.jetpack.io/devbox/internal/nix" "go.jetpack.io/devbox/internal/nix/nixprofile" + "go.jetpack.io/devbox/internal/ux" ) // syncNixProfile ensures the nix profile has the packages specified in wantStorePaths. @@ -28,7 +30,7 @@ func (d *Devbox) syncNixProfile(ctx context.Context, wantStorePaths []string) er } // Diff the store paths and install/remove packages as needed - add, remove := diffStorePaths(gotStorePaths, wantStorePaths) + remove, add := lo.Difference(gotStorePaths, wantStorePaths) if len(remove) > 0 { packagesToRemove := make([]string, 0, len(remove)) for _, p := range remove { @@ -36,9 +38,9 @@ func (d *Devbox) syncNixProfile(ctx context.Context, wantStorePaths []string) er packagesToRemove = append(packagesToRemove, fmt.Sprintf("%s@%s", storePath.Name, storePath.Version)) } if len(packagesToRemove) == 1 { - fmt.Fprintf(d.stderr, "Removing %s\n", strings.Join(packagesToRemove, ", ")) + ux.Finfo(d.stderr, "Removing %s\n", strings.Join(packagesToRemove, ", ")) } else { - fmt.Fprintf(d.stderr, "Removing packages: %s\n", strings.Join(packagesToRemove, ", ")) + ux.Finfo(d.stderr, "Removing packages: %s\n", strings.Join(packagesToRemove, ", ")) } if err := nix.ProfileRemove(profilePath, remove...); err != nil { @@ -70,27 +72,3 @@ func (d *Devbox) syncNixProfile(ctx context.Context, wantStorePaths []string) er } return nil } - -func diffStorePaths(got, want []string) (add, remove []string) { - gotSet := map[string]bool{} - for _, g := range got { - gotSet[g] = true - } - wantSet := map[string]bool{} - for _, w := range want { - wantSet[w] = true - } - - for _, g := range got { - if _, ok := wantSet[g]; !ok { - remove = append(remove, g) - } - } - - for _, w := range want { - if _, ok := gotSet[w]; !ok { - add = append(add, w) - } - } - return add, remove -} diff --git a/internal/devbox/packages.go b/internal/devbox/packages.go index e4fad6bb543..9f581269333 100644 --- a/internal/devbox/packages.go +++ b/internal/devbox/packages.go @@ -249,7 +249,7 @@ const ( // 1. Skipping certain operations that may not apply. // 2. User messaging to explain what operations are happening, because this function may take time to execute. // -// nolint:revive to turn off linter complaining about function complexity +// linter:revive is turned off due to complaining about function complexity func (d *Devbox) ensureStateIsUpToDate(ctx context.Context, mode installMode) error { //nolint:revive defer trace.StartRegion(ctx, "devboxEnsureStateIsUpToDate").End() defer debug.FunctionTimer().End() @@ -259,10 +259,7 @@ func (d *Devbox) ensureStateIsUpToDate(ctx context.Context, mode installMode) er // then we skip some operations below for speed. // Remove the local.lock file so that we re-compute the project state when // we are in the devbox environment again. - fmt.Printf("mode is not ensure and env is not enabled, so will remove local.lock on exit\n") - defer func() { _ = d.lockfile.RemoveLocal() }() - // return nil } // if mode is install or uninstall, then we need to update the nix-profile @@ -275,12 +272,10 @@ func (d *Devbox) ensureStateIsUpToDate(ctx context.Context, mode installMode) er if mode == ensure { // if mode is ensure and we are up to date, then we can skip the rest if upToDate { - fmt.Printf("state is up to date. Returning early\n") return nil } fmt.Fprintln(d.stderr, "Ensuring packages are installed.") } - fmt.Printf("state is not up to date. Continuing...\n") // Validate packages. Must be run up-front and definitely prior to computeEnv // and syncNixProfile below that will evaluate the flake and may give @@ -300,9 +295,6 @@ func (d *Devbox) ensureStateIsUpToDate(ctx context.Context, mode installMode) er return err } - fmt.Printf("mode is %s\n", mode) - fmt.Printf("env is enabled: %v\n", d.IsEnvEnabled()) - if err := shellgen.GenerateForPrintEnv(ctx, d); err != nil { return err } @@ -341,7 +333,6 @@ func (d *Devbox) ensureStateIsUpToDate(ctx context.Context, mode installMode) er } } - fmt.Printf("calling lockfile.tidy\n") // Ensure we clean out packages that are no longer needed. d.lockfile.Tidy() @@ -428,14 +419,16 @@ func (d *Devbox) InstallRunXPackages(ctx context.Context) error { // installNixPackagesToStore will install all the packages in the nix store, if // mode is install or update, and we're not in a devbox environment. -// This is done by running `nix build` on the flake +// This is done by running `nix build` on the flake. We do this so that the +// packages will be available in the nix store when computing the devbox environment +// and installing in the nix profile (even if offline). func (d *Devbox) installNixPackagesToStore(ctx context.Context) error { - packages, err := d.AllInstallablePackages() + packages, err := d.packagesToInstallInProfile(ctx) if err != nil { return err } - packages = lo.Filter(packages, devpkg.IsNix) // Remove non-nix packages from the list + names := []string{} installables := []string{} for _, pkg := range packages { i, err := pkg.Installable() @@ -443,12 +436,15 @@ func (d *Devbox) installNixPackagesToStore(ctx context.Context) error { return err } installables = append(installables, i) + names = append(names, pkg.String()) } if len(installables) == 0 { return nil } + ux.Finfo(d.stderr, "Installing to the nix store: %s. This may take a brief while.\n", strings.Join(names, " ")) + // --no-link to avoid generating the result objects return nix.Build(ctx, []string{"--no-link"}, installables...) } @@ -456,44 +452,13 @@ func (d *Devbox) installNixPackagesToStore(ctx context.Context) error { // validatePackagesToBeInstalled will ensure that packages are available to be installed // in the user's current system. func (d *Devbox) validatePackagesToBeInstalled(ctx context.Context) error { - // First, fetch the profile items from the nix-profile, - profileDir, err := d.profilePath() - if err != nil { - return err - } - profileItems, err := nixprofile.ProfileListItems(ctx, d.stderr, profileDir) + // First, get the packages to install + packagesToInstall, err := d.packagesToInstallInProfile(ctx) if err != nil { return err } - // Second, get and prepare all the packages that must be installed in this project - packages, err := d.AllInstallablePackages() - if err != nil { - return err - } - packages = lo.Filter(packages, devpkg.IsNix) // Remove non-nix packages from the list - if err := devpkg.FillNarInfoCache(ctx, packages...); err != nil { - return err - } - - // Third, compute which packages need to be installed - packagesToInstall := []*devpkg.Package{} - // Note: because devpkg.Package uses memoization when normalizing attribute paths (slow operation), - // and since we're reusing the Package objects, this O(n*m) loop becomes O(n+m) wrt the slow operation. - for _, pkg := range packages { - found := false - for _, item := range profileItems { - if item.Matches(pkg, d.lockfile) { - found = true - break - } - } - if !found { - packagesToInstall = append(packagesToInstall, pkg) - } - } - - // Last, validate that packages that need to be installed are in fact installable + // Then, validate that packages that need to be installed are in fact installable // on the user's current system. for _, pkg := range packagesToInstall { inCache, err := pkg.IsInBinaryCache() @@ -524,3 +489,43 @@ func (d *Devbox) validatePackagesToBeInstalled(ctx context.Context) error { } return nil } + +func (d *Devbox) packagesToInstallInProfile(ctx context.Context) ([]*devpkg.Package, error) { + // First, fetch the profile items from the nix-profile, + profileDir, err := d.profilePath() + if err != nil { + return nil, err + } + profileItems, err := nixprofile.ProfileListItems(ctx, d.stderr, profileDir) + if err != nil { + return nil, err + } + + // Second, get and prepare all the packages that must be installed in this project + packages, err := d.AllInstallablePackages() + if err != nil { + return nil, err + } + packages = lo.Filter(packages, devpkg.IsNix) // Remove non-nix packages from the list + if err := devpkg.FillNarInfoCache(ctx, packages...); err != nil { + return nil, err + } + + // Third, compute which packages need to be installed + packagesToInstall := []*devpkg.Package{} + // Note: because devpkg.Package uses memoization when normalizing attribute paths (slow operation), + // and since we're reusing the Package objects, this O(n*m) loop becomes O(n+m) wrt the slow operation. + for _, pkg := range packages { + found := false + for _, item := range profileItems { + if item.Matches(pkg, d.lockfile) { + found = true + break + } + } + if !found { + packagesToInstall = append(packagesToInstall, pkg) + } + } + return packagesToInstall, nil +} diff --git a/internal/lock/local.go b/internal/lock/local.go index b89a7ce5a65..cefc4cb2a11 100644 --- a/internal/lock/local.go +++ b/internal/lock/local.go @@ -5,7 +5,6 @@ package lock import ( "errors" - "fmt" "io/fs" "os" "path/filepath" @@ -51,8 +50,6 @@ func isLocalUpToDate(project devboxProject) (bool, error) { } func updateLocal(project devboxProject) error { - fmt.Printf("updating local.lock\n") - // debug.PrintStack() l, err := readLocal(project) if err != nil { return err diff --git a/internal/lock/lockfile.go b/internal/lock/lockfile.go index 86f6548fa53..5e76b9a14e9 100644 --- a/internal/lock/lockfile.go +++ b/internal/lock/lockfile.go @@ -55,7 +55,6 @@ func (f *File) Add(pkgs ...string) error { return err } } - fmt.Printf("calling lockfile.Add with pkgs: %v\n", pkgs) return f.Save() } diff --git a/internal/nix/build.go b/internal/nix/build.go index 4493ca6a40a..3211cec26ab 100644 --- a/internal/nix/build.go +++ b/internal/nix/build.go @@ -2,10 +2,10 @@ package nix import ( "context" - "fmt" "os/exec" "github.com/pkg/errors" + "go.jetpack.io/devbox/internal/debug" ) func Build(ctx context.Context, flags []string, installables ...string) error { @@ -15,15 +15,15 @@ func Build(ctx context.Context, flags []string, installables ...string) error { cmd.Args = append(cmd.Args, installables...) // We need to allow Unfree packages to be installed. We choose to not also add os.Environ() to the environment // to keep the command as pure as possible, even though we must pass --impure to nix build. - cmd.Env = allowUnfreeEnv([]string{}) // allowUnfreeEnv(os.Environ()) + cmd.Env = allowUnfreeEnv([]string{}) - out, err := cmd.Output() + debug.Log("Running cmd: %s\n", cmd) + _, err := cmd.Output() if err != nil { if exitErr := (&exec.ExitError{}); errors.As(err, &exitErr) { - fmt.Printf("Exit code: %d, output: %s\n", exitErr.ExitCode(), exitErr.Stderr) + debug.Log("Nix build exit code: %d, output: %s\n", exitErr.ExitCode(), exitErr.Stderr) } return err } - fmt.Printf("Ran nix build, output: %s\n", out) return nil }