From 3d91344817f2242fd36c202e2f056c5434a84712 Mon Sep 17 00:00:00 2001 From: Mike Landau Date: Wed, 26 Jul 2023 15:26:30 -0700 Subject: [PATCH] [lock] Refactor local lock (#1314) ## Summary Follow up to https://github.com/jetpack-io/devbox/pull/1291. This simplified the lockfile interface and mostly removes "local" lock from public view. It addresses this comment: https://github.com/jetpack-io/devbox/pull/1291#discussion_r1270102373 ## How was it tested? `devbox update` on a package that needs system info downloaded. --- devbox.lock | 7 ++-- internal/impl/devbox.go | 6 +-- internal/impl/packages.go | 20 ++-------- internal/impl/update.go | 4 -- internal/lock/local.go | 18 ++++++--- internal/lock/lockfile.go | 81 +++++++++++++++++++++++++++------------ internal/lock/resolve.go | 2 +- 7 files changed, 78 insertions(+), 60 deletions(-) diff --git a/devbox.lock b/devbox.lock index a52f1eda5b6..19796ad99e3 100644 --- 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 index a47dbbae289..6b722ed405f 100644 --- 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 index 28dc646a320..63f23a22c83 100644 --- 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 index 5ec95db0f27..661f181de01 100644 --- 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 index 39bbb1423c4..708bc665f9d 100644 --- 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 index ce82015aeee..95644b7e8e4 100644 --- a/internal/lock/lockfile.go +++ b/internal/lock/lockfile.go @@ -45,32 +45,32 @@ 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 } @@ -78,43 +78,46 @@ func (l *File) Resolve(pkg string) (*Package, error) { // 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 index 6e27f53fcba..b4e9b3e66d1 100644 --- 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)