Skip to content

Commit

Permalink
shellgen: delete flake.lock if flake.nix changes (#2388)
Browse files Browse the repository at this point in the history
Fix a bug where `.devbox/gen/flake` gets locked on old versions of
`.devbox/gen/flake/glibc-patch` by deleting the `flake.lock` file.

We only delete the lock file when the generated flake changes so that
Nix isn't forced to re-evaluate it every time.

The repro steps are:

1. Add a package that gets auto-patched (`devbox add [email protected]`).
2. `.devbox/gen/flake` gets locked on the patch flake.
3. Change the patched package (`devbox add [email protected]`).
4. The new patch flake isn't used because of
`.devbox/gen/flake/flake.lock`. Instead, the old version is used (from
the Nix store).

Fixes #2316.
Fixes #2370.
  • Loading branch information
gcurtis authored Oct 28, 2024
1 parent 399b7f3 commit 2f38449
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 21 deletions.
10 changes: 9 additions & 1 deletion internal/shellgen/flake_plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"log/slog"
"os"
"path/filepath"
"runtime/trace"
"slices"
Expand Down Expand Up @@ -317,7 +318,14 @@ func (g *glibcPatchFlake) writeTo(dir string) error {
slog.Debug("error copying system libcuda.so to flake", "dir", dir)
}
}
return writeFromTemplate(dir, g, "glibc-patch.nix", "flake.nix")
changed, err := writeFromTemplate(dir, g, "glibc-patch.nix", "flake.nix")
if err != nil {
return err
}
if changed {
_ = os.Remove(filepath.Join(dir, "flake.lock"))
}
return nil
}

