[CI] Composite action and os_name

[christophfroehlich]

• Add a composite action to list packages in the given path (needed for several reusable workflows). I used the public address for linking to the action, because due to the path set in the checkout-action it won't find the proper relative path to the action.yml file.
• Add a reusable workflow for ros-lint
• Add os_name input to ros-tooling (we should give the Tier 1 release for the respective distro)

Both were tested with ros-controls/control_toolbox#187

[christophfroehlich]

I'm wondering if we need this workflow at all.

The claim from https://github.com/ros-tooling/action-ros-lint is not relevant for us, because we don't use this as a pre-condition of the build jobs:

This command does not compile any code intentionally to make it as fast as possible (<2 min). The objective is to give contributors feedback about their change quickly. It may also be used to avoid wasting CI resources (no need to compile, and run the tests if it does not pass the linters).

What is the benefit compared to the pre-commit format job? All four linter are already in the pre-commit config
ament_copyright

ros2_control_ci/.pre-commit-config.yaml

Lines 102 to 110 in aa86661

	# Copyright
	- repo: local
	hooks:
	- id: ament_copyright
	name: ament_copyright
	description: Check if copyright notice is available in all files.
	stages: [commit]
	entry: ament_copyright
	language: system

ament_cmake

ros2_control_ci/.pre-commit-config.yaml

Lines 91 to 100 in aa86661

	# Cmake hooks
	- repo: local
	hooks:
	- id: ament_lint_cmake
	name: ament_lint_cmake
	description: Check format of CMakeLists.txt files.
	stages: [commit]
	entry: ament_lint_cmake
	language: system
	files: CMakeLists\.txt$

ament_cpplint

ros2_control_ci/.pre-commit-config.yaml

Lines 79 to 89 in aa86661

	# Maybe use https://github.com/cpplint/cpplint instead
	- repo: local
	hooks:
	- id: ament_cpplint
	name: ament_cpplint
	description: Static code analysis of C/C++ files.
	stages: [commit]
	entry: ament_cpplint
	language: system
	files: \.(h\+\+|h|hh|hxx|hpp|cuh|c|cc|cpp|cu|c\+\+|cxx|tpp|txx)$
	args: ["--linelength=100", "--filter=-whitespace/newline"]

ament_cppcheck

ros2_control_ci/.pre-commit-config.yaml

Lines 69 to 77 in aa86661

	- repo: local
	hooks:
	- id: ament_cppcheck
	name: ament_cppcheck
	description: Static code analysis of C/C++ files.
	stages: [commit]
	entry: ament_cppcheck
	language: system
	files: \.(h\+\+|h|hh|hxx|hpp|cuh|c|cc|cpp|cu|c\+\+|cxx|tpp|txx)$

[fmauch]

What is the benefit compared to the pre-commit format job? All four linter are already in the pre-commit config
ament_copyright

Yes, we asked ourselves the same question a while back and switched to a pre-commit only linting. Note though that in the current configs the ament linters are run during the commit stage only (probably because of the mentioned duplication?)

Removing the ros-lint workflow also makes maintaining things easier since now two lists of linters have to be updated and best kept in sync from my understanding?

So: +1 for removint the ci-lint job.

[christophfroehlich]

I don't know how to configure pre-commit, what does commit-stage mean in this context? The pre-commit workflow is called on PRs, this is all we want?

[fmauch]

In this sample:

ros2_control_ci/.pre-commit-config.yaml

Lines 102 to 110 in aa86661

	# Copyright
	- repo: local
	hooks:
	- id: ament_copyright
	name: ament_copyright
	description: Check if copyright notice is available in all files.
	stages: [commit]
	entry: ament_copyright
	language: system

stages: [commit] means that this only runs when the "command" invoking pre-commit is a commit action. In the pre-commit ci we do

- uses: pre-commit/[email protected]
      with:
        extra_args: --all-files --hook-stage manual

So only checks that have the manual stage enabled will run. See https://pre-commit.com/#confining-hooks-to-run-at-certain-stages for details.

So, if we remove the ros-lint workflow we would have to update the pre-commit configs by removing that line (or extend it by the manual stage).

[christophfroehlich]

Ah, now I understand. This means that with the current setup, these 4 linters weren't called twice in the CI. (pre-commit output is rather verbose, I wasn't aware of that)

I'd remove the stages: [commit] and all ros-lint jobs from all repos.

[fmauch]

This looks good, thank you!