diff --git a/versions-common/src/main/java/org/codehaus/mojo/versions/api/PomHelper.java b/versions-common/src/main/java/org/codehaus/mojo/versions/api/PomHelper.java index 9583bf28c..7feaf4263 100644 --- a/versions-common/src/main/java/org/codehaus/mojo/versions/api/PomHelper.java +++ b/versions-common/src/main/java/org/codehaus/mojo/versions/api/PomHelper.java @@ -106,7 +106,9 @@ public static Model getRawModel(MavenProject project) throws IOException { public static Model getRawModel(File moduleProjectFile) throws IOException { try (Reader reader = new BufferedReader(new InputStreamReader(Files.newInputStream(moduleProjectFile.toPath())))) { - return getRawModel(reader); + Model result = getRawModel(reader); + result.setPomFile(moduleProjectFile); + return result; } } @@ -1202,36 +1204,6 @@ public static void debugModules(Log logger, String message, Collection m } } - /** - * Modifies the collection of child modules removing those which cannot be found relative to the parent. - * - * @param logger The logger to log to. - * @param basedir the project basedir. - * @param childModules the child modules. - */ - public static void removeMissingChildModules(Log logger, File basedir, Collection childModules) { - logger.debug("Removing child modules which are missing..."); - Iterator i = childModules.iterator(); - while (i.hasNext()) { - String modulePath = i.next(); - File moduleFile = new File(basedir, modulePath); - - if (moduleFile.isDirectory() && new File(moduleFile, "pom.xml").isFile()) { - // it's a directory that exists - continue; - } - - if (moduleFile.isFile()) { - // it's the pom.xml file directly referenced and it exists. - continue; - } - - logger.debug("Removing missing child module " + modulePath); - i.remove(); - } - debugModules(logger, "After removing missing", childModules); - } - /** * Extracts the version from a raw model, interpolating from the parent if necessary. * @@ -1362,50 +1334,41 @@ public static ProjectBuildingRequest createProjectBuilderRequest( * @return A map of raw models keyed by path relative to the project's basedir. * @throws IOException if things go wrong. */ - public static Map getReactorModels(MavenProject project, Log logger) throws IOException { - Map result = new LinkedHashMap<>(); + public static Map getChildModels(MavenProject project, Log logger) throws IOException { + Map result = new LinkedHashMap<>(); final Model model = getRawModel(project); - final String path = ""; - result.put(path, model); - result.putAll(getReactorModels(path, model, project, logger)); + result.put(project.getFile(), model); + result.putAll(getChildModels(model, logger)); return result; } /** * Builds a sub-map of raw models keyed by module path. * - * @param path The relative path to base the sub-map on. - * @param model The model at the relative path. - * @param project The project to build from. + * @param model The root model * @param logger The logger for logging. * @return A map of raw models keyed by path relative to the project's basedir. * @throws IOException if things go wrong. */ - private static Map getReactorModels(String path, Model model, MavenProject project, Log logger) - throws IOException { - Map result = new LinkedHashMap<>(); - Map childResults = new LinkedHashMap<>(); - Set childModules = getAllChildModules(model, logger); + private static Map getChildModels(Model model, Log logger) throws IOException { + Map result = new LinkedHashMap<>(); + Map childResults = new LinkedHashMap<>(); - File baseDir = path.length() > 0 ? new File(project.getBasedir(), path) : project.getBasedir(); - removeMissingChildModules(logger, baseDir, childModules); + File baseDir = model.getPomFile().getParentFile(); - childModules.stream() + getAllChildModules(model, logger).parallelStream() .map(moduleName -> new File(baseDir, moduleName)) + .map(file -> file.isFile() ? file : new File(file, "pom.xml")) .filter(File::exists) - .forEach(moduleFile -> { - File pomFile = moduleFile.isDirectory() ? new File(moduleFile, "/pom.xml") : moduleFile; - String modulePath = (!path.isEmpty() && !path.endsWith("/") ? path + "/" : path) - + pomFile.getParentFile().getName(); - + .forEach(pomFile -> { try { // the aim of this goal is to fix problems when the project cannot be parsed by Maven, // so we have to work with the raw model and not the interpolated parsed model from maven Model moduleModel = getRawModel(pomFile); - result.put(modulePath, moduleModel); - childResults.putAll(getReactorModels(modulePath, moduleModel, project, logger)); + result.put(pomFile, moduleModel); + childResults.putAll(getChildModels(moduleModel, logger)); } catch (IOException e) { - logger.debug("Could not parse " + pomFile.getPath(), e); + logger.error("Could not parse " + pomFile.getPath(), e); } }); @@ -1421,10 +1384,10 @@ private static Map getReactorModels(String path, Model model, Mav * @param artifactId The artifactId of the parent. * @return a map of models that have a specified groupId and artifactId as parent keyed by path. */ - public static Map getChildModels(Map reactor, String groupId, String artifactId) { - final Map result = new LinkedHashMap<>(); - for (Map.Entry entry : reactor.entrySet()) { - final String path = entry.getKey(); + public static Map getChildModels(Map reactor, String groupId, String artifactId) { + final Map result = new LinkedHashMap<>(); + for (Map.Entry entry : reactor.entrySet()) { + final File path = entry.getKey(); final Model model = entry.getValue(); final Parent parent = model.getParent(); if (parent != null && groupId.equals(parent.getGroupId()) && artifactId.equals(parent.getArtifactId())) { @@ -1442,7 +1405,7 @@ public static Map getChildModels(Map reactor, Stri * @param artifactId The artifactId to match. * @return The model or null if the model was not in the reactor. */ - public static Model getModel(Map reactor, String groupId, String artifactId) { + public static Model getModel(Map reactor, String groupId, String artifactId) { return reactor.values().stream() .filter(model -> (groupId == null || groupId.equals(getGroupId(model))) && artifactId.equals(getArtifactId(model))) @@ -1459,8 +1422,7 @@ public static Model getModel(Map reactor, String groupId, String * @param artifactId The artifactId to match. * @return The model entry or null if the model was not in the reactor. */ - public static Map.Entry getModelEntry( - Map reactor, String groupId, String artifactId) { + public static Map.Entry getModelEntry(Map reactor, String groupId, String artifactId) { return reactor.entrySet().stream() .filter(e -> (groupId == null || groupId.equals(PomHelper.getGroupId(e.getValue()))) && artifactId.equals(PomHelper.getArtifactId(e.getValue()))) @@ -1475,7 +1437,7 @@ public static Map.Entry getModelEntry( * @param model The model. * @return The number of parents of this model in the reactor. */ - public static int getReactorParentCount(Map reactor, Model model) { + public static int getReactorParentCount(Map reactor, Model model) { if (model.getParent() == null) { return 0; } diff --git a/versions-common/src/main/java/org/codehaus/mojo/versions/api/VersionDetails.java b/versions-common/src/main/java/org/codehaus/mojo/versions/api/VersionDetails.java index d0365bfd8..245ebdf58 100644 --- a/versions-common/src/main/java/org/codehaus/mojo/versions/api/VersionDetails.java +++ b/versions-common/src/main/java/org/codehaus/mojo/versions/api/VersionDetails.java @@ -405,6 +405,8 @@ ArtifactVersion[] getAllUpdates(Optional updateScope, boolean includeSn * * @param scope most major segment where updates are allowed Optional.empty() for no restriction * @return {@linkplain Restriction} object based on the arguments + * @throws InvalidSegmentException if the requested segment is outside the bounds (less than 1 or greater than + * the segment count) */ Restriction restrictionFor(Optional scope) throws InvalidSegmentException; diff --git a/versions-common/src/main/java/org/codehaus/mojo/versions/api/VersionsHelper.java b/versions-common/src/main/java/org/codehaus/mojo/versions/api/VersionsHelper.java index 1004e11be..d76fe9d7b 100644 --- a/versions-common/src/main/java/org/codehaus/mojo/versions/api/VersionsHelper.java +++ b/versions-common/src/main/java/org/codehaus/mojo/versions/api/VersionsHelper.java @@ -217,6 +217,7 @@ Map lookupPluginsUpdates(Set plugins, bool * @param plugin The {@link Plugin} instance to look up. * @param allowSnapshots Include snapshots in the list of updates. * @return The plugin update details. + * @throws VersionRetrievalException thrown if version resolution fails * @since 1.0-beta-1 */ PluginUpdatesDetails lookupPluginUpdates(Plugin plugin, boolean allowSnapshots) throws VersionRetrievalException; diff --git a/versions-common/src/main/java/org/codehaus/mojo/versions/ordering/ReactorDepthComparator.java b/versions-common/src/main/java/org/codehaus/mojo/versions/ordering/ReactorDepthComparator.java index 25124511d..fd51a65cc 100644 --- a/versions-common/src/main/java/org/codehaus/mojo/versions/ordering/ReactorDepthComparator.java +++ b/versions-common/src/main/java/org/codehaus/mojo/versions/ordering/ReactorDepthComparator.java @@ -19,6 +19,7 @@ * under the License. */ +import java.io.File; import java.util.Comparator; import java.util.Map; @@ -31,14 +32,14 @@ * @author Stephen Connolly * @since 15-Sep-2010 14:54:42 */ -public class ReactorDepthComparator implements Comparator { - private final Map reactor; +public class ReactorDepthComparator implements Comparator { + private final Map reactor; - public ReactorDepthComparator(Map reactor) { + public ReactorDepthComparator(Map reactor) { this.reactor = reactor; } - public int compare(String o1, String o2) { + public int compare(File o1, File o2) { final Model m1 = reactor.get(o1); final Model m2 = reactor.get(o2); final int d1 = PomHelper.getReactorParentCount(reactor, m1); diff --git a/versions-common/src/main/java/org/codehaus/mojo/versions/utils/MavenProjectUtils.java b/versions-common/src/main/java/org/codehaus/mojo/versions/utils/MavenProjectUtils.java index e1fd64507..9832bef2b 100644 --- a/versions-common/src/main/java/org/codehaus/mojo/versions/utils/MavenProjectUtils.java +++ b/versions-common/src/main/java/org/codehaus/mojo/versions/utils/MavenProjectUtils.java @@ -75,7 +75,9 @@ public static Set extractDependenciesFromPlugins(MavenProject projec * @param processDependencyManagementTransitive if {@code true}, the original model will be considered * instead of the interpolated model, which does not contain * imported dependencies + * @param log {@link Log} instance (may not be null) * @return set of {@link Dependency} objects + * @throws VersionRetrievalException thrown if version information retrieval fails * or an empty set if none have been retrieveddependencies or an empty set if none have been retrieved */ public static Set extractDependenciesFromDependencyManagement( diff --git a/versions-common/src/test/java/org/codehaus/mojo/versions/api/PomHelperTest.java b/versions-common/src/test/java/org/codehaus/mojo/versions/api/PomHelperTest.java index 5b13ccbe2..3b6889502 100644 --- a/versions-common/src/test/java/org/codehaus/mojo/versions/api/PomHelperTest.java +++ b/versions-common/src/test/java/org/codehaus/mojo/versions/api/PomHelperTest.java @@ -25,7 +25,6 @@ import java.io.File; import java.io.StringReader; import java.net.URL; -import java.util.Map; import org.apache.maven.model.Model; import org.apache.maven.model.io.xpp3.MavenXpp3Reader; @@ -218,7 +217,6 @@ public void testSetProjectValueNewValueNonEmptyParent() throws XMLStreamExceptio public void testIssue505ChildModules() throws Exception { MavenProject project = mojoRule.readMavenProject(new File("src/test/resources/org/codehaus/mojo/versions/api/issue-505")); - Map reactorModels = PomHelper.getReactorModels(project, new SystemStreamLog()); - assertThat(reactorModels.keySet(), hasSize(3)); + assertThat(PomHelper.getChildModels(project, new SystemStreamLog()).entrySet(), hasSize(3)); } } diff --git a/versions-maven-plugin/src/it/it-set-issue-848/docs/moduleC/pom.xml b/versions-maven-plugin/src/it/it-set-issue-848/docs/moduleC/pom.xml new file mode 100644 index 000000000..ce77e2014 --- /dev/null +++ b/versions-maven-plugin/src/it/it-set-issue-848/docs/moduleC/pom.xml @@ -0,0 +1,16 @@ + + 4.0.0 + + + default-group + default-artifact + 1.0-SNAPSHOT + ../.. + + + moduleC + 1.0.0-SNAPSHOT + pom + + diff --git a/versions-maven-plugin/src/it/it-set-issue-848/invoker.properties b/versions-maven-plugin/src/it/it-set-issue-848/invoker.properties new file mode 100644 index 000000000..f21410e6a --- /dev/null +++ b/versions-maven-plugin/src/it/it-set-issue-848/invoker.properties @@ -0,0 +1,2 @@ +invoker.goals = ${project.groupId}:${project.artifactId}:${project.version}:set +invoker.mavenOpts = -DnewVersion=TEST diff --git a/versions-maven-plugin/src/it/it-set-issue-848/moduleA/moduleA1.xml b/versions-maven-plugin/src/it/it-set-issue-848/moduleA/moduleA1.xml new file mode 100644 index 000000000..f2f547048 --- /dev/null +++ b/versions-maven-plugin/src/it/it-set-issue-848/moduleA/moduleA1.xml @@ -0,0 +1,15 @@ + + 4.0.0 + + + default-group + default-artifact + 1.0-SNAPSHOT + + + moduleA1 + 1.0.0-SNAPSHOT + pom + + diff --git a/versions-maven-plugin/src/it/it-set-issue-848/moduleA/moduleA2.xml b/versions-maven-plugin/src/it/it-set-issue-848/moduleA/moduleA2.xml new file mode 100644 index 000000000..a44bebda2 --- /dev/null +++ b/versions-maven-plugin/src/it/it-set-issue-848/moduleA/moduleA2.xml @@ -0,0 +1,15 @@ + + 4.0.0 + + + default-group + default-artifact + 1.0-SNAPSHOT + + + moduleA2 + 1.0.0-SNAPSHOT + pom + + diff --git a/versions-maven-plugin/src/it/it-set-issue-848/moduleB/pom.xml b/versions-maven-plugin/src/it/it-set-issue-848/moduleB/pom.xml new file mode 100644 index 000000000..fd2328b9d --- /dev/null +++ b/versions-maven-plugin/src/it/it-set-issue-848/moduleB/pom.xml @@ -0,0 +1,15 @@ + + 4.0.0 + + + default-group + default-artifact + 1.0-SNAPSHOT + + + moduleB + 1.0.0-SNAPSHOT + pom + + diff --git a/versions-maven-plugin/src/it/it-set-issue-848/pom.xml b/versions-maven-plugin/src/it/it-set-issue-848/pom.xml new file mode 100644 index 000000000..09ed31108 --- /dev/null +++ b/versions-maven-plugin/src/it/it-set-issue-848/pom.xml @@ -0,0 +1,17 @@ + + 4.0.0 + + default-group + default-artifact + 1.0-SNAPSHOT + pom + + + moduleA/moduleA1.xml + moduleA/moduleA2.xml + moduleB/pom.xml + docs/moduleC + + + diff --git a/versions-maven-plugin/src/it/it-set-issue-848/verify.groovy b/versions-maven-plugin/src/it/it-set-issue-848/verify.groovy new file mode 100644 index 000000000..1368f7b1b --- /dev/null +++ b/versions-maven-plugin/src/it/it-set-issue-848/verify.groovy @@ -0,0 +1,5 @@ +assert new File( basedir, "pom.xml" ).text.contains( 'TEST' ) +assert new File( basedir, "moduleA/moduleA1.xml" ).text.contains( 'TEST' ) +assert new File( basedir, "moduleA/moduleA2.xml" ).text.contains( 'TEST' ) +assert new File( basedir, "moduleB/pom.xml" ).text.contains( 'TEST' ) +assert new File( basedir, "docs/moduleC/pom.xml" ).text.contains( 'TEST' ) diff --git a/versions-maven-plugin/src/main/java/org/codehaus/mojo/versions/SetMojo.java b/versions-maven-plugin/src/main/java/org/codehaus/mojo/versions/SetMojo.java index 44035838b..4cfdacae2 100644 --- a/versions-maven-plugin/src/main/java/org/codehaus/mojo/versions/SetMojo.java +++ b/versions-maven-plugin/src/main/java/org/codehaus/mojo/versions/SetMojo.java @@ -283,8 +283,7 @@ public void execute() throws MojoExecutionException, MojoFailureException { if (removeSnapshot && !nextSnapshot) { String version = getVersion(); if (version.endsWith(SNAPSHOT)) { - String release = version.substring(0, version.indexOf(SNAPSHOT)); - newVersion = release; + newVersion = version.substring(0, version.indexOf(SNAPSHOT)); getLog().info("SNAPSHOT found. BEFORE " + version + " --> AFTER: " + newVersion); } } @@ -332,8 +331,8 @@ public void execute() throws MojoExecutionException, MojoFailureException { : getProject(); getLog().info("Local aggregation root: " + project.getBasedir()); - Map reactorModels = PomHelper.getReactorModels(project, getLog()); - final SortedMap reactor = new TreeMap<>(new ReactorDepthComparator(reactorModels)); + Map reactorModels = PomHelper.getChildModels(project, getLog()); + final SortedMap reactor = new TreeMap<>(new ReactorDepthComparator(reactorModels)); reactor.putAll(reactorModels); // set of files to update @@ -371,8 +370,8 @@ public void execute() throws MojoExecutionException, MojoFailureException { reactor.values().parallelStream() .map(m -> PomHelper.getModelEntry(reactor, PomHelper.getGroupId(m), PomHelper.getArtifactId(m))) .filter(Objects::nonNull) - .map(Map.Entry::getKey) - .map(f -> getModuleProjectFile(project, f)) + .map(Map.Entry::getValue) + .map(Model::getPomFile) .forEach(files::add); } @@ -421,7 +420,7 @@ private static String fixNullOrEmpty(String value, String defaultValue) { private void applyChange( MavenProject project, - SortedMap reactor, + SortedMap reactor, Set files, String groupId, String artifactId, @@ -432,14 +431,14 @@ private void applyChange( addChange(groupId, artifactId, oldVersion, newVersion); // now fake out the triggering change - Map.Entry current = PomHelper.getModelEntry(reactor, groupId, artifactId); + Map.Entry current = PomHelper.getModelEntry(reactor, groupId, artifactId); if (current != null) { current.getValue().setVersion(newVersion); - files.add(getModuleProjectFile(project, current.getKey())); + files.add(current.getValue().getPomFile()); } - for (Map.Entry sourceEntry : reactor.entrySet()) { - final String sourcePath = sourceEntry.getKey(); + for (Map.Entry sourceEntry : reactor.entrySet()) { + final File sourcePath = sourceEntry.getKey(); final Model sourceModel = sourceEntry.getValue(); getLog().debug( @@ -463,12 +462,12 @@ private void applyChange( continue; } - files.add(getModuleProjectFile(project, sourcePath)); + files.add(sourceModel.getPomFile()); getLog().debug("Looking for modules which use " + ArtifactUtils.versionlessKey(sourceGroupId, sourceArtifactId) + " as their parent"); - for (Map.Entry stringModelEntry : processAllModules + for (Map.Entry stringModelEntry : processAllModules ? reactor.entrySet() : PomHelper.getChildModels(reactor, sourceGroupId, sourceArtifactId) .entrySet()) { @@ -514,20 +513,6 @@ private void applyChange( } } - private static File getModuleProjectFile(MavenProject project, String relativePath) { - final File moduleDir = new File(project.getBasedir(), relativePath); - final File projectBaseDir = project.getBasedir(); - - if (projectBaseDir.equals(moduleDir)) { - return project.getFile(); - } else if (moduleDir.isDirectory()) { - return new File(moduleDir, "pom.xml"); - } - // i don't think this should ever happen... but just in case - // the module references the file-name - return moduleDir; - } - /** * Updates the pom file. * diff --git a/versions-maven-plugin/src/main/java/org/codehaus/mojo/versions/UpdateChildModulesMojo.java b/versions-maven-plugin/src/main/java/org/codehaus/mojo/versions/UpdateChildModulesMojo.java index 11fda2089..55d6526c9 100644 --- a/versions-maven-plugin/src/main/java/org/codehaus/mojo/versions/UpdateChildModulesMojo.java +++ b/versions-maven-plugin/src/main/java/org/codehaus/mojo/versions/UpdateChildModulesMojo.java @@ -84,8 +84,8 @@ public void execute() throws MojoExecutionException, MojoFailureException { boolean didSomething = false; try { - final Map reactor = PomHelper.getReactorModels(getProject(), getLog()); - List order = new ArrayList<>(reactor.keySet()); + final Map reactor = PomHelper.getChildModels(getProject(), getLog()); + List order = new ArrayList<>(reactor.keySet()); order.sort((o1, o2) -> { Model m1 = reactor.get(o1); Model m2 = reactor.get(o2); @@ -99,7 +99,7 @@ public void execute() throws MojoExecutionException, MojoFailureException { return 0; }); - for (String sourcePath : order) { + for (File sourcePath : order) { Model sourceModel = reactor.get(sourcePath); getLog().debug( @@ -128,31 +128,20 @@ public void execute() throws MojoExecutionException, MojoFailureException { + ArtifactUtils.versionlessKey(sourceGroupId, sourceArtifactId) + " as their parent to update it to " + sourceVersion); - for (Map.Entry target : PomHelper.getChildModels( + for (Map.Entry target : PomHelper.getChildModels( reactor, sourceGroupId, sourceArtifactId) .entrySet()) { - String targetPath = target.getKey(); - - File moduleDir = new File(getProject().getBasedir(), targetPath); - - File moduleProjectFile; - - if (moduleDir.isDirectory()) { - moduleProjectFile = new File(moduleDir, "pom.xml"); - } else { - // i don't think this should ever happen... but just in case - // the module references the file-name - moduleProjectFile = moduleDir; - } + File moduleProjectFile = target.getKey(); + String moduleName = moduleProjectFile.getParent(); Model targetModel = target.getValue(); final Parent parent = targetModel.getParent(); if (sourceVersion.equals(parent.getVersion())) { - getLog().debug("Module: " + targetPath + " parent is " + getLog().debug("Module: " + moduleName + " parent is " + ArtifactUtils.versionlessKey(sourceGroupId, sourceArtifactId) + ":" + sourceVersion); } else { - getLog().info("Module: " + targetPath); + getLog().info("Module: " + moduleName); getLog().info(" parent was " + ArtifactUtils.versionlessKey(sourceGroupId, sourceArtifactId) + ":" + parent.getVersion());