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

tidy license check error for crossbeam-queue #92677

Closed
pierwill opened this issue Jan 8, 2022 · 7 comments · Fixed by #92740
Closed

tidy license check error for crossbeam-queue #92677

pierwill opened this issue Jan 8, 2022 · 7 comments · Fixed by #92740

Comments

@pierwill
Copy link
Member

pierwill commented Jan 8, 2022

Getting this error in #90842:

tidy error: invalid license `MIT/Apache-2.0 AND BSD-2-Clause` in `crossbeam-queue 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)`

I think this is a bug in tidy.

In src/tools/tidy/src/deps.rs EXCEPTIONS, there is already an exception for crossbeam-queue. However, exceptions are not allowed for the runtime crates that make up the standard library.

With the backtrace feature, std depends on object. Then object has a conditional dependency on indexmap, but this is not enabled under std. However, if you look at the whole workspace metadata at once (as tidy does), then the compiler enables the object features that require indexmap, and now also the indexmap feature for rustc-rayon too.

So tidy sees std -> object -> indexmap -> rustc-rayon -> rustc-rayon-core -> crossbeam-queue in the metadata, and thus includes all of that in "runtime" crates, and then the exception isn't allowed. But if you look at cargo tree --all-features -p std, they aren't really included at all.

Originally posted by @cuviper in #90842 (comment)

@pierwill
Copy link
Member Author

pierwill commented Jan 8, 2022

@rustbot label +E-help-wanted

@rustbot rustbot added the E-help-wanted Call for participation: Help is requested to fix this issue. label Jan 8, 2022
@pierwill
Copy link
Member Author

pierwill commented Jan 10, 2022

@oli-obk @rust-lang/cargo I'm having a hard time understanding how to approach fixing this.

Discussion on zulip went like:

cuviper: I'm not sure the data at hand will make it clear whether something
is a "true" runtime dependency, vs one enabled by compiler features

pierwill: Ah so it's a matter of tidy not having enough information about
where the dep is actually used?

cuviper: yeah, I think it's basically seeing a global view of the compiler+std
workspace, with unified features. But for that check of exclusions vs runtime
(std) crates, only the std metadata really matters.

pierwill: looks like tidy uses cargo_metadata for this...

@ehuss
Copy link
Contributor

ehuss commented Jan 10, 2022

I would start looking at the code here. One option is to run cargo metadata twice, once for the standard library and once for everything else. The one for the standard library would use library/test/Cargo.toml as the manifest path. It should then resolve features for just the standard library. (You may need to also set the current_dir, I don't remember.)

@cuviper
Copy link
Member

cuviper commented Jan 10, 2022

The only difference I see in metadata is "root": null versus "root": "test ...", and changing the current directory doesn't seem to affect it either. The tidy check is not using that root, but tries to walk dependencies from its own list of "runtime" crates.

@ehuss
Copy link
Contributor

ehuss commented Jan 10, 2022

Oh, hm, I forgot that the resolve was forced to be workspace-wide.

I don't think the information can be extracted from cargo at this time. You may need to add an exception here, and clearly document it.

Or. It looks like the license for crossbeam-queue has been changed. Would it be possible to update rustc-rayon-core to use crossbeam-queue 0.3?

@cuviper
Copy link
Member

cuviper commented Jan 10, 2022

Would it be possible to update rustc-rayon-core to use crossbeam-queue 0.3?

Oh, hmm, I can probably do that.

Really that fork needs a rebase onto current rayon-core, which doesn't even use crossbeam-queue at all, but that's a bigger ask since a lot of the core code changed significantly. Maybe I can just cherry-pick that part of the dependency changes.

@pierwill
Copy link
Member Author

@rustbot label -E-help-wanted

@rustbot rustbot removed the E-help-wanted Call for participation: Help is requested to fix this issue. label Jan 12, 2022
@bors bors closed this as completed in 42852d7 Jan 16, 2022
flip1995 pushed a commit to flip1995/rust that referenced this issue Jan 27, 2022
Update rayon and rustc-rayon

This updates rayon for various tools and rustc-rayon for the compiler's parallel mode.

- rayon v1.3.1 -> v1.5.1
- rayon-core v1.7.1 -> v1.9.1
- rustc-rayon v0.3.1 -> v0.3.2
- rustc-rayon-core v0.3.1 -> v0.3.2

... and indirectly, this updates all of crossbeam-* to their latest versions.

Fixes rust-lang#92677 by removing crossbeam-queue, but there's still a lingering question about how tidy discovers "runtime" dependencies. None of this is truly in the standard library's dependency tree at all.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants