mhammond / pywin32

Python for Windows (pywin32) Extensions
5.06k stars 799 forks source link

No compatibility with pipenv #1177

Open Devligue opened 6 years ago

Devligue commented 6 years ago

This problem was first issued under pipenv repo (pipenv/issues/1575) but as it turned out it was not pipenv that was at fault.

Problem description:

If we place pywin32 (or pypiwin32) in Pipfile for example like this:

[packages]
"pywin32" = {version = "*", sys_platform = "== 'win32'"}

and then we do pipenv install on Windows everything will be fine. But if we do the same under Linux then we are in bad luck. Even though pipenv is not installing this package (because we specified to install it only under windows) we will still get an error:

Warning: Your dependencies could not be resolved. You likely have a mismatch in your sub-dependencies.
  You can use $ pipenv install --skip-lock to bypass this mechanism, then run $ pipenv graph to inspect the situation.
Could not find a version that matches pywin32>=223
Tried: (no version found at all)

For the matter other system-specific packages like pywinusb or stdeb work fine with pipenv in identical scenario.

As stated by pipenv contributors:

(...) this is not an issue with pipenv, it's an issue with how pywin32 has organized their setup.py, we literally have no way to work around this and it is only a problem for this specific package.

mhammond commented 6 years ago

As stated by pipenv contributors:

(...) this is not an issue with pipenv, it's an issue with how pywin32 has organized their setup.py, we literally have no way to work around this and it is only a problem for this specific package.

What changes are necessary?

Devligue commented 6 years ago

@techalchemy could you elaborate more on that?

I don't think I can be of much help myself. I would expect setup.py too look something like this but as I saw, pywin32 setup.py is much more complicated.

But for what it is worth: both pywinusb and stdeb (that are Windows and Linux packages respectively), despite being meant to be used only under specific platform, can be installed with pip install also on unsupported platforms. On the other hand pywin32 (and pypiwin32) when pip install'ed on Linux will throw error:

Could not find a version that satisfies the requirement pywin32 (from versions: )
No matching distribution found for pywin32

Maybe it is just coincidence, but maybe it might help somehow.

techalchemy commented 6 years ago

@mhammond our resolver code attempts to resolve dependencies whether or not any given system can actually use them. In practical terms this means introspection of a given wheel. With pywin32 this isn’t possible due to the windows imports — I would imagine just introducing some conditional if sys.platform... before doing the imports

thopiekar commented 6 years ago

So why is your resolver not giving up and acts like the dependency has not been found? Or is a sys.platform != win32: raise ImportError("Your OS is not supported!") enough for you?

techalchemy commented 6 years ago

@thopiekar because we didn't write setuptools -- this is the default behavior. Setuptools expects to always be able to run a setupfile whether the package is suitable for a platform or not -- it will raise its own exceptions if the package has its own markers.

thopiekar commented 6 years ago

Ah, ok. So there is no platform-neutral file, did I get it correctly?

Well, if this is the expected behavior (defined by setuptools) then this should be fixed of course :+1:

thopiekar commented 6 years ago

@techalchemy I'm looking over the setup.py file. Is there any other module then winreg, which is only available on windows? I'm thinking now about splitting up setup.py into a OS-neutral setup.py and Windows-specific setup-win.py file. Don't know whether @mhammond will like this way of solving the issue here, but I think that would be at least something :)

bdbaddog commented 6 years ago

Maybe take a look at this repo? Looks like they're repackaging pywin32 for pypi

https://github.com/Googulator/pypiwin32

Devligue commented 6 years ago

@bdbaddog It does not matter, look at problem description. I have already tested both.

techalchemy commented 6 years ago

You can do windows specific stuff in an if block but technically with where python packaging is heading (see pep 517/518) it would be better to separate this kind of execution from the file that is only supposed to be metadata about what to install. setup.py is just supposed to tell setuptools ‘install this’ rather than actually running a bunch of platform specific things. I don’t know the specifics of this package but minimal compatibility just requires an if block around anything that would fail on Linux

thopiekar commented 6 years ago

