[MNG-6656] Introduce base for build/consumer process #286

rfscholte · 2019-10-01T17:31:18Z

Following this checklist to help us incorporate your
contribution quickly and easily:

Make sure there is a JIRA issue filed
for the change (usually before you start working on it). Trivial changes like typos do not
require a JIRA issue. Your pull request should address just this issue, without
pulling in other changes.
Each commit in the pull request should have a meaningful subject line and body.
Format the pull request title like [MNG-XXX] - Fixes bug in ApproximateQuantiles,
where you replace MNG-XXX with the appropriate JIRA issue. Best practice
is to use the JIRA issue title in the pull request title and in the first line of the
commit message.
Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
Run mvn clean verify to make sure basic checks pass. A more thorough check will
be performed on your pull request automatically.
You have run the Core IT successfully.

If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.

To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.

I hereby declare this contribution to be licenced under the Apache License Version 2.0, January 2004

… depend on maven-core

…hen using --fail-fast Otherwise the sessionFinished event may be sent before some projects/goals finish, which violates the contracts of execution listeners and makes it hard to properly life cycle components that are needed for goal execution. This closes #272

…rue' on child ITs (by default on Jenkins CI).

#269) * [MNG-6713] Fix ExclusionArtifactFilter to respect wildcard exclusions. * Moved `*` to a constant

…el-builder and maven-core Provide BuildPomXMlFilter to set CI Friendly properties

…into MNG-6656

pom.xml

...re/src/main/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactory.java

maven-core/src/main/java/org/apache/maven/xml/internal/DefaultBuildPomXMLFilterFactory.java

maven-core/src/test/java/org/apache/maven/xml/filter/ConsumerPomXMLFilterTest.java

maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java

maven-xml/src/test/java/org/apache/maven/xml/filter/ConsumerPomXMLFilterTest.java

rmannibucau

Put some minor comments (not directly related to the core feature so take them more as side notes).

On the feature I have mainly one point: are the transformed pom and real project pom attached? Think it makes sense to attach both but didnt spot the code doing that (original classifier for the project one maybe?)

maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelCacheManager.java

maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java

maven-xml/src/main/java/org/apache/maven/xml/filter/BuildPomXMLFilterFactory.java

maven-xml/src/main/java/org/apache/maven/xml/SAXEventUtils.java

maven-xml/src/main/java/org/apache/maven/xml/filter/DependencyKey.java

rfscholte · 2019-10-01T19:45:56Z

On the feature I have mainly one point: are the transformed pom and real project pom attached? Think it makes sense to attach both but didnt spot the code doing that (original classifier for the project one maybe?)

The pom is being transformed during the process, making it suitable for its purpose. At this moment in the beginning the pom is enriched with elements so it reflects the max pom is people/IDE/... are using right now. During distribution (install/deploy) the pom will be cleaned up from some build specific information as the parents relativePath and modules. Hence there should be no reason to attach more than one pom. You must trust that Maven is doing it correct, and for that reason I've tried to be very strict with class exposure, most are package private.

These are just examples that should give us hopefully enough feedback if the process works.

Tibor17 · 2019-10-01T20:06:52Z

@rfscholte
I found this in the emails. You should not throw RuntimeException in DefaultModelBuilder. It is bad practice and seen few more times in the Maven. You should find better subclass e.g. IllegalStateException or create a new exception class or use Callable instead of Runnable.

rmannibucau · 2019-10-01T20:41:49Z

The reason to attach both is that dev will prefer original pom and tools the deployed one.
At the end syntaxes can diverge significantly so exposing both is likely needed if we want to propose another format IMHO, otherwise v4 will stay in projects/sources too i think.

rfscholte · 2019-10-01T20:55:35Z

Here's how I see it:
Maven 3.7.x+ : experiment with adjusting files using Model 4.0.0
Maven 4.0.0 : apply adjusting files using Model 4.0.0
Maven 5.0.0: Introduce model 5.0.0, while distributing backport to 4.0.0 and additional files.

So after Maven 5.0.0 it could be there's a huge difference between local pom-file and the distributed files, so I don't see any reason to attach original file.
And it also comes with new challenges: what would be the filename of all attached poms?

My suggestion (to keep it simple) let's wait for feedback if the need is there. And if there's a need, I'd prefer a new goal to the maven-helper-plugin, which can show you all possible poms during any phase.
We need to get use to different output files, otherwise it'll block any chance of evolution.

...re/src/main/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactory.java

maven-core/src/main/java/org/apache/maven/xml/filter/ConsumerPomXMLFilter.java

maven-core/src/main/java/org/apache/maven/xml/filter/FastForwardFilter.java

maven-core/src/main/java/org/apache/maven/xml/internal/DefaultBuildPomXMLFilterFactory.java

maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java

maven-xml/src/main/java/org/apache/maven/xml/SAXEventUtils.java

eolivelli

Very interesting and promising work.
Thanks for moving this forward.

I left a few comments, mostly about security/performances.
I figured out that Romain also wrote similar comments over other lines of code.

Side note: maybe it is better to have a separate commit for the version change fron 3.6.x to 3.7.0, but it is not a big deal

maven-xml/src/main/java/org/apache/maven/xml/SAXEventFactory.java

maven-xml/src/test/java/org/apache/maven/xml/filter/AbstractXMLFilterTests.java

rmannibucau · 2019-10-02T04:51:45Z

