upgrade bundled maven to 3.9.1 #5679

mbien · 2023-03-19T06:47:20Z

highlights.

uses compiler:3.10.1 plugin by default -> release compiler option and module info files are now supported out of the box
https://maven.apache.org/docs/3.9.1/release-notes.html
https://maven.apache.org/docs/3.9.0/release-notes.html

tested

new project creation, build, run, output parsing etc
local/remote indexing, hints and queries

observations, saw some extra logging during local repo scan, e.g:

[maven-local-indexing] INFO org.eclipse.aether.internal.impl.DefaultArtifactResolver - Artifact org.sonatype.nexus.buildsupport:nexus-buildsupport-bouncycastle:pom:2.14.3-02 is present in the local repository, but cached from a remote repository ID that is unavailable in current build context, verifying that is downloadable from [central (https://repo1.maven.org/maven2/, default, releases+snapshots)]

about 20 lines total on a 4 GB .m2 folder - so not too bad.

java/maven.indexer/src/org/netbeans/modules/maven/indexer/NexusRepositoryIndexerImpl.java

mbien · 2023-03-19T06:55:00Z

java/maven/src/org/netbeans/modules/maven/execute/MavenProxySupport.java

-            col -= (posEnd - posStart);
+            try {
+                col -= (posEndField.getInt(this) - posStartField.getInt(this));
+            } catch (IllegalAccessException ex) {
+                Exceptions.printStackTrace(ex);
+            }


this is ugly, those fields are now private since codehaus-plexus/plexus-utils@797cab4

@sdedic question two: how do I trigger this code path to check if this still works?

I think we have two options here for the mid term: find a different solution for this, or talk to upstream and make the two fields accessible again so that we can remove this hack.

This whole thing is triggered when:

maven settings.xml specifies a proxy

the system does not use a proxy (so no http*_proxy env var or reachable PAC file)

a maven build or priming build is triggered, or other request in 'online' mode.

I will try to create a simple unittest to verify the settings.xml parsing produces the expected results.

java/maven/test/unit/data/projects/dependencies/golden/testDuplicatedDependencies

sdedic · 2023-03-20T12:36:04Z

since it diffs dependency trees and the test project uses different versions

Strange -- since it uses a specific version of the micronaut and/or other dependencies, the set of dependencies should not change ? Or maybe I did not understand ?

sdedic · 2023-03-20T12:41:42Z

Added a simple test that asserts that things parsed out of settings.xml using XPP match the expectation and line/column coordinates point to the approriate text content.

mbien · 2023-03-20T16:53:06Z

since it diffs dependency trees and the test project uses different versions

Strange -- since it uses a specific version of the micronaut and/or other dependencies, the set of dependencies should not change ? Or maybe I did not understand ?

you fixed that already, I wrote that comment before pinging you, so its already outdated.

sdedic · 2023-03-22T07:13:44Z

I'd recommend to add some notes to our release notes file, namely these things from Maven relnotes might affect the ability of NB to parse / work with existing user files (bcs. of existing user plugins):

The Maven Resolver transport has changed from Wagon to “native HTTP”, see Resolver Transport guide.

Low priority, but if the new http transport behaves differently from the old one in subtle details, some network issues may happen.

Maven 2.x was auto-injecting an ancient version of plexus-utils dependency into the plugin classpath, and Maven 3.x continued doing this to preserve backward compatibility. Starting with Maven 3.9, it does not happen anymore. This change may lead to plugin breakage.

this seems more important, as the 3rd/4rd party plugins may break bcs. of our upgrade.

Each line in .mvn/maven.config is now interpreted as a single argument. That is, if the file contains multiple arguments, these must now be placed on separate lines, see MNG-7684.

may affect existing project local configurations

sdedic · 2023-03-22T07:19:44Z

java/maven/build.xml

@@ -56,8 +56,8 @@
        <!-- we use jarjar to repackage simple json, to avoid clashes with 3rd party maven plugins possibly including it in their dependencies -->
        <taskdef name="jarjar" classname="org.pantsbuild.jarjar.JarJarTask" loaderref="lib.path.loader">
            <classpath>
-                <pathelement location="./external/asm-7.0.jar"/>
-                <pathelement location="./external/asm-commons-7.0.jar"/>
+                <pathelement location="./external/asm-9.4.jar"/>


Question: asm is provided in platform/libs.asm too, but it seems that since its version is driven by jarjar, it's better to have a separate compile-time external here ?

i am not sure actually. I was just thinking that the best approach with asm is likely to use the same version everywhere if possible (its sometimes shaded). It is a library which is known to be safe to update.

my thinking was: step 1, have everything use the same version, step 2 in future, check if we can remove the duplicates where it makes sense (if not we just leave it).

@sdedic has now been deduplicated via #5751

mbien · 2023-03-22T10:36:55Z

I forgot updating:
https://github.com/apache/netbeans/blob/4c90ff36e0ac1d80b324b76fca0befc13bd750f6/java/maven.embedder/external/binariesembedded-list

this is called manually from

netbeans/java/maven.embedder/build.xml

Lines 30 to 33 in 4c90ff3

    
               <!-- To check sha1 binaries that are in maven zip (better maven artefacts generation  
        
                    Call manually on maven upgrade 
        
               --> 
        
               <target name="checkhash" depends="init">

and also from:

netbeans/nbbuild/build.xml

Lines 248 to 252 in 4c90ff3

    
           <!-- create list of all maven coordinate --> 
        
           <target name="getallmavencoordinates"> 
        
               <concat destfile="${nb_all}/nbbuild/build/external.info"> 
        
                   <fileset dir="${nb_all}" includes="**/external/binaries-list" /> 
        
                   <fileset dir="${nb_all}" includes="**/external/binariesembedded-list" />

this runs checksums again over the contents of the zip, additional to the checksum over the zip itself.

@ebarboni I suppose I have to update this too?

mbien · 2023-03-22T11:27:40Z

updated the binariesembedded-list

squashed the upgrade commits into one and squashed the test+fix from @sdedic into another commit and moved that commit in front of the upgrade commit because both can be tested independently.

planning to merge it as is without further squashing.

ebarboni · 2023-03-22T11:29:15Z

I forgot updating: https://github.com/apache/netbeans/blob/4c90ff36e0ac1d80b324b76fca0befc13bd750f6/java/maven.embedder/external/binariesembedded-list

this is called manually from

netbeans/java/maven.embedder/build.xml

Lines 30 to 33 in 4c90ff3



<target name="checkhash" depends="init">

and also from:

netbeans/nbbuild/build.xml

Lines 248 to 252 in 4c90ff3



<target name="getallmavencoordinates">

<concat destfile="${nb_all}/nbbuild/build/external.info">

<fileset dir="${nb_all}" includes="**/external/binaries-list" />

<fileset dir="${nb_all}" includes="**/external/binariesembedded-list" />

this runs checksums again over the contents of the zip, additional to the checksum over the zip itself.

@ebarboni I suppose I have to update this too?

Yes this is needed :D. Was looking at doing a PR for 3.8.8 but see yours for 3.9.1.

I wonder if jdom1.0 references is needed ? I do a test removing it and it compiles ok. (maven dependency try still needed)

mbien · 2023-03-22T11:32:06Z

something went wrong during squashing and the golden file wasn't reverted to the old state. have to investigate.

ebarboni · 2023-03-22T11:49:28Z

@mbien forget my comment on jdom it's used in maven.osgi for example. But could be later a libs like many others ?

+ propagate system properties to maven session.

mbien · 2023-03-22T13:49:44Z

all tests green. double checked that the golden file didn't change. merging.

mbien · 2023-03-23T10:47:03Z

I'd recommend to add some notes to our release notes file

@sdedic added the Highlight Change to be highlighted in release notes and announcements label so that we don't forget

-> #5679 (comment)

mbien added Upgrade Library Library (Dependency) Upgrade Maven [ci] enable "build tools" tests ci:all-tests [ci] enable all tests labels Mar 19, 2023

mbien added this to the NB18 milestone Mar 19, 2023

mbien changed the title ~~upgraded bundled maven to 3.9.1~~ upgrade bundled maven to 3.9.1 Mar 19, 2023

mbien commented Mar 19, 2023

View reviewed changes

mbien removed the ci:all-tests [ci] enable all tests label Mar 19, 2023

This comment was marked as outdated.

Sign in to view

mbien force-pushed the maven-391 branch from 43acb66 to 5abdb9a Compare March 19, 2023 11:02

mbien added ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) and removed ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) labels Mar 19, 2023

mbien marked this pull request as ready for review March 19, 2023 12:52

mbien commented Mar 20, 2023

View reviewed changes

java/maven/test/unit/data/projects/dependencies/golden/testDuplicatedDependencies Outdated Show resolved Hide resolved

sdedic approved these changes Mar 22, 2023

View reviewed changes

mbien mentioned this pull request Mar 22, 2023

[maven hints] try to infer compiler plugin version from active maven version. #5693

Merged

mbien force-pushed the maven-391 branch from 24f4b2d to 1a6636c Compare March 22, 2023 11:23

mbien added ci:all-tests [ci] enable all tests and removed ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) labels Mar 22, 2023

