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

python: avoid using existing virtual env or poetry binary #552

Merged
merged 4 commits into from
Apr 26, 2023

Conversation

bobvanderlinden
Copy link
Contributor

@bobvanderlinden bobvanderlinden commented Apr 24, 2023

Currently poetry is called in one instance with just poetry. There were cases where the poetry binary from devenv's root project leaked into the devenv of the python-poetry test. This often is not a problem, but in this case it was a poetry version that was built against a different glibc, resulting in an error.

I've tried to harden the poetry module a bit more by explicitly calling all executables to avoid leakage of executables from a parent environment.

While debugging also found a minor issue when manually executing poetry after removing .venv, poetry will create a new .venv directory. In this case devenv should remove it and create a new symlink. This is what ln -sf was meant to do, however it instead created a symlink inside the .venv directory. I tried to avoid this by using --no-target-directory.

Lastly, this resets the VIRTUAL_ENV so that poetry won't try to use python from a VIRTUAL_ENV that was already active before devenv shell started.

@bobvanderlinden bobvanderlinden marked this pull request as ready for review April 24, 2023 15:45
@domenkozar
Copy link
Member

Seems to still have the same failure

This is needed as tools like poetry looks for VIRTUAL_ENV and attempts
to use the python interpreter from this environment.

We want poetry to use the python interpreter from PATH (set by devenv).
We want it to configure the python interpreter for the new venv.
@bobvanderlinden
Copy link
Contributor Author

Seems to still have the same failure

I had a hard time understanding what was going wrong. I think it's a combination:

It seems that all examples (that I run using devenv-test-example) are run against devenv modules from github:cachix/devenv. To circumvent this during development I used:

inputs:
  nixpkgs:
    url: github:NixOS/nixpkgs/nixpkgs-unstable
  devenv:
    url: /home/bob.vanderlinden/projects/devenv/src/modules

Next it seemed as it poetry would default to using a different Python version than what was in PATH. It used the one from the devenv root's venv. Unsetting VIRTUAL_ENV seemed to fix the problem locally.

I had a hard time getting stable results from the tests, as there are numerous factors that affect the test:

  • Am I in a devenv of ~/projects/devenv or not? (if not, it already worked)
  • Is there a lock file still in examples/python-poetry from a previous debug session?
  • Have I still got a devenv entry in examples/python-poetry/devenv.yaml?

Might need to look into the tests for a solution.

@bobvanderlinden
Copy link
Contributor Author

Could you try rerunning CI?

@bobvanderlinden bobvanderlinden changed the title Avoid accidental leaking of poetry binary python: avoid using existing virtual env Apr 25, 2023
@bobvanderlinden bobvanderlinden changed the title python: avoid using existing virtual env python: avoid using existing virtual env or poetry binary Apr 25, 2023
@bobvanderlinden
Copy link
Contributor Author

Yay, it succeeds now 👍 Just MySQL that is still failing.

@domenkozar domenkozar merged commit 9465ba9 into cachix:main Apr 26, 2023
74 of 166 checks passed
@bobvanderlinden
Copy link
Contributor Author

Thanks for your patience

@bobvanderlinden bobvanderlinden deleted the pr-python-poetry-test-leak branch April 26, 2023 10:08
@domenkozar
Copy link
Member

Thanks for fixing this :)

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.

None yet

2 participants