About attaching pom: i take gradle users as a reference. They create a fake pom which makes the project link completly lost. So having the original file enables to understand the build and going on nexus is a quick way to review a build so it is worth having it built-in, at least as a toggle but you are right it can belong to install plugin, does not need to be there while path is accessible.

eolivelli

LGTM

maven-model-builder/src/main/java/org/apache/maven/feature/Features.java

maven-core/src/main/java/org/apache/maven/internal/aether/ConsumerModelSourceTransformer.java

maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java

maven-core/src/main/java/org/apache/maven/project/ProjectModelResolver.java

maven-core/src/main/java/org/apache/maven/xml/internal/DefaultConsumerPomXMLFilterFactory.java

...el-builder/src/main/java/org/apache/maven/model/building/AbstractModelSourceTransformer.java

maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java

maven-model-builder/src/main/java/org/apache/maven/model/building/ModelSourceTransformer.java

maven-model-builder/src/main/java/org/apache/maven/model/building/TransformerContext.java

maven-xml/src/main/java/org/apache/maven/xml/sax/filter/BuildPomXMLFilterFactory.java

maven-xml/src/main/java/org/apache/maven/xml/sax/filter/BuildPomXMLFilterListener.java

maven-xml/src/test/java/org/apache/maven/xml/sax/LexicalHandlerVerifier.java

mthmulders

Overall, it looks good to me, although I have left a few questions here & there.

...re/src/main/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactory.java

maven-xml/src/main/java/org/apache/maven/xml/sax/filter/AbstractEventXMLFilter.java

michael-o

Impressive change!

maven-core/src/main/java/org/apache/maven/internal/aether/ConsumerModelSourceTransformer.java

maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java

maven-core/src/main/java/org/apache/maven/project/ReactorModelPool.java

...el-builder/src/main/java/org/apache/maven/model/building/AbstractModelSourceTransformer.java

...model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuildingRequest.java

rfscholte · 2020-06-22T21:22:39Z

Fixed in bdec668

rfscholte · 2022-05-17T19:58:26Z

maven-xml/src/main/java/org/apache/maven/xml/sax/filter/FastForwardFilter.java

+
+                    originalHandler = getContentHandler();
+
+                    ContentHandler outputContentHandler = getContentHandler();


@mthmulders here's the original code, where you'll see it doesn't write to its direct parent (which is another XMLFilter), but all the way up to the effective outputstream, to ensure these won't be part of the output manipulation. I don't know how @gnodet eventually solved this on an event based stream.

There's a call to enable() / disable() on the delegate parser. I'll have to add a unit test in order to debug what is happening and see where the problem is ...

jira-importer · 2025-07-06T15:52:41Z

Resolve #8364

rfscholte and others added 18 commits May 14, 2019 21:26

[MNG-6656] Introduce base for build/consumer pom

65a5a1b

[MNG-6656] Introduce base for build/consumer pom

dfb0c2c

[MNG-6656] separate module for maven-xml, maven-model-builder doesn't…

79c4813

… depend on maven-core

[MNG-6725] Skip '.mavenrc' via MAVEN_SKIP_RC=1 and '-Dmaven.skip.rc=t…

144076c

…rue' on child ITs (by default on Jenkins CI).

IT archives

b9c7c4a

improved handlers documentation: artifact vs dependency attributes

5822ea7

[INFRA-18812] Revert unnecessary commits after fixing INFRA-18812

0197248

fixed ArtifactHandlerTest after commit d5b0f4c

297a6b7

[MNG-6713] Fix ExclusionArtifactFilter to respect wildcard exclusions. (

e6240d5

#269) * [MNG-6713] Fix ExclusionArtifactFilter to respect wildcard exclusions. * Moved `*` to a constant

Merge remote-tracking branch 'remotes/origin/master' into MNG-6656

6e624f0

Move Filters to maven-xml filter, so it can be picked up by maven-mod…

5065aec

…el-builder and maven-core Provide BuildPomXMlFilter to set CI Friendly properties

Merge branch 'MNG-6656' of https://gitbox.apache.org/repos/asf/maven …

e12b732

…into MNG-6656

Add missing classes

82d8693

[MNG-6656] Support versionless parent

a684370

[MNG-6656] Support reactor managed versions

712b95f

Merge branch 'master' into MNG-6656

2f4d491

[MNG-6656] Update project versions

b46a4c7

sormuras reviewed Oct 1, 2019

View reviewed changes

pom.xml Show resolved Hide resolved

michael-o requested changes Oct 1, 2019

View reviewed changes

rmannibucau reviewed Oct 1, 2019

View reviewed changes

eolivelli reviewed Oct 1, 2019

View reviewed changes

eolivelli requested changes Oct 1, 2019

View reviewed changes

maven-xml/src/main/java/org/apache/maven/xml/SAXEventFactory.java Outdated Show resolved Hide resolved

maven-xml/src/test/java/org/apache/maven/xml/filter/AbstractXMLFilterTests.java Outdated Show resolved Hide resolved

rfscholte added 2 commits October 2, 2019 23:08

[MNG-6656] Applying review comments

b19b05c

[MNG-6656] Support LexicalHandler

c52f383

rfscholte added 3 commits June 7, 2020 14:25

Prevent NPE

be5d23d

Adjust apache-maven-wrapper pom for IT

020b503

Merge branch 'master' into MNG-6656

dc1d8c7

eolivelli reviewed Jun 14, 2020

View reviewed changes

maven-model-builder/src/main/java/org/apache/maven/feature/Features.java Show resolved Hide resolved