search

eclipse-iceoryx / iceoryx2

Eclipse iceoryx2™ - true zero-copy inter-process-communication in pure Rust
https://iceoryx.io
Apache License 2.0
1.03k stars 40 forks source link

[#486] support bazel in macos #487

Open xieyuschen opened 4 weeks ago

xieyuschen commented 4 weeks ago

Notes for Reviewer

Currently the bazel doesn't work in my desktop, so i want to support it in this PR/

Pre-Review Checklist for the PR Author

  1. [X] Add sensible notes for the reviewer
  2. [X] PR title is short, expressive and meaningful
  3. [X] Relevant issues are linked in the References section
  4. [X] Every source code file has a copyright header with SPDX-License-Identifier: Apache-2.0 OR MIT
  5. [X] Branch follows the naming format (iox2-123-introduce-posix-ipc-example)
  6. [X] Commits messages are according to this guideline
    • [X] Commit messages have the issue ID ([#123] Add posix ipc example)
    • [X] Commit author matches Eclipse Contributor Agreement (and ECA is signed)
  7. [X] Tests follow the best practice for testing
  8. [X] Changelog updated in the unreleased section including API breaking changes
  9. [ ] Assign PR to reviewer
  10. [ ] All checks have passed (except task-list-completed)

Checklist for the PR Reviewer

Post-review Checklist for the PR Author

  1. [ ] All open points are addressed and tracked via issues

References

Closes #486

codecov[bot] commented 4 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 79.21%. Comparing base (5f6a0dc) to head (2649f37).

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/eclipse-iceoryx/iceoryx2/pull/487/graphs/tree.svg?width=650&height=150&src=pr&token=FN3YFXTJCI&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=eclipse-iceoryx)](https://app.codecov.io/gh/eclipse-iceoryx/iceoryx2/pull/487?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=eclipse-iceoryx) ```diff @@ Coverage Diff @@ ## main #487 +/- ## ========================================== + Coverage 79.19% 79.21% +0.02% ========================================== Files 200 200 Lines 23716 23716 ========================================== + Hits 18781 18787 +6 + Misses 4935 4929 -6 ``` [see 9 files with indirect coverage changes](https://app.codecov.io/gh/eclipse-iceoryx/iceoryx2/pull/487/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=eclipse-iceoryx)
xieyuschen commented 4 weeks ago

@elfenpiff @elBoberido could you kindly help to review it?

orecham commented 4 weeks ago

Builds on my mac (examples work). Nice :)

I am no bazel wizard so I leave the details to @elBoberido

orecham commented 4 weeks ago

@xieyuschen BTW feel free to add yourself to the contributors section of the README if you like. You've contributed a lot of great improvements.

xieyuschen commented 3 weeks ago

@xieyuschen BTW feel free to add yourself to the contributors section of the README if you like. You've contributed a lot of great improvements.

thanks for your reminder, and i have added me inside the contributor section:)

xieyuschen commented 3 weeks ago

hi @elBoberido , could you kindly review it when you have time? Thanks:)

elBoberido commented 3 weeks ago

@xieyuschen I'm a bit hesitant to add changes in the bazel setup. We have users with bazel 6.2 and it took us quite some time to get this working in their setup. Adding macOS into the bazel basket will add more load on the project when something in bazel breaks. That's also the reason we removed bazel for Windows. For now, we will only support bazel on Linux until we have a better understanding of the build system.

Sorry if you put effort into this and it's not going to be merged soon. You can keep this PR open and we will revisit it some time later.

xieyuschen commented 3 weeks ago

hi @elBoberido , understood your concerns. I don't mind to put it here until we have a better understanding on bazel. I think the change here is straightforward as it allows the downloaded tools to be compatible with platform:)

Looks like I need to cherry-pick this commit when I need use bazel in my macos:(

Thanks for your review!

elBoberido commented 3 weeks ago

@xieyuschen are you actually using bazel on macOS? I had the impression you did it mainly to test the idea with the feature flag. If you intend to use iceoryx2 with bazel on macOS, this would of course change the situation.

xieyuschen commented 3 weeks ago

Hi @elBoberido currently I use bazel for the sake of testing feature, and no real usage because I haven't saw the limitations of cargo. I don't have a strong requirement now. Thanks

elBoberido commented 3 weeks ago

@xieyuschen okay, the we will review this for the v0.6 dev cycle. cargo will be our main build tool anyway.