search

JeanChristopheMorinPerso / rez-pip

PyPI/python package ingester/converter for the rez package manager
https://rez-pip.readthedocs.io/en/latest/
Apache License 2.0
23 stars 5 forks source link

Fail to install PySide6 #64

Open MrLixm opened 10 months ago

MrLixm commented 10 months ago

Hello ! I have been trying to use rez-pip to install PySide6 :

rez python -m rez_pip PySide6 --python-version ==3.9.13

Which fail with the following :

INFO     Installing requested packages for Python 3.9.13
INFO     Resolved 4 dependencies for python 3.9.13
INFO     Downloading...
INFO     Downloaded PySide6-6.6.0 to
         'C:\\Users\\lcoll\\AppData\\Local\\Temp\\rez-pip-targetce9bd26z\\wheels\\PySide6-6.6.0-cp
         38-abi3-win_amd64.whl' (7225 bytes)
INFO     Downloaded shiboken6-6.6.0 to
         'C:\\Users\\lcoll\\AppData\\Local\\Temp\\rez-pip-targetce9bd26z\\wheels\\shiboken6-6.6.0-
         cp38-abi3-win_amd64.whl' (1066845 bytes)
INFO     Downloaded PySide6-Essentials-6.6.0 to
         'C:\\Users\\lcoll\\AppData\\Local\\Temp\\rez-pip-targetce9bd26z\\wheels\\PySide6_Essentia
         ls-6.6.0-cp38-abi3-win_amd64.whl' (76703502 bytes)
INFO     Downloaded PySide6-Addons-6.6.0 to
         'C:\\Users\\lcoll\\AppData\\Local\\Temp\\rez-pip-targetce9bd26z\\wheels\\PySide6_Addons-6
         .6.0-cp38-abi3-win_amd64.whl' (110273318 bytes)
