Skip to content

[glibc] Add patch-glibc patch and fix when package in binary cache #1669

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 2 commits into from
Dec 6, 2023

Conversation

mikeland73
Copy link
Contributor

Summary

  • Adds new --patch-glibc flag
  • Fixes glibc feature (I think) when package is in binary cache. We were previouwly always installing the cached version instead of the patched version.

How was it tested?

  • Flag was tested using devbox add python --patch-glibc
  • I'm not 100% sure how to test the patch itself, but I did verify we are creating the patch flake

@mikeland73 mikeland73 requested review from loreto and savil December 6, 2023 19:43
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.

Great! I have a couple of projects I can test this locally with.

pkgs.ast.setPackageBool(name, "patch_glibc", v)
}
return nil
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

just to confirm, if the packages is a []string in json, then this will convert it to a map[string] (string | object)?

@savil
Copy link
Collaborator

savil commented Dec 6, 2023

I'm not 100% sure how to test the patch itself, but I did verify we are creating the patch flake

wait, we should only be generating the flake on linux (since this issue affects linux, and not macos)

@@ -25,6 +25,10 @@ const BinaryCache = "https://cache.nixos.org"
// ALERT: Callers in a perf-sensitive code path should call FillNarInfoCache
// before calling this function.
func (p *Package) IsInBinaryCache() (bool, error) {
// Patched glibc packages are not in the binary cache.
if p.PatchGlibc {
Copy link
Collaborator

Choose a reason for hiding this comment

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

this should be PatchGlibc && linux

or better still the PatchGlibc boolean should only be set if it is specified in the devbox.json and the current OS is linux

@mikeland73
Copy link
Contributor Author

wait, we should only be generating the flake on linux (since this issue affects linux, and not macos)

Only the linux version is patched but the flake still gets created. I'll fix

@mikeland73 mikeland73 merged commit 8c49985 into main Dec 6, 2023
@mikeland73 mikeland73 deleted the landau/patch-glibc branch December 6, 2023 21:04
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.

2 participants