Skip to content

Commit 22560d4

Browse files
committed
pyln-client: reimplement NodeVersion, simply.
This broke my build machine, because lightningd --version was malformed (I had no tags somehow in that branch). I dived into the code to figure out what was wrong, and I was horrified. 1. STOP. Never write this much code. 2. You just need a NodeVersion class. That's it. No others. 3. Don't throw away the entire first part if it starts with 'v'. Just remove the v. 4. Always fail on invalid strings in the constructor, NOT on the first time you use it. I have rewritten it. Signed-off-by: Rusty Russell <[email protected]>
1 parent 1e5a577 commit 22560d4

File tree

4 files changed

+47
-260
lines changed

4 files changed

+47
-260
lines changed

Diff for: contrib/pyln-client/pyln/client/__init__.py

+1-2
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
from .plugin import Plugin, monkey_patch, RpcException
33
from .gossmap import Gossmap, GossmapNode, GossmapChannel, GossmapHalfchannel, GossmapNodeId, LnFeatureBits
44
from .gossmapstats import GossmapStats
5-
from .version import NodeVersion, VersionSpec
5+
from .version import NodeVersion
66

77
__version__ = "25.02rc3"
88

@@ -22,5 +22,4 @@
2222
"LnFeatureBits",
2323
"GossmapStats",
2424
"NodeVersion",
25-
"VersionSpec",
2625
]

Diff for: contrib/pyln-client/pyln/client/version.py

+26-181
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,10 @@
11
from __future__ import annotations
22

3-
from dataclasses import dataclass
43
from functools import total_ordering
54
import re
6-
from typing import List, Optional, Protocol, runtime_checkable, Union
7-
8-
9-
_MODDED_PATTERN = "[0-9a-f]+-modded"
10-
5+
from typing import List, Union
116

127
@total_ordering
13-
@dataclass
148
class NodeVersion:
159
"""NodeVersion
1610
@@ -30,195 +24,46 @@ class NodeVersion:
3024
- `v23.11` < `v24.02`
3125
The oldest version is the smallest
3226
"""
33-
34-
version: str
35-
36-
def to_parts(self) -> List[_NodeVersionPart]:
37-
parts = self.version[1:].split(".")
38-
# If the first part contains a v we will ignore it
39-
if not parts[0][0].isdigit():
40-
parts[0] = parts[1:]
41-
42-
return [_NodeVersionPart.parse(p) for p in parts]
43-
44-
def strict_equal(self, other: NodeVersion) -> bool:
45-
if not isinstance(other, NodeVersion):
46-
raise TypeError(
47-
"`other` is expected to be of type `NodeVersion` but is `{type(other)}`"
48-
)
49-
else:
50-
return self.version == other.version
27+
def __init__(self, version: str):
28+
# e.g. v24.11-225-gda793e66b9
29+
if version.startswith('v'):
30+
version = version[1:]
31+
version = version.split('-')[0]
32+
parts = version.split('.')
33+
# rc is considered "close enough"
34+
if 'rc' in parts[-1]:
35+
parts[-1] = parts[-1].split('rc')[0]
36+
37+
self.parts: int = []
38+
for p in parts:
39+
self.parts.append(int(p))
5140

5241
def __eq__(self, other: Union[NodeVersion, str]) -> bool:
5342
if isinstance(other, str):
5443
other = NodeVersion(other)
5544
if not isinstance(other, NodeVersion):
5645
return False
5746

58-
if self.strict_equal(other):
59-
return True
60-
elif re.match(_MODDED_PATTERN, self.version):
47+
if len(self.parts) != len(other.parts):
6148
return False
62-
else:
63-
self_parts = [p.num for p in self.to_parts()]
64-
other_parts = [p.num for p in other.to_parts()]
65-
66-
if len(self_parts) != len(other_parts):
49+
for a, b in zip(self.parts, other.parts):
50+
if a != b:
6751
return False
68-
69-
for ps, po in zip(self_parts, other_parts):
70-
if ps != po:
71-
return False
72-
return True
52+
return True
7353

