Log which python version was used during compile

[graingert]

Changelog-friendly one-liner: log which python version to use/was used

Contributor checklist

• Provided the tests for the changes.
• Requested a review from another contributor.
• Gave a clear one-line description in the PR (that the maintainers can add to CHANGELOG.md on release).
• Assign the PR to an existing or new milestone for the target version (following Semantic Versioning).

Fixes: #827

[atugushev]

Hello @graingert,

Thanks for the contribution. Personally i don't like this idea. A requirements.txt most likely commited to source control, also many developers has different interpreters (for example minor version may be different or OS), so this will cause changes to this header in requirements.txt every time pip-compile has been called by different developer. IMO it might be incovenient.

[graingert]

how about logging the PEP 496 -- Environment Markers

[atugushev]

@auvipy looks like it's been approved by mistake? It's not even working.

[atugushev]

@graingert

how about logging the PEP 496 -- Environment Markers

I'm sorry, i'm not following. What do you mean by that?

[graingert]

well you need to create a requirements.txt for each combination of markers - I'd like to see that the correct ones were used

[graingert]

eg

{'implementation_name': '',
 'implementation_version': '0',
 'os_name': 'posix',
 'platform_machine': 'x86_64',
 'platform_python_implementation': 'CPython',
 'platform_release': '5.0.0-15-generic',
 'platform_system': 'Linux',
 'platform_version': '#16-Ubuntu SMP Mon May 6 17:41:33 UTC 2019',
 'python_full_version': '2.7.15+',
 'python_version': '2.7',
 'sys_platform': 'linux2'}

[graingert]

I also mean pep 508

[ulgens]

Why do we even need this?

[graingert]

It's very easy to run pip-compile from the wrong environment

[ulgens]

Environment is not the same thing with Python version. There are bunch of other tools to learn and manage environment you're in.

[graingert]

@ulgens yes, thank you I'm aware of those.

#651 suggests creating a different requirements.txt per environment, I'd like to be able to record that information to ensure that the same environment is always used to generate each requirements.txt

[graingert]

(for example minor version may be different or OS), so this will cause changes to this header in requirements.txt every time pip-compile has been called by different developer

Which is desirable because all these differences are exposed to requirement selection, and could cause a different requirements.txt to be produced

[ulgens]

Which is desirable because all these differences are exposed to requirement selection, and could cause a different requirements.txt to be produced

So you're saying that every repo using pip-tools should have different requirement files for every minor Python version?

[graingert]

@ulgens no that's #651

[rpkilby]

I agree with @atugushev - this will generate undesirable file churn and I'd guarantee users will complain about this. Even ignoring the differing Python versions, the timestamp will always update the output. I can imagine a build process that ensures the requirements.txt is always up-to-date, then commits the diff. This change would introduce commit noise that's just updating the timestamp.

[graingert]

The timestamp is the OS release date not the current time

[rpkilby]

The timestamp is the OS release date not the current time

Agh, did not catch that. 🤦‍♂

Still, the different environment factors between users will cause header churn.

[graingert]

But those are the environment factors that packages can use to change their requirements

…

On Tue, 28 May 2019, 21:21 Ryan P Kilby, ***@***.***> wrote: The timestamp is the OS release date not the current time Agh, did not catch that. 🤦‍♂ Still, the different environment factors between users will cause header churn. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#828?email_source=notifications&email_token=AADFATEXBAUET4HW6SX5S3DPXWH6JA5CNFSM4HQBJHPKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWNKL6A#issuecomment-496674296>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADFATBQ2KVTDUP4AOMVVM3PXWH6JANCNFSM4HQBJHPA> .

[rpkilby]

But those are the environment factors that packages can use to change their requirements

I'm not really following. Are you saying that a package would parse the header comment of a requirements.txt file to then change its requirements? This sounds like a feature that supports an atypical use case that in turn harms more general usage. Maybe you could write this information to a separate file? e.g., instead of parsing the header comment, you do:

$ pip-compile reqs.in -o requirements.some-marker.txt
$ python -c "import sys; print(sys.version)" > environment.some-marker.txt

Then parse the corresponding environment info file.

At the very least, I'd argue this feature should be made available via a CLI argument.

[graingert]

No

