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

Can't build recent gitoxide versions #124

Open
jcgruenhage opened this issue Jul 9, 2023 · 15 comments
Open

Can't build recent gitoxide versions #124

jcgruenhage opened this issue Jul 9, 2023 · 15 comments
Labels
bug Something isn't working third party Work item for a third-party dependency

Comments

@jcgruenhage
Copy link

jcgruenhage commented Jul 9, 2023

https://github.com/Byron/gitoxide doesn't successfully build with cargo-auditable anymore. It fails in

panic!(
"cargo metadata failure: {}",
String::from_utf8_lossy(&output.stderr)
);
. 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.

@Shnatsel
Copy link
Member

Shnatsel commented Jul 9, 2023

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!

@jcgruenhage
Copy link
Author

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

@Shnatsel
Copy link
Member

Shnatsel commented Jul 9, 2023

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
Copy link
Member

Shnatsel commented Jul 9, 2023

I've filed an issue upstream: rust-lang/cargo#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.

@alexandrevicenzi
Copy link

alexandrevicenzi commented Aug 24, 2023

I have the same issue building Wasmtime.

The conflict seems to be related to rust-lang/cargo#10788.

The build succeeds without cargo-auditable.

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

@alexandrevicenzi
Copy link

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']

Byron added a commit to GitoxideLabs/gitoxide that referenced this issue Sep 14, 2023
Use `prodash` instead of `dep:prodash` in gix-features and `tracing`
instead of `dep:tracing` in gitoxide-core.

The `dep:mydep` syntax removes the implicit `mydep` feature for optional
dependencies, this triggers a bug in cargo that affects
`cargo-auditable`. See rust-lang/cargo#12336

This affects some Linux distributions like NixOS which use
`cargo-auditable` by default. Related issues:

- NixOS/nixpkgs#253911
- rust-secure-code/cargo-auditable#124
Byron pushed a commit to GitoxideLabs/gitoxide that referenced this issue Sep 14, 2023
Use `prodash` instead of `dep:prodash` in gix-features and `tracing`
instead of `dep:tracing` in gitoxide-core.

The `dep:mydep` syntax removes the implicit `mydep` feature for optional
dependencies, this triggers a bug in cargo that affects
`cargo-auditable`. See rust-lang/cargo#12336

This affects some Linux distributions like NixOS which use
`cargo-auditable` by default. Related issues:

- NixOS/nixpkgs#253911
- rust-secure-code/cargo-auditable#124
@JohnRTitor
Copy link

Any update on this? It affects other packages as well, like lightningcss parcel-bundler/lightningcss#702.

@Shnatsel
Copy link
Member

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 rust-lang/rfcs#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 Shnatsel added bug Something isn't working third party Work item for a third-party dependency labels Jun 21, 2024
@JohnRTitor
Copy link

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.

@Shnatsel
Copy link
Member

Shnatsel commented Aug 5, 2024

rust-lang/rfcs#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.

@bjorn3
Copy link

bjorn3 commented Aug 6, 2024

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.

@Shnatsel
Copy link
Member

Shnatsel commented Aug 6, 2024

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
Copy link
Member

Shnatsel commented Aug 6, 2024

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
Copy link
Member

Shnatsel commented Aug 6, 2024

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
Copy link
Member

Shnatsel commented Aug 6, 2024

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 🫤

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working third party Work item for a third-party dependency
Projects
None yet
Development

No branches or pull requests

5 participants