@techalchemy I was also thinking about moving everything into an if clause, but this one would get quite long. That's why I created a separate file. There are many things in the setup.py, which I think are handled in setuptools nowadays. While sorting everything in individual files, I noticed that pywin32 defines own compilers, etc. It would need someone, who is familiar with setuptools to do a cleanup here. So instead of creating two different files, as done in https://github.com/mhammond/pywin32/pull/1191 , you would still use one file with one if-clause inside, @techalchemy ?

thopiekar commented 6 years ago

@bdbaddog I looked up who was working on this project and it was "xiovat". He cared about getting pywin32 moved to Github, but in the meantime, his GitHub profile has been removed. Don't know how to get in contact with him.

bdbaddog commented 6 years ago

@thopiekar - I don't know that developer, but had seen a reference to that pypi package when looking at another developers work and that's how I stumbled upon it. So sadly I won't be of any help tracking xiovat down.

bdbaddog commented 6 years ago

You might reach out to the folks on the python distutils mailing list for some assistance?

thopiekar commented 6 years ago

Not so far. But I think asking them for their opinion is the best. They should have an clear opinion how this problem should be solved in their normed way.

Overdrivr commented 6 years ago

In the meantime, what would be the recommended workaround ?

mhammond commented 6 years ago

So I'm guessing that trying to import setup.py in that environment is causing an exception which is suppressed and instead generates the messages mentioned above. Has anyone seen what the specific exception is? I suspect it might be the import of winreg.

I'm also assuming the expected behaviour is that pywin32 just gracefully fails to install, right? Given the winreg module isn't used in the top-level scope, it might be enough to catch the error and set winreg to None - just enough so that the module imports.

Regardless, I'm reluctant to make changes "just in case" it fixes the issue, so the next step is for someone with this issue to work out how to see the exception generated, probably be extracting setup.py from the wheel, modifying it, re-packing the wheel and seeing what happens - and assuming it is a simple import like that, do something like I suggest, make sure we get the desired behaviour, then either report back here or open a PR.

altendky commented 6 years ago

@mhammond

Below is in Linux.

 ~   master ●  git clone https://github.com/mhammond/pywin32
Cloning into 'pywin32'...
remote: Counting objects: 28534, done.
remote: Compressing objects: 100% (17/17), done.
remote: Total 28534 (delta 7), reused 5 (delta 2), pack-reused 28515
Receiving objects: 100% (28534/28534), 56.19 MiB | 22.49 MiB/s, done.
Resolving deltas: 100% (20088/20088), done.
 ~   master ●  cd pywin32
 ~/pywin32   master  python3 -m venv venv
 ~/pywin32   master  venv/bin/python -c 'import setup'
Traceback (most recent call last):
  File "/home/altendky/pywin32/setup.py", line 73, in <module>
    import winreg # py3k
ModuleNotFoundError: No module named 'winreg'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/home/altendky/pywin32/setup.py", line 75, in <module>
    import _winreg as winreg # py2k
ModuleNotFoundError: No module named '_winreg'
 ✘  ~/pywin32   master  git rev-parse HEAD
484aa6464dcb05d909ac63bfc43c39a59925367f

Am I reading this write that pywin32 has no dependencies? We're dealing with this so pipenv can tell there are no deps? Just a bit ironic if I'm reading correctly. :] I wonder about making a basic set of kwargs for the generic metadata, provide that in an if not sys.platform.startswith('win'): setup() call and put the entire rest of the script in an else (or separate file, whatever). I think something like this was mentioned above.

Fix that exception above and another pops up. I only went a couple deep at this point.

mhammond commented 6 years ago

Below is in Linux.

I tried playing with that, and the next issue is that MSVCCompiler fails to import - which is a reasonable thing for a windows specific setup.py to import. So I can't really see a sane way forward here :(

Am I reading this write that pywin32 has no dependencies? We're dealing with this so pipenv can tell there are no deps? Just a bit ironic if I'm reading correctly. :]

That's correct. And yeah, TBH I really don't understand what is being achieved by this, but it does sound like the right thing is to simply not try and import this script on platforms other than windows.

