rez icon indicating copy to clipboard operation
rez copied to clipboard

distlib parses version string instead of using version_info to make script names

Open cfxegbert opened this issue 10 months ago • 7 comments

Use version_info to determine python version when building scripts

cfxegbert avatar Apr 09 '24 18:04 cfxegbert

Hi @cfxegbert, can you tell us more about the reason for this fix please? The changes you made are inside a vendored project that we do not own.

When you install rez with install.py it creates the executable and the executable with the python major.minor version number

-rwxr-xr-x@ 1 root  wheel  232 Mar 13 10:42 rez
-rwxr-xr-x@ 1 root  wheel  232 Mar 13 10:42 rez-3.9
-rwxr-xr-x@ 1 root  wheel  252 Mar 13 10:42 rez-benchmark
-rwxr-xr-x@ 1 root  wheel  252 Mar 13 10:42 rez-benchmark-3.9
-rwxr-xr-x@ 1 root  wheel  242 Mar 13 10:42 rez-bind
-rwxr-xr-x@ 1 root  wheel  242 Mar 13 10:42 rez-bind-3.9
...

If you install with python-3.10 or later you will end up the files like rez-3.1 instead of rez-3.10.

The vendor library has already been modified by others so it is already divergent from the original source.

cfxegbert avatar Apr 10 '24 17:04 cfxegbert

It looks like a similar fix has be made upstream. https://github.com/pypa/distlib/blob/master/distlib/scripts.py#L298-L301

cfxegbert avatar Apr 10 '24 17:04 cfxegbert

@cfxegbert, I see in the vendor folder's README.md file that the description is

"Updated (June 2019) to enable wheel distribution based installations."

I wonder if we can just update to the latest release of distlib and it'll kill 2 birds with one stone (so we don't have a divergent library in our vendor folder)?

brycegbrazen avatar Apr 10 '24 19:04 brycegbrazen

There is also https://github.com/AcademySoftwareFoundation/rez/issues/1668.

If we update distlib to the latest/greatest do we have the tests in place to verify it? Looks like distlib is used in install.py, src/rez/utils/pip.py, src/rez/pip.py, src/rez/bind/hello_world.py and src/rez/tests/test_pip_utils.py

cfxegbert avatar Apr 10 '24 21:04 cfxegbert

@cfxegbert, I see in the vendor folder's README.md file that the description is

"Updated (June 2019) to enable wheel distribution based installations."

Changes that are not upstream:

util.py
a195feaba (Allan Johns       2021-06-16 15:18:33 +1000  218)
de4737f98 (Stephen Mackenzie 2022-08-28 17:39:30 -0400  219)                         # https://github.com/AcademySoftwareFoundation/rez/pull/1092
8412dc1a2 (Blazej Floch      2021-06-03 17:10:56 -0400  220)                         # Some packages have a trailing comma which would break the matching
8412dc1a2 (Blazej Floch      2021-06-03 17:10:56 -0400  221)                         if not ver_remaining:
8412dc1a2 (Blazej Floch      2021-06-03 17:10:56 -0400  222)                             break
a195feaba (Allan Johns       2021-06-16 15:18:33 +1000  223)                         # /end pull/1092
a195feaba (Allan Johns       2021-06-16 15:18:33 +1000  224)

cfxegbert avatar Apr 10 '24 21:04 cfxegbert

@cfxegbert Looks like it's fixed in this commit on the distlib GitHub. It's part of the v0.3.3 release.

brycegbrazen avatar Apr 10 '24 21:04 brycegbrazen

Closing this since I don't think that it's the right way to fix this.