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

force recreation of venv if it is not sane (#286) #1797

Merged
merged 4 commits into from Jan 9, 2020

Conversation

finswimmer
Copy link
Member

@finswimmer finswimmer commented Dec 27, 2019

With this PR a virtual environment is recreated if it's not sane, meaning the python and pip executable is not found within the venv path.

I'm not sure how to write a test for it. If it's necessary, please give me a hint :)

Fixes: #286

Pull Request Check List

This is just a reminder about the most common mistakes. Please make sure that you tick all appropriate boxes. But please read our contribution guide at least once, it will save you unnecessary review cycles!

  • Added tests for changed code.
  • Updated documentation for changed code.

@finswimmer finswimmer requested a review from Dec 27, 2019
@finswimmer finswimmer added Bug Venv labels Dec 27, 2019
Copy link
Member

@sdispater sdispater left a comment

We might go into a loop here since there is no guarantee that recreating the virtual environment will fix the issue. If we go this route we should at least display a warning that we recreate the environment because it's invalid.

Another approach, or an addition to this PR, could be to check if the environment is valid after creating it and, if it's not, we could warn the user. A lot of cases we had here comes from the fact that Debian-based systems do not include the venv module by default which leads to a broken environment. If it's only pip missing, we could try to install it by using the get-pip.py script.

@finswimmer finswimmer requested a review from sdispater Jan 3, 2020
@finswimmer
Copy link
Member Author

@finswimmer finswimmer commented Jan 5, 2020

I've added a more explicit warning.

I also have a variant in preparation where I check the sanity of the venv after creating this. But I have a problem to pass the tests in env/test_use, because the build_venv is mocked and just create a folder without any content. I'm not sure yet, how to fix these tests...

poetry/utils/env.py Outdated Show resolved Hide resolved
Co-Authored-By: Sébastien Eustace <sebastien.eustace@gmail.com>
@finswimmer finswimmer requested a review from sdispater Jan 8, 2020
@sdispater sdispater merged commit cbb1c18 into python-poetry:master Jan 9, 2020
16 checks passed
@finswimmer finswimmer mentioned this pull request Jan 10, 2020
2 tasks
@finswimmer finswimmer deleted the issue-0286-empty-venv branch Apr 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Venv
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants