Skip to content

Commit

Permalink
[lock] Refactor local lock (#1314)
Browse files Browse the repository at this point in the history
## 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.
  • Loading branch information
mikeland73 authored Jul 26, 2023
1 parent 714dbac commit 3d91344
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 60 deletions.
7 changes: 4 additions & 3 deletions devbox.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@
"lockfile_version": "1",
"packages": {
"[email protected]": {
"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"
},
"[email protected]": {
"last_modified": "2023-05-01T16:53:22Z",
Expand Down
6 changes: 1 addition & 5 deletions internal/impl/devbox.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
20 changes: 3 additions & 17 deletions internal/impl/packages.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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.")
Expand All @@ -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) {
Expand Down
4 changes: 0 additions & 4 deletions internal/impl/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
18 changes: 13 additions & 5 deletions internal/lock/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand Down
81 changes: 56 additions & 25 deletions internal/lock/lockfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion internal/lock/resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 3d91344

Please sign in to comment.