elharo commented on code in PR #11702:
URL: https://github.com/apache/maven/pull/11702#discussion_r2775474671
##########
impl/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java:
##########
@@ -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 must not be used.
Review Comment:
not "and must not be used" but rather the complete build fails.
##########
impl/maven-core/src/main/java/org/apache/maven/project/SourceHandlingContext.java:
##########
@@ -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 must not be used because %s
resources are configured via %s in <sources>.",
Review Comment:
must not be used --> cannot be used in modular projects
##########
impl/maven-core/src/main/java/org/apache/maven/project/SourceHandlingContext.java:
##########
@@ -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 must not be used because modular
sources are configured. "
Review Comment:
must not --> cannot
##########
impl/maven-core/src/test/java/org/apache/maven/project/ProjectBuilderTest.java:
##########
@@ -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)
Review Comment:
not sure how the AC are being used, but shouldn't AC7 be changed rather than
superseded?
##########
impl/maven-core/src/main/java/org/apache/maven/project/SourceHandlingContext.java:
##########
@@ -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 must not be used because %s
resources are configured via %s in <sources>.",
Review Comment:
cannot
##########
impl/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java:
##########
@@ -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
* 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
+ * rejected because it is not clear how to dispatch their
content between
Review Comment:
I don't think this is what we were looking for. The build fails completely,
not simply a warning
##########
impl/maven-core/src/test/java/org/apache/maven/project/ProjectBuilderTest.java:
##########
@@ -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).
+ * In modular projects, legacy directories must not occur because it is
not clear
Review Comment:
In modular projects, legacy directories must not occur --> Legacy
directories cannot be used in modular projects
--
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]
