[
https://issues.apache.org/jira/browse/MNG-7629?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17677200#comment-17677200
]
ASF GitHub Bot commented on MNG-7629:
-------------------------------------
mthmulders commented on code in PR #954:
URL: https://github.com/apache/maven/pull/954#discussion_r1070879137
##########
maven-core/src/main/java/org/apache/maven/ReactorReader.java:
##########
@@ -324,4 +300,113 @@ private static boolean isTestArtifact(Artifact artifact) {
return ("test-jar".equals(artifact.getProperty("type", "")))
|| ("jar".equals(artifact.getExtension()) &&
"tests".equals(artifact.getClassifier()));
}
+
+ private File find(Artifact artifact) {
+ Path target = getArtifactPath(artifact);
+ return Files.isRegularFile(target) ? target.toFile() : null;
+ }
+
+ public void processProject(MavenProject project) {
+ List<Artifact> artifacts = new ArrayList<>();
+
+ 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));
+ }
+ for (org.apache.maven.artifact.Artifact attached :
project.getAttachedArtifacts()) {
+ artifacts.add(RepositoryUtils.toArtifact(attached));
+ }
+
+ for (Artifact artifact : artifacts) {
+ if (artifact.getFile() != null && artifact.getFile().isFile()) {
+ Path target = getArtifactPath(artifact);
+ try {
+ LOGGER.debug("Copying {} to project local repository",
artifact);
+ Files.createDirectories(target.getParent());
+ Files.copy(
+ artifact.getFile().toPath(),
+ target,
+ StandardCopyOption.REPLACE_EXISTING,
+ StandardCopyOption.COPY_ATTRIBUTES);
+ } catch (IOException e) {
+ LOGGER.warn("Error while copying artifact to project local
repository", e);
+ }
+ }
+ }
+ }
+
+ private Path getArtifactPath(Artifact artifact) {
+ String groupId = artifact.getGroupId();
+ String artifactId = artifact.getArtifactId();
+ String version = artifact.getBaseVersion();
+ String classifier = artifact.getClassifier();
+ String extension = artifact.getExtension();
+ Path repo = getProjectLocalRepo();
+ return repo.resolve(groupId)
+ .resolve(artifactId)
+ .resolve(artifactId
+ + "-" + version
+ + (classifier != null && !classifier.isEmpty() ? "-" +
classifier : "")
+ + (extension != null && !extension.isEmpty() ? "." +
extension : ""));
+ }
+
+ private Path getProjectLocalRepo() {
+ Path root =
session.getRequest().getMultiModuleProjectDirectory().toPath();
+ return root.resolve("target").resolve("project-local-repo");
+ }
+
+ private MavenProject getProject(Artifact artifact) {
+ return getProjects()
+ .getOrDefault(artifact.getGroupId(), Collections.emptyMap())
+ .getOrDefault(artifact.getArtifactId(), Collections.emptyMap())
+ .getOrDefault(artifact.getBaseVersion(), null);
+ }
+
+ private Map<String, Map<String, Map<String, MavenProject>>> getProjects() {
+ // compute the projects mapping
+ if (projects == null) {
+ List<MavenProject> allProjects = session.getAllProjects();
+ if (allProjects != null) {
+ Map<String, Map<String, Map<String, MavenProject>>> map = new
HashMap<>();
Review Comment:
I'd appreciate if you could add a comment explaining what all the string
values mean. Something like
```java
// groupId -> (artifactId -> (version -> project)))
```
Just so that we poor humans have to parse all the code below in order to
understand the data structure ;-)
##########
maven-core/src/main/java/org/apache/maven/ReactorReader.java:
##########
@@ -20,41 +20,33 @@
import javax.inject.Inject;
import javax.inject.Named;
+import javax.inject.Singleton;
import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.HashSet;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Map;
-import java.util.Objects;
-import java.util.Optional;
-import java.util.function.Function;
+import java.nio.file.StandardCopyOption;
+import java.util.*;
Review Comment:
Did you deliberately replace with a wildcard import? Personally, I'm not a
big fan of them, but I don't know if we have a (formal) code style that says we
can or should not use it.
##########
maven-core/src/main/java/org/apache/maven/ReactorReader.java:
##########
@@ -324,4 +300,113 @@ private static boolean isTestArtifact(Artifact artifact) {
return ("test-jar".equals(artifact.getProperty("type", "")))
|| ("jar".equals(artifact.getExtension()) &&
"tests".equals(artifact.getClassifier()));
}
+
+ private File find(Artifact artifact) {
+ Path target = getArtifactPath(artifact);
+ return Files.isRegularFile(target) ? target.toFile() : null;
+ }
+
+ public void processProject(MavenProject project) {
+ List<Artifact> artifacts = new ArrayList<>();
+
+ 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));
+ }
+ for (org.apache.maven.artifact.Artifact attached :
project.getAttachedArtifacts()) {
+ artifacts.add(RepositoryUtils.toArtifact(attached));
+ }
+
+ for (Artifact artifact : artifacts) {
+ if (artifact.getFile() != null && artifact.getFile().isFile()) {
+ Path target = getArtifactPath(artifact);
+ try {
+ LOGGER.debug("Copying {} to project local repository",
artifact);
+ Files.createDirectories(target.getParent());
+ Files.copy(
+ artifact.getFile().toPath(),
+ target,
+ StandardCopyOption.REPLACE_EXISTING,
+ StandardCopyOption.COPY_ATTRIBUTES);
+ } catch (IOException e) {
+ LOGGER.warn("Error while copying artifact to project local
repository", e);
Review Comment:
Shouldn't this be an error? If copying doesn't succeed, the build might well
fail further down the line.
##########
maven-core/src/main/java/org/apache/maven/ReactorReader.java:
##########
@@ -324,4 +300,113 @@ private static boolean isTestArtifact(Artifact artifact) {
return ("test-jar".equals(artifact.getProperty("type", "")))
|| ("jar".equals(artifact.getExtension()) &&
"tests".equals(artifact.getClassifier()));
}
+
+ private File find(Artifact artifact) {
+ Path target = getArtifactPath(artifact);
+ return Files.isRegularFile(target) ? target.toFile() : null;
+ }
+
+ public void processProject(MavenProject project) {
+ List<Artifact> artifacts = new ArrayList<>();
+
+ 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));
+ }
+ for (org.apache.maven.artifact.Artifact attached :
project.getAttachedArtifacts()) {
+ artifacts.add(RepositoryUtils.toArtifact(attached));
+ }
+
+ for (Artifact artifact : artifacts) {
+ if (artifact.getFile() != null && artifact.getFile().isFile()) {
+ Path target = getArtifactPath(artifact);
+ try {
+ LOGGER.debug("Copying {} to project local repository",
artifact);
Review Comment:
I'm a bit in doubt here - should we log at debug or at info? It might be of
interest to the user, after all?
##########
maven-core/src/main/java/org/apache/maven/ReactorReader.java:
##########
@@ -324,4 +300,113 @@ private static boolean isTestArtifact(Artifact artifact) {
return ("test-jar".equals(artifact.getProperty("type", "")))
|| ("jar".equals(artifact.getExtension()) &&
"tests".equals(artifact.getClassifier()));
}
+
+ private File find(Artifact artifact) {
+ Path target = getArtifactPath(artifact);
+ return Files.isRegularFile(target) ? target.toFile() : null;
+ }
+
+ public void processProject(MavenProject project) {
+ List<Artifact> artifacts = new ArrayList<>();
+
+ 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));
+ }
+ for (org.apache.maven.artifact.Artifact attached :
project.getAttachedArtifacts()) {
+ artifacts.add(RepositoryUtils.toArtifact(attached));
+ }
+
+ for (Artifact artifact : artifacts) {
+ if (artifact.getFile() != null && artifact.getFile().isFile()) {
+ Path target = getArtifactPath(artifact);
+ try {
+ LOGGER.debug("Copying {} to project local repository",
artifact);
+ Files.createDirectories(target.getParent());
+ Files.copy(
+ artifact.getFile().toPath(),
+ target,
+ StandardCopyOption.REPLACE_EXISTING,
+ StandardCopyOption.COPY_ATTRIBUTES);
+ } catch (IOException e) {
+ LOGGER.warn("Error while copying artifact to project local
repository", e);
+ }
+ }
+ }
+ }
+
+ private Path getArtifactPath(Artifact artifact) {
+ String groupId = artifact.getGroupId();
+ String artifactId = artifact.getArtifactId();
+ String version = artifact.getBaseVersion();
+ String classifier = artifact.getClassifier();
+ String extension = artifact.getExtension();
+ Path repo = getProjectLocalRepo();
+ return repo.resolve(groupId)
+ .resolve(artifactId)
+ .resolve(artifactId
+ + "-" + version
+ + (classifier != null && !classifier.isEmpty() ? "-" +
classifier : "")
+ + (extension != null && !extension.isEmpty() ? "." +
extension : ""));
Review Comment:
Is it actually possible for `extension` to be `null`?
##########
maven-core/src/main/java/org/apache/maven/ReactorReader.java:
##########
@@ -324,4 +300,113 @@ private static boolean isTestArtifact(Artifact artifact) {
return ("test-jar".equals(artifact.getProperty("type", "")))
|| ("jar".equals(artifact.getExtension()) &&
"tests".equals(artifact.getClassifier()));
}
+
+ private File find(Artifact artifact) {
+ Path target = getArtifactPath(artifact);
+ return Files.isRegularFile(target) ? target.toFile() : null;
+ }
+
+ public void processProject(MavenProject project) {
Review Comment:
I think `processProject` is a bit of a vague name. As far as I can see, the
method name is not part of a generic interface - so why not call it something
like `installIntoProjectLocalRepository()`?
##########
maven-core/src/main/java/org/apache/maven/ReactorReader.java:
##########
@@ -237,21 +223,11 @@ private boolean isPackagedArtifactUpToDate(MavenProject
project, File packagedAr
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;
+ LOGGER.warn(
+ "File '{}' is more recent than the packaged
artifact for '{}', please run a full `mvn verify` build",
Review Comment:
Should this suggestion read `verify` or `package`?
> Improve resolution of modules within a multi-module build
> ---------------------------------------------------------
>
> Key: MNG-7629
> URL: https://issues.apache.org/jira/browse/MNG-7629
> Project: Maven
> Issue Type: Task
> Reporter: Guillaume Nodet
> Assignee: Guillaume Nodet
> Priority: Major
> Fix For: 4.0.0-alpha-4
>
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
