matrei commented on PR #37:
URL:
https://github.com/apache/grails-gradle-publish/pull/37#issuecomment-5780146237
The mechanism is right and the diagnosis of the `from(javadoc)` wiring is
correct: Gradle 9.4.1's `JvmPluginsHelper.maybeRegisterDocumentationJarTask`
reuses an existing `javadocJar` verbatim and only wires `from(javadoc)` into
one it registers itself. But I ran the change against the two configurations I
care most about and found one gap that leaves the reported bug unfixed for
grails-core, and one regression. Both reproduced against the PR head with the
functional-test harness.
### 1. The fix does not apply when `withJavadocJar()` was called before the
plugin's `afterEvaluate`
The guard is `!tasks.names.contains('javadocJar')`. If the build script (or
another plugin) has already called `java { withJavadocJar() }` by the time
`validateProjectPublishable` runs, Gradle has already registered `javadocJar`
with `from(javadoc)` wired in, the new branch is skipped, and everything else
is exactly as before — `javadoc` disabled, its stale output overlaid onto the
groovydoc.
I took the existing `java-already-configured` fixture (which does exactly
that), added `tasks.withType(Jar).configureEach { duplicatesStrategy =
DuplicatesStrategy.FAIL }`, planted `build/docs/javadoc/help-doc.html`, and ran
`publish` against this branch:
```
> Task :javadoc SKIPPED
> Task :javadocJar FAILED
Cannot copy file
'.../java-already-configured/build/docs/groovydoc/help-doc.html' to
'help-doc.html'
because file '.../java-already-configured/build/docs/javadoc/help-doc.html'
has already been copied there.
```
That is the #36 failure verbatim. And it is grails-core's configuration:
`CompilePlugin.configureJars` calls `withJavadocJar()` eagerly at apply time
(`build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/CompilePlugin.groovy:75-78`),
well before this plugin's `afterEvaluate`. So the module that reported the
issue is not covered, and "apache/grails-core#16371 can be reverted once this
ships" does not hold — it would have to stay.
The "unchanged paths" list frames the already-registered case as "projects
that register their own `javadocJar`", but `java { withJavadocJar() }` isn't
registering your own jar; it's the standard Gradle idiom, and it's the case
most consumers of this plugin are in. The `java-already-configured` fixture
exists precisely because of that, and it is the obvious place to add a case
with planted stale output.
For that path the `from(javadoc)` wiring is a given, so something has to
keep its contents out. The ancestor-`destinationDir` problem you hit with the
exclude approach only arises because the exclude did not exempt the groovydoc
directory — an `exclude` of "under `javadoc.destinationDir` and not under
`groovydoc.destinationDir`" sidesteps it, and your second new test then becomes
a real guard for that branch rather than for an approach the code no longer
takes. Alternatively, document in the README and the PR that the fix only
covers projects that leave `withJavadocJar()` to the plugin, and drop the claim
about #16371. Either way the PR description needs to match what the code does.
### 2. `assemble` no longer builds the javadoc jar on Groovy projects
Gradle wires `assemble.dependsOn(javadocJar)` only inside the branch where
it registers the jar itself
(`JvmPluginsHelper.maybeRegisterDocumentationJarTask`, lines 171-173 in
v9.4.1). Now that the plugin registers the task first, that wiring is gone.
Running `assemble` on the new `stale-javadoc-output` fixture:
- base (`1.0.x`): `[:compileJava, :compileGroovy, :processResources,
:classes, :jar, :groovydoc, :javadoc, :sourcesJar, :javadocJar, :assemble]`
- this branch: `[:compileJava, :compileGroovy, :processResources, :classes,
:jar, :sourcesJar, :assemble]`
`publish` still works because the publication artifact carries the task
dependency, which is why the new tests are green. But `./gradlew build` no
longer produces `-javadoc.jar` in `build/libs` for any Groovy project, and
anything that picks artifacts up from there silently loses it. It also
contradicts "only the jar's content is swapped".
Fix is one line in the new register branch, mirroring what Gradle does:
```groovy
def javadocJar = tasks.register('javadocJar', Jar) { ... }
tasks.named('assemble') { it.dependsOn(javadocJar) }
```
plus an assertion. Running `assemble` in the fixture and checking
`result.task(':javadocJar')` would have caught this; the existing
`publish`-only cases cannot.
### Smaller points
- Gradle marks the reuse path this fix relies on with `// TODO: Emit
deprecation if this task already exists.` (`JvmPluginsHelper.java:159`). Not a
blocker — the `explicit-jar-creation-without-gradle-assistance` fixture already
depends on it — but worth one line in the code comment so that whoever sees the
deprecation warning knows where it comes from and why.
- In the new "stale javadoc output is kept out" test, `javadoc` is in the
`assertBuildSuccess` ignore list, but with the fix the task is not in the graph
at all. `result.task(':javadoc') == null` states the actual mechanism ("the
javadoc output is never wired in") and would fail if anything reintroduced
`from(javadoc)`.
- README: "generated from the groovydoc for projects with Groovy sources" —
the trigger is the `groovy` plugin being applied (a `groovydoc` task existing),
not the presence of Groovy sources. A Java-only project with the groovy plugin
gets a groovydoc jar.
- `stale-javadoc-output/.../MyProject.groovy` prints `Hello from
SubProject2` — copy-paste from the multi-project fixture.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
