From b18747db2c4d67ceafc81a9772d2268f5a3cc158 Mon Sep 17 00:00:00 2001 From: bzamfir Date: Mon, 25 Jul 2022 23:06:22 +0300 Subject: [PATCH] update-properties does not work across parent-child pom Fixes #582 o Updated PomHelper to validate version properties also across parents. --- src/it-repo/dummy-parent-issue-582-1.0.pom | 23 ++++ .../invoker.properties | 1 + src/it/it-update-properties-issue-582/pom.xml | 20 ++++ .../verify.groovy | 3 + .../codehaus/mojo/versions/api/PomHelper.java | 110 ++++++++++-------- 5 files changed, 107 insertions(+), 50 deletions(-) create mode 100644 src/it-repo/dummy-parent-issue-582-1.0.pom create mode 100644 src/it/it-update-properties-issue-582/invoker.properties create mode 100644 src/it/it-update-properties-issue-582/pom.xml create mode 100644 src/it/it-update-properties-issue-582/verify.groovy diff --git a/src/it-repo/dummy-parent-issue-582-1.0.pom b/src/it-repo/dummy-parent-issue-582-1.0.pom new file mode 100644 index 0000000000..65fc001b6d --- /dev/null +++ b/src/it-repo/dummy-parent-issue-582-1.0.pom @@ -0,0 +1,23 @@ + + 4.0.0 + + localhost + dummy-parent-issue-582 + 1.0 + pom + + + 1.0 + + + + + localhost + dummy-api + ${api} + + + + + diff --git a/src/it/it-update-properties-issue-582/invoker.properties b/src/it/it-update-properties-issue-582/invoker.properties new file mode 100644 index 0000000000..9b9e55e13f --- /dev/null +++ b/src/it/it-update-properties-issue-582/invoker.properties @@ -0,0 +1 @@ +invoker.goals=${project.groupId}:${project.artifactId}:${project.version}:update-properties \ No newline at end of file diff --git a/src/it/it-update-properties-issue-582/pom.xml b/src/it/it-update-properties-issue-582/pom.xml new file mode 100644 index 0000000000..68203983f2 --- /dev/null +++ b/src/it/it-update-properties-issue-582/pom.xml @@ -0,0 +1,20 @@ + + 4.0.0 + + localhost + dummy-parent-issue-582 + 1.0 + + + localhost + it-update-properties-issue-582 + 1.0 + pom + update-properties with one property, with dependency in parent + + + 1.0 + + + diff --git a/src/it/it-update-properties-issue-582/verify.groovy b/src/it/it-update-properties-issue-582/verify.groovy new file mode 100644 index 0000000000..ab001bf4cc --- /dev/null +++ b/src/it/it-update-properties-issue-582/verify.groovy @@ -0,0 +1,3 @@ +pom = new File( basedir, "pom.xml" ).text; + +assert pom =~ /3.0<\/api>/ diff --git a/src/main/java/org/codehaus/mojo/versions/api/PomHelper.java b/src/main/java/org/codehaus/mojo/versions/api/PomHelper.java index ab727488a6..3cf9ed371a 100644 --- a/src/main/java/org/codehaus/mojo/versions/api/PomHelper.java +++ b/src/main/java/org/codehaus/mojo/versions/api/PomHelper.java @@ -290,7 +290,7 @@ public static boolean setProjectValue( final ModifiedPomXMLEventReader pom, Stri * * @param pom The pom. * @return the project version or null if the project version is not defined (i.e. inherited from - * parent version). + * parent version). * @throws XMLStreamException if something went wrong. */ public static String getProjectVersion( final ModifiedPomXMLEventReader pom ) @@ -442,7 +442,7 @@ else if ( "version".equals( elementName ) ) return null; } return helper.createDependencyArtifact( groupId, artifactId, VersionRange.createFromVersion( version ), "pom", - null, null, false ); + null, null, false ); } /** @@ -467,8 +467,8 @@ public static boolean setDependencyVersion( final ModifiedPomXMLEventReader pom, Set implicitPaths = new HashSet<>( Arrays.asList( "/project/parent/groupId", "/project/parent/artifactId", - "/project/parent/version", "/project/groupId", - "/project/artifactId", "/project/version" ) ); + "/project/parent/version", "/project/groupId", + "/project/artifactId", "/project/version" ) ); Map implicitProperties = new HashMap<>(); for ( Map.Entry entry : model.getProperties().entrySet() ) @@ -842,7 +842,7 @@ else if ( "version".equals( elementName ) ) } else if ( matchScopeRegex.matcher( path ).matches() ) { - if ( inMatchScope && pom.hasMark( 0 ) && pom.hasMark( 1 ) && ( haveGroupId || !needGroupId ) + if ( inMatchScope && pom.hasMark( 0 ) && pom.hasMark( 1 ) && (haveGroupId || !needGroupId) && haveArtifactId && haveOldVersion ) { pom.replaceBetween( 0, 1, newVersion ); @@ -895,7 +895,7 @@ public static PropertyVersionsBuilder[] getPropertyVersionsBuilders( VersionsHel if ( profile.getDependencyManagement() != null ) { addDependencyAssocations( helper, expressionEvaluator, result, - profile.getDependencyManagement().getDependencies(), false ); + profile.getDependencyManagement().getDependencies(), false ); } addDependencyAssocations( helper, expressionEvaluator, result, profile.getDependencies(), false ); if ( profile.getBuild() != null ) @@ -903,7 +903,7 @@ public static PropertyVersionsBuilder[] getPropertyVersionsBuilders( VersionsHel if ( profile.getBuild().getPluginManagement() != null ) { addPluginAssociations( helper, expressionEvaluator, result, - profile.getBuild().getPluginManagement().getPlugins() ); + profile.getBuild().getPluginManagement().getPlugins() ); } addPluginAssociations( helper, expressionEvaluator, result, profile.getBuild().getPlugins() ); } @@ -915,51 +915,61 @@ public static PropertyVersionsBuilder[] getPropertyVersionsBuilders( VersionsHel // second, we add all the properties in the pom addProperties( helper, result, null, model.getProperties() ); - if ( model.getDependencyManagement() != null ) + Model currentModel = model; + MavenProject currentPrj = project; + while ( currentPrj != null ) { - addDependencyAssocations( helper, expressionEvaluator, result, - model.getDependencyManagement().getDependencies(), false ); - } - addDependencyAssocations( helper, expressionEvaluator, result, model.getDependencies(), false ); - if ( model.getBuild() != null ) - { - if ( model.getBuild().getPluginManagement() != null ) + if ( currentModel.getDependencyManagement() != null ) { - addPluginAssociations( helper, expressionEvaluator, result, - model.getBuild().getPluginManagement().getPlugins() ); + addDependencyAssocations( helper, expressionEvaluator, result, + currentModel.getDependencyManagement().getDependencies(), false ); } - addPluginAssociations( helper, expressionEvaluator, result, model.getBuild().getPlugins() ); - } - if ( model.getReporting() != null ) - { - addReportPluginAssociations( helper, expressionEvaluator, result, model.getReporting().getPlugins() ); - } - - // third, we add any associations from the active profiles - for ( Profile profile : model.getProfiles() ) - { - if ( !activeProfiles.contains( profile.getId() ) ) + addDependencyAssocations( helper, expressionEvaluator, result, currentModel.getDependencies(), false ); + if ( currentModel.getBuild() != null ) { - continue; + if ( currentModel.getBuild().getPluginManagement() != null ) + { + addPluginAssociations( helper, expressionEvaluator, result, + currentModel.getBuild().getPluginManagement().getPlugins() ); + } + addPluginAssociations( helper, expressionEvaluator, result, currentModel.getBuild().getPlugins() ); } - if ( profile.getDependencyManagement() != null ) + if ( currentModel.getReporting() != null ) { - addDependencyAssocations( helper, expressionEvaluator, result, - profile.getDependencyManagement().getDependencies(), false ); + addReportPluginAssociations( helper, expressionEvaluator, result, currentModel.getReporting().getPlugins() ); } - addDependencyAssocations( helper, expressionEvaluator, result, profile.getDependencies(), false ); - if ( profile.getBuild() != null ) + + // third, we add any associations from the active profiles + for ( Profile profile : currentModel.getProfiles() ) { - if ( profile.getBuild().getPluginManagement() != null ) + if ( !activeProfiles.contains( profile.getId() ) ) { - addPluginAssociations( helper, expressionEvaluator, result, - profile.getBuild().getPluginManagement().getPlugins() ); + continue; + } + if ( profile.getDependencyManagement() != null ) + { + addDependencyAssocations( helper, expressionEvaluator, result, + profile.getDependencyManagement().getDependencies(), false ); + } + addDependencyAssocations( helper, expressionEvaluator, result, profile.getDependencies(), false ); + if ( profile.getBuild() != null ) + { + if ( profile.getBuild().getPluginManagement() != null ) + { + addPluginAssociations( helper, expressionEvaluator, result, + profile.getBuild().getPluginManagement().getPlugins() ); + } + addPluginAssociations( helper, expressionEvaluator, result, profile.getBuild().getPlugins() ); + } + if ( profile.getReporting() != null ) + { + addReportPluginAssociations( helper, expressionEvaluator, result, profile.getReporting().getPlugins() ); } - addPluginAssociations( helper, expressionEvaluator, result, profile.getBuild().getPlugins() ); } - if ( profile.getReporting() != null ) + currentPrj = currentPrj.getParent(); + if ( currentPrj != null ) { - addReportPluginAssociations( helper, expressionEvaluator, result, profile.getReporting().getPlugins() ); + currentModel = currentPrj.getOriginalModel(); } } @@ -1023,7 +1033,7 @@ private static void addPluginAssociations( VersionsHelper helper, ExpressionEval VersionRange versionRange = VersionRange.createFromVersion( (String) expressionEvaluator.evaluate( plugin.getVersion() ) ); property.addAssociation( helper.createPluginArtifact( groupId, artifactId, versionRange ), - true ); + true ); if ( !propertyRef.equals( version ) ) { addBounds( property, version, propertyRef, versionRange.toString() ); @@ -1080,7 +1090,7 @@ private static void addReportPluginAssociations( VersionsHelper helper, Expressi VersionRange versionRange = VersionRange.createFromVersion( (String) expressionEvaluator.evaluate( plugin.getVersion() ) ); property.addAssociation( helper.createPluginArtifact( groupId, artifactId, versionRange ), - true ); + true ); if ( !propertyRef.equals( version ) ) { addBounds( property, version, propertyRef, versionRange.toString() ); @@ -1136,11 +1146,11 @@ private static void addDependencyAssocations( VersionsHelper helper, ExpressionE VersionRange versionRange = VersionRange.createFromVersion( (String) expressionEvaluator.evaluate( dependency.getVersion() ) ); property.addAssociation( helper.createDependencyArtifact( groupId, artifactId, versionRange, - dependency.getType(), - dependency.getClassifier(), - dependency.getScope(), - dependency.isOptional() ), - usePluginRepositories ); + dependency.getType(), + dependency.getClassifier(), + dependency.getScope(), + dependency.isOptional() ), + usePluginRepositories ); if ( !propertyRef.equals( version ) ) { addBounds( property, version, propertyRef, versionRange.toString() ); @@ -1248,7 +1258,7 @@ public static void debugModules( Log logger, String message, Collection } else { - modules.forEach( module -> logger.debug( " " + module )); + modules.forEach( module -> logger.debug( " " + module ) ); } } @@ -1584,7 +1594,7 @@ public static int getReactorParentCount( Map reactor, Model model public static StringBuilder readXmlFile( File outFile ) throws IOException { - try( Reader reader = ReaderFactory.newXmlReader( outFile ) ) + try (Reader reader = ReaderFactory.newXmlReader( outFile )) { return new StringBuilder( IOUtil.toString( reader ) ); } @@ -1623,7 +1633,7 @@ public static List readImportedPOMsFromDependencyManagementSection( String scopeElement = "scope"; Set recognizedElements = new HashSet<>( Arrays.asList( groupIdElement, artifactIdElement, versionElement, typeElement, - scopeElement ) ); + scopeElement ) ); Map depData = new HashMap<>(); pom.rewind();