diff --git a/.github/workflows/linter-conan-v2.yml b/.github/workflows/linter-conan-v2.yml new file mode 100644 index 0000000000000..769623eebf2c9 --- /dev/null +++ b/.github/workflows/linter-conan-v2.yml @@ -0,0 +1,124 @@ +name: "[linter] Conan v2 migration" + +on: + pull_request: + +env: + PYTHONPATH: ${{github.workspace}} + PYVER: "3.8" + REQUIREMENTS: "pylint==2.14" + +jobs: + test_linter: + name: Test linter changes (v2 migration) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - name: Get changed files + uses: ./.github/actions/pr_changed_files + id: changed_files + with: + files: | + linter/** + .github/workflows/linter-conan-v2.yml + + - name: Get Conan v2 version + id: parse_conan_v2_version + if: steps.changed_files.outputs.any_changed == 'true' + uses: mikefarah/yq@master + with: + cmd: yq '.conan.version' '.c3i/config_v2.yml' + + - uses: actions/setup-python@v4 + if: steps.changed_files.outputs.any_changed == 'true' + with: + python-version: ${{ env.PYVER }} + + - name: Install requirements + if: steps.changed_files.outputs.any_changed == 'true' + run: | + pip install ${{ env.REQUIREMENTS }} conan==${{ steps.parse_conan_v2_version.outputs.result }} + + - name: Execute linter over all recipes in the repository + id: linter_recipes + if: steps.changed_files.outputs.any_changed == 'true' + run: | + echo '## Linter summary (recipes)' >> $GITHUB_STEP_SUMMARY + pylint --rcfile=linter/pylintrc_recipe `ls recipes/*/*/conanfile.py | shuf -n 500` --output-format=json --output=recipes.json --score=y --exit-zero + jq '[map( select(.type=="error")) | group_by (.message)[] | {message: .[0].message, length: length}] | sort_by(.length) | reverse' recipes.json > recipes2.json + jq -r '.[] | " * \(.message): \(.length)"' recipes2.json >> $GITHUB_STEP_SUMMARY + + - name: Execute linter over all test_package/recipes in the repository + id: linter_test_package + if: steps.changed_files.outputs.any_changed == 'true' + run: | + echo '## Linter summary (test_package)' >> $GITHUB_STEP_SUMMARY + pylint --rcfile=linter/pylintrc_testpackage `ls recipes/*/*/test_package/conanfile.py | shuf -n 500` --output-format=json --output=recipes.json --exit-zero + jq '[map( select(.type=="error")) | group_by (.message)[] | {message: .[0].message, length: length}] | sort_by(.length) | reverse' recipes.json > recipes2.json + jq -r '.[] | " * \(.message): \(.length)"' recipes2.json >> $GITHUB_STEP_SUMMARY + + conanfile_recipe: + name: Lint changed conanfile.py (v2 migration) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - name: Get changed files + id: changed-files + uses: ./.github/actions/pr_changed_files + with: + files: | + recipes/*/*/conanfile.py + - name: Get Conan v2 version + id: parse_conan_v2_version + if: steps.changed-files.outputs.any_changed == 'true' + uses: mikefarah/yq@master + with: + cmd: yq '.conan.version' '.c3i/config_v2.yml' + - uses: actions/setup-python@v4 + if: steps.changed-files.outputs.any_changed == 'true' + with: + python-version: ${{ env.PYVER }} + - name: Install dependencies + if: steps.changed-files.outputs.any_changed == 'true' + run: | + pip install ${{ env.REQUIREMENTS }} conan==${{ steps.parse_conan_v2_version.outputs.result }} + - name: Run linter + if: steps.changed-files.outputs.any_changed == 'true' + run: | + echo "::add-matcher::linter/recipe_linter.json" + for file in ${{ steps.changed-files.outputs.all_changed_files }}; do + pylint --rcfile=linter/pylintrc_recipe --output-format=parseable ${file} + done + + conanfile_test_package: + name: Lint changed test_package/conanfile.py (v2 migration) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - name: Get changed files + id: changed-files + uses: ./.github/actions/pr_changed_files + with: + files: | + recipes/*/*/test_*/conanfile.py + - name: Get Conan v2 version + id: parse_conan_v2_version + if: steps.changed-files.outputs.any_changed == 'true' + uses: mikefarah/yq@master + with: + cmd: yq '.conan.version' '.c3i/config_v2.yml' + - uses: actions/setup-python@v4 + if: steps.changed-files.outputs.any_changed == 'true' + with: + python-version: ${{ env.PYVER }} + - name: Install dependencies + if: steps.changed-files.outputs.any_changed == 'true' + run: | + pip install ${{ env.REQUIREMENTS }} conan==${{ steps.parse_conan_v2_version.outputs.result }} + - name: Run linter + if: steps.changed-files.outputs.any_changed == 'true' + run: | + echo "::add-matcher::linter/recipe_linter.json" + for file in ${{ steps.changed-files.outputs.all_changed_files }}; do + pylint --rcfile=linter/pylintrc_testpackage --ignore-paths="recipes/[^/]*/[^/]*/test_v2[^/]*/conanfile.py" --output-format=parseable ${file} + done diff --git a/docs/linters.md b/docs/linters.md index 075f373f209ef..1a76e25288b80 100644 --- a/docs/linters.md +++ b/docs/linters.md @@ -14,7 +14,12 @@ to configure plugins, warnings and errors which should be enabled or disabled. * [Understanding the different linters](#understanding-the-different-linters) * [Running the linters locally](#running-the-linters-locally) - * [Pylint configuration files](#pylint-configuration-files) + * [Pylint configuration files](#pylint-configuration-files) + * [Linter Warning and Errors](#linter-warning-and-errors) + * [E9005 - conan-missing-name: Every conan recipe must contain the attribute name](#e9005---conan-missing-name-every-conan-recipe-must-contain-the-attribute-name) + * [E9007 - conan-test-no-name: Do not add name attribute in test package recipes](#e9007---conan-test-no-name-do-not-add-name-attribute-in-test-package-recipes) + * [E9011 - conan-import-tools: Importing conan.tools or conan.tools.xxx.zzz.yyy should be considered as private](#e9011---conan-import-tools-importing-conantools-or-conantoolsxxxzzzyyy-should-be-considered-as-private) + * [E9012 - conan-attr-version: Recipe should not contain version attribute](#e9012---conan-attr-version-recipe-should-not-contain-version-attribute) ## Understanding the different linters @@ -32,3 +37,79 @@ Check the [Developing Recipes](developing_recipes_locally.md) for more informati - [Pylint Recipe](../linter/pylintrc_recipe): This `rcfile` lists plugins and rules to be executed over all recipes (not test package) and validate them. - [Pylint Test Package Recipe](../linter/pylintrc_testpackage): This `rcfile` lists plugins and rules to be executed over all recipes in test package folders only: + +## Linter Warning and Errors + +Here is the list of current warning and errors provided by pylint, when using CCI configuration. + +### E9005 - conan-missing-name: Every conan recipe must contain the attribute name + +The attribute `name` is always expected. On the other hand, `version` should not be listed. + +```python +def BazConanfile(ConanFile): + name = "baz" +``` + +### E9007 - conan-test-no-name: Do not add name attribute in test package recipes + +The test package is not a recipe, thus, it should not have a name + +```python +def TestPackageConanFile(ConanFile): + name = "test_package" # Wrong! +``` + +### E9011 - conan-import-tools: Importing conan.tools or conan.tools.xxx.zzz.yyy should be considered as private + +Documented on [conanfile.tools](https://docs.conan.io/1/reference/conanfile/tools.html): +It's not allowed to use `tools.xxx` directly: + +```python +from conan import tools +... + +tools.scm.Version(self.version) +``` + +Neither sub modules: + +```python +from conan.tools.apple.apple import is_apple_os +``` + +Only modules under `conan.tools` and `conan.tools.xxx` are allowed: + +```python +from conan.tools.files import rmdir +from conan.tools import scm +``` + +### E9012 - conan-attr-version: Recipe should not contain version attribute + +All recipes on CCI should be generic enough to support as many versions as possible, so enforcing a specific +version as attribute will not allow to re-use the same recipe for multiple release versions. + +```python +from conan import ConanFile + +class FooConanFile(ConanFile): + version = "1.0.0" # Wrong! +``` + +The package version should be passed as command argument, e.g: + + conan create all/ 1.0.0@ -pr:h=default -pr:b=default + +Or, if you are running Conan 2.0: + + conan create all/ --version=1.0.0 -pr:h=default -pr:b=default + +The only exception is when providing ``system`` packages, which are allowed. + +```python +from conan import ConanFile + +class FooConanFile(ConanFile): + version = "system" # Okay! +``` diff --git a/docs/v2_migration.md b/docs/v2_migration.md new file mode 100644 index 0000000000000..d372401b811e0 --- /dev/null +++ b/docs/v2_migration.md @@ -0,0 +1,147 @@ +# Preparing recipes for Conan 2.0 + +This is expected for recipes to be updates in each pull request. + +- Updated helpers are expected, this is enforced by the [v2_linter](v2_linter.md) +- Once a recipe publishes v2 packages, it must pass the v2 pipeline always. +- The v2 pipeline with **is required** for changes to be merged. + + +## Contents + + * [Using Layout](#using-layout) + * [With New Generators](#with-new-generators) + * [With Multiple Build Helpers](#with-multiple-build-helpers) + * [CMakeToolchain](#cmaketoolchain) + * [New conf_info properties](#new-conf_info-properties) + * [New cpp_info set_property model](#new-cpp_info-set_property-model) + * [Translating .names information to cmake_target_name, cmake_module_target_name and cmake_file_name](#translating-names-information-to-cmake_target_name-cmake_module_target_name-and-cmake_file_name) + * [Translating .filenames information to cmake_file_name, cmake_module_file_name and cmake_find_mode](#translating-filenames-information-to-cmake_file_name-cmake_module_file_name-and-cmake_find_mode) + * [Translating .build_modules to cmake_build_modules](#translating-build_modules-to-cmake_build_modules) + * [PkgConfigDeps](#pkgconfigdeps) + +> **Note**: Read about the [linter in pull requests](v2_linter.md) to learn how this is being enforced. + +It's time to start thinking seriously about Conan v2 and prepare recipes +for the incoming changes. Conan v2 comes with many +changes and improvements, you can read about them in the +[Conan documentation](https://docs.conan.io/1/conan_v2.html). + +This document is a practical guide, offering extended information particular to Conan +Center Index recipes to get them ready to upgrade to Conan 2.0. + +## Using Layout + +All recipes should use a layout. Without one, more manual configuration of folders (e.g. source, build, etc) +and package structure will be required. + +### With New Generators + +When doing this there is no need to manually define `self._subfolder_[...]` in a recipe. +Simply use `self.source_folder` and `self.build_folder` instead of "subfolder properties" that used to be the norm. + +### With Multiple Build Helpers + +When different build tools are use, at least one layout needs to be set. + +```python + def layout(self): + if self._use_cmake(): + cmake_layout(self) + else: # using autotools + basic_layout(self) +``` + +The `src_folder` must be the same when using different layouts and should +not depend on settings or options. + +## CMakeToolchain + +The old `CMake.definition` should be replaced by `CMakeToolchain.variables` and moved to the `generate` method. +However, certain options need to be passed as `cache_variables`. You'll need to check project's `CMakeLists.txt` +as there are a few cases to look out for: + +- When an `option` is configured before `project()` is called. + + ```cmake + cmake_minimum_required(3.1) + option(BUILD_EXAMPLES "Build examples using foorbar") + project(foobar) + ``` + +- When an variable is declared with `CACHE`. + + ```cmake + cmake_minimum_required(3.1) + project(foobar) + set(USE_JPEG ON CACHE BOOL "include jpeg support?") + ``` + +For more information refere to the [CMakeToolchain docs](https://docs.conan.io/1/reference/conanfile/tools/cmake/cmaketoolchain.html) +or check out the converstaion in conan-io/conan#11937 for the brave. + +## New conf_info properties + +As described in the documentation `self.user_info` has been depreated and you are now required to use +`self.conf_info` to define individual properties to expose to downstream recipes. +The [2.0 migrations docs](https://docs.conan.io/1/migrating_to_2.0/recipes.html#removed-self-user-info) +should cover the technical details, however for ConanCenterIndex we need to make sure there are no collisions +`conf_info` must be named `user.:`. + +For usage options of `conf_info`, the [documenation](https://docs.conan.io/1/reference/config_files/global_conf.html?highlight=conf_info#configuration-in-your-recipes) + +In ConanCenterIndex this will typically looks like: + +- defining a value + ```py + def package_info(self): + tool_path = os.path.join(self.package_folder, "bin", "tool") + self.conf_info.define("user.pkg:tool", tool_path) + ``` +- using a value + ```py + #generators = "VirtualBuildEnv", "VirtualRunEnv" + + def build_requirements(self): + self.tool_requires("tool/0.1") + + def build(self): + tool_path = self.conf_info.get("user.pkg:tool") + self.run(f"{tool_path} --build") + ``` + +> **Note**: This should only be used when absolutely required. In the vast majority of cases, the new +> ["Environments"](https://docs.conan.io/1/reference/conanfile/tools/env/environment.html?highlight=Virtual) +> will include the `self.cpp_info.bindirs` which will provide access to the tools in the correct scopes. + +## New cpp_info set_property model + +New Conan generators like +[CMakeDeps](https://docs.conan.io/1/reference/conanfile/tools/cmake/cmakedeps.html) +and +[PkgConfigDeps](https://docs.conan.io/1/reference/conanfile/tools/gnu/pkgconfigdeps.html), +don't listen to `cpp_info`'s ``.names``, ``.filenames`` or ``.build_modules`` attributes. +There is a new way of setting the `cpp_info` information with these +generators using the ``set_property(property_name, value)`` method. + +Both of these two models **will live together in recipes** to make +recipes compatible for both 1.x and 2.0 users. Deprecated feilds are not to be removed at this time. + +To understand the impact of these and the relation between different generates, refer to the +[migrating properties](https://docs.conan.io/1/migrating_to_2.0/properties.html) documentation. + +### Translating .names information to cmake_target_name, cmake_module_target_name and cmake_file_name + +The variation of `names` is covered by the [Conan documentation](https://docs.conan.io/1/migrating_to_2.0/properties.html#migrating-from-names-to-cmake-target-name). + +### Translating .filenames information to cmake_file_name, cmake_module_file_name and cmake_find_mode + +As for `filenames`, refer to [this section](https://docs.conan.io/1/migrating_to_2.0/properties.html#migrating-from-filenames-to-cmake-file-name). + +### Translating .build_modules to cmake_build_modules + +The variation of `build_modules` is covered by the [Conan documentation](https://docs.conan.io/1/migrating_to_2.0/properties.html#translating-build-modules-to-cmake-build-modules). + +### PkgConfigDeps + +For migrating, `names` used with `pkg_config`, see [Conan documentation](https://docs.conan.io/1/migrating_to_2.0/properties.html#migration-from-names-to-pkg-config-name) diff --git a/linter/check_layout_src_folder.py b/linter/check_layout_src_folder.py new file mode 100644 index 0000000000000..b7d418884ca45 --- /dev/null +++ b/linter/check_layout_src_folder.py @@ -0,0 +1,44 @@ +from pylint.checkers import BaseChecker +from pylint.interfaces import IAstroidChecker +from astroid import nodes + +WHY_SRC_FOLDER = "Setting the `src_folder` for layouts will help keep an organized and clean workspace when developing recipes locally. " \ + "The extra folder will help ensure there are no collisions between the upstream sources and recipe's exports - which " \ + "also extends to what happens in the cache when creating packages" + + +class LayoutSrcFolder(BaseChecker): + """ + Ensure `src_folder=src` when using built-in layouts + """ + + __implements__ = IAstroidChecker + + name = "conan-layout-src-folder" + msgs = { + "E9012": ( + "layout is missing `src_folder` argument which should be to `src`", + "conan-missing-layout-src-folder", + WHY_SRC_FOLDER, + ), + "E9013": ( + "layout should set `src_folder` to `src`", + "conan-layout-src-folder-is-src", + WHY_SRC_FOLDER, + ), + } + + def visit_call(self, node: nodes.Call) -> None: + if not isinstance(node.func, nodes.Name): + return + + if node.func.name in ["cmake_layout", "bazel_layout", "basic_layout"]: + for kw in node.keywords: + if kw.arg == "src_folder": + if not kw.value or kw.value.as_string().strip("\"'") != "src": + self.add_message( + "conan-layout-src-folder-is-src", node=node, line=node.lineno + ) + break + else: + self.add_message("conan-missing-layout-src-folder", node=node, line=node.lineno) diff --git a/linter/check_package_name.py b/linter/check_package_name.py new file mode 100644 index 0000000000000..fcc8f60447a9b --- /dev/null +++ b/linter/check_package_name.py @@ -0,0 +1,44 @@ +from pylint.checkers import BaseChecker +from pylint.interfaces import IAstroidChecker +from astroid import nodes, Const, AssignName +from pathlib import Path + + +class PackageName(BaseChecker): + """ + All packages must have a lower-case name + """ + + __implements__ = IAstroidChecker + + name = "conan-package-name" + msgs = { + "E9005": ( + "Missing name attribute", + "conan-missing-name", + "The member attribute `name` must be declared: `name = 'foobar'`." + ), + "E9007": ( + "No 'name' attribute in test_package conanfile", + "conan-test-no-name", + "No 'name' attribute in test_package conanfile." + ), + } + + def visit_classdef(self, node: nodes) -> None: + filename = Path(node.root().file) + is_test = filename.match('test_*/*.py') + + if node.basenames == ['ConanFile']: + for attr in node.body: + children = list(attr.get_children()) + if len(children) == 2 and \ + isinstance(children[0], AssignName) and \ + children[0].name == "name" and \ + isinstance(children[1], Const): + if is_test: + self.add_message("conan-test-no-name", node=attr, line=attr.lineno) + return + return + if not is_test: + self.add_message("conan-missing-name", node=node) diff --git a/linter/check_version_attribute.py b/linter/check_version_attribute.py new file mode 100644 index 0000000000000..66e61f47ef183 --- /dev/null +++ b/linter/check_version_attribute.py @@ -0,0 +1,33 @@ +from pylint.checkers import BaseChecker +from pylint.interfaces import IAstroidChecker +from astroid import nodes, Const, AssignName + + +class VersionAttribute(BaseChecker): + """ + All packages should not enforce a specific version in the recipe + """ + + __implements__ = IAstroidChecker + + name = "conan-attr-version" + msgs = { + "E9014": ( + "Recipe should not contain version attribute", + "conan-forced-version", + "Do not enforce a specific version in your recipe. Keep it generic for any version." + ), + } + + def visit_classdef(self, node: nodes) -> None: + if node.basenames == ['ConanFile']: + for attr in node.body: + children = list(attr.get_children()) + if len(children) == 2 and \ + isinstance(children[0], AssignName) and \ + children[0].name == "version" and \ + isinstance(children[1], Const): + value = children[1].as_string().replace('"', "").replace("'", "") + if value and value != "system": + self.add_message("conan-forced-version", node=attr, line=attr.lineno) + return diff --git a/linter/conanv2_transition.py b/linter/conanv2_transition.py new file mode 100644 index 0000000000000..6851a02e40104 --- /dev/null +++ b/linter/conanv2_transition.py @@ -0,0 +1,16 @@ +""" + +Pylint plugin/rules for conanfiles in Conan Center Index + +""" + +from pylint.lint import PyLinter +from linter.check_package_name import PackageName +from linter.check_layout_src_folder import LayoutSrcFolder +from linter.check_version_attribute import VersionAttribute + + +def register(linter: PyLinter) -> None: + linter.register_checker(PackageName(linter)) + linter.register_checker(LayoutSrcFolder(linter)) + linter.register_checker(VersionAttribute(linter)) diff --git a/linter/pylintrc_recipe b/linter/pylintrc_recipe index 5b4db4ad401f0..fda9c3b88a1ad 100644 --- a/linter/pylintrc_recipe +++ b/linter/pylintrc_recipe @@ -1,7 +1,6 @@ [MASTER] load-plugins=linter.conanv2_transition, linter.transform_conanfile, - linter.transform_imports py-version=3.6 recursive=no @@ -18,9 +17,7 @@ disable=fixme, wrong-import-order, # TODO: Remove import-outside-toplevel # TODO: Remove -enable=conan-bad-name, - conan-missing-name, - conan-import-conanfile, +enable=conan-missing-name, conan-forced-version [REPORTS] diff --git a/linter/pylintrc_testpackage b/linter/pylintrc_testpackage index c5cf96e8cadf7..d16ac3631f53f 100644 --- a/linter/pylintrc_testpackage +++ b/linter/pylintrc_testpackage @@ -1,7 +1,6 @@ [MASTER] load-plugins=linter.conanv2_transition, - linter.transform_conanfile, - linter.transform_imports + linter.transform_conanfile py-version=3.6 recursive=no @@ -22,8 +21,7 @@ disable=fixme, conan-missing-layout-src-folder, conan-layout-src-folder-is-src -enable=conan-test-no-name, - conan-import-conanfile +enable=conan-test-no-name [REPORTS] evaluation=max(0, 0 if fatal else 10.0 - ((float(5 * error) / statement) * 10)) diff --git a/linter/transform_conanfile.py b/linter/transform_conanfile.py index 36846a9b3b44b..0daf500ab5c4c 100644 --- a/linter/transform_conanfile.py +++ b/linter/transform_conanfile.py @@ -5,6 +5,8 @@ import textwrap import astroid from astroid.builder import AstroidBuilder +from astroid.builder import extract_node +from astroid.inference_tip import inference_tip from astroid.manager import AstroidManager @@ -41,24 +43,22 @@ def transform_conanfile(node): info_class = astroid.MANAGER.ast_from_module_name("conans.model.info").lookup( "ConanInfo") build_requires_class = astroid.MANAGER.ast_from_module_name( - "conans.client.graph.graph_manager").lookup("_RecipeBuildRequires") - file_copier_class = astroid.MANAGER.ast_from_module_name( - "conans.client.file_copier").lookup("FileCopier") - file_importer_class = astroid.MANAGER.ast_from_module_name( - "conans.client.importer").lookup("_FileImporter") + "conans.model.requires").lookup("BuildRequirements") + tool_requires_class = astroid.MANAGER.ast_from_module_name( + "conans.model.requires").lookup("ToolRequirements") + test_requires_class = astroid.MANAGER.ast_from_module_name( + "conans.model.requires").lookup("TestRequirements") python_requires_class = astroid.MANAGER.ast_from_module_name( "conans.client.graph.python_requires").lookup("PyRequires") dynamic_fields = { "conan_data": str_class, "build_requires": build_requires_class, - "test_requires" : build_requires_class, - "tool_requires": build_requires_class, + "test_requires" : test_requires_class, + "tool_requires": tool_requires_class, "info_build": info_class, "user_info_build": [_user_info_build_transform()], "info": info_class, - "copy": file_copier_class, - "copy_deps": file_importer_class, "python_requires": [str_class, python_requires_class], "recipe_folder": str_class, "settings_build": [_settings_transform()], @@ -73,3 +73,17 @@ def transform_conanfile(node): astroid.MANAGER.register_transform( astroid.ClassDef, transform_conanfile, lambda node: node.qname() == "conans.model.conan_file.ConanFile") + + +def _looks_like_settings(node: astroid.Attribute) -> bool: + return node.attrname == "settings" + +def infer_settings(node, context): + return astroid.MANAGER.ast_from_module_name( + "conans.model.settings").lookup("Settings")[1][0].instantiate_class().infer(context=context) + +astroid.MANAGER.register_transform( + astroid.Attribute, + inference_tip(infer_settings), + _looks_like_settings, +) \ No newline at end of file