Skip to content

Commit 5326d5b

Browse files
authored
[perf] Install all packages to profile in a single nix command (#2130)
## Summary This change installs all packages into profile with same command. If we fail due to priority conflict, we fallback to the previous one by one installation. This significantly improves performance on projects with many packages when all packages are already in store. This aims to solve a common CICD case where nothing has changed, all packages are in store, but we need to create the .devbox directory and nix profile from scratch. cc: @Lagoja this helps address some reported issues. ## How was it tested? Happy path: On project with 30+ packages, delete `.devbox` directory and ran `devbox install`. This change reduced total time from 10s to 2s. Sad path: On new project added `curl`, `ruby`, `bundler`, `go`. `ruby` and `bundler` conflict. I ran `devbox install` and it correctly fell back to one by one. I inspected the profile to ensure nix did not install anything twice.
1 parent b62b346 commit 5326d5b

File tree

4 files changed

+48
-45
lines changed

4 files changed

+48
-45
lines changed

internal/devbox/nixprofile.go

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package devbox
22

33
import (
44
"context"
5+
"errors"
56
"fmt"
67
"log/slog"
78
"strings"
@@ -62,22 +63,24 @@ func (d *Devbox) syncNixProfileFromFlake(ctx context.Context) error {
6263
}
6364
}
6465
if len(add) > 0 {
65-
// We need to install the packages in the nix profile one-by-one because
66-
// we do checks for insecure packages.
67-
// TODO: move the insecure package check here, and do `nix profile install installables...`
68-
// in one command for speed.
69-
for _, addPath := range add {
70-
if err = nix.ProfileInstall(ctx, &nix.ProfileInstallArgs{
71-
Installable: addPath,
72-
// Install in offline mode for speed. We know we should have all the files
73-
// locally in /nix/store since we have run `nix print-dev-env` prior to this.
74-
// Also avoids some "substituter not found for store-path" errors.
75-
Offline: true,
76-
ProfilePath: profilePath,
77-
Writer: d.stderr,
78-
}); err != nil {
79-
return fmt.Errorf("error installing package in nix profile %s: %w", addPath, err)
66+
if err = nix.ProfileInstall(ctx, &nix.ProfileInstallArgs{
67+
Installables: add,
68+
ProfilePath: profilePath,
69+
Writer: d.stderr,
70+
}); errors.Is(err, nix.ErrPriorityConflict) {
71+
// We need to install the packages one by one because there was possibly a priority conflict
72+
// This is slower, but uncommon.
73+
for _, addPath := range add {
74+
if err = nix.ProfileInstall(ctx, &nix.ProfileInstallArgs{
75+
Installables: []string{addPath},
76+
ProfilePath: profilePath,
77+
Writer: d.stderr,
78+
}); err != nil {
79+
return fmt.Errorf("error installing package in nix profile %s: %w", addPath, err)
80+
}
8081
}
82+
} else if err != nil {
83+
return fmt.Errorf("error installing packages in nix profile %s: %w", add, err)
8184
}
8285
}
8386
return nil

internal/nix/eval.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,6 @@ func Eval(path string) ([]byte, error) {
5555
return cmd.CombinedOutput(context.TODO())
5656
}
5757

58-
func AllowInsecurePackages() {
59-
os.Setenv("NIXPKGS_ALLOW_INSECURE", "1")
60-
}
61-
6258
func IsInsecureAllowed() bool {
6359
allowed, _ := strconv.ParseBool(os.Getenv("NIXPKGS_ALLOW_INSECURE"))
6460
return allowed

internal/nix/profiles.go

Lines changed: 21 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
package nix
55

66
import (
7+
"bytes"
78
"context"
89
"encoding/json"
910
"fmt"
@@ -14,7 +15,6 @@ import (
1415
"path/filepath"
1516

1617
"github.com/pkg/errors"
17-
"go.jetpack.io/devbox/internal/boxcli/usererr"
1818
"go.jetpack.io/devbox/internal/debug"
1919
"go.jetpack.io/devbox/internal/redact"
2020
)
@@ -32,48 +32,43 @@ func ProfileList(writer io.Writer, profilePath string, useJSON bool) (string, er
3232
}
3333

3434
type ProfileInstallArgs struct {
35-
Installable string
36-
Offline bool
37-
ProfilePath string
38-
Writer io.Writer
35+
Installables []string
36+
ProfilePath string
37+
Writer io.Writer
3938
}
4039

40+
var ErrPriorityConflict = errors.New("priority conflict")
41+
4142
func ProfileInstall(ctx context.Context, args *ProfileInstallArgs) error {
4243
defer debug.FunctionTimer().End()
43-
if !IsInsecureAllowed() && PackageIsInsecure(args.Installable) {
44-
knownVulnerabilities := PackageKnownVulnerabilities(args.Installable)
45-
errString := fmt.Sprintf("Package %s is insecure. \n\n", args.Installable)
46-
if len(knownVulnerabilities) > 0 {
47-
errString += fmt.Sprintf("Known vulnerabilities: %s \n\n", knownVulnerabilities)
48-
}
49-
errString += "To override use `devbox add <pkg> --allow-insecure`"
50-
return usererr.New(errString)
51-
}
5244

5345
cmd := command(
5446
"profile", "install",
5547
"--profile", args.ProfilePath,
56-
"--impure", // for NIXPKGS_ALLOW_UNFREE
48+
"--offline", // makes it faster. Package is already in store
49+
"--impure", // for NIXPKGS_ALLOW_UNFREE
5750
// Using an arbitrary priority to avoid conflicts with other packages.
5851
// Note that this is not really the priority we care about, since we
5952
// use the flake.nix to specify the priority.
6053
"--priority", nextPriority(args.ProfilePath),
6154
)
62-
if args.Offline {
63-
cmd.Args = append(cmd.Args, "--offline")
64-
}
65-
cmd.Args = append(cmd.Args, args.Installable)
55+
56+
cmd.Args = appendArgs(cmd.Args, args.Installables)
6657
cmd.Env = allowUnfreeEnv(os.Environ())
6758

68-
// If nix profile install runs as tty, the output is much nicer. If we ever
69-
// need to change this to our own writers, consider that you may need
70-
// to implement your own nicer output. --print-build-logs flag may be useful.
71-
cmd.Stdin = os.Stdin
72-
cmd.Stdout = args.Writer
73-
cmd.Stderr = args.Writer
59+
// We used to attach this function to stdout and in in order to get the more interactive output.
60+
// However, now we do the building in nix.Build, by the time we install in profile everything
61+
// should already be in the store. We need to capture the output so we can decide if a conflict
62+
// happened.
7463

7564
slog.Debug("running command", "cmd", cmd)
76-
return cmd.Run(ctx)
65+
out, err := cmd.CombinedOutput(ctx)
66+
67+
if bytes.Contains(out, []byte("error: An existing package already provides the following file")) {
68+
return ErrPriorityConflict
69+
}
70+
71+
return err
7772
}
7873

7974
// ProfileRemove removes packages from a profile.

testscripts/add/add_insecure.tst.txt

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
# Tests installing an insecure package.
2+
# This test is pretty slow, maybe there's a different package we can
3+
# use for testing.
4+
5+
# we could also isolate this test and run on its own.
6+
7+
exec devbox init
8+
exec devbox add [email protected] --allow-insecure python-2.7.18.6
9+
exec devbox install

0 commit comments

Comments
 (0)