-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
[boost] Fix scm.Version usage #12119
[boost] Fix scm.Version usage #12119
Conversation
Signed-off-by: Uilian Ries <uilianries@gmail.com>
I detected other pull requests that are modifying boost/all recipe:
This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there. |
Can you please modify |
Signed-off-by: Uilian Ries <uilianries@gmail.com>
Done. |
All green in build 2 (
|
Hooks produced the following warnings for commit ce9dd5fboost/1.71.0
boost/1.70.0
boost/1.73.0
boost/1.72.0
boost/1.75.0
boost/1.74.0
boost/1.77.0
boost/1.79.0
boost/1.78.0
boost/1.76.0
|
@@ -1345,7 +1345,7 @@ def _toolset_tag(self): | |||
if self._is_msvc: | |||
toolset_version = self._toolset_version.replace(".", "") | |||
else: | |||
toolset_version = str(Version(self.settings.compiler.version).major) | |||
toolset_version = str(Version(self.settings.compiler.version).major(fill=False)) |
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.
@uilianries please note that this is going to break again in next release because of conan-io/conan#11823
We don't have a universal fix right 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.
@lasote , what works from 1.50
(current release in CCI) to 1.52
and 2.x
? This is something we can add to the linter asap so we prevent changing recipes now and afterward again. This is the kind of change/fix that will probably spread to other recipes.
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.
Could we implement dummy fill
args for conan.tools.scm.Version
properties?
Otherwise seems like we'd have to set required_conan_version = ">=1.52.0"
here which would be pretty bad.
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.
vale, I gonna open a new PR to avoid 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.
@uilianries , please, open a PR reverting (?) the changes, but please mention and wait for @lasote or @jcar87 to say something about a possible working syntax. 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.
@jgsogo doing in 3..2..1
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.
Done, new PR available: #12131
Specify library name and version: boost/1.79.0
The new
scm.Version
no longer parsessemver
, it only splits the version string.fixes #12044
/cc @markferry