-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: relax version detection checks #30
fix: relax version detection checks #30
Conversation
vvm/utils/versioning.py
Outdated
match = _VERSION_RE.search(source_code) | ||
if match is None: | ||
match = _VERSION_RE.findall(source_code) | ||
if not match: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm not a fan of using truthy in predicates -- how about if len(matches) == 0:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reverted to search instead of findall, as there was no reason to use the latter here.
Co-authored-by: Charles Cooper <[email protected]>
vvm/utils/versioning.py
Outdated
# adapted from vyper/ast/pre_parse.py at commit c32b9b4c6f0d8 | ||
# convert npm to pep440 | ||
version_str = re.sub("^\\^", "~=", version_str) | ||
version_str = re.sub("^~(?!\\=)", "~=", version_str) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what does this line do? it's not in the compiler
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just removed it, it was doing a ~0.3.0
=> ~=0.3.0
conversion, but turns out it doesn't seems to have ever been used in the wild and people were sticking to caret notation in 0.3
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm. thank you!!
What I did
vvm should never perform checks more restrictive than the compiler itself. As opposed, if checks are not strict enough, the compiler will anyway fail later.
Hence, this PR relaxes the following version detection checks:
>=0.3.10, <0.4.0
^0.4.0
as it get converted to pep440 even in recent versions of the compiler.~0.3.1
as older versions of the compiler accept them.Additionally, this PR exposes
detect_version_specifier_set
such thattitanoboa
can check if the installed vyper version is in the set before callingdetect_vyper_version_from_source
.How I did it
Updated
_detect_version_specifier
to match Vyper's own pragma validation.How to verify it
See updated test.
Checklist