Find license identifiers in comments with ASCII art frames#560
Merged
carmenbianca merged 1 commit intofsfe:masterfrom Sep 22, 2022
Merged
Find license identifiers in comments with ASCII art frames#560carmenbianca merged 1 commit intofsfe:masterfrom
carmenbianca merged 1 commit intofsfe:masterfrom
Conversation
11 tasks
carmenbianca
approved these changes
Jul 18, 2022
Member
carmenbianca
left a comment
There was a problem hiding this comment.
Looks good to me. I'm not sure if the case this covers is a good general case, but we can cross that bridge when someone else's ASCII art breaks.
Thanks for the PR! Please make sure to add a change log entry, and add yourself to AUTHORS.rst if you want.
50deb5f to
ffe9692
Compare
Contributor
Author
|
Thanks for the quick review! I should've addressed everything. |
mxmehl
approved these changes
Jul 26, 2022
Member
mxmehl
left a comment
There was a problem hiding this comment.
Thanks, that's a clever approach a nice first step!
Contributor
Author
|
Hey all, thanks for the reviews! What are the next steps to get this merged and released? |
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Aug 20, 2022
…Simulacrum Initial implementation of REUSE This PR implements the first two steps of rust-lang#99414 by: * Adding some scaffolding for REUSE. The `.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 that `reuse lint` works. * Runs `reuse lint` in CI, in the `mingw-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 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`
Member
|
@pietroalbini Sorry for the delay. This gets merged now for the next release, which shouldn't be too long away. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The Rust project is working to adopt REUSE to annotate licenses, but when working on the initial implementation I stumbled on #343. The Rust repository has LLVM as one of its submodules, and REUSE currently errors out on some of the LLVM comments:
As #343 correctly pointed out, the problem is that LLVM uses ASCII art "frames" for those code comments, like:
The solution I came up with is to implement generic handling for these kinds of ASCII art, even in cases where the multiline delimiter (in this case
*) is not present. When there are some non-whitespace chars beforeSPDX-License-Identifier, the new code tries to strip the reverse of them from the end of the line. That correctly handles LLVM comments, but also any other ASCII art frame that's symmetric.Fixes #343