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-#2072 Include mqueue.h needs sys/stat.h for mode_t (fixes musl compile) #2073

Conversation

pseiderer
Copy link
Contributor

@pseiderer pseiderer commented Nov 1, 2023

As stated in the mq_open man page ([1]) the mqueue.h include needs additional sys/stat.h include for mode_t definition.

Fixes musl compile:

In file included from .../build/iceoryx-custom/iceoryx_platform/linux/source/mqueue.cpp:17:
.../build/iceoryx-custom/iceoryx_platform/linux/include/iceoryx_platform/mqueue.hpp:23:49: error: ‘mode_t’ has not been declared
23 | mqd_t iox_mq_open4(const char* name, int oflag, mode_t mode, struct mq_attr* attr);
| ^~~~~~
.../build/iceoryx-custom/iceoryx_platform/linux/source/mqueue.cpp:26:49: error: ‘mode_t’ has not been declared
26 | mqd_t iox_mq_open4(const char* name, int oflag, mode_t mode, struct mq_attr* attr)
| ^~~~~~

[1] https://man7.org/linux/man-pages/man3/mq_open.3.html

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. All touched (C/C++) source code files from iceoryx_hoofs are added to ./clang-tidy-diff-scans.txt
  11. Assign PR to reviewer

Notes for Reviewer

Checklist for the PR Reviewer

  • 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 added the bugfix Solves a bug label Nov 1, 2023
elBoberido
elBoberido previously approved these changes Nov 1, 2023
Copy link
Member

@elBoberido elBoberido left a comment

Choose a reason for hiding this comment

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

If you did not yet sign the ECA please do it, else the CI prevents to merge the PR. If you already signed it, it might take a day or two to sync.

Thanks for your contribution

@elBoberido
Copy link
Member

Oh, it seems you need to run clang-format or manually sort the includes in alphabetical order

Copy link

codecov bot commented Nov 1, 2023

Codecov Report

Merging #2073 (296c055) into master (8bc6075) will increase coverage by 0.03%.
Report is 41 commits behind head on master.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2073      +/-   ##
==========================================
+ Coverage   80.09%   80.13%   +0.03%     
==========================================
  Files         416      417       +1     
  Lines       16026    16055      +29     
  Branches     2250     2255       +5     
==========================================
+ Hits        12836    12865      +29     
- Misses       2384     2385       +1     
+ Partials      806      805       -1     
Flag Coverage Δ
unittests 79.91% <ø> (+0.03%) ⬆️
unittests_timing 15.29% <ø> (-0.03%) ⬇️

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

see 60 files with indirect coverage changes

@elBoberido
Copy link
Member

@pseiderer Okay, now the CI is fine. Would you please also add the fixed issue to the changelog? The link is in the PR template.

Once that is done we just have to wait for the ECA check to pass

… (fixes musl compile)

As stated in the mq_open man page ([1]) the mqueue.h include needs
additional sys/stat.h include for mode_t definition.

Fixes musl compile:

  In file included from .../build/iceoryx-custom/iceoryx_platform/linux/source/mqueue.cpp:17:
  .../build/iceoryx-custom/iceoryx_platform/linux/include/iceoryx_platform/mqueue.hpp:23:49: error: ‘mode_t’ has not been declared
     23 | mqd_t iox_mq_open4(const char* name, int oflag, mode_t mode, struct mq_attr* attr);
        |                                                 ^~~~~~
  .../build/iceoryx-custom/iceoryx_platform/linux/source/mqueue.cpp:26:49: error: ‘mode_t’ has not been declared
     26 | mqd_t iox_mq_open4(const char* name, int oflag, mode_t mode, struct mq_attr* attr)
        |                                                 ^~~~~~

[1] https://man7.org/linux/man-pages/man3/mq_open.3.html

Signed-off-by: Peter Seiderer <[email protected]>
@pseiderer pseiderer force-pushed the ps-devel-iox-2072-fix-musl-compile-001 branch from e983806 to 296c055 Compare November 5, 2023 10:33
Copy link
Member

@elBoberido elBoberido 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 now. Thanks for your contribution

@FerdinandSpitzschnueffler FerdinandSpitzschnueffler merged commit bf42822 into eclipse-iceoryx:master Nov 6, 2023
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Solves a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compile with musl libc fails: error: 'mode_t' has not been declared
3 participants