Skip to content
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

patchpkg: don't error on missing store refs #2293

Merged
merged 3 commits into from
Sep 23, 2024
Merged

Conversation

gcurtis
Copy link
Collaborator

@gcurtis gcurtis commented Sep 23, 2024

Don't exit non-zero when devbox patch is unable to restore some of the missing references to Python build dependencies.

Fixes #2289.

Don't exit non-zero when `devbox patch` is unable to restore some of
the missing references to Python build dependencies.

Fixes #2289.
@gcurtis gcurtis requested review from mikeland73 and savil September 23, 2024 14:42
Copy link
Collaborator

@savil savil left a comment

Choose a reason for hiding this comment

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

Cool, had to catch up on #2248 and #2249 which I hadn't reviewed already.

The enabled tests are unrelated to the restore-refs issue, right?

// 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.

@gcurtis
Copy link
Collaborator Author

gcurtis commented Sep 23, 2024

The enabled tests are unrelated to the restore-refs issue, right?

Right, I just enabled them by default when I added another test for this bug.

@gcurtis gcurtis merged commit f8f5511 into main Sep 23, 2024
24 checks passed
@gcurtis gcurtis deleted the gcurtis/patch-error branch September 23, 2024 19:05
@Lagoja Lagoja mentioned this pull request Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Error run devbox with python 3.10
3 participants