[config] Refactor config to prepare for imports (#1817)

## Summary

Currently we use config fields on the Config struct directly. This makes
it really hard to track what we use to modify the file vs what we are
using as the representation of the config. This was not a problem in the
past because the config and the `devbox.json` file are one in the same.
Since we are hoping to introduce imports, we need to make it more clear
what each field/function refers to.

In this PR, I split config into 2 structs:

* `configFile` which represents a single JSON file and is not exported
directly.
* `Config` a wrapper around a `Root` `configFile` and in the future
possible imports. The root is exported so that we don't have to wrap
every `configFile` field.

Note: `Config` has a bunch of functions that will need to be
re-implemented in order to support imports.

Other changes:

* Rename `Packages` struct to `PackagesMutator` and removed all
non-mutating functions. This makes the struct more purposeful.
* Made a few structs not-exported.
* Moved some tests around.

Note: Github diff UI doesn't do a great job with file name changes in
this PR so to make this PR easier to read I named the new Config file
`config2.go` while keeping the previous `config.go`. Before merging I'll
rename `config.go file.go` and `config2.go -> config.go`.

This code change should have no functional change on devbox.

## How was it tested?

builds, tests

Author: mikeland73

internal/boxcli/midcobra/telemetry.go

@@ -108,5 +108,5 @@ func getPackagesAndCommitHash(c *cobra.Command) ([]string, string) {
 		return []string{}, ""
 	}
 
-	return box.Config().Packages.VersionedNames(), box.Config().NixPkgsCommitHash()
+	return box.Config().PackagesVersionedNames(), box.Config().NixPkgsCommitHash()
 }

internal/boxcli/secrets.go

