Skip to content

Commit 0a8da02

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. Handle untagged versions cleanly. 5. 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 7e831ad commit 0a8da02

File tree

4 files changed

+60
-258
lines changed

4 files changed

+60
-258
lines changed

contrib/pyln-client/pyln/client/__init__.py

Lines changed: 1 addition & 2 deletions
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.02"
88

@@ -22,5 +22,4 @@
2222
"LnFeatureBits",
2323
"GossmapStats",
2424
"NodeVersion",
25-
"VersionSpec",
2625
]
Lines changed: 33 additions & 179 deletions
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
5-
import re
6-
from typing import List, Optional, Protocol, runtime_checkable, Union
7-
8-
9-
_MODDED_PATTERN = "[0-9a-f]+-modded"
4+
from typing import Union
105

116

127
@total_ordering
13-
@dataclass
148
class NodeVersion:
159
"""NodeVersion
1610
@@ -29,196 +23,56 @@ class NodeVersion:
2923
See `strict_equal` if an exact match is required
3024
- `v23.11` < `v24.02`
3125
The oldest version is the smallest
26+
- `vd6fa78c`
27+
This is an untagged version, such as in CI. This is assumed to be the latest, greater than
28+
any test.
3229
"""
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-
)
30+
def __init__(self, version: str):
31+
# e.g. v24.11-225-gda793e66b9
32+
if version.startswith('v'):
33+
version = version[1:]
34+
version = version.split('-')[0]
35+
parts = version.split('.')
36+
# rc is considered "close enough"
37+
if 'rc' in parts[-1]:
38+
parts[-1] = parts[-1].split('rc')[0]
39+
40+
self.parts: int = []
41+
42+
# Single part? It's a git version, so treat it as the future.
43+
if len(parts) == 1:
44+
self.parts.append(100)
4945
else:
50-
return self.version == other.version
46+
for p in parts:
47+
self.parts.append(int(p))
5148

5249
def __eq__(self, other: Union[NodeVersion, str]) -> bool:
5350
if isinstance(other, str):
5451
other = NodeVersion(other)
5552
if not isinstance(other, NodeVersion):
5653
return False
5754

58-
if self.strict_equal(other):
59-
return True
60-
elif re.match(_MODDED_PATTERN, self.version):
55+
if len(self.parts) != len(other.parts):
6156
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):
57+
for a, b in zip(self.parts, other.parts):
58+
if a != b:
6759
return False
68-
69-
for ps, po in zip(self_parts, other_parts):
70-
if ps != po:
71-
return False
72-
return True
60+
return True
7361

7462
def __lt__(self, other: Union[NodeVersion, str]) -> bool:
7563
if isinstance(other, str):
7664
other = NodeVersion(other)
7765
if not isinstance(other, NodeVersion):
7866
return NotImplemented
7967

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):
68+
# We want a zero-padded zip. Pad both to make one.
69+
totlen = max(len(self.parts), len(other.parts))
70+
for a, b in zip(self.parts + [0] * totlen, other.parts + [0] * totlen):
71+
if a < b:
72+
return True
73+
if a > b:
16174
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-
75+
return False
22076

221-
NodeVersionLike = Union[NodeVersion, str]
222-
VersionSpecLike = Union[VersionSpec, str]
22377

224-
__all__ = [NodeVersion, NodeVersionLike, VersionSpec, VersionSpecLike]
78+
__all__ = [NodeVersion]
Lines changed: 17 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -1,92 +1,49 @@
1-
from pyln.client.version import NodeVersion, VersionSpec, _NodeVersionPart, _CompareSpec
1+
from pyln.client.version import NodeVersion
22

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")
9+
_ = NodeVersion("vd6fa78c")
2910

3011

3112
def test_equality_classes_in_node_versions():
3213
assert NodeVersion("v24.02") == NodeVersion("v24.02")
14+
assert NodeVersion("v24.02") == NodeVersion("24.02")
15+
assert NodeVersion("v24.02-225") == NodeVersion("v24.02")
16+
3317
assert NodeVersion("v24.02") == NodeVersion("v24.02rc1")
34-
assert NodeVersion("v24.02rc1") == NodeVersion("v24.02")
3518
assert NodeVersion("v24.11-217-g77989b1-modded") == NodeVersion("v24.11")
36-
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")
19+
assert NodeVersion("vd6fa78c") == NodeVersion("vabcdefg")
4020

4121

4222
def test_inequality_of_node_versions():
4323
assert not NodeVersion("v24.02.1") > NodeVersion("v24.02.1")
4424
assert NodeVersion("v24.02.1") > NodeVersion("v24.02")
4525
assert NodeVersion("v24.02.1") > NodeVersion("v24.02rc1")
4626
assert NodeVersion("v24.02.1") > NodeVersion("v23.05")
27+
assert NodeVersion("v24.05") > NodeVersion("v24.02")
28+
assert NodeVersion("vd6fa78c") > NodeVersion("v26.02")
4729

4830
assert NodeVersion("v24.02.1") >= NodeVersion("v24.02.1")
4931
assert NodeVersion("v24.02.1") >= NodeVersion("v24.02")
5032
assert NodeVersion("v24.02.1") >= NodeVersion("v24.02rc1")
5133
assert NodeVersion("v24.02.1") >= NodeVersion("v23.05")
34+
assert NodeVersion("v24.05") >= NodeVersion("v24.02")
35+
assert NodeVersion("vd6fa78c") >= NodeVersion("v26.02")
5236

5337
assert NodeVersion("v24.02.1") <= NodeVersion("v24.02.1")
5438
assert not NodeVersion("v24.02.1") <= NodeVersion("v24.02")
5539
assert not NodeVersion("v24.02.1") <= NodeVersion("v24.02rc1")
5640
assert not NodeVersion("v24.02.1") <= NodeVersion("v23.05")
41+
assert not NodeVersion("v24.05") <= NodeVersion("v24.02")
42+
assert not NodeVersion("vd6fa78c") <= NodeVersion("v26.02")
5743

5844
assert not NodeVersion("v24.02.1") < NodeVersion("v24.02.1")
5945
assert not NodeVersion("v24.02.1") < NodeVersion("v24.02")
6046
assert not NodeVersion("v24.02.1") < NodeVersion("v24.02rc1")
6147
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")
48+
assert not NodeVersion("v24.05") < NodeVersion("v24.02")
49+
assert not NodeVersion("vd6fa78c") < NodeVersion("v26.02")

0 commit comments

Comments
 (0)