google / cargo-raze

Generate Bazel BUILD from Cargo dependencies!
Apache License 2.0
480 stars 104 forks source link

assertion failed: tree_output.status.success() in some binary-only targets #530

Open rdelfin opened 2 years ago

rdelfin commented 2 years ago

I'm in the process of trying to upgrade the version of cargo raze to 0.16 from 0.15 in a repo I am a part of, but when I upgrade, I consistently get the error thread 'main' panicked at 'assertion failed: tree_output.status.success()', src/features.rs:149:3 (I'll leave a full stack trace below).

Through bisecting, I've been able to narrow it down to 743bd26b2ac0e596c90fc8bd1040a2484adf562f (aka #478). What I don't understand is, what does this error mean, and why would my repo generate this when it worked fine in 0.15? Is this some thing we're doing wrong in our Cargo.toml, or is this a bug?

Backtrace:

thread 'main' panicked at 'assertion failed: tree_output.status.success()', src/features.rs:149:3
stack backtrace:
   0: rust_begin_unwind
             at /rustc/4b91a6ea7258a947e59c6522cd5898e7c0a6a88f/library/std/src/panicking.rs:584:5
   1: core::panicking::panic_fmt
             at /rustc/4b91a6ea7258a947e59c6522cd5898e7c0a6a88f/library/core/src/panicking.rs:142:14
   2: core::panicking::panic
             at /rustc/4b91a6ea7258a947e59c6522cd5898e7c0a6a88f/library/core/src/panicking.rs:48:5
   3: cargo_raze::features::run_cargo_tree
             at ./src/features.rs:149:3
   4: cargo_raze::features::get_per_platform_features_with_command
             at ./src/features.rs:99:34
   5: cargo_raze::features::get_per_platform_features
             at ./src/features.rs:68:3
   6: cargo_raze::metadata::RazeMetadataFetcher::fetch_metadata
             at ./src/metadata.rs:505:25
   7: cargo_raze::fetch_raze_metadata
             at ./src/bin/cargo-raze.rs:215:23
   8: cargo_raze::main
             at ./src/bin/cargo-raze.rs:92:23
   9: core::ops::function::FnOnce::call_once
             at /rustc/4b91a6ea7258a947e59c6522cd5898e7c0a6a88f/library/core/src/ops/function.rs:248:5

Minimum reproducible example: https://github.com/rdelfin/rust-bazel-test

rdelfin commented 2 years ago

Looking in further, by running cargo tree in the appropriate directory I get the following error:

 cargo tree --prefix=none --frozen --target=x86_64-unknown-linux-gnu --format="{p}|{f}|"
error: failed to get `actix-web` as a dependency of package `compile_with_bazel v0.0.0 (/home/rdelfin/code/WayveCode/3rdparty/rust)`

Caused by:
  failed to load source for dependency `actix-web`

Caused by:
  Unable to update registry `crates-io`

Caused by:
  attempting to make an HTTP request, but --frozen was specified

I'm not sure if this is because the Cargo.raze.lock file isn't being used in that call, or something else. FWIW, I have the following metadata settings (Cargo.toml is in 3rdparty/rust/Cargo.toml for context):

[package.metadata.raze]
workspace_path = "//3rdparty/rust/cargo"
package_aliases_dir = "."
targets = [
    "x86_64-unknown-linux-gnu",
    "aarch64-unknown-linux-gnu",
]
genmode = "Remote"

I wonder if the fact that the lock file is in a different directory is part of the issue

rdelfin commented 2 years ago

I've been able to narrow down to the following lines in my Cargo.toml:

[package.metadata.raze.binary_deps]
cxxbridge-cmd = "1.0.78"

Not sure why this is an issue, but it seems like the issue is binary deps

rdelfin commented 2 years ago

After further debugging, it looks like the output of the tree command failing is:

error: no packages to compile
rdelfin commented 2 years ago

You can find a minimum reproducible example here: https://github.com/rdelfin/rust-bazel-test

rdelfin commented 2 years ago

@sayrer I see you're the author of the PR. Would you know what the issue is here? I'm not terribly familiar with this level of cargo details

rdelfin commented 2 years ago

Apparently I accidentally made the repo private. It's now fixed

rdelfin commented 2 years ago

I've tracked down the issue to how the whole temporary cargo directory is generated. If you look at what it looks like on the example project I linked above, you see this file structure (most of the cxx-bridge directory omited for simplicity):

 $ tree
.
├── Cargo.lock
├── Cargo.toml
└── cxxbridge-cmd-1.0.78
    ├── Cargo.lock
    ├── Cargo.toml
    [...]

5 directories, 59 files

If you look at the Cargo.toml file, you'll notice it generates both a package at the root, and it defines a workspace (a lot of the file omited for brevity):

$ cat Cargo.toml
bench = []
bin = []
[...]

[dependencies]
cxx = "1.0.78"
cxxbridge-cmd = "1.0.78"

[...]

[lib]
[...]
path = "fake_lib.rs"
[...]

[package]
[...]
name = "compile_with_bazel"
[...]

[package.metadata.raze]
genmode = "Remote"
package_aliases_dir = "."
targets = ["x86_64-unknown-linux-gnu", "aarch64-unknown-linux-gnu"]
workspace_path = "//3rdparty/rust/cargo"

[package.metadata.raze.binary_deps]
cxxbridge-cmd = "1.0.78"
[package.metadata.raze.crates.cxxbridge-cmd."1.0.78"]
compile_data_attr = "glob([\"**/*.h\"])"
extra_aliased_targets = ["cargo_bin_cxxbridge"]

[...]

[workspace]
default-members = []
exclude = []
members = ["cxxbridge-cmd-1.0.78"]

Took me a while to figure out exactly what the issue was, but finally I managed to get a good, valid output by running cargo tree -p compile_with_bazel. The problem seems to be that, when we add binaries we introduce workspaces into the generated cargo project. That would be fine, except we also set default-members = [], which makes cargo tree fail consistently as it doesn't know what package you're trying to build. Commenting out that line fixes the issue. It seems to me we shouldn't be setting said argument in the first place.

rdelfin commented 2 years ago

Looking at the original PR, I just noticed there's a PR, #524 that fixes the issue