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 win32 vs msys2 conditions on Windows 10. #1338

Closed
verm opened this issue Mar 25, 2019 · 7 comments
Closed

Fix win32 vs msys2 conditions on Windows 10. #1338

verm opened this issue Mar 25, 2019 · 7 comments
Labels

Comments

@verm
Copy link

verm commented Mar 25, 2019

This is a new bug report in relation to issue #650

The conditionals are broken for using virtualenv inside msys2 as Python reports the os as 'win32'.

@pe244 came up with a solution in #650 which I've updated for the latest source. Unfortunately the bug report was closed due to being idle. It is still in issue on Windows 10.

The is_msys variable is somewhat of a hack but it does work.

If there are any fixes or testing required let me know this does work for me and I've deployed it a few dozen times.

Here is a patch for v16.0.0:

msys2_win10_2019-03-25.diff.txt

@gaborbernat
Copy link
Contributor

Want to open a PR?

@andy-maier
Copy link

andy-maier commented Apr 22, 2019

I also have the problem originally reported in #650, so I'd like to see this solved.

However, with MSYS2, one needs to carefully distinguish between its MSYS subsystem on the one hand and its MINGW64/32 subsystems on the other hand. Which the proposed change does not yet do at this point.

The MSYS subsystem is intended to be mainly a UNIX-like environment under Windows. The MINGW64/32 subsystems are intended to be mainly a build environment for native Windows executables. They have different tool chains that should not be mixed, and they have different python2/3 packages for each kind of subsystem, each built with the proper toolkit for the subsystem. You can see one difference between these subsystems in the way directory path names are constructed (see the examples below).

IMO, the MINGW64/32 subsystems are the more important ones to support, but if we are at it, we may as well support all three subsystems of MSYS2.

