This is an automated email from the ASF dual-hosted git repository. rombert pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/sling-agent-skills.git
commit 71114042b4bd8e6fed94ccb43de108904c145bf0 Author: Rishabh Kumar <[email protected]> AuthorDate: Thu Jul 23 21:37:09 2026 +0530 SLING-13270 : tighten update-sling-parent-pom skill to handle slf4j, logback and parent pom 60+ versions --- .../references/felix-http-jetty-jdk21.md | 36 +++++++++++++ .../references/git-blame-ignore-revs.md | 31 +++++++++++ .../references/pax-exam-timeouts.md | 32 ++++++++++++ .../references/slf4j2-paxexam-it.md | 61 ++++++++++++++++++++++ .../references/sling-java-version.md | 20 +++++++ 5 files changed, 180 insertions(+) diff --git a/update-sling-parent-pom/references/felix-http-jetty-jdk21.md b/update-sling-parent-pom/references/felix-http-jetty-jdk21.md new file mode 100644 index 0000000..caf984f --- /dev/null +++ b/update-sling-parent-pom/references/felix-http-jetty-jdk21.md @@ -0,0 +1,36 @@ +# pax-exam quickstart fails to start under JDK 21 (Unsupported class file major version 65) + +If a pax-exam IT using `SlingOptions.slingQuickstartOakTar()` (or similar quickstart helpers) fails only on newer JDKs (observed on JDK 21), the JUnit-visible symptom is misleading — it looks like a slow-startup/timeout problem: + +``` +org.ops4j.pax.swissbox.tracker.ServiceLookupException: gave up waiting for service org.apache.sling.engine.SlingRequestProcessor +``` + +Increasing the service-lookup timeout (see [pax-exam-timeouts.md](./pax-exam-timeouts.md)) does not fix this — the real error is buried earlier in the container log (`target/failsafe-reports/<TestClass>-output.txt`, not the summary `.txt`): + +``` +ERROR: Bundle org.apache.felix.http.jetty [..] Error starting ... (org.osgi.framework.BundleException: Activator start error ...) +Caused by: java.lang.IllegalArgumentException: Unsupported class file major version 65 +``` + +Class file major version 65 is Java 21 bytecode. The quickstart's default `org.apache.felix.http.jetty` (5.0.0) bundles an ASM version too old to parse Java 21 classes during bytecode analysis. When the bundle fails to activate, every service the Sling engine provides — including `SlingRequestProcessor` — never comes up. + +## Fix + +Pin the affected bundles back and bump ASM via `SlingOptions.versionResolver`, in the test-support class's `baseConfiguration()` (or equivalent), before building the pax-exam options: + +```java +SlingOptions.versionResolver.setVersion("org.apache.felix", "org.apache.felix.http.jetty", "4.2.0"); +SlingOptions.versionResolver.setVersion("org.apache.felix", "org.apache.felix.http.servlet-api", "2.0.0"); +SlingOptions.versionResolver.setVersion("org.ow2.asm", "asm", "9.7.1"); +SlingOptions.versionResolver.setVersion("org.ow2.asm", "asm-analysis", "9.7.1"); +SlingOptions.versionResolver.setVersion("org.ow2.asm", "asm-commons", "9.7.1"); +SlingOptions.versionResolver.setVersion("org.ow2.asm", "asm-tree", "9.7.1"); +SlingOptions.versionResolver.setVersion("org.ow2.asm", "asm-util", "9.7.1"); +``` + +This exact override is already used by `sling-org-apache-sling-graphql-core` — check other pax-exam-based sibling repos for the same pattern before troubleshooting from scratch. + +## Verifying the fix + +This only reproduces on JDK 21 (JDK 17 quickstart ITs pass fine even without the fix). Since the project's CI runs a JDK matrix (commonly 17 and 21, sometimes 11 too — check the Jenkinsfile/CI job), running `mvn clean verify` locally with only the lowest supported JDK is not sufficient to catch this. Verify with the **highest** JDK version the CI matrix uses before pushing. diff --git a/update-sling-parent-pom/references/git-blame-ignore-revs.md b/update-sling-parent-pom/references/git-blame-ignore-revs.md new file mode 100644 index 0000000..25b4c86 --- /dev/null +++ b/update-sling-parent-pom/references/git-blame-ignore-revs.md @@ -0,0 +1,31 @@ +# Keeping git blame clean after the spotless reformat + +A parent POM upgrade that introduces or changes Spotless formatting rules (see the main instructions' `mvn spotless:apply` step) reformats every source file the new rules touch. Left as-is, `git blame` (and GitHub's blame view) attributes every reformatted line to the upgrade commit instead of the commit that actually last changed that line's content. + +Per the [Update to Parent 60](https://cwiki.apache.org/confluence/spaces/SLING/pages/284790344/Update+to+Parent+60) migration notes: + +## Preferred: commit formatting separately + +If you control the commit sequence, run `mvn spotless:apply` and commit the resulting reformat **on its own**, separate from the parent-version/dependency changes. This makes the next step trivial and keeps the substantive diff reviewable on its own. + +## Either way: add the commit(s) to `.git-blame-ignore-revs` + +Create (or append to) a `.git-blame-ignore-revs` file at the repo root listing the full commit SHA(s) that contain reformatting, one per line, with a comment above each explaining why: + +``` +# SLING-NNNNN: sling-bundle-parent NN -> MM upgrade; includes the resulting +# mvn spotless:apply reformat of every source file. +<full-commit-sha> +``` + +GitHub's blame view picks this file up automatically at the default location — no repo configuration needed for that. For local `git blame`, either pass it explicitly or configure it once per clone: + +```bash +git blame --ignore-revs-file .git-blame-ignore-revs -- path/to/file +# or, to make it the default for this clone: +git config blame.ignoreRevsFile .git-blame-ignore-revs +``` + +Verify the file works before committing it — blame a file that was reformatted and confirm the reformatted lines now attribute to their real prior author/commit, not the upgrade commit. + +If the reformat ended up mixed into the same commit as substantive changes (rather than committed separately) and that commit is already pushed/shared (e.g. an open PR), do not rewrite it to split things apart — rewriting shared history requires a force-push, which needs explicit user approval and risks losing work for anyone else who fetched the branch. Just add that commit's SHA to `.git-blame-ignore-revs` as a follow-up commit; it still fully addresses the blame-noise problem even tho [...] diff --git a/update-sling-parent-pom/references/pax-exam-timeouts.md b/update-sling-parent-pom/references/pax-exam-timeouts.md new file mode 100644 index 0000000..a07141c --- /dev/null +++ b/update-sling-parent-pom/references/pax-exam-timeouts.md @@ -0,0 +1,32 @@ +# pax-exam IT fails with ServiceLookupException on slower CI agents + +Symptom — an IT using `@Inject`-annotated fields (e.g. `SlingRequestProcessor`, `ResourceResolverFactory`) fails intermittently, only on slower CI agents (observed on Windows), with: + +``` +org.ops4j.pax.swissbox.tracker.ServiceLookupException: gave up waiting for service org.apache.sling.engine.SlingRequestProcessor +``` + +Before assuming this is a timeout issue: check the full container log (`target/failsafe-reports/<TestClass>-output.txt`) for an actual bundle start failure — e.g. [felix-http-jetty-jdk21.md](./felix-http-jetty-jdk21.md). Increasing the timeout only helps if the required service genuinely just takes a long time to publish, not if it never comes up at all. If the failure happens at exactly ~10s (or whatever the current timeout is) every time regardless of platform, suspect a real startup f [...] + +## The gotcha: `CoreOptions.systemTimeout(millis)` does NOT fix this + +Do not use `org.ops4j.pax.exam.CoreOptions.systemTimeout(long)` in the pax-exam configuration — it has no effect on `@Inject`-field service-lookup timeouts. Those are read from a **system property** instead — verified by decompiling `org.ops4j.pax.exam.inject.internal.ServiceInjector` (in the `pax-exam-inject` artifact), whose `injectField` method reads it directly (parsed with `Integer.parseInt` and widened to `long`): + +```java +long timeout = Integer.parseInt(System.getProperty("pax.exam.service.timeout", "10000")); +``` + +The default is 10 seconds (`org.ops4j.pax.swissbox.tracker.ServiceLookup.DEFAULT_TIMEOUT`), which can be too short for a full Sling quickstart (OSGi + Oak + scripting engines) to publish its services on a loaded or slower CI agent. + +## Fix + +Set the system property via `CoreOptions.systemProperty(...)` in the pax-exam configuration composite, not `systemTimeout(...)`: + +```java +import static org.ops4j.pax.exam.CoreOptions.systemProperty; + +// in baseConfiguration(): +systemProperty("pax.exam.service.timeout").value("60000"), +``` + +This is a JVM system property passed to the **forked container JVM** (where the OSGi framework and injected test run), not the outer Maven/surefire JVM — so it must be added as a pax-exam `Option`, not set via `-D` on the `mvn` command line. diff --git a/update-sling-parent-pom/references/slf4j2-paxexam-it.md b/update-sling-parent-pom/references/slf4j2-paxexam-it.md new file mode 100644 index 0000000..6b3178d --- /dev/null +++ b/update-sling-parent-pom/references/slf4j2-paxexam-it.md @@ -0,0 +1,61 @@ +# slf4j 2.x vs pax-exam integration tests + +Newer Sling parent POM versions manage `slf4j.version` to a 2.x release. If the project's `slf4j-api` dependency has no explicit version (relying on the parent), it silently floats to 2.x at compile time. This breaks two things: + +1. **Unit tests**: an older pinned `logback-classic` (e.g. 1.2.3, built against slf4j 1.7.x) can't bind to slf4j-api 2.x. Symptom: + ``` + java.lang.ClassCastException: class org.slf4j.helpers.NOPLogger cannot be cast to class ch.qos.logback.classic.Logger + ``` + (SLF4J falls back to a no-op logger because the old logback binding is ignored as incompatible.) + +2. **Integration tests using `org.apache.sling.testing.paxexam`'s `slingQuickstartOakTar()`**: the bundle now compiles with `Import-Package: org.slf4j;version="[2.0,3)"`, but the Sling quickstart bundle set includes an older `pax-logging-api` that only **exports** `org.slf4j` at 1.7.x. The OSGi container fails to resolve the bundle: + ``` + ERROR: Bundle ... missing requirement ... osgi.wiring.package; (&(osgi.wiring.package=org.slf4j)(version>=2.0.0)(!(version>=3.0.0))) + ``` + +## Do not chase this by upgrading pax-exam infrastructure + +It is tempting to "fix forward" by bumping `org.apache.sling.testing.paxexam`, `org.ops4j.pax.exam`, and `org.apache.felix.framework` to versions whose quickstart bundles a newer `pax-logging-api`/`slf4j-api`. **Avoid this** — each of those major-version bumps drags in unrelated breaking changes: +- **`tinybundles` package renamed**: `org.ops4j.pax.tinybundles.core.TinyBundle` → `org.ops4j.pax.tinybundles.TinyBundle`, `.set()`/`.add()` → `.setHeader()`/`.addResource()` +- **`pax-exam-cm` version must be bumped in lockstep** with `org.ops4j.pax.exam.version` — mismatches throw `NoClassDefFoundError` for tinybundles classes during container bootstrap +- **Probe-bundle classloading regressions** for classes pulled from non-OSGi-exported packages (e.g. `org.apache.sling.servlethelpers.internalrequests.SlingInternalRequest`, which `org.apache.sling.servlet-helpers` never actually `Export-Package`s) + +Chasing these one at a time burns many build cycles for a problem that has a much smaller fix. + +## Correct fix: pin slf4j-api and logback-classic back + +Explicitly pin `slf4j-api` to the version the project used **before** the parent upgrade (do not let it inherit the parent's managed 2.x version), and keep `logback-classic` at its matching pre-upgrade version: + +```xml +<dependency> + <groupId>org.slf4j</groupId> + <artifactId>slf4j-api</artifactId> + <version>1.7.36</version> + <scope>provided</scope> +</dependency> +``` + +```xml +<dependency> + <groupId>ch.qos.logback</groupId> + <artifactId>logback-classic</artifactId> + <version>1.2.3</version> + <scope>test</scope> +</dependency> +``` + +This keeps the bundle's `Import-Package: org.slf4j` range at 1.7.x, matching what the pax-exam quickstart's `pax-logging-api` exports — no pax-exam/felix/tinybundles version changes needed. + +## If a sibling repo already migrated + +If other repos in the same org/workspace use the same pax-exam quickstart IT pattern (`slingQuickstartOakTar()`), check whether one has already been migrated to the target parent version before troubleshooting from scratch — its pinned `slf4j-api`/`logback-classic`/`org.apache.sling.testing.paxexam`/`org.ops4j.pax.exam.version`/`org.apache.felix.framework` versions are a validated reference point. + +## If testing.paxexam still needs a version bump + +Some issues (e.g. an outdated `javax.servlet-api` bundled by an old `org.apache.sling.testing.paxexam`) do require bumping `org.apache.sling.testing.paxexam`. When you do: +- Bump only as far as needed, not necessarily to the latest release — check each candidate version's own POM for its required `org.ops4j.pax.exam.version` and prefer the smallest jump that resolves the issue, since larger jumps (e.g. into a version requiring pax-exam 4.14.0+) are what triggers the tinybundles/pax-exam-cm/probe-classloading cascade above. +- Always set `org.ops4j.pax.exam.version` (and any other `org.ops4j.pax.exam:*` artifacts pinned in the project) to match what that `testing.paxexam` version's own POM declares — check `~/.m2/repository/org/apache/sling/org.apache.sling.testing.paxexam/<version>/*.pom` for its `<org.ops4j.pax.exam.version>` property. + +## See also + +Once the bundle resolves and the quickstart starts, the same pax-exam IT can still fail for unrelated reasons on newer JDKs or slower CI agents — see [felix-http-jetty-jdk21.md](./felix-http-jetty-jdk21.md) and [pax-exam-timeouts.md](./pax-exam-timeouts.md). diff --git a/update-sling-parent-pom/references/sling-java-version.md b/update-sling-parent-pom/references/sling-java-version.md new file mode 100644 index 0000000..87d8bfd --- /dev/null +++ b/update-sling-parent-pom/references/sling-java-version.md @@ -0,0 +1,20 @@ +# Setting the mandatory sling.java.version property + +Newer Sling parent POM versions require the property `sling.java.version` to be set explicitly. Older parent versions defaulted it (commonly to `8`); the newer parent no longer supplies a default, so the build fails with: + +``` +[ERROR] Rule 0: org.apache.maven.enforcer.rules.property.RequireProperty failed with message: +[ERROR] Property "sling.java.version" is required for this build and must be between 8 and 99. +``` + +This property controls the Java **API/class-file compliance level** the bundle is compiled against (via `--release`) — it is independent of the JDK used to run the build. + +Fix: add the property to the project's own `<properties>`, set to the value the project used to build against before the upgrade (check the old parent version's default, or the project's previous `maven-compiler-plugin` `source`/`target`/`release` if set explicitly): + +```xml +<properties> + <sling.java.version>8</sling.java.version> +</properties> +``` + +Do not default to a higher value (e.g. matching the JDK used to build) unless the project already required it — that would silently raise the bundle's minimum Java API requirement for consumers.
