Maven4 "legacy" mode and more #2380

cstamas · 2025-05-22T21:07:00Z

Simple ability to make Maven 4 "work like Maven 3" would.

Changes:

Resolver session (scope manager, transitive dep mgr) - DONE
model validation (in "Maven3" mode model 4.1.0 should be rejected) - DONE
scope validation (obey what scopeMgr holds) - DONE
inline POM transformation in Maven3 mode (instead of consumer POM transformation) - DONE
added IT that confirms: maven4 in maven3 mode does not need flatten plugin

Issues:
https://issues.apache.org/jira/browse/MNG-8742
https://issues.apache.org/jira/browse/MNG-8743
https://issues.apache.org/jira/browse/MNG-8744

Simple ability to make Maven 4 "work like Maven 3" would. TBD "inliner" transformer.

gnodet · 2025-05-23T10:54:28Z

api/maven-api-core/src/main/java/org/apache/maven/api/Constants.java

+     * @since 4.0.0
+     */
+    @Config(type = "java.lang.Boolean", defaultValue = "true")
+    public static final String MAVEN_MODERN_PERSONALITY = "maven.modernPersonality";


Would it be more intuitive to have a config 'maven3compatibility' (I don't like this naming either) with a default value to 'false' ?

Agreed, but went with maven.maven3Personality as Maven4 is per def "compatible" with Maven3, this is more like "run Maven4 fully in Maven3 mode".

elharo · 2025-05-23T20:54:26Z

api/maven-api-core/src/main/java/org/apache/maven/api/Constants.java

+     *
+     * @since 4.0.0
+     */
+    @Config(type = "java.lang.Boolean", defaultValue = "false")


MAVEN_MAVEN3_PERSONALITY sounds weird and duplicative. Maybe just MAVEN3_PERSONALITY?

This is really just to keep the "convention" of maven. prefixed properties. I agree, is awkward, but that is what it is.

The principle I picked up at Google (where this was explicitly called out) is to always follow the style guidelines even when surrounding existing code does not. Consistency does not trump correctness.

If we consider prefixing incorrect, then we should change them all.

elharo · 2025-05-23T20:54:52Z

api/maven-api-core/src/main/java/org/apache/maven/api/services/ModelBuilder.java

    String MODEL_VERSION_4_1_0 = "4.1.0";

-    List<String> VALID_MODEL_VERSIONS = List.of(MODEL_VERSION_4_0_0, MODEL_VERSION_4_1_0);
+    List<String> ALL_KNOWN_MODEL_VERSIONS = List.of(MODEL_VERSION_4_0_0, MODEL_VERSION_4_1_0);


KNOWN_MODEL_VERSIONS (delete ALL_)

elharo · 2025-05-23T20:55:48Z

...-core/src/main/java/org/apache/maven/internal/transformation/impl/PomInlinerTransformer.java

+    public void injectTransformedArtifacts(RepositorySystemSession session, MavenProject currentProject)
+            throws IOException {
+        if (!Features.consumerPom(session.getConfigProperties())) {
+            // TODO


do this here?

marked as draft

elharo · 2025-05-23T20:57:49Z

impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelValidator.java

    @Override
    @SuppressWarnings("checkstyle:MethodLength")
-    public void validateFileModel(Model m, int validationLevel, ModelProblemCollector problems) {
+    public void validateFileModel(Session s, Model m, int validationLevel, ModelProblemCollector problems) {


s --> session
wouldn't hurt to rename m to model too

Yeah, when am modifying existing code, I am trying to "adhere" to existing "style". I agree, but doing that (in fact doing all what you propose) would come with a LOT of unrelated diff...

Pankraz76

nicely done.

Pankraz76 · 2025-05-24T20:39:27Z

...mpl/src/main/java/org/apache/maven/impl/resolver/scopes/Maven3ScopeManagerConfiguration.java

 import org.eclipse.aether.impl.scope.ScopeManagerConfiguration;
 import org.eclipse.aether.internal.impl.scope.ScopeManagerDump;
-import org.eclipse.aether.scope.DependencyScope;
-import org.eclipse.aether.scope.ResolutionScope;


and what should I do with already imported org.apache.maven.api.DependencyScope?

ah sorry. I was expecting this to cause the full qualified access below. suspecting CI to avoid this in first place.

Pankraz76 · 2025-05-24T20:39:58Z

...mpl/src/main/java/org/apache/maven/impl/resolver/scopes/Maven3ScopeManagerConfiguration.java

+                .orElse(null);

-        ArrayList<ResolutionScope> result = new ArrayList<>();
+        ArrayList<org.eclipse.aether.scope.ResolutionScope> result = new ArrayList<>();


Suggested change

ArrayList<org.eclipse.aether.scope.ResolutionScope> result = new ArrayList<>();

ArrayList<ResolutionScope> result = new ArrayList<>();

might use import again.

Pankraz76 · 2025-05-24T20:44:52Z

src/site/markdown/maven-configuration.md

 | `maven.logger.showThreadId` | `Boolean` | If you would like to output the current thread id, then set to true. Defaults to false. |  `false`  | 4.0.0 | User properties |
 | `maven.logger.showThreadName` | `Boolean` | Set to true if you want to output the current thread name. Defaults to true. |  `true`  | 4.0.0 | User properties |
 | `maven.logger.warnLevelString` | `String` | The string value output for the warn level. Defaults to WARN. |  `WARN`  | 4.0.0 | User properties |
+| `maven.maven3Personality` | `Boolean` | User property for controlling "maven personality". If set to {@code true} Maven will behave as previous major version, Maven 3. |  `false`  | 4.0.0 | User properties |


Suggested change

| `maven.maven3Personality` | `Boolean` | User property for controlling "maven personality". If set to {@code true} Maven will behave as previous major version, Maven 3. | `false` | 4.0.0 | User properties |

| `maven.maven3Personality` | `Boolean` | User property for controlling "maven personality". If activated Maven will behave as previous major version, Maven 3. | `false` | 4.0.0 | User properties |

this is just an idea as true/false is kind of technical detail and quite known.
Might give business case more attention, rather then some data type used.

This is generated content.

gnodet · 2025-05-26T14:24:50Z

...-core/src/main/java/org/apache/maven/internal/transformation/impl/PomInlinerTransformer.java

+                String newVersion = version;
+                int lastEnd = -1;
+                if (version != null) {
+                    Matcher m = PLACEHOLDER_EXPRESSION.matcher(version.trim());


Using the Interpolator would be easier and safer here.

Pankraz76

nice coding so far.

Pankraz76 · 2025-05-26T18:34:10Z

...ven-core/src/main/java/org/apache/maven/internal/transformation/impl/TransformerSupport.java

+        throw new IllegalStateException("This transformer does not use this call.");
+    }
+
+    protected static final String NAMESPACE_FORMAT = "http://maven.apache.org/POM/%s";


consider grouping all fields like we would do for constructors: https://checkstyle.sourceforge.io/checks/coding/constructorsdeclarationgrouping.html

Pankraz76 · 2025-05-26T18:39:12Z

...-core/src/main/java/org/apache/maven/internal/transformation/impl/PomInlinerTransformer.java

+                    Matcher m = PLACEHOLDER_EXPRESSION.matcher(version.trim());
+                    while (m.find()) {
+                        String property = m.group(1);
+                        if (!session.getConfigProperties().containsKey(property)) {
+                            throw new IllegalArgumentException("Cannot inline property " + property);
+                        }
+                        String propertyValue =
+                                (String) session.getConfigProperties().get(property);
+                        if (lastEnd < 0) {
+                            newVersion = version.substring(0, m.start());
+                        } else {
+                            newVersion += version.substring(lastEnd, m.start());
+                        }
+                        lastEnd = m.end();
+                        newVersion += propertyValue;
+                    }
+                    if (!Objects.equals(version, newVersion)) {
+                        model = model.withVersion(newVersion);
+                        Path tmpPom = Files.createTempFile(
+                                project.getArtifactId() + "-" + project.getVersion() + "-", ".xml");
+                        write(model, tmpPom);
+                        project.setFile(tmpPom.toFile());
+                    }
+                }


Suggested change

Matcher m = PLACEHOLDER_EXPRESSION.matcher(version.trim());

while (m.find()) {

String property = m.group(1);

if (!session.getConfigProperties().containsKey(property)) {

throw new IllegalArgumentException("Cannot inline property " + property);

}

String propertyValue =

(String) session.getConfigProperties().get(property);

if (lastEnd < 0) {

newVersion = version.substring(0, m.start());

} else {

newVersion += version.substring(lastEnd, m.start());

}

lastEnd = m.end();

newVersion += propertyValue;

}

if (!Objects.equals(version, newVersion)) {

model = model.withVersion(newVersion);

Path tmpPom = Files.createTempFile(

project.getArtifactId() + "-" + project.getVersion() + "-", ".xml");

write(model, tmpPom);

project.setFile(tmpPom.toFile());

}

}

handleVersion(session, project, version, -1, version, model);

could give dedicated attention to this concern. Operating on all the fields is kind of smell also.

private static void handleVersion(RepositorySystemSession session, MavenProject project, String version, int lastEnd, String newVersion, Model model) throws IOException { Matcher m = PLACEHOLDER_EXPRESSION.matcher(version.trim()); while (m.find()) { String property = m.group(1); if (!session.getConfigProperties().containsKey(property)) { throw new IllegalArgumentException("Cannot inline property " + property); } String propertyValue = (String) session.getConfigProperties().get(property); if (lastEnd < 0) { newVersion = version.substring(0, m.start()); } else { newVersion += version.substring(lastEnd, m.start()); } lastEnd = m.end(); newVersion += propertyValue; } if (!Objects.equals(version, newVersion)) { model = model.withVersion(newVersion); Path tmpPom = Files.createTempFile( project.getArtifactId() + "-" + project.getVersion() + "-", ".xml"); write(model, tmpPom); project.setFile(tmpPom.toFile()); } }

Pankraz76 · 2025-05-26T18:40:06Z

...ven-core/src/main/java/org/apache/maven/internal/transformation/impl/TransformerSupport.java

+        MavenStaxReader reader = new MavenStaxReader();
+        try (InputStream is = Files.newInputStream(src)) {
+            return reader.read(is, false, null);
+        }


Suggested change

MavenStaxReader reader = new MavenStaxReader();

try (InputStream is = Files.newInputStream(src)) {

return reader.read(is, false, null);

}

try (InputStream is = Files.newInputStream(src)) {

return new MavenStaxReader().read(is, false, null);

}

could inline.

Pankraz76 · 2025-05-26T18:43:07Z

...ven-core/src/main/java/org/apache/maven/internal/transformation/impl/TransformerSupport.java

+        String version = model.getModelVersion();
+        Files.createDirectories(dest.getParent());
+        try (Writer w = Files.newBufferedWriter(dest)) {
+            MavenStaxWriter writer = new MavenStaxWriter();
+            writer.setNamespace(String.format(NAMESPACE_FORMAT, version));
+            writer.setSchemaLocation(String.format(SCHEMA_LOCATION_FORMAT, version));
+            writer.setAddLocationInformation(false);
+            writer.write(w, model);
+        }
+    }


Suggested change

String version = model.getModelVersion();

Files.createDirectories(dest.getParent());

try (Writer w = Files.newBufferedWriter(dest)) {

MavenStaxWriter writer = new MavenStaxWriter();

writer.setNamespace(String.format(NAMESPACE_FORMAT, version));

writer.setSchemaLocation(String.format(SCHEMA_LOCATION_FORMAT, version));

writer.setAddLocationInformation(false);

writer.write(w, model);

}

}

Files.createDirectories(dest.getParent());

try (Writer w = Files.newBufferedWriter(dest)) {

write(model.getModelVersion(), w, new MavenStaxWriter())

}

}

