Version._parse_version_str(version) fails if version is a packaging.version.LegacyVersion

[dennisvang]

Description

PyUpdater 4.0 fails due to dsdev-utils.helpers.Version._parse_version_str() with an AttributeError: 'LegacyVersion' object has no attribute 'major' if version string is not PEP440 compliant.

Cause

This line assumes version is a packaging.version.Version, so packaging.version.LegacyVersion is not handled.

The problem is introduced in pr #8 (issue #7).

How to reproduce

Using dsdev-utils 1.3.0 on python 3.9.7:

from dsdev_utils.helpers import Version
version_str = '1.18.0-64-g987cb3c'  # e.g. git describe output
Version(version_str)._parse_version_str(version_str)

Notes

The LegacyVersion is deprecated, although it is still used by packaging.version.parse().

I think it would be nice to handle e.g. output of git describe gracefully, if it starts with a valid PEP440 version, such as '1.18.0-64-g987cb3c'. Could treat this as a dev release with local identifier, for example doing something along the lines of:

version_str = '1.18.0-64-g987cb3c'
version_str.replace('-', '.dev0+', 1)

On the other hand, perhaps it makes more sense just to require PEP440 compliance and let the user handle this...

Anyway, even if PEP440 compliance is required, I think the non-compliant (LegacyVersion) case should still be handled gracefully by _parse_version_str().

[JMSwag]

I do not have the bandwidth to take care of this right now. A PR would be greatly appreciated!

[dennisvang]

@JMSwag I would have some time to create a PR for a quick fix, but I'm not sure what would be best.

Some options:

1. just raise an exception with a more descriptive error message, e.g. "Version string must be PEP440 compliant."
2. return some kind of Version-based null-object instead of a LegacyVersion

I guess anything beyond that, such as the proposal in issue290, would become a lot more invasive.

[JMSwag]

@dennisvang I like option 1.

[dennisvang]

@JMSwag After some more digging I'm not sure this is the way to go.

For example, pyupdater's Package expects dsdev-utils.helpers.Version to be able to handle package file names, such as "my-app-1.2.3.zip". See Digital-Sapphire/PyUpdater#299.

This ability was removed here: e964aa2

If you drop support for non-pep440-compliant version strings, as is currently the case (though inadvertently), dsdev-utils.helpers.Version is not much more than a thin wrapper around packaging.version.Version.

In that case, is there a compelling reason to keep dsdev-utils.helpers.Version around at all?

[dennisvang]

The current implementation of Version also fails to parse pyupdater's "internal" versions (e.g. 4.4.2.0.5) properly, because packaging.version.parse does not recognize the numerical channel (0 in the example) as a pre or post release.

This breaks e.g. pyupdater.client.updates.get_highest_version, as can be seen in pyupdater's TestChannelStrict failing.

[dennisvang]

The following fails for pre-release version strings, although it passes for basic stable version strings.

version_string = "1.0a"
v = Version(version_string)
assert v == Version(str(v))

Also see Digital-Sapphire/PyUpdater#315 (comment)