utils/envs: fix envs in MSYS2 always broken#3713
Merged
abn merged 1 commit intopython-poetry:masterfrom Mar 21, 2021
Merged
Conversation
danyeaw
force-pushed
the
fix-msys2-venv
branch
from
73e1eda to
7ecb72f
Compare
Fixes python-poetry#2867, the virtual environment found seems to be broken messages when running in MSYS2. This change detects when in MSYS2 in order to use the bin directory for the virtualenv instead of Scripts which is normally used in Windows.
danyeaw
force-pushed
the
fix-msys2-venv
branch
from
7ecb72f to
f0bacb8
Compare
7 tasks
Contributor
Author
|
Hi all, could I please get a review on this PR? This has a huge impact on being able to deploy cross platform OSS apps for Windows. Thanks! |
abn
requested changes
Mar 20, 2021
|
|
||
| def __init__(self, path: Path, base: Optional[Path] = None) -> None: | ||
| self._is_windows = sys.platform == "win32" | ||
| self._is_mingw = sysconfig.get_platform() == "mingw" |
Member
There was a problem hiding this comment.
Suggested change
| self._is_mingw = sysconfig.get_platform() == "mingw" | |
| self._is_mingw = sys.platform == "msys" |
This should work here, right? Would be good to keep the usage consistent. I am not too familiar with using mingw, would be good to get a confirmation.
Contributor
Author
There was a problem hiding this comment.
@abn Thanks for the review, I really appreciate it!
No that won't work:
$ python
Python 3.8.8 (default, Feb 20 2021, 07:16:03) [GCC 10.2.0 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> print(sys.platform)
win32
Comment on lines
+884
to
+887
| if not self._is_windows or self._is_mingw: | ||
| bin_dir = "bin" | ||
| else: | ||
| bin_dir = "Scripts" |
Member
There was a problem hiding this comment.
Suggested change
| if not self._is_windows or self._is_mingw: | |
| bin_dir = "bin" | |
| else: | |
| bin_dir = "Scripts" | |
| bin = {"win32": "Scripts"}.get(sys.platform, "bin") |
Would this be slightly better maintainable considering we only care about exceptions?
Contributor
Author
There was a problem hiding this comment.
This doesn't work:
>>> bin = {"win32": "Scripts"}.get(sys.platform, "bin")
>>> print(bin)
Scripts
Member
There was a problem hiding this comment.
Given msys seems to only patch get_platform() , guess that is expected.
abn
approved these changes
Mar 21, 2021
Contributor
Author
|
Thanks @abn! |
8 tasks
s0600204
added a commit
to s0600204/poetry
that referenced
this pull request
Sep 2, 2021
python, when installed via (msys) MingW on Windows, can be found under a path structure similar to that you might find on *nix systems. The install-poetry.py script was failing to take this into consideration, causing installation issues winth Windows + MingW. A similar fault in poetry itself was resolved in python-poetry#3713.
2 tasks
s0600204
added a commit
to s0600204/poetry
that referenced
this pull request
Sep 3, 2021
When running `python` within the various shells made available on Windows by MSys2, `sysconfig.get_platform()` returns: * MSys2 : `msys-3.2.0-x86-64` * MinGW x32 : `mingw_i686` * MinGW x64 : `mingw_x86_64` * MinGW UCRT64 : `mingw_x86_64_ucrt` * MinGW Clang64 : `mingw_x86_64_clang` In the case that `sysconfig.get_platform()` *does* return `mingw` (with no suffix), this change should still work as expected.
abn
pushed a commit
that referenced
this pull request
Oct 8, 2021
When running `python` within the various shells made available on Windows by MSys2, `sysconfig.get_platform()` returns: * MSys2 : `msys-3.2.0-x86-64` * MinGW x32 : `mingw_i686` * MinGW x64 : `mingw_x86_64` * MinGW UCRT64 : `mingw_x86_64_ucrt` * MinGW Clang64 : `mingw_x86_64_clang` In the case that `sysconfig.get_platform()` *does* return `mingw` (with no suffix), this change should still work as expected. Related-to: #3713
1nF0rmed
pushed a commit
to 1nF0rmed/poetry
that referenced
this pull request
Nov 15, 2021
When running `python` within the various shells made available on Windows by MSys2, `sysconfig.get_platform()` returns: * MSys2 : `msys-3.2.0-x86-64` * MinGW x32 : `mingw_i686` * MinGW x64 : `mingw_x86_64` * MinGW UCRT64 : `mingw_x86_64_ucrt` * MinGW Clang64 : `mingw_x86_64_clang` In the case that `sysconfig.get_platform()` *does* return `mingw` (with no suffix), this change should still work as expected. Related-to: python-poetry#3713
s0600204
added a commit
to s0600204/install.python-poetry.org
that referenced
this pull request
Dec 26, 2021
python[.exe], when installed via (msys) MinGW on Windows, can be found under a path structure similar to that you might find on *nix systems. The install-python.py script was not taking this into consideration, causing installation failure under Windows + MinGW. A similar fault in poetry itself was resolved with python-poetry/poetry#3713.
This was referenced Dec 26, 2021
neersighted
pushed a commit
to python-poetry/install.python-poetry.org
that referenced
this pull request
Jan 21, 2022
python[.exe], when installed via (msys) MinGW on Windows, can be found under a path structure similar to that you might find on *nix systems. The install-python.py script was not taking this into consideration, causing installation failure under Windows + MinGW. A similar fault in poetry itself was resolved with python-poetry/poetry#3713.
1 task
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When running poetry in MSYS2 it gives a warning that the virtual environment found seems to be broken messages. This change detects when in MSYS2 in order to use the bin directory for the virtualenv instead of Scripts which is normally used in Windows.
Pull Request Check List
Resolves: #2867