Skip to content

Commit e584340

Browse files
gcurtisgcurtis
authored
nix,impl: avoid modifying flake.nix; don't use --impure (#749)
The previous devbox shell/run startup time was taking about 3.5x longer than `nix develop` for "warm" runs where the flake had already been evaluated once: $ time nix develop .devbox/gen/flake -c echo hi hi 0.12s user 0.04s system 45% cpu 0.356 total $ time devbox run -- echo hi hi 0.66s user 0.17s system 62% cpu 1.330 total This was due to two reasons: 1. Devbox was passing `--impure` so that `NIXPKGS_ALLOW_UNFREE` could be used. This prevents Nix from using its eval cache. Fix it by setting the `allowUnfree` attribute directly in the flake. 2. Under certain circumstances, Nix uses the flake file's modified time stamp to determine if it can use its eval cache. Devbox unconditionally writes generated files on every run, which was changing the modified timestamp of the flake. Update the `writeFromTemplate` function to not write to existing files if the contents haven't changed. After these changes, the new devbox times are more in-line with Nix: $ time devbox run -- echo hi hi 0.19s user 0.09s system 59% cpu 0.466 total Also ran `nixpgs-fmt` on the flake file templates and added tests to ensure they stay correct.
1 parent 48c2162 commit e584340

File tree

9 files changed

+301
-35
lines changed

9 files changed

+301
-35
lines changed

‎internal/impl/generate.go‎

Lines changed: 71 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,10 @@
44
package impl
55