What does the traceback here look like when used in the way that's causing a problem for people? ie, what is the call site from setuptools here, and are we sure there's no way to side-step the issue there (eg, with a NotImplemented exception or similar?) If not, there should be :) I understand that probably means talking to the distutils/setuptools team :(

altendky commented 6 years ago

Correct or not I think the suggestion is that setup.py should be functional enough to allow access to the information it describes on any platform. For example, to check and see what platforms the package supports. :] Or, perhaps you are downloading on one platform and want to identify the dependencies you need to download for the other.

I just brought it up in #pypa, we'll see if we get some interest.

mhammond commented 6 years ago

Correct or not I think the suggestion is that setup.py should be functional enough to allow access to the information it describes on any platform.

I agree with that in general, although that also extends to setuptools itself - if msvccompiler could be imported (but obviously not used) it would be far less messy - but it's difficult to make the case msvccompiler needs to be imported anywhere too. (Edit: so I guess I actually don't agree with that in general :)

I also still don't understand why this is being pushed down to packages. Looking at the linked pipfile issue, the intent is clearly that the pipfile can specify a platform - the best and obvious solution is that setup.py not be imported on a platform where it's not requested and known to not work. There's no reason to import it to discover dependencies in that scenario either. I'm not sympathetic to "I should be able to discover dependencies on platforms where it doesn't work just out of interest" and I'm not convinced by the comment in that pipfile issue that a windows-only package is wrong to have windows-only constructs in its setup.py (in the same way I'm not convinced msvccompiler.py is wrong to have windows specific constructs in it) - so I still don't understand this issue.

altendky commented 6 years ago