Also, we should make a decision whether or not the old MINGW+MSYS (http://www.mingw.org/wiki/MSYS, i.e. before MSYS2) still should be supported by Pip or not. I think with the presence of MSYS2 the old MINGW+MSYS have no real reason to be used anymore, but that's just my opinion.

In the current MSYS2 version, the python command resolves to Python 2 in the MSYS subsystem and to Python 3 in the MINGW64/32 subsystems, so I am using the python2 command in the following examples to avoid that additional confusion:

In MSYS2, the MSYSTEM environment variable is guaranteed to be set to the upper case name of the active subsystem. Because the MINGW64/32 subsystem is intended to be a native WIndows environment, Python's sys.platform is "win32", while for the MSYS subsystem it is "msys".

Python 2 packages in MSYS2:

# pacman -Q |grep "python2 "        # lists installed packages ending with 'python2'
mingw-w64-x86_64-python2 2.7.16-1   # the one used in the MINGW64 subsystem
python2 2.7.15-3                    # the one used in the MSYS subsystem

MSYS2 with MINGW64 subsystem:

# echo $PATH
/mingw64/bin:/usr/local/bin:/usr/bin:/bin:/c/Windows/System32:/c/Windows:/c/Windows/System32/Wbem:/c/Windows/System32/WindowsPowerShell/v1.0/:/usr/bin/site_perl:/usr/bin/vendor_perl:/usr/bin/core_perl
# which -a python2
/mingw64/bin/python2      # the one from the MINGW64 python2 package
/usr/bin/python2          # the one from the MSYS python2 package
/bin/python2              # the one from the MSYS python2 package
# python2 --version
Python 2.7.16
# python2 -c "import sys; print(sys.path)"
['', 'C:/msys64/mingw64/lib/python27.zip', 'C:/msys64/mingw64/lib/python2.7', 'C:/msys64/mingw64/lib/python2.7/plat-win32', 'C:/msys64/mingw64/lib/python2.7/lib-tk', 'C:/msys64/mingw64/lib/python2.7/lib-old', 'C:/msys64/mingw64/lib/python2.7/lib-dynload', 'C:/building/msys64/mingw64', 'C:/msys64/mingw64/lib/python2.7/site-packages']
# python2 -c "import sys; print(sys.platform)"
win32
# python2 -c "import os; print(os.getenv('MSYSTEM'))"
MINGW64

MSYS2 with MSYS2 subsystem:

# echo $PATH
/usr/local/bin:/usr/bin:/bin:/opt/bin:/c/Windows/System32:/c/Windows:/c/Windows/System32/Wbem:/c/Windows/System32/WindowsPowerShell/v1.0/:/usr/bin/site_perl:/usr/bin/vendor_perl:/usr/bin/core_perl
# which -a python2
/usr/bin/python2          # the one from the MSYS python2 package
/bin/python2              # the one from the MSYS python2 package
# python2 --version
Python 2.7.15
# python2 -c "import sys; print(sys.path)"
['', '/usr/lib/python27.zip', '/usr/lib/python2.7', '/usr/lib/python2.7/plat-msys', '/usr/lib/python2.7/lib-tk', '/usr/lib/python2.7/lib-old', '/usr/lib/python2.7/lib-dynload', '/usr/lib/python2.7/site-packages']
# python2 -c "import sys; print(sys.platform)"
msys
# python2 -c "import os; print(os.getenv('MSYSTEM'))"
MSYS

Sorry for the lengthy explanations, but if we want to support these MSYS2 subsystems properly, the proposed change needs to be extended.

@pfmoore
Copy link
Member

pfmoore commented Apr 22, 2019

As noted in #650, I believe this is something that should be fixed at the MSYS2 end. They patch their Python build quite heavily, so it's down to them to patch it in a way that means that pip and virtualenv work correctly.

The MSYS2 build of Python isn't supported by the Python core developers, so I don't think pip/virtualenv are likely (speaking as a core dev on both projects) to add special case code to support that build.

@andy-maier
Copy link

andy-maier commented Apr 22, 2019

With all the conditional code dependent on IS_WIN that is already in the current virtualenv.py code, I feel it is a bit too simple to argue that it is MSYS2's fault. That code in virtualenv.py has several inconsistencies (sometimes it uses IS_WIN, sometimes sys.platform=='win32', sometimes it invokes os.symlink without checking whether it exists, to name just a few). I don't think the problem can be solved by asking MSYS2 not to patch Python in a way that makes virtualenv fail. I think it takes quite some cleanup in virtualenv as well, if only to remove technical debt.

On the arguments used in #650: If we just look at the MINGW64/32 subsystem of MSYS2, it is not so bad anymore. sys.platform is win32, and when not using a virtualenv, it does have the correct directories in sys.path (the statement in #650 that a directory was missing,was made when looking at a virtualenv that did not work), its sys.prefix is correct, and pip installs and works correctly. Maybe that wasn't the case in 2014 when #650 was discussed, but it is the case now.

The main thing that makes MINGW64/3 different from Windows is that it has UNIX paths. And that cannot be addressed by having MSYS2 patching Python differently. That is in fact something that needs to be properly recognized in virtualenv. Of course, virtualenv can decide to ignore MSYS2, but arguing with the way they patched Python is not really a valid argument, IMO.

Update: Having said that it has UNIX paths, is even not quite correct. If you look at sys.path and sys.prefix, you see that it has Windows paths with forward slashes. Which means it ought to work on native Windows (which is its purpose).

@andy-maier
Copy link

PS: I'm willing to help on this one, but you need to let me to... :-)

@pfmoore
Copy link
Member

pfmoore commented Apr 22, 2019

With all the conditional code dependent on IS_WIN that is already in the current virtualenv.py code, I feel it is a bit too simple to argue that it is MSYS2's fault.

The point is that MSYS2 Python says it's Windows, but then it doesn't behave in the way that claim would imply (or it says it's Unix, and doesn't behave in the way that claim would imply - I don't know which). It's not that having conditional code in applications (and everything you say above could apply to any one of many applications, not just virtualenv) is wrong, just that the applications need to be able to rely on the meaning of the checks (like sys.platform == "win32") that they make.

One question - have you talked to the MSYS2 Python maintainers at all, to find out if there are reasons why they have the behaviours that they do? Are they willing to accept patches to make it easier for applications to write code that works portably on MSYS2 as well as on the core Python supported platforms?

Looking at the specifics, the MSYS subsystem appears to be returning a value that's not technically valid according to the sys.platform docs which only allow for arbitrary values on non-Linux Unix systems. So MSYS is technically claiming to be a Unix system here - and maybe it is, I don't know how good its POSIX emulation is.

The MINGW subsystem is claiming to be Windows. But I'm not clear in what ways it isn't. Looking at the patch reverenced in the original issue here, I see the following:

  • is_msys needs functools as a required module. It's not clear why, but presumably that's because of some difference in the MSYS2 Python stdlib compared to the standard stdlib.
  • is_msys needs a sitecustomize.py file, but it's not clear why, but it looks like MSYS2 Python needs an extra directory added to PATH, which seems like it's something that MSYS2 should fix.
  • There are a couple of checks that need to say is_win or is_msys - which is simply a result of the match having made a distinction in the first place.
  • There's a symlink check that looks out of date. It's now protected by IS_WIN so is probably fine if msys is just treated as Windows,
  • There may be places where making MSYS2 not test as being Windows means that it takes a different path, but it's hard to say - and it's unlikely (from what you describe) that it's correct to treat MINGW as Unix in those cases.

The main thing that makes MINGW64/3 different from Windows is that it has UNIX paths

Well, from your example, only for the MSYS subsystem not the MINGW62/MINGW32 subsystem. The only thing I see there is that os.sep is / rather than \ - and that could be changed in msys python.

Anyway, you've not convinced me. Maybe the other maintainers will have different views, but I still feel that msys is not a supported platform for us, and any fixes should be handled by them. (Of course, another option would be for them to provide a customised msys virtualenv and pip via their own channels, if they wanted to avoid changing their Python build).

@gaborbernat gaborbernat added this to the 20.1.0 milestone Jan 2, 2020
@gaborbernat
Copy link
Contributor

I think the rewrite branch fixed this.

@pypa pypa locked and limited conversation to collaborators Jan 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants