niess / python-appimage

AppImage distributions of Python
https://python-appimage.readthedocs.io/en/latest/
GNU General Public License v3.0
174 stars 24 forks source link

Individual calls to 'pip install' for each requirement in execute(), leading to incorrect requirement resolution #79

Open honnibal opened 3 days ago

honnibal commented 3 days ago

Hi,

Apologies if this isn't the right place to raise this, or if the issue is already under discussion elsewhere.

In the build command, the requirements aren't passed to pip together, but are instead installed with successive calls to pip install: https://github.com/niess/python-appimage/blob/03bab9b38d5bdc6ffeebdec8436e8d0d114c7c02/python_appimage/commands/build/app.py#L288

This doesn't give pip a chance to do any dependency resolution. In my application I'm getting an error:

RuntimeError: ERROR: pip's dependency resolver does not currently take into account all the packages that are installed. This behaviour is the source of the following dependency conflicts.
spacy-pkuseg 1.0.0 requires numpy<3.0.0,>=2.0.0; python_version >= "3.9", but you have numpy 1.26.4 which is incompatible.

If I do pip install on the requirements file, it works, because pip is able to resolve a set of dependencies that are compatible for all the requirements. But when pip is called for each requirement one-by-one, you get the subdependencies of the first package, which might be incompatible with subsequent packages. If the intention is for the user to provide an exactly-resolved set of requirements, the calls to pip should use the --no-deps flag, so that pip doesn't pull in the wrong dependencies as it processes each requirement.

Currently the best workaround I can find is to have the appimage requirements.txt actually point to a different file, like this:

"-r/path/to/real-requirements.txt"
niess commented 16 hours ago

Hello @honnibal ,

thank you for raising this point. I didn't actually realize. I guess that the initial reason for doing so was primarily cosmetic (i.e. logout each dep being installed). But, meanwhile the local+ feature was added, which requires filtering the list of requirement. Thus, a solution could be to rebuild a filtered requirements file (excluding the local+ lines), and to pip install from this, instead of running pip install for each filtered requirement line.

I am open to a PR, if you'd like to patch this issue. Otherwise, I'll do it, but later on.

honnibal commented 15 hours ago

Thanks for replying so quickly.

I think this might be one of those "easier to do than to review" changes? It's pretty small and I likely won't get it 100% right first time, leading to some back and forth. In the meantime my workaround is working for me, so it isn't blocking me.