Skip to content

Fail on legacy config in modular projects #11702

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

Open
ascheman wants to merge 2 commits into
base: master
Choose a base branch
from
+92 −71
Open

Fail on legacy config in modular projects #11702

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c1b431b
Fail on legacy config in modular projects
ascheman Feb 6, 2026
a89fc07
Address review feedback on wording
ascheman Feb 8, 2026
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
42 changes: 22 additions & 20 deletions impl/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -693,27 +693,27 @@ private void initProject(MavenProject project, ModelBuilderResult result) {

/*
* `sourceDirectory`, `testSourceDirectory` and `scriptSourceDirectory`
* are ignored if the POM file contains at least one enabled <source> element
* are not used if the POM file contains at least one enabled <source> element
Copy link
Contributor

@elharo elharo Feb 10, 2026

Choose a reason for hiding this comment

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

again here don't we need the build to fail in this case?

Copy link
Contributor

@desruisseaux desruisseaux Feb 10, 2026

Choose a reason for hiding this comment

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

Yes, I think that it is a change that was forgot in the documentation. The actual code raises an error as I read it.

* for the corresponding scope and language. This rule exists because
* Maven provides default values for those elements which may conflict
* with user's configuration.
*
* Additionally, for modular projects, legacy directories are unconditionally
* ignored because it is not clear how to dispatch their content between
* different modules. A warning is emitted if these properties are explicitly set.
* rejected because it is not clear how to dispatch their content between
Copy link
Contributor

@elharo elharo Feb 6, 2026

Choose a reason for hiding this comment

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

I don't think this is what we were looking for. The build fails completely, not simply a warning

Copy link
Contributor

@desruisseaux desruisseaux Feb 7, 2026

Choose a reason for hiding this comment

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

Proposal: replace "A warning is emitted if …" by "The build fails if …".

* different modules. The build fails if these properties are explicitly set.
*/
if (!sourceContext.hasSources(Language.SCRIPT, ProjectScope.MAIN)) {
project.addScriptSourceRoot(build.getScriptSourceDirectory());
}
if (isModularProject) {
// Modular projects: unconditionally ignore legacy directories, warn if explicitly set
warnIfExplicitLegacyDirectory(
// Modular projects: legacy directories conflict with modular sources
failIfLegacyDirectoryPresent(
build.getSourceDirectory(),
baseDir.resolve("src/main/java"),
"<sourceDirectory>",
project.getId(),
result);
warnIfExplicitLegacyDirectory(
failIfLegacyDirectoryPresent(
build.getTestSourceDirectory(),
baseDir.resolve("src/test/java"),
"<testSourceDirectory>",
Expand Down Expand Up @@ -906,15 +906,17 @@ private void initProject(MavenProject project, ModelBuilderResult result) {
}

/**
* Warns about legacy directory usage in a modular project. Two cases are handled:
* Fails the build if a legacy directory is present in a modular project.
* <p>
* "Present" means either:
* <ul>
* <li>Case 1: The default legacy directory exists on the filesystem (e.g., src/main/java exists)</li>
* <li>Case 2: An explicit legacy directory is configured that differs from the default</li>
* <li><b>Configuration presence</b>: an explicit configuration differs from the default</li>
* <li><b>Physical presence</b>: the default directory exists on the filesystem</li>
* </ul>
* Legacy directories are unconditionally ignored in modular projects because it is not clear
* how to dispatch their content between different modules.
* In both cases, the legacy directory conflicts with modular sources and cannot be used.
* Failing the build forces the user to resolve the conflict explicitly.
*/
private void warnIfExplicitLegacyDirectory(
private void failIfLegacyDirectoryPresent(
String configuredDir,
Path defaultDir,
String elementName,
Expand All @@ -924,26 +926,26 @@ private void warnIfExplicitLegacyDirectory(
Path configuredPath = Path.of(configuredDir).toAbsolutePath().normalize();
Path defaultPath = defaultDir.toAbsolutePath().normalize();
if (!configuredPath.equals(defaultPath)) {
// Case 2: Explicit configuration differs from default - always warn
// Configuration presence: explicit config differs from default
String message = String.format(
"Legacy %s is ignored in modular project %s. "
"Legacy %s cannot be used in modular project %s."
+ "In modular projects, source directories must be defined via <sources> "
+ "with a module element for each module.",
elementName, projectId);
logger.warn(message);
logger.error(message);
result.getProblemCollector()
.reportProblem(new org.apache.maven.impl.model.DefaultModelProblem(
message, Severity.WARNING, Version.V41, null, -1, -1, null));
message, Severity.ERROR, Version.V41, null, -1, -1, null));
} else if (Files.isDirectory(defaultPath)) {
// Case 1: Default configuration, but the default directory exists on filesystem
// Physical presence: default directory exists on filesystem
String message = String.format(
"Legacy %s '%s' exists but is ignored in modular project %s. "
"Legacy %s '%s' exists but cannot be used in modular project %s."
+ "In modular projects, source directories must be defined via <sources>.",
elementName, defaultPath, projectId);
logger.warn(message);
logger.error(message);
result.getProblemCollector()
.reportProblem(new org.apache.maven.impl.model.DefaultModelProblem(
message, Severity.WARNING, Version.V41, null, -1, -1, null));
message, Severity.ERROR, Version.V41, null, -1, -1, null));
}
}
}
Expand Down
44 changes: 30 additions & 14 deletions impl/maven-core/src/main/java/org/apache/maven/project/SourceHandlingContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,11 +221,19 @@ void handleResourceConfiguration(ProjectScope scope) {
if (hasResourcesInSources) {
// Modular project with resources configured via <sources> - already added above
if (hasExplicitLegacyResources(resources, scopeId)) {
LOGGER.warn(
"Legacy {} element is ignored because {} resources are configured via {} in <sources>.",
legacyElement,
scopeId,
sourcesConfig);
String message = String.format(
"Legacy %s element cannot be used because %s resources are configured via %s in <sources>.",
legacyElement, scopeId, sourcesConfig);
LOGGER.error(message);
result.getProblemCollector()
.reportProblem(new DefaultModelProblem(
message,
Severity.ERROR,
Version.V41,
project.getModel().getDelegate(),
-1,
-1,
null));
} else {
LOGGER.debug(
"{} resources configured via <sources> element, ignoring legacy {} element.",
Expand All @@ -236,13 +244,13 @@ void handleResourceConfiguration(ProjectScope scope) {
// Modular project without resources in <sources> - inject module-aware defaults
if (hasExplicitLegacyResources(resources, scopeId)) {
String message = "Legacy " + legacyElement
+ " element is ignored because modular sources are configured. "
+ " element cannot be used because modular sources are configured. "
+ "Use " + sourcesConfig + " in <sources> for custom resource paths.";
LOGGER.warn(message);
LOGGER.error(message);
result.getProblemCollector()
.reportProblem(new DefaultModelProblem(
message,
Severity.WARNING,
Severity.ERROR,
Version.V41,
project.getModel().getDelegate(),
-1,
Expand All @@ -265,11 +273,19 @@ void handleResourceConfiguration(ProjectScope scope) {
if (hasResourcesInSources) {
// Resources configured via <sources> - already added above
if (hasExplicitLegacyResources(resources, scopeId)) {
LOGGER.warn(
"Legacy {} element is ignored because {} resources are configured via {} in <sources>.",
legacyElement,
scopeId,
sourcesConfig);
String message = String.format(
"Legacy %s element cannot be used because %s resources are configured via %s in <sources>.",
legacyElement, scopeId, sourcesConfig);
LOGGER.error(message);
result.getProblemCollector()
.reportProblem(new DefaultModelProblem(
message,
Severity.ERROR,
Version.V41,
project.getModel().getDelegate(),
-1,
-1,
null));
} else {
LOGGER.debug(
"{} resources configured via <sources> element, ignoring legacy {} element.",
Expand Down Expand Up @@ -319,7 +335,7 @@ private DefaultSourceRoot createModularResourceRoot(String module, ProjectScope
*
* @param resources list of resources to check
* @param scope scope (main or test)
* @return true if explicit legacy resources are present that would be ignored
* @return true if explicit legacy resources are present that conflict with modular sources
*/
private boolean hasExplicitLegacyResources(List<Resource> resources, String scope) {
if (resources.isEmpty()) {
Expand Down
77 changes: 40 additions & 37 deletions impl/maven-core/src/test/java/org/apache/maven/project/ProjectBuilderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,19 +422,21 @@ void testModularSourcesInjectResourceRoots() throws Exception {
}

/**
* Tests that when modular sources are configured alongside explicit legacy resources,
* the legacy resources are ignored and a warning is issued.
* Tests that when modular sources are configured alongside explicit legacy resources, an error is raised.
* <p>
* This verifies the behavior described in the design:
* - Modular projects with explicit legacy {@code <resources>} configuration should issue a warning
* - Modular projects with explicit legacy {@code <resources>} configuration should raise an error
* - The modular resource roots are injected instead of using the legacy configuration
* <p>
* Acceptance Criterion: AC2 (unified source tracking for all lang/scope combinations)
* Acceptance Criteria:
* - AC2 (unified source tracking for all lang/scope combinations)
* - AC8 (legacy directories error - supersedes AC7 which originally used WARNING)
Copy link
Contributor

@elharo elharo Feb 6, 2026

Choose a reason for hiding this comment

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

not sure how the AC are being used, but shouldn't AC7 be changed rather than superseded?

*
* @see <a href="https://github.com/apache/maven/issues/11612">Issue #11612</a>
* @see <a href="https://github.com/apache/maven/issues/11701#issuecomment-3858462609">AC8 definition</a>
*/
@Test
void testModularSourcesWithExplicitResourcesIssuesWarning() throws Exception {
void testModularSourcesWithExplicitResourcesIssuesError() throws Exception {
File pom = getProject("modular-sources-with-explicit-resources");

MavenSession mavenSession = createMavenSession(null);
Expand All @@ -447,19 +449,19 @@ void testModularSourcesWithExplicitResourcesIssuesWarning() throws Exception {

MavenProject project = result.getProject();

// Verify warnings are issued for ignored legacy resources
List<ModelProblem> warnings = result.getProblems().stream()
.filter(p -> p.getSeverity() == ModelProblem.Severity.WARNING)
.filter(p -> p.getMessage().contains("Legacy") && p.getMessage().contains("ignored"))
// Verify errors are raised for conflicting legacy resources (AC8)
List<ModelProblem> errors = result.getProblems().stream()
.filter(p -> p.getSeverity() == ModelProblem.Severity.ERROR)
.filter(p -> p.getMessage().contains("Legacy") && p.getMessage().contains("cannot be used"))
.toList();

assertEquals(2, warnings.size(), "Should have 2 warnings (one for resources, one for testResources)");
assertEquals(2, errors.size(), "Should have 2 errors (one for resources, one for testResources)");
assertTrue(
warnings.stream().anyMatch(w -> w.getMessage().contains("<resources>")),
"Should warn about ignored <resources>");
errors.stream().anyMatch(e -> e.getMessage().contains("<resources>")),
"Should error about conflicting <resources>");
assertTrue(
warnings.stream().anyMatch(w -> w.getMessage().contains("<testResources>")),
"Should warn about ignored <testResources>");
errors.stream().anyMatch(e -> e.getMessage().contains("<testResources>")),
"Should error about conflicting <testResources>");

// Verify modular resources are still injected correctly
List<SourceRoot> mainResourceRoots = project.getEnabledSourceRoots(ProjectScope.MAIN, Language.RESOURCES)
Expand All @@ -478,23 +480,23 @@ void testModularSourcesWithExplicitResourcesIssuesWarning() throws Exception {
}

/**
* Tests that legacy sourceDirectory and testSourceDirectory are ignored in modular projects.
* Tests that legacy sourceDirectory and testSourceDirectory raise an error in modular projects.
* <p>
* In modular projects, legacy directories are unconditionally ignored because it is not clear
* how to dispatch their content between different modules. A warning is emitted if these
* properties are explicitly set (differ from Super POM defaults).
* Legacy directories cannot be used in modular projects because it is not clear
* how to dispatch their content between different modules. An error is raised if these
* properties are explicitly set (and differ from Super POM defaults).
* <p>
* This verifies:
* - WARNINGs are emitted for explicitly set legacy directories in modular projects
* - sourceDirectory and testSourceDirectory are both ignored
* - ERRORs are raised for explicitly set legacy directories in modular projects
* - Only modular sources from {@code <sources>} are used
* <p>
* Acceptance Criteria:
* - AC1 (boolean flags eliminated - uses hasSources() for main/test detection)
* - AC7 (legacy directories warning - {@code <sourceDirectory>} and {@code <testSourceDirectory>}
* are unconditionally ignored with a WARNING in modular projects)
* - AC8 (legacy directories error - supersedes AC7 which originally used WARNING;
* {@code <sourceDirectory>} and {@code <testSourceDirectory>} are raising an ERROR in modular projects)
*
* @see <a href="https://github.com/apache/maven/issues/11612">Issue #11612</a>
* @see <a href="https://github.com/apache/maven/issues/11701#issuecomment-3858462609">AC8 definition</a>
*/
@Test
void testMixedSourcesModularMainClassicTest() throws Exception {
Expand All @@ -510,20 +512,21 @@ void testMixedSourcesModularMainClassicTest() throws Exception {

MavenProject project = result.getProject();

// Verify WARNINGs are emitted for explicitly set legacy directories
List<ModelProblem> warnings = result.getProblems().stream()
.filter(p -> p.getSeverity() == ModelProblem.Severity.WARNING)
.filter(p -> p.getMessage().contains("Legacy") && p.getMessage().contains("ignored in modular project"))
// Verify ERRORs are raised for explicitly set legacy directories (AC8)
List<ModelProblem> errors = result.getProblems().stream()
.filter(p -> p.getSeverity() == ModelProblem.Severity.ERROR)
.filter(p -> p.getMessage().contains("Legacy")
&& p.getMessage().contains("cannot be used in modular project"))
.toList();

// Should have 2 warnings: one for sourceDirectory, one for testSourceDirectory
assertEquals(2, warnings.size(), "Should have 2 warnings for ignored legacy directories");
// Should have 2 errors: one for sourceDirectory, one for testSourceDirectory
assertEquals(2, errors.size(), "Should have 2 errors for conflicting legacy directories");
assertTrue(
warnings.stream().anyMatch(w -> w.getMessage().contains("<sourceDirectory>")),
"Should warn about ignored <sourceDirectory>");
errors.stream().anyMatch(e -> e.getMessage().contains("<sourceDirectory>")),
"Should error about conflicting <sourceDirectory>");
assertTrue(
warnings.stream().anyMatch(w -> w.getMessage().contains("<testSourceDirectory>")),
"Should warn about ignored <testSourceDirectory>");
errors.stream().anyMatch(e -> e.getMessage().contains("<testSourceDirectory>")),
"Should error about conflicting <testSourceDirectory>");

// Get main Java source roots - should have modular sources, not classic sourceDirectory
List<SourceRoot> mainJavaRoots = project.getEnabledSourceRoots(ProjectScope.MAIN, Language.JAVA_FAMILY)
Expand All @@ -541,17 +544,17 @@ void testMixedSourcesModularMainClassicTest() throws Exception {
assertTrue(mainModules.contains("org.foo.moduleA"), "Should have main source for moduleA");
assertTrue(mainModules.contains("org.foo.moduleB"), "Should have main source for moduleB");

// Verify the classic sourceDirectory is NOT used (should be ignored)
// Verify the classic sourceDirectory is NOT used (rejected in modular projects)
boolean hasClassicMainSource = mainJavaRoots.stream().anyMatch(sr -> sr.directory()
.toString()
.replace(File.separatorChar, '/')
.contains("src/classic/main/java"));
assertTrue(!hasClassicMainSource, "Classic sourceDirectory should be ignored");
assertTrue(!hasClassicMainSource, "Classic sourceDirectory cannot be used");

// Test sources should NOT be added (legacy testSourceDirectory is ignored in modular projects)
// Test sources should NOT be added (legacy testSourceDirectory is rejected in modular projects)
List<SourceRoot> testJavaRoots = project.getEnabledSourceRoots(ProjectScope.TEST, Language.JAVA_FAMILY)
.toList();
assertEquals(0, testJavaRoots.size(), "Should have no test Java sources (legacy is ignored)");
assertEquals(0, testJavaRoots.size(), "Should have no test Java sources (legacy is rejected)");
}

/**
Expand All @@ -563,7 +566,7 @@ void testMixedSourcesModularMainClassicTest() throws Exception {
* <p>
* This verifies:
* - An ERROR is reported when both modular and non-modular sources exist in {@code <sources>}
* - sourceDirectory is ignored because {@code <source scope="main" lang="java">} exists
* - sourceDirectory is not used because {@code <source scope="main" lang="java">} exists
* <p>
* Acceptance Criteria:
* - AC1 (boolean flags eliminated - uses hasSources() for source detection)
Expand Down