Skip to content

Commit 2c7cfaa

Browse files
[sonic-package-manager] switch from poetry-semver to semantic_version due to bugs found in poetry-semver (sonic-net#1710)
#### What I did I replaced underneeth a library which I used for semver functionality. #### How I did it Tried to keep existing Version/VersionConstraint API but replaced library used underneeth. I also added two UT that failed with poetry-semver and show the motivation for this change. UT description: - ```test_invalid_version``` This test is to verify that **invalid** semantic version strings are rejected with exception by ```Version.parse```. E.g. "1.2.3-0123" is not a valid semantic version due to this (https://semver.org/#spec-item-9): ``` A pre-release version MAY be denoted by appending a hyphen and a series of dot separated identifiers immediately following the patch version. Identifiers MUST comprise only ASCII alphanumerics and hyphens [0-9A-Za-z-]. Identifiers MUST NOT be empty. Numeric identifiers MUST NOT include leading zeroes ``` - ```test_version_comparison``` This test checks whether the implementation correctly compare two version strings (this is needed due to the logic that checks if we are upgrading or downgrading). According to https://semver.org/#spec-item-11, "1.0.0-alpha" < "1.0.0". #### How to verify it Run UT, run some basic sonic-package-manager commands.
1 parent 7821a3f commit 2c7cfaa

File tree

9 files changed

+162
-39
lines changed

9 files changed

+162
-39
lines changed

setup.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@
181181
'netaddr>=0.8.0',
182182
'netifaces>=0.10.7',
183183
'pexpect>=4.8.0',
184-
'poetry-semver>=0.1.0',
184+
'semantic-version>=2.8.5',
185185
'prettyprinter>=0.18.0',
186186
'pyroute2>=0.5.14, <0.6.1',
187187
'requests>=2.25.0',

sonic_package_manager/constraint.py

+56-7
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,26 @@
33
""" Package version constraints module. """
44

55
import re
6-
from abc import ABC
76
from dataclasses import dataclass, field
87
from typing import Dict, Union
98

10-
import semver
9+
import semantic_version
1110

11+
from sonic_package_manager.version import Version
1212

13-
class VersionConstraint(semver.VersionConstraint, ABC):
14-
""" Extends VersionConstraint from semver package. """
1513

16-
@staticmethod
17-
def parse(constraint_expression: str) -> 'VersionConstraint':
14+
class VersionConstraint:
15+
""" Version constraint representation. """
16+
17+
def __init__(self, *args, **kwargs):
18+
self._constraint = semantic_version.SimpleSpec(*args, **kwargs)
19+
20+
@property
21+
def expression(self):
22+
return self._constraint.expression
23+
24+
@classmethod
25+
def parse(cls, constraint_expression: str) -> 'VersionConstraint':
1826
""" Parse version constraint.
1927
2028
Args:
@@ -23,7 +31,48 @@ def parse(constraint_expression: str) -> 'VersionConstraint':
2331
The resulting VersionConstraint object.
2432
"""
2533

26-
return semver.parse_constraint(constraint_expression)
34+
return cls(constraint_expression)
35+
36+
def allows(self, version: Version) -> bool:
37+
""" Checks if other version is allowed by this constraint
38+
39+
Args:
40+
version: Version to check against this constraint.
41+
Returns:
42+
Boolean wether this constraint allows version.
43+
"""
44+
45+
return self._constraint.match(version._version)
46+
47+
def is_exact(self) -> bool:
48+
""" Is the version constraint exact, meaning only one version is allowed.
49+
50+
Returns:
51+
Boolean wether this constraint is exact.
52+
"""
53+
54+
clause = self._constraint.clause
55+
return hasattr(clause, 'target') and clause.operator == '=='
56+
57+
def get_exact_version(self) -> Version:
58+
""" Returns an exact version for this constraint if it is exact constraint.
59+
60+
Returns:
61+
Exact version in case this constraint is exact.
62+
Raises:
63+
AttributeError: when constraint is not exact
64+
"""
65+
66+
return self._constraint.clause.target
67+
68+
def __str__(self):
69+
return self._constraint.__str__()
70+
71+
def __repr__(self):
72+
return self._constraint.__repr__()
73+
74+
def __eq__(self, other):
75+
return self._constraint.__eq__(other._constraint)
2776

2877

2978
@dataclass

sonic_package_manager/manager.py

+9-9
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@
5555
from sonic_package_manager.utils import DockerReference
5656
from sonic_package_manager.version import (
5757
Version,
58-
VersionRange,
5958
version_to_tag,
6059
tag_to_version
6160
)
@@ -122,13 +121,14 @@ def package_constraint_to_reference(constraint: PackageConstraint) -> PackageRef
122121
# Allow only specific version for now.
123122
# Later we can improve package manager to support
124123
# installing packages using expressions like 'package>1.0.0'
125-
if version_constraint == VersionRange(): # empty range means any version
124+
if version_constraint.expression == '*':
126125
return PackageReference(package_name, None)
127-
if not isinstance(version_constraint, Version):
126+
if not version_constraint.is_exact():
128127
raise PackageManagerError(f'Can only install specific version. '
129128
f'Use only following expression "{package_name}=<version>" '
130129
f'to install specific version')
131-
return PackageReference(package_name, version_to_tag(version_constraint))
130+
version = version_constraint.get_exact_version()
131+
return PackageReference(package_name, version_to_tag(version))
132132

133133

134134
def parse_reference_expression(expression):
@@ -156,7 +156,7 @@ def validate_package_base_os_constraints(package: Package, sonic_version_info: D
156156

157157
version = Version.parse(sonic_version_info[component])
158158

159-
if not constraint.allows_all(version):
159+
if not constraint.allows(version):
160160
raise PackageSonicRequirementError(package.name, component, constraint, version)
161161

162162

@@ -178,7 +178,7 @@ def validate_package_tree(packages: Dict[str, Package]):
178178

179179
installed_version = dependency_package.version
180180
log.debug(f'dependency package is installed {dependency.name}: {installed_version}')
181-
if not dependency.constraint.allows_all(installed_version):
181+
if not dependency.constraint.allows(installed_version):
182182
raise PackageDependencyError(package.name, dependency, installed_version)
183183

184184
dependency_components = dependency.components
@@ -197,7 +197,7 @@ def validate_package_tree(packages: Dict[str, Package]):
197197
log.debug(f'dependency package {dependency.name}: '
198198
f'component {component} version is {component_version}')
199199

200-
if not constraint.allows_all(component_version):
200+
if not constraint.allows(component_version):
201201
raise PackageComponentDependencyError(package.name, dependency, component,
202202
constraint, component_version)
203203

@@ -209,7 +209,7 @@ def validate_package_tree(packages: Dict[str, Package]):
209209

210210
installed_version = conflicting_package.version
211211
log.debug(f'conflicting package is installed {conflict.name}: {installed_version}')
212-
if conflict.constraint.allows_all(installed_version):
212+
if conflict.constraint.allows(installed_version):
213213
raise PackageConflictError(package.name, conflict, installed_version)
214214

215215
for component, constraint in conflicting_package.components.items():
@@ -220,7 +220,7 @@ def validate_package_tree(packages: Dict[str, Package]):
220220
log.debug(f'conflicting package {dependency.name}: '
221221
f'component {component} version is {component_version}')
222222

223-
if constraint.allows_all(component_version):
223+
if constraint.allows(component_version):
224224
raise PackageComponentConflictError(package.name, dependency, component,
225225
constraint, component_version)
226226

sonic_package_manager/manifest.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ def unmarshal(self, value):
151151

152152
# TODO: add description for each field
153153
SCHEMA = ManifestRoot('root', [
154-
ManifestField('version', ParsedMarshaller(Version), Version(1, 0, 0)),
154+
ManifestField('version', ParsedMarshaller(Version), Version.parse('1.0.0')),
155155
ManifestRoot('package', [
156156
ManifestField('version', ParsedMarshaller(Version)),
157157
ManifestField('name', DefaultMarshaller(str)),

sonic_package_manager/version.py

+63-3
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,70 @@
22

33
""" Version and helpers routines. """
44

5-
import semver
5+
import semantic_version
66

7-
Version = semver.Version
8-
VersionRange = semver.VersionRange
7+
8+
class Version:
9+
""" Version class represents SemVer 2.0 compliant version """
10+
11+
@classmethod
12+
def parse(cls, version_string: str) -> 'Version':
13+
""" Construct Version from version_string.
14+
15+
Args:
16+
version_string: SemVer compatible version string.
17+
Returns:
18+
Version object.
19+
Raises:
20+
ValueError: when version_string does not follow SemVer.
21+
"""
22+
23+
return cls(version_string)
24+
25+
def __init__(self, *args, **kwargs):
26+
self._version = semantic_version.Version(*args, **kwargs)
27+
28+
@property
29+
def major(self):
30+
return self._version.major
31+
32+
@property
33+
def minor(self):
34+
return self._version.minor
35+
36+
@property
37+
def patch(self):
38+
return self._version.patch
39+
40+
def __str__(self):
41+
return self._version.__str__()
42+
43+
def __repr__(self):
44+
return self._version.__repr__()
45+
46+
def __iter__(self):
47+
return self._version.__iter__()
48+
49+
def __cmp__(self, other):
50+
return self._version.__cmp__(other._version)
51+
52+
def __eq__(self, other):
53+
return self._version.__eq__(other._version)
54+
55+
def __ne__(self, other):
56+
return self._version.__ne__(other._version)
57+
58+
def __lt__(self, other):
59+
return self._version.__lt__(other._version)
60+
61+
def __le__(self, other):
62+
return self._version.__le__(other._version)
63+
64+
def __gt__(self, other):
65+
return self._version.__gt__(other._version)
66+
67+
def __ge__(self, other):
68+
return self._version.__ge__(other._version)
969

1070

1171
def version_to_tag(ver: Version) -> str:

tests/sonic_package_manager/test_constraint.py

+18-4
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,22 @@
11
#!/usr/bin/env python
22

3+
import pytest
4+
35
from sonic_package_manager import version
4-
from sonic_package_manager.constraint import PackageConstraint
5-
from sonic_package_manager.version import Version, VersionRange
6+
from sonic_package_manager.constraint import PackageConstraint, VersionConstraint
7+
from sonic_package_manager.version import Version
8+
9+
10+
@pytest.mark.parametrize('invalid_version', ['1.2.3-0123', '1.2', '1.0.0+artiary+version'])
11+
def test_invalid_version(invalid_version):
12+
with pytest.raises(Exception):
13+
Version.parse(invalid_version)
14+
15+
16+
@pytest.mark.parametrize(('newer', 'older'),
17+
[('0.1.1', '0.1.1-alpha')])
18+
def test_version_comparison(newer, older):
19+
assert Version.parse(newer) > Version.parse(older)
620

721

822
def test_constraint():
@@ -28,7 +42,7 @@ def test_constraint_strict():
2842

2943

3044
def test_constraint_match():
31-
package_constraint = PackageConstraint.parse('swss==1.2*.*')
45+
package_constraint = PackageConstraint.parse('swss==1.2.*')
3246
assert package_constraint.name == 'swss'
3347
assert not package_constraint.constraint.allows(Version.parse('1.1.1'))
3448
assert package_constraint.constraint.allows(Version.parse('1.2.0'))
@@ -47,7 +61,7 @@ def test_constraint_multiple():
4761
def test_constraint_only_name():
4862
package_constraint = PackageConstraint.parse('swss')
4963
assert package_constraint.name == 'swss'
50-
assert package_constraint.constraint == VersionRange()
64+
assert package_constraint.constraint == VersionConstraint('*')
5165

5266

5367
def test_constraint_from_dict():

tests/sonic_package_manager/test_database.py

+3-3
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ def test_database_get_package(fake_db):
1717
assert swss_package.built_in
1818
assert swss_package.repository == 'docker-orchagent'
1919
assert swss_package.default_reference == '1.0.0'
20-
assert swss_package.version == Version(1, 0, 0)
20+
assert swss_package.version == Version.parse('1.0.0')
2121

2222

2323
def test_database_get_package_not_builtin(fake_db):
@@ -52,11 +52,11 @@ def test_database_add_package_existing(fake_db):
5252
def test_database_update_package(fake_db):
5353
test_package = fake_db.get_package('test-package-2')
5454
test_package.installed = True
55-
test_package.version = Version(1, 2, 3)
55+
test_package.version = Version.parse('1.2.3')
5656
fake_db.update_package(test_package)
5757
test_package = fake_db.get_package('test-package-2')
5858
assert test_package.installed
59-
assert test_package.version == Version(1, 2, 3)
59+
assert test_package.version == Version.parse('1.2.3')
6060

6161

6262
def test_database_update_package_non_existing(fake_db):

tests/sonic_package_manager/test_manager.py

+11-10
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#!/usr/bin/env python
22

3+
import re
34
from unittest.mock import Mock, call
45

56
import pytest
@@ -26,8 +27,8 @@ def test_installation_dependencies(package_manager, fake_metadata_resolver, mock
2627
manifest = fake_metadata_resolver.metadata_store['Azure/docker-test']['1.6.0']['manifest']
2728
manifest['package']['depends'] = ['swss^2.0.0']
2829
with pytest.raises(PackageInstallationError,
29-
match='Package test-package requires swss>=2.0.0,<3.0.0 '
30-
'but version 1.0.0 is installed'):
30+
match=re.escape('Package test-package requires swss^2.0.0 '
31+
'but version 1.0.0 is installed')):
3132
package_manager.install('test-package')
3233

3334

@@ -80,8 +81,8 @@ def test_installation_components_dependencies_not_satisfied(package_manager, fak
8081
},
8182
]
8283
with pytest.raises(PackageInstallationError,
83-
match='Package test-package requires libswsscommon >=1.1.0,<2.0.0 '
84-
'in package swss>=1.0.0 but version 1.0.0 is installed'):
84+
match=re.escape('Package test-package requires libswsscommon ^1.1.0 '
85+
'in package swss>=1.0.0 but version 1.0.0 is installed')):
8586
package_manager.install('test-package')
8687

8788

@@ -98,8 +99,8 @@ def test_installation_components_dependencies_implicit(package_manager, fake_met
9899
},
99100
]
100101
with pytest.raises(PackageInstallationError,
101-
match='Package test-package requires libswsscommon >=2.1.0,<3.0.0 '
102-
'in package swss>=1.0.0 but version 1.0.0 is installed'):
102+
match=re.escape('Package test-package requires libswsscommon ^2.1.0 '
103+
'in package swss>=1.0.0 but version 1.0.0 is installed')):
103104
package_manager.install('test-package')
104105

105106

@@ -125,8 +126,8 @@ def test_installation_breaks(package_manager, fake_metadata_resolver):
125126
manifest = fake_metadata_resolver.metadata_store['Azure/docker-test']['1.6.0']['manifest']
126127
manifest['package']['breaks'] = ['swss^1.0.0']
127128
with pytest.raises(PackageInstallationError,
128-
match='Package test-package conflicts with '
129-
'swss>=1.0.0,<2.0.0 but version 1.0.0 is installed'):
129+
match=re.escape('Package test-package conflicts with '
130+
'swss^1.0.0 but version 1.0.0 is installed')):
130131
package_manager.install('test-package')
131132

132133

@@ -294,7 +295,7 @@ def test_manager_upgrade(package_manager, sonic_fs):
294295

295296
package_manager.install('test-package-6=2.0.0')
296297
upgraded_package = package_manager.get_installed_package('test-package-6')
297-
assert upgraded_package.entry.version == Version(2, 0, 0)
298+
assert upgraded_package.entry.version == Version.parse('2.0.0')
298299
assert upgraded_package.entry.default_reference == package.entry.default_reference
299300

300301

@@ -304,7 +305,7 @@ def test_manager_package_reset(package_manager, sonic_fs):
304305

305306
package_manager.reset('test-package-6')
306307
upgraded_package = package_manager.get_installed_package('test-package-6')
307-
assert upgraded_package.entry.version == Version(1, 5, 0)
308+
assert upgraded_package.entry.version == Version.parse('1.5.0')
308309

309310

310311
def test_manager_migration(package_manager, fake_db_for_migration):

tests/sonic_package_manager/test_manifest.py

-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
from sonic_package_manager.constraint import ComponentConstraints
66
from sonic_package_manager.manifest import Manifest, ManifestError
7-
from sonic_package_manager.version import VersionRange
87

98

109
def test_manifest_v1_defaults():

0 commit comments

Comments
 (0)