66
import (
7+
"bytes"
78
"embed"
8-
"fmt"
9+
"io"
10+
"io/fs"
911
"os"
1012
"os/exec"
1113
"path/filepath"
@@ -20,7 +22,7 @@ import (
2022
"go.jetpack.io/devbox/internal/plugin"
2123
)
2224

23-
//go:embed tmpl/* tmpl/.*
25+
//go:embed tmpl/*
2426
var tmplFS embed.FS
2527

2628
var shellFiles = []string{"development.nix", "shell.nix"}
@@ -62,26 +64,81 @@ func generateForShell(rootPath string, plan *plansdk.ShellPlan, pluginManager *p
6264
return nil
6365
}
6466

65-
func writeFromTemplate(path string, plan interface{}, tmplName string) error {
66-
embeddedPath := fmt.Sprintf("tmpl/%s.tmpl", tmplName)
67+
// Cache and buffers for generating templated files.
68+
var (
69+
tmplCache = map[string]*template.Template{}
6770

68-
// Should we clear the directory so we start "fresh"?
69-
outPath := filepath.Join(path, tmplName)
70-
outDir := filepath.Dir(outPath)
71-
err := os.MkdirAll(outDir, 0755) // Ensure directory exists.
71+
// Most generated files are < 4KiB.
72+
tmplNewBuf = bytes.NewBuffer(make([]byte, 0, 4096))
73+
tmplOldBuf = bytes.NewBuffer(make([]byte, 0, 4096))
74+
)
75+
76+
func writeFromTemplate(path string, plan any, tmplName string) error {
77+
tmplKey := tmplName + ".tmpl"
78+
tmpl := tmplCache[tmplKey]
79+
if tmpl == nil {
80+
tmpl = template.New(tmplKey)
81+
tmpl.Funcs(templateFuncs)
82+
83+
var err error
84+
tmpl, err = tmpl.ParseFS(tmplFS, "tmpl/"+tmplKey)
85+
if err != nil {
86+
return errors.WithStack(err)
87+
}
88+
tmplCache[tmplKey] = tmpl
89+
}
90+
tmplNewBuf.Reset()
91+
if err := tmpl.Execute(tmplNewBuf, plan); err != nil {
92+
return errors.WithStack(err)
93+
}
94+
95+
// In some circumstances, Nix looks at the mod time of a file when
96+
// caching, so we only want to update the file if something has
97+
// changed. Blindly overwriting the file could invalidate Nix's cache
98+
// every time, slowing down evaluation considerably.
99+
var (
100+
outPath = filepath.Join(path, tmplName)
101+
flag = os.O_RDWR | os.O_CREATE
102+
perm = fs.FileMode(0644)
103+
)
104+
outFile, err := os.OpenFile(outPath, flag, perm)
105+
if errors.Is(err, os.ErrNotExist) {
106+
if err := os.MkdirAll(path, 0755); err != nil {
107+
return errors.WithStack(err)
108+
}
109+
outFile, err = os.OpenFile(outPath, flag, perm)
110+
}
72111
if err != nil {
73112
return errors.WithStack(err)
74113
}
114+
defer outFile.Close()
75115

76-
f, err := os.Create(outPath)
77-
defer func() {
78-
_ = f.Close()
79-
}()
116+
// Only read len(tmplWriteBuf) + 1 from the existing file so we can
117+
// check if the lengths are different without reading the whole thing.
118+
tmplOldBuf.Reset()
119+
tmplOldBuf.Grow(tmplNewBuf.Len() + 1)
120+
_, err = io.Copy(tmplOldBuf, io.LimitReader(outFile, int64(tmplNewBuf.Len())+1))
80121
if err != nil {
81122
return errors.WithStack(err)
82123
}
83-
t := template.Must(template.New(tmplName+".tmpl").Funcs(templateFuncs).ParseFS(tmplFS, embeddedPath))
84-
return errors.WithStack(t.Execute(f, plan))
124+
if bytes.Equal(tmplNewBuf.Bytes(), tmplOldBuf.Bytes()) {
125+
return nil
126+
}
127+
128+
// Replace the existing file contents.
129+
if _, err := outFile.Seek(0, io.SeekStart); err != nil {
130+
return errors.WithStack(err)
131+
}
132+
if err := outFile.Truncate(int64(tmplNewBuf.Len())); err != nil {
133+
return errors.WithStack(err)
134+
}
135+
if _, err := io.Copy(outFile, tmplNewBuf); err != nil {
136+
return errors.WithStack(err)
137+
}
138+
if err := outFile.Close(); err != nil {
139+
return errors.WithStack(err)
140+
}
141+
return nil
85142
}
86143

87144
func toJSON(a any) string {

‎internal/impl/generate_test.go‎

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
package impl
2+
3+
import (
4+
"os"
5+
"path/filepath"
6+
"strings"
7+
"testing"
8+
9+
"github.com/google/go-cmp/cmp"
10+
)
11+
12+
func TestWriteFromTemplate(t *testing.T) {
13+
dir := filepath.Join(t.TempDir(), "makeme")
14+
outPath := filepath.Join(dir, "flake.nix")
15+
err := writeFromTemplate(dir, testFlakeTmplPlan, "flake.nix")
16+
if err != nil {
17+
t.Fatal("got error writing flake template:", err)
18+
}
19+
fi, err := os.Stat(outPath)
20+
if err != nil {
21+
t.Fatal("got stat error for new flake file:", err)
22+
}
23+
originalModTime := fi.ModTime()
24+
cmpGoldenFile(t, outPath, "testdata/flake.nix.golden")
25+
26+
t.Run("WriteUnmodified", func(t *testing.T) {
27+
err = writeFromTemplate(dir, testFlakeTmplPlan, "flake.nix")
28+
if err != nil {
29+
t.Fatal("got error writing flake template:", err)
30+
}
31+
fi, err := os.Stat(outPath)
32+
if err != nil {
33+
t.Fatal("got stat error for flake file:", err)
34+
}
35+
if !originalModTime.Equal(fi.ModTime()) {
36+
t.Errorf("flake mod time changed from %s to %s", originalModTime, fi.ModTime())
37+
}
38+
cmpGoldenFile(t, outPath, "testdata/flake.nix.golden")
39+
})
40+
t.Run("WriteModifiedSmaller", func(t *testing.T) {
41+
emptyPlan := struct {
42+
NixpkgsInfo struct {
43+
URL string
44+
}
45+
Definitions []string
46+
DevPackages []string
47+
}{}
48+
err = writeFromTemplate(dir, emptyPlan, "flake.nix")
49+
if err != nil {
50+
t.Fatal("got error writing flake template:", err)
51+
}
52+
fi, err := os.Stat(filepath.Join(dir, "flake.nix"))
53+
if err != nil {
54+
t.Fatal("got stat error for flake file:", err)
55+
}
56+
if originalModTime.Equal(fi.ModTime()) {
57+
t.Errorf("flake mod time didn't change from %s", originalModTime)
58+
}
59+
cmpGoldenFile(t, outPath, "testdata/flake-empty.nix.golden")
60+
})
61+
t.Run("WriteModifiedBigger", func(t *testing.T) {
62+
err = writeFromTemplate(dir, testFlakeTmplPlan, "flake.nix")
63+
if err != nil {
64+
t.Fatal("got error writing flake template:", err)
65+
}
66+
fi, err := os.Stat(filepath.Join(dir, "flake.nix"))
67+
if err != nil {
68+
t.Fatal("got stat error for flake file:", err)
69+
}
70+
if originalModTime.Equal(fi.ModTime()) {
71+
t.Errorf("flake mod time didn't change from %s", originalModTime)
72+
}
73+
cmpGoldenFile(t, outPath, "testdata/flake.nix.golden")
74+
})
75+
}
76+
77+
func cmpGoldenFile(t *testing.T, gotPath, wantGoldenPath string) {
78+
got, err := os.ReadFile(gotPath)
79+
if err != nil {
80+
t.Fatal("got error reading generated file:", err)
81+
}
82+
if *update {
83+
err = os.WriteFile(wantGoldenPath, got, 0666)
84+
if err != nil {
85+
t.Error("got error updating golden file:", err)
86+
}
87+
}
88+
golden, err := os.ReadFile(wantGoldenPath)
89+
if err != nil {
90+
t.Fatal("got error reading golden file:", err)
91+
}
92+
diff := cmp.Diff(golden, got)
93+
if diff != "" {
94+
t.Errorf(strings.TrimSpace(`
95+
got wrong file contents (-%s +%s):
96+
97+
%s
98+
If the new file is correct, you can update the golden file with:
99+
100+
go test -run "^%s$" -update`),
101+
filepath.Base(wantGoldenPath), filepath.Base(gotPath),
102+
diff, t.Name())
103+
}
104+
}
105+
106+
var testFlakeTmplPlan = &struct {
107+
NixpkgsInfo struct {
108+
URL string
109+
}
110+
Definitions []string
111+
DevPackages []string
112+
}{
113+
NixpkgsInfo: struct {
114+
URL string
115+
}{
116+
URL: "https://github.com/nixos/nixpkgs/archive/b9c00c1d41ccd6385da243415299b39aa73357be.tar.gz",
117+
},
118+
Definitions: []string{
119+
"php = pkgs.php.withExtensions ({ enabled, all }: enabled ++ (with all; [ blackfire ]));",
120+
"php81Packages.composer = php.packages.composer;",
121+
},
122+
DevPackages: []string{
123+
"php",
124+
"php81Packages.composer",
125+
"php81Extensions.blackfire",
126+
"flyctl",
127+
"postgresql",
128+
"tree",
129+
"git",
130+
"zsh",
131+
"openssh",
132+
"vim",
133+
"sqlite",
134+
"jq",
135+
"delve",
136+
"ripgrep",
137+
"shellcheck",
138+
"terraform",
139+
"xz",
140+
"zstd",
141+
"gnupg",
142+
"go_1_20",
143+
"python3",
144+
"graphviz",
145+
},
146+
}

‎internal/impl/packages.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,6 @@ func (d *Devbox) installNixProfile() (err error) {
219219
"-f", filepath.Join(d.projectDir, ".devbox/gen/development.nix"),
220220
)
221221

222-
cmd.Env = nix.DefaultEnv()
223222
cmd.Stdout = &nix.PackageInstallWriter{Writer: d.writer}
224223

225224
cmd.Stderr = cmd.Stdout

‎internal/impl/testdata/flake-empty.nix.golden‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
{
2+
description = "A devbox shell";
3+
4+
inputs = {
5+
nixpkgs.url = "";
6+
flake-utils.url = "github:numtide/flake-utils";
7+
};
8+
9+
outputs = { self, nixpkgs, flake-utils }:
10+
flake-utils.lib.eachDefaultSystem (system:
11+
let
12+
pkgs = (import nixpkgs {
13+
inherit system;
14+
config.allowUnfree = true;
15+
});
16+
in
17+
{
18+
devShell = pkgs.mkShell {
19+
buildInputs = with pkgs; [
20+
];
21+
};
22+
}
23+
);
24+
}

‎internal/impl/testdata/flake.nix.golden‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
{
2+
description = "A devbox shell";
3+
4+
inputs = {
5+
nixpkgs.url = "https://github.com/nixos/nixpkgs/archive/b9c00c1d41ccd6385da243415299b39aa73357be.tar.gz";
6+
flake-utils.url = "github:numtide/flake-utils";
7+
};
8+
9+
outputs = { self, nixpkgs, flake-utils }:
10+
flake-utils.lib.eachDefaultSystem (system:
11+
let
12+
pkgs = (import nixpkgs {
13+
inherit system;
14+
config.allowUnfree = true;
15+
});
16+
php = pkgs.php.withExtensions ({ enabled, all }: enabled ++ (with all; [ blackfire ]));
17+
php81Packages.composer = php.packages.composer;
18+
in
19+
{
20+
devShell = pkgs.mkShell {
21+
buildInputs = with pkgs; [
22+
php
23+
php81Packages.composer
24+
php81Extensions.blackfire
25+
flyctl
26+
postgresql
27+
tree
28+
git
29+
zsh
30+
openssh
31+
vim
32+
sqlite
33+
jq
34+
delve
35+
ripgrep
36+
shellcheck
37+
terraform
38+
xz
39+
zstd
40+
gnupg
41+
go_1_20
42+
python3
43+
graphviz
44+
];
45+
};
46+
}
47+
);
48+
}

‎internal/impl/tmpl/flake.nix.tmpl‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,23 +3,26 @@
33

44
inputs = {
55
nixpkgs.url = "{{ .NixpkgsInfo.URL }}";
6-
76
flake-utils.url = "github:numtide/flake-utils";
87
};
98

109
outputs = { self, nixpkgs, flake-utils }:
1110
flake-utils.lib.eachDefaultSystem (system:
12-
let pkgs = nixpkgs.legacyPackages.${system};
13-
{{- range .Definitions}}
14-
{{.}}
15-
{{- end }}
16-
17-
in {
11+
let
12+
pkgs = (import nixpkgs {
13+
inherit system;
14+
config.allowUnfree = true;
15+
});
16+
{{- range .Definitions}}
17+
{{.}}
18+
{{- end }}
19+
in
20+
{
1821
devShell = pkgs.mkShell {
1922
buildInputs = with pkgs; [
2023
{{- range .DevPackages}}
2124
{{.}}
22-
{{end -}}
25+
{{- end}}
2326
];
2427
};
2528
}

0 commit comments

Comments
 (0)