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

fix: use original interpreter path as 'home' value in pyvenv.cfg #226

Merged
merged 1 commit into from
Nov 20, 2023
Merged

Conversation

mattem
Copy link
Member

@mattem mattem commented Nov 20, 2023

Use the original (source) interpreters bin path as the 'home' value in the generated pyvenv.cfg file.

Closes #159


Type of change

  • Bug fix (change which fixes an issue)

For changes visible to end-users

  • Suggested release notes are provided below:

Ensure correct home pathing is used when generating the venv.

Test plan

  • Covered by existing test cases
  • Manual testing; please provide instructions so we can reproduce:

Updated e2e smoke to use py3.11

@mattem mattem merged commit e5c2112 into main Nov 20, 2023
2 checks passed
@mattem mattem deleted the py311 branch November 20, 2023 15:42
@shinypb
Copy link
Contributor

shinypb commented Nov 27, 2023

Thanks for making this fix, @mattem! This patch absolutely fixed my Python 3.11 woes.

@alexeagle Any chance of you doing a release in the near future? I've pinned our usage of aspect_rules_py to this commit in the meantime, but I'd love to get back to using a bazel_mod statement rather than git_repository if possible.

I appreciate you both! Thank you. 🙇

@alexeagle
Copy link
Member

@shinypb sure I'll push a tag right now.

@shinypb
Copy link
Contributor

shinypb commented Nov 28, 2023

@alexeagle Howdy! That never made it to the central registry 🤔 It's still showing 0.4.0 as the latest release:
https://registry.bazel.build/modules/aspect_rules_py

@honnix
Copy link

honnix commented Nov 30, 2023

Thanks for fixing this.

If I'm not mistaken,

echo "home = ${VBIN_LOCATION}" > "${PYVENV_CFG}"
needs the same treatment.

I tried using:

py_venv(
    name = "my_venv",
    deps = ["//src:my_lib"],
)

to create a venv under <workspace root>/.my_venv and the content of pyvenv.cfg is not correct.

Also the content of first_party.path also looks strange:

../../../..
../../../../site-packages
../../../../site-packages
../../../../site-packages
../../../../site-packages
../../../../site-packages
../../../../src
../../../../

I have dependencies of my_lib so I assume those ../../../../site-packages are due to that.

honnix added a commit to honnix/rules_py that referenced this pull request Nov 30, 2023
@honnix
Copy link

honnix commented Nov 30, 2023

I tried proposing a small fix for the pyvenv.cfg in #228 . Unfortunately I will not be able to sign CLA personally, so please feel free to discard the PR and get the fix in. Thank you.

mattem added a commit that referenced this pull request Dec 18, 2023
This is a follow-up of #226 to fix another place.
mattem added a commit that referenced this pull request Dec 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Python 3.11
4 participants