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.

Sign up for GitHub

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

Jump to bottom

build.rs stdout creates non-deterministic issues #2974

Closed
nmattia opened this issue Oct 31, 2024 · 2 comments · Fixed by #2976
Closed

build.rs stdout creates non-deterministic issues #2974

nmattia opened this issue Oct 31, 2024 · 2 comments · Fixed by #2976

Comments

@nmattia
Copy link
Contributor

nmattia commented Oct 31, 2024

The build script runner infers "execroot" from $CWD. This "execroot" includes sandbox-specific paths which get included in stdout. For instance, when building cc-rs:

...
ARFLAGS = None
running: ZERO_AR_DATE="1" "/usr/bin/ar" "cq" "/var/tmp/bazel-output/sandbox/linux-sandbox/3676/execroot/_main/bazel-out/k8-opt/bin/external/crate_index__psm-0.1.21/_bs.out_dir/libpsm_s.a" "/var/tmp/bazel-output/sandbox/linux-sandbox/3676/execroot/_main/bazel-out/k8-opt/bin/external/crate_index__psm-0.1.21/_bs.out_dir/src/arch/x86_64.o"
exit status: 0
running: "/usr/bin/ar" "s" "/var/tmp/bazel-output/sandbox/linux-sandbox/3676/execroot/_main/bazel-out/k8-opt/bin/external/crate_index__psm-0.1.21/_bs.out_dir/libpsm_s.a"
exit status: 0
cargo:rustc-link-lib=static=psm_s
cargo:rustc-link-search=native=/var/tmp/bazel-output/sandbox/linux-sandbox/3676/execroot/_main/bazel-out/k8-opt/bin/external/crate_index__psm-0.1.21/_bs.out_dir

