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

[ascheman]

When a project uses the Maven 4 module source hierarchy, the JAR plugin produces one JAR per Java module and attaches them as separate artifacts, without assigning a file to the project's main artifact — its path stays the (directory) build output.

With allowIncompleteProjects=true, install warned about the missing main file but still passed the directory to the installer, which failed:

Failed to install artifact …:jar:…: is a folder

(reported by @desruisseaux while building apache/sis.)

This drops the main artifact from the install request in that case and installs the POM + the attached per-module JARs only. A new InstallMojoTest case asserts the directory main artifact is excluded from the request (it fails without this change).

The existing "change packaging to 'pom'" warning is kept as-is for minimality; it now reads a bit oddly since the attachments install successfully — happy to reword if preferred.

[gnodet-bot]

The fix correctly handles the module-source-hierarchy case by excluding the main artifact (whose path is a directory) from the installer request. One observation about the user-facing messaging:

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[gnodet-bot]

⚠️ 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]

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!

[gnodet-bot]

Re-review after new commits — previous finding addressed.

The misleading warning block has been replaced with getLog().info(...) that accurately describes the outcome (POM + attachments installed). The logic is correct: iterating over installables while removing from the toInstall copy is safe (no ConcurrentModificationException). The new test correctly validates the fix — without it, the main artifact's key would appear in the assertion, causing a failure.

⚠️ Pre-fix validation: dynamic test run was not feasible (no-read-permission on the working directory). Static trace: the test's assertEquals on artifacts.stream().map(Artifact::key).toList() would include maven-install-test:jar:1.0-SNAPSHOT before the fix (main artifact not removed), and would not include it after — the assertion would fail before the fix, pass after. Test correctly validates the fix.

LGTM.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[gnodet]

LGTM as a workaround for Maven 4.0 — without this, module source hierarchy is completely unusable (the JAR plugin creates the per-module JARs, then install crashes on the directory-backed main artifact).

The proper fix belongs in Maven core and is tracked in apache/maven#13396 (Packaging.producesMainArtifact() API, targeted at 4.1.0). Once that lands, the JAR plugin will be able to signal "no main artifact" and install/deploy will handle it natively — making this code path dead.

One suggestion: consider dropping the allowIncompleteProjects=true requirement. When the main artifact path is a directory and attached artifacts exist, it's clearly an intentional multi-artifact build (module source hierarchy), not a misconfiguration. Requiring users to discover and set allowIncompleteProjects=true is a friction point, and the flag name is misleading for this use case. The auto-detection could simply be:

if (!isValidPath(installable) && installable == project.getMainArtifact().orElse(null)) {
    if (attachedArtifacts.isEmpty()) {
        throw new MojoException("...");
    }
    // Module source hierarchy (or similar): install POM + attachments only
    getLog().info("...");
    toInstall.remove(installable);
}

That way it just works without any flag.

Fully automatic review from Claude Code

Dismissing approval — suggestion to drop allowIncompleteProjects gate should be addressed first.

[gnodet]

This is a necessary workaround for Maven 4.0 (the proper fix is apache/maven#13396, targeting 4.1.0 with a Packaging.producesMainArtifact() API).

However, rather than gating on allowIncompleteProjects (a flag whose name and semantics don't match this use case), the install plugin should auto-detect module source hierarchy via the source roots API and handle it specifically:

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

This way:

• Module source hierarchy just works — no flag needed
• The existing allowIncompleteProjects logic stays intact for other edge cases (unchanged behavior for non-module projects)
• The check is precise: only triggers when the project actually declares <source><module>…</module></source>

See inline suggestion for the concrete change. The helper method above and its import should be added to the class. Test should be updated to remove @MojoParameter(name = "allowIncompleteProjects", value = "true") and instead set up source roots with a module.

Fully automatic review from Claude Code

[gnodet]

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());
}

[gnodet]

Alternative implementation in #468 — auto-detects module source hierarchy via ProjectManager.getSourceRoots() / SourceRoot.module() instead of gating on allowIncompleteProjects. No flag needed, existing allowIncompleteProjects behavior preserved unchanged.

Fully automatic review from Claude Code