cargo-auditable icon indicating copy to clipboard operation
cargo-auditable copied to clipboard

`dep:` syntax causes build failures in rare cases

Open jcgruenhage opened this issue 2 years ago • 20 comments

https://github.com/Byron/gitoxide doesn't successfully build with cargo-auditable anymore. It fails in https://github.com/rust-secure-code/cargo-auditable/blob/9fa7fb3988b4d2414397b9550a3f3fd1aa1bc8f2/cargo-auditable/src/collect_audit_data.rs#L77-L80. I'm not sure if gitoxide or cargo-auditable is to blame here, and the error message isn't all that helpful to me either:

thread 'main' panicked at 'cargo metadata failure: error: Package `gix-features v0.31.0 (/home/jcgruenhage/dev/github.com/Byron/gitoxide/gix-features)` does not have feature `prodash`. It has an optional dependency with that name, but that dependency uses the "dep:" syntax in the features table, so it does not have an implicit feature with that name.
', cargo-auditable/src/collect_audit_data.rs:77:9

That reads to me as if something tried to enable a prodash feature on gix-features, but I couldn't find such a thing.

For context, afaict, v0.23.0 is the last version that builds. Anything newer than that runs into the above issue. I ran into this while trying to update the version of gitoxide we ship on Void Linux.

jcgruenhage avatar Jul 09 '23 08:07 jcgruenhage

The error originates in cargo metadata command that is part of the official Cargo distribution, and that cargo auditable runs to get information about the crates.

I can reproduce the issue on the latest version of the repo from git and Rust 1.70. I will investigate further. Thank you for the report!

Shnatsel avatar Jul 09 '23 09:07 Shnatsel

Thanks for looking into this, reporting it is the least I can do when seeing something like this ;)

jcgruenhage avatar Jul 09 '23 09:07 jcgruenhage

It seems to be a bug in cargo. It passes --cfg 'feature="prodash"' to rustc even though there is no such feature in the crate, only a dep: declaration. Then, when cargo auditable extracts the list of features passed to rustc and feeds it back to cargo, cargo itself reports an error because the feature it passed to rustc doesn't actually exist.

Unfortunately this is something that will have to be fixed in Cargo, so I cannot provide an ETA for the fix. I will make a minimal reproducing example and submit the issue upstream.

Shnatsel avatar Jul 09 '23 09:07 Shnatsel

I've filed an issue upstream: https://github.com/rust-lang/cargo/issues/12336

But the turnaround on Cargo issues is not very quick, so I might have to work around this somehow - perhaps with another call to cargo metadata to get the list of actually existing features, and filter the --cfg 'feature=foo' directives against that? That will slow down the build but at least it will work.

Shnatsel avatar Jul 09 '23 10:07 Shnatsel

I have the same issue building Wasmtime.

The conflict seems to be related to https://github.com/rust-lang/cargo/issues/10788.

The build succeeds without cargo-auditable.

Are there any workarounds for this issue that do not include disabling cargo-auditable?

alexandrevicenzi avatar Aug 24 '23 09:08 alexandrevicenzi

I found a workaround that seems to work.

Patched Wasmtime.

-wasi = ['wasi-cap-std-sync', 'wasmtime-wasi', 'cap-std', 'wasi-common']
+wasi = ['wasi-cap-std-sync', 'wasmtime-wasi', 'dep:cap-std', 'wasi-common']

alexandrevicenzi avatar Aug 25 '23 14:08 alexandrevicenzi

Any update on this? It affects other packages as well, like lightningcss https://github.com/parcel-bundler/lightningcss/issues/702.

JohnRTitor avatar Jun 21 '24 18:06 JohnRTitor

Unfortunately this is a bug in Cargo itself, and AFAIK there is nothing I can really do to fix that in cargo auditable.

The proper way to do this would be in Cargo, or to integrate cargo auditable functionality into Cargo itself making the external tool obsolete. Sadly the progress on upstreaming is slow - the prerequisite https://github.com/rust-lang/rfcs/pull/3553 is not yet merged (although a prototype implementation is underway) and the actual RFC for cargo auditable-like functionality has been recently postponed.

Shnatsel avatar Jun 21 '24 18:06 Shnatsel

I don't really understand, it works in cargo, but with auditable it errors out. Usual workaround is to remove dep: from the features table.

JohnRTitor avatar Jun 21 '24 18:06 JohnRTitor

https://github.com/rust-lang/rfcs/pull/3553 would be the proper, correct solution to this problem.

Until then, there may be hope for working around Cargo passing nonexistent features to rustc, which causes these failures.

When compiling a crate, Cargo sets the current working directory of the child rustc process to the crate’s directory. Therefore, we could discover the Cargo.toml file for the crate, parse it, extract a list of all actually existing features, and filter the features passed by cargo to rustc against that list.

However, that ties cargo auditable to the evolving Cargo.toml format with a lot of complexity and edge cases. We might be able to bypass parsing it ourselves by invoking cargo metadata --offline --no-deps and parsing its output.

Shnatsel avatar Aug 05 '24 21:08 Shnatsel

If you run cargo metadata directly on the Cargo.toml of the dependency, that will generate/update a Cargo.lock file, clobbering the cached source for the crate.

bjorn3 avatar Aug 06 '24 12:08 bjorn3

Fortunately that is not a problem if I run cargo metadata --no-deps, which is what I am going to do. This creates its own problems - it is difficult to determine what package the Cargo.toml actually belongs to, which is silly. Starting with Cargo 1.71 I can use the default_workspace_members field, but for earlier versions I will probably need some cursed heuristics if I want to keep --no-deps.

Besides that, I will only need to run cargo metadata on workspace members, and all workspace members already have their Cargo.lock generated or the build would not proceed. So I could actually afford to run without --no-deps, if I'm willing to take the compilation time hit.

Shnatsel avatar Aug 06 '24 12:08 Shnatsel

I have prototyped a fix in the https://github.com/rust-secure-code/cargo-auditable/tree/fix-dep-features branch. Tests pass, but I don't have a way to check if that fixes the issue because the latest versions of gitoxide, wasmtime and lightningcss build successfully even without these changes.

Shnatsel avatar Aug 06 '24 15:08 Shnatsel

Okay, I managed to reproduce this on gitoxide if I check out v0.24.0 tag. My "fix" doesn't actually fix it. Debugging time!

Shnatsel avatar Aug 06 '24 16:08 Shnatsel

Okay, it seems that cargo metadata is buggy and reports that gix-features has the feature prodash when it actually doesn't, it just has a dep:prodash syntax without the actual feature.

I don't think I can work around the issue in cargo auditable when there are this many different bugs in Cargo around it :face_with_diagonal_mouth:

Shnatsel avatar Aug 06 '24 16:08 Shnatsel