[nix.System] call EnsureNixInstalled from devbox.Open, and cleanup function API (#1374)

## Summary

`nix.System` is widely used. So, in #1357 we wanted to change its API to
just
return `string` instead of `string, error`.

To do so, we cache the result in a `nix` package variable. And populate
it
during `EnsureNixInstalled`.

However, `EnsureNixInstalled` was only called from Cobra command
functions. But
`devbox` as a library also now needs to call it. IMO, it should always
have called
it since we do rely on `nix` being installed. This PR adds this.

It also fixes up the API to:
`System: string` and `ComputeSystem: (string, error)`.

NOTE: inside `System`, I still do a best-effort to avoid panic by
calling
`ComputeSystem`. This can happen in edge-cases like some tests
`generate_test/TestWriteFromTemplate`
that exercise internal functions reliant on `nix.System` without calling
`devbox.Open`.

## How was it tested?

tests should pass

Author: savil

.github/workflows/cli-tests.yaml

@@ -90,7 +90,7 @@ jobs:
           - run-devbox-json-tests: true
             os: macos-latest
     runs-on: ${{ matrix.os }}
-    timeout-minutes: ${{ (github.ref == 'refs/heads/main' || inputs.run-mac-tests) && 37 || 20 }}
+    timeout-minutes: ${{ (github.ref == 'refs/heads/main' || inputs.run-mac-tests) && 37 || 25 }}
     steps:
       - uses: actions/checkout@v3
       - uses: actions/setup-go@v4
@@ -119,7 +119,7 @@ jobs:
           DEVBOX_DEBUG: ${{ (!matrix.run-devbox-json-tests || inputs.example-debug) && '1' || '0' }}
           DEVBOX_RUN_DEVBOX_JSON_TESTS: ${{ matrix.run-devbox-json-tests }}
           # Used in `go test -timeout` flag. Needs a value that time.ParseDuration can parse.
-          DEVBOX_GOLANG_TEST_TIMEOUT: "${{ (github.ref == 'refs/heads/main' || inputs.run-mac-tests) && '35m' || '20m' }}"
+          DEVBOX_GOLANG_TEST_TIMEOUT: "${{ (github.ref == 'refs/heads/main' || inputs.run-mac-tests) && '35m' || '25m' }}"
         run: |
           echo "::group::Nix version"
           nix --version

internal/devconfig/packages.go

@@ -249,7 +249,7 @@ func NewPackage(name string, values map[string]any) Package {
 // If the package has a list of excluded platforms, it is enabled on all platforms
 // except those.
 func (p *Package) IsEnabledOnPlatform() bool {
-	platform := nix.MustGetSystem()
+	platform := nix.System()
 	if len(p.Platforms) > 0 {
 		for _, plt := range p.Platforms {
 			if plt == platform {

internal/devpkg/package.go

@@ -227,12 +227,8 @@ func (p *Package) NormalizedDevboxPackageReference() (string, error) {
 	}
 
 	if path != "" {
-		s, err := nix.System()
-		if err != nil {
-			return "", err
-		}
 		url, fragment, _ := strings.Cut(path, "#")
-		return fmt.Sprintf("%s#legacyPackages.%s.%s", url, s, fragment), nil
+		return fmt.Sprintf("%s#legacyPackages.%s.%s", url, nix.System(), fragment), nil
 	}
 
 	return "", nil
@@ -480,17 +476,12 @@ func (p *Package) IsInBinaryCache() (bool, error) {
 		return false, err
 	}
 
-	userSystem, err := nix.System()
-	if err != nil {
-		return false, err
-	}
-
 	if entry.Systems == nil {
 		return false, nil
 	}
 
 	// Check if the user's system's info is present in the lockfile
-	_, ok := entry.Systems[userSystem]
+	_, ok := entry.Systems[nix.System()]
 	if !ok {
 		return false, nil
 	}
@@ -519,12 +510,7 @@ func (p *Package) InputAddressedPath() (string, error) {
 		return "", err
 	}
 
-	userSystem, err := nix.System()
-	if err != nil {
-		return "", err
-	}
-
-	sysInfo := entry.Systems[userSystem]
+	sysInfo := entry.Systems[nix.System()]
 	return sysInfo.StorePath, nil
 }
 

internal/impl/devbox.go

@@ -73,6 +73,7 @@ type Devbox struct {
 var legacyPackagesWarningHasBeenShown = false
 
 func Open(opts *devopt.Opts) (*Devbox, error) {
+
 	projectDir, err := findProjectDir(opts.Dir)
 	if err != nil {
 		return nil, err
@@ -128,6 +129,10 @@ func Open(opts *devopt.Opts) (*Devbox, error) {
 		)
 	}
 
+	if err := nix.EnsureNixInstalled(box.writer, func() *bool { return nil } /*withDaemonFunc*/); err != nil {
+		return nil, err
+	}
+
 	return box, nil
 }
 

internal/impl/update.go

@@ -126,15 +126,12 @@ func (d *Devbox) mergeResolvedPackageToLockfile(
 
 	// Add any missing system infos for packages whose versions did not change.
 	if featureflag.RemoveNixpkgs.Enabled() {
-		userSystem, err := nix.System()
-		if err != nil {
-			return err
-		}
 
 		if lockfile.Packages[pkg.Raw].Systems == nil {
 			lockfile.Packages[pkg.Raw].Systems = map[string]*lock.SystemInfo{}
 		}
 
+		userSystem := nix.System()
 		updated := false
 		for sysName, newSysInfo := range resolved.Systems {
 			if sysName == userSystem {

internal/impl/update_test.go

@@ -156,8 +156,7 @@ func TestUpdateOtherSysInfoIsReplaced(t *testing.T) {
 	require.Equal(t, "store_path2", lockfile.Packages[raw].Systems[sys2].StorePath)
 }
 
-func currentSystem(t *testing.T) string {
-	sys, err := nix.System() // NOTE: we could mock this too, if it helps.
-	require.NoError(t, err)
+func currentSystem(_t *testing.T) string {
+	sys := nix.System() // NOTE: we could mock this too, if it helps.
 	return sys
 }

internal/lock/resolve.go

@@ -61,11 +61,7 @@ func (f *File) FetchResolvedPackage(pkg string) (*Package, error) {
 }
 
 func selectForSystem(pkg *searcher.PackageVersion) (searcher.PackageInfo, error) {
-	currentSystem, err := nix.System()
-	if err != nil {
-		return searcher.PackageInfo{}, err
-	}
-	if pi, ok := pkg.Systems[currentSystem]; ok {
+	if pi, ok := pkg.Systems[nix.System()]; ok {
 		return pi, nil
 	}
 	if pi, ok := pkg.Systems["x86_64-linux"]; ok {

internal/nix/install.go

@@ -98,8 +98,9 @@ func isRoot() bool {
 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()
+			// call ComputeSystem to ensure its value is internally cached so other
+			// callers can rely on just calling System
+			err = ComputeSystem()
 		}
 	}()
 

internal/nix/nix.go

@@ -118,44 +118,42 @@ func ExperimentalFlags() []string {
 	}
 }
 
-// TODO: rename to System
-func MustGetSystem() string {
+func System() string {
 	if cachedSystem == "" {
-		// For internal calls (during tests), this may not be initialized properly
-		// so do a best-effort attempt to initialize it.
-		_, err := System()
-		if err != nil {
-			panic("MustGetSystem called before being initialized by System")
+		// While this should have been initialized, we do a best-effort to avoid
+		// a panic.
+		if err := ComputeSystem(); err != nil {
+			panic("System called before being initialized by ComputeSystem")
 		}
 	}
 	return cachedSystem
 }
 
 var cachedSystem string
 
-// TODO: rename to ComputeSystem or ComputePlatform?
-func System() (string, error) {
+func ComputeSystem() 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
 	// and flake.nix files.
 	// This is also used by unit tests.
+	if cachedSystem != "" {
+		return nil
+	}
 	override := os.Getenv("__DEVBOX_NIX_SYSTEM")
 	if override != "" {
-		return override, nil
-	}
-
-	if cachedSystem == "" {
+		cachedSystem = override
+	} else {
 		cmd := exec.Command(
 			"nix", "eval", "--impure", "--raw", "--expr", "builtins.currentSystem",
 		)
 		cmd.Args = append(cmd.Args, ExperimentalFlags()...)
 		out, err := cmd.Output()
 		if err != nil {
-			return "", errors.WithStack(err)
+			return err
 		}
 		cachedSystem = string(out)
 	}
-	return cachedSystem, nil
+	return nil
 }
 
 // version is the cached output of `nix --version`.

internal/nix/nixprofile/profile.go

@@ -265,12 +265,7 @@ func ProfileInstall(args *ProfileInstallArgs) error {
 		if exists, err := input.ValidateInstallsOnSystem(); err != nil {
 			return err
 		} else if !exists {
-			platform, err := nix.System()
-			if err != nil {
-				platform = ""
-			} else {
-				platform = " " + platform
-			}
+			platform := " " + nix.System()
 			return usererr.New(
 				"package %s cannot be installed on your platform%s. "+
 					"Run `devbox add %[1]s --exclude-platform%[2]s` and re-try.",