tidy: Better license checks. #69443

ehuss · 2020-02-24T22:03:53Z

This implements some improvements to the license checks in tidy:

Use cargo_metadata instead of parsing vendored crates. This allows license checks to run without vendoring enabled, and allows the checks to run on PR builds.
Check for stale entries.
Check that the licenses for exceptions are what we think they are.
Verify exceptions do not leak into the runtime.

Closes #62618
Closes #62619
Closes #63238 (I think)

There are some substantive changes here. The follow licenses have changed from the original comments:

openssl BSD+advertising clause to Apache-2.0
pest MPL2 to MIT/Apache-2.0
smallvec MPL2 to MIT/Apache-2.0
clippy lints MPL2 to MIT OR Apache-2.0

rust-highfive · 2020-02-24T22:04:04Z

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

ehuss · 2020-02-24T22:04:10Z

cc @brson @gnzlbg FYI

ehuss · 2020-02-24T22:10:59Z

src/tools/tidy/src/deps.rs

 ];

+/// These are the root crates that are part of the runtime. The licenses for
+/// these and all their dependencies *must not* be in the exception list.
+const RUNTIME_CRATES: &[&str] = &["std", "core", "alloc", "panic_abort", "panic_unwind"];


I am assuming libtest is not really considered the "runtime" in the same vein as std is, but I can add it if people think it should. It adds a number of dependencies, but they are all permissively licensed.

I think this should include libtest, particularly as it might be linked in for e.g. black_box even in non-test executables.

Sure. It can always be revisited if someone wants to add an exception, it's probably better to be safer here. (although black_box is in core now, right?)

Just FYI for anyone curious, test adds the following dependencies:

getopts

proc_macro

rustc-std-workspace-std

term

test

unicode-width

