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 #1323 remove cpptoml patch and fix cmake 3.23 warning for release 1.0 #1326

Conversation

elBoberido
Copy link
Member

@elBoberido elBoberido commented Apr 8, 2022

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
    • Commit messages have the issue ID (iox-#123 commit text)
    • Commit messages are signed (git commit -s)
    • Commit author matches Eclipse Contributor Agreement (and ECA is signed)
  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

Notes for Reviewer

Backport of #1325 and #1310 to release 1.0 branch

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
    • Each unit test case has a unique UUID
  • Public API changes are documented via doxygen
  • Copyright owner are updated in the changed files
  • PR title describes the changes

Post-review Checklist for the PR Author

  1. All open points are addressed and tracked via issues

References

clalancette and others added 3 commits April 8, 2022 13:49
When building iceoryx_posh with CMake 3.23, it throws a warning:

Ignoring empty string ("") provided on the command line.

This happens because when CMAKE_TOOLCHAIN_FILE is not
defined, TOOLCHAIN_FILE is also not defined and thus is empty.
Workaround this new behavior by setting a variable that contains
a bunch of things, and may also *optionally* include the
define for the TOOLCHAIN file, which removes the warning.

Signed-off-by: Chris Lalancette <[email protected]>
@elBoberido elBoberido added the bugfix Solves a bug label Apr 8, 2022
@elBoberido elBoberido self-assigned this Apr 8, 2022
@codecov
Copy link

codecov bot commented Apr 8, 2022

Codecov Report

Merging #1326 (e3c1c4a) into release_1.0 (18a3198) will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##           release_1.0    #1326      +/-   ##
===============================================
+ Coverage        73.96%   73.98%   +0.01%     
===============================================
  Files              319      319              
  Lines            11425    11425              
  Branches          1972     1972              
===============================================
+ Hits              8451     8453       +2     
  Misses            2196     2196              
+ Partials           778      776       -2     
Flag Coverage Δ
unittests 72.77% <ø> (ø)
unittests_timing 30.51% <ø> (-0.15%) ⬇️

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

Impacted Files Coverage Δ
...h/source/roudi/roudi_config_toml_file_provider.cpp 62.50% <ø> (ø)
iceoryx_utils/source/concurrent/loffli.cpp 79.41% <0.00%> (-5.89%) ⬇️
...nternal/roudi/introspection/port_introspection.inl 71.89% <0.00%> (-0.73%) ⬇️
iceoryx_posh/source/roudi/process_manager.cpp 61.70% <0.00%> (+0.30%) ⬆️
iceoryx_posh/source/roudi/port_manager.cpp 78.52% <0.00%> (+0.32%) ⬆️
..._utils/source/posix_wrapper/unix_domain_socket.cpp 56.88% <0.00%> (+0.45%) ⬆️
iceoryx_utils/source/posix_wrapper/timer.cpp 60.52% <0.00%> (+0.87%) ⬆️
...ils/include/iceoryx_utils/internal/cxx/smart_c.inl 89.06% <0.00%> (+1.56%) ⬆️

@dkroenke
Copy link
Member

dkroenke commented Apr 8, 2022

@clalancette @Blast545

We add here the bugfix for cpptoml to prevent another CMake warning from #1310
for the case that CMake 3.23 is used on galactic.

ROS CI build (ROS Galactic, Ubuntu Focal, packages-up-to rmw_cyclonedds_cpp)

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

The question here is if we are allowed to make changes in galactic since it is already released.

@Blast545
Copy link

Blast545 commented Apr 8, 2022

@dkroenke AFAIK changes are allowed as long as they don't break API/ABI:
https://docs.ros.org/en/rolling/How-To-Guides/Package-maintainer-guide.html#backporting-to-released-distributions

@elBoberido elBoberido merged commit 9449618 into eclipse-iceoryx:release_1.0 Apr 8, 2022
@elBoberido elBoberido deleted the iox-#1323-remove-cpptoml-patch-release-1.0 branch April 8, 2022 16:15
@dkroenke dkroenke mentioned this pull request Apr 8, 2022
21 tasks
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.

Double build issue in ROS2 build farm CMake warning: empty command-line option
5 participants