Skip to content

Commit bc719ca

Browse files
authored
[nix] Build all packages in same nix command (#2032)
## Summary This changes how `installNixPackagesToStore` calls `nix.Build` so that it installs all installables in same nix command. It does this by grouping all installables into two groups, one for allow-insecure and other for default and runs `build` on both groups (if not empty) I was able to remove `packageInstallErrorHandler` because it is already called in `packagesToInstallInStore` so we will never actually try to build something that can't build on the system, or if it requires `--allow-insecure` we will notice it then. This does change our segment logging to group packages together for install times, so we will need to do some more work on the metrics side to determine slow packages. ## How was it tested? * Installed multiple packages not present in my nix store at once, including a combination of secure and insecure ones. * Tried to install something that doesn't build on my system, saw correct error message ## Perf test Anecdotal test: Using the following <img width="257" alt="image" src="https://github.com/jetify-com/devbox/assets/544948/8b1ab39c-7a1b-40d3-ab5a-516cde3b1525"> Before <img width="261" alt="image" src="https://github.com/jetify-com/devbox/assets/544948/ce397f59-1eaf-41dc-8256-6306f36e596b"> After <img width="293" alt="image" src="https://github.com/jetify-com/devbox/assets/544948/55ed3214-ff1d-4115-b767-7ad5d29dedd3">
1 parent 0d3f7f9 commit bc719ca

File tree

1 file changed

+69
-60
lines changed

1 file changed

+69
-60
lines changed

internal/devbox/packages.go

Lines changed: 69 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -430,80 +430,89 @@ func (d *Devbox) InstallRunXPackages(ctx context.Context) error {
430430
// and installing in the nix profile (even if offline).
431431
func (d *Devbox) installNixPackagesToStore(ctx context.Context, mode installMode) error {
432432
packages, err := d.packagesToInstallInStore(ctx, mode)
433-
if err != nil {
433+
if err != nil || len(packages) == 0 {
434434
return err
435435
}
436436

437-
stepNum := 0
438-
total := len(packages)
439-
for _, pkg := range packages {
440-
stepNum += 1
437+
// --no-link to avoid generating the result objects
438+
flags := []string{"--no-link"}
439+
if mode == update {
440+
flags = append(flags, "--refresh")
441+
}
441442

442-
stepMsg := fmt.Sprintf("[%d/%d] %s", stepNum, total, pkg)
443-
fmt.Fprintf(d.stderr, stepMsg+"\n")
443+
args := &nix.BuildArgs{
444+
Flags: flags,
445+
Writer: d.stderr,
446+
}
447+
args.ExtraSubstituter, _ = d.providers.NixCache.URI(ctx)
448+
// TODO (Landau): handle errors that are not auth.ErrNotLoggedIn
449+
// Only lookup credentials if we have a cache to use
450+
if args.ExtraSubstituter != "" {
451+
creds, err := d.providers.NixCache.Credentials(ctx)
452+
if err == nil {
453+
args.Env = creds.Env()
454+
}
444455

445-
installables, err := pkg.Installables()
456+
u, err := user.Current()
446457
if err != nil {
447-
return err
458+
err = redact.Errorf("lookup current user: %v", err)
459+
debug.Log("error configuring cache: %v", err)
448460
}
461+
err = d.providers.NixCache.Configure(ctx, u.Username)
462+
if err != nil {
463+
debug.Log("error configuring cache: %v", err)
449464

450-
// --no-link to avoid generating the result objects
451-
flags := []string{"--no-link"}
452-
if mode == update {
453-
flags = append(flags, "--refresh")
465+
var daemonErr *nix.DaemonError
466+
if errors.As(err, &daemonErr) {
467+
// Error here to give the user a chance to restart the daemon.
468+
return usererr.New("Devbox configured Nix to use %q as a cache. Please restart the Nix daemon and re-run Devbox.", args.ExtraSubstituter)
469+
}
470+
// Other errors indicate we couldn't update nix.conf, so just warn and continue
471+
// by building from source if necessary.
472+
ux.Fwarning(d.stderr, "Devbox was unable to configure Nix to use your organization's private cache. Some packages might be built from source.")
454473
}
474+
}
455475

456-
args := &nix.BuildArgs{
457-
AllowInsecure: pkg.HasAllowInsecure(),
458-
Flags: flags,
459-
Writer: d.stderr,
476+
packageNames := lo.Map(
477+
packages,
478+
func(p *devpkg.Package, _ int) string { return p.Raw },
479+
)
480+
ux.Finfo(
481+
d.stderr,
482+
"Installing the following packages to the nix store: %s\n",
483+
strings.Join(packageNames, ", "),
484+
)
485+
486+
installables := map[bool][]string{false: {}, true: {}}
487+
for _, pkg := range packages {
488+
pkgInstallables, err := pkg.Installables()
489+
if err != nil {
490+
return err
460491
}
461-
args.ExtraSubstituter, _ = d.providers.NixCache.URI(ctx)
462-
// TODO (Landau): handle errors that are not auth.ErrNotLoggedIn
463-
// Only lookup credentials if we have a cache to use
464-
if args.ExtraSubstituter != "" {
465-
creds, err := d.providers.NixCache.Credentials(ctx)
466-
if err == nil {
467-
args.Env = creds.Env()
468-
}
492+
installables[pkg.HasAllowInsecure()] = append(
493+
installables[pkg.HasAllowInsecure()],
494+
pkgInstallables...,
495+
)
496+
}
469497

470-
u, err := user.Current()
471-
if err != nil {
472-
err = redact.Errorf("lookup current user: %v", err)
473-
debug.Log("error configuring cache: %v", err)
474-
}
475-
err = d.providers.NixCache.Configure(ctx, u.Username)
476-
if err != nil {
477-
debug.Log("error configuring cache: %v", err)
478-
479-
var daemonErr *nix.DaemonError
480-
if errors.As(err, &daemonErr) {
481-
// Error here to give the user a chance to restart the daemon.
482-
return usererr.New("Devbox configured Nix to use %q as a cache. Please restart the Nix daemon and re-run Devbox.", args.ExtraSubstituter)
483-
}
484-
// Other errors indicate we couldn't update nix.conf, so just warn and continue
485-
// by building from source if necessary.
486-
ux.Fwarning(d.stderr, "Devbox was unable to configure Nix to use your organization's private cache. Some packages might be built from source.")
487-
}
498+
for allowInsecure, installables := range installables {
499+
if len(installables) == 0 {
500+
continue
488501
}
489-
for _, installable := range installables {
490-
eventStart := time.Now()
491-
err = nix.Build(ctx, args, installable)
492-
if err != nil {
493-
fmt.Fprintf(d.stderr, "%s: ", stepMsg)
494-
color.New(color.FgRed).Fprintf(d.stderr, "Fail\n")
495-
return packageInstallErrorHandler(err, pkg, installable)
496-
}
497-
telemetry.Event(telemetry.EventNixBuildSuccess, telemetry.Metadata{
498-
EventStart: eventStart,
499-
Packages: []string{pkg.Raw},
500-
})
502+
eventStart := time.Now()
503+
args.AllowInsecure = allowInsecure
504+
err = nix.Build(ctx, args, installables...)
505+
if err != nil {
506+
color.New(color.FgRed).Fprintf(d.stderr, "Fail\n")
507+
return err
501508
}
502-
503-
fmt.Fprintf(d.stderr, "%s: ", stepMsg)
504-
color.New(color.FgGreen).Fprintf(d.stderr, "Success\n")
509+
telemetry.Event(telemetry.EventNixBuildSuccess, telemetry.Metadata{
510+
EventStart: eventStart,
511+
Packages: packageNames,
512+
})
505513
}
506-
return err
514+
515+
return nil
507516
}
508517

509518
func (d *Devbox) packagesToInstallInStore(ctx context.Context, mode installMode) ([]*devpkg.Package, error) {
@@ -540,7 +549,7 @@ func (d *Devbox) packagesToInstallInStore(ctx context.Context, mode installMode)
540549
}
541550
}
542551

543-
return packagesToInstall, nil
552+
return lo.Uniq(packagesToInstall), nil
544553
}
545554

546555
// packageInstallErrorHandler checks for two kinds of errors to print custom messages for so that Devbox users

0 commit comments

Comments
 (0)