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: poetry env broken on Windows (#4615) #4682

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tom-bowles
Copy link

Resolves: #4615

Looks like this if statement was copied from further up in the function and only one of the branches updated to use self._path.

@tom-bowles
Copy link
Author

@sdispater - this is a trivial fix and the problem is quite severe on Windows.

@abn
Copy link
Member

abn commented Oct 30, 2021

@tom-bowles can you rebase this so the cirrus tests can have a rerun please?

@tom-bowles
Copy link
Author

@abn - Thanks for taking a look! I've rebased now.

@abn
Copy link
Member

abn commented Oct 31, 2021

@tom-bowles should have mentioned this earlier, any cance we can add in a test case for this so we do not regress?

@tom-bowles
Copy link
Author

@abn - no worries, I'll have a crack at that today

@tom-bowles
Copy link
Author

@abn - Added test.

Copy link
Member

@neersighted neersighted left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM -- we just need the formatting fixed (run pre-commit run --all-files) by black to merge.


venv = VirtualEnv(venv_path)

assert Path(venv.python) == base_dir_path
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this test fails on Python 3.6 on Windows, so I spoke a little too soon. Can we take a look at that failure and get it corrected?

@neersighted neersighted self-assigned this Nov 12, 2021
s0600204 added a commit to s0600204/poetry that referenced this pull request Dec 26, 2021
(Also includes correction for non-mingw Windows from python-poetry#4682)
@dimbleby
Copy link
Contributor

Looks like this was fixed incidentally at #5007.

Apparently that one was allowed through without testcases so I suppose it could just about be worth keeping this open if anyone is motivated, more than a year later, to add a unit test.

But I expect this should now be closed.

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.

poetry env doesn't work on Windows after install-poetry.py
4 participants