Use packaging.version to check version equality

[uranusjr]

Fix #9083. I’m 99.999% sure the fix is correct, but didn’t include a test since it’s late here and I have to get up early tomorrow. I’ll leave this to someone with more time 🙂

[pradyunsg]

This looks good, just that it needs tests while we still have this fresh in our minds. :)

[pfmoore]

Agreed this should have a test, but otherwise looks fine. I don't have time to write a test just now, but is it as simple as trying to install a package with version 1.0+local-1?

[pradyunsg]

So... Looks like I'll be writing the test. :)

[pfmoore]

... or I could get to it this evening... 🙂

[pradyunsg]

ACK. I'm happy to defer to you. :)

[pfmoore]

I tried this test:

def test_new_resolver_can_install_silly_version(script):
    create_basic_wheel_for_package(
        script,
        "simple",
        "0.1.0+local-1",
    )
    script.pip(
        "install",
        "--no-cache-dir", "--no-index",
        "--find-links", script.scratch_path,
        "simple"
    )
    assert_installed(script, simple="0.1.0+local-1")

but it failed with "ERROR: Requested simple from file:///C:/Users/.../test_new_resolver_can_install_0/workspace/scratch/simple-0.1.0%2Blocal-1-py2.py3-none-any.whl has different version in metadata: '0.1.0+local.1'"

which seems to be getting raised from _check_metadata_consistency. So I think there's more to do here 🙁

[pfmoore]

(To be clear, I don't intend to look at the _check_metadata_consistency issue, as that's a question of the functionality, not writing a test). @uranusjr will you be able to look at this?

[pradyunsg]

Gotta love when assertions break because people are doing weird things. :)

[pfmoore]

Have I mentioned I hate normalisation? 😉

[uranusjr]

is it as simple as trying to install a package with version 1.0+local-1?

Yes, but vague spots in version normalisation means we probably want to test 1.0+local_1, 1.0+local-1, and 1.0+local.1. They currently can be normalised into either 1.0+local.1 or 1.0+local_1 in the wheel’s filename, so there are 6 variants.

but it failed with "ERROR: Requested simple from file:///C:/Users/.../test_new_resolver_can_install_0/workspace/scratch/simple-0.1.0%2Blocal-1-py2.py3-none-any.whl has different version in metadata: '0.1.0+local.1'"

create_basic_wheel_for_package() is missing edge cases in the implementation. simple-0.1.0+local-1-py2.py3-none-any.whl is not a valid wheel name (the - after local must be replaced with _).

[uranusjr]

LOL it turns out pkg_resources (and therefore pip) can’t even handle 0.1.0+local-1 correctly.

import importlib.metadata
import subprocess
import sys
import pkg_resources
from tests.lib.wheel import make_wheel

# Metadata uses dash, filename uses dot.
whl = 'simple-0.1.0+local.1-py3-none-any.whl'
make_wheel('simple', '0.1.0+local-1').save_to(whl)

# Install it.
subprocess.check_call([sys.executable, '-m', 'pip', 'install', '-q', f'./{whl}'])

# Prints '0.1.0+local-1' correctly.
print(importlib.metadata.version('simple'))

# Prints '0.1.0+local'... Huh?
print(pkg_resources.working_set.by_key['simple'].version)

When can we switch to importlib.metadata?

[uranusjr]

Test added. I adapted Paul’s implementation to use tests.lib.wheel.make_wheel() for finer control of version string in METADATA and the wheel’s file name.

[pradyunsg]

When can we switch to importlib.metadata?

ASAP. Let's get 20.3 out the door and yank out entire bits of pip's codebase. :)