INFO     Downloaded 4 wheels
INFO     Installing PySide6-6.6.0 wheel
INFO     Installing PySide6-Addons-6.6.0 wheel
Traceback (most recent call last):
  File "Z:\packages-dev\python\3.10.7\lib\runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "Z:\packages-dev\python\3.10.7\lib\runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "c:\program files\rez\lib\site-packages\rez_pip\__main__.py", line 3, in <module>
    rez_pip.cli.run()
  File "c:\program files\rez\lib\site-packages\rez_pip\cli.py", line 318, in run
    _run(args, pipArgs, pipWorkArea)
  File "c:\program files\rez\lib\site-packages\rez_pip\cli.py", line 200, in _run
    dist, isPure = rez_pip.install.installWheel(
  File "c:\program files\rez\lib\site-packages\rez_pip\install.py", line 89, in installWheel
    installer.install(
  File "c:\program files\rez\lib\site-packages\installer\_core.py", line 109, in install
    record = destination.write_file(
  File "c:\program files\rez\lib\site-packages\installer\destinations.py", line 207, in write_file
    return self.write_to_fs(scheme, path_, stream, is_executable)
  File "c:\program files\rez\lib\site-packages\installer\destinations.py", line 167, in write_to_fs
    raise FileExistsError(message)
FileExistsError: File already exists: C:\Users\lcoll\AppData\Local\Temp\rez-pip-targetce9bd26z\installed\3.9.13\python\PySide6/__init__.py

It seems that PySide6-Addons is overriding some stuff in the existing __init__.py of the just-installed PySide6 which does not seem to be expected by rez.

I have not investigated further but will report anything I find below.

context

JeanChristopheMorinPerso commented 10 months ago

Yes, this is a known issue. It's not an issue with rez-pip or rez. It's an issue mainly with PySide. They ship an __init__.py file in every wheel, which they shouldn't do because anyway PySide6-Addons depends on PySide6. But it's a tricky situation. The installer library could also be mare less strict, but I doubt they'll want that. I still have to create an issue with them to see if they would accept reducing the strictness or not.

MrLixm commented 10 months ago

Good to know, is there any known workaround ?

JeanChristopheMorinPerso commented 10 months ago

Not for now unfortunately. I could whip something up though. Thee is two solutions, one of which just came to mind.

  1. Override the write_to_fs of https://github.com/pypa/installer/blob/main/src/installer/destinations.py#L147 to avoid this error.
  2. Install wheels into their own directory before bundling them into rez packages.

Both options have downsides. Option 1 has the downside that I have to maintain it and keep it up to date with upstream. The downside of option 2 is not necessarily linked to option. How will Python react if you install both PySide6 and PySide6-Essentials in the same environment? Both will provide a PySide6 importable module. So if you were to rez-env PySide6 PySide6-Essentials, then only thing you'll be able to import is what PySide6 provides and you won't be able to use stuff from essentials. But that's more an issue with how PySide6 is packaged.

Though, PySide6 depends on PySide6-Essentials and PySide6-Addons (https://inspector.pypi.io/project/pyside6/6.6.0/packages/14/cd/f8117f92430564af4cb60bc38d93de8dcb1d051c955cf0a62e6563fe7f55/PySide6-6.6.0-cp38-abi3-manylinux_2_28_x86_64.whl/PySide6-6.6.0.dist-info/METADATA#line.41).

All in all, I don't really know what we should do.

MrLixm commented 10 months ago

Thanks for the explanations even I don't fully understand the issue yet. How does the usual pip install PySide6 works ? Because in a random venv sites-packages I can find :

PySide6/
PySide6_Essentials-6.5.1.dist-info/
PySide6_Addons-6.5.1.dist-info/

which means at some point it combines PySide6_Essentials with PySide6 ? Is PySide6_Addons even useful by itself ?

With my limited knowledge on this issue I would argue that at least having a solution that allows you to retrieve the packages, even in a non-usable state, is better than just failing. The developer can at least use rez-pip as part of the build process and write additional steps to package PySide6 to a functional rez-package. So your solution 2. doesn't sound that bad.

JeanChristopheMorinPerso commented 10 months ago

All the PySide6 variants ship with the same folder structure.

And so on. The dist-info folders that you see are just packaging metadata, not the actual python packages.

A wheel is a zip file with a specified file structure. So installers basically unzip wheels (and do some other work to validate the files and stuff) into the install directory. Pip is less strict than installer and will not complain if it tries to install a file and it already exists (though it might complain down the line, if the file that replaced the existing one is different than the original file. But I don't know if that is really true).

MrLixm commented 10 months ago

Ha I see so actually the PySide6.__init__.py is useless because it is anyway fully overridden by the PySide6-Essentials.__init__.py. Wouldn't that use case then fit into the hook feature you mentioned at some point ? Where we could have a hook after downloading PySide6 that delete the __init__.py to leave it free for the one from PySide6-Essentials, assuming that PySide6 is always installed first.

JeanChristopheMorinPerso commented 10 months ago

Ha I see so actually the PySide6.__init__.py is useless because it is anyway fully overridden by the PySide6-Essentials.__init__.py.

Kind of yes, but it could be debated. Same for the __init__.py in PySide6-Addons.

Wouldn't that use case then fit into the hook feature you mentioned at some point ?

Yes

Where we could have a hook after downloading PySide6 that delete the __init__.py to leave it free for the one from PySide6-Essentials, assuming that PySide6 is always installed first.

No. A hook would potentially do something like merge PySide6 packages into one, because PySide6 is really meant to be distributed this way, in one single root folder and not in multiple separated folders. This could be a pre rez package creation step (after wheels got installed into the temp directory).

But I'm not yet convinced that we necessarily need to take the hook route.

On top of that, I think we would still be stuck with the issue where PySide can't find shiboken.

JeanChristopheMorinPerso commented 10 months ago

Thinking about this more, a workaround is to install PySide6-Essentials. If you just install this one, it will work. Don't install PySide6. This at least works on Linux.

(.venv) [jcmorin@arch01 rez-pip]$ rez-pip2 PySide6-Essentials --python-version 3.11 --prefix /tmp/asd
(.venv) [jcmorin@arch01 rez-pip]$ rez-env --paths ~/rez_packages:/tmp/asd PySide6_Essentials -- python -c 'import PySide6.QtCore; print(PySide6.QtCore)'
<module 'PySide6.QtCore' from '/tmp/asd/PySide6_Essentials/6.6.0/9645a50b415bcaad6bcde791aeb0859a43c56501/python/PySide6/QtCore.abi3.so'>

It's far from ideal though, since you need to depend on PySide6_Essentials instead of PySide6... But if you need a short term workaround, it kind of works (unless you also need PySide6-Addons, but you'd have the same issue with the built-in rez-pip).

JeanChristopheMorinPerso commented 10 months ago

To fix the problem with PySide6-Essentials + PySide6-Addons, I'd like to explore a possibility of adding a symlink farm to rez. Nix does exactly this and conda also does the same thing:

An alternative could be to see if PySide6 could be converted to a namespace package, though I don't think it would work for PySide.

MrLixm commented 10 months ago

Thanks for the suggestions I manage to have a custom build working by calling rez-pip in 2 steps :

and then merging the 2 packages with a python script. (the script is actually a buid.py called during rez-build, for PySide6 I manually created the package.py)

JeanChristopheMorinPerso commented 10 months ago

https://spack.readthedocs.io/en/latest/environments.html#filesystem-views

JeanChristopheMorinPerso commented 7 months ago

I started some work on a plugin system that could solve this, see https://github.com/JeanChristopheMorinPerso/rez-pip/pull/91.