linking C libraries staticly and Bazel compatibility

[irajtaghlidi]

This PR addresses two issues I encountered with Bazel that Cargo can handle. My approach was to solve them with minimal changes while remaining practical and aligned with Cargo and Rust guidelines.

* Since mbedtls-sys is responsible for providing the C libraries to dependent crates, we do not need to instruct Cargo to link them in the mbedtls-platform-support crate as well. While Cargo can handle this additional request, Bazel’s sandboxing prevents sharing the intermediate build script outputs of one crate with a dependent crate by default. Only .rlib or other library output files of dependent crates are accessible.

• The default linking type in cargo:rustc-link-lib is dynamic. However, since we are building vendor C libraries from source, we need to explicitly link them statically in the mbedtls-sys crate.

• Cargo has access to the entire file system at all stages, so passing DEP_KEY=VALUE from one crate's build script to a dependent crate's build script is possible through various methods. However, because Bazel’s Rust rules strictly limit the input data for each build task, it's preferable to share information by referencing the OUT_DIR of dependencies.
  Now, mbedtls-sys crate provides cargo:include by pointing to its OUT_DIR, and mbedtls-platform-support can access header files using the DEP_MBEDTLS_INCLUDE environment variable. Then we don't need to customize the build process in Bazel for that.

[irajtaghlidi]

Ok, Now I can see the linking error with the GNU linker (ld) which I was looking for to reproduce but since the recent version of Rust toolchain uses LLVM linker it will not happen. I'll update my PR to address it.

[jethrogb]

I don't understand this PR. Are you copying the include dir to out_dir?

[irajtaghlidi]

I don't understand this PR. Are you copying the include dir to out_dir?

I don't copy. it is automatically provided in OUT_DIR after building with CMake.

ls "/home/iraj/fortanix/playground/bazel/rust-mbedtls/target/debug/build/mbedtls-sys-auto-6b1c4dec2d5df1f4/out"
bindings.rs  build  config.h  include  lib  mod-bindings.rs

[jethrogb]

it is automatically provided in OUT_DIR after building with CMake.

Oh, interesting.

[irajtaghlidi]

I had to revert my initial change to resolve a linking issue with the mbedtls_printf method in the mbedtls-platform-support lib crate. My goal was to make rust-mbedtls build natively with Bazel without any additional customization. However, it appears that both the mbedtls-sys and mbedtls-platform-support crates need to be linked to C static libraries. And I haven’t been able to find a solution to link only in the sys crate.

For the reference:
Then we have to provide mbedtls-sys's build script output dir as a data input for the mbedtls-platform-support using annotation in WORKSPACE.bazel like this example:

crates_repository(
    # ...
    annotations = {
        "mbedtls-platform-support": [crate.annotation(
            # compile_data or data
            data = [
                "@crate_index__mbedtls-sys-auto-2.28.7//:build_script_build",
            ],
        )],
    }
}

[jethrogb]

Not sure if this var is used anywhere else.

[irajtaghlidi]

It doesn't. This part is just trying to provide header files location to dependent crates that can be read by DEP_MBEDTLS_INCLUDE environment variable. Either from the source directory or alternatives (out_dir). and from dependent crates, it does not matter as long as it has read access.
Cargo uses a flat model and you have full access to the file system but Bazel sandboxing requires some practices to follow for more compatibility.

[jethrogb]

So then the var can be deleted?

[irajtaghlidi]

self.mbedtls_include is still being used by Bindgen but I'll check if headers are also accessible in there, Since it's running after the CMake step.

fn main() {
    let cfg = BuildConfig::new();
    cfg.create_config_h();
    cfg.print_rerun_files();
    cfg.cmake();
    cfg.bindgen();
}

[irajtaghlidi]

Now, Only CMake depends on the source path and other components are using OUT_DIR content.

[Taowyoo]

What's the affect of newly added static=?
Does it just explicitly ensure static link?

[irajtaghlidi]

What's the effect of newly added static=?
Does it just explicitly ensure static link?

It does not have an effect in Cargo but setting static linking explicitly makes Bazel's Rust rule work properly.

More Context:
Since they are built script's output in a lib (rlib) crate, it should and would linked statically to the final ELF and Cargo handles it automatically even if we don't mention it.
But Rust's rule in Bazel is trusting what we asked for. Since the default link value is dynamic, it tries to find and link these native static libraries (.a archive files) dynamically which fails.
link the generated crate to a native library

So it would be better for both to define explicitly.

[Pagten]

@irajtaghlidi why doesn't this change have an effect when building with Cargo?

On the page you linked above, I see

The kind of library and the modifiers can also be specified in a #[link] attribute. If the kind is not specified in the link attribute or on the command-line, it will link a dynamic library by default, except when building a static executable. If the kind is specified on the command-line, it will override the kind specified in a link attribute.

In the original code, the kind is not specified in the link attribute nor on the command line, so wouldn't cargo try to link a dynamic library?

I understand that the mbedtls-sys build script builds a static lib version of mbedtls and hence we want to link that static lib, but I haven't spotted how the build script tells Cargo that it's a static lib. The documentation (or at least my interpretation of it) seems to indicate that Cargo would try to link a dynamic lib by default, so I would expect the original code to fail. Probably I'm missing something...

[jethrogb]

My guess is Rust tries to link in whatever way it can, it finds that the only available option is static, and does that.

I also note that mbedtls-platform-support already does this:

println!("cargo:rustc-link-lib=static=mbedtls");
println!("cargo:rustc-link-lib=static=mbedx509");
println!("cargo:rustc-link-lib=static=mbedcrypto");

[jethrogb]

Anyway, this specific change seems fine.

[irajtaghlidi]

Hi team, please review this PR and share your final thoughts.