I would add test as a runtime crate just to be on the conservative side. test will still be linked at runtime, (while procmacro etc. won't). Even if it is just in the test binary, it might still be confusing that the Rust compiler produces binaries with different license checking guarantees.

ehuss · 2020-02-24T22:11:06Z

src/tools/tidy/src/deps.rs

+    // Build dependencies *shouldn't* matter unless they do some kind of
+    // codegen. For now we'll assume they don't.


I could change this to include build deps if people think it should. That is, should we restrict the licenses of build deps of libstd in case they inject code into the resulting crate?

Mark-Simulacrum · 2020-02-25T02:01:30Z

src/tools/tidy/src/deps.rs

+    "winapi-util",
+    "winapi-x86_64-pc-windows-gnu",
+    "wincolor",
+    "hermit-abi",


Since we're changing the whole list anyway, mind sorting it while we're at it?

Mark-Simulacrum · 2020-02-25T02:02:36Z

src/tools/tidy/src/deps.rs

+                // This package doesn't declare a license expression. Manual
+                // inspection of the license file is necessary, which appears
+                // to be BSD-3-Clause.
+                assert!(pkg.license.is_none());


cc @cramertj (?) -- could we get this populated perhaps?

nikomatsakis · 2020-02-25T20:03:35Z

cc @skade

Dylan-DPC-zz · 2020-03-11T19:37:32Z

r? @Mark-Simulacrum

Mark-Simulacrum · 2020-03-11T22:16:07Z

I'm going to r? @skade here since they've been involved with license work

skade · 2020-03-12T06:50:16Z

r+

I'm okay with this, with the comment that I posted above: #69443 (comment). This probably merits its own ticket and shouldn't be discussed in a comment section, it's rather unclear what the story there should be. Is test part of the runtime or not?

Thanks for the patch, @ehuss

Mark-Simulacrum · 2020-03-12T10:58:35Z

@bors r=skade,Mark-Simulacrum

I've done my best to also check the implementation, and it looks correct to me.

bors · 2020-03-12T10:58:36Z

📌 Commit 432bec14aefc3c23b4d4a4c8d8fb873eb6da9c81 has been approved by skade,Mark-Simulacrum

Removed in rust-lang#69247 while this PR was waiting to merge.

ehuss · 2020-03-13T02:25:37Z

@bors r=skade,Mark-Simulacrum
Some tests were broken since this was opened (removed chalk-engine and chalk-macros from exception list, removed in #69247).

bors · 2020-03-13T02:25:38Z

📌 Commit ed0158d has been approved by skade,Mark-Simulacrum

…mulacrum tidy: Better license checks. This implements some improvements to the license checks in tidy: * Use `cargo_metadata` instead of parsing vendored crates. This allows license checks to run without vendoring enabled, and allows the checks to run on PR builds. * Check for stale entries. * Check that the licenses for exceptions are what we think they are. * Verify exceptions do not leak into the runtime. Closes rust-lang#62618 Closes rust-lang#62619 Closes rust-lang#63238 (I think) There are some substantive changes here. The follow licenses have changed from the original comments: * openssl BSD+advertising clause to Apache-2.0 * pest MPL2 to MIT/Apache-2.0 * smallvec MPL2 to MIT/Apache-2.0 * clippy lints MPL2 to MIT OR Apache-2.0

@ghost

Rollup of 9 pull requests Successful merges: - #69036 (rustc: don't resolve Instances which would produce malformed shims.) - #69443 (tidy: Better license checks.) - #69814 (Smaller and more correct generator codegen) - #69929 (Regenerate tables for Unicode 13.0.0) - #69959 (std: Don't abort process when printing panics in tests) - #69969 (unix: Set a guard page at the end of signal stacks) - #70005 ([rustdoc] Improve visibility for code blocks warnings) - #70088 (Use copy bound in atomic operations to generate simpler MIR) - #70095 (Implement -Zlink-native-libraries) Failed merges: r? @ghost

@ghost

Rollup of 9 pull requests Successful merges: - #68941 (Properly handle Spans that reference imported SourceFiles) - #69036 (rustc: don't resolve Instances which would produce malformed shims.) - #69443 (tidy: Better license checks.) - #69814 (Smaller and more correct generator codegen) - #69929 (Regenerate tables for Unicode 13.0.0) - #69959 (std: Don't abort process when printing panics in tests) - #69969 (unix: Set a guard page at the end of signal stacks) - #70005 ([rustdoc] Improve visibility for code blocks warnings) - #70088 (Use copy bound in atomic operations to generate simpler MIR) Failed merges: r? @ghost

rust-highfive assigned nikomatsakis Feb 24, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 24, 2020

ehuss commented Feb 24, 2020

View reviewed changes

Mark-Simulacrum reviewed Feb 25, 2020

View reviewed changes

rust-highfive assigned Mark-Simulacrum and unassigned nikomatsakis Mar 11, 2020

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 12, 2020

ehuss added 8 commits March 12, 2020 19:19

tidy: Use cargo_metadata for license checks.

349fcb9

tidy: Check if exceptions are no longer used.

3f45da0

tidy: check the licenses don't change

be10f14

tidy: Verify the runtime crates don't have license exceptions.

bc738f2

tidy: Add some clarifying comments in license checks.

3c48c89

tidy: Add test to RUNTIME_CRATES.

2e46faa

tidy: Sort WHITELIST.

9a53cf3

tidy: Remove chalk-engine/chalk-macros.

ed0158d

Removed in rust-lang#69247 while this PR was waiting to merge.

ehuss force-pushed the tidy-license branch from 432bec1 to ed0158d Compare March 13, 2020 02:23

Manishearth mentioned this pull request Mar 17, 2020

Rollup of 12 pull requests #70085

Closed

Centril mentioned this pull request Mar 19, 2020

Rollup of 9 pull requests #70130

Closed

Centril mentioned this pull request Mar 19, 2020

Rollup of 9 pull requests #70132

Merged

bors merged commit 61fe2e4 into rust-lang:master Mar 19, 2020

SimonSapin mentioned this pull request Mar 20, 2020

Importing modules from inside include! no longer works #70185

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tidy: Better license checks. #69443

tidy: Better license checks. #69443

ehuss commented Feb 24, 2020

rust-highfive commented Feb 24, 2020

ehuss commented Feb 24, 2020

ehuss Feb 24, 2020

Mark-Simulacrum Feb 25, 2020

ehuss Feb 25, 2020

skade Mar 12, 2020

ehuss Feb 24, 2020 •

edited

Loading

Mark-Simulacrum Feb 25, 2020

Mark-Simulacrum Feb 25, 2020

nikomatsakis commented Feb 25, 2020

Dylan-DPC-zz commented Mar 11, 2020

Mark-Simulacrum commented Mar 11, 2020

skade commented Mar 12, 2020 •

edited

Loading

Mark-Simulacrum commented Mar 12, 2020

bors commented Mar 12, 2020

ehuss commented Mar 13, 2020

bors commented Mar 13, 2020

		// Build dependencies shouldn't matter unless they do some kind of
		// codegen. For now we'll assume they don't.

tidy: Better license checks. #69443

tidy: Better license checks. #69443

Conversation

ehuss commented Feb 24, 2020

rust-highfive commented Feb 24, 2020

ehuss commented Feb 24, 2020

ehuss Feb 24, 2020

Choose a reason for hiding this comment

Mark-Simulacrum Feb 25, 2020

Choose a reason for hiding this comment

ehuss Feb 25, 2020

Choose a reason for hiding this comment

skade Mar 12, 2020

Choose a reason for hiding this comment

ehuss Feb 24, 2020 • edited Loading

Choose a reason for hiding this comment

Mark-Simulacrum Feb 25, 2020

Choose a reason for hiding this comment

Mark-Simulacrum Feb 25, 2020

Choose a reason for hiding this comment

nikomatsakis commented Feb 25, 2020

Dylan-DPC-zz commented Mar 11, 2020

Mark-Simulacrum commented Mar 11, 2020

skade commented Mar 12, 2020 • edited Loading

Mark-Simulacrum commented Mar 12, 2020

bors commented Mar 12, 2020

ehuss commented Mar 13, 2020

bors commented Mar 13, 2020

ehuss Feb 24, 2020 •

edited

Loading

skade commented Mar 12, 2020 •

edited

Loading