You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/12/18 19:55:26 UTC

[GitHub] [maven] mthmulders commented on a diff in pull request #912: [MNG-7629] Fix resolution of reactor dependencies

mthmulders commented on code in PR #912:
URL: https://github.com/apache/maven/pull/912#discussion_r1051655836


##########
maven-core/src/main/java/org/apache/maven/ReactorReader.java:
##########
@@ -64,28 +63,21 @@
 class ReactorReader implements MavenWorkspaceReader {
     public static final String HINT = "reactor";
 
-    private static final Collection<String> COMPILE_PHASE_TYPES =
-            Arrays.asList("jar", "ejb-client", "war", "rar", "ejb3", "par", "sar", "wsr", "har", "app-client");
-
     private static final Logger LOGGER = LoggerFactory.getLogger(ReactorReader.class);
 
     private final MavenSession session;
     private final Map<String, MavenProject> projectsByGAV;
     private final Map<String, List<MavenProject>> projectsByGA;
     private final WorkspaceRepository repository;
 
-    private Function<MavenProject, String> projectIntoKey =
-            s -> ArtifactUtils.key(s.getGroupId(), s.getArtifactId(), s.getVersion());
-
-    private Function<MavenProject, String> projectIntoVersionlessKey =
-            s -> ArtifactUtils.versionlessKey(s.getGroupId(), s.getArtifactId());
-
     @Inject
     ReactorReader(MavenSession session) {
         this.session = session;
-        this.projectsByGAV = session.getAllProjects().stream().collect(toMap(projectIntoKey, identity()));
+        this.projectsByGAV = session.getAllProjects().stream()

Review Comment:
   I don't see how this is necessary for this issue, but I think it is equivalent from a technical point of view.



##########
maven-core/src/main/java/org/apache/maven/ReactorReader.java:
##########
@@ -139,187 +132,75 @@ private File find(MavenProject project, Artifact artifact) {
             return project.getFile();
         }
 
-        Artifact projectArtifact = findMatchingArtifact(project, artifact);
-        File packagedArtifactFile = determinePreviouslyPackagedArtifactFile(project, projectArtifact);
-
-        if (hasArtifactFileFromPackagePhase(projectArtifact)) {
-            return projectArtifact.getFile();
-        }
-        // Check whether an earlier Maven run might have produced an artifact that is still on disk.
-        else if (packagedArtifactFile != null
-                && packagedArtifactFile.exists()
-                && isPackagedArtifactUpToDate(project, packagedArtifactFile, artifact)) {
-            return packagedArtifactFile;
-        } else if (!hasBeenPackagedDuringThisSession(project)) {
-            // fallback to loose class files only if artifacts haven't been packaged yet
-            // and only for plain old jars. Not war files, not ear files, not anything else.
-            return determineBuildOutputDirectoryForArtifact(project, artifact);
-        }
-
-        // The fall-through indicates that the artifact cannot be found;
-        // for instance if package produced nothing or classifier problems.
-        return null;
+        Path target = getArtifactPath(artifact);
+        return Files.isRegularFile(target) ? target.toFile() : null;
     }
 
-    private File determineBuildOutputDirectoryForArtifact(final MavenProject project, final Artifact artifact) {
-        if (isTestArtifact(artifact)) {
-            if (project.hasLifecyclePhase("test-compile")) {
-                return new File(project.getBuild().getTestOutputDirectory());
-            }
-        } else {
-            String type = artifact.getProperty("type", "");
-            File outputDirectory = new File(project.getBuild().getOutputDirectory());
+    public void processProject(MavenProject project) {
+        List<Artifact> artifacts = new ArrayList<>();
 
-            // Check if the project is being built during this session, and if we can expect any output.
-            // There is no need to check if the build has created any outputs, see MNG-2222.
-            boolean projectCompiledDuringThisSession =
-                    project.hasLifecyclePhase("compile") && COMPILE_PHASE_TYPES.contains(type);
-
-            // Check if the project is part of the session (not filtered by -pl, -rf, etc). If so, we check
-            // if a possible earlier Maven invocation produced some output for that project which we can use.
-            boolean projectHasOutputFromPreviousSession =
-                    !session.getProjects().contains(project) && outputDirectory.exists();
-
-            if (projectHasOutputFromPreviousSession || projectCompiledDuringThisSession) {
-                return outputDirectory;
-            }
+        artifacts.add(RepositoryUtils.toArtifact(new ProjectArtifact(project)));
+        if (!"pom".equals(project.getPackaging())) {
+            org.apache.maven.artifact.Artifact mavenMainArtifact = project.getArtifact();
+            artifacts.add(RepositoryUtils.toArtifact(mavenMainArtifact));
         }
-
-        // The fall-through indicates that the artifact cannot be found;
-        // for instance if package produced nothing or classifier problems.
-        return null;
-    }
-
-    private File determinePreviouslyPackagedArtifactFile(MavenProject project, Artifact artifact) {
-        if (artifact == null) {
-            return null;
+        for (org.apache.maven.artifact.Artifact attached : project.getAttachedArtifacts()) {
+            artifacts.add(RepositoryUtils.toArtifact(attached));
         }
 
-        String fileName = String.format("%s.%s", project.getBuild().getFinalName(), artifact.getExtension());
-        return new File(project.getBuild().getDirectory(), fileName);
-    }
-
-    private boolean hasArtifactFileFromPackagePhase(Artifact projectArtifact) {
-        return projectArtifact != null
-                && projectArtifact.getFile() != null
-                && projectArtifact.getFile().exists();
-    }
-
-    private boolean isPackagedArtifactUpToDate(MavenProject project, File packagedArtifactFile, Artifact artifact) {
-        Path outputDirectory = Paths.get(project.getBuild().getOutputDirectory());
-        if (!outputDirectory.toFile().exists()) {
-            return true;
-        }
-
-        try (Stream<Path> outputFiles = Files.walk(outputDirectory)) {
-            // Not using File#lastModified() to avoid a Linux JDK8 milliseconds precision bug: JDK-8177809.
-            long artifactLastModified =
-                    Files.getLastModifiedTime(packagedArtifactFile.toPath()).toMillis();
-
-            if (session.getProjectBuildingRequest().getBuildStartTime() != null) {
-                long buildStartTime =
-                        session.getProjectBuildingRequest().getBuildStartTime().getTime();
-                if (artifactLastModified > buildStartTime) {
-                    return true;
-                }
-            }
-
-            Iterator<Path> iterator = outputFiles.iterator();
-            while (iterator.hasNext()) {
-                Path outputFile = iterator.next();
-
-                if (Files.isDirectory(outputFile)) {
-                    continue;
-                }
-
-                long outputFileLastModified =
-                        Files.getLastModifiedTime(outputFile).toMillis();
-                if (outputFileLastModified > artifactLastModified) {
-                    File alternative = determineBuildOutputDirectoryForArtifact(project, artifact);
-                    if (alternative != null) {
-                        LOGGER.warn(
-                                "File '{}' is more recent than the packaged artifact for '{}'; using '{}' instead",
-                                relativizeOutputFile(outputFile),
-                                project.getArtifactId(),
-                                relativizeOutputFile(alternative.toPath()));
-                    } else {
-                        LOGGER.warn(
-                                "File '{}' is more recent than the packaged artifact for '{}'; "
-                                        + "cannot use the build output directory for this type of artifact",
-                                relativizeOutputFile(outputFile),
-                                project.getArtifactId());
-                    }
-                    return false;
+        for (Artifact artifact : artifacts) {
+            if (artifact.getFile() != null && artifact.getFile().isFile()) {
+                Path target = getArtifactPath(artifact);
+                try {
+                    Files.createDirectories(target.getParent());
+                    Files.copy(artifact.getFile().toPath(), target, StandardCopyOption.REPLACE_EXISTING);
+                } catch (IOException e) {
+                    LOGGER.warn("Error while copying artifact to project local repository", e);
                 }
             }
-
-            return true;
-        } catch (IOException e) {
-            LOGGER.warn(
-                    "An I/O error occurred while checking if the packaged artifact is up-to-date "
-                            + "against the build output directory. "
-                            + "Continuing with the assumption that it is up-to-date.",
-                    e);
-            return true;
         }
     }
 
-    private boolean hasBeenPackagedDuringThisSession(MavenProject project) {
-        return project.hasLifecyclePhase("package")
-                || project.hasLifecyclePhase("install")
-                || project.hasLifecyclePhase("deploy");
+    private Path getArtifactPath(Artifact artifact) {
+        String groupId = artifact.getGroupId();
+        String artifactId = artifact.getArtifactId();
+        String classifier = artifact.getClassifier();
+        String extension = artifact.getExtension();
+        Path root = session.getRequest().getMultiModuleProjectDirectory().toPath();
+        Path repo = root.resolve("target/project-local-repo");

Review Comment:
   Will this work as expected on operating systems that have the `\` as the file separator (e.g., Windows)?



##########
maven-core/src/main/java/org/apache/maven/ReactorReader.java:
##########
@@ -139,187 +132,75 @@ private File find(MavenProject project, Artifact artifact) {
             return project.getFile();
         }
 
-        Artifact projectArtifact = findMatchingArtifact(project, artifact);
-        File packagedArtifactFile = determinePreviouslyPackagedArtifactFile(project, projectArtifact);
-
-        if (hasArtifactFileFromPackagePhase(projectArtifact)) {
-            return projectArtifact.getFile();
-        }
-        // Check whether an earlier Maven run might have produced an artifact that is still on disk.
-        else if (packagedArtifactFile != null
-                && packagedArtifactFile.exists()
-                && isPackagedArtifactUpToDate(project, packagedArtifactFile, artifact)) {
-            return packagedArtifactFile;
-        } else if (!hasBeenPackagedDuringThisSession(project)) {
-            // fallback to loose class files only if artifacts haven't been packaged yet
-            // and only for plain old jars. Not war files, not ear files, not anything else.
-            return determineBuildOutputDirectoryForArtifact(project, artifact);
-        }
-
-        // The fall-through indicates that the artifact cannot be found;
-        // for instance if package produced nothing or classifier problems.
-        return null;
+        Path target = getArtifactPath(artifact);
+        return Files.isRegularFile(target) ? target.toFile() : null;
     }
 
-    private File determineBuildOutputDirectoryForArtifact(final MavenProject project, final Artifact artifact) {
-        if (isTestArtifact(artifact)) {
-            if (project.hasLifecyclePhase("test-compile")) {
-                return new File(project.getBuild().getTestOutputDirectory());
-            }
-        } else {
-            String type = artifact.getProperty("type", "");
-            File outputDirectory = new File(project.getBuild().getOutputDirectory());
+    public void processProject(MavenProject project) {
+        List<Artifact> artifacts = new ArrayList<>();
 
-            // Check if the project is being built during this session, and if we can expect any output.
-            // There is no need to check if the build has created any outputs, see MNG-2222.
-            boolean projectCompiledDuringThisSession =
-                    project.hasLifecyclePhase("compile") && COMPILE_PHASE_TYPES.contains(type);
-
-            // Check if the project is part of the session (not filtered by -pl, -rf, etc). If so, we check
-            // if a possible earlier Maven invocation produced some output for that project which we can use.
-            boolean projectHasOutputFromPreviousSession =
-                    !session.getProjects().contains(project) && outputDirectory.exists();
-
-            if (projectHasOutputFromPreviousSession || projectCompiledDuringThisSession) {
-                return outputDirectory;
-            }
+        artifacts.add(RepositoryUtils.toArtifact(new ProjectArtifact(project)));
+        if (!"pom".equals(project.getPackaging())) {
+            org.apache.maven.artifact.Artifact mavenMainArtifact = project.getArtifact();
+            artifacts.add(RepositoryUtils.toArtifact(mavenMainArtifact));
         }
-
-        // The fall-through indicates that the artifact cannot be found;
-        // for instance if package produced nothing or classifier problems.
-        return null;
-    }
-
-    private File determinePreviouslyPackagedArtifactFile(MavenProject project, Artifact artifact) {
-        if (artifact == null) {
-            return null;
+        for (org.apache.maven.artifact.Artifact attached : project.getAttachedArtifacts()) {
+            artifacts.add(RepositoryUtils.toArtifact(attached));
         }
 
-        String fileName = String.format("%s.%s", project.getBuild().getFinalName(), artifact.getExtension());
-        return new File(project.getBuild().getDirectory(), fileName);
-    }
-
-    private boolean hasArtifactFileFromPackagePhase(Artifact projectArtifact) {
-        return projectArtifact != null
-                && projectArtifact.getFile() != null
-                && projectArtifact.getFile().exists();
-    }
-
-    private boolean isPackagedArtifactUpToDate(MavenProject project, File packagedArtifactFile, Artifact artifact) {
-        Path outputDirectory = Paths.get(project.getBuild().getOutputDirectory());
-        if (!outputDirectory.toFile().exists()) {
-            return true;
-        }
-
-        try (Stream<Path> outputFiles = Files.walk(outputDirectory)) {
-            // Not using File#lastModified() to avoid a Linux JDK8 milliseconds precision bug: JDK-8177809.
-            long artifactLastModified =
-                    Files.getLastModifiedTime(packagedArtifactFile.toPath()).toMillis();
-
-            if (session.getProjectBuildingRequest().getBuildStartTime() != null) {
-                long buildStartTime =
-                        session.getProjectBuildingRequest().getBuildStartTime().getTime();
-                if (artifactLastModified > buildStartTime) {
-                    return true;
-                }
-            }
-
-            Iterator<Path> iterator = outputFiles.iterator();
-            while (iterator.hasNext()) {
-                Path outputFile = iterator.next();
-
-                if (Files.isDirectory(outputFile)) {
-                    continue;
-                }
-
-                long outputFileLastModified =
-                        Files.getLastModifiedTime(outputFile).toMillis();
-                if (outputFileLastModified > artifactLastModified) {
-                    File alternative = determineBuildOutputDirectoryForArtifact(project, artifact);
-                    if (alternative != null) {
-                        LOGGER.warn(
-                                "File '{}' is more recent than the packaged artifact for '{}'; using '{}' instead",
-                                relativizeOutputFile(outputFile),
-                                project.getArtifactId(),
-                                relativizeOutputFile(alternative.toPath()));
-                    } else {
-                        LOGGER.warn(
-                                "File '{}' is more recent than the packaged artifact for '{}'; "
-                                        + "cannot use the build output directory for this type of artifact",
-                                relativizeOutputFile(outputFile),
-                                project.getArtifactId());
-                    }
-                    return false;
+        for (Artifact artifact : artifacts) {
+            if (artifact.getFile() != null && artifact.getFile().isFile()) {
+                Path target = getArtifactPath(artifact);
+                try {
+                    Files.createDirectories(target.getParent());
+                    Files.copy(artifact.getFile().toPath(), target, StandardCopyOption.REPLACE_EXISTING);

Review Comment:
   Would it be worth writing a log message to signal which file is copied to where?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org