Respect platform's wheels for dependencies (Fixes #558)

[vphilippon]

Keep pip.Wheel monkey patch only in the get_hashes context.

Fixes #558

Contributor checklist

• Provided the tests for the changes
• Added the changes to CHANGELOG.md
• Requested (or received) a review from another contributor

[vphilippon]

Requesting review from @suutari-ai and @jdufresne
(As a side note, any reason why I wasn't able to add you as "official reviewers"?)

[jdufresne]

Typo platoforms -> platforms (multiple docstrings)

[jdufresne]

I would wrap this in a try/finally in case an exception is raised and then caught somewhere else in the stack.

try:
    yield
finally:
     Wheel.supported = original_wheel_supported
     Wheel.support_index_min = original_support_index_min
     self._available_candidates_cache = original_cache

[vphilippon]

Well I learned something today, I though the exiting part would always be called, even with an exception, without the try. Just tried it out and you're right, will fix this, thanks.

[jdufresne]

LGTM. Thanks for investigating and fixing this!

[suutari-ai]

Thanks @vphilippon. Good work!

Nevertheless, don't get me wrong, but I think this is still quite hacky, and I don't see the point. Why don't you just revert the commit 4900c7c which is causing this? Why would I ever want hashes of non-compatible wheels to my requirements.txt?

If you're trying to generate a requirements.txt which works for all platforms and Python versions, then this would make sense, but since the dependencies aren't calculated for all platforms, but only for the current one, the requirements.txt doesn't work for another platform anyway. Creating a requirements.txt which works for all platforms is requested in #563 and if that is ever implemented this hack would cause more harm than good.

As I said in PR #460, my opinion is that requirements.txt should be locking a single set of the requirements which is tested to work. The point of locking the versions is to get reproducible environment and if the platform or Python version is different, then it's no more reproducible: Those packages might act very differently on another platform and, as seen here, even the required packages might be different.

The whole point why I want locked requirements is confidence and having platform independent requirements.txt doesn't help there.

If I want to install a project for another platform (which is untested), then I should be using the requirements.in to generate a new lock file (requirements-{new_platform}.txt) and then install from that. If it works, the new lock file can be committed as a sign of "this is also a supported and tested platform".

[suutari-ai]

As a side note, any reason why I wasn't able to add you as "official reviewers"?

Because I wasn't member of @jazzband yet. I joined now. Didn't know it's that easy to join.

[jdufresne]

Nevertheless, don't get me wrong, but I think this is still quite hacky, and I don't see the point. Why don't you just revert the commit 4900c7c which is causing this? Why would I ever want hashes of non-compatible wheels to my requirements.txt?

A lot of this is described in the commit message and issue #520. But I'll summarize.

I work on a project that has a diverse group of environments:

• Different Linux distros
• MacOS
• All Pythons are 3.4+ but could be on a different version

My Project is Python 3 only. For security purpose, I want to generate package hashes for some additional verification that the download is correct.

In 1.9.0, when generating requirements.txt on one machine it might fail when running on another machine. It failed because the hashes generated were not compatible with the running OS & Python
version. As I saw it, I had some options:

1. Update pip-tools to generate all hashes
2. Stop generating hashes
3. Unify all developers on one platform

I saw 1 as the best tradeoff at the time (and still do).

If you're trying to generate a requirements.txt which works for all platforms and Python versions, then this would make sense, but since the dependencies aren't calculated for all platforms, but only for the current one, the requirements.txt doesn't work for another platform anyway. Creating a requirements.txt which works for all platforms is requested in #563 and if that is ever implemented this hack would cause more harm than good.

Maybe it is just "luck" of my dependencies, but I'm not seeing an issue regarding transitive packags in my project.

The whole point why I want locked requirements is confidence and having platform independent requirements.txt doesn't help there.

I understand. But with hashes specific to binary platforms, this means the requirements.txt is only useful for identical environments to which it was generated. This seems limiting to me.

[vphilippon]

@suutari-ai Thanks for the review and detailed explanation of your concern. As @jdufresne mentionned, while a compiled requirements.txt is not guaranteed to be cross-platform compatible, it can be in some cases, like his.

Why would I ever want hashes of non-compatible wheels to my requirements.txt?

In the case where a requirements.txt is cross-platform (i.e. there are no dependencies difference between platforms), but the packages still provides different wheels per platform (i.e. these packages requires compilation), having all hashes would keep the happy case of having a requirements.txt that is cross-platform compatible.

Currently, I see no harm in having --generate-hashes including all hashes.
I agree with you that reusing a requirements.txt on multiple platforms is something that should be done really carefully, and on a case-by-case basis after making sure there are no dependencies difference. That's the case of @jdufresne. Following the "we are all adult here" mindset of Python, I wouldn't feel justified to remove this feature as I was able to make it work and it provides some value to some users.

If it had been impossible (or way too hard) to keep the feature without breaking the dependencies resolving, I would have suggested to revert of course.

@suutari-ai I think a case justifying the feature to be kept has been provided. If you have a case or example that would prove this feature to be harmful to the users, outweighing the gain, please provide it (this is exactly why I requested your review, I definitely wanted a second opinion on this).

[jdufresne]

If it had been impossible (or way too hard) to keep the feature without breaking the dependencies resolving, I would have suggested to revert of course.

@suutari-ai I think a case justifying the feature has been provided. If you have a case or example that would prove this feature to be harmful to the users, outweighing the gain, please provide it.

I think this is very fair. If an argument or evidence can be provided to show that this will be too difficult to maintain or is breaking previously working environments. Then I agree, reverting the feature is justified.

[suutari-ai]

@jdufresne:

1. Update pip-tools to generate all hashes
2. Stop generating hashes
3. Unify all developers on one platform

You're missing couple options:

4. Unify all developers to use one of N pre-specified platforms and generate a requirements-{platform_tag}.txt for each supported platform
5. Let CI and deployment scripts use requirements.txt, but developers on a different platform use requirements.in.

IMHO this would make sense, since having only the hashes of the main platform in the txt file should help ensuring that CI is testing the same stuff as is installed to production.

But anyway. I'm not against merging this. Even though it's not something that I want, it's still a feature that someone finds useful. And merging this should make pip-compile work better and would only make it easier to add an option to disable the 'generate hashes for all platforms'.

[suutari-ai]

@vphilippon:

Currently, I see no harm in having --generate-hashes including all hashes.
I agree with you that reusing a requirements.txt on multiple platforms is something that should be done really carefully, and on a case-by-case basis after making sure there are no dependencies difference.

For me the 'generate hashes for all platforms' feature is more harmful than useful, because I don't want hashes of another platform to my txt files. It's also encouraging users to reuse the same requirements.txt on different platforms, which IMHO is not a good idea.

• There could be dependencies which are missing from the txt file. Happens when the generating platform didn't have those dependencies, but the using platform has. (E.g. enum34 on python 2). Then those dependencies are not pinned and things might break when they get upgraded silently.
• There could be extra dependencies in the txt file. Happens when the generating platform has some dependencies which the using platform doesn't.
• It's also possible that some version of a dependency is not available for all platforms which would affect which version is pinned.

But the feature is there already. Fixing it is wise. 👍

[jdufresne]

Unify all developers to use one of N pre-specified platforms and generate a requirements-{platform_tag}.txt for each supported platform

This seems like a mess to manage, honestly.

Let CI and deployment scripts use requirements.txt, but developers on a different platform use requirements.in.

A problem with this is the requirements in requirements.in aren't pinned. So different development environment will be using different versions of the dependencies, causing subtle behavior differences.

Easily pinned dependencies was the killer feature to adopt pip-tools in the first place.

For me the 'generate hashes for all platforms' feature is more harmful than useful, because I don't want hashes of another platform to my txt files.

Hmm, this is interesting to me. @suutari-ai I think you make a convincing argument. I may be persuaded that generating all hashes was a bad idea for a requirements.txt that will be used on a production server. I honestly don't know what is best at this point. If there is large agreement to revert the feature, I won't stand in the way.

[suutari-ai]

Unify all developers to use one of N pre-specified platforms and generate a requirements-{platform_tag}.txt for each supported platform.

This seems like a mess to manage, honestly.

Yeah, this certainly needs some tooling and a new feature so that it would be possible to generate a txt for another platform.

Let CI and deployment scripts use requirements.txt, but developers on a different platform use requirements.in.

A problem with this is the requirements in requirements.in aren't pinned. So different development environment will be using different versions of the dependencies, causing subtle behavior differences.

Different platforms will also cause behavior differences. Only way to get exactly same behavior is to remove that factor too.

Also, if it's not working with a certain version of a requirement, you should either (a) make it work with it by changing your code or (b) limit out the non-working versions in requirements.in (using specifiers like <ver or ~=). The locked requirements.txt should be easily upgradeable anyway, so the project should usually just work after regenerating the requirements.txt.

For me the 'generate hashes for all platforms' feature is more harmful than useful, because I don't want hashes of another platform to my txt files.

Hmm, this is interesting to me. @suutari-ai I think you make a convincing argument. I may be persuaded that generating all hashes was a bad idea for a requirements.txt that will be used on a production server. I honestly don't know what is best at this point. If there is large agreement to revert the feature, I won't stand in the way.

Maybe it should be optional at least? (Which would be another PR, of course.)

For Prequ, I just reverted it (suutari#12), since my plan there is to implement the 'generate a txt for another platform' feature, but it seems that pip-tools is going to another direction. That's OK too, since I'll probably just use my own tool anyway. 😄

If the feature is not reverted, then this fix should be merged at least.

[vphilippon]

@suutari-ai @jdufresne Thanks you both for your input on this.
I'll go ahead and merge the fix.