Skip to content
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

Feature/min conan version #7360

Merged
merged 9 commits into from Jul 22, 2020

Conversation

memsharded
Copy link
Member

@memsharded memsharded commented Jul 14, 2020

Changelog: Feature: Implement required_conan_version in conanfile.py, will raise if the current Conan version does not match the defined version range.
Docs: conan-io/docs#1788

Proposal for discussing #7138

@memsharded memsharded added this to the 1.28 milestone Jul 14, 2020
Copy link
Member

@jgsogo jgsogo left a comment

Nor really requesting changes, but I would like to wait for feedback on #7138 (comment). Those are my concerns about this feature.

@memsharded memsharded requested a review from czoido Jul 16, 2020
@memsharded memsharded requested a review from jgsogo Jul 16, 2020
@memsharded memsharded assigned czoido and unassigned jgsogo Jul 20, 2020
@memsharded
Copy link
Member Author

@memsharded memsharded commented Jul 20, 2020

This PR proposes the simple syntax:

from conans import ConanFile

min_conan_version = "1.26"

class Lib(ConanFile):
    pass

Max version is not possible, min_conan_version is an exact version, not a range. This is a bit different to the https://docs.conan.io/en/latest/reference/config_files/conan.conf.html required_conan_version. Maybe it should match both the name and the behavior to be able to define a range? Wdyt?

@memsharded memsharded requested a review from danimtb Jul 20, 2020
@danimtb
Copy link
Member

@danimtb danimtb commented Jul 21, 2020

As discussed earlier today, let's converge this to the same name and version range used in conan.conf

czoido
czoido approved these changes Jul 21, 2020
Copy link
Contributor

@czoido czoido left a comment

Maybe the name of conans/test/functional/conanfile/min_version_test.py and MinVersionTest should be changed to conans/test/functional/conanfile/required_version_test.py and RequiredConanVersionTest ?

@memsharded
Copy link
Member Author

@memsharded memsharded commented Jul 21, 2020

Maybe the name of conans/test/functional/conanfile/min_version_test.py and MinVersionTest should be changed to conans/test/functional/conanfile/required_version_test.py and RequiredConanVersionTest ?

Agree, names changed

def validate_conan_version(required_range):
result = satisfies(client_version, required_range, loose=True)
if not result:
raise ConanException("The current Conan version ({}) does not match to the required"
Copy link
Member

@danimtb danimtb Jul 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
raise ConanException("The current Conan version ({}) does not match to the required"
raise ConanException("The current Conan version ({}) does not match the recipe required"

Copy link
Member

@danimtb danimtb Jul 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see this message is used to raise from Conan required version from config as well as from recipe, but would like a more clear message about it

Copy link
Member Author

@memsharded memsharded Jul 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot be changed, because it is common.

Copy link
Member Author

@memsharded memsharded Jul 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have improved the message grammar, and added a capture of the error, so when you are reading a conanfile of a dependency it will throw and print:

pkg/version@user/channel: ERROR: Cannot load recipe
The conanfile at path .... version not satisfied message

lasote
lasote approved these changes Jul 22, 2020
@memsharded memsharded merged commit 72b12e7 into conan-io:develop Jul 22, 2020
2 checks passed
@memsharded memsharded deleted the feature/min_conan_version branch Jul 22, 2020
@prince-chrismc
Copy link

@prince-chrismc prince-chrismc commented Jul 31, 2020

This feature will make my life so much better... I didn't even know I needed this

@memsharded
Copy link
Member Author

@memsharded memsharded commented Jul 31, 2020

This feature will make my life so much better... I didn't even know I needed this

Happy to know it. Available now in Conan 1.28 just released. Remember that you also have the conan.conf variable, so you can define the required conan version also for all packages, and manage it with conan config install, so you can force your team to upgrade whenever you want.

@prince-chrismc
Copy link

@prince-chrismc prince-chrismc commented Aug 1, 2020

so you can force your team to upgrade whenever you want.

😈 muhahaha, that's exactly my plan!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants