Skip to content

[#1958] Add configurable Cargo build warning - #1959

Open
jcfr wants to merge 1 commit into
eclipse-iceoryx:mainfrom
jcfr:iox2-1958-configurable-cargo-build-warning
Open

[#1958] Add configurable Cargo build warning#1959
jcfr wants to merge 1 commit into
eclipse-iceoryx:mainfrom
jcfr:iox2-1958-configurable-cargo-build-warning

Conversation

@jcfr

@jcfr jcfr commented Sep 2, 2026

Copy link
Copy Markdown

Notes for Reviewer

This adds the opt-in CMake option IOX2_SUPPRESS_CARGO_BUILD_WARNING. It defaults to OFF, preserving the existing warning unchanged. When enabled, the same diagnostic is emitted as a status message so embedding projects that intentionally use the Cargo source-build path can acknowledge that policy without carrying a downstream patch.

The change affects diagnostic severity only. Cargo target selection, build flags, generated build recipes, artifacts, and installation behavior are unchanged.

No corresponding Bazel change is needed: Bazel builds Rust targets directly through rules_rust and has neither the CMake RUST_BUILD_ARTIFACT_PATH choice nor an equivalent Cargo source-build warning.

Focused validation covered three configurations:

  • Default: configure succeeds and retains the existing warning.
  • IOX2_SUPPRESS_CARGO_BUILD_WARNING=ON: configure succeeds, emits the diagnostic as status, and emits no CMake warning.
  • IOX2_SUPPRESS_CARGO_BUILD_WARNING=ON with RUST_BUILD_ARTIFACT_PATH: configure succeeds and emits neither Cargo source-build diagnostic.

The generated iceoryx2-ffi-c-build-step recipes for the default and opt-in Cargo configurations are identical after normalizing their build-directory paths. git diff --check also passes.

Pre-Review Checklist for the PR Author

  • Add sensible notes for the reviewer
  • PR title is short, expressive and meaningful
  • Consider switching the PR to a draft (Convert to draft)
    • Opening ready for review because the focused configure validation passes.
  • Relevant issues are linked in the References section
  • Branch follows the naming format (iox2-123-introduce-posix-ipc-example)
  • Commits messages are according to this guideline
    • Commit messages have the issue ID ([#123] Add posix ipc example)
    • Keep in mind to use the same email that was used to sign the Eclipse Contributor Agreement
  • Tests follow the best practice for testing
  • Changelog updated in the unreleased section including API breaking changes

PR Reviewer Reminders

  • Commits are properly organized and messages are according to the guideline
  • Unit tests have been written for new behavior
  • Public API is documented
  • PR title describes the changes

References

Closes #1958

@elBoberido elBoberido left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The PR is fine. This option was actually meant to make the life of the developers easier. I still don't fully understand the use case for production build, but that's fine. For example, the CLI tools are not build with the cmake build and one would still need to invoke cargo. But if it makes life easier, then I'm all for merging the PR.

You might need to sign the ECA. If you already did, it might take some time to sync everything and make the CI happy.

@codecov

codecov Bot commented Sep 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.51%. Comparing base (c98c64d) to head (939e12e).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1959      +/-   ##
==========================================
- Coverage   76.53%   76.51%   -0.03%     
==========================================
  Files         452      452              
  Lines       46046    46046              
  Branches     1489     1489              
==========================================
- Hits        35243    35230      -13     
- Misses       9541     9554      +13     
  Partials     1262     1262              
Flag Coverage Δ
CPP 62.61% <ø> (ø)
Rust 76.36% <ø> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 6 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@elBoberido

Copy link
Copy Markdown
Member

@jcfr friendly ping. Did you have time to look into the issue with the ECA?

You also need to rebase the branch in order to fix the merge conflict.

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.

Allow projects to acknowledge an intentional Cargo build from CMake

2 participants