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

Generate license on demand #2189

Merged
merged 18 commits into from Apr 27, 2022
Merged

Generate license on demand #2189

merged 18 commits into from Apr 27, 2022

Conversation

infeo
Copy link
Member

@infeo infeo commented Apr 19, 2022

Summary

The EULA/license of the mac & windows installers are generated during the build process and are not static anymore.

Motivation

With every dependency update the content of the license file displayed in our app and installers changes. But for the changes to appear in our released artifacts, it needs to be commited. This causes a lot of boilerplate commits ("updating license" ) and additionally, during a release one can easily forget to update the license file.

This PR removes this commit necessity and generates the most up-to-date license on the fly during build of release artifacts for macOS and Windows.

@infeo infeo added type:enhancement Improvements on an existing feature misc:installer Affects installer or launcher labels Apr 19, 2022
@infeo infeo added this to the 1.6.9 milestone Apr 19, 2022
@infeo infeo self-assigned this Apr 19, 2022
Copy link
Member

@overheadhunter overheadhunter left a comment

Choose a reason for hiding this comment

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

I support the general idea to automate this, but I have two remarks:

  1. Since we now have three different .ftl files, the build step should be more specific than "Generate license", e.g. "Generate license shown in .dmg"
  2. Please deduplicate licenseMerges.xml, if possible including the merges inside the pom.xml

infeo and others added 2 commits April 20, 2022 10:31
Co-authored-by: Sebastian Stenzel <overheadhunter@users.noreply.github.com>
Copy link
Member

@overheadhunter overheadhunter left a comment

Choose a reason for hiding this comment

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

THIRD-PARTY.txt will be regenerated, so either add it to .gitignore or - and this is probably even cleaner - make sure it is generated in /target, which is preferred for generated resources but requires you to test it (both when starting from IDE as well as when building the jar)

Edit: Also, now the .ftl file that is used "inside the app" lies outside the app's sources. Which makes it even harder to know the exact purpose of those three competing templates.

@infeo
Copy link
Member Author

infeo commented Apr 20, 2022

Which makes it even harder to know the exact purpose of those three competing templates.

The three (actual four) templates are:

  • generating a txt-file to be used inside the app
  • generating a rtf-file used for the installers (windwos and mac)

In dc38942 @tobihagemann adjusted the existing rtf-template for macOS..


make sure it is generated in /target, which is preferred for generated resources but requires you to test it

Fixed in bcfba21. By default, the Third-Party file is placed inside target/generated-sources/license and later (automagically) copied to the target/classes/. From there it is accessible for the app and packaged into the jar.

@overheadhunter
Copy link
Member

overheadhunter commented Apr 21, 2022

I just tested the .dmg from the CI build that you ran yesterday. While it works as expected and this is certainly not a regression, I'm asking myself if we could make the RTF a little more beautiful, as it currently looks like this:

Screenshot 2022-04-21 at 10 58 02

Maybe make the project names clickable and hide the URLs?

@infeo
Copy link
Member Author

infeo commented Apr 22, 2022

Maybe make the project names clickable and hide the URLs?

This would only work for the EXE and DMG installer, the MSI installer does not allow clickable links afaik. Additionally, some links do not work (e.g. https://openjdk.java.net/projects/openjfx/javafx-base/). Another Idea would be to discard all links and just show the name of the project and the corresponding license.

@overheadhunter
Copy link
Member

This would only work for the EXE and DMG installer, the MSI installer does not allow clickable links afaik.

My comment was about the .dmg file. Since we have separate templates for each use case...

Additionally, some links do not work (e.g. https://openjdk.java.net/projects/openjfx/javafx-base/). Another Idea would be to discard all links and just show the name of the project and the corresponding license.

Broken links are broken regardless of whether we use <a href="broken">displayname</a> or <a href="broken">broken</a>.

Copy link
Member

@overheadhunter overheadhunter left a comment

Choose a reason for hiding this comment

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

please move or rename third-party.merges, since having this in the project root is confusing and there is no clue this is related to the license names and the Maven license plugin

Copy link
Member

@overheadhunter overheadhunter left a comment

Choose a reason for hiding this comment

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

I'm really confused by the fact that some paths refer to a different base dir than others... This exceeds my review skills.

But since it works, I won't question it. 😄

@sonarcloud
Copy link

sonarcloud bot commented Apr 26, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@infeo infeo merged commit 8f97235 into develop Apr 27, 2022
@infeo infeo deleted the feature/on-the-fly-license branch April 27, 2022 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
misc:installer Affects installer or launcher type:enhancement Improvements on an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants