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

clang_format fails with GitHub action but passes locally #318

Open
christianrauch opened this issue Dec 1, 2021 · 7 comments
Open

clang_format fails with GitHub action but passes locally #318

christianrauch opened this issue Dec 1, 2021 · 7 comments
Labels
bug Something isn't working

Comments

@christianrauch
Copy link

christianrauch commented Dec 1, 2021

Description

I am using a custom clang-format style and when running the clang_format linter locally (colcon test), it passes without suggested code changes. In the GitHub action, this fails, suggesting changes that should not be suggested according to custom format style.

E.g. the linter on the GitHub action suggests:

Error: -       for (const libcamera::Size &size : formats.sizes(pixelformat))
Error: +       for (const libcamera::Size & size : formats.sizes(pixelformat))

while the format style defines:

PointerAlignment: Right

Expected Behavior

GitHub action should behave the same as locally.

Actual Behavior

clang-format behaviour differs between local installation and GitHub action

To Reproduce

  1. create custom clang-format style
  2. adapt code
  3. test locally
  4. test remote via action-ros2-lint

System (please complete the following information)

  • OS: Ubuntu 20.04
  • ROS 2 Distro: foxy

Additional context

It appears that the local and remove clang-format versions differ:

local clang (Ubuntu repo):

$ clang-format --version
clang-format version 10.0.0-4ubuntu1

ros-tooling/action-ros2-lint:

Unpacking clang-format (1:10.0-50~exp1)
@christianrauch christianrauch added the bug Something isn't working label Dec 1, 2021
@emersonknapp
Copy link
Contributor

emersonknapp commented Dec 1, 2021

What exact command are you running locally to get the result you want?
On what code?
How are you applying your custom style to it?
Do you have a link to a failed workflow run we can refer to that is getting the undesired result?

@christianrauch
Copy link
Author

The repo is at: https://github.com/christianrauch/camera_ros_ci_test. I added compiler errors to the repo to test if the CI fails for them (it does not) but you can see in the latest workflow is https://github.com/christianrauch/camera_ros_ci_test/runs/4375884681?check_suite_focus=true.

The package depends on libcamera. If you are using Ubuntu, you can try my ppa https://launchpad.net/~christianrauch/+archive/ubuntu/libcamera.

To compile and test:

mkdir -p /tmp/ws/src
cd /tmp/ws/src
git clone https://github.com/christianrauch/camera_ros_ci_test.git
cd camera_ros_ci_test
git checkout 1feff5899fc790ca6e63aa8405a570fe6c8141b1 # (this is one commit before master without the added errors)
cd ../..
source /opt/ros/rolling/setup.bash
colcon build
colcon test --event-handlers console_direct+ --packages-select camera_ros

The last command will run the test and they should pass 100% tests passed, 0 tests failed out of 3.

@christianrauch
Copy link
Author

For better comparison, here are two workflows to test the package using the same code base, but different actions:

  1. via action-ros-ci https://github.com/christianrauch/camera_ros_ci_test/actions/workflows/main.yml
  2. via action-ros-lint https://github.com/christianrauch/camera_ros_ci_test/actions/workflows/lint.yml

The action-ros-ci workflow basically uses colcon build & colcon test as above to compile the node and run the tests. This workflow runs as expected. In the action-ros-lint workflow, however, ament_clang_format fails. I would expect that both workflows provide the same test results.

@emersonknapp
Copy link
Contributor

emersonknapp commented Dec 2, 2021

So in the CMake you have

set(ament_cmake_clang_format_CONFIG_FILE "${CMAKE_SOURCE_DIR}/.clang-format")

But action-ros-lint just calls the linter, it isn't using the ament_lint_cmake portion. You'll need to pass it an argument in the workflow configuration, like

      - uses: ros-tooling/[email protected]
        with:
          linter: clang_format
          arguments: SOMETHING_HERE 
          package-name: camera_ros

Not familiar with clang_format directly, but I assume it can take a config file as an input argument somehow

@christianrauch
Copy link
Author

So in the CMake you have [...]

Well, yes. I mentioned that I am using a custom clang-format style. I assumed the lint action would just use the CMake settings and hence see that ament_cmake_clang_format_CONFIG_FILE is set.

But action-ros-lint just calls the linter, it isn't using the ament_lint_cmake portion.

Do you mean the lint action is not using the CMake file at all?

The arguments argument is not documented well. The only reference I found is in https://github.com/ros-tooling/action-ros-lint/blob/master/action.yml. You also have to provide the argument to all linters, but I guess they all take different arguments? E.g. the xmllint likely does not take a .clang-format file.

There should be more documentation and an example of how to configure the action-ros-lint arguments.

@emersonknapp
Copy link
Contributor

Do you mean the lint action is not using the CMake file at all?

Correct, it just invokes the ament_clang_format commandline tool on the specified package directories. It's meant to be a very lightweight pre-check that finishes a lot faster than your full colcon test

There should be more documentation and an example of how to configure the action-ros-lint arguments.

PRs welcome :)

@christianrauch
Copy link
Author

PRs welcome :)

I didn't even know that this feature exists... so I am going to leave this to someone who knows how things work :-)

I will use the action-ros-ci instead, since it also checks for compile errors. But I will leave this issue open so that the documentation issue around setting custom format styles can be addressed by someone with the right knowledge in this area.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants