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.

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

cargo_build_script: difference in CARGO_PKG_VERSION for buildscript compile compared to standard cargo #3139

Open
bazaah opened this issue Dec 27, 2024 · 0 comments

Comments

@bazaah
Copy link

bazaah commented Dec 27, 2024

So, this is more of a question than a bug report (but a bug report will be provided nevertheless).

Context

While attempting to use the rustls crate with the aws-lc-rs backend, I ran into an interesting linker error, that only exhibited itself during any binary-ish (rust_binary, rust_test, etc) target build.

It looks like this, cleaned up:

error: linking with `<toolchains_llvm sysroot'd cc_wrapper>` failed: exit status: 1
  |
  = note: lld: error: undefined symbol: aws_lc_0_24_0_CRYPTO_library_init
          >>> referenced by aws_lc_rs.cfa660e1dffe16e2-cgu.6
          >>>               <...>/bazel-out/k8-fastbuild/bin/external/crates_io__aws-lc-rs-1.12.0/libaws_lc_rs-966159758.rlib(aws_lc_rs-966159758.aws_lc_rs.cfa660e1dffe16e2-cgu.6.rcgu.o):(aws_lc_rs::init::_$u7b$$u7b$closure$u7d$$u7d$::h59c78008fe3385f8)
          collect2: error: ld returned 1 exit status

<... and many, many more such errors>

Bug description

After some hours beating around the wrong bushes, I eventually realized something interesting (and the cause of the link issues); aws-lc-sys uses a versioned prefix for their exported symbols in the form aws_lc_%version_%symbol, and consequently, aws-lc-rs wants to link against (in my case) aws_lc_0_24_0_%symbol prefixed symbols.

However, the .rlib of the aws-lc-sys crate, contained symbols like aws_lc_0_0_0_%symbol -- and if you've ever watched the output of build --subcommands on rust targets, you know where this is going. rules_rust will typically set the CARGO_PKG_VERSION of non crate builds to 0.0.0; however differently to upstream cargo, it also sets the compile of the build script itself to 0.0.0, rather than inheriting the "parent" crate's CARGO_PKG_VERSION. This, combined with aws-lc-sys's use of std::env!, uses the compile time value, rather than what is set at the build script runtime.

And here we come to the question:

  • Is this a rules_rust issue?
    • I think aws-lc-sys is doing the wrong thing here, and should instead be reading this value at runtime
    • But the official rust build tool (cargo) handles this case fine, and not supporting this behavior can and will lead to other subtle issues down the road

I'm asking here first, because technically this issue only exists under rules_rust's cargo machinery. Otherwise (and maybe regardless of what happens here) I'll open an issue with them, too because it seems weird to prebake the prefix into the builder, rather than reading it, from the buildenv during builder runtime.

MVB

WORKSPACE
# ./WORKSPACE
# vim: sw=4 cc=80 et:

load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")

http_archive(
    name = "bazel_skylib",
    sha256 = "bc283cdfcd526a52c3201279cda4bc298652efa898b10b4db0837dc51652756f",
    urls = [
        "https://mirror.bazel.build/github.com/bazelbuild/bazel-skylib/releases/download/1.7.1/bazel-skylib-1.7.1.tar.gz",
        "https://github.com/bazelbuild/bazel-skylib/releases/download/1.7.1/bazel-skylib-1.7.1.tar.gz",
    ],
)

load("@bazel_skylib//:workspace.bzl", "bazel_skylib_workspace")
bazel_skylib_workspace()

RC_VERSION="0.0.13"
http_archive(
    name = "rules_cc",
    urls = ["https://github.com/bazelbuild/rules_cc/releases/download/{v}/rules_cc-{v}.tar.gz".format(v = RC_VERSION)],
    sha256 = "d9bdd3ec66b6871456ec9c965809f43a0901e692d754885e89293807762d3d80",
    strip_prefix = "rules_cc-{v}".format(v = RC_VERSION),
)
load("@rules_cc//cc:repositories.bzl", "rules_cc_dependencies", "rules_cc_toolchains")
rules_cc_dependencies()
rules_cc_toolchains()

