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

chore: Properly extract copyright information from bundled packages #45833

Merged
merged 3 commits into from
Jun 16, 2024

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Jun 12, 2024

Summary

Currently we only extract comments, this does not work as most dependencies are either minified or do not contain license headers.
So this instead adds a webpack plugin to iterate all bundled sources, extract the license information and create a proper .license file.

I really hope to drop this when migrating to vite 😅

Checklist

@susnux susnux added the 3. to review Waiting for reviews label Jun 12, 2024
@AndyScherzinger
Copy link
Member

Hi @carmenbianca - just wanted to get some feedback (given you have some time for it of course). 😃

During the quest of making the Nextcloud server REUSE compliant we faces Javascript compiled assets - which are combined from various sources - welcome to modern web development... - so they come with various licenses in some scenarios, like

https://github.com/nextcloud/server/pull/45833/files#diff-efb7ecbf3b0f9ef7721e44bea59a37e4a447e4f64e190d21dcc61befead2b719

as one example. Given the licenses are compatible would the AND operator be the right choice? I guess so, but just wanted to be sure, since I personally got it wrong on the first try of the Android repo and than had to fix it afterwards, thanks to your feedback.

@AndyScherzinger AndyScherzinger added this to the Nextcloud 30 milestone Jun 12, 2024
@carmenbianca
Copy link

@AndyScherzinger If I understand correctly, the source code from multiple files is mashed into a single file?

AND is a safe bet in that case.

Alternatively, instead of doing AND, you can define the SPDX-License-Identifier: tag several times, once for each source file. I'd have to re-read the SPDX spec, but these tags should be implicitly AND-ed.

@AndyScherzinger
Copy link
Member

@carmenbianca yes, that is correct.

So @susnux than we can keep the current logic for the moment.

Of course it would still be interesting to know if a list of license ID lines would implicitly be ANDed.

@AndyScherzinger AndyScherzinger force-pushed the fix/properly-extract-licenses branch 3 times, most recently from c98a8e4 to 9f18646 Compare June 14, 2024 07:46
@susnux susnux force-pushed the fix/properly-extract-licenses branch from f2b9b7a to bcb7977 Compare June 14, 2024 15:42
@nextcloud nextcloud deleted a comment from susnux Jun 14, 2024
@susnux susnux force-pushed the fix/properly-extract-licenses branch from bcb7977 to 61d3e5a Compare June 14, 2024 18:04
…ed assets

This will create proper extracted license information for assets and stores it in `fist/file.js.license`.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
package.json Outdated Show resolved Hide resolved
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@AndyScherzinger
Copy link
Member

/compile /

Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
@AndyScherzinger AndyScherzinger merged commit c9d8357 into master Jun 16, 2024
109 checks passed
@AndyScherzinger AndyScherzinger deleted the fix/properly-extract-licenses branch June 16, 2024 18:16
@AndyScherzinger
Copy link
Member

Woohoo, all green @susnux 🎉

Now I will have to find the wrong license identifier sources and how to get the license comment into some dist JS file where there is none.

Again thanks a lot for your contributions in getting this topic done for the server repo! Makes my days 💯🎉🙏😊👍💪

@blizzz blizzz mentioned this pull request Jul 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants