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

[BUG] - v0.43.0+ cargo-bazel generates bad selects #2679

Open
timbess opened this issue Jun 4, 2024 · 2 comments
Open

[BUG] - v0.43.0+ cargo-bazel generates bad selects #2679

timbess opened this issue Jun 4, 2024 · 2 comments

Comments

@timbess
Copy link

timbess commented Jun 4, 2024

#2636

After this PR, it seems that tar is now building xattr on windows despite it only being listed as a unix dependency. When I patch the cargo-bazel toolchain to use 0.42.1 binaries with 0.44.0 it generates the proper platform specific selects:

Cargo-Bazel.lock.json v0.44.0 -> v0.42.1

         "deps": {
           "common": [
             {
               "id": "filetime 0.2.20",
               "target": "filetime"
-            },
-            {
-              "id": "xattr 0.2.3",
-              "target": "xattr"
             }
           ],
           "selects": {
             "cfg(unix)": [
               {
                 "id": "libc 0.2.151",
                 "target": "libc"
+              },
+              {
+                "id": "xattr 0.2.3",
+                "target": "xattr"
               }
             ]
           }
         },
@timbess timbess changed the title [BUG] - v0.43.0+ cargo-bazel optional types generates bad selects [BUG] - v0.43.0+ cargo-bazel generates bad selects Jun 4, 2024
@timbess
Copy link
Author

timbess commented Jun 4, 2024

I debugged the issue a bit. It seems like the cargo tree query is including all build dependencies which are actually for the host triple, but are being counted towards the target triple here:

let output = self
.cargo_bin
.command()?
.current_dir(manifest_dir)
.arg("tree")
.arg("--locked")
.arg("--manifest-path")
.arg(manifest_path)
.arg("--edges")
.arg("normal,build,dev")
.arg("--prefix=depth")
// https://doc.rust-lang.org/cargo/commands/cargo-tree.html#tree-formatting-options
.arg("--format=|{p}|{f}|")
.arg("--color=never")
.arg("--workspace")
.arg("--target")
.arg(target_triple.to_cargo())
.env("RUSTC", &self.rustc_bin)
.stdout(std::process::Stdio::piped())
.stderr(std::process::Stdio::piped())
.spawn()
.with_context(|| {
format!(
"Error spawning cargo in child process to compute features for target '{}', manifest path '{}'",
target_triple,
manifest_path.display()
)
})?;
target_triple_to_child.insert(target_triple, output);

If I remove build from the edges, it fixes it:

     .arg("--edges") 
     .arg("normal,dev") 

I added a second query for just build deps, but it seems cargo tree doesn't include transitive normal deps under build deps unfortunately.

I couldn't really find a way to include whether it's a build dependency in the cargo tree format string which would be a more ideal solution IMO.

@timbess
Copy link
Author

timbess commented Jun 4, 2024

I made a PR against Cargo to hopefully add enough metadata to fix this issue rust-lang/cargo#14008

Nevermind just found this #2212, I was going down the wrong path with that solution.

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

No branches or pull requests

2 participants