Required Conan version #7183

uilianries · 2020-06-10T21:58:25Z

The general configuration required_conan_version validates the current Conan client version to the required version.

If the current version is out of the range, a warning message
is displayed before executing any command.

Changelog: Feature: Configuration for checking the required Conan client version.
Docs: conan-io/docs#1740

Refer to the issue that supports this Pull Request.
If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
I've read the Contributing guide.
I've followed the PEP8 style guides for Python code.
I've opened another PR in the Conan docs repo to the develop branch, documenting this one.

^{Note: By default this PR will skip the slower tests and will use a limited set of python versions. Check here how to increase the testing level by writing some tags in the current PR body text.}

The general configuration `required_conan_version` validates the current Conan client version to the required version. If the current version is out of the range, a warning message is displayed before executing any command. Signed-off-by: Uilian Ries <uilianries@gmail.com>

uilianries · 2020-06-10T22:00:01Z

conans/client/conf/__init__.py

+    @property
+    def required_conan_version(self):
+        try:
+            return self.get_item("general.required_conan_version")


I didn't add support env var, and I don't think that's a good idea. For instance, the company wants a specific version, but the developer overrides that version by env vars

I want to stop the spread of env-vars as a config mechanism along the conan codebase, because it leads to bad practices, like setting env-vars in recipes, in profiles, that affect how Conan behaves as an application.

The whole purpose of this feature is that developers cannot use a Conan version that they company configuration don't want them to use. Still possible to conan config set general.required_conan_version.

I am ok with using env-vars that are loaded at app startup time, popped from the environment, and used to initialize the Conan config object. But that is another feature (that will be breaking for other variables), in the meantime I don't want pull requests to keep adding these env-vars.

uilianries · 2020-06-10T22:02:12Z

conans/client/conf/required_version.py

+            out.warn("The current Conan version ({}) does not match to the required version ({})."
+                     .format(client_version, required_version))
+        elif result != client_version:
+            out.warn(result)


it should not happen ... however, we get the max_satisfying output with no treatment, which means, any change in semver library could generate a false-positive here, like an info message, that's why I preferred to check the obtained version x client_version

Just make this the condition to raise Error.

uilianries · 2020-06-10T22:03:33Z

conans/test/functional/configuration/required_version_test.py

+        self.client = TestClient()
+
+    def test_wrong_version(self):
+        # include_prerelease is required due the suffix -dev


The develop branch used -dev by default on its version. Thus, include_prerelease is required. Another option is mocking the client version and using a stable format.

memsharded · 2020-06-10T22:46:25Z

conans/client/conf/__init__.py

@@ -261,6 +262,7 @@ class ConanClientConfigParser(ConfigParser, object):
            ("CONAN_MSBUILD_VERBOSITY", "msbuild_verbosity", None),
            ("CONAN_CACERT_PATH", "cacert_path", None),
            ("CONAN_DEFAULT_PACKAGE_ID_MODE", "default_package_id_mode", None),
+            ("CONAN_REQUIRED_CONAN_VERSION", "required_conan_version", None),


Use only conan.conf, not env-var

memsharded · 2020-06-10T22:48:20Z

conans/client/conf/required_version.py