@@ -247,7 +247,7 @@ func secretsInitFunc(
 	secrets := box.UninitializedSecrets(ctx)
 
 	if _, err := secrets.ProjectConfig(); err == nil &&
-		box.Config().EnvFrom != "jetpack-cloud" {
+		!box.Config().IsEnvsecEnabled() {
 		// Handle edge case where directory is already set up, but devbox.json is
 		// not configured to use jetpack-cloud.
 		ux.Finfo(
@@ -257,6 +257,6 @@ func secretsInitFunc(
 	} else if err := secrets.NewProject(ctx, flags.force); err != nil {
 		return errors.WithStack(err)
 	}
-	box.Config().SetStringField("EnvFrom", "jetpack-cloud")
-	return box.Config().SaveTo(box.ProjectDir())
+	box.Config().Root.SetStringField("EnvFrom", "jetpack-cloud")
+	return box.Config().Root.SaveTo(box.ProjectDir())
 }

internal/devbox/devbox.go

@@ -522,11 +522,11 @@ func (d *Devbox) GenerateEnvrcFile(ctx context.Context, force bool, envFlags dev
 
 // saveCfg writes the config file to the devbox directory.
 func (d *Devbox) saveCfg() error {
-	return d.cfg.SaveTo(d.ProjectDir())
+	return d.cfg.Root.SaveTo(d.ProjectDir())
 }
 
 func (d *Devbox) Services() (services.Services, error) {
-	pluginSvcs, err := d.pluginManager.GetServices(d.InstallablePackages(), d.cfg.Include)
+	pluginSvcs, err := d.pluginManager.GetServices(d.InstallablePackages(), d.cfg.Include())
 	if err != nil {
 		return nil, err
 	}
@@ -881,7 +881,7 @@ func (d *Devbox) computeEnv(ctx context.Context, usePrintDevEnvCache bool) (map[
 	// We still need to be able to add env variables to non-service binaries
 	// (e.g. ruby). This would involve understanding what binaries are associated
 	// to a given plugin.
-	pluginEnv, err := d.pluginManager.Env(d.InstallablePackages(), d.cfg.Include, env)
+	pluginEnv, err := d.pluginManager.Env(d.InstallablePackages(), d.cfg.Include(), env)
 	if err != nil {
 		return nil, err
 	}
@@ -1008,7 +1008,7 @@ func (d *Devbox) flakeDir() string {
 func (d *Devbox) PackageNames() []string {
 	// TODO savil: centralize implementation by calling d.configPackages and getting pkg.Raw
 	// Skipping for now to avoid propagating the error value.
-	return d.cfg.Packages.VersionedNames()
+	return d.cfg.PackagesVersionedNames()
 }
 
 // ConfigPackages returns the packages that are defined in devbox.json
@@ -1033,7 +1033,7 @@ func (d *Devbox) AllInstallablePackages() ([]*devpkg.Package, error) {
 
 func (d *Devbox) Includes() []plugin.Includable {
 	includes := []plugin.Includable{}
-	for _, includePath := range d.cfg.Include {
+	for _, includePath := range d.cfg.Include() {
 		if include, err := d.pluginManager.ParseInclude(includePath); err == nil {
 			includes = append(includes, include)
 		}
@@ -1140,14 +1140,14 @@ func (d *Devbox) configEnvs(
 				}
 			}
 		}
-	} else if d.cfg.EnvFrom != "" {
+	} else if d.cfg.Root.EnvFrom != "" {
 		return nil, usererr.New(
 			"unknown from_env value: %s. Supported value is: %q.",
-			d.cfg.EnvFrom,
+			d.cfg.Root.EnvFrom,
 			"jetpack-cloud",
 		)
 	}
-	for k, v := range d.cfg.Env {
+	for k, v := range d.cfg.Env() {
 		env[k] = v
 	}
 	return conf.OSExpandEnvMap(env, existingEnv, d.ProjectDir()), nil

internal/devbox/dir_test.go

@@ -139,30 +139,3 @@ func TestFindParentDirAtPath(t *testing.T) {
 		})
 	}
 }
-
-func TestNixpkgsValidation(t *testing.T) {
-	testCases := map[string]struct {
-		commit   string
-		isErrant bool
-	}{
-		"invalid_nixpkg_commit": {"1234545", true},
-		"valid_nixpkg_commit":   {"af9e00071d0971eb292fd5abef334e66eda3cb69", false},
-	}
-
-	for name, testCase := range testCases {
-		t.Run(name, func(t *testing.T) {
-			assert := assert.New(t)
-
-			err := devconfig.ValidateNixpkg(&devconfig.Config{
-				Nixpkgs: &devconfig.NixpkgsConfig{
-					Commit: testCase.commit,
-				},
-			})
-			if testCase.isErrant {
-				assert.Error(err)
-			} else {
-				assert.NoError(err)
-			}
-		})
-	}
-}

internal/devbox/docgen/docgen.go

@@ -51,10 +51,10 @@ func GenerateReadme(
 	}
 
 	return tmpl.Execute(f, map[string]any{
-		"Name":        devbox.Config().Name,
-		"Description": devbox.Config().Description,
+		"Name":        devbox.Config().Root.Name,
+		"Description": devbox.Config().Root.Description,
 		"Scripts":     devbox.Config().Scripts(),
-		"EnvVars":     devbox.Config().Env,
+		"EnvVars":     devbox.Config().Env(),
 		"InitHook":    devbox.Config().InitHook(),
 		"Packages":    devbox.ConfigPackages(),
 	})

internal/devbox/packages.go

@@ -98,7 +98,7 @@ func (d *Devbox) Add(ctx context.Context, pkgsNames []string, opts devopt.AddOpt
 		}
 
 		ux.Finfo(d.stderr, "Adding package %q to devbox.json\n", packageNameForConfig)
-		d.cfg.Packages.Add(packageNameForConfig)
+		d.cfg.PackageMutator().Add(packageNameForConfig)
 		addedPackageNames = append(addedPackageNames, packageNameForConfig)
 	}
 
@@ -120,27 +120,27 @@ func (d *Devbox) Add(ctx context.Context, pkgsNames []string, opts devopt.AddOpt
 
 func (d *Devbox) setPackageOptions(pkgs []string, opts devopt.AddOpts) error {
 	for _, pkg := range pkgs {
-		if err := d.cfg.Packages.AddPlatforms(
+		if err := d.cfg.PackageMutator().AddPlatforms(
 			d.stderr, pkg, opts.Platforms); err != nil {
 			return err
 		}
-		if err := d.cfg.Packages.ExcludePlatforms(
+		if err := d.cfg.PackageMutator().ExcludePlatforms(
 			d.stderr, pkg, opts.ExcludePlatforms); err != nil {
 			return err
 		}
-		if err := d.cfg.Packages.SetDisablePlugin(
+		if err := d.cfg.PackageMutator().SetDisablePlugin(
 			pkg, opts.DisablePlugin); err != nil {
 			return err
 		}
-		if err := d.cfg.Packages.SetPatchGLibc(
+		if err := d.cfg.PackageMutator().SetPatchGLibc(
 			pkg, opts.PatchGlibc); err != nil {
 			return err
 		}
-		if err := d.cfg.Packages.SetOutputs(
+		if err := d.cfg.PackageMutator().SetOutputs(
 			d.stderr, pkg, opts.Outputs); err != nil {
 			return err
 		}
-		if err := d.cfg.Packages.SetAllowInsecure(
+		if err := d.cfg.PackageMutator().SetAllowInsecure(
 			d.stderr, pkg, opts.AllowInsecure); err != nil {
 			return err
 		}
@@ -191,7 +191,7 @@ func (d *Devbox) Remove(ctx context.Context, pkgs ...string) error {
 		found, _ := d.findPackageByName(pkg)
 		if found != nil {
 			packagesToUninstall = append(packagesToUninstall, found.Raw)
-			d.cfg.Packages.Remove(found.Raw)
+			d.cfg.PackageMutator().Remove(found.Raw)
 		} else {
 			missingPkgs = append(missingPkgs, pkg)
 		}
@@ -553,7 +553,7 @@ func (d *Devbox) moveAllowInsecureFromLockfile(writer io.Writer, lockfile *lock.
 		if err != nil {
 			return fmt.Errorf("failed to get package's store name for package %q with error %w", versionedName, err)
 		}
-		if err := cfg.Packages.SetAllowInsecure(writer, versionedName, []string{storeName}); err != nil {
+		if err := cfg.PackageMutator().SetAllowInsecure(writer, versionedName, []string{storeName}); err != nil {
 			return fmt.Errorf("failed to set allow_insecure in devbox.json for package %q with error %w", versionedName, err)
 		}
 	}

internal/devbox/update.go

@@ -31,7 +31,7 @@ func (d *Devbox) Update(ctx context.Context, opts devopt.UpdateOpts) error {
 			fmt.Fprintf(d.stderr, "Updating %s -> %s\n", pkg.Raw, pkg.LegacyToVersioned())
 
 			// Get the package from the config to get the Platforms and ExcludedPlatforms later
-			cfgPackage, ok := d.cfg.Packages.Get(pkg.Raw)
+			cfgPackage, ok := d.cfg.Root.GetPackage(pkg.Raw)
 			if !ok {
 				return fmt.Errorf("package %s not found in config", pkg.Raw)
 			}