could delegate as well.

Pankraz76 · 2025-05-26T18:43:54Z

api/maven-api-core/src/main/java/org/apache/maven/api/services/ModelBuilder.java


    String MODEL_VERSION_4_1_0 = "4.1.0";

-    List<String> ALL_KNOWN_MODEL_VERSIONS = List.of(MODEL_VERSION_4_0_0, MODEL_VERSION_4_1_0);


yes, all is kind of implied.

Pankraz76

good structure already.

Pankraz76 · 2025-06-04T15:32:21Z

...-core/src/main/java/org/apache/maven/internal/transformation/impl/PomInlinerTransformer.java

-                                project.getArtifactId() + "-" + project.getVersion() + "-", ".xml");
-                        write(model, tmpPom);
-                        project.setFile(tmpPom.toFile());
+                        needsInlining(session).addAll(usedProperties);


might give dedication as well, if makes sense.

Suggested change

needsInlining(session).addAll(usedProperties);

needsInlining(session).addAll(getUsedProperties(interpolator));

might be not performant as n+1 iteration.

because the code does something like simple map operation on kind collection interpolator.interpolate(). Idk this structure so well im sorry. Somehow can not reach the code, so my knowledge very short.

Pankraz76 · 2025-06-04T15:33:44Z

...-core/src/main/java/org/apache/maven/internal/transformation/impl/PomInlinerTransformer.java

                }
                String newVersion;
                if (version != null) {
+                    HashSet<String> usedProperties = new HashSet<>();


Pankraz76 · 2025-06-04T15:34:48Z

...-core/src/main/java/org/apache/maven/internal/transformation/impl/PomInlinerTransformer.java

                        return (String) session.getConfigProperties().get(property);
                    });
                    if (!Objects.equals(version, newVersion)) {
-                        if (parentVersion) {


cohesion. good giving dedicated concern.

Pankraz76 · 2025-06-04T15:45:47Z

...-core/src/main/java/org/apache/maven/internal/transformation/impl/PomInlinerTransformer.java

+            return artifacts;
+        }
+        ArrayList<Artifact> newArtifacts = new ArrayList<>(artifacts.size());
+        for (Artifact artifact : artifacts) {


Suggested change

for (Artifact artifact : artifacts) {

return artifacts.map(....)

could avoid overhead, use only for body as map operation.

Its already very nice functional and compact. Lets complete this intent.

its just 1 LOC

return isEmpty() ? artifacts : artifacts.map(toFoo())

Pankraz76 · 2025-06-06T16:18:01Z

...ven-core/src/main/java/org/apache/maven/internal/transformation/impl/TransformerSupport.java

-        MavenStaxReader reader = new MavenStaxReader();
        try (InputStream is = Files.newInputStream(src)) {
-            return reader.read(is, false, null);
+            return new MavenStaxReader().read(is, false, null);


Pankraz76 · 2025-06-06T16:19:36Z

...-core/src/main/java/org/apache/maven/internal/transformation/impl/PomInlinerTransformer.java

                    if (!Objects.equals(version, newVersion)) {
-                        model = model.withVersion(newVersion);
+                        if (parentVersion) {
+                            model = model.withParent(model.getParent().withVersion(newVersion));


model = parentVersion ? model.withParent(model.getParent().withVersion(newVersion)) : model.withVersion(newVersion);

jira-importer · 2025-07-07T09:15:09Z

Resolve #9552

Maven4 "legacy" mode and more

7128e95

Simple ability to make Maven 4 "work like Maven 3" would. TBD "inliner" transformer.

cstamas self-assigned this May 22, 2025

cstamas requested a review from gnodet May 22, 2025 21:07

gnodet reviewed May 23, 2025

View reviewed changes

cstamas added 7 commits May 23, 2025 14:36

Apply PR comments

e664445

Fix javadoc

0c69e92

Update generated stuff

ceb2528

Rename const, tidy up

2e16d72

Merge remote-tracking branch 'upstream/master' into maven4-maven3-mode

e6ca248

WIP

f24faf6

Drop unused

121c95c

cstamas requested a review from gnodet May 23, 2025 16:18

cstamas marked this pull request as ready for review May 23, 2025 17:45

elharo reviewed May 23, 2025

View reviewed changes

cstamas marked this pull request as draft May 23, 2025 21:11

Pankraz76 reviewed May 24, 2025

View reviewed changes

cstamas added 2 commits May 26, 2025 14:19

Rename ALL_KNOWN_MODEL_VERSIONS -> KNOWN_MODEL_VERSIONS

5e63387

Make inliner

d501096

gnodet reviewed May 26, 2025

View reviewed changes

Pankraz76 reviewed May 26, 2025

View reviewed changes

cstamas added 9 commits June 4, 2025 12:04

Merge remote-tracking branch 'upstream/master' into maven4-maven3-mode

2c4c5d5

Reword

8d8f12c

Rework to interpolator, add generated

6bfa6c6

Merge remote-tracking branch 'upstream/master' into maven4-maven3-mode

c1b17f3

Handle parent as well

30e3475

Explain the intent

83bf9ad

Simplify

d02447e

Fix inliner, add IT

308f822

Make IT pass by nuking project local repo

b007d89

cstamas marked this pull request as ready for review June 4, 2025 14:23

cstamas requested a review from slawekjaranowski June 4, 2025 14:23

Pankraz76 reviewed Jun 4, 2025

View reviewed changes

gnodet added the enhancement New feature or request label Jun 4, 2025

gnodet added this to the 4.0.0-rc-4 milestone Jun 4, 2025

gnodet approved these changes Jun 6, 2025

View reviewed changes

Pankraz76 reviewed Jun 6, 2025

View reviewed changes

cstamas added 2 commits June 6, 2025 18:46

Merge remote-tracking branch 'upstream/master' into maven4-maven3-mode

1179f61

Meh

c57c8e4

cstamas merged commit 5000b59 into apache:master Jun 6, 2025
18 of 19 checks passed

cstamas deleted the maven4-maven3-mode branch June 6, 2025 20:01

cstamas mentioned this pull request Aug 6, 2025

Add integration test for Maven 4 new dependency scopes (MNG-8750) #11025

Merged

	ArrayList<org.eclipse.aether.scope.ResolutionScope> result = new ArrayList<>();
	ArrayList<ResolutionScope> result = new ArrayList<>();

	\| `maven.maven3Personality` \| `Boolean` \| User property for controlling "maven personality". If set to {@code true} Maven will behave as previous major version, Maven 3. \| `false` \| 4.0.0 \| User properties \|
	\| `maven.maven3Personality` \| `Boolean` \| User property for controlling "maven personality". If activated Maven will behave as previous major version, Maven 3. \| `false` \| 4.0.0 \| User properties \|


		String MODEL_VERSION_4_1_0 = "4.1.0";

		List<String> ALL_KNOWN_MODEL_VERSIONS = List.of(MODEL_VERSION_4_0_0, MODEL_VERSION_4_1_0);

	needsInlining(session).addAll(usedProperties);
	needsInlining(session).addAll(getUsedProperties(interpolator));

	for (Artifact artifact : artifacts) {
	return artifacts.map(....)

Maven4 "legacy" mode and more #2380

Maven4 "legacy" mode and more #2380

Uh oh!

Conversation

cstamas commented May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cstamas May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Pankraz76 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Pankraz76 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Pankraz76 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Pankraz76 Jun 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cstamas commented May 22, 2025 •

edited

Loading

cstamas May 23, 2025 •

edited

Loading

Pankraz76 Jun 4, 2025 •

edited

Loading