chore: licence compliance for published artifacts - #2
Conversation
dskarzh
left a comment
There was a problem hiding this comment.
Review summary
Reviewed 3 changed files in Licence compliance for the published artifacts. Left 4 comment(s) inline.
The compliance approach is sound: the modification notice wording matches the sibling repos, the NOTICE copy fails loudly if the file is missing from the archive, and the empirical verification table in the description covers the packaging outcomes well. The inline comments are about robustness of the two new licence-staging mechanisms (both fail silently under certain conditions) and two consistency nits.
Additional findings
These observations are about existing code outside the PR's diff — spotted while reading surrounding context.
- pom.xml:65 — The
central-publishing-maven-plugin.versionproperty is set to0.7.0but is never referenced: the plugin declaration in thegpgprofile (line 287) hardcodes0.8.0. One of the two should go — as it stands the property misleads anyone bumping the version.
This review was auto-generated. Findings may contain errors — please verify before applying changes.
642d6ac to
f23cc96
Compare
dskarzh
left a comment
There was a problem hiding this comment.
Re-review summary
Re-reviewed chore: licence compliance for published artifacts — verified 4 finding(s) from previous review.
| Status | Count |
|---|---|
| ✅ Resolved | 2 |
| 💬 Acknowledged | 2 |
All previous findings have been addressed.
Finding details
- ✅ pom.xml:142 —
${maven.multiModuleProjectDirectory}resolved to the invocation directory (no.mvnmarker), so in-module builds would silently ship jars without the licence files — Fixed in code: both new executions now use${basedir}/.., the same repo-root resolution as the pre-existing copy execution, which is anchored to the module's position rather than the invocation directory. Verified in-module by the author. - 💬 pom.xml:154 — the new
add-resourceexecution overlaps the pre-existingcopy-resourcesexecution, with divergent repo-root resolution — Developer: "existing execution kept deliberately to stay close to upstream; both mechanisms now resolve the root the same way, and the remaining overlap writes identical bytes to the same path." - ✅ pom.xml:125 —
build-helper-maven-pluginversion hardcoded inline instead of following the pom's properties-block convention — Fixed in code. - 💬 pom.xml:246 — the javadoc staging path mirrors an internal default of maven-javadoc-plugin 3.10.1 and will silently break if a spring-boot parent bump moves it — Developer: "deliberate; exposure is narrow, releases are rare, manual, and jar contents are inspected, and the coupling is documented in the pom comment."
Also reviewed the fix commits (repo-root resolution change, the version property, and the 2.8.8TB2 bump) — no new issues. The bump is consistent across all 21 poms with no leftover 2.8.8TB/5.21.0TB references, <scm><tag> matches, and ${basedir}/.. resolves to the repo root for every module in the gpg release profile (the nested test modules resolve it one level short, but they are not part of the released module set, matching the pre-existing execution's behavior).
One note: the previous review's body-level observation — the unused central-publishing-maven-plugin.version property (0.7.0) vs. the version hardcoded in the gpg profile (0.8.0) — is still present. It concerns existing code outside this PR's diff, so deferring it is reasonable, just flagging that it was not picked up.
This re-review was auto-generated. Findings may contain errors — please verify before applying changes.
dskarzh
left a comment
There was a problem hiding this comment.
Review summary
Reviewed 21 changed files in chore: licence compliance for published artifacts. Left 2 comments inline.
The packaging changes consistently propagate the TB2 versions and add the intended licence and attribution resources. The remaining comments concern the accuracy of the modification notice and reproducibility of the Swagger UI input used for an immutable Maven release. Previously discussed and resolved review topics were excluded.
This review was auto-generated. Findings may contain errors — please verify before applying changes.
50960f0 to
cad032a
Compare
The one upstream source file this fork modifies, PropertyResolverUtils.java, now carries a modification notice with an SPDX-FileCopyrightText tag, and the ThingsBoard-authored springdoc-swagger-ui module pom carries the short-form SPDX header (SPDX-FileCopyrightText plus SPDX-License-Identifier: Apache-2.0). The README is replaced with a fork-descriptive one whose delta list against upstream 2.8.8 serves as the Apache-2.0 section 4(b) notice for the changed files that cannot reasonably carry one in-file — the poms and test fixtures whose only change is the version string. The published artifacts are completed: the springdoc-swagger-ui antrun step now copies the Swagger UI NOTICE into the webjar's META-INF, and the licence texts are staged into the sources and javadoc jars of every module (the main jars already carried them). The swagger-ui archive consumed by the webjar packaging is bumped to the 5.21.0TB2 tag, and the pom metadata urls point at this repository instead of upstream's site.
Releases were pushed to Maven Central through the central-publishing-maven-plugin, which registers itself as an extension and takes over mvn deploy. The plugin is removed and a distributionManagement block pointing at repo.thingsboard.io/artifactory/libs-release-public takes its place, the same publishing setup as other ThingsBoard-hosted libraries, so a plain mvn deploy -Pgpg now targets the ThingsBoard repository. The credentials come from the thingsboard-public-repo server entry in the deploying machine's settings.xml. Signing, sources and javadoc packaging in the gpg profile are unchanged. The unused central-publishing-maven-plugin.version property is left in place to keep the upstream delta minimal.
cad032a to
92e21a0
Compare
Attribution and licence hygiene for this fork, same series as thingsboard/tbel#51, thingsboard/protobuf-dynamic#4 and thingsboard/swagger-ui#1.
Targets
thingsboard, notmain—maintracks upstream and carries none of the fork's work.Changes
Mark the file we have modified. The fork's entire source delta against upstream
v2.8.8is one line inPropertyResolverUtils.java(LOGGER.warn→LOGGER.trace). Apache-2.0 §4(b) requires modified files to carry prominent notices stating that they were changed, so it gets one — the same wording used in the other three repos. The upstream header is left untouched and no ThingsBoard copyright is applied to upstream code. Everything else in the fork delta is version strings (2.8.8→2.8.8TBin poms and test fixtures), where theTBsuffix itself is the distinguishing mark, and the newspringdoc-swagger-uimodule, which is new work rather than a modification.Preserve the Swagger UI attribution in the published webjar.
springdoc-swagger-uibuilds its jar by downloading thethingsboard/swagger-uiarchive and moving onlydist/into place — the upstreamNOTICE(SmartBear Software's attribution, which §4(d) requires to travel with distributions) was unpacked and then left behind, sospringdoc-swagger-ui-2.8.8TB.jarshipped without it. The antrun step now also copiesNOTICEintoMETA-INF/.Ship the licence texts in the sources and javadoc jars. The main jars already carry
META-INF/LICENSEandMETA-INF/COPYRIGHTvia the existingcopy-resourcesexecution (left untouched), but that copies straight intotarget/classes, which neithermaven-source-pluginnormaven-javadoc-pluginlooks at — the published-sourcesand-javadocjars contained no licence at all (verified against the2.8.8TBartifacts on Maven Central). Two additions in the parent pom fix this for every module:build-helper-maven-plugin:add-resourceregistersLICENSE/COPYRIGHTas a project resource, which the sources jar picks up.add-resourcerather than a<resources>declaration because declaring<resources>would replace the filtered resource configuration inherited fromspring-boot-starter-parentfor every module.copy-resourcesexecution in thegpgprofile stages them intotarget/reports/apidocs/META-INFatprepare-package, beforejavadoc:jarruns. That path is where the javadoc plugin version managed byspring-boot-starter-parent3.4.8 (3.10.1) writes its output — verified empirically, not assumed.Bump
<swagger-ui.version>to5.21.0TB2— the tag suggested in chore: licence compliance for published artifacts swagger-ui#1 for the release that will include theNOTICEaddendum and the per-file notices added there.Bump the version to
2.8.8TB2(last commit) —2.8.8TBis on Maven Central and immutable, so the release this PR feeds needs a new version anyway; setting it here means merging and deploying is all that is left. The same commit updates<scm><tag>tov2.8.8TB2, fixing en passant that it previously named a tag that has never existed in this repo (tags stop atv2.4.0TB).Fix the urls in the pom metadata. Project
<url>pointed athttps://springdoc.org/, which is upstream's site, not this repository;<scm>used SSH urls forconnection/url(kept fordeveloperConnection, where SSH is appropriate). Thespringdoc-swagger-ui<licenses><url>pointed at the upstream GitHub repo instead of the licence text.Make the licence declarations machine-readable and describe the fork in the README. The same pass chore: licence compliance for published artifacts swagger-ui#1 got: the modification notice on
PropertyResolverUtils.javagains anSPDX-FileCopyrightTexttag, so the modification copyright is machine-readable while staying scoped to the modifications; the ThingsBoard-authoredspringdoc-swagger-ui/pom.xml— new work, which until now declared its licence only through its<licenses>block — carries the short-form SPDX header (SPDX-FileCopyrightText: Copyright 2024 ThingsBoard, Inc.+SPDX-License-Identifier: Apache-2.0); and the README, which documented upstream's packages and sponsors, is replaced with a fork-descriptive one. Its delta list against upstream 2.8.8 doubles as the §4(b) modification notice for the changed files that cannot reasonably carry one in-file — the poms and test fixtures whose only change is the version string. All three files scan to exactly Apache-2.0 with both copyright statements parsed. The module's<licenses><name>is also switched fromApache 2.0to the SPDX full nameApache License 2.0, matching the tbel fork, so the declared licence in the published pom maps to SPDX without guesswork.Publish releases to the ThingsBoard repository. Deployment went to Maven Central through the
central-publishing-maven-plugin, which registers itself as an extension and takes overmvn deploy. That plugin is removed and a<distributionManagement>block pointing atrepo.thingsboard.io/artifactory/libs-release-publictakes its place — the same publishing setup as other ThingsBoard-hosted libraries. Signing and the sources/javadoc packaging in thegpgprofile are unchanged; the deploying machine needs athingsboard-public-reposerver entry in itssettings.xmlinstead of thecentralone. The platform already resolves dependencies from that repository, so consumers need no change.Verification
Built with
mvn clean package -Pgpg -DskipTestson JDK 17 (-Pgpgso the reactor matches the released module set, and so the sources/javadoc jars are actually produced). The full-reactor build ran before the version bumps, i.e. against the5.21.0TBarchive — the bumped versions cannot fully build until the tag exists, and nothing about the packaging differs. After the bumps, the modules that do not depend on the webjar were rebuilt at2.8.8TB2to confirm the poms are consistent. Every produced jar was inspected:LICENSE,COPYRIGHT-sourcesjarsLICENSE,COPYRIGHT-javadocjarsLICENSE,COPYRIGHTspringdoc-swagger-uijarLICENSE,COPYRIGHTNOTICEThe test suite was not run: the only source change is a comment, the rest is packaging and metadata. Note for anyone building locally: the build requires an older JDK —
kotlin-maven-plugin1.9.25 inspringdoc-openapi-starter-commonfails to parse the JDK 25 version string. That predates this PR.Follow-up needed from someone with write access
5.21.0TB2(or adjust the property here if another name is chosen).2.8.8TB2, so no version work is needed. Tag itv2.8.8TB2, which is what the pom's<scm><tag>now declares. Note the destination is nowrepo.thingsboard.io(see the publishing bullet), so the deploying machine'ssettings.xmlneeds thethingsboard-public-repocredentials rather than the Sonatype ones.One thing worth checking before that release: the webjar resource path embeds the version (
META-INF/resources/webjars/swagger-ui/5.21.0TB2/after the bump), so it is worth grepping consumers for the literal old path in case anything hardcodes it.