-
Notifications
You must be signed in to change notification settings - Fork 108
Refactor bazel rules to make the build more hermetic #1264
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
base: main
Are you sure you want to change the base?
Conversation
8259227 to
5b62d66
Compare
5b62d66 to
000f22a
Compare
|
@elfenpiff / @elBoberido ready for review! I was unable to assign a PR reviewer or assign myself as an assignee, so I'm mentioning you here as an alternative |
|
@alextac98 thanks for the bazel contribution :) You need to run We'll have the iceoryx2 v0.8.0 release in a few days and we try to limit the changes before the release. We'll have a closer look at it but we will likely only merge this after the release is created. Is your intention to get this into v0.8.0? Also, if it does not take much effort, could you do a rebase to main instead of a merge? |
f6edb7e to
8c7bded
Compare
|
@elBoberido all comments fixed! It would be super ideal if we could merge it into v0.8.0, but I totally understand if it is a bit close to release. We were hoping to use a stable release once these changes make it in, but weren't aware there was a release happening so soon. I also added a hotfix to a bazel rule from a recent change, and tagged it to #301 |
|
@alextac98 good catch with the header-only library. I would cherry-pick that commit for the release if this PR does not get in. Or you could create a PR with just that commit. It would simplify the work on this PR since the CI will run automatically once a contributor has a merged PR. |
|
I'll separate it out then to make things faster |
|
We need to consider that this potentially break the Workspace build, not sure if want to continue with support on that. |
|
At least for the foreseeable future, we need to support bazel 7.1 and also workspaces |
|
I'll work on making it work for Workspace build as well then! Will likely go past tomorrow's release, so I'm happy to push this off to next release. |
|
@alextac98 give us a few days to check if we want to support Workspaces for our 0.9 release. Btw, since your first PR was merged, the CI on this PR will run each time you push you changes |
8c7bded to
bbd5193
Compare
|
Oh, I forgot to tell you that the CI does not run on draft PRs. You can mark it ready for review and add a |
Notes for Reviewer
Here are the major changes:
rules_rust_bindgencargoandrustcbinaries to make the bazel build more hermetic.Additional note, these are based on changes necessary for us to integrate Iceoryx2 into our bazel project at Inversion Space. I used that as a testing environment.
Pre-Review Checklist for the PR Author
Convert to draft)iox2-123-introduce-posix-ipc-example)[#123] Add posix ipc example)task-list-completed)PR Reviewer Reminders
References
Closes #1263
hotfix for #301