format_requirement is lowercasing markers, which violates PEP 508

[jfly]

I discovered this issue while using pipenv. See pypa/pipenv#2113. I believe this bug was introduced in
#452 as a band-aid fix to #431.

I've never actually used pip-tools myself, so I'm not sure how to reproduce this issue with pip-tools, but it should be pretty clear from the pipenv bug report what's going on here.

I'm working on a fix for this over in pypa/pipenv#2123, but I thought it would be useful to report this upstream ASAP, in case this is already a known issue, or you have a better idea for a fix.

[vphilippon]

Hi @jfly, thanks for taking the time to report here too!

I'm pretty sure you've put the finger right on it. We'll have to improve that lowercasing logic a bit.

[AndydeCleyre]

I can work on this if someone can provide a reproduction process.

[jfly]

@AndydeCleyre I still have never actually used pip-tools directly, but you can see a repro in the test I added to pipenv over here: https://github.com/pypa/pipenv/pull/2123/files#diff-ce6487d3c4a96b746d4e7c37ce59de79cd9f46b7aa0d2e9e26443ee135b517e1. Is that enough to go off of?

[AndydeCleyre]

Thanks, but I'm not sure yet. pip-tools doesn't currently operate on Pipfiles, so I need to test with a requirements.in file (same syntax as requirements.txt).

With this requirements.in:

pytz ; platform_python_implementation == 'CPython'

And the following command:

$ pip-compile requirements.in

The output looks good:

pytz==2021.1 ; platform_python_implementation == 'CPython'

If I use the wrong case in the input file (platform_python_implementation == 'cpython'), the marker doesn't match the environment, and the output is blank.

Is that the case we're talking about here? If so, what's the proper thing to do when a user uses the wrong case for the marker value in the input file? Should we try to catch common mistakes?

[AndydeCleyre]

I don't see that this is an issue, at least any longer. If anyone can demonstrate it (provide a reproduction), please do, otherwise I'll close this in a few weeks.

[jfly]

@AndydeCleyre I just spent a little while reading through pip-tools source code, and I see the same code that pipenv copied here:

pip-tools/piptools/utils.py

Line 121 in afc255b

line = str(ireq.req).lower()

Note how this code lowercases the entire requirement, which was a problem over in pipenv because it could include markers that shouldn't be lowercased. However, over in pip-tools, it looks like markers get passed around separately and are tacked onto the formatted requirements string after the lowercasing.

I was unable to find a codepath in pip-tools that led to this format_requirement function getting called with an InstallRequirement with a non-null markers attribute. Maybe there is a path, but I just don't know pip-tools well enough to find it if it does!

I think it's ok to close this out.