Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upREMOVE ALL THE LICENSE THINGS #53654
Conversation
rust-highfive
assigned
joshtriplett
Aug 24, 2018
rust-highfive
added
the
S-waiting-on-review
label
Aug 24, 2018
mark-i-m
referenced this pull request
Aug 24, 2018
Merged
tidy: Stop requiring a license header #53617
This comment has been minimized.
This comment has been minimized.
|
Looks good to me, let's go ahead with this as soon as the tidy change goes in. Please feel free to r=me once that goes in. |
This comment has been minimized.
This comment has been minimized.
|
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
This comment has been minimized.
This comment has been minimized.
|
For future reference, this is the bash script I am using to delete the licenses. It deletes everything in all subdirectories of the current working directory recursively: for file in `find -type f -name '*.rs'` ; do
sed -i -re '/\/\/ Copyright [0-9-]+ The Rust Project Developers. See the COPYRIGHT/d' $file
sed -i -re '/\/\/ file at the top-level directory of this distribution and at/d' $file
sed -i -re 'N;/\/\/ http:\/\/rust-lang.org\/COPYRIGHT.\n\/\//d' $file
sed -i -re '/\/\/ Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or/d' $file
sed -i -re '/\/\/ http:\/\/www.apache.org\/licenses\/LICENSE-2.0> or the MIT license/d' $file
sed -i -re '/\/\/ <LICENSE-MIT or http:\/\/opensource.org\/licenses\/MIT>, at your/d' $file
sed -i -re '/\/\/ option. This file may not be copied, modified, or distributed/d' $file
sed -i -re '/\/\/ except according to those terms./d' $file
sed -i '/./,$!d' $file
done |
This comment has been minimized.
This comment has been minimized.
|
I would prefer that we do all of the licenses at once rather than in multiple PRs to keep consistency, and also don't see that there's much point in waiting unless I'm missing something. We might/will end up with maybe 2 commits but I'd prefer to avoid doing rote changes like this incrementally since it usually creates dozens of commits that aren't very interesting. |
This comment has been minimized.
This comment has been minimized.
We need to wait for the other PR. Otherwise, travis will fail.
I fine with this, but some poor soul has to review thousands of lines of deleted licenses, and breaking it up might make it a bit easier. Let me know what you think. |
This comment has been minimized.
This comment has been minimized.
|
So I tried running my script on a clean cloned repo (without submodules): $ git status | wc -l
10889So the diff will be about 100k lines... Do you still want me to put it all in one commit? |
This comment has been minimized.
This comment has been minimized.
|
You can add numbered commits for keeping track of review status, and rebase them all into one at the end when merging the PR. |
This comment has been minimized.
This comment has been minimized.
|
Right, we have to wait for a separate PR, but there's no reason to split up this PR so to speak. I am willing to volunteer to verify the diff, which should also be relatively simple to do locally via some rg and uniq I think. |
This comment has been minimized.
This comment has been minimized.
|
Ok, I will push a second commit shortly with the remaining licenses. Thanks! |
mark-i-m
changed the title
Remove a whole bunch of licenses from librustc
REMOVE ALL THE LICENSE THINGS
Aug 24, 2018
This comment has been minimized.
This comment has been minimized.
|
This is by far the largest PR I've ever made |
This comment has been minimized.
This comment has been minimized.
|
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
This comment has been minimized.
This comment has been minimized.
|
@Mark-Simulacrum Could even change |
This comment has been minimized.
This comment has been minimized.
|
@BurntPizza Eventually, but not until the existing ones are removed. |
This comment has been minimized.
This comment has been minimized.
|
Warning: Conflict ahead with #53609... whoever lands first... |
This comment has been minimized.
This comment has been minimized.
|
Thanks for the heads up! |
kennytm
added
S-blocked
and removed
S-waiting-on-review
labels
Aug 24, 2018
This comment has been minimized.
This comment has been minimized.
|
|
mark-i-m
force-pushed the
mark-i-m:licenses0
branch
from
db9922a
to
d1113c3
Aug 24, 2018
This comment has been minimized.
This comment has been minimized.
|
Rebased. Still blocked on #53617, which is part of a rollup. |
This comment has been minimized.
This comment has been minimized.
|
Ugh... I messed up the rebase. |
This comment has been minimized.
This comment has been minimized.
|
@Mark-Simulacrum Hmm... actually I think this might be harder than expected. All of the line numbers in the .stderr files of test will need to be updated. |
mark-i-m
force-pushed the
mark-i-m:licenses0
branch
from
d1113c3
to
d6b98c0
Aug 24, 2018
This comment has been minimized.
This comment has been minimized.
|
@mark-i-m |
This comment has been minimized.
This comment has been minimized.
|
Hmm... I see. So would you recommend going on a sub-crate by sub-crate bases, as I was originally going to do? |
This comment has been minimized.
This comment has been minimized.
|
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
This comment has been minimized.
This comment has been minimized.
|
No, my only recommendation is to cooperate with the infra team on this PR. |
This comment has been minimized.
This comment has been minimized.
So I agree that we should do it in one PR for the reasons you mention... |
This comment has been minimized.
This comment has been minimized.
|
Landing this post-edition seems fine, I don't have a strong opinion. With regards to closing the tree, we certainly can, but as @Centril mentions this might not be the time. I don't really have strong opinions in that regard. |
This comment has been minimized.
This comment has been minimized.
|
I don't mind waiting |
This comment has been minimized.
This comment has been minimized.
|
Hi, could you add the bash script you ran into the commit message? |
This comment has been minimized.
This comment has been minimized.
|
I think this shouldn't be blocked anymore since #53654 has been merged? |
This comment has been minimized.
This comment has been minimized.
|
@bemeurer I think you mean #53617. That's true, but we are waiting for the edition release because changing this many files (especially the tests) will require some coordination to avoid conflicts. |
This comment has been minimized.
This comment has been minimized.
|
I'm going to close this for now since I think it's going to need to be recreated anyway in a few weeks/months; probably ~weeks since once RC 1 goes out edition-related development in this repository should slow down somewhat. |
mark-i-m commentedAug 24, 2018
•
edited
Pursuant to #53617 and #43498, all license headers are heretofore banished!
r? @joshtriplett
This is blocked on #53617There are no actual changes in this PR, just removing all of the licenses.
I will submit more of these if this seems to you like an ok way to proceed. I tried to keep it all in one commit to avoid cluttering the git history.
EDIT: copied from below. Here is the script I used to remove the headers:
EDIT2: This is currently blocked on the 2018 edition release. Removing all of these licenses will require locking the tree briefly, and we don't want to slow down higher-priority work on the edition right now.