This path part is variable and changes from build to build: /var/tmp/bazel-output/sandbox/linux-sandbox/3676. When converted to build script outputs (rules_rust's understanding of cargo:rustc-...-style lines) this build-specific "execroot" is redacted.

The raw (sandbox path dependent) stdout however is written as is as an output, and the output is tracked by bazel. In some cases this can be fixed in the create itself, so that e.g. OUT_DIR is not actually used (see here, here and here for instance).

However in general, what is the benefit of keeping stdout.log in the build outputs? Would it make sense to disable this in general and make it opt-in (potentially on a crate by crate basis)?
@nmattia nmattia changed the title stdout creates non-deterministic issues build.rs stdout creates non-deterministic issues Oct 31, 2024
@UebelAndre
Copy link
Collaborator

However in general, what is the benefit of keeping stdout.log in the build outputs? Would it make sense to disable this in general and make it opt-in (potentially on a crate by crate basis)?

To me it's mostly for debugging. I would be in favor of removing the output or making the output conditional based on some flag.

@UebelAndre UebelAndre mentioned this issue Nov 1, 2024
Merged
github-merge-queue bot pushed a commit that referenced this issue Nov 3, 2024
@UebelAndre
Made cargo_build_script streams output group optional. (#2976)
034f2cc 
This change introduces the
`--@rules_rust//cargo/settings:debug_std_streams_output_group` flag
which can be used to enable the `streams` output group on
`cargo_build_script` targets. For greater reproducibility, the flag is
defaulted to `false`.

closes #2974
@nmattia
Copy link
Contributor Author

nmattia commented Nov 3, 2024

Thanks a lot @UebelAndre !

nmattia added a commit to dfinity/ic that referenced this issue Nov 5, 2024
@nmattia
fix(IDX): make replica build more deterministic
9f0d834 
This includes a couple of fixes to make the replica build more
deterministic.

This in theory allows anyone building `//rs/replica` with Bazel in the
Docker image to get a bit-for-bit reproducible replica executable, and
this should also improve Bazel cache hit rates.

This fixes for the following issues:

* Non-reproducible `jemalloc` build: the `tikv-jemalloc-sys` crate
  vendors `jemalloc` and builds it as part of `build.rs`. Unfortunately
  that build in not deterministic. To make it more deterministic we
  build `jemalloc` separately, which also speeds up rebuilds as the C
  code does not need to be rebuilt when rust versions change. We only
  enable this in Linux; this also includes a patch to support this in
  `rules_rust`: bazelbuild/rules_rust#2981

* Non-reproducible `rules_rust` build log: this includes a backport of a
  fix that disables build logs in `rules_rust` by default (backported
  because our build is not compatible with the latest `rules_rust`)
  bazelbuild/rules_rust#2974

* Non-reproducible make & pkgconfig toolchains: some toolchains packaged
  by `rules_foreign_cc` cause build determinism issues so instead we use
  the ones installed in the container and remove build logs: bazel-contrib/rules_foreign_cc#1313

* Non-reproducible obj file generation in cc-rs: the `cc-rs` crate used
  in many C builds, including the (ASM) build of `ring`'s crypto bits,
  generates object files that include the Bazel sandbox full path:
  rust-lang/cc-rs#1271

* Non-reproducible codegen: `cranelift-isle` and
  `cranelift-codegen-meta` include references to source files as
  absolute paths that include the Bazel sandbox path:
  bytecodealliance/wasmtime#9553
nmattia added a commit to dfinity/ic that referenced this issue Nov 5, 2024
@nmattia
fix(IDX): make replica build more deterministic
7021376 
This includes a couple of fixes to make the replica build more
deterministic.

This in theory allows anyone building `//rs/replica` with Bazel in the
Docker image to get a bit-for-bit reproducible replica executable, and
this should also improve Bazel cache hit rates.

This fixes for the following issues:

* Non-reproducible `jemalloc` build: the `tikv-jemalloc-sys` crate
  vendors `jemalloc` and builds it as part of `build.rs`. Unfortunately
  that build in not deterministic. To make it more deterministic we
  build `jemalloc` separately, which also speeds up rebuilds as the C
  code does not need to be rebuilt when rust versions change. We only
  enable this in Linux; this also includes a patch to support this in
  `rules_rust`: bazelbuild/rules_rust#2981

* Non-reproducible `rules_rust` build log: this includes a backport of a
  fix that disables build logs in `rules_rust` by default (backported
  because our build is not compatible with the latest `rules_rust`)
  bazelbuild/rules_rust#2974

* Non-reproducible make & pkgconfig toolchains: some toolchains packaged
  by `rules_foreign_cc` cause build determinism issues so instead we use
  the ones installed in the container and remove build logs: bazel-contrib/rules_foreign_cc#1313

* Non-reproducible obj file generation in cc-rs: the `cc-rs` crate used
  in many C builds, including the (ASM) build of `ring`'s crypto bits,
  generates object files that include the Bazel sandbox full path:
  rust-lang/cc-rs#1271

* Non-reproducible codegen: `cranelift-isle` and
  `cranelift-codegen-meta` include references to source files as
  absolute paths that include the Bazel sandbox path:
  bytecodealliance/wasmtime#9553
@nmattia nmattia mentioned this issue Nov 5, 2024
Merged
github-merge-queue bot pushed a commit to dfinity/ic that referenced this issue Nov 5, 2024
@nmattia
fix(IDX): make replica build more deterministic (#2441)
74352d1 
This includes a couple of fixes to make the replica build more
deterministic.

This in theory allows anyone building `//rs/replica` with Bazel in the
Docker image to get a bit-for-bit reproducible replica executable, and
this should also improve Bazel cache hit rates.

This fixes for the following issues:

* Non-reproducible `jemalloc` build: the `tikv-jemalloc-sys` crate
vendors `jemalloc` and builds it as part of `build.rs`. Unfortunately
that build in not deterministic. To make it more deterministic we build
`jemalloc` separately, which also speeds up rebuilds as the C code does
not need to be rebuilt when rust versions change. We only enable this in
Linux; this also includes a patch to support this in `rules_rust`:
bazelbuild/rules_rust#2981

* Non-reproducible `rules_rust` build log: this includes a backport of a
fix that disables build logs in `rules_rust` by default (backported
because our build is not compatible with the latest `rules_rust`)
bazelbuild/rules_rust#2974

* Non-reproducible make & pkgconfig toolchains: some toolchains packaged
by `rules_foreign_cc` cause build determinism issues so instead we use
the ones installed in the container and remove build logs:
bazel-contrib/rules_foreign_cc#1313

* Non-reproducible obj file generation in cc-rs: the `cc-rs` crate used
in many C builds, including the (ASM) build of `ring`'s crypto bits,
generates object files that include the Bazel sandbox full path:
rust-lang/cc-rs#1271

* Non-reproducible codegen: `cranelift-isle` and
`cranelift-codegen-meta` include references to source files as absolute
paths that include the Bazel sandbox path:
bytecodealliance/wasmtime#9553
alin-at-dfinity pushed a commit to dfinity/ic that referenced this issue Nov 7, 2024
@nmattia @alin-at-dfinity
fix(IDX): make replica build more deterministic (#2441)
fc88220 
This includes a couple of fixes to make the replica build more
deterministic.

This in theory allows anyone building `//rs/replica` with Bazel in the
Docker image to get a bit-for-bit reproducible replica executable, and
this should also improve Bazel cache hit rates.

This fixes for the following issues:

* Non-reproducible `jemalloc` build: the `tikv-jemalloc-sys` crate
vendors `jemalloc` and builds it as part of `build.rs`. Unfortunately
that build in not deterministic. To make it more deterministic we build
`jemalloc` separately, which also speeds up rebuilds as the C code does
not need to be rebuilt when rust versions change. We only enable this in
Linux; this also includes a patch to support this in `rules_rust`:
bazelbuild/rules_rust#2981

* Non-reproducible `rules_rust` build log: this includes a backport of a
fix that disables build logs in `rules_rust` by default (backported
because our build is not compatible with the latest `rules_rust`)
bazelbuild/rules_rust#2974

* Non-reproducible make & pkgconfig toolchains: some toolchains packaged
by `rules_foreign_cc` cause build determinism issues so instead we use
the ones installed in the container and remove build logs:
bazel-contrib/rules_foreign_cc#1313

* Non-reproducible obj file generation in cc-rs: the `cc-rs` crate used
in many C builds, including the (ASM) build of `ring`'s crypto bits,
generates object files that include the Bazel sandbox full path:
rust-lang/cc-rs#1271

* Non-reproducible codegen: `cranelift-isle` and
`cranelift-codegen-meta` include references to source files as absolute
paths that include the Bazel sandbox path:
bytecodealliance/wasmtime#9553
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Made cargo_build_script streams output group optional. UebelAndre/rules_rust
2 participants
@nmattia @UebelAndre