Skip to content

Commit

Permalink
fix(IDX): make replica build more deterministic (#2441)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
nmattia authored and alin-at-dfinity committed Nov 7, 2024
1 parent e1508be commit fc88220
Show file tree
Hide file tree
Showing 13 changed files with 328 additions and 20 deletions.
42 changes: 38 additions & 4 deletions Cargo.Bazel.Fuzzing.json.lock
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"checksum": "d49c2d6535ae0e33df43f23bf7877c04341c237a0782c81faa1e85ae9a4e8a6b",
"checksum": "7da030b68225ba2206b41223d4722879a382c66805a9cfacc6eaaf5e05a4be60",
"crates": {
"abnf 0.12.0": {
"name": "abnf",
Expand Down Expand Up @@ -10646,7 +10646,13 @@
"repository": {
"Http": {
"url": "https://static.crates.io/crates/cc/1.0.83/download",
"sha256": "f1174fb0b6ec23863f8b971027804a42614e347eafb0a95bf0b12cdae21fc4d0"
"sha256": "f1174fb0b6ec23863f8b971027804a42614e347eafb0a95bf0b12cdae21fc4d0",
"patch_args": [
"-p1"
],
"patches": [
"@@//bazel:cc_rs.patch"
]
}
},
"targets": [
Expand Down Expand Up @@ -14351,7 +14357,13 @@
"repository": {
"Http": {
"url": "https://static.crates.io/crates/cranelift-codegen-meta/0.112.2/download",
"sha256": "5a0ce0273d7a493ef8f31f606849a4e931c19187a4923f5f87fc1f2b13109981"
"sha256": "5a0ce0273d7a493ef8f31f606849a4e931c19187a4923f5f87fc1f2b13109981",
"patch_args": [
"-p4"
],
"patches": [
"@@//bazel:cranelift-codegen-meta.patch"
]
}
},
"targets": [
Expand Down Expand Up @@ -14624,7 +14636,13 @@
"repository": {
"Http": {
"url": "https://static.crates.io/crates/cranelift-isle/0.112.2/download",
"sha256": "230cb33572b9926e210f2ca28145f2bc87f389e1456560932168e2591feb65c1"
"sha256": "230cb33572b9926e210f2ca28145f2bc87f389e1456560932168e2591feb65c1",
"patch_args": [
"-p4"
],
"patches": [
"@@//bazel:cranelift-isle.patch"
]
}
},
"targets": [
Expand Down Expand Up @@ -70592,6 +70610,14 @@
"compile_data_glob": [
"**"
],
"data": {
"common": [],
"selects": {
"x86_64-unknown-linux-gnu": [
"@jemalloc//:libjemalloc"
]
}
},
"data_glob": [
"**"
],
Expand All @@ -70604,6 +70630,14 @@
],
"selects": {}
},
"build_script_env": {
"common": {},
"selects": {
"x86_64-unknown-linux-gnu": {
"JEMALLOC_OVERRIDE": "$(location @jemalloc//:libjemalloc)"
}
}
},
"links": "jemalloc"
},
"license": "MIT/Apache-2.0",
Expand Down
42 changes: 38 additions & 4 deletions Cargo.Bazel.json.lock
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"checksum": "4002433162202c7300c4f17bf7105e405ab6e3b9f9cd9e3d56c3ebe98e04d509",
"checksum": "85964520a01715b114641f0ce8c02f15881adc9a219fd27c7364bfc4ad7df216",
"crates": {
"abnf 0.12.0": {
"name": "abnf",
Expand Down Expand Up @@ -10537,7 +10537,13 @@
"repository": {
"Http": {
"url": "https://static.crates.io/crates/cc/1.0.83/download",
"sha256": "f1174fb0b6ec23863f8b971027804a42614e347eafb0a95bf0b12cdae21fc4d0"
"sha256": "f1174fb0b6ec23863f8b971027804a42614e347eafb0a95bf0b12cdae21fc4d0",
"patch_args": [
"-p1"
],
"patches": [
"@@//bazel:cc_rs.patch"
]
}
},
"targets": [
Expand Down Expand Up @@ -14174,7 +14180,13 @@
"repository": {
"Http": {
"url": "https://static.crates.io/crates/cranelift-codegen-meta/0.112.2/download",
"sha256": "5a0ce0273d7a493ef8f31f606849a4e931c19187a4923f5f87fc1f2b13109981"
"sha256": "5a0ce0273d7a493ef8f31f606849a4e931c19187a4923f5f87fc1f2b13109981",
"patch_args": [
"-p4"
],
"patches": [
"@@//bazel:cranelift-codegen-meta.patch"
]
}
},
"targets": [
Expand Down Expand Up @@ -14447,7 +14459,13 @@
"repository": {
"Http": {
"url": "https://static.crates.io/crates/cranelift-isle/0.112.2/download",
"sha256": "230cb33572b9926e210f2ca28145f2bc87f389e1456560932168e2591feb65c1"
"sha256": "230cb33572b9926e210f2ca28145f2bc87f389e1456560932168e2591feb65c1",
"patch_args": [
"-p4"
],
"patches": [
"@@//bazel:cranelift-isle.patch"
]
}
},
"targets": [
Expand Down Expand Up @@ -70432,6 +70450,14 @@
"compile_data_glob": [
"**"
],
"data": {
"common": [],
"selects": {
"x86_64-unknown-linux-gnu": [
"@jemalloc//:libjemalloc"
]
}
},
"data_glob": [
"**"
],
Expand All @@ -70444,6 +70470,14 @@
],
"selects": {}
},
"build_script_env": {
"common": {},
"selects": {
"x86_64-unknown-linux-gnu": {
"JEMALLOC_OVERRIDE": "$(location @jemalloc//:libjemalloc)"
}
}
},
"links": "jemalloc"
},
"license": "MIT/Apache-2.0",
Expand Down
22 changes: 22 additions & 0 deletions MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,28 @@ bazel_dep(name = "aspect_bazel_lib", version = "2.9.0")
bazel_dep(name = "rules_cc", version = "0.0.13")
bazel_dep(name = "platforms", version = "0.0.10")

# configure/make dependencies
bazel_dep(name = "rules_foreign_cc", version = "0.12.0")

register_toolchains(
"@rules_foreign_cc//toolchains:preinstalled_pkgconfig_toolchain",
"@rules_foreign_cc//toolchains:preinstalled_make_toolchain",
)

# Use HEAD to include this commit which is needed for preinstalled toolchains to work
# https://github.com/bazel-contrib/rules_foreign_cc/commit/d03f7ae79ddda0ad228b17048b9e2dc0efcc8e95
#
# Use a patch to work around determinism issues in make & pkgconfig toolchains
# https://github.com/bazel-contrib/rules_foreign_cc/issues/1313
archive_override(
module_name = "rules_foreign_cc",
integrity = "sha384-bTtlZejENu+3rnOsCg1nmSZJl54++7nB0zgzWT+jtZJ1QyMRwkV4ieOaeORQTdjY",
patch_strip = 1,
patches = ["//bazel:rules_foreign_cc.patch"],
strip_prefix = "rules_foreign_cc-77d4483fadbb1b7bcace18ed8e8e87e8791050f6",
urls = ["https://github.com/bazelbuild/rules_foreign_cc/archive/77d4483fadbb1b7bcace18ed8e8e87e8791050f6.tar.gz"],
)

# Python dependencies

bazel_dep(name = "rules_python", version = "0.35.0")
Expand Down
18 changes: 6 additions & 12 deletions WORKSPACE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ workspace(

load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
load("//bazel:mainnet-canisters.bzl", "canisters")
load("//third_party/jemalloc:repository.bzl", "jemalloc_repository")
load("//third_party/lmdb:repository.bzl", "lmdb_repository")

# We cannot derive the Bazel repository names (e.g. @mainnet_nns_registry_canister) directly
Expand Down Expand Up @@ -85,6 +86,9 @@ sol_register_toolchains(

http_archive(
name = "rules_rust",
# Back-ported fix: https://github.com/bazelbuild/rules_rust/pull/2981
patch_args = ["-p1"],
patches = ["//bazel:rules_rust.patch"],
sha256 = "85e2013727ab26fb22abdffe4b2ac0c27a2d5b6296167ba63d8f6e13140f51f9",
urls = ["https://github.com/bazelbuild/rules_rust/releases/download/0.53.0/rules_rust-v0.53.0.tar.gz"],
)
Expand All @@ -110,18 +114,6 @@ rust_register_toolchains(
],
)

# Necessary for our ic-os Makefile build
http_archive(
name = "rules_foreign_cc",
sha256 = "db6fcdb4f5ac217658f2c3aabd61e618d7fadc1cdf7d806ab1b52f2709d3fc66",
strip_prefix = "rules_foreign_cc-9acbb356916760192d4c16301a69267fe44e6dec",
url = "https://github.com/bazelbuild/rules_foreign_cc/archive/9acbb356916760192d4c16301a69267fe44e6dec.tar.gz",
)

load("@rules_foreign_cc//foreign_cc:repositories.bzl", "rules_foreign_cc_dependencies")

rules_foreign_cc_dependencies()

load("//bazel:external_crates.bzl", "external_crates_repository")
load("//bazel/sanitizers_enabled_env:defs.bzl", "sanitizers_enabled_env")

Expand Down Expand Up @@ -157,6 +149,8 @@ rules_motoko_dependencies()

lmdb_repository()

jemalloc_repository()

http_archive(
name = "buildifier_prebuilt",
sha256 = "72b5bb0853aac597cce6482ee6c62513318e7f2c0050bc7c319d75d03d8a3875",
Expand Down
18 changes: 18 additions & 0 deletions bazel/cc_rs.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
diff --git a/src/lib.rs b/src/lib.rs
index 2fe30b9..88cd566 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -1121,6 +1121,13 @@ impl Build {
.ok_or_else(|| Error::new(ErrorKind::InvalidArgument, "parent() failure"))?
.to_string_lossy();
let mut hasher = hash_map::DefaultHasher::new();
+ let out_dir = self.get_out_dir().expect("Could not get out dir");
+
+ let prefix = out_dir.parent().expect("Could not get parent");
+ let prefix: &str = &prefix.to_string_lossy();
+
+ let err = format!("could not strip prefix {prefix} from {dirname}");
+ let dirname = dirname.strip_prefix(prefix).expect(&err);
hasher.write(dirname.to_string().as_bytes());
dst.join(format!("{:016x}-{}", hasher.finish(), basename))
.with_extension("o")
13 changes: 13 additions & 0 deletions bazel/cranelift-codegen-meta.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Fix for https://github.com/bytecodealliance/wasmtime/issues/9553
diff --git a/cranelift/codegen/meta/src/srcgen.rs b/cranelift/codegen/meta/src/srcgen.rs
index d3c321e5b..5b94ddd19 100644
--- a/cranelift/codegen/meta/src/srcgen.rs
+++ b/cranelift/codegen/meta/src/srcgen.rs
@@ -94,7 +94,6 @@ impl Formatter {
directory: &std::path::Path,
) -> Result<(), error::Error> {
let path = directory.join(&filename);
- eprintln!("Writing generated file: {}", path.display());
let mut f = fs::File::create(path)?;

for l in self.lines.iter().map(|l| l.as_bytes()) {
30 changes: 30 additions & 0 deletions bazel/cranelift-isle.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# Fix for https://github.com/bytecodealliance/wasmtime/issues/9553
diff --git a/cranelift/isle/isle/src/codegen.rs b/cranelift/isle/isle/src/codegen.rs
index 158285832..d292e43c0 100644
--- a/cranelift/isle/isle/src/codegen.rs
+++ b/cranelift/isle/isle/src/codegen.rs
@@ -127,9 +127,6 @@ impl<'a> Codegen<'a> {
"// Generated automatically from the instruction-selection DSL code in:",
)
.unwrap();
- for file in &self.files.file_names {
- writeln!(code, "// - {file}").unwrap();
- }

if !options.exclude_global_allow_pragmas {
writeln!(
@@ -641,12 +638,11 @@ impl<L: Length, C> Length for ContextIterWrapper<L, C> {{
stack.push((Self::validate_block(ret_kind, body), "", scope));
}

- &ControlFlow::Return { pos, result } => {
+ &ControlFlow::Return { pos: _pos, result } => {
writeln!(
ctx.out,
- "{}// Rule at {}.",
+ "{}",
&ctx.indent,
- pos.pretty_print_line(&self.files)
)?;
write!(ctx.out, "{}", &ctx.indent)?;
match ret_kind {
30 changes: 30 additions & 0 deletions bazel/external_crates.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ def external_crates_repository(name, cargo_lockfile, lockfile, sanitizers_enable
"canbench": [crate.annotation(
gen_binaries = True,
)],
"cc": [crate.annotation(
# Patch for determinism issues
# https://github.com/rust-lang/cc-rs/issues/1271
patch_args = ["-p1"],
patches = ["@@//bazel:cc_rs.patch"],
)],
"curve25519-dalek": [crate.annotation(
rustc_flags = [
"-C",
Expand Down Expand Up @@ -68,6 +74,30 @@ def external_crates_repository(name, cargo_lockfile, lockfile, sanitizers_enable
patch_args = ["-p1"],
patches = ["@rustix-patch//file:downloaded"],
)],
"tikv-jemalloc-sys": [crate.annotation(
# Avoid building jemalloc from rust (in part bc it creates builder-specific config files)
build_script_data = crate.select([], {
"x86_64-unknown-linux-gnu": [
"@jemalloc//:libjemalloc",
],
}),
build_script_env = crate.select(
{},
{
"x86_64-unknown-linux-gnu": {"JEMALLOC_OVERRIDE": "$(location @jemalloc//:libjemalloc)"},
},
),
)],
"cranelift-isle": [crate.annotation(
# Patch for determinism issues
patch_args = ["-p4"],
patches = ["@@//bazel:cranelift-isle.patch"],
)],
"cranelift-codegen-meta": [crate.annotation(
# Patch for determinism issues
patch_args = ["-p4"],
patches = ["@@//bazel:cranelift-codegen-meta.patch"],
)],
"secp256k1-sys": [crate.annotation(
# This specific version is used by ic-btc-kyt canister, which
# requires an extra cfg flag to avoid linking issues.
Expand Down
14 changes: 14 additions & 0 deletions bazel/rules_foreign_cc.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# Avoid build logs that create sources of non-determinism
diff --git a/foreign_cc/private/framework.bzl b/foreign_cc/private/framework.bzl
index 33129b8..7326107 100644
--- a/foreign_cc/private/framework.bzl
+++ b/foreign_cc/private/framework.bzl
@@ -616,7 +616,7 @@ def wrap_outputs(ctx, lib_name, configure_name, script_text, env_prelude, build_
cleanup_on_success_function = create_function(
ctx,
"cleanup_on_success",
- "rm -rf $$BUILD_TMPDIR$$ $$EXT_BUILD_DEPS$$",
+ "rm -rf $$BUILD_TMPDIR$$ $$EXT_BUILD_DEPS$$ && echo > $$BUILD_LOG$$",
)
cleanup_on_failure_function = create_function(
ctx,
Loading

0 comments on commit fc88220

Please sign in to comment.