-
-
Notifications
You must be signed in to change notification settings - Fork 639
fix: Fix bazel vendor support for requirements with environment markers #2997
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
fix: Fix bazel vendor support for requirements with environment markers #2997
Conversation
Fixes `bazel vendor` support for requirements files that contain environment markers. During a vendored `bazel build`, when evaluate_markers_py() is run it needs PYTHONHOME set to properly find the home of the vendored libraries. Resolves (bazel-contrib#2996)
aignas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To make it possible to merge this it would be great if you could add a changelog note.
Any reason why you are using pip_parse instead of bzlmod example? It already is using the env markers.
| Label("@pypi__packaging//:BUILD.bazel"), | ||
| Label("//:BUILD.bazel"), | ||
| ], | ||
| "PYTHONHOME": str(interpreter.dirname), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Solution LGTM in general.
Done.
Ah, didn't realize that. Verified the |
aignas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved the CI config up to be next to other examples/bzlmod invocations.
Fixes
bazel vendorsupport for requirements files that contain environment markers. During a vendoredbazel build, when evaluate_markers_py() is run it needs PYTHONHOME set to properly find the home of the vendored libraries.Resolves #2996