7454
def __lt__(self, other: Union[NodeVersion, str]) -> bool:
7555
if isinstance(other, str):
7656
other = NodeVersion(other)
7757
if not isinstance(other, NodeVersion):
7858
return NotImplemented
7959

80-
# If we are in CI the version will by a hex ending on modded
81-
# We will assume it is the latest version
82-
if re.match(_MODDED_PATTERN, self.version):
83-
return False
84-
elif re.match(_MODDED_PATTERN, other.version):
85-
return True
86-
else:
87-
self_parts = [p.num for p in self.to_parts()]
88-
other_parts = [p.num for p in other.to_parts()]
89-
90-
# zip truncates to shortes length
91-
for sp, op in zip(self_parts, other_parts):
92-
if sp < op:
93-
return True
94-
if sp > op:
95-
return False
96-
97-
# If the initial parts are all equal the longest version is the biggest
98-
#
99-
# self = 'v24.02'
100-
# other = 'v24.02.1'
101-
return len(self_parts) < len(other_parts)
102-
103-
def matches(self, version_spec: VersionSpecLike) -> bool:
104-
"""Returns True if the version matches the spec
105-
106-
The `version_spec` can be represented as a string and has 8 operators
107-
which are `=`, `===`, `!=`, `!===`, `<`, `<=`, `>`, `>=`.
108-
109-
The `=` is the equality operator. The verson_spec `=v24.02` matches
110-
all versions that equal `v24.02` including release candidates such as `v24.02rc1`.
111-
You can use the strict-equality operator `===` if strict equality is required.
112-
113-
Specifiers can be combined by separating the with a comma ','. The `version_spec`
114-
`>=v23.11, <v24.02" includes any version which is greater than or equal to `v23.11`
115-
and smaller than `v24.02`.
116-
"""
117-
spec = VersionSpec.parse(version_spec)
118-
return spec.matches(self)
119-
120-
121-
@dataclass
122-
class _NodeVersionPart:
123-
num: int
124-
text: Optional[str] = None
125-
126-
@classmethod
127-
def parse(cls, part: str) -> _NodeVersionPart:
128-
# We assume all parts start with a number and are followed by a text
129-
# E.g: v24.01rc2 has two parts
130-
# - "24" -> num = 24, text = None
131-
# - "01rc" -> num = 01, text = "rc"
132-
133-
number = re.search(r"\d+", part).group()
134-
text = part[len(number):]
135-
text_opt = text if text != "" else None
136-
return _NodeVersionPart(int(number), text_opt)
137-
138-
139-
@runtime_checkable
140-
class VersionSpec(Protocol):
141-
def matches(self, other: NodeVersionLike) -> bool:
142-
...
143-
144-
@classmethod
145-
def parse(cls, spec: VersionSpecLike) -> VersionSpec:
146-
if isinstance(spec, VersionSpec):
147-
return spec
148-
else:
149-
parts = [p.strip() for p in spec.split(",")]
150-
subspecs = [_CompareSpec.parse(p) for p in parts]
151-
return _AndVersionSpecifier(subspecs)
152-
153-
154-
@dataclass
155-
class _AndVersionSpecifier(VersionSpec):
156-
specs: List[VersionSpec]
157-
158-
def matches(self, other: NodeVersionLike) -> bool:
159-
for spec in self.specs:
160-
if not spec.matches(other):
60+
# We want a zero-padded zip. Pad both to make one.
61+
totlen = max(len(self.parts), len(other.parts))
62+
for a, b in zip(self.parts + [0] * totlen, other.parts + [0] * totlen):
63+
if a < b:
64+
return True
65+
if a > b:
16166
return False
162-
return True
163-
164-
165-
_OPERATORS = [
166-
"===", # Strictly equal
167-
"!===", # not strictly equal
168-
"=", # Equal
169-
">=", # Greater or equal
170-
"<=", # Less or equal
171-
"<", # less
172-
">", # greater than
173-
"!=", # not equal
174-
]
175-
176-
177-
@dataclass
178-
class _CompareSpec(VersionSpec):
179-
operator: str
180-
version: NodeVersion
181-
182-
def __post_init__(self):
183-
if self.operator not in _OPERATORS:
184-
raise ValueError(f"Invalid operator '{self.operator}'")
185-
186-
def matches(self, other: NodeVersionLike):
187-
if isinstance(other, str):
188-
other = NodeVersion(other)
189-
if self.operator == "===":
190-
return other.strict_equal(self.version)
191-
if self.operator == "!===":
192-
return not other.strict_equal(self.version)
193-
if self.operator == "=":
194-
return other == self.version
195-
if self.operator == ">=":
196-
return other >= self.version
197-
if self.operator == "<=":
198-
return other <= self.version
199-
if self.operator == "<":
200-
return other < self.version
201-
if self.operator == ">":
202-
return other > self.version
203-
if self.operator == "!=":
204-
return other != self.version
205-
else:
206-
ValueError("Unknown operator")
207-
208-
@classmethod
209-
def parse(cls, spec_string: str) -> _CompareSpec:
210-
spec_string = spec_string.strip()
211-
212-
for op in _OPERATORS:
213-
if spec_string.startswith(op):
214-
version = spec_string[len(op):]
215-
version = version.strip()
216-
return _CompareSpec(op, NodeVersion(version))
217-
218-
raise ValueError(f"Failed to parse '{spec_string}'")
219-
220-
221-
NodeVersionLike = Union[NodeVersion, str]
222-
VersionSpecLike = Union[VersionSpec, str]
67+
return False
22368

