Skip to content

Fix file exclusion behavior in ament_cppcheck and ament_cpplint#299

Merged
mm318 merged 3 commits intoament:masterfrom
mm318:cppcheck_cpplint_exclude
Feb 22, 2021
Merged

Fix file exclusion behavior in ament_cppcheck and ament_cpplint#299
mm318 merged 3 commits intoament:masterfrom
mm318:cppcheck_cpplint_exclude

Conversation

@mm318
Copy link
Copy Markdown
Member

@mm318 mm318 commented Feb 14, 2021

Closes #295

This pull request fixes the issue described at #234 (comment) and possibly replaces #238.

This pull request also makes the behavior between ament_cppcheck and ament_cpplint consistent.

Signed-off-by: Miaofei <miaofei@amazon.com>
Signed-off-by: Miaofei <miaofei@amazon.com>
@mm318
Copy link
Copy Markdown
Member Author

mm318 commented Feb 14, 2021

CI run with build --packages-up-to ament_cmake_cpplint ament_cmake_cppcheck and test --packages-select ament_cmake_cpplint ament_cmake_cppcheck ament_cpplint ament_cppcheck, because it seems like there's currently no core packages (under ros2.repos) that is using the ament_cpplint(... EXCLUDE ...) and ament_cppcheck(... EXCLUDE ...) features.

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@mm318 mm318 requested a review from ahcorde February 17, 2021 08:53
@mm318 mm318 self-assigned this Feb 17, 2021
@mm318 mm318 requested a review from audrow February 19, 2021 17:59
Copy link
Copy Markdown
Contributor

@audrow audrow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

Signed-off-by: Miaofei <miaofei@amazon.com>
@mm318
Copy link
Copy Markdown
Member Author

mm318 commented Feb 22, 2021

Rerunning CI after fix

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@mm318 mm318 merged commit d17140b into ament:master Feb 22, 2021
aprotyas added a commit to aprotyas/ament_lint that referenced this pull request Oct 17, 2021
This commit fixes the faulty file exclusion behavior reported in
ament#326.

Specifically, it walks down the path where `ament_copyright` has
been invoked to build a map of files to be tested, respecting
the exclusion list by globbing and matching against files in the
tree.

Changes inspired by ament#299.

Signed-off-by: Abrar Rahman Protyasha <aprotyas@u.rochester.edu>
aprotyas added a commit to aprotyas/ament_lint that referenced this pull request Oct 17, 2021
This commit fixes the faulty file exclusion behavior reported in
ament#326.

Specifically, the exclusion list is matched against traversed
files in the `crawler` module.

Changes inspired by ament#299.

Signed-off-by: Abrar Rahman Protyasha <aprotyas@u.rochester.edu>
kenji-miyake pushed a commit to kenji-miyake/ament_lint that referenced this pull request Oct 20, 2021
…t#299)

* fix exclude behavior in ament_cppcheck and ament_cpplint

Signed-off-by: Miaofei <miaofei@amazon.com>

* fix flake8 errors

Signed-off-by: Miaofei <miaofei@amazon.com>

* add missing realpath() conversion

Signed-off-by: Miaofei <miaofei@amazon.com>
audrow pushed a commit that referenced this pull request Nov 4, 2021
* [ament_copyright] Fix file exclusion behavior

This commit fixes the faulty file exclusion behavior reported in
#326.

Specifically, the exclusion list is matched against traversed
files in the `crawler` module.

Changes inspired by #299.

Signed-off-by: Abrar Rahman Protyasha <aprotyas@u.rochester.edu>

* Update excluded file path in copyright tests

Since file names are not indiscriminately matched throughout the
search tree anymore, the excluded files listed in the copyright
tests need to be updated relative to the root of the package.

Signed-off-by: Abrar Rahman Protyasha <aprotyas@u.rochester.edu>

* Add test cases to check exclusion behavior

Specifically, these tests check for:

- Incorrect exclusion of single filenames.
- Correct exclusion of relatively/absolutely addressed filenames.
- Correct exclusion of wildcarded paths.

Signed-off-by: Abrar Rahman Protyasha <aprotyas@u.rochester.edu>

* Add unit tests for crawler module

These unit tests make sure both search and exclusion behaviors are
correctly demonstrated by the `ament_copyright.crawler` module.

Signed-off-by: Abrar Rahman Protyasha <aprotyas@u.rochester.edu>
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

Successfully merging this pull request may close these issues.

ament_cpplint doesn't support EXCLUDE parameter

2 participants