Skip to content

Skip the main artifact when its file is a directory (module source hierarchy) #466

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ascheman wants to merge 1 commit into from
Closed

Skip the main artifact when its file is a directory (module source hierarchy) #466

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 10 additions & 8 deletions src/main/java/org/apache/maven/plugins/install/InstallMojo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ private ArtifactInstallerRequest processProject(Project project) {
ProjectManager projectManager = getProjectManager();
Collection<ProducedArtifact> installables = projectManager.getAllArtifacts(project);
Collection<ProducedArtifact> attachedArtifacts = projectManager.getAttachedArtifacts(project);
List<ProducedArtifact> toInstall = new ArrayList<>(installables);

getArtifactManager().setPath(project.getPomArtifact(), project.getPomPath());

Expand All @@ -260,13 +261,14 @@ private ArtifactInstallerRequest processProject(Project project) {
"The packaging for this project did not assign a file to the build artifact");
} else {
if (allowIncompleteProjects) {
getLog().warn("");
getLog().warn("The packaging plugin for this project did not assign");
getLog().warn(
"a main file to the project but it has attachments. Change packaging to 'pom'.");
getLog().warn("");
getLog().warn("Incomplete projects like this will fail in future Maven versions!");
getLog().warn("");
// The main artifact has no file: its path is e.g. the output directory of a module
// source hierarchy, where the JAR plugin attaches one JAR per Java module instead of
// assigning a single main JAR. Install the POM and the attachments only; do not hand
// the directory to the installer, which would otherwise fail with "is a folder".
getLog().info("The project's main artifact has no file"
+ " (e.g. a module source hierarchy produces one artifact per Java module);"
+ " installing the POM and the attached artifacts only.");
toInstall.remove(installable);

@gnodet-bot gnodet-bot Oct 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Misleading warning for module-source-hierarchy projects

After this change, for module-source-hierarchy projects the install succeeds (POM + attachments are installed), yet the surrounding warning still says:

"Incomplete projects like this will fail in future Maven versions!"

This is now factually wrong for the case this PR targets — the install does not fail. The warning was written for the truly broken case (missing file + attachments but wrong packaging), not for legitimate module-source-hierarchy usage.

You noted this in the PR description ("it now reads a bit oddly") — worth addressing: either split the warning so it only fires for the unresolved case, or soften the wording when the remove path is taken. Something like:

getLog().info("Skipping directory-backed main artifact — "
    + "installing POM and attachments only (module source hierarchy).");

rather than emitting a doom-and-gloom warning that contradicts the successful outcome.

@ascheman ascheman Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed: replaced the misleading "will fail in future Maven versions" warning with an informational log message on the remove path — when the directory-backed main artifact is skipped, it now says the POM and attachments are installed only. Applied the same to maven-deploy-plugin#717 for consistency. Thanks for the catch!

} else {
throw new MojoException("The packaging plugin for this project did not assign "
+ "a main file to the project but it has attachments. Change packaging to 'pom'.");
Comment on lines 262 to 274

@gnodet gnodet Oct 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
} else {
if (allowIncompleteProjects) {
getLog().warn("");
getLog().warn("The packaging plugin for this project did not assign");
getLog().warn(
"a main file to the project but it has attachments. Change packaging to 'pom'.");
getLog().warn("");
getLog().warn("Incomplete projects like this will fail in future Maven versions!");
getLog().warn("");
// The main artifact has no file: its path is e.g. the output directory of a module
// source hierarchy, where the JAR plugin attaches one JAR per Java module instead of
// assigning a single main JAR. Install the POM and the attachments only; do not hand
// the directory to the installer, which would otherwise fail with "is a folder".
getLog().info("The project's main artifact has no file"
+ " (e.g. a module source hierarchy produces one artifact per Java module);"
+ " installing the POM and the attached artifacts only.");
toInstall.remove(installable);
} else {
throw new MojoException("The packaging plugin for this project did not assign "
+ "a main file to the project but it has attachments. Change packaging to 'pom'.");
} else if (usesModuleSourceHierarchy(project, projectManager)) {
getLog().info("Module source hierarchy: no main artifact produced;"
+ " installing POM and per-module artifacts only.");
toInstall.remove(installable);
} else if (allowIncompleteProjects) {
getLog().warn("");
getLog().warn("The packaging plugin for this project did not assign");
getLog().warn(
"a main file to the project but it has attachments. Change packaging to 'pom'.");
getLog().warn("");
getLog().warn("Incomplete projects like this will fail in future Maven versions!");
getLog().warn("");
toInstall.remove(installable);
} else {
throw new MojoException("The packaging plugin for this project did not assign "
+ "a main file to the project but it has attachments. Change packaging to 'pom'.");

Flattens the nested if/else into a chain with module source hierarchy first (auto-detected, no flag), then allowIncompleteProjects (preserved for other edge cases), then error. Also adds toInstall.remove(installable) in the allowIncompleteProjects branch (the original PR only removes it in the module-hierarchy branch).

Needs a helper method added to the class:

private static boolean usesModuleSourceHierarchy(Project project, ProjectManager projectManager) {
    return projectManager.getSourceRoots(project).stream()
            .anyMatch(sr -> sr.module().isPresent());
}

Expand All @@ -279,7 +281,7 @@ private ArtifactInstallerRequest processProject(Project project) {
}
}

return ArtifactInstallerRequest.build(session, installables);
return ArtifactInstallerRequest.build(session, toInstall);
}

private boolean isValidPath(Artifact a) {
Expand Down
28 changes: 28 additions & 0 deletions src/test/java/org/apache/maven/plugins/install/InstallMojoTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,34 @@ void basicInstallWithAttachedArtifacts(InstallMojo mojo) throws Exception {
artifacts.stream().map(Artifact::key).toList());
}

@Test
@InjectMojo(goal = "install")
@MojoParameter(name = "installAtEnd", value = "false")
@MojoParameter(name = "allowIncompleteProjects", value = "true")
void mainArtifactWithDirectoryPathIsSkippedWhenAttachmentsExist(InstallMojo mojo) throws Exception {
// Module source hierarchy: the JAR plugin attaches one JAR per Java module and never assigns a
// file to the main artifact, whose path stays the (directory) build output. Install must install
// the POM + attachments and must NOT hand the directory to the installer (which fails with
// "is a folder"). So the main artifact is dropped from the install request.
assertNotNull(mojo);
Project project = (Project) getVariableValueFromObject(mojo, "project");
projectManager.attachArtifact(
project,
new ProducedArtifactStub("org.apache.maven.test", "foo.bar", "", "1.0-SNAPSHOT", "jar"),
Paths.get(getBasedir(), "target/test-classes/unit/attached-artifact-test-1.0-SNAPSHOT.jar"));
artifactManager.setPath(project.getMainArtifact().get(), Paths.get(getBasedir(), "target/test-classes/unit"));

ArtifactInstallerRequest request = execute(mojo);

assertNotNull(request);
Collection<ProducedArtifact> artifacts = request.getArtifacts();
assertEquals(
Arrays.asList(
"org.apache.maven.test:maven-install-test:pom:1.0-SNAPSHOT",
"org.apache.maven.test:foo.bar:jar:1.0-SNAPSHOT"),
artifacts.stream().map(Artifact::key).toList());
}

@Test
@InjectMojo(goal = "install")
void installIfArtifactFileIsNull(InstallMojo mojo) throws Exception {
Expand Down
Loading