[graingert]

I'm trying to find a way to rephrase, maybe it would be better to see an example? https://www.python.org/dev/peps/pep-0508/#examples

[vphilippon]

I see what @graingert is trying to achieve here, and I agree with the intention.
When running a pip-compile, the output is only guaranteed to be valid on the current "Python environment" (the combination of OS/Python version and other environment markers that have an effect on the dependency selection), as I tried to cover in #651.

As an example, if you do a pip-compile on Windows, you might pick up Windows-only packages (ex: pywin32), and if you want to use the resulting requirements.txt on linux, it won't work.
Same goes for Py2/Py3 specific packages (ex: futures), which might be a more common case.

So the intention of adding in the PEP508 markers, or whatever information that indicates on which "Python Environment" the requirements.txt was generated, and is guaranteed to be valid, is a great one. It would help with issue investigation and could also raise user awareness on the fact that you shouldn't assume you requirements.txt to be compatible cross-environment by default.

But @atugushev, @rpkilby and @ulgens all raised valid concerns about this. It's true that for a lot of cases, the requirements.txt is compatible for multiple all environments.

Should we go for an option to include/exclude that information from the header? Does someone have an idea to expose the cross-environment risk to users, while not imposing overhead for the majority that isn't at risk?

[georgek]

Another example happens when modules end up in the standard library. It happened to me with the typing module which was selectively required by some package for Py<3.5. It was especially confusing because it manifested in some strange pip error due to it not wanting to silently pull in packages when the hashes option is used.

But I'm not sure it's really a problem. We generate a requirements.txt and then that's used to make an environment under the specific version of Python we use to deploy our code. If that environment can't be created because someone updated requirements.txt from the wrong Python version then our testing would catch that very quickly. Unless I'm missing something this seems like a workflow and testing problem not specific to pip-tools.

[ssbarnea]

@atugushev Please review again and request changes if you really think is bad. IMHO, the change is benefic as it only produces a more detailed comment and if python version changes it is a good enough version to gather attention of user.

IMHO, updating dependencies should always be done with a controlled version of python, so this help doing that in a consistent way, while w/o risk of breaking.

If nobody complains I will look into merging this after few days.

[graingert]

It might be better to make a requirements file that's compatible with all python versions and all platforms using conditional deps, like poetry does

[ulgens]

It might be better to make a requirements file that's compatible with all python versions and all platforms using conditional deps, like poetry does

Thank you, that is what I was saying 2 years ago 🙂

[ssbarnea]

I have nothing against a multi-python version lock file being produced but I am also realistic. I do find a small improvement merged today far more useful than an ideal one that never ends up being merged. That mentioned feature is clearly not easy to achieve and if we looks at how long even simple PRs progress,...

As you probably seen, I decided to sacrifice a little bit of my weekend to help with open PRs. I am happy to see others replying and I hope that this may motivate them to resume helping that awesome project.

[codecov]

Codecov Report

Merging #828 (57f6c43) into master (32a8834) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 57f6c43 differs from pull request most recent head cbee456. Consider uploading reports for the commit cbee456 to get more accurate results

@@           Coverage Diff           @@
##           master     #828   +/-   ##
=======================================
  Coverage   99.67%   99.67%           
=======================================
  Files          34       34           
  Lines        3042     3044    +2     
  Branches      327      327           
=======================================
+ Hits         3032     3034    +2     
  Misses          5        5           
  Partials        5        5           

Impacted Files	Coverage Δ
tests/test_cli_compile.py	100.00% <ø> (ø)
piptools/writer.py	100.00% <100.00%> (ø)
tests/test_writer.py	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 32a8834...cbee456. Read the comment docs.

[atugushev]

Hey @ssbarnea!

Thanks for pinging. I'm still not fan of the idea, see my first comment #828 (review). But I intentionally didn't request changes so that this could be merged if people do like the feature.

[ssbarnea]

@atugushev as it did get lots of supporters I merged it. In its current implementation it does only log major python version so the worry about variance in outcome is much lower. Based on my experience with multiple versions of python, it is really bad idea to compile requirements using different versions of python. I had to lock python version when doing this in multiple projects just to avoid the issue you mentioned.

It it proves to be problematic we can tune it to make it optional.