devpkg: fix empty patch mode (#2278)

Fix a case where an empty patch mode wouldn't default to "auto". This
was causing Devbox to skip patching when it should not have.

Got rid of the sync.OnceValue function for determining if a package
needs to be patched. This was originally done to avoid a call to
nix.System(), but that call doesn't happen anymore.

Author: gcurtis

internal/devpkg/narinfo_cache.go

@@ -219,8 +219,8 @@ func (p *Package) outputsForOutputName(output string) ([]lock.Output, error) {
 // the package to query it from the binary cache.
 func (p *Package) isEligibleForBinaryCache() (bool, error) {
 	defer debug.FunctionTimer().End()
-	// Patched glibc packages are not in the binary cache.
-	if p.PatchGlibc() {
+	// Patched packages are not in the binary cache.
+	if p.Patch {
 		return false, nil
 	}
 	sysInfo, err := p.sysInfoIfExists()

internal/devpkg/package.go

@@ -4,9 +4,11 @@
 package devpkg
 
 import (
+	"cmp"
 	"context"
 	"fmt"
 	"io"
+	"log/slog"
 	"path/filepath"
 	"regexp"
 	"strings"
@@ -81,10 +83,9 @@ type Package struct {
 	// Outputs is a list of outputs to build from the package's derivation.
 	outputs outputs
 
-	// patch applies a function to the package's derivation that
-	// patches any ELF binaries to use the latest version of nixpkgs#glibc.
-	// It's a function to allow deferring nix System call until it's needed.
-	patch func() bool
+	// Patch controls if Devbox environments will do additional patching to
+	// address known issues with the package.
+	Patch bool
 
 	// AllowInsecure are a list of nix packages that are whitelisted to be
 	// installed even if they are marked as insecure.
@@ -110,7 +111,7 @@ func PackagesFromConfig(packages []configfile.Package, l lock.Locker) []*Package
 	for _, cfgPkg := range packages {
 		pkg := newPackage(cfgPkg.VersionedName(), cfgPkg.IsEnabledOnPlatform, l)
 		pkg.DisablePlugin = cfgPkg.DisablePlugin
-		pkg.patch = patchGlibcFunc(pkg.CanonicalName(), cfgPkg.Patch)
+		pkg.Patch = pkgNeedsPatch(pkg.CanonicalName(), cfgPkg.Patch)
 		pkg.outputs.selectedNames = lo.Uniq(append(pkg.outputs.selectedNames, cfgPkg.Outputs...))
 		pkg.AllowInsecure = cfgPkg.AllowInsecure
 		result = append(result, pkg)
@@ -125,7 +126,7 @@ func PackageFromStringWithDefaults(raw string, locker lock.Locker) *Package {
 func PackageFromStringWithOptions(raw string, locker lock.Locker, opts devopt.AddOpts) *Package {
 	pkg := PackageFromStringWithDefaults(raw, locker)
 	pkg.DisablePlugin = opts.DisablePlugin
-	pkg.patch = patchGlibcFunc(pkg.CanonicalName(), configfile.PatchMode(opts.Patch))
+	pkg.Patch = pkgNeedsPatch(pkg.CanonicalName(), configfile.PatchMode(opts.Patch))
 	pkg.outputs.selectedNames = lo.Uniq(append(pkg.outputs.selectedNames, opts.Outputs...))
 	pkg.AllowInsecure = opts.AllowInsecure
 	return pkg
@@ -155,6 +156,7 @@ func newPackage(raw string, isInstallable func() bool, locker lock.Locker) *Pack
 	pkg.resolve = sync.OnceValue(func() error { return nil })
 	pkg.setInstallable(parsed, locker.ProjectDir())
 	pkg.outputs = outputs{selectedNames: strings.Split(parsed.Outputs, ",")}
+	pkg.Patch = pkgNeedsPatch(pkg.CanonicalName(), configfile.PatchAuto)
 	return pkg
 }
 
@@ -177,27 +179,31 @@ func resolve(pkg *Package) error {
 	return nil
 }
 
-func patchGlibcFunc(canonicalName string, mode configfile.PatchMode) func() bool {
-	return sync.OnceValue(func() (patch bool) {
-		switch mode {
-		case configfile.PatchAuto:
-			patch = canonicalName == "python"
-		case configfile.PatchAlways:
-			patch = true
-		case configfile.PatchNever:
-			patch = false
-		}
-		return patch
-	})
-}
-
 func (p *Package) setInstallable(i flake.Installable, projectDir string) {
 	if i.Ref.Type == flake.TypePath && !filepath.IsAbs(i.Ref.Path) {
 		i.Ref.Path = filepath.Join(projectDir, i.Ref.Path)
 	}
 	p.installable = i
 }
 
+func pkgNeedsPatch(canonicalName string, mode configfile.PatchMode) (patch bool) {
+	mode = cmp.Or(mode, configfile.PatchAuto)
+	switch mode {
+	case configfile.PatchAuto:
+		patch = canonicalName == "python"
+	case configfile.PatchAlways:
+		patch = true
+	case configfile.PatchNever:
+		patch = false
+	}
+	if patch {
+		slog.Debug("package needs patching", "pkg", canonicalName, "mode", mode)
+	} else {
+		slog.Debug("package doesn't need patching", "pkg", canonicalName, "mode", mode)
+	}
+	return patch
+}
+
 var inputNameRegex = regexp.MustCompile("[^a-zA-Z0-9-]+")
 
 // FlakeInputName generates a name for the input that will be used in the
@@ -249,10 +255,6 @@ func (p *Package) IsInstallable() bool {
 	return p.isInstallable()
 }
 
-func (p *Package) PatchGlibc() bool {
-	return p.patch != nil && p.patch()
-}
-
 // Installables for this package. Installables is a nix concept defined here:
 // https://nixos.org/manual/nix/stable/command-ref/new-cli/nix.html#installables
 func (p *Package) Installables() ([]string, error) {

internal/shellgen/flake_input.go

@@ -84,7 +84,7 @@ func (f *flakeInput) BuildInputsForSymlinkJoin() ([]*SymlinkJoin, error) {
 			return nil, err
 		}
 
-		if pkg.PatchGlibc() {
+		if pkg.Patch {
 			return nil, errors.New("patch_glibc is not yet supported for packages with non-default outputs")
 		}
 
@@ -125,7 +125,7 @@ func (f *flakeInput) BuildInputs() ([]string, error) {
 		if attributePathErr != nil {
 			err = attributePathErr
 		}
-		if pkg.PatchGlibc() {
+		if pkg.Patch {
 			// When the package comes from the glibc flake, the
 			// "legacyPackages" portion of the attribute path
 			// becomes just "packages" (matching the standard flake
@@ -179,7 +179,7 @@ func flakeInputs(ctx context.Context, packages []*devpkg.Package) []flakeInput {
 		// Packages that need a glibc patch are assigned to the special
 		// glibc-patched flake input. This input refers to the
 		// glibc-patch.nix flake.
-		if pkg.PatchGlibc() {
+		if pkg.Patch {
 			nixpkgsGlibc := flakeInputs.getOrAppend(glibcPatchFlakeRef)
 			nixpkgsGlibc.Name = "glibc-patch"
 			nixpkgsGlibc.URL = glibcPatchFlakeRef

internal/shellgen/flake_plan.go

@@ -106,7 +106,7 @@ func newGlibcPatchFlake(nixpkgsGlibcRev string, packages []*devpkg.Package) (gli
 		NixpkgsGlibcFlakeRef: "flake:nixpkgs/" + nixpkgsGlibcRev,
 	}
 	for _, pkg := range packages {
-		if !pkg.PatchGlibc() {
+		if !pkg.Patch {
 			continue
 		}