-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Initial implementation of REUSE #99415
Conversation
Should we just bump that builder to 22.04? mingw-check doesn't produce artifacts and to my knowledge we're not relying on it to test old Ubuntu compat in some other way either. This is hopefully not too hard to do. |
While REUSE is indeed in the Ubuntu 22.04 repositories (yay!), REUSE currently has a bug when parsing some files in the LLVM source code. This means REUSE will fail when running it in source tarballs of rustc, and that bug prevents us from passing the |
Hm, okay. I think we should still track using a distro version -- it's much easier for local experimentation, at least, if I don't need to mess with pip :) Let's also update the PR description with that justification, which seems much better than "not in 18.04". I think as-is this PR doesn't really merit merging, since we're basically not adding any testing (that I can see) and add a presumably quickly unnecessary TODO license which is kinda weird (and with our luck, will break someone downstream who is validating that we're MIT + APACHE 2.0).
Maybe this is a key point I don't yet follow -- is this something you expect a Debian developer to do? Naively, I'd expect us to just be copying a few files here.
I'm also wondering how this will work down the line. I think there's definitely value regardless, but it seems like if we can blanket-override license metadata present in files (e.g., headers) it becomes much easier to accidentally declare an overly broad generalization. Does reuse lint yell at us if we have a file header that's incompatible with |
Won't have time to address feedback before I get back from vacation, marking it as a draft. |
Done.
Yeah, my expectation was to ping the Debian developer I've been in contact with (@sylvestre) once the PR was merged to let him upload their contents. I guess I can also do them myself in this PR if you prefer.
I did some digging and experimentation, and it turns out the current logic of what REUSE includes is undocumented and a bit weird. There are three sources of licensing information:
The way REUSE currently includes them is There is an open issue about this that is blocked on defining the precedence, which is blocked on defining a new In any way, the consensus in fsfe/reuse-tool#246 seems to be to ignore the comments in the file if a dep5 file is present, not the other way around. Given all of this, it's going to be easier to define all our licensing metadata in the Within a Another problem I discovered in my issue tracker rabbit hole is that |
To be clear, I agree and want to, but my worry was more about us silently overriding licenses on files without realizing (e.g., as you say from subtrees). It seems like we might be able to run with two different reuse configs (one basically blank) and lint that files don't set anything that conflicts with that? Not sure if it's possible given the precedence order you suggest is a consensus (which seems weird to me... local state should typically win IMO), but maybe that's something to follow up that chain on. I think this is all likely to be an improvement regardless, not a blocking concern, but I'd love for tooling to better support us here. I guess in Rust code we'll keep our tidy lints banning file specific headers and make sure they include dep5 headers. I'll review the patch itself in a bit, but can you say if there's reasons to wait to import the Debian declarations? Seems like doing them in this PR gives us a better picture. |
As you wish :) |
Reading the thread again, the reasoning for that choice was overriding the license defined in vendored files that can't be touched, if for example they include invalid syntax. The discussion then moved to fsfe/reuse-docs#70, when there are multiple alternatives being discussed.
Ok, I'll import the annotations in this PR either later today or tomorrow. |
Co-Authored-By: Angus Lees <[email protected]> Co-Authored-By: Fabian Grünbichler <[email protected]> Co-Authored-By: Hiroaki Nakamura <[email protected]> Co-Authored-By: Jordan Justen <[email protected]> Co-Authored-By: Luca Bruno <[email protected]> Co-Authored-By: Sylvestre Ledru <[email protected]> Co-Authored-By: Ximin Luo <[email protected]>
Imported the debian copyright file and removed all the |
I am impressed, @pietroalbini, how well you figured out the current chain of blockers of REUSE.yaml, which would solve the problems mentioned here as well as many others! Indeed, spdx/spdx-spec#502 is basically the first thing we'd need to tackle. Please feel free to share your interest and opinion on this issue. Inside SPDX, a lot of workforce is currently used up by defining the whole format of SPDX 3.0, but it may be good to already consider the REUSE.yaml usecase. |
OK, I'm going to go ahead and approve this PR; it seems like the right step on #99414. It's also a fairly easily reversible decision, and while I think there's a number of interesting questions to answer around that (e.g., how we unify this with tidy's license checking, etc.), this is still roughly moving us in the right direction. I'll also note that I didn't review the exact associations mapped here; I think we'll want to iterate further and probably seek help from Foundation resources on understanding what we want licenses to be (and work to reconcile with what they are). @bors r+ rollup=iffy |
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#99415 (Initial implementation of REUSE) - rust-lang#99544 (Expose `Utf8Lossy` as `Utf8Chunks`) - rust-lang#100585 (Fix trailing space showing up in example) - rust-lang#100596 (Remove unnecessary stderr files) - rust-lang#100642 (Update fortanix-sgx-abi and export some useful SGX usercall traits) - rust-lang#100691 (Make `same_type_modulo_infer` a proper `TypeRelation`) - rust-lang#100693 (Add LLVM15-specific codegen test for `try`/`?`s that now optimize away) - rust-lang#100710 (Windows: Load synch functions together) - rust-lang#100807 (Add TaKO8Ki to translation-related mention groups) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
This PR implements the first two steps of #99414 by:
.reuse/dep5
file now marks every file as the custom "TODO" license, which I'll remove in a future PR once Debian imports their metadata. The TODO license is needed so thatreuse lint
works.reuse lint
in CI, in themingw-check
builder. REUSE currently has a bug when parsing some files in the LLVM source code. This means REUSE will fail when running it in source tarballs of rustc, and that bug prevents us from passing the--include-submodules
flag in CI. I opened Find license identifiers in comments with ASCII art frames fsfe/reuse-tool#560 upstream with a fix, and as soon as it's merged/released I planned to bump the pinned version to include the fix we need.r? @Mark-Simulacrum