[lock] Refactor local lock (#1314)

mikeland73 · web-flow · commit 3d91344817f2 · 2023-07-26T15:26:30.000-07:00
## Summary Follow up to #1291. This simplified the lockfile interface and mostly removes "local" lock from public view. It addresses this comment: #1291 (comment) ## How was it tested? `devbox update` on a package that needs system info downloaded.
diff --git a/devbox.lock b/devbox.lock
@@ -2,9 +2,10 @@
   "lockfile_version": "1",
   "packages": {
     "go@1.20": {
-      "last_modified": "2023-05-25T03:54:59Z",
-      "resolved": "github:NixOS/nixpkgs/8d4d822bc0efa9de6eddc79cb0d82897a9baa750#go",
-      "version": "1.20.4"
+      "last_modified": "2023-06-29T16:20:38Z",
+      "resolved": "github:NixOS/nixpkgs/3c614fbc76fc152f3e1bc4b2263da6d90adf80fb#go",
+      "source": "devbox-search",
+      "version": "1.20.5"
     },
     "golangci-lint@1.52.2": {
       "last_modified": "2023-05-01T16:53:22Z",
diff --git a/internal/impl/devbox.go b/internal/impl/devbox.go
@@ -893,11 +893,7 @@ func (d *Devbox) nixEnv(ctx context.Context) (map[string]string, error) {
 	usePrintDevEnvCache := false
 
 	// If lockfile is up-to-date, we can use the print-dev-env cache.
-	lockFile, err := lock.Local(d)
-	if err != nil {
-		return nil, err
-	}
-	upToDate, err := lockFile.IsUpToDate()
+	upToDate, err := d.lockfile.IsUpToDateAndInstalled()
 	if err != nil {
 		return nil, err
 	}
diff --git a/internal/impl/packages.go b/internal/impl/packages.go
@@ -22,7 +22,6 @@ import (
 
 	"go.jetpack.io/devbox/internal/boxcli/usererr"
 	"go.jetpack.io/devbox/internal/debug"
-	"go.jetpack.io/devbox/internal/lock"
 	"go.jetpack.io/devbox/internal/nix"
 	"go.jetpack.io/devbox/internal/plugin"
 	"go.jetpack.io/devbox/internal/ux"
@@ -188,18 +187,9 @@ const (
 func (d *Devbox) ensurePackagesAreInstalled(ctx context.Context, mode installMode) error {
 	defer trace.StartRegion(ctx, "ensurePackages").End()
 
-	localLock, err := lock.Local(d)
-	if err != nil {
-		return err
-	}
-
-	upToDate, err := localLock.IsUpToDate()
-	if err != nil {
+	if upToDate, err := d.lockfile.IsUpToDateAndInstalled(); err != nil || upToDate {
 		return err
 	}
-	if upToDate {
-		return nil
-	}
 
 	if mode == ensure {
 		fmt.Fprintln(d.writer, "Ensuring packages are installed.")
@@ -218,18 +208,14 @@ func (d *Devbox) ensurePackagesAreInstalled(ctx context.Context, mode installMod
 	}
 
 	// Force print-dev-env cache to be recomputed.
-	if _, err = d.computeNixEnv(ctx, false /*use cache*/); err != nil {
+	if _, err := d.computeNixEnv(ctx, false /*use cache*/); err != nil {
 		return err
 	}
 
 	// Ensure we clean out packages that are no longer needed.
 	d.lockfile.Tidy()
 
-	if err = d.lockfile.Save(); err != nil {
-		return err
-	}
-
-	return localLock.Update()
+	return d.lockfile.Save()
 }
 
 func (d *Devbox) profilePath() (string, error) {
diff --git a/internal/impl/update.go b/internal/impl/update.go
@@ -54,10 +54,6 @@ func (d *Devbox) Update(ctx context.Context, pkgs ...string) error {
 		}
 	}
 
-	if err := d.lockfile.Save(); err != nil {
-		return err
-	}
-
 	if err := d.ensurePackagesAreInstalled(ctx, ensure); err != nil {
 		return err
 	}
diff --git a/internal/lock/local.go b/internal/lock/local.go
@@ -34,16 +34,24 @@ func (l *localLockFile) equals(other *localLockFile) bool {
 		l.DevboxVersion == other.DevboxVersion
 }
 
-func (l *localLockFile) IsUpToDate() (bool, error) {
-	newLock, err := forProject(l.project)
+func isLocalUpToDate(project devboxProject) (bool, error) {
+	filesystemLock, err := readLocal(project)
+	if err != nil {
+		return false, err
+	}
+	newLock, err := forProject(project)
 	if err != nil {
 		return false, err
 	}
 
-	return l.equals(newLock), nil
+	return filesystemLock.equals(newLock), nil
 }
 
-func (l *localLockFile) Update() error {
+func updateLocal(project devboxProject) error {
+	l, err := readLocal(project)
+	if err != nil {
+		return err
+	}
 	newLock, err := forProject(l.project)
 	if err != nil {
 		return err
@@ -53,7 +61,7 @@ func (l *localLockFile) Update() error {
 	return cuecfg.WriteFile(localLockFilePath(l.project), l)
 }
 
-func Local(project devboxProject) (*localLockFile, error) {
+func readLocal(project devboxProject) (*localLockFile, error) {
 	lockFile := &localLockFile{project: project}
 	err := cuecfg.ParseFile(localLockFilePath(project), lockFile)
 	if errors.Is(err, fs.ErrNotExist) {
diff --git a/internal/lock/lockfile.go b/internal/lock/lockfile.go
@@ -45,76 +45,79 @@ func GetFile(project devboxProject) (*File, error) {
 	return lockFile, nil
 }
 
-func (l *File) Add(pkgs ...string) error {
+func (f *File) Add(pkgs ...string) error {
 	for _, p := range pkgs {
-		if _, err := l.Resolve(p); err != nil {
+		if _, err := f.Resolve(p); err != nil {
 			return err
 		}
 	}
-	return l.Save()
+	return f.Save()
 }
 
-func (l *File) Remove(pkgs ...string) error {
+func (f *File) Remove(pkgs ...string) error {
 	for _, p := range pkgs {
-		delete(l.Packages, p)
+		delete(f.Packages, p)
 	}
-	return l.Save()
+	return f.Save()
 }
 
 // Resolve updates the in memory copy for performance but does not write to disk
 // This avoids writing values that may need to be removed in case of error.
-func (l *File) Resolve(pkg string) (*Package, error) {
-	entry, hasEntry := l.Packages[pkg]
+func (f *File) Resolve(pkg string) (*Package, error) {
+	entry, hasEntry := f.Packages[pkg]
 
 	if !hasEntry || entry.Resolved == "" {
 		locked := &Package{}
 		var err error
 		if _, _, versioned := searcher.ParseVersionedPackage(pkg); versioned {
-			locked, err = l.FetchResolvedPackage(pkg)
+			locked, err = f.FetchResolvedPackage(pkg)
 			if err != nil {
 				return nil, err
 			}
 		} else if IsLegacyPackage(pkg) {
 			// These are legacy packages without a version. Resolve to nixpkgs with
 			// whatever hash is in the devbox.json
 			locked = &Package{
-				Resolved: l.LegacyNixpkgsPath(pkg),
+				Resolved: f.LegacyNixpkgsPath(pkg),
 				Source:   nixpkgSource,
 			}
 		}
-		l.Packages[pkg] = locked
+		f.Packages[pkg] = locked
 	}
 
-	return l.Packages[pkg], nil
+	return f.Packages[pkg], nil
 }
 
-func (l *File) ForceResolve(pkg string) (*Package, error) {
-	delete(l.Packages, pkg)
-	return l.Resolve(pkg)
+func (f *File) ForceResolve(pkg string) (*Package, error) {
+	delete(f.Packages, pkg)
+	return f.Resolve(pkg)
 }
 
-func (l *File) Save() error {
-	return cuecfg.WriteFile(lockFilePath(l.devboxProject), l)
+func (f *File) Save() error {
+	if err := cuecfg.WriteFile(lockFilePath(f.devboxProject), f); err != nil {
+		return err
+	}
+	return updateLocal(f.devboxProject)
 }
 
-func (l *File) LegacyNixpkgsPath(pkg string) string {
+func (f *File) LegacyNixpkgsPath(pkg string) string {
 	return fmt.Sprintf(
 		"github:NixOS/nixpkgs/%s#%s",
-		l.NixPkgsCommitHash(),
+		f.NixPkgsCommitHash(),
 		pkg,
 	)
 }
 
-func (l *File) Get(pkg string) *Package {
-	entry, hasEntry := l.Packages[pkg]
+func (f *File) Get(pkg string) *Package {
+	entry, hasEntry := f.Packages[pkg]
 	if !hasEntry || entry.Resolved == "" {
 		return nil
 	}
 	return entry
 }
 
-func (l *File) HasAllowInsecurePackages() bool {
-	for _, pkg := range l.Packages {
+func (f *File) HasAllowInsecurePackages() bool {
+	for _, pkg := range f.Packages {
 		if pkg.AllowInsecure {
 			return true
 		}
@@ -137,8 +140,36 @@ func IsLegacyPackage(pkg string) bool {
 
 // Tidy ensures that the lockfile has the set of packages corresponding to the devbox.json config.
 // It gets rid of older packages that are no longer needed.
-func (l *File) Tidy() {
-	l.Packages = lo.PickByKeys(l.Packages, l.devboxProject.Packages())
+func (f *File) Tidy() {
+	f.Packages = lo.PickByKeys(f.Packages, f.devboxProject.Packages())
+}
+
+// IsUpToDateAndInstalled returns true if the lockfile is up to date and the
+// local hashes match, which generally indicates all packages are correctly
+// installed and print-dev-env has been computed and cached.
+func (f *File) IsUpToDateAndInstalled() (bool, error) {
+	if dirty, err := f.isDirty(); err != nil {
+		return false, err
+	} else if dirty {
+		return false, nil
+	}
+	return isLocalUpToDate(f.devboxProject)
+}
+
+func (f *File) isDirty() (bool, error) {
+	currentHash, err := cuecfg.Hash(f)
+	if err != nil {
+		return false, err
+	}
+	fileSystemLockFile, err := GetFile(f.devboxProject)
+	if err != nil {
+		return false, err
+	}
+	filesystemHash, err := cuecfg.Hash(fileSystemLockFile)
+	if err != nil {
+		return false, err
+	}
+	return currentHash != filesystemHash, nil
 }
 
 func lockFilePath(project devboxProject) string {
diff --git a/internal/lock/resolve.go b/internal/lock/resolve.go
@@ -22,7 +22,7 @@ import (
 // not changed. This can happen when doing `devbox update` and search has
 // a newer hash than the lock file but same version. In that case we don't want
 // to update because it would be slow and wasteful.
-func (l *File) FetchResolvedPackage(pkg string) (*Package, error) {
+func (f *File) FetchResolvedPackage(pkg string) (*Package, error) {
 	name, version, _ := searcher.ParseVersionedPackage(pkg)
 	if version == "" {
 		return nil, usererr.New("No version specified for %q.", name)

Original file line number	Diff line number	Diff line change
`@@ -54,10 +54,6 @@ func (d *Devbox) Update(ctx context.Context, pkgs ...string) error {`
`54`	`54`	`}`
`55`	`55`	`}`
`56`	`56`
`57`		`- if err := d.lockfile.Save(); err != nil {`
`58`		`- return err`
`59`		`- }`
`60`		`-`
`61`	`57`	`if err := d.ensurePackagesAreInstalled(ctx, ensure); err != nil {`
`62`	`58`	`return err`
`63`	`59`	`}`