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 doesn't check the dependencies from submodules/subtrees #83554

Closed
JohnTitor opened this issue Mar 27, 2021 · 3 comments
Closed

Tidy doesn't check the dependencies from submodules/subtrees #83554

JohnTitor opened this issue Mar 27, 2021 · 3 comments
Assignees
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@JohnTitor
Copy link
Member

JohnTitor commented Mar 27, 2021

Filing this so that I don't forget to fix it...
We have a license check on tidy for each dependency but it doesn't check dependencies coming from submodules/subtrees. For instance, tidy doesn't say anything about the libm crate, which comes from rustfmt (rustfmt-nightly -> bytecount -> packed_simd2 -> libm). Ideally, tidy should also check those dependencies.

Originally discussed on #83239 (comment).

@JohnTitor JohnTitor added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) C-bug Category: This is a bug. labels Mar 27, 2021
@JohnTitor JohnTitor self-assigned this Mar 27, 2021
@ehuss
Copy link
Contributor

ehuss commented Mar 27, 2021

Can you say more about why tidy would complain about adding libm? From what I see, it has a "MIT or Apache-2.0" license which is in the blanket allow list.

The license checks should be applied to everything in Cargo.lock. There are 3 categories (runtime, rustc, and tools) which are each treated differently. Only rustc has a "speedbump" list (PERMITTED_DEPENDENCIES), is that maybe what this is referring to? I'm not sure it has ever been discussed using that for anything other than rustc.

There's only one thing that is not validated that I am aware of, and that is rust-analyzer (#74269) because it is not in the lock file.

@JohnTitor
Copy link
Member Author

Oh, I totally misunderstood! So, we actually run the license check but PERMITTED_DEPENDENCIES only has direct rustc_* crates dependencies, right? Then I think the issue is that whether we want to expand the list to submodules/subtrees but I'd like to see @Mark-Simulacrum's opinion since I don't have a strong thought for it.

@JohnTitor JohnTitor added C-enhancement Category: An issue proposing an enhancement or a PR with one. and removed C-bug Category: This is a bug. labels Mar 28, 2021
@Mark-Simulacrum
Copy link
Member

Ah, interesting. Yeah, I don't think the speed bump is directly necessary at this time, thanks for clarifying.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

No branches or pull requests

3 participants