gnodet commented on code in PR #12069:
URL: https://github.com/apache/maven/pull/12069#discussion_r3867882883
##########
impl/maven-core/src/main/java/org/apache/maven/ReactorReader.java:
##########
@@ -217,13 +220,49 @@ private File
determineBuildOutputDirectoryForArtifact(final MavenProject project
if (projectHasOutputFromPreviousSession ||
projectCompiledDuringThisSession) {
return outputDirectory;
}
+
Review Comment:
**[high]** The empty JAR injection silently masks missing compiled output.
Report plugins that inspect bytecode or source paths resolved from the
classpath (`maven-javadoc-plugin`, `maven-jxr-plugin`, `spotbugs-maven-plugin`)
would silently produce empty or incomplete output instead of failing with a
clear error. Today the failure is explicit — users know they need `mvn compile
site`. With this workaround, the build passes but the output is wrong.
##########
impl/maven-core/src/main/java/org/apache/maven/ReactorReader.java:
##########
@@ -217,13 +220,49 @@ private File
determineBuildOutputDirectoryForArtifact(final MavenProject project
if (projectHasOutputFromPreviousSession ||
projectCompiledDuringThisSession) {
return outputDirectory;
}
+
+ if (isSiteGoalRequested() && "jar".equals(artifact.getExtension())
&& "jar".equals(type)) {
+ return ensureEmptyArtifactFile(artifact);
+ }
}
// The fall-through indicates that the artifact cannot be found;
// for instance if package produced nothing or classifier problems.
return null;
}
+ private File ensureEmptyArtifactFile(final Artifact artifact) {
+ Path target = getEmptyArtifactPath(artifact);
+ return emptyArtifactCache.computeIfAbsent(target, path -> {
+ if (Files.isRegularFile(path)) {
+ return path.toFile();
+ }
+ try {
+ Files.createDirectories(path.getParent());
+ try (JarOutputStream ignored = new
JarOutputStream(Files.newOutputStream(path))) {
+ // create an empty jar for site reports that inspect
reactor artifacts before package
+ }
+ return path.toFile();
+ } catch (IOException e) {
+ LOGGER.warn("Unable to create empty reactor artifact for
'{}'.", artifact, e);
+ return null;
+ }
+ });
+ }
+
+ private boolean isSiteGoalRequested() {
Review Comment:
**[medium]** The `isSiteGoal` method hardcodes lifecycle phase names
(`pre-site`, `site`, `post-site`, `site-deploy`) and uses `endsWith(":site")`
pattern matching, coupling a low-level artifact resolution component to
lifecycle-specific knowledge. `ReactorReader`'s responsibility is workspace
artifact resolution — it should not contain lifecycle-specific workarounds.
This sets a precedent for adding other lifecycle-specific branches in the
future.
##########
its/core-it-suite/src/test/java/org/apache/maven/it/MavenITgh12064SiteReactorDependenciesTest.java:
##########
@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.it;
+
+import java.io.File;
+
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+
+/**
+ * This is a test set for <a
href="https://github.com/apache/maven/issues/12064";>GH-12064</a>.
+ */
+class MavenITgh12064SiteReactorDependenciesTest extends
AbstractMavenIntegrationTestCase {
+
+ @Test
+ void testSiteLifecycleResolvesReactorDependencies() throws Exception {
+ File testDir = extractResources("/gh-12064-site-reactor-dependencies");
+
+ Verifier verifier = newVerifier(testDir.getAbsolutePath());
+ verifier.setAutoclean(false);
+ verifier.deleteDirectory("producer/target");
+ verifier.deleteDirectory("consumer/target");
+ verifier.deleteArtifacts("org.apache.maven.its.gh12064");
+ verifier.addCliArgument("site");
+ verifier.execute();
+ verifier.verifyErrorFreeLog();
+
+ verifier.verifyFilePresent("consumer/target/site/dependencies.html");
+ assertFalse(new File(testDir, "producer/target/classes").exists(),
"site must not require compile first");
+ assertFalse(
+ new File(
Review Comment:
**[low]** The integration test only exercises basic `project-info-reports`
(`dependencies.html`). It does not test what happens when a report plugin that
needs compiled classes (`maven-javadoc-plugin`, `spotbugs-maven-plugin`) is
configured, which is the failure mode that matters most. A test that configures
`maven-javadoc-plugin` and verifies the output is complete (or at least
non-empty) would strengthen confidence in the approach.
--
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]
