pythondistdeps: "environment markers" (PEP-508) support is broken

[ignatenkobrain]

Right now people can use something like argparse; python_version=='2.6'. We need somehow to parse that.

https://www.python.org/dev/peps/pep-0508/#environment-markers

[ignatenkobrain]

cc @ncoghlan

[Conan-Kudo]

Wouldn't this be evaluated at build-time and not included in the installed information?

[ignatenkobrain]

Hmm, I'm actually not sure..

[Conan-Kudo]

Keep in mind that the distdeps generator only deals with the installed resultant egg/wheel data. All the stuff in requirements.txt and setup.py is not parsed at all by the dependency generator.

[ncoghlan]

Hmm, that's actually a good question - I'm not entirely sure what pip/setuptools do with the "Requires-Dist" in METADATA where environment markers are concerned.

However, my expectation would be that they just copy them through verbatim, environment markers and all, rather than only copying through the ones that definitely apply.

The motivation for that behaviour would be wheel files that are compatible with multiple versions: since the wheel file may be cross-version and/or cross-platform, it needs to express environment dependent dependencies. That METADATA file would then be copied verbatim into the installation directory.

[Conan-Kudo]

@ncoghlan It's important to note we don't read the data directly. We get the data through the pkg_resources module, requesting the requirements through the Python runtime. If that works like I think it does, it should implicitly filter out things based on the environment markers before the dependency generator even gets it.

[ignatenkobrain]

@Conan-Kudo @ncoghlan

⋊> ~/P/f/r/p/hypothesis-python-3.44.24 on master ⨯ python2 /usr/lib/rpm/pythondistdeps.py --requires /usr/lib/python2.7/site-packages/hypothesis-3.44.24-py2.7.egg-info/                                   21:41:49
python(abi) == 2.7
python2.7dist(attrs) >= 16.0.0
python2.7dist(coverage)
python2.7dist(enum34)
⋊> ~/P/f/r/p/hypothesis-python-3.44.24 on master ⨯ /usr/lib/rpm/pythondistdeps.py --requires /usr/lib/python2.7/site-packages/hypothesis-3.44.24-py2.7.egg-info/                                           21:41:53
python(abi) == 2.7
python2.7dist(attrs) >= 16.0.0
python2.7dist(coverage)

So pkg_resources depend on interpreter it's being invoked with =(

[ignatenkobrain]

@ncoghlan do you think it would be possible to "fake" python version so pkg_resources would give correct result?

[ignatenkobrain]

It actually seems like setuptools bug, because Distribution knows real python version, but requires() ignore that.

[ignatenkobrain]

Not sure if upstream will accept PR: pypa/setuptools#1275

If they won't, we should be able to trivially mock platform.python_version() for particular call.

[ncoghlan]

@ignatenkobrain Attempting to fake the state that environment markers check (and doing so incompletely) is currently causing pain in pipenv: pypa/pipenv#857

But if you can get pkg_resources to support this natively, then I think that would be the ideal outcome (and based on reading the linked issue, Jason seems amenable to that approach)

[decathorpe]

This is now also breaking some things in rawhide / python 3.8, because the dependency importlib_metadata; python_version < "3.8" generates a dependency on python3dist(importlib-metadata) even with python 3.8.

[Conan-Kudo]

@decathorpe Environment markers are supposed to be processed at build time and filtered out. Where are you seeing this?

[decathorpe]

@Conan-Kudo

https://src.fedoraproject.org/rpms/python-keyring

The automatically generated Requires include python3.8dist(importlib-metadata) on rawhide, with python 3.8, but the dependency is specified as importlib_metadata; python_version < "3.8".

I assume this is a problem with the interaction with setuptools-scm, since this works when using plain setup.py, but not here with setup.py + setup.cfg + setuptools-scm.

[decathorpe]

@Conan-Kudo Ugh ... nevermind. It's a packaging error with manually specified Requires 🤦‍♂️ I didn't even look there because the automatic dependency generator has been active for so long ... Sorry for the noise.

[hroncok]

We do.

[hroncok]

FTR I believe this is done here:

python-rpm-packaging/scripts/pythondistdeps.py

Lines 118 to 119 in a18ca48

	if req.marker.evaluate(get_marker_env(self, extra)):
	extra_deps.append(req)