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

Find license identifiers in comments with ASCII art frames #560

Merged
merged 1 commit into from
Sep 22, 2022

Conversation

pietroalbini
Copy link
Contributor

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:

reuse._util - ERROR - Could not parse 'Apache-2.0 WITH LLVM-exception                    *|'
reuse.project - ERROR - 'src/llvm-project/lldb/source/Plugins/Plugins.def.in' holds an SPDX expression that cannot be parsed, skipping the file                                                                      

As #343 correctly pointed out, the problem is that LLVM uses ASCII art "frames" for those code comments, like:

/***********************************************************\
|* SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception *|
\***********************************************************/

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 before SPDX-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

Copy link
Member

@carmenbianca carmenbianca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@pietroalbini
Copy link
Contributor Author

Thanks for the quick review! I should've addressed everything.

Copy link
Member

@mxmehl mxmehl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, that's a clever approach a nice first step!

@pietroalbini
Copy link
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`
@mxmehl mxmehl added this to the v1.1.0 milestone Aug 31, 2022
@carmenbianca
Copy link
Member

@pietroalbini Sorry for the delay. This gets merged now for the next release, which shouldn't be too long away.

@carmenbianca carmenbianca merged commit 373f6a8 into fsfe:master Sep 22, 2022
@pietroalbini pietroalbini deleted the pa-ascii-art-frame branch September 22, 2022 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants