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

iox-#1176 Make ACL support optional #2359

Merged

Conversation

elBoberido
Copy link
Member

@elBoberido elBoberido commented Oct 13, 2024

Notes for Reviewer

This PR makes the adds a compile time option to disable the ACL feature, even on supported OSes. This is useful in cases WSL (Windows Subsystem for Linux) which does not support ACLs but is identified as Linux.

The PR also adds the feature flag for bazel builds.

The feature is deactivated by using the same dummy implementation like it is done on platforms that do not support ACLs.

To reduce code duplication. The platform files for ACSs are also refactored with the new C++17 approach to conditionally include headers.

Furthermore, it fixes a cache issue with the Cirrus CI and add a link to the link checker ignore list due to repeatedly Too Many Requests errors in the CI.

Pre-Review Checklist for the PR Author

  1. Code follows the coding style of CONTRIBUTING.md
  2. Tests follow the best practice for testing
  3. Changelog updated in the unreleased section including API breaking changes
  4. Branch follows the naming format (iox-123-this-is-a-branch)
  5. Commits messages are according to this guideline
  6. Update the PR title
    • Follow the same conventions as for commit messages
    • Link to the relevant issue
  7. Relevant issues are linked
  8. Add sensible notes for the reviewer
  9. All checks have passed (except task-list-completed)
  10. Assign PR to reviewer

Checklist for the PR Reviewer

  • Consider a second reviewer for complex new features or larger refactorings
  • Commits are properly organized and messages are according to the guideline
  • Code according to our coding style and naming conventions
  • Unit tests have been written for new behavior
  • Public API changes are documented via doxygen
  • Copyright owner are updated in the changed files
  • All touched (C/C++) source code files from iceoryx_hoofs have been added to ./clang-tidy-diff-scans.txt
  • PR title describes the changes

Post-review Checklist for the PR Author

  1. All open points are addressed and tracked via issues

References

@elBoberido elBoberido force-pushed the iox-1176-make-acl-support-optional branch 2 times, most recently from 046203d to e597773 Compare October 13, 2024 20:22
@elBoberido elBoberido force-pushed the iox-1176-make-acl-support-optional branch from e597773 to aa51f20 Compare October 13, 2024 22:12
Copy link

codecov bot commented Oct 13, 2024

Codecov Report

Attention: Patch coverage is 82.35294% with 6 lines in your changes missing coverage. Please review.

Project coverage is 78.32%. Comparing base (89d73e0) to head (4356597).
Report is 11 commits behind head on main.

Files with missing lines Patch % Lines
iceoryx_platform/generic/source/acl_feature_on.cpp 75.00% 6 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2359   +/-   ##
=======================================
  Coverage   78.32%   78.32%           
=======================================
  Files         439      440    +1     
  Lines       16162    16186   +24     
  Branches     2314     2314           
=======================================
+ Hits        12659    12678   +19     
- Misses       2652     2658    +6     
+ Partials      851      850    -1     
Flag Coverage Δ
unittests 78.15% <82.35%> (+<0.01%) ⬆️
unittests_timing 15.05% <0.00%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...ceoryx_hoofs/posix/filesystem/source/posix_acl.cpp 65.54% <100.00%> (ø)
iceoryx_platform/generic/source/acl_feature_on.cpp 75.00% <75.00%> (ø)

... and 2 files with indirect coverage changes

@elBoberido elBoberido force-pushed the iox-1176-make-acl-support-optional branch 2 times, most recently from 275eb95 to 23fd506 Compare October 14, 2024 17:20
@elBoberido elBoberido force-pushed the iox-1176-make-acl-support-optional branch from 23fd506 to 5b7b477 Compare October 14, 2024 21:09
@elBoberido elBoberido self-assigned this Oct 14, 2024
@elBoberido elBoberido force-pushed the iox-1176-make-acl-support-optional branch from 3ac6839 to 4d47226 Compare October 14, 2024 23:20
@elBoberido elBoberido changed the title [WIP] iox-#1176 Make ACL support optional iox-#1176 Make ACL support optional Oct 14, 2024
elfenpiff
elfenpiff previously approved these changes Oct 15, 2024
bazel build --//:feature_acl=off //...
```

Alternatively, you can persist this setting in a `.bazelrc` file to apply it automatically in all builds:
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have in iceoryx2 a .bazelrc file where we can add this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Every project has a local .bazelrc and the flag needs to be set in the respective user project

@elBoberido elBoberido merged commit f88bc9c into eclipse-iceoryx:main Oct 15, 2024
27 checks passed
@elBoberido elBoberido deleted the iox-1176-make-acl-support-optional branch October 15, 2024 12:45
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.

Make ACL support optional
2 participants