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 should not forward data to build.rs rust_binary #2767

Closed
ashi009 opened this issue Jul 29, 2024 · 2 comments
Closed

cargo_build_script should not forward data to build.rs rust_binary #2767

ashi009 opened this issue Jul 29, 2024 · 2 comments
Labels

Comments

@ashi009
Copy link

ashi009 commented Jul 29, 2024

When embedding generated targets in the cargo_build_script's data attribute, it will be embedded as runfiles for the actual script's rust_binary target. As cargo_build_script performs config transition for the script attribute, all the data dependencies will be built again using the exec configuration. This is incorrect, as the data required by the build.rs should be built into the target configuration to be linked correctly when cross-compiling.

Taking glib-sys as an example:

rust_library(
    name = "glib_sys",
    deps = [
        "@crate_index__glib-sys-0.18.1//:build_script_build",
        "@crate_index__libc-0.2.155//:libc",
        "@glib//:glib",
    ],
	...
)

cargo_build_script(
    name = "glib-sys_bs",
    crate_name = "build_script_build",
    crate_root = "build.rs",
    data = glob(
        allow_empty = True,
        include = ["**"],
        exclude = [
            "**/* *",
            ".tmp_git_root/**/*",
            "BUILD",
            "BUILD.bazel",
            "WORKSPACE",
            "WORKSPACE.bazel",
        ],
    ) + [
        "@glib//:glib", # <-- target cfg
    ],
    deps = [
        "@crate_index__system-deps-6.2.2//:system_deps",
    ],
	...
)

which expands to:

cargo_build_script(
    name = "glib-sys_bs",
    data = [
		...
        "@glib//:glib",
    ],
    deps = ["@crate_index__system-deps-6.2.2//:system_deps"],
    script = "@crate_index__glib-sys-0.18.1//:glib-sys_bs_",
	...
)

rust_binary(
    name = "glib-sys_bs_",
    compile_data = [],
    crate_features = [],
    crate_name = "build_script_build",
    crate_root = "@crate_index__glib-sys-0.18.1//:build.rs",
    data = [
		...
        "@glib//:glib", # <-- exec cfg
    ],
    deps = ["@crate_index__system-deps-6.2.2//:system_deps"],
	...
)
@ashi009
Copy link
Author

ashi009 commented Sep 11, 2024

@UebelAndre Is this fixed by #2858?

My fix was to avoid using runfiles, but to run build script at the crate root.

 cargo/private/cargo_build_script.bzl         | 2 +-
 cargo/private/cargo_build_script_wrapper.bzl | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/cargo/private/cargo_build_script.bzl b/cargo/private/cargo_build_script.bzl
index 6524ce2..588e2cc 100644
--- a/cargo/private/cargo_build_script.bzl
+++ b/cargo/private/cargo_build_script.bzl
@@ -144,7 +144,7 @@ def _cargo_build_script_impl(ctx):
     flags_out = ctx.actions.declare_file(ctx.label.name + ".flags")
     link_flags = ctx.actions.declare_file(ctx.label.name + ".linkflags")
     link_search_paths = ctx.actions.declare_file(ctx.label.name + ".linksearchpaths")  # rustc-link-search, propagated from transitive dependencies
-    manifest_dir = "%s.runfiles/%s/%s" % (script.path, ctx.label.workspace_name or ctx.workspace_name, ctx.label.package)
+    manifest_dir = "%s/%s" % (ctx.label.workspace_root, ctx.label.package)
     compilation_mode_opt_level = get_compilation_mode_opts(ctx, toolchain).opt_level
 
     streams = struct(
diff --git a/cargo/private/cargo_build_script_wrapper.bzl b/cargo/private/cargo_build_script_wrapper.bzl
index 1860e61..8549499 100644
--- a/cargo/private/cargo_build_script_wrapper.bzl
+++ b/cargo/private/cargo_build_script_wrapper.bzl
@@ -154,7 +154,6 @@ def cargo_build_script(
         crate_features = crate_features,
         deps = deps,
         proc_macro_deps = proc_macro_deps,
-        data = data,
         compile_data = compile_data,
         rustc_env = rustc_env,
         rustc_env_files = rustc_env_files,

@UebelAndre
Copy link
Collaborator

This should be fixed by #2826. Closing out unless someone finds it's still an issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants