Skip to content
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
62 changes: 36 additions & 26 deletions internal/patchpkg/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,32 +96,11 @@ func (d *DerivationBuilder) Build(ctx context.Context, pkgStorePath string) erro

func (d *DerivationBuilder) build(ctx context.Context, pkg, out *packageFS) error {
if d.RestoreRefs {
// Find store path references to build inputs that were removed
// from Python.
refs, err := d.findRemovedRefs(ctx, pkg)
if err != nil {
return err
}

// Group the references we want to restore by file path.
d.bytePatches = make(map[string][]fileSlice, len(refs))
for _, ref := range refs {
d.bytePatches[ref.path] = append(d.bytePatches[ref.path], ref)
}

// If any of those references have shared libraries, add them
// back to Python's RPATH.
if d.glibcPatcher != nil {
nixStore := cmp.Or(os.Getenv("NIX_STORE"), "/nix/store")
seen := make(map[string]bool)
for _, ref := range refs {
storePath := filepath.Join(nixStore, string(ref.data))
if seen[storePath] {
continue
}
seen[storePath] = true
d.glibcPatcher.prependRPATH(newPackageFS(storePath))
}
if err := d.restoreMissingRefs(ctx, pkg); err != nil {
// Don't break the flake build if we're unable to
// restore some of the refs. Having some is still an
// improvement.
slog.ErrorContext(ctx, "unable to restore all removed refs", "err", err)
Copy link
Collaborator

Choose a reason for hiding this comment

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

any insight into the possible causes for why we're unable to restore some of the refs?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not entirely sure. It finds missing refs by searching for the package name (in this case llvm-16.0.6) in the buildInputs dependency tree of Python. Finding the packages this way is really just a heuristic, so it's not implausible that we miss something.

}
}

Expand Down Expand Up @@ -152,6 +131,37 @@ func (d *DerivationBuilder) build(ctx context.Context, pkg, out *packageFS) erro
return cmd.Run()
}

func (d *DerivationBuilder) restoreMissingRefs(ctx context.Context, pkg *packageFS) error {
// Find store path references to build inputs that were removed
// from Python.
refs, err := d.findRemovedRefs(ctx, pkg)
if err != nil {
return err
}

// Group the references we want to restore by file path.
d.bytePatches = make(map[string][]fileSlice, len(refs))
for _, ref := range refs {
d.bytePatches[ref.path] = append(d.bytePatches[ref.path], ref)
}

// If any of those references have shared libraries, add them
// back to Python's RPATH.
if d.glibcPatcher != nil {
nixStore := cmp.Or(os.Getenv("NIX_STORE"), "/nix/store")
seen := make(map[string]bool)
for _, ref := range refs {
storePath := filepath.Join(nixStore, string(ref.data))
if seen[storePath] {
continue
}
seen[storePath] = true
d.glibcPatcher.prependRPATH(newPackageFS(storePath))
}
}
return nil
}

func (d *DerivationBuilder) copyDir(out *packageFS, path string) error {
path, err := out.OSPath(path)
if err != nil {
Expand Down
96 changes: 96 additions & 0 deletions testscripts/languages/python_patch_missing_ref.test.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
# Python Auto-Patch Handles Missing Ref
#
# Check that `devbox patch --restore-refs` doesn't break the flake build when a
# a store path cannot be restored.
#
# The nixpkgs commit hash and version of Python chosen in this test is very
# specific. Most versions don't encounter this error, so be careful that the
# test still fails with Devbox v0.13.0 if changing the devbox.lock.
#
# https://github.com/jetify-com/devbox/issues/2289

exec devbox install

-- devbox.json --
{
"packages": {
"python": "latest"
},
"env": {
"PIP_DISABLE_PIP_VERSION_CHECK": "1",
"PIP_NO_INPUT": "1",
"PIP_NO_PYTHON_VERSION_WARNING": "1",
"PIP_PROGRESS_BAR": "off",
"PIP_REQUIRE_VIRTUALENV": "1",
"PIP_ROOT_USER_ACTION": "ignore"
},
"shell": {
"scripts": {
"venv": ". $VENV_DIR/bin/activate && \"$@\""
}
}
}

-- devbox.lock --
{
"lockfile_version": "1",
"packages": {
"python@latest": {
"last_modified": "2024-09-10T15:01:03Z",
"plugin_version": "0.0.4",
"resolved": "github:NixOS/nixpkgs/5ed627539ac84809c78b2dd6d26a5cebeb5ae269#python3",
"source": "devbox-search",
"version": "3.12.5",
"systems": {
"aarch64-darwin": {
"outputs": [
{
"name": "out",
"path": "/nix/store/9pj4rzx5pbynkkxq1srzwjhywmcfxws3-python3-3.12.5",
"default": true
}
],
"store_path": "/nix/store/9pj4rzx5pbynkkxq1srzwjhywmcfxws3-python3-3.12.5"
},
"aarch64-linux": {
"outputs": [
{
"name": "out",
"path": "/nix/store/6iq3nhgdyp8a5wzwf097zf2mn4zyqxr6-python3-3.12.5",
"default": true
},
{
"name": "debug",
"path": "/nix/store/xc4hygp28y7g1rvjf0vi7fj0d83a75pj-python3-3.12.5-debug"
}
],
"store_path": "/nix/store/6iq3nhgdyp8a5wzwf097zf2mn4zyqxr6-python3-3.12.5"
},
"x86_64-darwin": {
"outputs": [
{
"name": "out",
"path": "/nix/store/ks8acr22s4iggnmvxydm5czl30racy32-python3-3.12.5",
"default": true
}
],
"store_path": "/nix/store/ks8acr22s4iggnmvxydm5czl30racy32-python3-3.12.5"
},
"x86_64-linux": {
"outputs": [
{
"name": "out",
"path": "/nix/store/h3i0acpmr8mrjx07519xxmidv8mpax4y-python3-3.12.5",
"default": true
},
{
"name": "debug",
"path": "/nix/store/0a39pi2s6kxqc3kjjz2y9yzibd62zhhb-python3-3.12.5-debug"
}
],
"store_path": "/nix/store/h3i0acpmr8mrjx07519xxmidv8mpax4y-python3-3.12.5"
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@
# libstdc++.so. The nixpkgs Python interpreter doesn't search standard system
# paths, so Devbox must patch it or provide the location of native dependencies.

[!env:DEVBOX_RUN_FAILING_TESTS] skip 'this test doesn''t pass on Linux yet'

exec devbox install

# pip install numpy
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
# Check that an older version of the Python interpreter (3.7) can import and run
# pip packages that are built from source.

[!env:DEVBOX_RUN_FAILING_TESTS] skip 'this test doesn''t pass on Linux yet'

exec devbox install

# pip install psycopg2
Expand Down
Loading