Skip to content

devconfig,shellgen: option to patch ELF binaries with newer glibc #1574

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Oct 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions internal/devconfig/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@ func (c *Config) Equals(other *Config) bool {
}

func (c *Config) NixPkgsCommitHash() string {
// The commit hash for nixpkgs-unstable on 2023-01-25 from status.nixos.org
const DefaultNixpkgsCommit = "f80ac848e3d6f0c12c52758c0f25c10c97ca3b62"
// The commit hash for nixpkgs-unstable on 2023-10-25 from status.nixos.org
const DefaultNixpkgsCommit = "75a52265bda7fd25e06e3a67dee3f0354e73243c"

if c == nil || c.Nixpkgs == nil || c.Nixpkgs.Commit == "" {
return DefaultNixpkgsCommit
Expand Down
4 changes: 4 additions & 0 deletions internal/devconfig/packages.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,10 @@ type Package struct {

Platforms []string `json:"platforms,omitempty"`
ExcludedPlatforms []string `json:"excluded_platforms,omitempty"`

// PatchGlibc applies a function to the package's derivation that
// patches any ELF binaries to use the latest version of nixpkgs#glibc.
PatchGlibc bool `json:"patch_glibc,omitempty"`
}

func NewVersionOnlyPackage(name, version string) Package {
Expand Down
10 changes: 8 additions & 2 deletions internal/devpkg/package.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ type Package struct {
// example: github:nixos/nixpkgs/5233fd2ba76a3accb5aaa999c00509a11fd0793c#hello
Raw string

// PatchGlibc applies a function to the package's derivation that
// patches any ELF binaries to use the latest version of nixpkgs#glibc.
PatchGlibc bool

// isInstallable is true if the package may be enabled on the current platform.
isInstallable bool

Expand All @@ -65,8 +69,10 @@ func PackageFromStrings(rawNames []string, l lock.Locker) []*Package {

func PackagesFromConfig(config *devconfig.Config, l lock.Locker) []*Package {
result := []*Package{}
for _, pkg := range config.Packages.Collection {
result = append(result, newPackage(pkg.VersionedName(), pkg.IsEnabledOnPlatform(), l))
for _, cfgPkg := range config.Packages.Collection {
pkg := newPackage(cfgPkg.VersionedName(), cfgPkg.IsEnabledOnPlatform(), l)
pkg.PatchGlibc = cfgPkg.PatchGlibc
result = append(result, pkg)
}
return result
}
Expand Down
14 changes: 14 additions & 0 deletions internal/impl/devbox.go
Original file line number Diff line number Diff line change
Expand Up @@ -891,7 +891,14 @@ func (d *Devbox) computeNixEnv(ctx context.Context, usePrintDevEnvCache bool) (m
// Motivation: if a user removes a package from their devbox it should no longer
// be available in their environment.
buildInputs := strings.Split(env["buildInputs"], " ")
glibcPatchPath := ""
nixEnvPath = filterPathList(nixEnvPath, func(path string) bool {
// TODO(gcurtis): this is a massive hack. Please get rid
// of this and install the package to the profile.
if strings.Contains(path, "patched-glibc") {
glibcPatchPath = path
return true
}
for _, input := range buildInputs {
// input is of the form: /nix/store/<hash>-<package-name>-<version>
// path is of the form: /nix/store/<hash>-<package-name>-<version>/bin
Expand All @@ -904,6 +911,13 @@ func (d *Devbox) computeNixEnv(ctx context.Context, usePrintDevEnvCache bool) (m
})
debug.Log("PATH after filtering with buildInputs (%v) is: %s", buildInputs, nixEnvPath)

// TODO(gcurtis): this is a massive hack. Please get rid
// of this and install the package to the profile.
if glibcPatchPath != "" {
nixEnvPath = glibcPatchPath + ":" + nixEnvPath
debug.Log("PATH after glibc-patch hack is: %s", nixEnvPath)
}

runXPaths, err := d.RunXPaths(ctx)
if err != nil {
return nil, err
Expand Down
101 changes: 67 additions & 34 deletions internal/shellgen/flake_input.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,18 @@ package shellgen
import (
"context"
"runtime/trace"
"slices"
"strings"

"github.com/samber/lo"
"go.jetpack.io/devbox/internal/boxcli/featureflag"
"go.jetpack.io/devbox/internal/debug"
"go.jetpack.io/devbox/internal/devpkg"
"go.jetpack.io/devbox/internal/goutil"
"go.jetpack.io/devbox/internal/nix"
)

const glibcPatchFlakeRef = "path:./glibc-patch"

type flakeInput struct {
Name string
Packages []*devpkg.Package
Expand Down Expand Up @@ -54,6 +57,13 @@ func (f *flakeInput) BuildInputs() ([]string, error) {
if attributePathErr != nil {
err = attributePathErr
}
if pkg.PatchGlibc {
// When the package comes from the glibc flake, the
// "legacyPackages" portion of the attribute path
// becomes just "packages" (matching the standard flake
// output schema).
return strings.Replace(attributePath, "legacyPackages", "packages", 1)
}
return attributePath
})
if err != nil {
Expand All @@ -77,48 +87,71 @@ func (f *flakeInput) BuildInputs() ([]string, error) {
// i.e. have a commit hash and always resolve to the same package/version.
// Note: inputs returned by this function include plugin packages. (php only for now)
// It's not entirely clear we always want to add plugin packages to the top level
func flakeInputs(ctx context.Context, packages []*devpkg.Package) []*flakeInput {
func flakeInputs(ctx context.Context, packages []*devpkg.Package) []flakeInput {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was the motivation to returning the struct by value, instead of a pointer to the struct?

I generally find that returning a pointer like []*flakeInput leads to easier-to-follow code. For example, line 136 below feels kinda complicated because we're trying to return the struct-by-value.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slices should generally contain values unless there's a specific reason they're needed. A slice of pointers allocates each element separately on the heap and adds a layer of indirection. A slice of values is a single chunk of memory which is nicer for GC and locality. We weren't using any of the pointer semantics (other than keying the lookup while building the slice) so I figured I'd switch it.

I moved the "get-or-append" logic into its own method to try and address your feedback about line 136.

defer trace.StartRegion(ctx, "flakeInputs").End()

// Use the verbose name flakeInputs to distinguish from `inputs`
// which refer to `nix.Input` in most of the codebase.
flakeInputs := map[string]*flakeInput{}

packages = lo.Filter(packages, func(item *devpkg.Package, _ int) bool {
// Non nix packages (e.g. runx) don't belong in the flake
if !item.IsNix() {
return false
var flakeInputs keyedSlice
for _, pkg := range packages {
// Non-nix packages (e.g. runx) don't belong in the flake
if !pkg.IsNix() {
continue
}

// Include packages (like local or remote flakes) that cannot be
// fetched from a Binary Cache Store.
if !featureflag.RemoveNixpkgs.Enabled() {
return true
// Don't include cached packages (like local or remote flakes)
// that can be fetched from a Binary Cache Store.
if featureflag.RemoveNixpkgs.Enabled() {
// TODO(savil): return error?
cached, err := pkg.IsInBinaryCache()
if err != nil {
debug.Log("error checking if package is in binary cache: %v", err)
}
if err == nil && cached {
continue
}
}

inCache, err := item.IsInBinaryCache()
if err != nil {
// Ignore this error for now. TODO savil: return error?
return true
// 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 {
nixpkgsGlibc := flakeInputs.getOrAppend(glibcPatchFlakeRef)
nixpkgsGlibc.Name = "glibc-patch"
nixpkgsGlibc.URL = glibcPatchFlakeRef
nixpkgsGlibc.Packages = append(nixpkgsGlibc.Packages, pkg)
continue
}
return !inCache
})

order := []string{}
for _, pkg := range packages {
if flkInput, ok := flakeInputs[pkg.URLForFlakeInput()]; !ok {
order = append(order, pkg.URLForFlakeInput())
flakeInputs[pkg.URLForFlakeInput()] = &flakeInput{
Name: pkg.FlakeInputName(),
URL: pkg.URLForFlakeInput(),
Packages: []*devpkg.Package{pkg},
}
} else {
flkInput.Packages = lo.Uniq(
append(flakeInputs[pkg.URLForFlakeInput()].Packages, pkg),
)
pkgURL := pkg.URLForFlakeInput()
flake := flakeInputs.getOrAppend(pkgURL)
flake.Name = pkg.FlakeInputName()
flake.URL = pkgURL

// TODO(gcurtis): is the uniqueness check necessary? We're
// comparing pointers.
if !slices.Contains(flake.Packages, pkg) {
flake.Packages = append(flake.Packages, pkg)
}
}
return flakeInputs.slice
}

return goutil.PickByKeysSorted(flakeInputs, order)
// keyedSlice keys the elements of an append-only slice for fast lookups.
type keyedSlice struct {
slice []flakeInput
lookup map[string]int
}

// getOrAppend returns a pointer to the slice element with a given key. If the
// key doesn't exist, a new element is automatically appended to the slice. The
// pointer is valid until the next append.
func (k *keyedSlice) getOrAppend(key string) *flakeInput {
if k.lookup == nil {
k.lookup = make(map[string]int)
}
if i, ok := k.lookup[key]; ok {
return &k.slice[i]
}
k.slice = append(k.slice, flakeInput{})
k.lookup[key] = len(k.slice) - 1
return &k.slice[len(k.slice)-1]
}
93 changes: 92 additions & 1 deletion internal/shellgen/flake_plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ package shellgen

import (
"context"
"fmt"
"path/filepath"
"runtime/trace"
"strings"

"go.jetpack.io/devbox/internal/devpkg"
"go.jetpack.io/devbox/internal/nix"
Expand All @@ -13,8 +16,8 @@ import (
type flakePlan struct {
BinaryCache string
NixpkgsInfo *NixpkgsInfo
FlakeInputs []*flakeInput
Packages []*devpkg.Package
FlakeInputs []flakeInput
System string
}

Expand Down Expand Up @@ -65,3 +68,91 @@ func newFlakePlan(ctx context.Context, devbox devboxer) (*flakePlan, error) {
System: nix.System(),
}, nil
}

func (f *flakePlan) needsGlibcPatch() bool {
for _, in := range f.FlakeInputs {
if in.URL == glibcPatchFlakeRef {
return true
}
}
return false
}

type glibcPatchFlake struct {
// NixpkgsGlibcFlakeRef is a flake reference to the nixpkgs flake
// containing the new glibc package.
NixpkgsGlibcFlakeRef string

// Inputs is the attribute set of flake inputs. The key is the input
// name and the value is a flake reference.
Inputs map[string]string

// Outputs is the attribute set of flake outputs. It follows the
// standard flake output schema of system.name = derivation. The
// derivation can be any valid Nix expression.
Outputs struct {
Packages map[string]map[string]string
}
}

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

err := flake.addPackageOutput(pkg)
if err != nil {
return glibcPatchFlake{}, err
}
}
return flake, nil
}

func (g *glibcPatchFlake) addPackageOutput(pkg *devpkg.Package) error {
if g.Inputs == nil {
g.Inputs = make(map[string]string)
}
inputName := pkg.FlakeInputName()
g.Inputs[inputName] = pkg.URLForFlakeInput()

attrPath, err := pkg.FullPackageAttributePath()
if err != nil {
return err
}
// Remove the legacyPackages.<system> prefix.
outputName := strings.SplitN(attrPath, ".", 3)[2]

if g.Outputs.Packages == nil {
g.Outputs.Packages = map[string]map[string]string{nix.System(): {}}
}
if cached, err := pkg.IsInBinaryCache(); err == nil && cached {
if expr, err := g.fetchClosureExpr(pkg); err == nil {
g.Outputs.Packages[nix.System()][outputName] = expr
return nil
}
}
g.Outputs.Packages[nix.System()][outputName] = strings.Join([]string{"pkgs", inputName, nix.System(), outputName}, ".")
return nil
}

func (g *glibcPatchFlake) fetchClosureExpr(pkg *devpkg.Package) (string, error) {
storePath, err := pkg.InputAddressedPath()
if err != nil {
return "", err
}
return fmt.Sprintf(`builtins.fetchClosure {
fromStore = "%s";
fromPath = "%s";
inputAddressed = true;
}`, devpkg.BinaryCache, storePath), nil
}

func (g *glibcPatchFlake) writeTo(dir string) error {
err := writeFromTemplate(dir, g, "glibc-patch.nix", "flake.nix")
if err != nil {
return err
}
return writeGlibcPatchScript(filepath.Join(dir, "glibc-patch.bash"))
}
Loading