One scenario I imagine theoretically (as in I don't know if pipenv does this or not) is that it would be nice to resolve a set of dependencies that is as consistent as possible across all platforms for a given package. To do this you would want to be able to analyze the subdependencies for all platforms from any single one of them.

At some point this is kind of about setup.py being a config file... That allows programming in it. It's a good example of the issues that can bring about. Not blaming anyone, just observing.

techalchemy commented 6 years ago

Thanks for the thoughtful discussions all around and as always @altendky has it on the head— this hits at the core issue in python packaging which is that setup.py is basically a config file that lets you run code. In a lot of cases we have to ignore markers to resolve dependencies properly and/or it would be better to ignore markers, so it’s not just this package we are talking about. Mostly it is possible to run a setup script no matter what restrictions the user places on installation to at least get the dependency output. Obviously you are under no obligations, but if there were an escape hatch for non-windows platforms it would really be ideal

altendky commented 6 years ago

I'm going to try to give us some actual code to talk about. I'm assuming that most of what is done globally could be done in the build extension or such. We'll see how it pans out.

altendky commented 6 years ago

But, I just pushed a PR so we can look at some code. Basically, there's a couple conditionals and ext_modules and data_files are just set empty if not on Windows. Oh, and the 'building on the wrong platform' message was moved down to where it's checked value is actually used rather than being global.

@techalchemy, any chance you'd have time to see if this is useful to pipenv? It's not totally obvious to me from the outside and I presume you could check the insides much more quickly than the hours it'd take me to figure out where to look and what about (I'm in no rush on this so I'm fine if that means waiting awhile).

I do have some use of global in the PR... :[ If this ends up helpful and would be considered for pywin32 I'd plan to make that at least a bit less ugly.

altendky commented 6 years ago

@mhammond, @techalchemy, I hope this is a representative summary. I will happily correct it if not. I presume @dstufft is busy but perhaps this is enough to generate a good response or maybe suggest someone else that could dig into more detail.

@dstufft, any chance you could provide some guidance on what the platform compatibility expectations are of a good setup.py file?

Summary

pipenv on not-Windows had issues with pywin32 as a dependency because the pywin32 setup.py imports Windows-only things. @techalchemy has suggested that this should be corrected in pywin32 such that pipenv can get a full dependency list on all platforms. @mhammond has suggested that pipenv should just not look at pywin32 at all since the Pipfiles in question mark it as a Windows-only dependency and that given that it would be fine for pywin32's setup.py to be only usable on Windows.

Question

Should setup.py files be able to be processed on any platform including those on which the package can not be installed? Or, should package management tools like pipenv plan to be unable to see package metadata such as install_requires for packages not compatible with the current platform?

dstufft commented 6 years ago

I don't think that this is actually specific to setup.py / distutils / setuptools here, the question ultimately comes down to "inside of an sdist, can we require platform specific code". This doesn't go away with PEP 517, because you could ask the same question of the PEP 517 hooks for say, discovering dependencies or the like.

I think the answer to that has to be, "yes, you can do platform specific things inside of a sdist". The reason I think that is that the alternatives all severely limit what you can do. For instance, you might need to check if some feature is supported on the target CPU/GPU/Platform/whatever, and if so, install different dependencies and the like. We purposely made the PEP 517 hooks dynamic to allow specifically that sort of thing. The only alternatives I can think of to failing in that case, is to emit incorrect information, which seems to be the worst possible case.

That being said, I think that projects should also generally strive to avoid doing that unless they actually have to. In this case I would probably say, without actually looking at the setup.py in question, that it may be best to try to defer importing the platform specific still until inside of a build command or similar if possible, so that you still fail if you try to do something like actually build it on an unsupported platform, but you can still do stuff like enable pipenv here.

Without diving deeper into what specifically pipenv itself is doing, I can't offer more specific advice there. However I think it's reasonable to say that in some cases, you simply can't resolve the dependencies, and they're going to need to provide a wheel that you can use during the dependency resolution, instead of the sdist.

dstufft commented 6 years ago

I'd just also add, that it's somewhat unfortunate that the design of setup.py generally means that dealing with this problem is more or less dependent on each individual package going out of their way to be the best citizen it can, within the constraints they have. Hopefully in the future with PEP 517/518 this answer is something that build systems will have to solve, and individual projects will just use those build systems.

altendky commented 6 years ago

@dstufft, if you can't get to the platform specific code to check for a feature, wouldn't that be a good indication that you can't have that feature? And that could be handled explicitly rather than some exception failing discovery of any information at all. As to deferring the platform-specific code, I that's what #1240 was intended to do.

1240 would certainly provide 'incorrect information' about ext_modules and data_files at present by reporting them totally empty. Perhaps this is an acceptable amount of lying on an unsupported OS? Maybe actually defining the supported platform would make this more acceptable, such as classifiers=['Operating System :: Microsoft :: Windows'].

I'm trying to do too much and so I still need to figure out how to see if #1240 actually satisfies pipenv.

@dstufft, thanks for your time.

dstufft commented 6 years ago

@altendky Not exactly!

For instance, pipenv is doing this to resolve dependencies for the lockfile right? So what happens if you try to generate a lock file, and the sdist tries to inspect the platform to determine what dependencies to include, in cases where there are no defaults to fall back to? If it returns nothing, then pipenv is going to assume it has no dependencies and then lock it like that.. but that's not true! It has dependencies, it just can't figure out if it wants A or B, because it can't talk to the platform to figure it out.

The case is basically a subset of the case of not being able to provide wheels. Sometimes a project just honestly does something out of the box enough that it needs to do some install time discovery. That is hopefully a narrow enough use case that it is uncommon, but it's not an impossible use case. For pywin32 what I know of pipenv and such, it seems like it should be resolvable.

jaymegordo commented 3 years ago

+1 Still having this issue with an app built for both mac and win. Pipenv fails to create the lock file on mac when pywin32 exists in the Pipfile.

charlesbaynham commented 2 years ago

+1 This arises for us when a project built on Windows has its requirements fixed via pip-compile which then gets built on a Linux CI server. pywin32 comes in via jupyter.

mhammond commented 2 years ago

I have no idea what I can do here - pywin32 is clearly windows only and there's nothing I can do about that. The problem is in tools which don't understand that and try and use it elsewhere.

Avasam commented 2 months ago

https://peps.python.org/pep-0751/ is an new attempt to standardize and fix this issue on an ecosystem level.

Until then, I can only recommend to use tools that don't lock your dependencies to a specific environment if you need to support multiple environments. (like PDM, Poetry, uv). Or to create environment specific lock files yourself.