224-
__all__ = [NodeVersion, NodeVersionLike, VersionSpec, VersionSpecLike]
69+
__all__ = [NodeVersion]

Diff for: contrib/pyln-client/tests/test_version.py

+11-60
Original file line numberDiff line numberDiff line change
@@ -3,90 +3,41 @@
33

44
def test_create_version():
55
# These are the strings returned by `lightningd --version`
6-
_ = NodeVersion("v24.02")
7-
_ = NodeVersion("23.08.1")
8-
9-
10-
def test_parse_parts():
11-
assert _NodeVersionPart.parse("2rc2") == _NodeVersionPart(2, "rc2")
12-
assert _NodeVersionPart.parse("0rc1") == _NodeVersionPart(0, "rc1")
13-
assert _NodeVersionPart.parse("2") == _NodeVersionPart(2, None)
14-
assert _NodeVersionPart.parse("2").text is None
15-
16-
17-
def test_version_to_parts():
18-
19-
assert NodeVersion("v24.02rc1").to_parts() == [
20-
_NodeVersionPart(24),
21-
_NodeVersionPart(2, "rc1"),
22-
]
23-
24-
assert NodeVersion("v24.02.1").to_parts() == [
25-
_NodeVersionPart(24),
26-
_NodeVersionPart(2),
27-
_NodeVersionPart(1),
28-
]
6+
_ = NodeVersion("v24.11-232-g5a76c7a")
7+
_ = NodeVersion("v24.11-225-gda793e6-modded")
8+
_ = NodeVersion("v24.11")
299

3010

3111
def test_equality_classes_in_node_versions():
3212
assert NodeVersion("v24.02") == NodeVersion("v24.02")
33-
assert NodeVersion("v24.02") == NodeVersion("v24.02rc1")
34-
assert NodeVersion("v24.02rc1") == NodeVersion("v24.02")
35-
assert NodeVersion("v24.11-217-g77989b1-modded") == NodeVersion("v24.11")
13+
assert NodeVersion("v24.02") == NodeVersion("24.02")
14+
assert NodeVersion("v24.02-225") == NodeVersion("v24.02")
3615

