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

restore python linters #25360

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
124 changes: 124 additions & 0 deletions .github/workflows/linter-conan-v2.yml
Original file line number Diff line number Diff line change
@@ -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
83 changes: 82 additions & 1 deletion docs/linters.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)<!-- endToc -->
* [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)<!-- endToc -->

## Understanding the different linters

Expand All @@ -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!
```
147 changes: 147 additions & 0 deletions docs/v2_migration.md
Original file line number Diff line number Diff line change
@@ -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.

<!-- toc -->
## 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)<!-- endToc -->

> **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.<recipe_name>:<variable>`.

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)
Loading
Loading