func (g *glibcPatchFlake) LogValue() slog.Value {
Expand Down
38 changes: 21 additions & 17 deletions internal/shellgen/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,13 @@ func GenerateForPrintEnv(ctx context.Context, devbox devboxer) error {
outPath := genPath(devbox)

// Preserving shell.nix to avoid breaking old-style .envrc users
err = writeFromTemplate(outPath, plan, "shell.nix", "shell.nix")
_, err = writeFromTemplate(outPath, plan, "shell.nix", "shell.nix")
if err != nil {
return errors.WithStack(err)
}

// Gitignore file is added to the .devbox directory
err = writeFromTemplate(filepath.Join(devbox.ProjectDir(), ".devbox"), plan, ".gitignore", ".gitignore")
_, err = writeFromTemplate(filepath.Join(devbox.ProjectDir(), ".devbox"), plan, ".gitignore", ".gitignore")
if err != nil {
return errors.WithStack(err)
}
Expand All @@ -70,7 +70,7 @@ var (
tmplBuf bytes.Buffer
)

func writeFromTemplate(path string, plan any, tmplName, generatedName string) error {
func writeFromTemplate(path string, plan any, tmplName, generatedName string) (changed bool, err error) {
tmplKey := tmplName + ".tmpl"
tmpl := tmplCache[tmplKey]
if tmpl == nil {
Expand All @@ -81,64 +81,64 @@ func writeFromTemplate(path string, plan any, tmplName, generatedName string) er
glob := "tmpl/" + tmplKey
tmpl, err = tmpl.ParseFS(tmplFS, glob)
if err != nil {
return redact.Errorf("parse embedded tmplFS glob %q: %v", redact.Safe(glob), redact.Safe(err))
return false, redact.Errorf("parse embedded tmplFS glob %q: %v", redact.Safe(glob), redact.Safe(err))
}
tmplCache[tmplKey] = tmpl
}
tmplBuf.Reset()
if err := tmpl.Execute(&tmplBuf, plan); err != nil {
return redact.Errorf("execute template %s: %v", redact.Safe(tmplKey), err)
return false, redact.Errorf("execute template %s: %v", redact.Safe(tmplKey), err)
}

// In some circumstances, Nix looks at the mod time of a file when
// caching, so we only want to update the file if something has
// changed. Blindly overwriting the file could invalidate Nix's cache
// every time, slowing down evaluation considerably.
err := overwriteFileIfChanged(filepath.Join(path, generatedName), tmplBuf.Bytes(), 0o644)
changed, err = overwriteFileIfChanged(filepath.Join(path, generatedName), tmplBuf.Bytes(), 0o644)
if err != nil {
return redact.Errorf("write %s to file: %v", redact.Safe(tmplName), err)
return changed, redact.Errorf("write %s to file: %v", redact.Safe(tmplName), err)
}
return nil
return changed, nil
}

// overwriteFileIfChanged checks that the contents of f == data, and overwrites
// f if they differ. It also ensures that f's permissions are set to perm.
func overwriteFileIfChanged(path string, data []byte, perm os.FileMode) error {
func overwriteFileIfChanged(path string, data []byte, perm os.FileMode) (changed bool, err error) {
flag := os.O_RDWR | os.O_CREATE
file, err := os.OpenFile(path, flag, perm)
if errors.Is(err, os.ErrNotExist) {
if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil {
return err
return false, err
}

// Definitely a new file if we had to make the directory.
return os.WriteFile(path, data, perm)
return true, os.WriteFile(path, data, perm)
}
if err != nil {
return err
return false, err
}
defer file.Close()

fi, err := file.Stat()
if err != nil || fi.Mode().Perm() != perm {
if err := file.Chmod(perm); err != nil {
return err
return false, err
}
}

// Fast path - check if the lengths differ.
if err == nil && fi.Size() != int64(len(data)) {
return overwriteFile(file, data, 0)
return true, overwriteFile(file, data, 0)
}

r := bufio.NewReader(file)
for offset := range data {
b, err := r.ReadByte()
if err != nil || b != data[offset] {
return overwriteFile(file, data, offset)
return true, overwriteFile(file, data, offset)
}
}
return nil
return false, nil
}

// overwriteFile truncates f to len(data) and writes data[offset:] beginning at
Expand Down Expand Up @@ -168,5 +168,9 @@ var templateFuncs = template.FuncMap{

func makeFlakeFile(d devboxer, plan *flakePlan) error {
flakeDir := FlakePath(d)
return writeFromTemplate(flakeDir, plan, "flake.nix", "flake.nix")
changed, err := writeFromTemplate(flakeDir, plan, "flake.nix", "flake.nix")
if changed {
_ = os.Remove(filepath.Join(flakeDir, "flake.lock"))
}
return err
}
6 changes: 3 additions & 3 deletions internal/shellgen/generate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,14 @@ func TestWriteFromTemplate(t *testing.T) {
t.Setenv("__DEVBOX_NIX_SYSTEM", "x86_64-linux")
dir := filepath.Join(t.TempDir(), "makeme")
outPath := filepath.Join(dir, "flake.nix")
err := writeFromTemplate(dir, testFlakeTmplPlan, "flake.nix", "flake.nix")
_, err := writeFromTemplate(dir, testFlakeTmplPlan, "flake.nix", "flake.nix")
if err != nil {
t.Fatal("got error writing flake template:", err)
}
cmpGoldenFile(t, outPath, "testdata/flake.nix.golden")

t.Run("WriteUnmodified", func(t *testing.T) {
err = writeFromTemplate(dir, testFlakeTmplPlan, "flake.nix", "flake.nix")
_, err = writeFromTemplate(dir, testFlakeTmplPlan, "flake.nix", "flake.nix")
if err != nil {
t.Fatal("got error writing flake template:", err)
}
Expand All @@ -49,7 +49,7 @@ func TestWriteFromTemplate(t *testing.T) {
FlakeInputs: []flakeInput{},
System: "x86_64-linux",
}
err = writeFromTemplate(dir, emptyPlan, "flake.nix", "flake.nix")
_, err = writeFromTemplate(dir, emptyPlan, "flake.nix", "flake.nix")
if err != nil {
t.Fatal("got error writing flake template:", err)
}
Expand Down

0 comments on commit 2f38449

Please sign in to comment.