[WIP] Feature/pip install modern #602

lambdaclan · 2019-04-19T08:58:15Z

Description

[In collaboration with #599]

The Python community is steadily moving away from eggs and transitioning into wheels.

Wheels offer faster installation for pure Python and native C extension packages, avoid arbitrary code execution for installation. (avoids setup.py) and offer more consistent installs across platforms and machines.

One of the most important differences is the cross platform compatibility. Furthermore the current
implementation of the pip install command is using install options to split and point the installation to a custom directory (libs, scripts, includes etc) is enforcing the use of source packages and disables the
modern pip default of using a wheel if available. This means that everything that is not a pure python package needs a proper, working and matching compiler setup making it a real pain for Windows users whom may need multiple visual studios versions etc...

The difference with #599 is that it enforces the use of wheels via PEP 517 which seems to be the way forward. If a package does not have a wheel then one is being built prior to installation ensuring
consistency.

There is still ongoing discussion here from the pip maintainers but a few things that got my attention are the following statements:

we intend (at some point) to remove the legacy "install via setup.py" code path from pip.

We have that legacy mode at the moment - setup.py installs are (at least in my mind) legacy since PEP 517 support was released. The problem is that we can’t support that (or any other) legacy mode indefinitely, so the question is when (and how) we remove it.

Another interesting discussion to follow is the one here about handling install options with wheel packages.

Relates # (issue)
rez #475, #503 maybe more...

pip pypa/pip#4611 pypa/pip#4501

Type of change

Breaking change (fix or feature that would cause existing functionality to not work as expected)

Remarks

Requires pip >=19.0
Requires distlib >= 0.28

How Has This Been Tested?

I have installed a plethora of packages from known to obscure with good success rate.
Some issues still occur (unable to build a wheel - very rare) depending on the platform but the same problems would also occur with the current implementation.

I have also successfully installed the majority of the packages listed on the Python wheels website which do not yet provide a wheel without issues.

Test Configuration:

Python version: 2.7.15
OS: Linux, Windows 10
Toolchain: pip 19.0.3, distlib 0.2.8

Todos

Tests
Documentation
Further investigation about dependency gathering (run-time and optional)
Metadata and distlib API research notably DependencyFinder and Metadata 2.1 along with PEP 508

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works

mottosso · 2019-04-24T09:14:25Z

Differences between this and #599

This PR..

Enforces use of wheel
Backwards breaking
No update to distlib

(1) seems like a good idea, though not 100% sure we'd want to require anything this close to bleeding-edge pip.

For (2), this doesn't have to be a breaking change I think. What do you think about the proposal in #599, of maintaining current behavior and adding a --use-wheel argument? It could speed up merging and provide a transition path for anyone interested in using this new feature. Without it, it's a little risky to update Rez once this has been merged, especially for those not keeping up to date with PRs.

For (3), I thought we needed to update it? You found a way of using the still-old release? :O

JeanChristopheMorinRodeoFX · 2019-04-24T13:34:52Z

though not 100% sure we'd want to require anything this close to bleeding-edge pip

Wheels are far from being bleeding edge. They've been used for a couple of years now. Wheels are the standard since a year or 2.

Without it, it's a little risky to update Rez once this has been merged, especially for those not keeping up to date with PRs.

If there are some people that don't look at release notes or even testing before updating, they live an extremely extreme life. Though, I know Allan likes to not break compatibility AFAIK. On my side, if we can avoid the breakage, why not. But if it's necessary, why not too.

mottosso · 2019-04-24T13:42:36Z

Wheels are far from being bleeding edge.

I was actually referring to pip-19.

JeanChristopheMorinRodeoFX · 2019-04-24T13:57:00Z

I see, thanks for the clarification. I don't see any problem with using pip >= 19. 19.0 was released in January. 19.1 just got released (like 11 hours ago). So >19.0,<19.1 isn't considered bleeding edge while 19.1+ could be considered bleeding edge, but normally pip is quite stable. In 4-5 years, I've rarely hit pip issues. If there is something to worry about, it's PEP-518, not pip>=19.0 nor PEP-517` IMO.

nerdvegas · 2019-05-26T01:11:44Z

I agree with assessment of current rez-pip limitations. Wheels seem the way forward. Backwards compatibility in rez is paramount. For this reason I am in favour of supporting this new code path either via a `--use-wheel` option as Marcus suggests; or, a separate cli altogether (rez-pip2 or rez-wheel perhaps - leaning towards rez-pip2). The reason I suggest a separate cli is because I'd be concerned about ending up with a confusing mix of wheel-compatible and non-wheel-compatible options available in the same tool (ie some options that make sense in combo with --use-wheel and some that do not). Ira if you're getting good results then I'm keen to see more. Thx A

…

On Wed, Apr 24, 2019 at 11:57 PM JeanChristopheMorinRodeoFX < ***@***.***> wrote: I see, thanks for the clarification. I don't see any problem with using pip >= 19. 19.0 was released in January. 19.1 just got released (like 11 hours ago). So >19.0,<19.1 isn't considered bleeding edge while 19.1+ could be considered bleeding edge, but normally pip is quite stable. In 4-5 years, I've rarely hit pip issues. If there is something to worry about, it's PEP-518 <https://www.python.org/dev/peps/pep-0518/>, not pip>=19.0 nor PEP-517` IMO. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#602 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAMOUSXBFMDK2YRFKDZV4YTPSBRK3ANCNFSM4HHC5GKA> .

nerdvegas · 2019-05-26T01:38:23Z

Hi @lambdaclan , @motto,

How does this and #599 relate? Are they competing implementations that we need to choose from? Would like to talk more about these PRs in reference to one another before moving forward.

Thx
A

lambdaclan · 2019-05-26T23:50:23Z

Hello Allan, thank you very much for taking the time to look at the pull request.

I have been using my changes above for quite some time without any issues and I am sure Marcus can attest to this since he has been doing the same.

Although the PR from Marcus is based on this one we are doing things a bit differently such as me enforcing wheel installs. I have also updated distlib but I did not push those changes so as not to make the PR harder to read. I will do so before the final merge. The PR clearly states that a new version of distlib is required for wheel installs to work.

Maybe I was overly ambitious in thinking that we can get rid of the old style installs since they are considered legacy now but since backwards compatibility is important then I can keep the original install method in place.

As such, I am happy to comply with the suggested solution of adding a new separate cli that will only work with wheels instead of using a flag such as --use-wheels to avoid the issue of wheel compatible incompatible packages as you mention. Using a specific command for wheel installs will be immediately apparent that packages are installed using wheel packages .

In doing so, It will also be easier to deprecate the old code base when the time comes rather than trying to keep the code from a flag option.

@nerdvegas @mottosso I am willing to do the changes outlined above if it is OK with you so please let me know so that I can proceed.

As I mentioned in another post, I do see some duplication of work happening and it is a shame. We need a way to track who is doing what to encourage people to work on other features/issues or promote collaboration between community members. This is really important in my opinion, what do you think would be a suitable solution? Maybe having a rez kanban board?

I am happy to work with Marcus on this if he wants otherwise I will just carry on.

Looking forward to your input. Thank you.

Ira

mottosso · 2019-05-27T07:16:42Z

How does this and #599 relate?

In short, #599, #595 and #596 is consolidated into #614 which is akin to #628, but different.

I would recommend consolidating this into #628 as well, so we can have a look at both and determine what we like and dislike about each and ultimately consolidate each into one for final merge. Smorgasbord!

We need a way to track who is doing what to encourage people to work on other features/issues or promote collaboration between community members.

IMO this is a little over the top. A simple "Hey, I'm working on X" in the chat should suffice. The reason for these duplicate PRs isn't a lack of kanban, but rather:

I needed pip
You had a branch I wasn't able to merge (squashed)
I re-implemented the feature on my end and made WIP Feature/pip wheels windows #599
You refactored your branch and made [WIP] Feature/pip install modern #602
I continue the work in WIP Implements #612, Useful pip #614, where things start to diverge
You continue yours in [Feature] Pure python package detection #628, divergence continues

And as a result, we've got some lovely exploration and features to choose between. :)

lambdaclan · 2019-05-27T07:43:02Z

I would recommend consolidating this into #628 as well, so we can have a look at both and determine what we like and dislike about each and ultimately consolidate each into one for final merge. Smorgasbord!

I do not mind either way so I will leave it up to Allan to decide. We will still need to extract the wheel installation method to a new separate cli tool as outlined above and add some tests. I am happy to help
if Marcus is on it then I will do something else.

We need a way to track who is doing what to encourage people to work on other features/issues or promote collaboration between community members.

IMO this is a little over the top. A simple "Hey, I'm working on X" in the chat should suffice. The reason for these duplicate PRs isn't a lack of kanban, but rather:
1. I needed pip

2. You had a branch I wasn't able to merge (squashed)

3. I re-implemented the feature on my end and made #599

4. You refactored your branch and made #602

5. I continue the work in #614, where things start to diverge

6. You continue yours in #628, divergence continues
And as a result, we've got some lovely exploration and features to choose between. :)

That would be nice but unfortunately I do not think that everyone publicly announces what they are working on furthermore a message on chat can easily missed by other members of the community and also not seen at all by people not part of the chatting group.

Having a kanban is more official and lets everyone know what active contributors are working on. It will also allow us to work on features/issues that matter based on a roadmap.

I am not in control so there is not much I can do but these are my thoughts. I dont want to know how many people have branches doing the same work as others because their studio needs it...

I want to make clear that I do not mind collaborating on the same issue with you or anyone else but we do need a way to track everything.

mottosso · 2019-05-27T07:50:49Z

I am happy to help if Marcus is on it then I will do something else.

Sure, I'm happy to handle pip. Each of yours and Allans requirements (both here and #628) are included in #614 so we could continue from there.

nerdvegas · 2019-06-23T10:25:51Z

src/rez/pip.py

-        for installed_file in distribution.list_installed_files(allow_fail=True):
-            source_file = os.path.normpath(os.path.join(destpath, installed_file[0]))
+        for installed_file in distribution.list_installed_files():
+            if 'bin' in installed_file[0]:


It's not obvious what's going on here, a comment would help. Also, I take it that this covers a case where installed_file[0] is present within a 'bin' dir; in this case, a substring search isn't robust enough, I would search for 'bin' in the list generated from str.split(os.sep). I gather that placement of said bin dir is important also, I'd check for that as well.

I seems like I also commented this line in https://github.com/nerdvegas/rez/pull/628/files#r296459995. I'm always confused when the same change appears in two different PRs.

Here is the specification of the database of installed python distributions: https://www.python.org/dev/peps/pep-0376/#record. I will probably help to do a proper implementation that will handle all cases.

I have been looking into ways to improve the readability of this. Since this is installed as a wheel distribution, all the installed files are listed in a CSV file called RECORD including the bin scripts as such there is no worry of missing any files from the list.

I am aware of the specification for the database generated pointed out by JC but there seems to be some inconsistencies when combining the wheel install with the target flag to set a directory. Specifically, the bin script paths in the RECORD file
are wrong.

Essentially it points the bin scripts to be two directories up instead of being in the base location.

Have a look at an example install of pydocstyle library:

The bin folder is right there along with the rest of source files but a quick look into the RECORD file, revelas that the bin location should be somewhere else...

This is why you see all that weird parsing and path combining taking place. I am trying to see if I am missing something but the rest of the path are correct starting right from the destpath location as base.

Using --prefix instead of --target will put the files in a site-packages directory and the scripts in a bin folder

This time the RECORD file points the bin scripts to be 3 directories up from base.

which is strangely correct but doing so will require to parse the source packages out of lib/python/site-packages..

I will keep looking into for a bit longer and see if I find anything.

Relevant issue: pypa/pip#3934

I am now moving the bin folder to where the RECORD file expects it to be ../../bin and everything is working fine. The checks have become very simple.

Yeah I see what you mean, it's odd!

nerdvegas · 2019-06-23T10:27:59Z

src/rez/pip.py

+                if is_exe(source_file):
+                    if destination_file.startswith("%s%s" % (os.path.join("python", "bin"), os.path.sep)):
+                        destination_file = os.path.join("bin", os.path.basename(source_file))
+                        _, _file = os.path.split(destination_file)


_file = os.path.basename(destination_file)

nerdvegas · 2019-06-23T10:39:36Z

Hey @lambdaclan,

I'm pretty happy to merge this very soon, I just had one suggestion regarding backwards compatibility. Since support for pep517 was added in pip-19, I think it'd be sufficient to check the pip version and use this new code in that case, rather than introducing a --use-wheel as suggested before. It feels like that may be a cleaner way to go about it, and will avoid the confusion of extra options. It seems to me that wheels are prevalent enough that we can go that route regardless.

What do you think? I'll be back on this tomorrow, I'm happy to make those changes if we want to do that and if you haven't already. I know this is holding up #628 so I'm keen to get it merged.

Cheers
A

JeanChristopheMorinPerso · 2019-06-24T14:49:45Z

src/rez/pip.py


        with make_package(name, packages_path, make_root=make_root) as pkg:
-            pkg.version = distribution.version
+            pkg.version = distribution.metadata.version


Note that we need to be careful with this. The python version schema specification isn't 100% compatible with rez.

I have been using metadata in a couple of places for summary and name, those shouldn't be an issue I guess. I never had an issue with version but I can revert back to the original if necessary.

This is an issue I have been getting but I fixed it on the other PR

98be115

I'd second JC here, even if initially there's no remapping at all, there should be a separate pip_to_rez_version function.

Taking a look at https://www.python.org/dev/peps/pep-0440/#version-scheme though, there are some things to take into account. I can see two potential issues
1: version epochs (they make no sense to rez, so they'd just get stripped of the leading N!;
2: python versions are case insensitive, so they should probably be lowercased when converted to a rez version.

Local versions are also not compatible with rez (https://www.python.org/dev/peps/pep-0440/#local-version-segments) BTW, local is something we use at RodeoFX for our pip packages when we fork a repo for example. Though, we don't use rez-pip since we have our own implementation. But some other studios could use the local versions. At Rodeo, we basically convert package-1.0.0+<something> to package-1.0.0-<something>.

Most possibilities are described in https://www.python.org/dev/peps/pep-0440/#examples-of-compliant-version-schemes.

lambdaclan · 2019-06-24T23:46:57Z

Hello @nerdvegas and @JeanChristopheMorinRodeoFX

Apologies for the delayed reply. With the latest progress taking place in the repository it is becoming diffucult to keep track of all the incoming emails.

I was waiting to see what will happen between this PR and Marcuse's one from #614

I am happy to help if Marcus is on it then I will do something else.

Sure, I'm happy to handle pip. Each of yours and Allans requirements (both here and #628) are included in #614 so we could continue from there.

Since you have decided to move forwards with this PR and #628 then I will go ahead and the do the changes requested.

If anything else pops up please comment on either PR and I will look into it. I will keep an eye on both to make sure I do not miss anything.

Remove staging directory paths required by old pip install method. Add comment to explain why the bin path although unused has been retained. Relates pypa/pip#5983, #475, #503

Update the pip install arguments to use a specific target directory to install the requested library. Set the install command to use PEP 517 for building source distributions to ensure a wheel is always built before installation. Relates pypa/pip#4501, #475, #503

Build a wheel before installation if the requested package does not already have a .whl file. No longer check for egg distributions. Relates #475, #503

Stop checking for dependencies using run requires. Relates #475, #503

Use metadata attributes to retrieve core data such as the name, version and summary of the requested package. Relates https://www.python.org/dev/peps/pep-0426/#core-metadata, #475, #503

Use wheel dist-info metadata and record files to perform the installation of the requested package including all its dependencies. BREAKING CHANGE: Currently rez is using `distlib` 0.2.4 released in late 2016. That version of the library does not correctly detect installed wheel distributions meaning that there is no access to metadata and record files. To fix your project, update your `distlib` library located in the src/vendor directory.

Revert dependency gathering method because new API methods such as distutils DependencyFinder and metadata extra, dependency fields are still not well documented or widely adopted with package maintainers. Ensure each dependency is installed as an individual rez package to avoid duplicates. Relates nerdvegas#475, nerdvegas#503

nerdvegas · 2019-06-25T00:54:23Z

Thanks Ira! Both rez-pip and the installation process are things I want to get sorted asap, so I'll be as responsive as possible on this. Just a note - my family and I are moving Friday this week so I'll more than likely go dark for 3-4 days, so if I don't reply to emails around then, that's why. Cheers A

…

On Tue, Jun 25, 2019 at 9:46 AM Ira ***@***.***> wrote: Hello @nerdvegas <https://github.com/nerdvegas> and @JeanChristopheMorinRodeoFX <https://github.com/JeanChristopheMorinRodeoFX> Apologies for the delayed reply. With the latest progress taking place in the repository it is becoming diffucult to keep track of all the incoming emails. I was waiting to see what will happen between this PR and Marcuse's one from #614 <#614> I am happy to help if Marcus is on it then I will do something else. Sure, I'm happy to handle pip. Each of yours and Allans requirements (both here and #628 <#628>) are included in #614 <#614> so we could continue from there. Since you have decided to move forwards with this PR and #628 <#628> then I will go ahead and the do the changes requested. If anything else pops up please comment on either PR and I will look into it. I will keep an eye on both to make sure I do not miss anything. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#602>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAMOUST26YVXOTF5AISRN5DP4FMHHANCNFSM4HHC5GKA> .

lambdaclan · 2019-06-25T01:02:57Z

Hello Allan, no problem at all, thanks for letting us know.

I hope the move goes smoothly!

I am going through your and JC comments now and should have something ready for review fairly soon.

Ira

JeanChristopheMorinPerso · 2019-06-25T01:13:57Z

Thanks for letting us know @nerdvegas . @lambdaclan I'll keep an eye open and will do another pass of review when you will push. You might have to look at #628 because I might have do some comments there that are applicable here (sorry about that).

Just so we are all on the same page, this PR enforces the usage of wheels only and I think we will need a PR to update distlib right?

lambdaclan · 2019-06-25T01:35:52Z

Hello JC, thank you very much for your time reviewing my PRs.

Feedback from you and Allan is highly appreciated since I am fairly new to the project and want to make sure that everything works as intended without comprising existing functionality.

Indeed, this PR enforces the usage of wheels, if a packages does not have a wheel one is automatically built during install and then process continues as expected. As pip updates keep coming in we will possibly need to make adjustments to rez-pip as well. For example enforcing pep-517 might become the default in the near future.

In terms of distlib, it is a pre-requisite for this feature (and #628) to work so I can either make another PR or add it to this one. I purposely kept it out to make it easier to read.

PR #628 depends on this one since wheel installation method is a per-requisite. Let us get this one sorted out first and then I will move onto that one.

Hope this clears things a bit.

Ira

nerdvegas · 2019-06-25T09:43:18Z

Hey @lambdaclan,

It seems like retaining backwards compatibility for pip-<19 may be more effort than it's worth. I think it'd be fine to move ahead without out it, so long as there's a check for this which prints an obvious error message. Let's just go with that?

nerdvegas · 2019-06-25T09:43:45Z

Um, that close was an accident!!

nerdvegas · 2019-06-25T10:17:20Z

src/rez/pip.py

    distributions = [d for d in distribution_path.get_distributions()]

+    folders = [folder for folder in os.listdir(destpath) if os.path.isdir(os.path.join(destpath, folder))]


I would add a comment along the lines of, "moving bin folder to expected relative location, rather than under python/" etc. Also, simpler code would suffice:

staged_binpath = os.path.join(destpath, "bin") if os.path.isdir(staged_binpath): shutil.move(...)

Yes indeed, since we only care about that path no reason to loop. It always seems to be under the same location so no need to recurse the directory for other bin folders. That is what I had in mind initially.

nerdvegas · 2019-06-25T10:19:45Z

src/rez/pip.py

+            # destpath = /tmp/pip-HBkRGa-rez/re]z_staging/python
+            # installed_file[0] = ../../bin/pydocstyle
+            # normpath of /tmp/pip-HBkRGa-rez/rez_staging/python../../bin/pydocstyle =  /tmp/pip-HBkRGa-rez/rez_staging/bin/pydocstyle OK
+            # normapth of /tmp/pip-HBkRGa-rez/rez_staging/python/../../bin/pydocstyle = /tmp/pip-HBkRGa-rez/bin/pydocstyle WRONG


The comment and implementation threw me for a little bit here.

The comment should just explain the odd case: that ../../bin/etc is provided by distlib when in fact ../bin/etc seems to be the correct path instead. I was a little confused because the comment here delves into the implementation more than it does explain the situation.

Agreed, I was overly verbose aplogies. I will remove the implementation details and reword the comment a bit.

nerdvegas · 2019-06-25T10:24:56Z

src/rez/pip.py

+            # installed_file[0] = ../../bin/pydocstyle
+            # normpath of /tmp/pip-HBkRGa-rez/rez_staging/python../../bin/pydocstyle =  /tmp/pip-HBkRGa-rez/rez_staging/bin/pydocstyle OK
+            # normapth of /tmp/pip-HBkRGa-rez/rez_staging/python/../../bin/pydocstyle = /tmp/pip-HBkRGa-rez/bin/pydocstyle WRONG
+            if installed_file[0].startswith("."):


I would make this clearer, perhaps like so:

installed_filepath = installed_file[0] bin_prefix = os.sep.join('..', '..', 'bin') + os.sep ... if installed_filepath.startswith(bin_prefix): # account for extra parentdir as explained above installed = os.path.join(destpath, '_', installed_filepath) else: ...

Never knew that normpath will normalize a pathname by collapsing all redundant separators and up-level references. I always thought it was limited to dots and slashes, awesome. 👍

nerdvegas · 2019-06-25T10:29:24Z

src/rez/pip.py


            if os.path.exists(source_file):
-                destination_file = installed_file[0].split(stagingsep)[1]
+                destination_file = source_file.split(stagingsep)[1]


I apologise for my own (old) rubbish code! This split is odd, simple use of os.path.relpath would be clearer

destination_file = os.path.relpath(source_file, stagingdir)

nerdvegas · 2019-06-25T10:32:17Z

src/rez/pip.py

@@ -317,11 +332,10 @@ def make_root(variant, path):

        variant_reqs.append("python-%s" % py_ver)

-        name, _ = parse_name_and_version(distribution.name_and_version)
-        name = distribution.name[0:len(name)].replace("-", "_")
+        name = distribution.metadata.name


The rez package name can't be simply set to the dist name, because some pip packages have hyphen in the name. In rez this is not a valid package name (it would be interpreted as the start of the version, eg my-pkg-1.2 is 'my', version 'pkg-1.2').

In fact my own original code wasn't clear enough - even if all it's doing is replacing hyphen with underscore (as the original code did), it would be clearer if it were in a func such as pip_to_rez_package_name. This behaviour also needs to stay, for backwards compatibility reasons (if a studio used new rez-pip which then mapped to a different rez pkg name, that would cause a lot of problems).

nerdvegas · 2019-06-25T10:51:09Z

Thanks @lambdaclan, please see comments. I think we're close.

I'll merge the distlib version update PR along with this once it's ready.

FYI tomorrow night is my last chance to do the merge before I move house! My pc will be packed up after that :)

Cheers all
A

JeanChristopheMorinPerso · 2019-06-25T23:00:53Z

@lambdaclan Have you tested with both a package that is only available as a source distribution and also tested with a wheel and an egg (Maybe we can skip eggs, as this is quite an old format that I've not seen in a while in PyPI)?

Thinking quickly, PyYAML for Linux doesn't have a wheel in PyPI and same for psutil, PyPI only contain sources for these packages on Linux.

lambdaclan · 2019-06-25T23:23:38Z

Hello @nerdvegas

It seems like retaining backwards compatibility for pip-<19 may be more effort than it's worth. I think it'd be fine to move ahead without out it, so long as there's a check for this which prints an obvious error message. Let's just go with that?

Sounds good to me, definitely keeping the old codebase in for installations from source would take some time, with wheels available for almost every package a pip version check as you said, should suffice. Furthermore, when a wheel is not available one is being built automatically.

@JeanChristopheMorinRodeoFX

Have you tested with both a package that is only available as a source distribution and also tested with a wheel and an egg (Maybe we can skip eggs, as this is quite an old format that I've not seen in a while in PyPI)?

Thinking quickly, PyYAML for Linux doesn't have a wheel in PyPI and same for psutil, PyPI only contain sources for these packages on Linux.

Hello JC, I have been using this for quite sometime and did not have any issues, the issue I has I mentioned before was a Schema error -->

In the PR post I mentioned the following:

I have installed a plethora of packages from known to obscure with good success rate.
Some issues still occur (unable to build a wheel - very rare) depending on the platform but the same problems would also occur with the current implementation.

I have also successfully installed the majority of the packages listed on the Python wheels website which do not yet provide a wheel without issues.

Saying that, I am happy to try more packages that might give issues such as the ones you mention so I will try them both (PyYAML and psutil) and let you know how it goes.

I'll merge the distlib version update PR along with this once it's ready.
FYI tomorrow night is my last chance to do the merge before I move house! My pc will be packed up after that :)

That is great Allan, lets try to get this merged today if everything goes smoothly. I will start working on the changes necessary as per your comments right away. In terms of the metadata version name, summary etc, I revert back to the orginal function for getting those now that I am aware of the issues.

Ira

rez will now enforce wheel distribution based installations. A version of pip >= 19 is required in order to do so. Add pip version check to the callbacks of rez-pip command to validate that an up to date pip is being used. Relates: #602 #628 https://pip.pypa.io/en/stable/news/#id49

lambdaclan · 2019-06-26T02:56:55Z

Tests:

pip < 19

ignore the info showing valid pip version, I have hard coded the check to 20 to make the error happen for the test

PyYAML

psutil

Review round 3. Only thing left is the version conversion I believe. Can you provide an example of a version that will not work via distribution.version? I can seem to find any libraries with exotic versions to try...

Something like 1!1.0RC1 (epoch plus case insensitive?) and what it should turn into.

I am asure we can use this to extract and convert it to a rez compatible version --> https://www.python.org/dev/peps/pep-0440/#appendix-b-parsing-version-strings-with-regular-expressions

lambdaclan · 2019-06-26T03:51:56Z

Using the REGEX mentioned above, we can easilly retrieve all the parts of a pip version and modify it accordingly.

rez-pip -i pydocstyle

.....

within the pip_to_rez_version function

parse segments and construct rez-compatible version
....

Sample output:

configparser
version_segements:  {'pre': None, 'dev_l': None, 'dev_n': None, 'post_l': None, 'pre_n': None, 'pre_l': None, 'dev': None, 'epoch': None, 'post_n1': None, 'post_n2': None, 'release': u'3.7.4', 'post': None, 'local': None}
pydocstyle
version_segements:  {'pre': None, 'dev_l': None, 'dev_n': None, 'post_l': None, 'pre_n': None, 'pre_l': None, 'dev': None, 'epoch': None, 'post_n1': None, 'post_n2': None, 'release': u'3.0.0', 'post': None, 'local': None}
six
version_segements:  {'pre': None, 'dev_l': None, 'dev_n': None, 'post_l': None, 'pre_n': None, 'pre_l': None, 'dev': None, 'epoch': None, 'post_n1': None, 'post_n2': None, 'release': u'1.12.0', 'post': None, 'local': None}
snowballstemmer
version_segements:  {'pre': None, 'dev_l': None, 'dev_n': None, 'post_l': None, 'pre_n': None, 'pre_l': None, 'dev': None, 'epoch': None, 'post_n1': None, 'post_n2': None, 'release': u'1.2.1', 'post': None, 'local': None}

What I need to know is some sort of a ruleset, for example if only release key is set then keep as it is if it has epoch etc then remove it etc.

Examples

version with epoch --> 1!1.0 ---> rez compatible version: 1.0
version with uppercase --> 1.1RC1 ---> rez compatible version: 1.1rc1
version with local segments --> 1.0+ubuntu-1 ----> rez compatible version: 1.0-ubuntu_1

Edit: Went ahead and implemented a rough approach of the above, I am sure changes will be required upon review but at least we have something to work with.

nerdvegas · 2019-06-26T10:12:59Z

Looks good, only some minor changes. My testing worked, let's deal with any further issues in separate PR. Thanks for the hard work!

lambdaclan · 2019-06-26T10:24:47Z

Wow, that was fast! Sounds good Allan. Please let me know of anything that needs to be changed and I will take care of it. I will focus on #628 next.

mottosso mentioned this pull request Apr 24, 2019

WIP Implements #612, Useful pip #614

Closed

lambdaclan mentioned this pull request May 17, 2019

[Feature] Pure python package detection #628

Merged

2 tasks

nerdvegas mentioned this pull request May 26, 2019

Make rez pip useful #612

Closed

nerdvegas added the rez-pip ingesting py pkgs into rez (pip, wheels, etc) label Jun 2, 2019

nerdvegas reviewed Jun 23, 2019

View reviewed changes

JeanChristopheMorinPerso reviewed Jun 24, 2019

View reviewed changes

lambdaclan added 7 commits June 25, 2019 08:48

refactor(rez-pip): remove include and data paths

ef14e91

Remove staging directory paths required by old pip install method. Add comment to explain why the bin path although unused has been retained. Relates pypa/pip#5983, #475, #503

refactor(rez-pip): remove egg inclusion from distribution path creation

1f31198

Build a wheel before installation if the requested package does not already have a .whl file. No longer check for egg distributions. Relates #475, #503

refactor(rez-pip): remove old way of checking for package dependencies

2791b3f

Stop checking for dependencies using run requires. Relates #475, #503

refactor(rez-pip): use dist info metadata attributes

f8c73e4

Use metadata attributes to retrieve core data such as the name, version and summary of the requested package. Relates https://www.python.org/dev/peps/pep-0426/#core-metadata, #475, #503

nerdvegas closed this Jun 25, 2019

nerdvegas reopened this Jun 25, 2019

nerdvegas reviewed Jun 25, 2019

View reviewed changes

lambdaclan added 6 commits June 26, 2019 09:38

refactor(rez-pip): move bin folder to expected relative location

cf22cd2

refactor(rez-pip): reword confusing comment (remove implementation info)

f188394

refactor(rez-pip): make extra parent dir issue more apparent

e892a97

refactor(rez-pip): avoid odd path splitting and use relpath

8cb96ed

refactor(rez-pip): revert the name creation method for rez compatibility

e0b398f

refactor(rez-pip): add pip to rez version conversion based on pep440

cdc4371

nerdvegas merged commit cdc4371 into AcademySoftwareFoundation:master Jun 26, 2019

lambdaclan mentioned this pull request Jun 28, 2019

[Fix] Wheel pip regressions #656

Merged

6 tasks

This was referenced Jul 8, 2019

[Feature] Pip QOL commands -- EXPERIMENTAL #657

Closed

[Fix] Package schema and request errors #658

Closed

[Fix] Windows rez-bind pip #659

Merged

lambdaclan deleted the feature/pip-install-modern branch April 6, 2020 06:37

JeanChristopheMorinPerso mentioned this pull request Mar 11, 2023

Rez pip doesn't handle optional requirements #475

Closed

		distributions = [d for d in distribution_path.get_distributions()]

		folders = [folder for folder in os.listdir(destpath) if os.path.isdir(os.path.join(destpath, folder))]

[WIP] Feature/pip install modern #602

[WIP] Feature/pip install modern #602

Conversation

lambdaclan commented Apr 19, 2019 • edited Loading

Description

Type of change

Remarks

How Has This Been Tested?

Todos

Checklist:

mottosso commented Apr 24, 2019

JeanChristopheMorinRodeoFX commented Apr 24, 2019 • edited Loading

mottosso commented Apr 24, 2019

JeanChristopheMorinRodeoFX commented Apr 24, 2019

nerdvegas commented May 26, 2019 via email

nerdvegas commented May 26, 2019

lambdaclan commented May 26, 2019

mottosso commented May 27, 2019 • edited Loading

lambdaclan commented May 27, 2019

mottosso commented May 27, 2019

Choose a reason for hiding this comment

JeanChristopheMorinPerso Jun 24, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nerdvegas commented Jun 23, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lambdaclan commented Jun 24, 2019

nerdvegas commented Jun 25, 2019 via email

lambdaclan commented Jun 25, 2019

JeanChristopheMorinPerso commented Jun 25, 2019

lambdaclan commented Jun 25, 2019

nerdvegas commented Jun 25, 2019

nerdvegas commented Jun 25, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nerdvegas Jun 25, 2019 • edited Loading

Choose a reason for hiding this comment

nerdvegas commented Jun 25, 2019

JeanChristopheMorinPerso commented Jun 25, 2019 • edited Loading

lambdaclan commented Jun 25, 2019 • edited Loading

lambdaclan commented Jun 26, 2019 • edited Loading

pip < 19

PyYAML

psutil

lambdaclan commented Jun 26, 2019 • edited Loading

Examples

nerdvegas commented Jun 26, 2019

lambdaclan commented Jun 26, 2019

lambdaclan commented Apr 19, 2019 •

edited

Loading

JeanChristopheMorinRodeoFX commented Apr 24, 2019 •

edited

Loading

mottosso commented May 27, 2019 •

edited

Loading

JeanChristopheMorinPerso Jun 24, 2019 •

edited

Loading

nerdvegas Jun 25, 2019 •

edited

Loading

JeanChristopheMorinPerso commented Jun 25, 2019 •

edited

Loading

lambdaclan commented Jun 25, 2019 •

edited

Loading

lambdaclan commented Jun 26, 2019 •

edited

Loading

lambdaclan commented Jun 26, 2019 •

edited

Loading