sdedic and others added 2 commits March 22, 2023 13:14

Added tests for settings proxy parsing.

0333ca9

+ propagate system properties to maven session.

upgrade bundled maven to 3.9.1.

d7118e8

mbien force-pushed the maven-391 branch from 1a6636c to d7118e8 Compare March 22, 2023 12:21

mbien merged commit 72861f2 into apache:master Mar 22, 2023

mbien added the Highlight Change to be highlighted in release notes and announcements label Mar 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

upgrade bundled maven to 3.9.1 #5679

upgrade bundled maven to 3.9.1 #5679

mbien commented Mar 19, 2023 •

edited

Loading

mbien Mar 19, 2023

mbien Mar 20, 2023

sdedic Mar 20, 2023

This comment was marked as outdated.

sdedic commented Mar 20, 2023 •

edited

Loading

sdedic commented Mar 20, 2023

mbien commented Mar 20, 2023

sdedic commented Mar 22, 2023

sdedic Mar 22, 2023

mbien Mar 22, 2023

mbien Apr 4, 2023

mbien commented Mar 22, 2023

mbien commented Mar 22, 2023

ebarboni commented Mar 22, 2023

mbien commented Mar 22, 2023

ebarboni commented Mar 22, 2023

mbien commented Mar 22, 2023

mbien commented Mar 23, 2023

upgrade bundled maven to 3.9.1 #5679

upgrade bundled maven to 3.9.1 #5679

Conversation

mbien commented Mar 19, 2023 • edited Loading

mbien Mar 19, 2023

Choose a reason for hiding this comment

mbien Mar 20, 2023

Choose a reason for hiding this comment

sdedic Mar 20, 2023

Choose a reason for hiding this comment

This comment was marked as outdated.

sdedic commented Mar 20, 2023 • edited Loading

sdedic commented Mar 20, 2023

mbien commented Mar 20, 2023

sdedic commented Mar 22, 2023

sdedic Mar 22, 2023

Choose a reason for hiding this comment

mbien Mar 22, 2023

Choose a reason for hiding this comment

mbien Apr 4, 2023

Choose a reason for hiding this comment

mbien commented Mar 22, 2023

mbien commented Mar 22, 2023

ebarboni commented Mar 22, 2023

mbien commented Mar 22, 2023

ebarboni commented Mar 22, 2023

mbien commented Mar 22, 2023

mbien commented Mar 23, 2023

mbien commented Mar 19, 2023 •

edited

Loading

sdedic commented Mar 20, 2023 •

edited

Loading