+        output = ""
+        result = satisfying([client_version], required_version, output)
+        if not result:
+            out.warn("The current Conan version ({}) does not match to the required version ({})."


Implement first a raise exception, and we will see if a warn makes sense in the future. Always introduce features with more restrictive behavior, it can be relaxed later, but the opposite is more difficult.

memsharded · 2020-06-10T22:49:11Z

conans/test/functional/configuration/required_version_test.py

+class RequiredVersionTest(unittest.TestCase):
+
+    def setUp(self):
+        self.client = TestClient()


No need for a setUp() for 1 line only, unnecessary verbosity and indirection

memsharded · 2020-06-10T22:54:00Z

conans/client/conf/required_version.py

+            out.warn("The current Conan version ({}) does not match to the required version ({})."
+                     .format(client_version, required_version))
+        elif result != client_version:
+            out.warn(result)


Just make this the condition to raise Error.

jgsogo

Two comments before reviewing:

I see no reason to support prerelease versions. I would expect a valid semver (not loose). I'm sure we can add some patch to run the tests.
IMO, this functionality doesn't belong to the command layer, but to the API one, if we connect a different command-line implementation to the conan_api, this validation should run. I think we should move this check closer to some init, not sure if ConanAPI or ConanApp

wdyt?

memsharded · 2020-06-11T08:36:26Z

I see no reason to support prerelease versions. I would expect a valid semver (not loose). I'm sure we can add some patch to run the tests.

Ok

IMO, this functionality doesn't belong to the command layer, but to the API one, if we connect a different command-line implementation to the conan_api, this validation should run. I think we should move this check closer to some init, not sure if ConanAPI or ConanApp

Yes, agree. I wanted to do a refactor that moved to the api this functionality and the auto_config_install one. I am fine with doing it in a later Pull Request too.

jgsogo · 2020-06-11T12:45:25Z

Ok, we can use #7170 to check that it is behaving the same with the current CLI and the new one.

Signed-off-by: Uilian Ries <uilianries@gmail.com>

uilianries · 2020-06-11T21:38:35Z

Done! I've moved to ConanAPI.

Signed-off-by: Uilian Ries <uilianries@gmail.com>

memsharded · 2020-06-11T22:57:43Z

conans/client/conf/__init__.py

+    @property
+    def required_conan_version(self):
+        try:
+            return self.get_item("general.required_conan_version")


I want to stop the spread of env-vars as a config mechanism along the conan codebase, because it leads to bad practices, like setting env-vars in recipes, in profiles, that affect how Conan behaves as an application.

The whole purpose of this feature is that developers cannot use a Conan version that they company configuration don't want them to use. Still possible to conan config set general.required_conan_version.

I am ok with using env-vars that are loaded at app startup time, popped from the environment, and used to initialize the Conan config object. But that is another feature (that will be breaking for other variables), in the meantime I don't want pull requests to keep adding these env-vars.

conans/test/functional/configuration/required_version_test.py

conans/client/conf/required_version.py

uilianries · 2020-06-12T12:02:23Z

I am ok with using env-vars that are loaded at app startup time, popped from the environment, and used to initialize the Conan config object. But that is another feature (that will be breaking for other variables), in the meantime I don't want pull requests to keep adding these env-vars.

@memsharded I agree. If the company wants a specific version, the developer shouldn't be able to change. For instance, Windows Policies works like that, but is based on server configuration, and user permission. We would need conan config install with encrypted values or read-only access, otherwise, anyone would be able to change conan.conf

Signed-off-by: Uilian Ries <uilianries@gmail.com>

uilianries · 2020-06-12T12:50:41Z

@jgsogo I've moved to semver methods, indeed is better now, because is decoupled and requires few lines to parse/validate.

uilianries commented Jun 10, 2020

View reviewed changes

memsharded reviewed Jun 10, 2020

View reviewed changes

jgsogo reviewed Jun 11, 2020

View reviewed changes

conan-io#7136 Improve functional tests

94dbc4d

Signed-off-by: Uilian Ries <uilianries@gmail.com>

conan-io#7136 Remove env vars

5431589

Signed-off-by: Uilian Ries <uilianries@gmail.com>

memsharded approved these changes Jun 11, 2020

View reviewed changes

jgsogo suggested changes Jun 12, 2020

View reviewed changes

conans/test/functional/configuration/required_version_test.py Outdated Show resolved Hide resolved

conans/test/functional/configuration/required_version_test.py Show resolved Hide resolved

conans/client/conf/required_version.py Outdated Show resolved Hide resolved

conan-io#7136 Use semver methods

b5f3ff0

Signed-off-by: Uilian Ries <uilianries@gmail.com>

uilianries mentioned this pull request Jun 12, 2020

Add required_conan_version config conan-io/docs#1740

Merged

jgsogo added this to the 1.27 milestone Jun 12, 2020

jgsogo approved these changes Jun 12, 2020

View reviewed changes

jgsogo assigned memsharded Jun 12, 2020

memsharded merged commit 444d445 into conan-io:develop Jun 15, 2020

uilianries mentioned this pull request Jul 31, 2020

geographiclib: fix cmake name + add topics + del fPIC option if shared conan-io/conan-center-index#2349

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Required Conan version #7183

Required Conan version #7183

uilianries commented Jun 10, 2020 •

edited by czoido

uilianries Jun 10, 2020

memsharded Jun 11, 2020

uilianries Jun 10, 2020

memsharded Jun 10, 2020

uilianries Jun 10, 2020

memsharded Jun 10, 2020

memsharded Jun 10, 2020

memsharded Jun 10, 2020

memsharded Jun 10, 2020

jgsogo left a comment

memsharded commented Jun 11, 2020

jgsogo commented Jun 11, 2020

uilianries commented Jun 11, 2020

memsharded Jun 11, 2020

uilianries commented Jun 12, 2020

uilianries commented Jun 12, 2020

Required Conan version #7183

Required Conan version #7183

Conversation

uilianries commented Jun 10, 2020 • edited by czoido

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jgsogo left a comment

Choose a reason for hiding this comment

memsharded commented Jun 11, 2020

jgsogo commented Jun 11, 2020

uilianries commented Jun 11, 2020

Choose a reason for hiding this comment

uilianries commented Jun 12, 2020

uilianries commented Jun 12, 2020

uilianries commented Jun 10, 2020 •

edited by czoido