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`?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
