[UX] Better dedup packages we nix build, and improve UX printouts (#1827)

## Summary

We have two separate steps for "installing packages".
1. We use nix build to add to the nix store.
2. We later ensure the nix profile is updated to have items pointing to
the nix store packages.

For the first step, we should only run `nix build` if the package's
outputs are not in the nix store. For this, we check `nix store ls
<path>`. We get `<path>` from `nix path-info <installable>`.

Previously, we were checking if the packages were in the `nix profile`,
rather than directly querying the nix store. This would lead to
re-running `nix build` for when we've done `devbox add` outside a
devbox-environment, and then started a devbox-environment (example:
doing `devbox add` followed by `devbox shell`).

For the second step, we can remove the `[1/3] <package>` being removed
print outs. This was already removed for packages being added. Reason:
this step should be quick! We've already added the packages to the nix
store via `nix build`. It also confuses users who are likely not aware
of a `nix profile` concept, nor should we make them aware of it.

**Alternatives:**
Also tried `nix build --profile <path> <installable>` but this doesn't
seem to return the installed package in `nix profile list`. Not sure
why, but it doesmean its not useful for de-duplicating packages that
remain to be installed.


**Upside:**
Ths store-path deduping is great. Works much faster. Re-doing `devbox
add <package>` for a package already in `/nix/store` is much faster
since we skip the `nix build` step entirely. As it should be.

**Downside:**
A user adding or removing packages directly from editing the
`devbox.json` will not see any nice per-package-steps installation
output. (Note, this was changed when adding nix-proflile-from-flake, but
mentioning for completeness of analyzing the UX)

## How was it tested?

`devbox add` a package already in store no longer shows the per-package
installation steps
`devbox shell` subsequently does not again show we are adding the
package.

`devbox add` a package not in store does show the per-package-steps
installation output.

Need to do more rigorous testing:
- [ ] add a local flake
- [ ] add a remote flake

Author: savil

internal/devbox/nixprofile.go

@@ -6,9 +6,9 @@ import (
 	"strings"
 
 	"github.com/samber/lo"
+	"go.jetpack.io/devbox/internal/debug"
 	"go.jetpack.io/devbox/internal/nix"
 	"go.jetpack.io/devbox/internal/nix/nixprofile"
-	"go.jetpack.io/devbox/internal/ux"
 )
 
 // syncNixProfileFromFlake ensures the nix profile has the packages from the buildInputs
@@ -53,11 +53,7 @@ func (d *Devbox) syncNixProfileFromFlake(ctx context.Context) error {
 			storePath := nix.NewStorePathParts(p)
 			packagesToRemove = append(packagesToRemove, fmt.Sprintf("%s@%s", storePath.Name, storePath.Version))
 		}
-		if len(packagesToRemove) == 1 {
-			ux.Finfo(d.stderr, "Removing %s\n", strings.Join(packagesToRemove, ", "))
-		} else {
-			ux.Finfo(d.stderr, "Removing packages: %s\n", strings.Join(packagesToRemove, ", "))
-		}
+		debug.Log("Removing packages from nix profile: %s\n", strings.Join(packagesToRemove, ", "))
 
 		if err := nix.ProfileRemove(profilePath, remove...); err != nil {
 			return err

internal/devbox/packages.go

@@ -422,7 +422,7 @@ func (d *Devbox) InstallRunXPackages(ctx context.Context) error {
 // 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.packagesToInstallInProfile(ctx)
+	packages, err := d.packagesToInstallInStore(ctx)
 	if err != nil {
 		return err
 	}
@@ -489,7 +489,7 @@ func (d *Devbox) installNixPackagesToStore(ctx context.Context) error {
 	return err
 }
 
-func (d *Devbox) packagesToInstallInProfile(ctx context.Context) ([]*devpkg.Package, error) {
+func (d *Devbox) packagesToInstallInStore(ctx context.Context) ([]*devpkg.Package, error) {
 	// First, fetch the profile items from the nix-profile,
 	profileDir, err := d.profilePath()
 	if err != nil {
@@ -511,7 +511,7 @@ func (d *Devbox) packagesToInstallInProfile(ctx context.Context) ([]*devpkg.Pack
 	}
 
 	// Third, compute which packages need to be installed
-	packagesToInstall := []*devpkg.Package{}
+	packagesNotInProfile := []*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 {
@@ -523,9 +523,29 @@ func (d *Devbox) packagesToInstallInProfile(ctx context.Context) ([]*devpkg.Pack
 			}
 		}
 		if !found {
+			packagesNotInProfile = append(packagesNotInProfile, pkg)
+		}
+	}
+
+	packagesToInstall := []*devpkg.Package{}
+	for _, pkg := range packagesNotInProfile {
+		installable, err := pkg.Installable()
+		if err != nil {
+			return nil, err
+		}
+		storePaths, err := nix.StorePathsFromInstallable(ctx, installable)
+		if err != nil {
+			return nil, err
+		}
+		isInStore, err := nix.StorePathsAreInStore(ctx, storePaths)
+		if err != nil {
+			return nil, err
+		}
+		if !isInStore {
 			packagesToInstall = append(packagesToInstall, pkg)
 		}
 	}
+
 	return packagesToInstall, nil
 }
 

internal/nix/store.go

@@ -2,7 +2,15 @@ package nix
 
 import (
 	"context"
+	"encoding/json"
+	"errors"
+	"fmt"
+	"os"
+	"os/exec"
 	"strings"
+
+	"go.jetpack.io/devbox/internal/debug"
+	"golang.org/x/exp/maps"
 )
 
 func StorePathFromHashPart(ctx context.Context, hash, storeAddr string) (string, error) {
@@ -13,3 +21,56 @@ func StorePathFromHashPart(ctx context.Context, hash, storeAddr string) (string,
 	}
 	return strings.TrimSpace(string(resultBytes)), nil
 }
+
+func StorePathsFromInstallable(ctx context.Context, installable string) ([]string, error) {
+	// --impure for NIXPKGS_ALLOW_UNFREE
+	cmd := commandContext(ctx, "path-info", installable, "--json", "--impure")
+	cmd.Env = allowUnfreeEnv(os.Environ())
+	debug.Log("Running cmd %s", cmd)
+	resultBytes, err := cmd.Output()
+	if err != nil {
+		return nil, err
+	}
+	return parseStorePathFromInstallableOutput(installable, resultBytes)
+}
+
+// StorePathAreInStore returns true if the store path is in the store
+// It relies on `nix store ls` to check if the store path is in the store
+func StorePathsAreInStore(ctx context.Context, storePaths []string) (bool, error) {
+	for _, storePath := range storePaths {
+		cmd := commandContext(ctx, "store", "ls", storePath)
+		debug.Log("Running cmd %s", cmd)
+		if err := cmd.Run(); err != nil {
+			if exitErr := (&exec.ExitError{}); errors.As(err, &exitErr) {
+				return false, nil
+			}
+			return false, err
+		}
+	}
+	return true, nil
+}
+
+// parseStorePathFromInstallableOutput parses the output of `nix store path-from-installable --json`
+// This function is decomposed out of StorePathFromInstallable to make it testable.
+func parseStorePathFromInstallableOutput(installable string, output []byte) ([]string, error) {
+	// Newer nix versions (like 2.20)
+	var out1 map[string]any
+	if err := json.Unmarshal(output, &out1); err == nil {
+		return maps.Keys(out1), nil
+	}
+
+	// Older nix versions (like 2.17)
+	var out2 []struct {
+		Path  string `json:"path"`
+		Valid bool   `json:"valid"`
+	}
+	if err := json.Unmarshal(output, &out2); err == nil {
+		res := []string{}
+		for _, outValue := range out2 {
+			res = append(res, outValue.Path)
+		}
+		return res, nil
+	}
+
+	return nil, fmt.Errorf("failed to parse store path from installable (%s) output: %s", installable, output)
+}

internal/nix/store_test.go

@@ -1 +1,38 @@
 package nix
+
+import (
+	"slices"
+	"testing"
+)
+
+func TestParseStorePathFromInstallableOutput(t *testing.T) {
+	testCases := []struct {
+		name     string
+		input    string
+		expected []string
+	}{
+		{
+			name: "go-basic-nix-2-20-1",
+			// snipped the actual output for brevity. We mainly care about the first key in the JSON.
+			input:    `{"/nix/store/fgkl3qk8p5hnd07b0dhzfky3ys5gxjmq-go-1.22.0":{"deriver":"/nix/store/clr3bm8njqysvyw4r4x4xmldhz4knrff-go-1.22.0.drv"}}`,
+			expected: []string{"/nix/store/fgkl3qk8p5hnd07b0dhzfky3ys5gxjmq-go-1.22.0"},
+		},
+		{
+			name:     "go-basic-nix-2-17-0",
+			input:    `[{"path":"/nix/store/fgkl3qk8p5hnd07b0dhzfky3ys5gxjmq-go-1.22.0","valid":false}]`,
+			expected: []string{"/nix/store/fgkl3qk8p5hnd07b0dhzfky3ys5gxjmq-go-1.22.0"},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			actual, err := parseStorePathFromInstallableOutput(tc.name, []byte(tc.input))
+			if err != nil {
+				t.Errorf("Expected no error but got error: %s", err)
+			}
+			if !slices.Equal(tc.expected, actual) {
+				t.Errorf("Expected store path %s but got %s", tc.expected, actual)
+			}
+		})
+	}
+}

typos.toml

@@ -8,4 +8,5 @@ extend-exclude=[
   "**/testdata/**", 
   "internal/cachehash/hash_test.go", 
   "internal/devpkg/package_test.go",
+  "internal/nix/store_test.go",
 ]