PROTOBUF_VERSION = "27.5"
http_archive(
    name = "protobuf",
    sha256 = "79cc6d09d02706c5a73e900ea842b5b3dae160f371b6654774947fe781851423",
    strip_prefix = "protobuf-{v}".format(v = PROTOBUF_VERSION),
    urls = [
        "https://github.com/protocolbuffers/protobuf/releases/download/v{v}/protobuf-{v}.tar.gz".format(v = PROTOBUF_VERSION),
    ],
)

RR_VERSION="0.56.0"
RUST_VERSION="1.78.0"
http_archive(
    name = "rules_rust",
    integrity = "sha256-8TBqrAsli3kN8BrZq8arsN8LZUFsdLTvJ/Sqsph4CmQ=",
    urls = [
      "https://github.com/bazelbuild/rules_rust/releases/download/{v}/rules_rust-{v}.tar.gz".format(v = RR_VERSION)
    ],
)

load("@rules_rust//rust:repositories.bzl", "rules_rust_dependencies", "rust_register_toolchains")
load("@rules_rust//crate_universe:repositories.bzl", "crate_universe_dependencies")
rules_rust_dependencies()
rust_register_toolchains(
    edition = "2021",
    versions = [RUST_VERSION, "nightly"],
    extra_target_triples = [],
    toolchain_triples={"x86_64-unknown-linux-gnu": "rust_linux_x86_64"}
)
crate_universe_dependencies(rust_version = RUST_VERSION)

load("@rules_rust//crate_universe:defs.bzl", "crates_repository", "crate")

ANNOTATIONS = {
# Uncomment this to build the MVB successfully
#
#    "aws-lc-sys" : [
#        crate.annotation(
#            patches = ["@//:patch/fix-symbol-versioning.patch"],
#            patch_args = ["-p1"],
#        ),
#    ],
}

crates_repository(
    name = "crates_io",
    cargo_lockfile = "//:Cargo.lock",
    lockfile = "//:Cargo.Bazel.lock",
    packages = {
        "aws-lc-rs": crate.spec(
            version = "1",
        ),
    },
    annotations = ANNOTATIONS
)

load("@crates_io//:defs.bzl", "crate_repositories")
crate_repositories()
BUILD
# ./BUILD
# vim: sw=4 cc=80 et:

package(default_visibility = ["//visibility:public"])

load("@rules_rust//rust:defs.bzl", "rust_binary")

alias(
    name   = "bin",
    actual = ":bug_report",
)

rust_binary(
    name = "bug_report",
    srcs = ["src/main.rs"],
    deps = ["@crates_io//:aws-lc-rs"],
)
src/main.rs
// ./src/main.rs

fn main() {
    /*
     * Just need to use _something_ from the crate to ensure rustc does
     * actually bother to link
     */
    aws_lc_rs::init();

    println!("Hello, world!")
}
patch/fix-symbol-versioning.patch
diff --git a/builder/main.rs b/builder/main.rs
index 3619a421d1..4bebe1ac57 100644
--- a/builder/main.rs
+++ b/builder/main.rs
@@ -210,7 +210,9 @@ impl OutputLib {
 const VERSION: &str = env!("CARGO_PKG_VERSION");

 fn prefix_string() -> String {
-    format!("aws_lc_{}", VERSION.to_string().replace('.', "_"))
+    let version = std::env::var("CARGO_PKG_VERSION")
+        .expect("CARGO_PKG_VERSION to be set when running the build script");
+    format!("aws_lc_{}", version.replace('.', "_"))
 }

 #[cfg(feature = "bindgen")]
.bazelversion
7.4.0
.bazelrc
# ./.bazelrc
# vim: sw=4 ft=sh cc=80 et:

common                  --noenable_bzlmod

build                   --attempt_to_print_relative_paths
build                   --verbose_failures
build                   --experimental_reuse_sandbox_directories
build                   --incompatible_strict_action_env
build                   --collapse_duplicate_defines

Or see the attached rules_rust_mvb.tar.gz

You can easily trigger this bug on any linux system with build-essential (or equivalent) installed via:

touch Cargo.lock Cargo.Bazel.lock && CARGO_BAZEL_REPIN=1 bazel sync --only=crates_io && bazel build -s :bug_report

If you want to investigate the sys .rlib file:

nm bazel-bin/external/crates_io__aws-lc-sys-*/libaws_lc_sys*.rlib | grep -F 'aws_lc_0_0_0' | head
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant