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

Trouble with ament_cmake_*_ADDITIONAL_EXCLUDE #366

Closed
jtbandes opened this issue Apr 7, 2022 · 4 comments
Closed

Trouble with ament_cmake_*_ADDITIONAL_EXCLUDE #366

jtbandes opened this issue Apr 7, 2022 · 4 comments

Comments

@jtbandes
Copy link

jtbandes commented Apr 7, 2022

In this PR, I'm adding a vendor folder that contains some git submodules. I want to prevent the ament_lint_auto linters from running on the code under vendor, since we don't control that code.

While #343 would be an ideal solution for me, since it is still open, I have been looking for ways to disable individual linters for the vendor directory. I found that some of them have variables to set exclude paths, so I tried this:

  list(APPEND ament_cmake_cpplint_ADDITIONAL_EXCLUDE ${PROJECT_SOURCE_DIR}/vendor)
  list(APPEND ament_cmake_cppcheck_ADDITIONAL_EXCLUDE ${PROJECT_SOURCE_DIR}/vendor)

(I also tried relative paths without the ${PROJECT_SOURCE_DIR})

However I still get lint errors on files in the vendor directory. For example:

$ colcon test --event-handlers=console_direct+

...

test 1
    Start 1: cppcheck

1: Test command: /usr/bin/python3 "-u" "/opt/ros/galactic/share/ament_cmake_test/cmake/run_test.py" "/home/parallels/Desktop/rosbag2_storage_mcap/build/rosbag2_storage_mcap/test_results/rosbag2_storage_mcap/cppcheck.xunit.xml" "--package-name" "rosbag2_storage_mcap" "--output-file" "/home/parallels/Desktop/rosbag2_storage_mcap/build/rosbag2_storage_mcap/ament_cppcheck/cppcheck.txt" "--command" "/opt/ros/galactic/bin/ament_cppcheck" "--xunit-file" "/home/parallels/Desktop/rosbag2_storage_mcap/build/rosbag2_storage_mcap/test_results/rosbag2_storage_mcap/cppcheck.xunit.xml" "--exclude" "/home/parallels/Desktop/rosbag2_storage_mcap/rosbag2_storage_mcap/vendor" "--include_dirs" "/home/parallels/Desktop/rosbag2_storage_mcap/rosbag2_storage_mcap/vendor/mcap/cpp/mcap/include" "/home/parallels/Desktop/rosbag2_storage_mcap/rosbag2_storage_mcap/vendor/lz4/lib" "/home/parallels/Desktop/rosbag2_storage_mcap/rosbag2_storage_mcap/vendor/zstd/lib"
1: Test timeout computed to be: 300
1: -- run_test.py: invoking following command in '/home/parallels/Desktop/rosbag2_storage_mcap/rosbag2_storage_mcap':
1:  - /opt/ros/galactic/bin/ament_cppcheck --xunit-file /home/parallels/Desktop/rosbag2_storage_mcap/build/rosbag2_storage_mcap/test_results/rosbag2_storage_mcap/cppcheck.xunit.xml --exclude /home/parallels/Desktop/rosbag2_storage_mcap/rosbag2_storage_mcap/vendor --include_dirs /home/parallels/Desktop/rosbag2_storage_mcap/rosbag2_storage_mcap/vendor/mcap/cpp/mcap/include /home/parallels/Desktop/rosbag2_storage_mcap/rosbag2_storage_mcap/vendor/lz4/lib /home/parallels/Desktop/rosbag2_storage_mcap/rosbag2_storage_mcap/vendor/zstd/lib
[Processing: rosbag2_storage_mcap]                   
1: [vendor/lz4/programs/util.h:436]: (error: doubleFree) Memory pointed to by 'ptr' is freed twice.
1: [vendor/zstd/contrib/pzstd/ErrorHolder.h:16]: (error: syntaxError) Code 'namespacepzstd{' is invalid C code. Use --std or --language to configure the language.

...

Does anyone have advice on what I might be doing wrong? Is there a way to correctly exclude these files?

@aprotyas
Copy link
Contributor

