Concat THIRD-PARTY to LICENSE in bin release

[bchapuis]

@Perdjesk What do you think about this alternative. The idea is to introduce a template (override.ftl) that serves as a basis to create a file for overriding the licenses of the dependencies (override.properties). In this file, we can then manually uniformize the license name and pick the least restrictive licenses (e.g. dual licensing). The maintenance of this file will require to perform a diff with a newly generated file before publishing a release and to make a few manual modifications.

I like that in this alternative, we have the same ordering and naming between the LICENSE file, the override.properties file, and the lib/ directory of the binary distribution. We loose the grouping by license, but I'm not sure that it is a requirement.

[Perdjesk]

Doing some testing in more depth regards of this configuration, I figured it is wrong to have it to true.

The case that triggered enabling this is in fact correct. It can happens that a direct dependency with scope test has a dependency that is included in the jar/binary release.

org.apiguardian:apiguardian-api is a dependency of org.junit.jupiter:junit-jupiter-api and org.apache.calcite:calcite-core.
Given that calcite is included with scope compile, it is the expected to see:

+- org.junit.jupiter:junit-jupiter-api:jar:5.7.1:test
  +- org.apiguardian:apiguardian-api:jar:1.1.0:compile
  +- org.opentest4j:opentest4j:jar:1.2.0:test
  \- org.junit.platform:junit-platform-commons:jar:1.7.1:test

And org.apiguardian:apiguardian-api must be in the LICENSE file.

[bchapuis]

Should I remove this line?

I will try to merge this tonight and to publish a second release candidate.

[Perdjesk]

Yes it has to be removed to be compliant with the policy that any bundled bit in release binary must be listed.

[Perdjesk]

Removing this incorrect configuration allow then to properly handle the circular dependency to baremaps components in the LICENSE:

<excludedGroups>^org\.apache\.baremaps</excludedGroups>

To remove:

org.apache.baremaps--baremaps-core--0.7.2-SNAPSHOT=Apache License 2.0
org.apache.baremaps--baremaps-ogcapi--0.7.2-SNAPSHOT=Apache License 2.0
org.apache.baremaps--baremaps-server--0.7.2-SNAPSHOT=Apache License 2.0

[bchapuis]

This makes me wonder if we could exclude all the Apache projects.

<excludedGroups>^org\.apache</excludedGroups>

[Perdjesk]

I pulled the branch and did some more in-depth analysis.

I think this approach is better and embrace the fact that we have to deal with a manual step/verification of the content added in LICENSE for: license merging, choose one license per bundled dependency. Having a file within source repository allow for proper review of changes.

However in order to make this maintainable and usable, we should have a way to identify drift between dependencies in pom.xmls and the content of override.properties so that they are aligned. This is necessary as well to catch earlier before release candidates missing content in override.properties as the license plugin will always try to pull license out of the remote repository.

Here is the trial I did locally to address this point:

--- a/baremaps-cli/pom.xml
+++ b/baremaps-cli/pom.xml
@@ -190,19 +190,35 @@
         <version>2.2.0</version>
         <configuration>
           <force>true</force>
-          <outputDirectory>${thirdparty.directory}</outputDirectory>
-          <thirdPartyFilename>${thirdparty.filename}</thirdPartyFilename>
           <excludedScopes>test,provided</excludedScopes>
-          <excludeTransitiveDependencies>true</excludeTransitiveDependencies>
-          <fileTemplate>${basedir}/src/license/bundle.ftl</fileTemplate>
-          <overrideFile>${basedir}/src/license/override.properties</overrideFile>
+          <excludedGroups>^org\.apache\.baremaps</excludedGroups>
+          <overrideUrl>file://${basedir}/src/license/override.properties</overrideUrl>
+          <failOnMissing>true</failOnMissing>
         </configuration>
         <executions>
           <execution>
+            <id>1</id>
             <goals>
               <goal>add-third-party</goal>
             </goals>
             <phase>prepare-package</phase>
+            <configuration>
+              <outputDirectory>${thirdparty.directory}</outputDirectory>
+              <thirdPartyFilename>${thirdparty.filename}</thirdPartyFilename>
+              <fileTemplate>${basedir}/src/license/bundle.ftl</fileTemplate>
+            </configuration>
+          </execution>
+          <execution>
+            <id>2</id>
+            <goals>
+              <goal>add-third-party</goal>
+            </goals>
+            <phase>generate-sources</phase>
+            <configuration>
+              <fileTemplate>${basedir}/src/license/override.ftl</fileTemplate>
+              <outputDirectory>${thirdparty.directory}</outputDirectory>
+              <thirdPartyFilename>override.properties</thirdPartyFilename>
+            </configuration>
           </execution>
         </executions>
       </plugin>

Changing a dependency version:

% ./mvnw generate-sources
% diff baremaps-cli/src/license/override.properties baremaps-cli/target/generated-sources/license/override.properties
44c44
< info.picocli--picocli--4.6.3=Apache License 2.0
---
> info.picocli--picocli--4.7.5=The Apache Software License, version 2.0

Adding a dependency:

% ./mvnw generate-sources
% diff baremaps-cli/src/license/override.properties baremaps-cli/target/generated-sources/license/override.properties
18a19
> com.google.code.gson--gson--2.10.1=Apache-2.0

Removing a dependency:

% ./mvnw generate-sources
% diff baremaps-cli/src/license/override.properties baremaps-cli/target/generated-sources/license/override.properties
200,201d200
< org.roaringbitmap--RoaringBitmap--0.9.38=Apache License 2.0
< org.roaringbitmap--shims--0.9.38=Apache License 2.0

The CI could enforce that there is no difference between the generated override.properties and the one present in code repository, thus ensuring the in-sync. Thus requiring resolution of the difference by contributor in PR.

[bchapuis]

This is a pretty cool idea! From what I understand, the generation takes the actual override into account. We could actually override the override.properties file, but I prefer if we do this with the diff command before releasing.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication