Avoid using LegacyVersion from packaging

[yan12125]

Summary of changes

Closes #2466
Closes pypa/packaging#368

Pull Request Checklist

• Changes have tests
• News fragment added in changelog.d/.
  (See documentation for details)

[yan12125]

So, test_test_command_install_requirements does not work with pip 20.3. I suspect it a pip regression. Should I wait until the pip issue is resolved or it's fine to add an workaround here first and try to solve the pip issue later? - issue fixed, see below.

[di]

So, test_test_command_install_requirements does not work with pip 20.3. I suspect it a pip regression. Should I wait until the pip issue is resolved or it's fine to add an workaround here first and try to solve the pip issue later?

What exactly is the regression? That pip won't accept the dependency links the test was originally creating?

[yan12125]

What exactly is the regression? That pip won't accept the dependency links the test was originally creating?

pip no longer accepts local paths (e.g., /tmp/foo/bar-1.0.tar.gz) as dependency links. Fortunately I managed to make tests pass by turning local paths into URLs (e.g., file:///tmp/foo/bar-1.0.tar.gz) in the second commit 12fe738, so the test suite can work with pip 20.3+ now.

[yan12125]

Thanks for the review. I added news fragments for fixes in this PR.

[jaraco]

Thanks for the contrib. I'm sorry this code (the source, not the patch) is so bad. Your patch is looking good, although I think there are some cases not considered here and not covered by the tests. Can you add to the tests (possibly in the doctests) the cases that are being affected by this change, namely,

• a name that was previously supported with a legacy version in the name (e.g. 1.3_04)
• two names, one of which is legacy and the other which is pep 440
• two legacy versions

I suspect there's another bug lingering here, revealed in the bug report, where it seems that thus function is expecting a single filename, but is getting full path names. With the lenient LegacyVersion, that seemed to work adequately, but now that this code is more strict, it's not clear what should happen. It might be worth refining that expectation as well (disallow path names or document and test the expected behavior).

[jaraco]

I suspect parse is used here because it's important to support non-conformant versions (e.g. 1.3_04). I don't know if there are any systems still relying on these legacy versions, but I would not be surprised if there are. At the very least, I'd consider this a 'change' if not 'breaking'.

[yan12125]

Thanks this is indeed a breaking change. I changed the category in the latest version of the patch.

[jaraco]

This now looks to me like we're getting dangerously close to re-implementing LegacyVersion as _NonVersion. It seems to me if packaging is removing support for the Legacy Version, then probably Setuptools should do so too. Here's what I'd like to happen:

• in the Packaging project (or possibly on another forum like packaging-problems or distutils-sig), discuss the deprecation and removal of LegacyVersion and what they intend to happen in cases where packages still use those versions.
• Follow the spirit of that change. For example, if they're just testing the waters to see who complains about the deprecation, we should complain and explain this use case that's broken by the change. If they have a plan to wean users off of LegacyVersions by dropping support for them, then we should honor that intent and wean our users off of them as well.

I don't want setuptools to be the dumping ground for legacy behavior. If users want that behavior but the PyPA wishes to dissuade that behavior, Setuptools would like to honor that intent.

[jaraco]

I left pypa/packaging#368 (comment) to inquire about the intentions for packaging in the deprecation.

[yan12125]

Thanks very much for explanations. Sounds like more discussions are needed before doing code changes. I'll mark this PR as draft.

[jaraco]

I didn't want to wait on this issue to resolve #2485, so I cherry-picked your commit onto main (thanks for keeping them separate) and force-pushed this branch without that commit.

[yan12125]

I didn't want to wait on this issue to resolve #2485, so I cherry-picked your commit onto main (thanks for keeping them separate) and force-pushed this branch without that commit.

Thanks for cherry-picking. Tests are red again - not fully compatible with pytest 6.2+ this time. I will create another PR for that.

UPDATE: created the fix for pytest 6.2 at #2487

[worstpractice]

@yan12125 Great work!

Any blockers remaining for this PR?

[yan12125]

Thanks! However, I consider this pull request abandoned, as my approach will likely introduce more issues in the long term. I will close this PR in the hopes of avoiding further confusion. Please follow #2497 instead.