37-
assert NodeVersion("v24.02") != NodeVersion("v24.02.1")
38-
assert NodeVersion("v24.02rc1") != NodeVersion("v24.02.1")
39-
assert NodeVersion("v23.10") != NodeVersion("v23.02")
16+
assert NodeVersion("v24.02") != NodeVersion("v24.02rc1")
17+
assert NodeVersion("v24.11-217-g77989b1-modded") == NodeVersion("v24.11")
4018

4119

4220
def test_inequality_of_node_versions():
4321
assert not NodeVersion("v24.02.1") > NodeVersion("v24.02.1")
4422
assert NodeVersion("v24.02.1") > NodeVersion("v24.02")
4523
assert NodeVersion("v24.02.1") > NodeVersion("v24.02rc1")
4624
assert NodeVersion("v24.02.1") > NodeVersion("v23.05")
25+
assert NodeVersion("v24.05") > NodeVersion("v24.02")
4726

4827
assert NodeVersion("v24.02.1") >= NodeVersion("v24.02.1")
4928
assert NodeVersion("v24.02.1") >= NodeVersion("v24.02")
5029
assert NodeVersion("v24.02.1") >= NodeVersion("v24.02rc1")
5130
assert NodeVersion("v24.02.1") >= NodeVersion("v23.05")
31+
assert NodeVersion("v24.05") >= NodeVersion("v24.02")
5232

5333
assert NodeVersion("v24.02.1") <= NodeVersion("v24.02.1")
5434
assert not NodeVersion("v24.02.1") <= NodeVersion("v24.02")
5535
assert not NodeVersion("v24.02.1") <= NodeVersion("v24.02rc1")
5636
assert not NodeVersion("v24.02.1") <= NodeVersion("v23.05")
37+
assert not NodeVersion("v24.05") <= NodeVersion("v24.02")
5738

5839
assert not NodeVersion("v24.02.1") < NodeVersion("v24.02.1")
5940
assert not NodeVersion("v24.02.1") < NodeVersion("v24.02")
6041
assert not NodeVersion("v24.02.1") < NodeVersion("v24.02rc1")
6142
assert not NodeVersion("v24.02.1") < NodeVersion("v23.05")
62-
63-
64-
def test_comparision_parse():
65-
assert _CompareSpec.parse("===v24.02").operator == "==="
66-
assert _CompareSpec.parse("=v24.02").operator == "="
67-
assert _CompareSpec.parse("!===v24.02").operator == "!==="
68-
assert _CompareSpec.parse("!=v24.02").operator == "!="
69-
assert _CompareSpec.parse(">v24.02").operator == ">"
70-
assert _CompareSpec.parse("<v24.02").operator == "<"
71-
assert _CompareSpec.parse(">=v24.02").operator == ">="
72-
assert _CompareSpec.parse("<=v24.02").operator == "<="
73-
74-
75-
def test_compare_spec_from_string():
76-
assert VersionSpec.parse("=v24.02").matches("v24.02rc1")
77-
assert VersionSpec.parse("=v24.02").matches("v24.02")
78-
assert not VersionSpec.parse("=v24.02").matches("v24.02.1")
79-
80-
# Yes, I use weird spaces here as a part of the test
81-
list_spec = VersionSpec.parse(">= v24.02, !=== v24.02rc1")
82-
assert list_spec.matches("v24.02")
83-
assert list_spec.matches("v24.02.1")
84-
85-
assert not list_spec.matches("v24.02rc1")
86-
assert not list_spec.matches("v23.11")
87-
88-
89-
def test_ci_modded_version_is_always_latest():
90-
v1 = NodeVersion("1a86e50-modded")
91-
92-
assert v1 > NodeVersion("v24.02")
43+
assert not NodeVersion("v24.05") < NodeVersion("v24.02")

0 commit comments

Comments
 (0)