Enable bzlmod upstream #1179
Open
Enable bzlmod upstream #1179
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
add --noincompatible_disallow_empty_glob so empty globs dont error in bzlmod export CARGO_HOME=/tmp/cargo so bindgen download succeed in sandbox ensure the bindgen/cbindgen repos are imported configure crate_universe using the crate module extension with the same manifest as before so rust dependencies keep working under bzlmod add bazel/extensions.bzl for bindgen and cbindgen. This lets MODULE.bazel instantiate those components via use_extension. update memory_mapping_tests.rs so that it works with sandboxed filesystem. Signed-off-by: Nicolae Dicu <[email protected]>
Signed-off-by: Nicolae Dicu <[email protected]>
nicu1989
force-pushed
the
nicu1989_enable_bzlmod_upstream
branch
from
0b50860 to
a62e595
Compare
dkroenke
reviewed
Nov 13, 2025
Member
dkroenke
left a comment
dkroenke
left a comment
There was a problem hiding this comment.
Thanks for the contribution! There are only minor remarks to do.
I will next review eclipse-iceoryx/iceoryx#2505 which needs to be merged first.
Comment on lines
+1
to
+12
| # ******************************************************************************* | ||
| # Copyright (c) 2025 Contributors to the Eclipse Foundation | ||
| # | ||
| # See the NOTICE file(s) distributed with this work for additional | ||
| # information regarding copyright ownership. | ||
| # | ||
| # This program and the accompanying materials are made available under the | ||
| # terms of the Apache License Version 2.0 which is available at | ||
| # https://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
| # ******************************************************************************* No newline at end of file |
Member
There was a problem hiding this comment.
To make the preflight-check pass:
Suggested change
| # ******************************************************************************* | |
| # Copyright (c) 2025 Contributors to the Eclipse Foundation | |
| # | |
| # See the NOTICE file(s) distributed with this work for additional | |
| # information regarding copyright ownership. | |
| # | |
| # This program and the accompanying materials are made available under the | |
| # terms of the Apache License Version 2.0 which is available at | |
| # https://www.apache.org/licenses/LICENSE-2.0 | |
| # | |
| # SPDX-License-Identifier: Apache-2.0 | |
| # ******************************************************************************* | |
| # Copyright (c) 2025 Contributors to the Eclipse Foundation | |
| # | |
| # See the NOTICE file(s) distributed with this work for additional | |
| # information regarding copyright ownership. | |
| # | |
| # This program and the accompanying materials are made available under the | |
| # terms of the Apache Software License 2.0 which is available at | |
| # https://www.apache.org/licenses/LICENSE-2.0, or the MIT license | |
| # which is available at https://opensource.org/licenses/MIT. | |
| # | |
| # SPDX-License-Identifier: Apache-2.0 OR MIT |
Comment on lines
+1
to
+12
| # ******************************************************************************* | ||
| # Copyright (c) 2025 Contributors to the Eclipse Foundation | ||
| # | ||
| # See the NOTICE file(s) distributed with this work for additional | ||
| # information regarding copyright ownership. | ||
| # | ||
| # This program and the accompanying materials are made available under the | ||
| # terms of the Apache License Version 2.0 which is available at | ||
| # https://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
| # ******************************************************************************* |
Member
There was a problem hiding this comment.
To make the preflight-check pass:
Suggested change
| # ******************************************************************************* | |
| # Copyright (c) 2025 Contributors to the Eclipse Foundation | |
| # | |
| # See the NOTICE file(s) distributed with this work for additional | |
| # information regarding copyright ownership. | |
| # | |
| # This program and the accompanying materials are made available under the | |
| # terms of the Apache License Version 2.0 which is available at | |
| # https://www.apache.org/licenses/LICENSE-2.0 | |
| # | |
| # SPDX-License-Identifier: Apache-2.0 | |
| # ******************************************************************************* | |
| # Copyright (c) 2025 Contributors to the Eclipse Foundation | |
| # | |
| # See the NOTICE file(s) distributed with this work for additional | |
| # information regarding copyright ownership. | |
| # | |
| # This program and the accompanying materials are made available under the | |
| # terms of the Apache Software License 2.0 which is available at | |
| # https://www.apache.org/licenses/LICENSE-2.0, or the MIT license | |
| # which is available at https://opensource.org/licenses/MIT. | |
| # | |
| # SPDX-License-Identifier: Apache-2.0 OR MIT |
| build --noincompatible_disallow_empty_glob | ||
|
|
||
| build --action_env=CARGO_BAZEL_REPIN=true | ||
| build --action_env=CARGO_HOME=/tmp/cargo |
Member
There was a problem hiding this comment.
That is normally not needed.
Suggested change
| build --action_env=CARGO_HOME=/tmp/cargo |
Member
There was a problem hiding this comment.
Maybe this file should more express what is inside (definitions of dependencies for building iceoryx2).
What about naming it iceoryx2_deps.bzl?
| ############################################################################### | ||
| module( | ||
| name = "iceoryx2", | ||
| version = "0.1.0", |
Member
There was a problem hiding this comment.
Suggested change
| version = "0.1.0", | |
| version = "0.7.0", |
Member
There was a problem hiding this comment.
The script to update the iceoryx2 version also needs to be adjusted.
Member
There was a problem hiding this comment.
We can add something like this to internal/scripts/update_versions.sh line 152:
sed -i -z 's/name = "iceoryx2",\n version = "'"${OLD_VERSION}"'"/name = "iceoryx2",\n version = "'"${NEW_VERSION}"'"/g' \
MODULE.bazel
Comment on lines
+33
to
+37
| git_override( | ||
| module_name = "iceoryx", | ||
| commit = "6760c0088c61e8bf68070e2d163a37caab7515b1", | ||
| remote = "https://github.com/qorix-group/iceoryx.git", | ||
| ) |
Member
There was a problem hiding this comment.
We need to change this to upstream once eclipse-iceoryx/iceoryx#2505 got merged
Member
There was a problem hiding this comment.
Once #301 is completed, likely in the next two weeks, this is superfluous and can be removed.
Comment on lines
+20
to
+41
| def _iceoryx2_extra_deps(): | ||
| maybe( | ||
| repo_rule = http_archive, | ||
| name = "bindgen", | ||
| sha256 = "b7e2321ee8c617f14ccc5b9f39b3a804db173ee217e924ad93ed16af6bc62b1d", | ||
| strip_prefix = "bindgen-cli-x86_64-unknown-linux-gnu", | ||
| urls = ["https://github.com/rust-lang/rust-bindgen/releases/download/v0.69.5/bindgen-cli-x86_64-unknown-linux-gnu.tar.xz"], | ||
| build_file_content = """ | ||
| filegroup( | ||
| name = "bindgen-cli", | ||
| srcs = ["bindgen"], | ||
| visibility = ["//visibility:public"], | ||
| ) | ||
| """, | ||
| ) | ||
| maybe( | ||
| repo_rule = http_file, | ||
| name = "cbindgen", | ||
| sha256 = "521836d00863cb129283054e5090eb17563614e6328b7a1610e30949a05feaea", | ||
| urls = ["https://github.com/mozilla/cbindgen/releases/download/0.26.0/cbindgen"], | ||
| executable = True, | ||
| ) |
Member
There was a problem hiding this comment.
Optional: We could make the version numbers better configurable.
Suggested change
| def _iceoryx2_extra_deps(): | |
| maybe( | |
| repo_rule = http_archive, | |
| name = "bindgen", | |
| sha256 = "b7e2321ee8c617f14ccc5b9f39b3a804db173ee217e924ad93ed16af6bc62b1d", | |
| strip_prefix = "bindgen-cli-x86_64-unknown-linux-gnu", | |
| urls = ["https://github.com/rust-lang/rust-bindgen/releases/download/v0.69.5/bindgen-cli-x86_64-unknown-linux-gnu.tar.xz"], | |
| build_file_content = """ | |
| filegroup( | |
| name = "bindgen-cli", | |
| srcs = ["bindgen"], | |
| visibility = ["//visibility:public"], | |
| ) | |
| """, | |
| ) | |
| maybe( | |
| repo_rule = http_file, | |
| name = "cbindgen", | |
| sha256 = "521836d00863cb129283054e5090eb17563614e6328b7a1610e30949a05feaea", | |
| urls = ["https://github.com/mozilla/cbindgen/releases/download/0.26.0/cbindgen"], | |
| executable = True, | |
| ) | |
| BINDGEN_VERSION = "v0.69.5" | |
| CBINDGEN_VERSION = "0.26.0" | |
| def _iceoryx2_extra_deps(): | |
| maybe( | |
| repo_rule = http_archive, | |
| name = "bindgen", | |
| sha256 = "b7e2321ee8c617f14ccc5b9f39b3a804db173ee217e924ad93ed16af6bc62b1d", | |
| strip_prefix = "bindgen-cli-x86_64-unknown-linux-gnu", | |
| urls = ["https://github.com/rust-lang/rust-bindgen/releases/download/{version}/bindgen-cli-x86_64-unknown-linux-gnu.tar.xz".format(version = BINDGEN_VERSION)], | |
| build_file_content = """ | |
| filegroup( | |
| name = "bindgen-cli", | |
| srcs = ["bindgen"], | |
| visibility = ["//visibility:public"], | |
| ) | |
| """, | |
| ) | |
| maybe( | |
| repo_rule = http_file, | |
| name = "cbindgen", | |
| sha256 = "521836d00863cb129283054e5090eb17563614e6328b7a1610e30949a05feaea", | |
| urls = ["https://github.com/mozilla/cbindgen/releases/download/{version}/cbindgen".format(version = CBINDGEN_VERSION)], | |
| executable = True, | |
| ) |
Member
|
Would you add an entry in |
Author
@dkroenke, thank you for all the points, they are helpful. I will start addressing them. |
dkroenke
reviewed
Nov 13, 2025
| bazel_dep(name = "platforms", version = "0.0.11") | ||
| bazel_dep(name = "rules_cc", version = "0.1.1") | ||
| bazel_dep(name = "rules_rust", version = "0.58.0") | ||
| bazel_dep(name = "iceoryx", version = "2.95.7") |
Member
There was a problem hiding this comment.
@nicu1989 We are currently in the process of copying the needed parts from iceoryx_hoofs to iceoryx2 so that this dependency can be omitted.
I would propose that we wait with this PR until it is done so that we do not need to merge eclipse-iceoryx/iceoryx#2505. Would this be feasible?
CC @elBoberido
There was a problem hiding this comment.
@dkroenke @elBoberido I think we can wait if it is only few weeks . But will reduce a lot of integration effort(with different os and versions) for us once we have it as early as possible.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Notes for Reviewer
Pre-Review Checklist for the PR Author
Convert to draft)SPDX-License-Identifier: Apache-2.0 OR MITiox2-123-introduce-posix-ipc-example)[#123] Add posix ipc example)task-list-completed)Checklist for the PR Reviewer
Post-review Checklist for the PR Author
References
Closes #