-
Notifications
You must be signed in to change notification settings - Fork 992
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
tools.scm.Version in develop as is in develop2 #11823
Merged
czoido
merged 1 commit into
conan-io:develop
from
lasote:bugfix/different_tools_Version_behavior
Aug 10, 2022
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,200 @@ | ||
from conan.tools.scm.git import Git | ||
from conans.model.version import Version | ||
# develop2 uncomment the next line | ||
# from conans.model.version import Version | ||
|
||
|
||
# ######## DEVELOP2: Delete all the contents from here, this is copy-paste from the Version model | ||
# ######## at conans.model.version (for an easier migration) | ||
from functools import total_ordering | ||
from conans.errors import ConanException | ||
|
||
|
||
@total_ordering | ||
class _VersionItem: | ||
""" a single "digit" in a version, like X.Y.Z all X and Y and Z are VersionItems | ||
They can be int or strings | ||
""" | ||
def __init__(self, item): | ||
try: | ||
self._v = int(item) | ||
except ValueError: | ||
self._v = item | ||
|
||
@property | ||
def value(self): | ||
return self._v | ||
|
||
def __str__(self): | ||
return str(self._v) | ||
|
||
def __add__(self, other): | ||
# necessary for the "bump()" functionality. Other aritmetic operations are missing | ||
return self._v + other | ||
|
||
def __eq__(self, other): | ||
if not isinstance(other, _VersionItem): | ||
other = _VersionItem(other) | ||
return self._v == other._v | ||
|
||
def __hash__(self): | ||
return hash(self._v) | ||
|
||
def __lt__(self, other): | ||
""" | ||
@type other: _VersionItem | ||
""" | ||
if not isinstance(other, _VersionItem): | ||
other = _VersionItem(other) | ||
try: | ||
return self._v < other._v | ||
except TypeError: | ||
return str(self._v) < str(other._v) | ||
|
||
|
||
@total_ordering | ||
class Version: | ||
""" | ||
This is NOT an implementation of semver, as users may use any pattern in their versions. | ||
It is just a helper to parse "." or "-" and compare taking into account integers when possible | ||
""" | ||
def __init__(self, value): | ||
value = str(value) | ||
self._value = value | ||
|
||
items = value.rsplit("+", 1) # split for build | ||
if len(items) == 2: | ||
value, build = items | ||
self._build = Version(build) # This is a nested version by itself | ||
else: | ||
value = items[0] | ||
self._build = None | ||
|
||
items = value.rsplit("-", 1) # split for pre-release | ||
if len(items) == 2: | ||
value, pre = items | ||
self._pre = Version(pre) # This is a nested version by itself | ||
else: | ||
value = items[0] | ||
self._pre = None | ||
items = value.split(".") | ||
items = [_VersionItem(item) for item in items] | ||
self._items = tuple(items) | ||
while items and items[-1].value == 0: | ||
del items[-1] | ||
self._nonzero_items = tuple(items) | ||
|
||
def bump(self, index): | ||
""" | ||
Increments by 1 the version field at the specified index, setting to 0 the fields | ||
on the right. | ||
2.5 => bump(1) => 2.6 | ||
1.5.7 => bump(0) => 2.0.0 | ||
""" | ||
# this method is used to compute version ranges from tilde ~1.2 and caret ^1.2.1 ranges | ||
# TODO: at this moment it only works for digits, cannot increment pre-release or builds | ||
# better not make it public yet, keep it internal | ||
items = list(self._items[:index]) | ||
try: | ||
items.append(self._items[index]+1) | ||
except TypeError: | ||
raise ConanException(f"Cannot bump '{self._value} version index {index}, not an int") | ||
items.extend([0] * (len(items) - index - 1)) | ||
v = ".".join(str(i) for i in items) | ||
# prerelease and build are dropped while bumping digits | ||
result = Version(v) | ||
return result | ||
|
||
def upper_bound(self, index): | ||
items = list(self._items[:index]) | ||
try: | ||
items.append(self._items[index] + 1) | ||
except TypeError: | ||
raise ConanException(f"Cannot bump '{self._value} version index {index}, not an int") | ||
items.extend([0] * (len(items) - index - 1)) | ||
v = ".".join(str(i) for i in items) | ||
v += "-" # Exclude prereleases | ||
result = Version(v) | ||
return result | ||
|
||
@property | ||
def pre(self): | ||
return self._pre | ||
|
||
@property | ||
def build(self): | ||
return self._build | ||
|
||
@property | ||
def main(self): | ||
return self._items | ||
|
||
@property | ||
def major(self): | ||
try: | ||
return self.main[0] | ||
except IndexError: | ||
return None | ||
|
||
@property | ||
def minor(self): | ||
try: | ||
return self.main[1] | ||
except IndexError: | ||
return None | ||
|
||
@property | ||
def patch(self): | ||
try: | ||
return self.main[2] | ||
except IndexError: | ||
return None | ||
|
||
@property | ||
def micro(self): | ||
try: | ||
return self.main[3] | ||
except IndexError: | ||
return None | ||
|
||
def __str__(self): | ||
return self._value | ||
|
||
def __repr__(self): | ||
return self._value | ||
|
||
def __eq__(self, other): | ||
if other is None: | ||
return False | ||
if isinstance(other, str): | ||
other = Version(other) | ||
|
||
return (self._nonzero_items, self._pre, self._build) ==\ | ||
(other._nonzero_items, other._pre, other._build) | ||
|
||
def __hash__(self): | ||
return hash((self._nonzero_items, self._pre, self._build)) | ||
|
||
def __lt__(self, other): | ||
if other is None: | ||
return False | ||
if not isinstance(other, Version): | ||
other = Version(other) | ||
|
||
if self._pre: | ||
if other._pre: # both are pre-releases | ||
return (self._nonzero_items, self._pre, self._build) < \ | ||
(other._nonzero_items, other._pre, other._build) | ||
else: # Left hand is pre-release, right side is regular | ||
if self._nonzero_items == other._nonzero_items: # Problem only happens if both equal | ||
return True | ||
else: | ||
return self._nonzero_items < other._nonzero_items | ||
else: | ||
if other._pre: # Left hand is regular, right side is pre-release | ||
if self._nonzero_items == other._nonzero_items: # Problem only happens if both equal | ||
return False | ||
else: | ||
return self._nonzero_items < other._nonzero_items | ||
else: # None of them is pre-release | ||
return (self._nonzero_items, self._build) < (other._nonzero_items, other._build) | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Could you please add
as_list
, so it could be minimal compatible with what we have in Conan Center Index now.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 wouldn't like that, I think that
as_list
is an ugly interface that if we do that it will "infect" Conan 2.0.If necessary, please let us know the use cases of such method, and we can think about what is the right interface for it.
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 still prefer the legacy, as described in this PR. The only thing is re-editing recipes again in CCI. Also, we will need to create some
if conan_version else
because the different behavior from 1.49 (i guess) til 1.52There 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 haven't found occurrences of
as_list
in conan-center-index (only one method in openssl recipe), am I missing something?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.
Boost recipe: https://github.com/conan-io/conan-center-index/blob/master/recipes/boost/all/conanfile.py#L1461
If you are running GH search, forget, it's useless most of time.
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.
Furthermore,
as_list
is not a documented/public method at all, shouldn't be used in any of theVersion
implementations? Whilemajor
andminor
are. Not sure why they used that in the boost recipe.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.
The problem was, Boost recipe need to compare some major and minor versions, but the new scm.Version does not support semver, so it return "1.2" for Version.minor(fill=False). Of course, the previous behavior, with semver, was much better. I gonna revert those changes and keep aligned with this new PR. Thanks.
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.
Not sure what you mean. This new
Version
in this PR supports semver like comparison between versions, no problem with that. How is that related to theas_list
usage, which is done to just extract the major, minor and patch to compose some strings (suffixes) in the boost recipe?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.
conan.tools.scm.Version('1.2.3').minor()
it returns1.2.Z
conan.tools.scm.Version('1.2.3').minor(fill=False)
it returns1.2
2
.conan.tools.scm.Version('1.2.3')[1]
Anyway, this new PR fixes the case, so we can use Version.major as a property again.
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.
When I said
The new Version
, I meant which is production right now and running on CCI, not this PR. This PR is correct, it follows the previous and expected behavior.