python3.10: environment markers assume python 3.1

[asottile]

Environment

• pip version: (excuse the buggy output here, that'll get fixed in sys.version fixes for python 3.10 #6729)

  $ pip --version
  pip 19.1.1 from /home/asottile/workspace/virtualenv/pyvenv10/lib/python3.10/site-packages/pip (python 3.1)

• Python version: (yes, it's fake -- more info here)

  $ python --version --version
  Python 3.10.0a0 (default, Jul 18 2019, 06:01:33) 
  [GCC 7.4.0]

• OS:

  $ lsb_release -a
  No LSB modules are available.
  Distributor ID:	Ubuntu
  Description:	Ubuntu 18.04.2 LTS
  Release:	18.04
  Codename:	bionic

Description

environment markers such as python_version < "3.2" are matching while python 3.10 is being used

Expected behavior

I expect them to not match, notably that "3.10" should be considered greater than "3.2"

How to Reproduce

I ran into this while running pip install pre-commit

Output

$ pip install pre-commit
Collecting pre-commit
  Using cached https://files.pythonhosted.org/packages/59/22/13e65062888c1f53619ef557f885817874aabc447e1cbb4cf774ceaf222f/pre_commit-1.17.0-py2.py3-none-any.whl
Collecting aspy.yaml (from pre-commit)
  Using cached https://files.pythonhosted.org/packages/99/ce/78be097b00817ccf02deaf481eb7a603eecee6fa216e82fa7848cd265449/aspy.yaml-1.3.0-py2.py3-none-any.whl
Collecting pyyaml (from pre-commit)
  Using cached https://files.pythonhosted.org/packages/a3/65/837fefac7475963d1eccf4aa684c23b95aa6c1d033a2c5965ccb11e22623/PyYAML-5.1.1.tar.gz
Collecting cfgv>=2.0.0 (from pre-commit)
  Using cached https://files.pythonhosted.org/packages/2f/ec/3c0a56fbc00e6b649c1dc809dc3f12c5796fbfb7940d1167b9bddc67b818/cfgv-2.0.0-py2.py3-none-any.whl
Collecting six (from pre-commit)
  Using cached https://files.pythonhosted.org/packages/73/fb/00a976f728d0d1fecfe898238ce23f502a721c0ac0ecfedb80e0d88c64e9/six-1.12.0-py2.py3-none-any.whl
Collecting toml (from pre-commit)
  Using cached https://files.pythonhosted.org/packages/a2/12/ced7105d2de62fa7c8fb5fce92cc4ce66b57c95fb875e9318dba7f8c5db0/toml-0.10.0-py2.py3-none-any.whl
Collecting importlib-metadata (from pre-commit)
  Using cached https://files.pythonhosted.org/packages/bd/23/dce4879ec58acf3959580bfe769926ed8198727250c5e395e6785c764a02/importlib_metadata-0.18-py2.py3-none-any.whl
Collecting identify>=1.0.0 (from pre-commit)
  Using cached https://files.pythonhosted.org/packages/94/f7/0ad891a459ee50e8774636798ce00e0b445addd8555661da9dbb546676a6/identify-1.4.5-py2.py3-none-any.whl
Collecting futures; python_version < "3.2" (from pre-commit)
  Using cached https://files.pythonhosted.org/packages/cc/26/b61e3a4eb50653e8a7339d84eeaa46d1e93b92951978873c220ae64d0733/futures-3.1.1.tar.gz
Collecting virtualenv>=15.2 (from pre-commit)
  Using cached https://files.pythonhosted.org/packages/eb/9f/33373d471bb9337c8db86b763052964c42f5079ea0de9517bc88acfbad26/virtualenv-16.6.2-py2.py3-none-any.whl
Collecting importlib-resources; python_version < "3.7" (from pre-commit)
  Using cached https://files.pythonhosted.org/packages/2f/f7/b4aa02cdd3ee7ebba375969d77c00826aa15c5db84247d23c89522dccbfa/importlib_resources-1.0.2-py2.py3-none-any.whl
Collecting nodeenv>=0.11.1 (from pre-commit)
  Using cached https://files.pythonhosted.org/packages/00/6e/ed417bd1ed417ab3feada52d0c89ab0ed87d150f91590badf84273e047c9/nodeenv-1.3.3.tar.gz
Collecting zipp>=0.5 (from importlib-metadata->pre-commit)
  Using cached https://files.pythonhosted.org/packages/da/bd/1a5fdf15aa44231fd09f63ecf175b60f057ae37ec65b343bb009364923f3/zipp-0.5.2-py2.py3-none-any.whl
Collecting typing; python_version < "3.5" (from importlib-resources; python_version < "3.7"->pre-commit)
  Using cached https://files.pythonhosted.org/packages/28/b8/a1d6b7cf322f91305bcb5e7d8f6c3028954d1e3e716cddc1cdce2ac63247/typing-3.7.4-py3-none-any.whl
Installing collected packages: pyyaml, aspy.yaml, six, cfgv, toml, zipp, importlib-metadata, identify, futures, virtualenv, typing, importlib-resources, nodeenv, pre-commit
  Running setup.py install for pyyaml ... done
  Running setup.py install for futures ... done
  Running setup.py install for nodeenv ... done
Successfully installed aspy.yaml-1.3.0 cfgv-2.0.0 futures-3.1.1 identify-1.4.5 importlib-metadata-0.18 importlib-resources-1.0.2 nodeenv-1.3.3 pre-commit-1.17.0 pyyaml-5.1.1 six-1.12.0 toml-0.10.0 typing-3.7.4 virtualenv-16.6.2 zipp-0.5.2

The notable interesting lines are any of the ones with python_version as those should have all been skipped

[asottile]

at least from my reading of PEP 508 we shouldn't need to adjust a PEP to fix this:

version_cmp = wsp* '<' | '<=' | '!=' | '==' | '>=' | '>' | '~=' | '==='
...
marker_op = version_cmp | (wsp* 'in') | (wsp* 'not' wsp+ 'in')
...
Comparisons in marker expressions are typed by the comparison operator. The <marker_op> operators that are not in <version_cmp> perform the same as they do for strings in Python. The <version_cmp> operators use the PEP-440 [4] version comparison rules when those are defined (that is when both sides have a valid version specifier). If there is no defined PEP-440 behaviour and the operator exists in Python, then the operator falls back to the Python behaviour. Otherwise an error should be raised. e.g. the following will result in errors:

[uranusjr]

Looks like packaging.markers is the root cause here, but I’m not sure what is the best way to fix it. Right now packaging.markers simply performs string comparison on all markers, so python_version < '3.2' would still be true even if we fix platform.python_version()[:3]. Should we list version markers separately from non-version ones (e.g. os_name) and implement different logic for them? Can we just use packaging.version for that? Does packaging.version work for python_full_version?

/cc @dstufft

[pradyunsg]

Yep yep - we should fix packaging. We can use Version for those checks.

Could you file an issue on pypa/packaging for it?

[uranusjr]

Seems like pypa/packaging#119 already partially covers this.

[pradyunsg]

Hehe. @pganssle has already, helpfully, filed a PR for this on packaging. 🙃

[Zac-HD]

pypa/packaging#165 has since been merged.

[pradyunsg]

Indeed! Thanks @Zac-HD!

@xavfernandez FYI - we'd want to update packaging prior to the 19.3 release.

[pradyunsg]

AFAICT, this should be resolved on master now.

[xavfernandez]

Yup, #7166 should have taken care of it.