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

Add automatic JS license generation #11810

Merged
merged 9 commits into from
Jun 12, 2020
Merged

Conversation

silverwind
Copy link
Member

@silverwind silverwind commented Jun 8, 2020

Removed librejs file and replaced it with a plaintext file that is built from all JS dependencies that are included in the webpack build. It does not cover the few remaining statically vendored files and fomantic is added manually because it's not yet in the webpack build process.

Fixes: #11630

Sample License (source):

--------------------------------------------------------------------------------
[email protected] - MIT
--------------------------------------------------------------------------------
# The MIT License

Permission is hereby granted, free of charge, to any person obtaining a copy of
this software and associated documentation files (the 'Software'), to deal in
the Software without restriction, including without limitation the rights to
use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of
the Software, and to permit persons to whom the Software is furnished to do so,
subject to the following conditions:

The above copyright notice and this permission notice shall be included in all
copies or substantial portions of the Software.

THE SOFTWARE IS PROVIDED 'AS IS', WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER
IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.

@6543
Copy link
Member

6543 commented Jun 8, 2020

lint error

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jun 8, 2020
@silverwind
Copy link
Member Author

Lint fixed. Fingers crossed the integration test passes.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jun 8, 2020
@silverwind
Copy link
Member Author

silverwind commented Jun 8, 2020

Completed GET /js/licenses.txt 404 Not Found in 331.473µs
    links_test.go:41: 
          Error Trace:  integration_test.go:427
                              links_test.go:41
          Error:        Not equal: 
                        expected: 200
                        actual  : 404
          Test:         TestLinksNoLogin
          Messages:     Request: GET /js/licenses.txt

I guess webpack files are not available by the time these test run. Should I just remove that test or try getting the webpack build in? (which might cause the tests to run 2-3 mins longer)

@6543
Copy link
Member

6543 commented Jun 8, 2020

@silverwind I think we can remove this url from the test

@silverwind
Copy link
Member Author

silverwind commented Jun 8, 2020

You're right. While I do think that there is some value in testing the webpack build output, it's probably not warranted the extra load it creates on CI. Frontend build was removed in #11159 from the testing steps, btw.

@lafriks
Copy link
Member

lafriks commented Jun 8, 2020

But it is not compatible with librejs imho

@silverwind
Copy link
Member Author

silverwind commented Jun 8, 2020

LibreJS is a meme imho. Its Firefox extension has a whopping 257 users and their approach can not possibly work with bundlers like webpack anyways.

@silverwind
Copy link
Member Author

silverwind commented Jun 8, 2020

I'm not against the idea of librejs but I see no sane way of handing out the licensing info in a parsable format. Of course, if there is a webpack plugin to handle this task, I'm all for adding it, but there appears to be none. We certainly don't want to manually handle this like we did before (the file had many errors and was generally inaccurate because it did not list indirect dependencies).

Also see https://lists.gnu.org/archive/html/bug-librejs/2019-02/msg00003.html

@techknowlogick techknowlogick added the type/docs This PR mainly updates/creates documentation label Jun 9, 2020
@techknowlogick techknowlogick added this to the 1.13.0 milestone Jun 9, 2020
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jun 9, 2020
@6543
Copy link
Member

6543 commented Jun 9, 2020

🚀

@silverwind
Copy link
Member Author

BTW we do have the option to generate HTML as well using this mechanism but I see no point in doing it just for Librejs because they can only block non-free scripts on a per-file level so for example would block our index.js chunk if one of the modules within is non-free so the approach with the HTML is systematically flawed and the only proper way to do it would be inline comments in the js files but Librejs does not yet support that.

Removed librejs file and replaced it with a plaintext file that is built
from all JS dependencies that are included in the webpack build. It does
not cover the few remaining statically vendored files and fomantic is
added manually because it's not yet in the webpack build process.

Fixes: go-gitea#11630
@silverwind
Copy link
Member Author

Rebased and added one more tweak that localizes the previously hardcoded word licenses.

@silverwind
Copy link
Member Author

silverwind commented Jun 10, 2020

Reduced footer text to just "Licences". I feel it's not fully correct to label them as JS as we're also depending on modules that only provide like svg/css/fonts and more.

@silverwind
Copy link
Member Author

With vendored golang dependencies, it shouldn't be too hard to also add them to the output. Once they are unvendored, it will be more tricky thought and we'd probably need a tool to retrieve them.

@lafriks lafriks merged commit bc4f7ba into go-gitea:master Jun 12, 2020
@silverwind silverwind deleted the license branch June 12, 2020 11:38
ydelafollye pushed a commit to ydelafollye/gitea that referenced this pull request Jul 31, 2020
* Add automatic JS license generation

Removed librejs file and replaced it with a plaintext file that is built
from all JS dependencies that are included in the webpack build. It does
not cover the few remaining statically vendored files and fomantic is
added manually because it's not yet in the webpack build process.

Fixes: go-gitea#11630

* fix lint

* remove jslicense, we're not librejs compatible any more

* remove license.txt test as it depens on absent files

* small optimization

* trailing comma

* localize and capitalize the word 'licenses'

* reduce text to just 'Licenses'

Co-authored-by: Lauris BH <[email protected]>
silverwind added a commit to silverwind/gitea that referenced this pull request Aug 5, 2020
For the sake of performance and simplicity, remove this seemingly useless
license header. It's related to LibreJS which we already pretty much
killed of in an earlier commit [1]. Initially added in [2]. Note that
the StaticUrlPrefix here was never actually correctly resolved and has
rendered including the template fences.

[1] go-gitea#11810
[2] go-gitea@a915a09
lunny pushed a commit that referenced this pull request Aug 6, 2020
For the sake of performance and simplicity, remove this seemingly useless
license header. It's related to LibreJS which we already pretty much
killed of in an earlier commit [1]. Initially added in [2]. Note that
the StaticUrlPrefix here was never actually correctly resolved and has
rendered including the template fences.

[1] #11810
[2] a915a09

Co-authored-by: techknowlogick <[email protected]>
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/docs This PR mainly updates/creates documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace public/vendor/librejs.html
6 participants