ynput / ayon-launcher

AYON desktop application launcher
Apache License 2.0
31 stars 13 forks source link

Pip freeze fix #104

Closed iLLiCiTiT closed 6 months ago

iLLiCiTiT commented 6 months ago

Changelog Description

Requirements in installer json do contain correct git urls.

Additional info

Since pip version 24, freeze does not seem to handle git urls, and stores filepath to source package instead. This PR uses combination of requirements from venv and export from poetry.

This is not ideal solution, but should be working.

BigRoy commented 6 months ago

Since pip version 24

Issue is actually around since at least 20.01 according to this StackOverflow comment.

Anyway, more details surrounding the original issue are here on the Ynput Community Forum

Could you elaborate why you picked this fix over just taking a different output format from pip?

Since this also worked:

.\.poetry\bin\poetry.exe run pip --no-color list --format=freeze 

As such, the actual returned result from:

$FreezeContent = & "$($env:POETRY_HOME)\bin\poetry" run python -m pip --no-color freeze

Would still have the invalid values?


Or were those dependencies never supposed to be there?

iLLiCiTiT commented 6 months ago

The thing is that ayon dependencies tool first step is to install all dependencies that launcher had. If there is just version, but it was installed using git, you may end up with different package, or it may not find the package at all. e.g. acre is not on pypi (as far as I know), so it would actually crash because pip would not be able to install it.

In other words, the packages are not just metadata, but are really used. Real solution is to use only pypi packages.

BigRoy commented 6 months ago

This PR did fix a lot, but not everything.

Rebuilding the launcher gave me this JSON: AYON-1.0.2-dev.1-win-setup.exe.json

Note one of the paths now being:

git+https://github.com/pypeclub/acre.git@HEAD

Which the Ayon Dependency Tool doesn't like: ayon_dependency_tool_error

So it did fix some - but not the full usage of the dependency tool.

BigRoy commented 6 months ago

The HEAD reference originates from the poetry lock in the repository - see here