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

regression: pip_compile ignores env attribute #2270

Closed
kpark-hrp opened this issue Oct 3, 2024 · 3 comments · Fixed by #2277
Closed

regression: pip_compile ignores env attribute #2270

kpark-hrp opened this issue Oct 3, 2024 · 3 comments · Fixed by #2277

Comments

@kpark-hrp
Copy link

🐞 bug report

Affected Rule

The issue is caused by the rule: pip_compile but exposed to public as compile_pip_requirements

During this change below, env parameter used to be captured by kwargs and then passed as attrs to respective py_binary. But now env within kwargs is being ignored, so any environment variables that is set with pip_compile rule is being ignored.

e923f9e#diff-a92cc6468f47191b8e22d4bbe2564d3968d3dd4bbc383b9f3df3c4c520e1583eL147-R165

Is this a regression?

Yes. env parameter that was passed to pip_compile was respected in 0.34.0 but now it's not on 0.36.0.

Description

A clear and concise description of the problem...

🔬 Minimal Reproduction

compile_pip_requirements(
    ...,
    env = {
        "PIP_CONFIG_FILE": "<path>",
    },
    ...
)

Then run bazel run :requirements.update on 0.34.0 vs 0.36.0

🔥 Exception or Error

No particular exception, but the environment variables are not picked up when running bazel run :requirements.update

🌍 Your Environment

Operating System: MacOS 14.7

Output of bazel version: bazel 6.5.0

Rules_python version: 0.34.0 trying to upgrade to 0.36.0

@kpark-hrp kpark-hrp changed the title env parameter is dropped when forwarding to py_binary in pip_compile regression: pip_compile ignores when env attribute Oct 3, 2024
@kpark-hrp kpark-hrp changed the title regression: pip_compile ignores when env attribute regression: pip_compile ignores env attribute Oct 3, 2024
@aignas
Copy link
Collaborator

aignas commented Oct 7, 2024

FYI @cj81499 and @lpulley.

@lpulley
Copy link
Contributor

lpulley commented Oct 7, 2024

Whipped up #2277. Haven't tested it yet, but does it seem reasonable thus far?

@lpulley
Copy link
Contributor

lpulley commented Oct 7, 2024

I'm not sure there's a great way to write a test for this, but the X.update and X_test targets are the same except that X_test gets a different env and gets **kwargs passed through.

I did test an X.update by hand on my patch with

env = {"CUSTOM_COMPILE_COMMAND": "__TEST_REQUIREMENTS_CUSTOM_COMPILE_COMMAND__"}

and that works as expected, so I think it'll fix this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants