Improve the bundle jar license and notice remove using exclude #1991

gh-yzou · 2025-07-02T16:30:57Z

Previously we added addLicenseFilesToJar job to help removing the LICENSE and NOTICE from dependency jars and also add our own Jars.
The addLicenseFilesToJar does this brute forcely, this PR improves the process by using exclude, and we are also able to remove this extra addLicenseFilesToJar job.

snazy · 2025-07-02T16:43:25Z

We were not able to reuse the current shadowJar plugin for maven publish due to the shadowJar plugin replaces the regular source Jar with the shadowJar during maven publish.

I doubt this statement is true. The shadow plugin does not change anything wrt to sourceJar.

dimas-b

Thanks for this refactoring @gh-yzou ! The new build script looks much better to me... I still have a couple of comments :)

dimas-b · 2025-07-02T16:42:42Z

plugins/spark/README.md

--conf spark.sql.catalog.polaris.credential="root:secret" \
--conf spark.sql.catalog.polaris.scope='PRINCIPAL_ROLE:ALL' \
--conf spark.sql.catalog.polaris.token-refresh-enabled=true \
+--conf spark.sql.catalog.<catalog-name>.warehouse=<catalog-name> \


nit: why not use polaris as the example catalog name?

This is the shell command without filling the actual value, i added an example in the README with specific value filled.

dimas-b · 2025-07-02T16:46:59Z

plugins/spark/v3.5/spark-bundle/build.gradle.kts

+  // customized LICENSE and NOTICE file are called CUSTOM-LICENSE and CUSTOM-NOTICE,
+  // and renamed to LICENSE and NOTICE after include, this is to avoid the file
+  // being excluded due to the exclude pattern matching used above.
+  from("${projectDir}/CUSTOM-LICENSE") { rename { "LICENSE" } }


If CUSTOM is only to avoid falling under the exclude rules above, how about naming it BUNDLE-LICENSE instead? I think that name is clearer.

Alternatively (because the simple LICENSE file name feel more appropriate to me), how about the following?

Copy ${projectDir}/LICENSE" to build/license/BUNDLE-LICENSE

Include build/license/BUNDLE-LICENSE with rename inside the shadow task.

The alternative seems unnecessary complicated to me, I think name with BUNDLE-* is a perfect name, which indicates the license specifically for the bundle jar, which is different than the regular LICENSE and NOTICE we use.

@jbonofre : WDYT about BUNDLE-LICENSE?

@gh-yzou : I edited by comment about copy at build time. I hope the intent is clearer now. Sorry about the confusion.

yeah, i think i got your original comment, i don't think we need to do those extra copies, which seems unnecessary, and i think BUNDLE-LICENSE and BUNDLE-NOTICE is a good name there, we can see what @jbonofre think about the original file name, just note that in the final jar, the name will still be LICENSE and NOTICE, that is just the file name in the original project.

These LICENSE files are intended only for binary distributions. In the source code, using the name BUNDLE-LICENSE is perfectly fine, as long as we ensure the correct file name is used in the final published binary artifacts. Let's make sure the naming is adjusted appropriately during the packaging step.

@dimas-b I reverted the extra module change in this PR, I think we can investigate into how to reuse the shadowJar plugin in a followup PR. And I think @jbonofre is ok with the current license name, since it is not a user facing name, I think we can always improve on top of it later. could you please take one more look of this PR?

gh-yzou · 2025-07-02T16:59:21Z

@snazy it doesn't change the sourceJar, however, it updates the generated .pom file to only dependent on the bundle jar, and it stop publishes the original sourceJar, where we only got the following jars

repo .m2/repository/org/apache/polaris/polaris-spark-3.5_2.12/1.1.0-incubating-SNAPSHOT

maven-metadata-local.xml
polaris-spark-3.5_2.12-1.1.0-incubating-SNAPSHOT-bundle.jar
polaris-spark-3.5_2.12-1.1.0-incubating-SNAPSHOT-javadoc.jar
polaris-spark-3.5_2.12-1.1.0-incubating-SNAPSHOT.module
polaris-spark-3.5_2.12-1.1.0-incubating-SNAPSHOT.pom
polaris-spark-3.5_2.12-1.1.0-incubating-SNAPSHOT-sources.jar

Therefore, we wasn't able to directly reuse the shadow jar. However, we are able to reuse it now, and with a separate bundle project, it also make things much more clear.

RussellSpitzer · 2025-07-02T21:05:17Z

plugins/spark/v3.5/spark-bundle/build.gradle.kts

+  // add polaris customized LICENSE and NOTICE for the bundle jar at top level. Note that the
+  // customized LICENSE and NOTICE file are called BUNDLE-LICENSE and BUNDLE-NOTICE,
+  // and renamed to LICENSE and NOTICE after include, this is to avoid the file
+  // being excluded due to the exclude pattern matching used above.


I'm surprised a bit that this works based on the documentation for the exclude which I thought was the last thing that is run after the steps like this would execute but I'm glad that it does

Yeah, I need to make sure the original file name doesn't match any of the matching rules to avoid being excluded, the rename actually happens after the excluding. I have verified manually, the LICENSE and NOTICE is the only one I see at top level, and they are the right one

snazy · 2025-07-04T11:31:01Z

@snazy it doesn't change the sourceJar, however, it updates the generated .pom file to only dependent on the bundle jar, and it stop publishes the original sourceJar, where we only got the following jars
...

I still don't understand what you're trying to say here. In the PR description you say: the shadowJar plugin replaces the regular source Jar with the shadowJar, in the comment you say stop publishes the original sourceJar, but the listing below shows she source jar.

There's nothing that stops users of the shadow plugin to publish both the "base" jar (e.g. using a different classifier) and the shaded jar from a single project.

dimas-b · 2025-07-04T20:38:19Z

plugins/spark/v3.5/spark-bundle/build.gradle.kts

+
+dependencies { implementation(project(":polaris-spark-${sparkMajorVersion}_${scalaVersion}")) }
+
+tasks.named<ShadowJar>("shadowJar") {


After this PR the bundled artifact maven coordinates are going to be different from what is currently in 1.0.0 RC6 (that is polaris-spark-3.5_2.12-1.0.0-incubating-bundle.jar).

Since the polaris-spark artifact name is occupied by the non-bundled Spark Client jar already (without a qualifier), I believe it is not possible to offer maven redirects, so this is going to be a breaking change in terms or Maven coordinates of Polaris artifacts. Regardless of whether we expect users to download this jar via Maven or now, I think it deserves to be mentioned in the CHANGELOG (unless we include this PR into 1.0.0).

Cf. https://lists.apache.org/thread/hrdchvljflcknyq9c7o7by9jhpv204op

Sure, i can add the jar name format update in the change log

dimas-b · 2025-07-07T14:18:20Z

plugins/spark/v3.5/spark-bundle/build.gradle.kts

+  exclude("META-INF/**/*NOTICE*")
+  // exclude the top level LICENSE, LICENSE-*.txt and NOTICE
+  exclude("LICENSE*")
+  exclude("NOTICE*")


Could we do the same kind of filter in the existing spark client module? If not, what prevents up from doing it?

This is not needed for the other spark client module. This is needed by the bundle jar because the bundle jar packs all the dependencies in one jar, and therefore need a customized license. however, for the regular spark client, we do not pack any dependency in the jar, it is pure source code from Polaris, and the regular poalris license it good enough.

I mean, why not use the same build code for the bunded jar in the old Spark module? Why do we need a new module?

There are couple of reasons:

with this approach, we will be able to reuse the current shadowJar plugin for maven publish, instead of this hack added https://github.com/apache/polaris/pull/1991/files#diff-d380e458e986c6183b1e23fcea1169811acfc68b2662ff9c88bb26b821238607L136, which i think make things more consistent.

This seems a more commonly used approach to publish bundle jar or uber jar, such as Iceberg.

This could make the module responsibility more clear, for example, the bundle-license and bundle-notice is only needed for the bundle project, not the regular project.

gh-yzou · 2025-07-07T17:47:49Z

@snazy sorry for the confusion, by source jar, i don't refer to the *-source.jar, I am referring to the jar without any classifier, here is what we expected to see once we do publish to maven

maven-metadata-local.xml
polaris-spark-3.5_2.13-1.1.0-incubating-SNAPSHOT-bundle.jar
polaris-spark-3.5_2.13-1.1.0-incubating-SNAPSHOT-javadoc.jar
polaris-spark-3.5_2.13-1.1.0-incubating-SNAPSHOT-sources.jar
polaris-spark-3.5_2.13-1.1.0-incubating-SNAPSHOT-test-fixtures.jar
polaris-spark-3.5_2.13-1.1.0-incubating-SNAPSHOT.jar
polaris-spark-3.5_2.13-1.1.0-incubating-SNAPSHOT.module
polaris-spark-3.5_2.13-1.1.0-incubating-SNAPSHOT.pom

where you can see we have a jar with name polaris-spark-3.5_2.13-1.1.0-incubating-SNAPSHOT.jar. However, after reuse the shadowJar plugin, what we see are the following

maven-metadata-local.xml
polaris-spark-3.5_2.12-1.1.0-incubating-SNAPSHOT-bundle.jar
polaris-spark-3.5_2.12-1.1.0-incubating-SNAPSHOT-javadoc.jar
polaris-spark-3.5_2.12-1.1.0-incubating-SNAPSHOT.module
polaris-spark-3.5_2.12-1.1.0-incubating-SNAPSHOT.pom
polaris-spark-3.5_2.12-1.1.0-incubating-SNAPSHOT-sources.jar

and when customer uses --packages org.apach.polaris:polaris-spark:1.1.0-incubating-SNAPSHOT, it is actually installing the bundle jar. However, when user is using the --packages options, we expect the polaris-spark-3.5_2.13-1.1.0-incubating-SNAPSHOT.jar is installed, and the other dependency will be downloaded accordingly. The polaris-spark-3.5_2.13-1.1.0-incubating-SNAPSHOT-bundle.jar is only intended for the --jars use case.
In this Pr, we added another bundle project to reuse the shadowJar plugin to help publishing the bundle jar, where we will see the following jar gets published

maven-metadata-local.xml
polaris-spark-bundle-3.5_2.12-1.1.0-incubating-SNAPSHOT-javadoc.jar
polaris-spark-bundle-3.5_2.12-1.1.0-incubating-SNAPSHOT-sources.jar
polaris-spark-bundle-3.5_2.12-1.1.0-incubating-SNAPSHOT.jar
polaris-spark-bundle-3.5_2.12-1.1.0-incubating-SNAPSHOT.module
polaris-spark-bundle-3.5_2.12-1.1.0-incubating-SNAPSHOT.pom

and polaris-spark-bundle-3.5_2.12-1.1.0-incubating-SNAPSHOT.jar is used as the --jars option, the original polari-spark project continue remain for the --package use case, with the following jars

polaris-spark-3.5_2.12-1.1.0-incubating-SNAPSHOT-javadoc.jar
polaris-spark-3.5_2.12-1.1.0-incubating-SNAPSHOT-sources.jar
polaris-spark-3.5_2.12-1.1.0-incubating-SNAPSHOT-test-fixtures.jar
polaris-spark-3.5_2.12-1.1.0-incubating-SNAPSHOT.jar
polaris-spark-3.5_2.12-1.1.0-incubating-SNAPSHOT.module
polaris-spark-3.5_2.12-1.1.0-incubating-SNAPSHOT.pom

snazy · 2025-07-08T07:30:34Z

@gh-yzou I still do not understand why you need to add a new Gradle project just to have one other jar. It is possible to build and publish both the "raw" and the "shadow" jar from a single project, we do this in Nessie's Spark extensions.

BTW: It is concerning that the term "customer" is being used. Polaris is an Apache project, not a commercial project!

gh-yzou · 2025-07-08T17:33:36Z

@snazy I don't need a separate project to publish both Jars (polaris-spark-3.5_2.13-1.1.0-incubating-SNAPSHOT.jar and polaris-spark-3.5_2.12-1.1.0-incubating-SNAPSHOT-bundle.jar). This is what happening today, however, we are not able to reuse the shadowJar plugin.

Regarding to nessie, i assume you are referring this https://github.com/projectnessie/nessie/blob/main/integrations/spark-extensions/build.gradle.kts#L84. However, it seems that this shadowJar packs the dependencies and doesn't have any classifier, which means the shadowJar packed will be the project jar when publish. I did a compile of nessie project, and here is what I see

nessie-spark-extensions-3.5_2.12-0.104.3-SNAPSHOT-javadoc.jar
nessie-spark-extensions-3.5_2.12-0.104.3-SNAPSHOT-license-report.zip
nessie-spark-extensions-3.5_2.12-0.104.3-SNAPSHOT-scaladoc.jar
nessie-spark-extensions-3.5_2.12-0.104.3-SNAPSHOT-sources.jar
nessie-spark-extensions-3.5_2.12-0.104.3-SNAPSHOT.jar
nessie-spark-extensions-3.5_2.12-0.104.3-SNAPSHOT.module
nessie-spark-extensions-3.5_2.12-0.104.3-SNAPSHOT.pom

For which, i believe nessie-spark-extensions-3.5_2.12-0.104.3-SNAPSHOT.jar is the shadowJar. As we have mentioned, the shadowJar is packed for the --jars use case, where we pack everything. For --package use case, the dependencies are suppose to be downloaded while installing the project, and we just need the original project jar (not the shadowJar).

Regarding to "he term "customer" is being used", are you referring to the jar name changes, like from project-spark-xxx-bundle.jar to project-spark-bundle-xxx.jar. Since this is for the --jar use case, and the jar name updates along with the version name for every release, i think the impact should be relative low, especially this is still an experimenting project. We are also making sure that we doc this change clearly in the CHANGELOG

snazy · 2025-07-09T09:51:58Z

@gh-yzou It is possible to publish both the "non-shadow" and the "shadow" jar from one project, even with the classifiers reversed. It is rather a matter of configuring the Gradle artifacts and (Maven) publications. So I suggest to consider this approach.

Regarding to "he term "customer" is being used", are you referring to the jar name changes

No, I'm referring to the usage of the commercial term "customers", which inappropriate in Apache projects. Apache projects have users and are by definition not commercial.

gh-yzou · 2025-07-09T18:45:56Z

@snazy sure, we can look into how to improve the shadowJar plugin. What I can do is for his Pr just do the improvement of license, and we can do the other improvement in a separate PR.

jbonofre

LGTM.

I would suggest to add a note about the artifact coordinates change as users may be surprised by this change.

dimas-b

Thanks, @gh-yzou ! The current state of this PR LGTM!

dimas-b · 2025-07-09T23:03:50Z

nit: The PR title seems a bit outdated?

gh-yzou · 2025-07-09T23:05:14Z

@dimas-b i just rebased about an hour ago, but I can rebase it again.

dimas-b · 2025-07-09T23:05:17Z

plugins/spark/v3.5/spark/build.gradle.kts

@@ -1,3 +1,5 @@
+import com.github.jengelman.gradle.plugins.shadow.tasks.ShadowJar


oops, looks like this import should be below the copyright comment

good catch, let me move it

@dimas-b i fixed the order, and also updated the PR title and description

dimas-b · 2025-07-09T23:07:17Z

no need to rebase, but the PR title still mentions a new "project", which is no longer the case, right?

github-project-automation bot added this to Basic Kanban Board Jul 2, 2025

github-project-automation bot moved this to PRs In Progress in Basic Kanban Board Jul 2, 2025

gh-yzou requested review from RussellSpitzer, flyrain and dimas-b July 2, 2025 16:31

dimas-b reviewed Jul 2, 2025

View reviewed changes

RussellSpitzer reviewed Jul 2, 2025

View reviewed changes

gh-yzou requested a review from jbonofre July 3, 2025 17:05

dimas-b reviewed Jul 4, 2025

View reviewed changes

dimas-b reviewed Jul 7, 2025

View reviewed changes

gh-yzou force-pushed the yzou-spark-client-bundle branch from a506adf to 672cd1f Compare July 8, 2025 00:57

jbonofre previously approved these changes Jul 9, 2025

View reviewed changes

github-project-automation bot moved this from PRs In Progress to Ready to merge in Basic Kanban Board Jul 9, 2025

gh-yzou dismissed jbonofre’s stale review via 1e53608 July 9, 2025 22:46

gh-yzou force-pushed the yzou-spark-client-bundle branch from 672cd1f to 1e53608 Compare July 9, 2025 22:46

gh-yzou requested review from RussellSpitzer, dimas-b and jbonofre July 9, 2025 22:57

dimas-b previously approved these changes Jul 9, 2025

View reviewed changes

dimas-b reviewed Jul 9, 2025

View reviewed changes

gh-yzou added 10 commits July 9, 2025 16:07

add polaris-bundle-project

f31b358

address comments

3b7f5fb

update comment

8a24292

add changelog

f3de59d

fix spark client regtests

5cb3827

update PR

28a2abd

update readme

5118ddf

fix notebook

79d94ae

fix notebook change

d474668

fix oder

1239a9e

gh-yzou changed the title ~~Add polaris-spark-bundle project to help packing and publish the Spark Client shadow Jar~~ Improve the bundle jar license and notice remove using excldue Jul 9, 2025

gh-yzou dismissed dimas-b’s stale review via 1239a9e July 9, 2025 23:18

gh-yzou force-pushed the yzou-spark-client-bundle branch from c42dad3 to 1239a9e Compare July 9, 2025 23:18

gh-yzou requested a review from dimas-b July 9, 2025 23:19

dimas-b approved these changes Jul 9, 2025

View reviewed changes

dimas-b changed the title ~~Improve the bundle jar license and notice remove using excldue~~ Improve the bundle jar license and notice remove using exclude Jul 9, 2025

flyrain approved these changes Jul 10, 2025

View reviewed changes

flyrain merged commit 33d9940 into apache:main Jul 10, 2025
11 checks passed

github-project-automation bot moved this from Ready to merge to Done in Basic Kanban Board Jul 10, 2025


		dependencies { implementation(project(":polaris-spark-${sparkMajorVersion}_${scalaVersion}")) }

		tasks.named<ShadowJar>("shadowJar") {

		@@ -1,3 +1,5 @@
		import com.github.jengelman.gradle.plugins.shadow.tasks.ShadowJar

Improve the bundle jar license and notice remove using exclude #1991

Improve the bundle jar license and notice remove using exclude #1991

Uh oh!

Conversation

gh-yzou commented Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

snazy commented Jul 2, 2025

Uh oh!

dimas-b left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dimas-b Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gh-yzou Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gh-yzou commented Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gh-yzou Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

snazy commented Jul 4, 2025

Uh oh!

dimas-b Jul 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dimas-b Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gh-yzou commented Jul 7, 2025

Uh oh!

snazy commented Jul 8, 2025

Uh oh!

gh-yzou commented Jul 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

snazy commented Jul 9, 2025

Uh oh!

gh-yzou commented Jul 9, 2025

Uh oh!

jbonofre left a comment

Choose a reason for hiding this comment

Uh oh!

dimas-b left a comment

Choose a reason for hiding this comment

Uh oh!

gh-yzou commented Jul 2, 2025 •

edited

Loading

dimas-b Jul 2, 2025 •

edited

Loading

gh-yzou Jul 2, 2025 •

edited

Loading

gh-yzou commented Jul 2, 2025 •

edited

Loading

gh-yzou Jul 2, 2025 •

edited

Loading

dimas-b Jul 4, 2025 •

edited

Loading

dimas-b Jul 7, 2025 •

edited

Loading

gh-yzou commented Jul 8, 2025 •

edited

Loading

dimas-b commented Jul 9, 2025 •

edited

Loading