aprotyas commented Apr 7, 2022

@jtbandes the linters only exclude files if they're provided with relative/absolute paths to the files or with wildcard expressions that expand to the relative/absolute paths to the files. I'm not sure which linters do the latter, but I implemented it for copyright and uncrustify (#326).

Having said that, this worked for me in ros2/geometry2#469:

  set(
    _linter_excludes
    include/tf2/LinearMath/Matrix3x3.h
    include/tf2/LinearMath/MinMax.h
    include/tf2/LinearMath/QuadWord.h
    include/tf2/LinearMath/Quaternion.h
    include/tf2/LinearMath/Scalar.h
    include/tf2/LinearMath/Transform.h
    include/tf2/LinearMath/Vector3.h
  )


  ament_copyright(EXCLUDE ${_linter_excludes})
  ament_cppcheck(
    EXCLUDE ${_linter_excludes}
    LANGUAGE c++
  )
  ament_cpplint(EXCLUDE ${_linter_excludes})
  ament_lint_cmake()
  ament_uncrustify(
    EXCLUDE ${_linter_excludes}
    LANGUAGE c++
  )

This exclusion behavior ought to be documented better. Feel free to open a PR about how to exclude files if you have time!

@jtbandes
Copy link
Author

jtbandes commented Apr 7, 2022

Hmm, thanks for the ideas. I tried this:

  find_package(ament_cmake_cppcheck REQUIRED)
  list(APPEND AMENT_LINT_AUTO_EXCLUDE
    ament_cmake_uncrustify
    ament_cmake_copyright
    ament_cmake_cpplint
    ament_cmake_cppcheck
  )
  file(GLOB_RECURSE _linter_excludes ${PROJECT_SOURCE_DIR}/vendor *)
  ament_cppcheck(EXCLUDE ${_linter_excludes})

But for some reason seems to put most of the files before --exclude, and only one after:

1: - /opt/ros/galactic/bin/ament_cppcheck --xunit-file /home/parallels/Desktop/rosbag2_storage_mcap/build/rosbag2_storage_mcap/test_results/rosbag2_storage_mcap/cppcheck.xunit.xml /home/parallels/Desktop/rosbag2_storage_mcap/rosbag2_storage_mcap/CHANGELOG.rst /home/parallels/Desktop/rosbag2_storage_mcap/rosbag2_storage_mcap/CMakeLists.txt /home/parallels/Desktop/rosbag2_storage_mcap/rosbag2_storage_mcap/package.xml /home/parallels/Desktop/rosbag2_storage_mcap/rosbag2_storage_mcap/plugin_description.xml /home/parallels/Desktop/rosbag2_storage_mcap/rosbag2_storage_mcap/src/mcap_storage.cpp /home/parallels/Desktop/rosbag2_storage_mcap/rosbag2_storage_mcap/src/message_definition_cache.cpp /home/parallels/Desktop/rosbag2_storage_mcap/rosbag2_storage_mcap/src/message_definition_cache.hpp /home/parallels/Desktop/rosbag2_storage_mcap/rosbag2_storage_mcap/vendor/lz4/.circleci/config.yml
...a bunch more files...
/home/parallels/Desktop/rosbag2_storage_mcap/rosbag2_storage_mcap/vendor/zstd/zlibWrapper/zstd_zlibwrapper.h
--exclude /home/parallels/Desktop/rosbag2_storage_mcap/rosbag2_storage_mcap/.clang-format

Any idea why? Am I misusing the EXCLUDE param?

@aprotyas
Copy link
Contributor

aprotyas commented Apr 7, 2022

No, what you're doing is correct.

File exclusion was fixed for ament_cmake_cppcheck in #329, but it hasn't been backported to Galactic. I just opened a backport PR: #367.

@jtbandes
Copy link
Author

jtbandes commented Apr 8, 2022

Thanks for the tips. I was able to work around the issue by manually passing a file list to each linter command: ros-tooling/rosbag2_storage_mcap@03bd8c0

@jtbandes jtbandes closed this as completed Apr 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants