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 2023/01/12 07:07:17 UTC
[GitHub] [maven] gnodet opened a new pull request, #954: [MNG-7629] Change reactor reader to copy packaged artifacts and reuse them across builds if needed
gnodet opened a new pull request, #954:
URL: https://github.com/apache/maven/pull/954
Following this checklist to help us incorporate your
contribution quickly and easily:
- [ ] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/MNG) filed
for the change (usually before you start working on it). Trivial changes like typos do not
require a JIRA issue. Your pull request should address just this issue, without
pulling in other changes.
- [ ] Each commit in the pull request should have a meaningful subject line and body.
- [ ] Format the pull request title like `[MNG-XXX] SUMMARY`,
where you replace `MNG-XXX` and `SUMMARY` with the appropriate JIRA issue.
- [ ] Also format the first line of the commit message like `[MNG-XXX] SUMMARY`.
Best practice is to use the JIRA issue title in both the pull request title and in the first line of the commit message.
- [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [ ] Run `mvn clean verify` to make sure basic checks pass. A more thorough check will
be performed on your pull request automatically.
- [ ] You have run the [Core IT][core-its] successfully.
If your pull request is about ~20 lines of code you don't need to sign an
[Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf) if you are unsure
please ask on the developers list.
To make clear that you license your contribution under
the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
you have to acknowledge this by using the following check-box.
- [ ] I hereby declare this contribution to be licenced under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
- [ ] In any other case, please file an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
[core-its]: https://maven.apache.org/core-its/core-it-suite/
--
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
[GitHub] [maven] cstamas commented on a diff in pull request #954: [MNG-7629] Change reactor reader to copy packaged artifacts and reuse them across builds if needed
Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #954:
URL: https://github.com/apache/maven/pull/954#discussion_r1073687908
##########
maven-core/src/main/java/org/apache/maven/ReactorReader.java:
##########
@@ -324,4 +273,185 @@ private static boolean isTestArtifact(Artifact artifact) {
return ("test-jar".equals(artifact.getProperty("type", "")))
|| ("jar".equals(artifact.getExtension()) && "tests".equals(artifact.getClassifier()));
}
+
+ private File findInProjectLocalRepository(Artifact artifact) {
+ Path target = getArtifactPath(artifact);
+ return Files.isRegularFile(target) ? target.toFile() : null;
+ }
+
+ /**
+ * Copy packaged and attached artifacts from this project to the
+ * project local repository.
+ * This allows a subsequent build to resume while still being able
+ * to locate attached artifacts.
+ *
+ * @param project the project to copy artifacts from
+ */
+ private void installIntoProjectLocalRepository(MavenProject project) {
+ if (!hasBeenPackagedDuringThisSession(project)) {
+ return;
+ }
+ getProjectArtifacts(project).filter(this::isRegularFile).forEach(this::installIntoProjectLocalRepository);
+ }
+
+ private void cleanProjectLocalRepository(MavenProject project) {
+ try {
+ Path artifactPath = getProjectLocalRepo()
+ .resolve(project.getGroupId())
+ .resolve(project.getArtifactId())
+ .resolve(project.getVersion());
+ if (Files.isDirectory(artifactPath)) {
+ try (Stream<Path> paths = Files.list(artifactPath)) {
+ for (Path path : (Iterable<Path>) paths::iterator) {
+ Files.delete(path);
+ }
+ }
+ try {
+ Files.delete(artifactPath);
+ Files.delete(artifactPath.getParent());
+ Files.delete(artifactPath.getParent().getParent());
+ } catch (DirectoryNotEmptyException e) {
+ // ignore
+ }
+ }
+ } catch (IOException e) {
+ LOGGER.error("Error while cleaning project local repository", e);
+ }
+ }
+
+ /**
+ * Retrieve a stream of the project's artifacts
+ */
+ private Stream<Artifact> getProjectArtifacts(MavenProject project) {
+ Stream<org.apache.maven.artifact.Artifact> artifacts = Stream.concat(
+ Stream.concat(
+ // pom artifact
+ Stream.of(new ProjectArtifact(project)),
+ // main project artifact if not a pom
+ "pom".equals(project.getPackaging()) ? Stream.empty() : Stream.of(project.getArtifact())),
+ // attached artifacts
+ project.getAttachedArtifacts().stream());
+ return artifacts.map(RepositoryUtils::toArtifact);
+ }
+
+ private boolean isRegularFile(Artifact artifact) {
+ return artifact.getFile() != null && artifact.getFile().isFile();
+ }
+
+ private void installIntoProjectLocalRepository(Artifact artifact) {
+ Path target = getArtifactPath(artifact);
+ try {
+ LOGGER.info("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.error("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(version)
+ .resolve(artifactId
+ + "-" + version
+ + (classifier != null && !classifier.isEmpty() ? "-" + classifier : "")
+ + "." + extension);
+ }
+
+ private Path getProjectLocalRepo() {
+ Path root = session.getRequest().getMultiModuleProjectDirectory().toPath();
Review Comment:
Cool, was unaware it is always available... and big :+1: to solve it in generic way
--
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
[GitHub] [maven] gnodet commented on a diff in pull request #954: [MNG-7629] Change reactor reader to copy packaged artifacts and reuse them across builds if needed
Posted by GitBox <gi...@apache.org>.
gnodet commented on code in PR #954:
URL: https://github.com/apache/maven/pull/954#discussion_r1071812263
##########
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:
Renamed
--
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
[GitHub] [maven] gnodet commented on pull request #954: [MNG-7629] Change reactor reader to copy packaged artifacts and reuse them across builds if needed
Posted by GitBox <gi...@apache.org>.
gnodet commented on PR #954:
URL: https://github.com/apache/maven/pull/954#issuecomment-1386130672
> > ... in order to be able to partially run mvn test.
>
> I'm not sure I understand. You mean "run `mvn test` on a multi-module project without requiring the user to have ran `mvn package` first"?
Yes. I haven't removed this part in this PR. If the main artifacts have not been packaged, they will point to the `target/classes`.
--
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
[GitHub] [maven] mthmulders commented on a diff in pull request #954: [MNG-7629] Change reactor reader to copy packaged artifacts and reuse them across builds if needed
Posted by GitBox <gi...@apache.org>.
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet commented on a diff in pull request #954: [MNG-7629] Change reactor reader to copy packaged artifacts and reuse them across builds if needed
Posted by GitBox <gi...@apache.org>.
gnodet commented on code in PR #954:
URL: https://github.com/apache/maven/pull/954#discussion_r1071616763
##########
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:
You're right, it cannot.
--
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
[GitHub] [maven] mthmulders commented on pull request #954: [MNG-7629] Change reactor reader to copy packaged artifacts and reuse them across builds if needed
Posted by GitBox <gi...@apache.org>.
mthmulders commented on PR #954:
URL: https://github.com/apache/maven/pull/954#issuecomment-1385937080
> ... in order to be able to partially run mvn test.
I'm not sure I understand. You mean "run `mvn test` on a multi-module project without requiring the user to have ran `mvn package` first"?
--
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
[GitHub] [maven] gnodet commented on pull request #954: [MNG-7629] Change reactor reader to copy packaged artifacts and reuse them across builds if needed
Posted by GitBox <gi...@apache.org>.
gnodet commented on PR #954:
URL: https://github.com/apache/maven/pull/954#issuecomment-1387205641
In the internal project local repository structure, I've added a child directory with the version of a given artifact. This gives a structure which is closer to the maven local repository, and it allows deleting artifacts during the clean phase without having to rely on a heuristic to split between classifier / version / extension.
@mthmulders @MartinKanters I'm done on this PR, so unless you have further comments, an approval before the merge would be nice !
--
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
[GitHub] [maven] gnodet commented on a diff in pull request #954: [MNG-7629] Change reactor reader to copy packaged artifacts and reuse them across builds if needed
Posted by GitBox <gi...@apache.org>.
gnodet commented on code in PR #954:
URL: https://github.com/apache/maven/pull/954#discussion_r1073701518
##########
maven-core/src/main/java/org/apache/maven/ReactorReader.java:
##########
@@ -324,4 +273,185 @@ private static boolean isTestArtifact(Artifact artifact) {
return ("test-jar".equals(artifact.getProperty("type", "")))
|| ("jar".equals(artifact.getExtension()) && "tests".equals(artifact.getClassifier()));
}
+
+ private File findInProjectLocalRepository(Artifact artifact) {
+ Path target = getArtifactPath(artifact);
+ return Files.isRegularFile(target) ? target.toFile() : null;
+ }
+
+ /**
+ * Copy packaged and attached artifacts from this project to the
+ * project local repository.
+ * This allows a subsequent build to resume while still being able
+ * to locate attached artifacts.
+ *
+ * @param project the project to copy artifacts from
+ */
+ private void installIntoProjectLocalRepository(MavenProject project) {
+ if (!hasBeenPackagedDuringThisSession(project)) {
+ return;
+ }
+ getProjectArtifacts(project).filter(this::isRegularFile).forEach(this::installIntoProjectLocalRepository);
+ }
+
+ private void cleanProjectLocalRepository(MavenProject project) {
+ try {
+ Path artifactPath = getProjectLocalRepo()
+ .resolve(project.getGroupId())
+ .resolve(project.getArtifactId())
+ .resolve(project.getVersion());
+ if (Files.isDirectory(artifactPath)) {
+ try (Stream<Path> paths = Files.list(artifactPath)) {
+ for (Path path : (Iterable<Path>) paths::iterator) {
+ Files.delete(path);
+ }
+ }
+ try {
+ Files.delete(artifactPath);
+ Files.delete(artifactPath.getParent());
+ Files.delete(artifactPath.getParent().getParent());
+ } catch (DirectoryNotEmptyException e) {
+ // ignore
+ }
+ }
+ } catch (IOException e) {
+ LOGGER.error("Error while cleaning project local repository", e);
+ }
+ }
+
+ /**
+ * Retrieve a stream of the project's artifacts
+ */
+ private Stream<Artifact> getProjectArtifacts(MavenProject project) {
+ Stream<org.apache.maven.artifact.Artifact> artifacts = Stream.concat(
+ Stream.concat(
+ // pom artifact
+ Stream.of(new ProjectArtifact(project)),
+ // main project artifact if not a pom
+ "pom".equals(project.getPackaging()) ? Stream.empty() : Stream.of(project.getArtifact())),
+ // attached artifacts
+ project.getAttachedArtifacts().stream());
+ return artifacts.map(RepositoryUtils::toArtifact);
+ }
+
+ private boolean isRegularFile(Artifact artifact) {
+ return artifact.getFile() != null && artifact.getFile().isFile();
+ }
+
+ private void installIntoProjectLocalRepository(Artifact artifact) {
+ Path target = getArtifactPath(artifact);
+ try {
+ LOGGER.info("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.error("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(version)
+ .resolve(artifactId
+ + "-" + version
+ + (classifier != null && !classifier.isEmpty() ? "-" + classifier : "")
+ + "." + extension);
+ }
+
+ private Path getProjectLocalRepo() {
+ Path root = session.getRequest().getMultiModuleProjectDirectory().toPath();
+ return root.resolve("target").resolve("project-local-repo");
Review Comment:
I could use something like the following to retrieve the build directory of the top most project if it's in the reactor:
```
private Path getProjectLocalRepo() {
File mmpd = session.getRequest().getMultiModuleProjectDirectory();
return session.getProjects().stream()
.filter(project -> mmpd.equals(project.getBasedir()))
.findFirst()
.map(project -> project.getBuild().getDirectory())
.map(Paths::get)
.orElseGet(() -> mmpd.toPath().resolve("target"))
.resolve("project-local-repo");
}
```
--
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
[GitHub] [maven] gnodet commented on a diff in pull request #954: [MNG-7629] Change reactor reader to copy packaged artifacts and reuse them across builds if needed
Posted by GitBox <gi...@apache.org>.
gnodet commented on code in PR #954:
URL: https://github.com/apache/maven/pull/954#discussion_r1071625330
##########
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:
Fixed
##########
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:
I don't think so. The copying of artifacts should not really alter the current build, as those artifacts are already packaged anyway and the original packaged artifacts should be used. It would mainly affect a subsequent build, which may fail as it can do now, so I'd think a warning is sufficient.
##########
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:
Raised to info level.
##########
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:
Fixed in the javadoc description for the `getProjects()` method and on the `projects` field.
--
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
[GitHub] [maven] gnodet commented on a diff in pull request #954: [MNG-7629] Change reactor reader to copy packaged artifacts and reuse them across builds if needed
Posted by GitBox <gi...@apache.org>.
gnodet commented on code in PR #954:
URL: https://github.com/apache/maven/pull/954#discussion_r1073666636
##########
maven-core/src/main/java/org/apache/maven/ReactorReader.java:
##########
@@ -324,4 +273,185 @@ private static boolean isTestArtifact(Artifact artifact) {
return ("test-jar".equals(artifact.getProperty("type", "")))
|| ("jar".equals(artifact.getExtension()) && "tests".equals(artifact.getClassifier()));
}
+
+ private File findInProjectLocalRepository(Artifact artifact) {
+ Path target = getArtifactPath(artifact);
+ return Files.isRegularFile(target) ? target.toFile() : null;
+ }
+
+ /**
+ * Copy packaged and attached artifacts from this project to the
+ * project local repository.
+ * This allows a subsequent build to resume while still being able
+ * to locate attached artifacts.
+ *
+ * @param project the project to copy artifacts from
+ */
+ private void installIntoProjectLocalRepository(MavenProject project) {
+ if (!hasBeenPackagedDuringThisSession(project)) {
+ return;
+ }
+ getProjectArtifacts(project).filter(this::isRegularFile).forEach(this::installIntoProjectLocalRepository);
+ }
+
+ private void cleanProjectLocalRepository(MavenProject project) {
+ try {
+ Path artifactPath = getProjectLocalRepo()
+ .resolve(project.getGroupId())
+ .resolve(project.getArtifactId())
+ .resolve(project.getVersion());
+ if (Files.isDirectory(artifactPath)) {
+ try (Stream<Path> paths = Files.list(artifactPath)) {
+ for (Path path : (Iterable<Path>) paths::iterator) {
+ Files.delete(path);
+ }
+ }
+ try {
+ Files.delete(artifactPath);
+ Files.delete(artifactPath.getParent());
+ Files.delete(artifactPath.getParent().getParent());
+ } catch (DirectoryNotEmptyException e) {
+ // ignore
+ }
+ }
+ } catch (IOException e) {
+ LOGGER.error("Error while cleaning project local repository", e);
+ }
+ }
+
+ /**
+ * Retrieve a stream of the project's artifacts
+ */
+ private Stream<Artifact> getProjectArtifacts(MavenProject project) {
+ Stream<org.apache.maven.artifact.Artifact> artifacts = Stream.concat(
+ Stream.concat(
+ // pom artifact
+ Stream.of(new ProjectArtifact(project)),
+ // main project artifact if not a pom
+ "pom".equals(project.getPackaging()) ? Stream.empty() : Stream.of(project.getArtifact())),
+ // attached artifacts
+ project.getAttachedArtifacts().stream());
+ return artifacts.map(RepositoryUtils::toArtifact);
+ }
+
+ private boolean isRegularFile(Artifact artifact) {
+ return artifact.getFile() != null && artifact.getFile().isFile();
+ }
+
+ private void installIntoProjectLocalRepository(Artifact artifact) {
+ Path target = getArtifactPath(artifact);
+ try {
+ LOGGER.info("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.error("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(version)
+ .resolve(artifactId
+ + "-" + version
+ + (classifier != null && !classifier.isEmpty() ? "-" + classifier : "")
+ + "." + extension);
+ }
+
+ private Path getProjectLocalRepo() {
+ Path root = session.getRequest().getMultiModuleProjectDirectory().toPath();
+ return root.resolve("target").resolve("project-local-repo");
Review Comment:
I'm not really sure this is configurable. Is it ?
--
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
[GitHub] [maven] gnodet commented on a diff in pull request #954: [MNG-7629] Change reactor reader to copy packaged artifacts and reuse them across builds if needed
Posted by GitBox <gi...@apache.org>.
gnodet commented on code in PR #954:
URL: https://github.com/apache/maven/pull/954#discussion_r1073679949
##########
maven-core/src/main/java/org/apache/maven/ReactorReader.java:
##########
@@ -324,4 +273,185 @@ private static boolean isTestArtifact(Artifact artifact) {
return ("test-jar".equals(artifact.getProperty("type", "")))
|| ("jar".equals(artifact.getExtension()) && "tests".equals(artifact.getClassifier()));
}
+
+ private File findInProjectLocalRepository(Artifact artifact) {
+ Path target = getArtifactPath(artifact);
+ return Files.isRegularFile(target) ? target.toFile() : null;
+ }
+
+ /**
+ * Copy packaged and attached artifacts from this project to the
+ * project local repository.
+ * This allows a subsequent build to resume while still being able
+ * to locate attached artifacts.
+ *
+ * @param project the project to copy artifacts from
+ */
+ private void installIntoProjectLocalRepository(MavenProject project) {
+ if (!hasBeenPackagedDuringThisSession(project)) {
+ return;
+ }
+ getProjectArtifacts(project).filter(this::isRegularFile).forEach(this::installIntoProjectLocalRepository);
+ }
+
+ private void cleanProjectLocalRepository(MavenProject project) {
+ try {
+ Path artifactPath = getProjectLocalRepo()
+ .resolve(project.getGroupId())
+ .resolve(project.getArtifactId())
+ .resolve(project.getVersion());
+ if (Files.isDirectory(artifactPath)) {
+ try (Stream<Path> paths = Files.list(artifactPath)) {
+ for (Path path : (Iterable<Path>) paths::iterator) {
+ Files.delete(path);
+ }
+ }
+ try {
+ Files.delete(artifactPath);
+ Files.delete(artifactPath.getParent());
+ Files.delete(artifactPath.getParent().getParent());
+ } catch (DirectoryNotEmptyException e) {
+ // ignore
+ }
+ }
+ } catch (IOException e) {
+ LOGGER.error("Error while cleaning project local repository", e);
+ }
+ }
+
+ /**
+ * Retrieve a stream of the project's artifacts
+ */
+ private Stream<Artifact> getProjectArtifacts(MavenProject project) {
+ Stream<org.apache.maven.artifact.Artifact> artifacts = Stream.concat(
+ Stream.concat(
+ // pom artifact
+ Stream.of(new ProjectArtifact(project)),
+ // main project artifact if not a pom
+ "pom".equals(project.getPackaging()) ? Stream.empty() : Stream.of(project.getArtifact())),
+ // attached artifacts
+ project.getAttachedArtifacts().stream());
+ return artifacts.map(RepositoryUtils::toArtifact);
+ }
+
+ private boolean isRegularFile(Artifact artifact) {
+ return artifact.getFile() != null && artifact.getFile().isFile();
+ }
+
+ private void installIntoProjectLocalRepository(Artifact artifact) {
+ Path target = getArtifactPath(artifact);
+ try {
+ LOGGER.info("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.error("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(version)
+ .resolve(artifactId
+ + "-" + version
+ + (classifier != null && !classifier.isEmpty() ? "-" + classifier : "")
+ + "." + extension);
+ }
+
+ private Path getProjectLocalRepo() {
+ Path root = session.getRequest().getMultiModuleProjectDirectory().toPath();
Review Comment:
First, [this property is always available](https://github.com/apache/maven/blob/c2c6dd09219744cfe45df34cabed6684fe5c9213/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java#L308-L320), and second, if no `.mvn` parent directory exists, the current directory will be used. So this is maybe not perfect, and I'd really like to have this root/top directory property problem solved in the near future, but in the mean time, that's the best we have afaik.
--
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
[GitHub] [maven] gnodet commented on a diff in pull request #954: [MNG-7629] Change reactor reader to copy packaged artifacts and reuse them across builds if needed
Posted by GitBox <gi...@apache.org>.
gnodet commented on code in PR #954:
URL: https://github.com/apache/maven/pull/954#discussion_r1071614557
##########
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:
The idea is to run at least the `package` phase, but I thought we wanted to promote running `mvn verify` rather than `mvn install`.
--
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
[GitHub] [maven] gnodet commented on a diff in pull request #954: [MNG-7629] Change reactor reader to copy packaged artifacts and reuse them across builds if needed
Posted by GitBox <gi...@apache.org>.
gnodet commented on code in PR #954:
URL: https://github.com/apache/maven/pull/954#discussion_r1072176566
##########
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:
Changed my mind. `mvn package` looks better.
--
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
[GitHub] [maven] cstamas commented on a diff in pull request #954: [MNG-7629] Change reactor reader to copy packaged artifacts and reuse them across builds if needed
Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #954:
URL: https://github.com/apache/maven/pull/954#discussion_r1073654940
##########
maven-core/src/main/java/org/apache/maven/ReactorReader.java:
##########
@@ -324,4 +273,185 @@ private static boolean isTestArtifact(Artifact artifact) {
return ("test-jar".equals(artifact.getProperty("type", "")))
|| ("jar".equals(artifact.getExtension()) && "tests".equals(artifact.getClassifier()));
}
+
+ private File findInProjectLocalRepository(Artifact artifact) {
+ Path target = getArtifactPath(artifact);
+ return Files.isRegularFile(target) ? target.toFile() : null;
+ }
+
+ /**
+ * Copy packaged and attached artifacts from this project to the
+ * project local repository.
+ * This allows a subsequent build to resume while still being able
+ * to locate attached artifacts.
+ *
+ * @param project the project to copy artifacts from
+ */
+ private void installIntoProjectLocalRepository(MavenProject project) {
+ if (!hasBeenPackagedDuringThisSession(project)) {
+ return;
+ }
+ getProjectArtifacts(project).filter(this::isRegularFile).forEach(this::installIntoProjectLocalRepository);
+ }
+
+ private void cleanProjectLocalRepository(MavenProject project) {
+ try {
+ Path artifactPath = getProjectLocalRepo()
+ .resolve(project.getGroupId())
+ .resolve(project.getArtifactId())
+ .resolve(project.getVersion());
+ if (Files.isDirectory(artifactPath)) {
+ try (Stream<Path> paths = Files.list(artifactPath)) {
+ for (Path path : (Iterable<Path>) paths::iterator) {
+ Files.delete(path);
+ }
+ }
+ try {
+ Files.delete(artifactPath);
+ Files.delete(artifactPath.getParent());
+ Files.delete(artifactPath.getParent().getParent());
+ } catch (DirectoryNotEmptyException e) {
+ // ignore
+ }
+ }
+ } catch (IOException e) {
+ LOGGER.error("Error while cleaning project local repository", e);
+ }
+ }
+
+ /**
+ * Retrieve a stream of the project's artifacts
+ */
+ private Stream<Artifact> getProjectArtifacts(MavenProject project) {
+ Stream<org.apache.maven.artifact.Artifact> artifacts = Stream.concat(
+ Stream.concat(
+ // pom artifact
+ Stream.of(new ProjectArtifact(project)),
+ // main project artifact if not a pom
+ "pom".equals(project.getPackaging()) ? Stream.empty() : Stream.of(project.getArtifact())),
+ // attached artifacts
+ project.getAttachedArtifacts().stream());
+ return artifacts.map(RepositoryUtils::toArtifact);
+ }
+
+ private boolean isRegularFile(Artifact artifact) {
+ return artifact.getFile() != null && artifact.getFile().isFile();
+ }
+
+ private void installIntoProjectLocalRepository(Artifact artifact) {
+ Path target = getArtifactPath(artifact);
+ try {
+ LOGGER.info("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.error("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(version)
+ .resolve(artifactId
+ + "-" + version
+ + (classifier != null && !classifier.isEmpty() ? "-" + classifier : "")
+ + "." + extension);
+ }
+
+ private Path getProjectLocalRepo() {
+ Path root = session.getRequest().getMultiModuleProjectDirectory().toPath();
+ return root.resolve("target").resolve("project-local-repo");
Review Comment:
um, this is hardcoded?
--
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
[GitHub] [maven] cstamas commented on a diff in pull request #954: [MNG-7629] Change reactor reader to copy packaged artifacts and reuse them across builds if needed
Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #954:
URL: https://github.com/apache/maven/pull/954#discussion_r1073658196
##########
maven-core/src/main/java/org/apache/maven/ReactorReader.java:
##########
@@ -324,4 +273,185 @@ private static boolean isTestArtifact(Artifact artifact) {
return ("test-jar".equals(artifact.getProperty("type", "")))
|| ("jar".equals(artifact.getExtension()) && "tests".equals(artifact.getClassifier()));
}
+
+ private File findInProjectLocalRepository(Artifact artifact) {
+ Path target = getArtifactPath(artifact);
+ return Files.isRegularFile(target) ? target.toFile() : null;
+ }
+
+ /**
+ * Copy packaged and attached artifacts from this project to the
+ * project local repository.
+ * This allows a subsequent build to resume while still being able
+ * to locate attached artifacts.
+ *
+ * @param project the project to copy artifacts from
+ */
+ private void installIntoProjectLocalRepository(MavenProject project) {
+ if (!hasBeenPackagedDuringThisSession(project)) {
+ return;
+ }
+ getProjectArtifacts(project).filter(this::isRegularFile).forEach(this::installIntoProjectLocalRepository);
+ }
+
+ private void cleanProjectLocalRepository(MavenProject project) {
+ try {
+ Path artifactPath = getProjectLocalRepo()
+ .resolve(project.getGroupId())
+ .resolve(project.getArtifactId())
+ .resolve(project.getVersion());
+ if (Files.isDirectory(artifactPath)) {
+ try (Stream<Path> paths = Files.list(artifactPath)) {
+ for (Path path : (Iterable<Path>) paths::iterator) {
+ Files.delete(path);
+ }
+ }
+ try {
+ Files.delete(artifactPath);
+ Files.delete(artifactPath.getParent());
+ Files.delete(artifactPath.getParent().getParent());
+ } catch (DirectoryNotEmptyException e) {
+ // ignore
+ }
+ }
+ } catch (IOException e) {
+ LOGGER.error("Error while cleaning project local repository", e);
+ }
+ }
+
+ /**
+ * Retrieve a stream of the project's artifacts
+ */
+ private Stream<Artifact> getProjectArtifacts(MavenProject project) {
+ Stream<org.apache.maven.artifact.Artifact> artifacts = Stream.concat(
+ Stream.concat(
+ // pom artifact
+ Stream.of(new ProjectArtifact(project)),
+ // main project artifact if not a pom
+ "pom".equals(project.getPackaging()) ? Stream.empty() : Stream.of(project.getArtifact())),
+ // attached artifacts
+ project.getAttachedArtifacts().stream());
+ return artifacts.map(RepositoryUtils::toArtifact);
+ }
+
+ private boolean isRegularFile(Artifact artifact) {
+ return artifact.getFile() != null && artifact.getFile().isFile();
+ }
+
+ private void installIntoProjectLocalRepository(Artifact artifact) {
+ Path target = getArtifactPath(artifact);
+ try {
+ LOGGER.info("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.error("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(version)
+ .resolve(artifactId
+ + "-" + version
+ + (classifier != null && !classifier.isEmpty() ? "-" + classifier : "")
+ + "." + extension);
+ }
+
+ private Path getProjectLocalRepo() {
+ Path root = session.getRequest().getMultiModuleProjectDirectory().toPath();
Review Comment:
Also, the use of `session.getRequest().getMultiModuleProjectDirectory()` implies existence of top level `.mvn`?
--
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
[GitHub] [maven] cstamas commented on a diff in pull request #954: [MNG-7629] Change reactor reader to copy packaged artifacts and reuse them across builds if needed
Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #954:
URL: https://github.com/apache/maven/pull/954#discussion_r1073654940
##########
maven-core/src/main/java/org/apache/maven/ReactorReader.java:
##########
@@ -324,4 +273,185 @@ private static boolean isTestArtifact(Artifact artifact) {
return ("test-jar".equals(artifact.getProperty("type", "")))
|| ("jar".equals(artifact.getExtension()) && "tests".equals(artifact.getClassifier()));
}
+
+ private File findInProjectLocalRepository(Artifact artifact) {
+ Path target = getArtifactPath(artifact);
+ return Files.isRegularFile(target) ? target.toFile() : null;
+ }
+
+ /**
+ * Copy packaged and attached artifacts from this project to the
+ * project local repository.
+ * This allows a subsequent build to resume while still being able
+ * to locate attached artifacts.
+ *
+ * @param project the project to copy artifacts from
+ */
+ private void installIntoProjectLocalRepository(MavenProject project) {
+ if (!hasBeenPackagedDuringThisSession(project)) {
+ return;
+ }
+ getProjectArtifacts(project).filter(this::isRegularFile).forEach(this::installIntoProjectLocalRepository);
+ }
+
+ private void cleanProjectLocalRepository(MavenProject project) {
+ try {
+ Path artifactPath = getProjectLocalRepo()
+ .resolve(project.getGroupId())
+ .resolve(project.getArtifactId())
+ .resolve(project.getVersion());
+ if (Files.isDirectory(artifactPath)) {
+ try (Stream<Path> paths = Files.list(artifactPath)) {
+ for (Path path : (Iterable<Path>) paths::iterator) {
+ Files.delete(path);
+ }
+ }
+ try {
+ Files.delete(artifactPath);
+ Files.delete(artifactPath.getParent());
+ Files.delete(artifactPath.getParent().getParent());
+ } catch (DirectoryNotEmptyException e) {
+ // ignore
+ }
+ }
+ } catch (IOException e) {
+ LOGGER.error("Error while cleaning project local repository", e);
+ }
+ }
+
+ /**
+ * Retrieve a stream of the project's artifacts
+ */
+ private Stream<Artifact> getProjectArtifacts(MavenProject project) {
+ Stream<org.apache.maven.artifact.Artifact> artifacts = Stream.concat(
+ Stream.concat(
+ // pom artifact
+ Stream.of(new ProjectArtifact(project)),
+ // main project artifact if not a pom
+ "pom".equals(project.getPackaging()) ? Stream.empty() : Stream.of(project.getArtifact())),
+ // attached artifacts
+ project.getAttachedArtifacts().stream());
+ return artifacts.map(RepositoryUtils::toArtifact);
+ }
+
+ private boolean isRegularFile(Artifact artifact) {
+ return artifact.getFile() != null && artifact.getFile().isFile();
+ }
+
+ private void installIntoProjectLocalRepository(Artifact artifact) {
+ Path target = getArtifactPath(artifact);
+ try {
+ LOGGER.info("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.error("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(version)
+ .resolve(artifactId
+ + "-" + version
+ + (classifier != null && !classifier.isEmpty() ? "-" + classifier : "")
+ + "." + extension);
+ }
+
+ private Path getProjectLocalRepo() {
+ Path root = session.getRequest().getMultiModuleProjectDirectory().toPath();
+ return root.resolve("target").resolve("project-local-repo");
Review Comment:
um, `target` is hardcoded?
--
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
[GitHub] [maven] gnodet commented on pull request #954: [MNG-7629] Change reactor reader to copy packaged artifacts and reuse them across builds if needed
Posted by GitBox <gi...@apache.org>.
gnodet commented on PR #954:
URL: https://github.com/apache/maven/pull/954#issuecomment-1387156322
> > > ... in order to be able to partially run mvn test.
> >
> >
> > I'm not sure I understand. You mean "run `mvn test` on a multi-module project without requiring the user to have ran `mvn package` first"?
>
> Yes. I haven't removed this part in this PR. If the main artifacts have not been packaged, they will point to the `target/classes`.
I've been able to remove [the check to load the packaged artifacts](https://github.com/apache/maven/pull/954/commits/197b743db736adf419627f56456e2d4b13f2d2e7).
--
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
[GitHub] [maven] cstamas commented on a diff in pull request #954: [MNG-7629] Change reactor reader to copy packaged artifacts and reuse them across builds if needed
Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #954:
URL: https://github.com/apache/maven/pull/954#discussion_r1073675245
##########
maven-core/src/main/java/org/apache/maven/ReactorReader.java:
##########
@@ -324,4 +273,185 @@ private static boolean isTestArtifact(Artifact artifact) {
return ("test-jar".equals(artifact.getProperty("type", "")))
|| ("jar".equals(artifact.getExtension()) && "tests".equals(artifact.getClassifier()));
}
+
+ private File findInProjectLocalRepository(Artifact artifact) {
+ Path target = getArtifactPath(artifact);
+ return Files.isRegularFile(target) ? target.toFile() : null;
+ }
+
+ /**
+ * Copy packaged and attached artifacts from this project to the
+ * project local repository.
+ * This allows a subsequent build to resume while still being able
+ * to locate attached artifacts.
+ *
+ * @param project the project to copy artifacts from
+ */
+ private void installIntoProjectLocalRepository(MavenProject project) {
+ if (!hasBeenPackagedDuringThisSession(project)) {
+ return;
+ }
+ getProjectArtifacts(project).filter(this::isRegularFile).forEach(this::installIntoProjectLocalRepository);
+ }
+
+ private void cleanProjectLocalRepository(MavenProject project) {
+ try {
+ Path artifactPath = getProjectLocalRepo()
+ .resolve(project.getGroupId())
+ .resolve(project.getArtifactId())
+ .resolve(project.getVersion());
+ if (Files.isDirectory(artifactPath)) {
+ try (Stream<Path> paths = Files.list(artifactPath)) {
+ for (Path path : (Iterable<Path>) paths::iterator) {
+ Files.delete(path);
+ }
+ }
+ try {
+ Files.delete(artifactPath);
+ Files.delete(artifactPath.getParent());
+ Files.delete(artifactPath.getParent().getParent());
+ } catch (DirectoryNotEmptyException e) {
+ // ignore
+ }
+ }
+ } catch (IOException e) {
+ LOGGER.error("Error while cleaning project local repository", e);
+ }
+ }
+
+ /**
+ * Retrieve a stream of the project's artifacts
+ */
+ private Stream<Artifact> getProjectArtifacts(MavenProject project) {
+ Stream<org.apache.maven.artifact.Artifact> artifacts = Stream.concat(
+ Stream.concat(
+ // pom artifact
+ Stream.of(new ProjectArtifact(project)),
+ // main project artifact if not a pom
+ "pom".equals(project.getPackaging()) ? Stream.empty() : Stream.of(project.getArtifact())),
+ // attached artifacts
+ project.getAttachedArtifacts().stream());
+ return artifacts.map(RepositoryUtils::toArtifact);
+ }
+
+ private boolean isRegularFile(Artifact artifact) {
+ return artifact.getFile() != null && artifact.getFile().isFile();
+ }
+
+ private void installIntoProjectLocalRepository(Artifact artifact) {
+ Path target = getArtifactPath(artifact);
+ try {
+ LOGGER.info("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.error("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(version)
+ .resolve(artifactId
+ + "-" + version
+ + (classifier != null && !classifier.isEmpty() ? "-" + classifier : "")
+ + "." + extension);
+ }
+
+ private Path getProjectLocalRepo() {
+ Path root = session.getRequest().getMultiModuleProjectDirectory().toPath();
+ return root.resolve("target").resolve("project-local-repo");
Review Comment:
Default is in super POM https://github.com/apache/maven/blob/20f7c65a520ee4c6a5dde0de4d58e304b1bde442/maven-model-builder/src/main/resources/org/apache/maven/model/pom-4.0.0.xml#L54
--
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
[GitHub] [maven] cstamas commented on a diff in pull request #954: [MNG-7629] Change reactor reader to copy packaged artifacts and reuse them across builds if needed
Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #954:
URL: https://github.com/apache/maven/pull/954#discussion_r1073670208
##########
maven-core/src/main/java/org/apache/maven/ReactorReader.java:
##########
@@ -324,4 +273,185 @@ private static boolean isTestArtifact(Artifact artifact) {
return ("test-jar".equals(artifact.getProperty("type", "")))
|| ("jar".equals(artifact.getExtension()) && "tests".equals(artifact.getClassifier()));
}
+
+ private File findInProjectLocalRepository(Artifact artifact) {
+ Path target = getArtifactPath(artifact);
+ return Files.isRegularFile(target) ? target.toFile() : null;
+ }
+
+ /**
+ * Copy packaged and attached artifacts from this project to the
+ * project local repository.
+ * This allows a subsequent build to resume while still being able
+ * to locate attached artifacts.
+ *
+ * @param project the project to copy artifacts from
+ */
+ private void installIntoProjectLocalRepository(MavenProject project) {
+ if (!hasBeenPackagedDuringThisSession(project)) {
+ return;
+ }
+ getProjectArtifacts(project).filter(this::isRegularFile).forEach(this::installIntoProjectLocalRepository);
+ }
+
+ private void cleanProjectLocalRepository(MavenProject project) {
+ try {
+ Path artifactPath = getProjectLocalRepo()
+ .resolve(project.getGroupId())
+ .resolve(project.getArtifactId())
+ .resolve(project.getVersion());
+ if (Files.isDirectory(artifactPath)) {
+ try (Stream<Path> paths = Files.list(artifactPath)) {
+ for (Path path : (Iterable<Path>) paths::iterator) {
+ Files.delete(path);
+ }
+ }
+ try {
+ Files.delete(artifactPath);
+ Files.delete(artifactPath.getParent());
+ Files.delete(artifactPath.getParent().getParent());
+ } catch (DirectoryNotEmptyException e) {
+ // ignore
+ }
+ }
+ } catch (IOException e) {
+ LOGGER.error("Error while cleaning project local repository", e);
+ }
+ }
+
+ /**
+ * Retrieve a stream of the project's artifacts
+ */
+ private Stream<Artifact> getProjectArtifacts(MavenProject project) {
+ Stream<org.apache.maven.artifact.Artifact> artifacts = Stream.concat(
+ Stream.concat(
+ // pom artifact
+ Stream.of(new ProjectArtifact(project)),
+ // main project artifact if not a pom
+ "pom".equals(project.getPackaging()) ? Stream.empty() : Stream.of(project.getArtifact())),
+ // attached artifacts
+ project.getAttachedArtifacts().stream());
+ return artifacts.map(RepositoryUtils::toArtifact);
+ }
+
+ private boolean isRegularFile(Artifact artifact) {
+ return artifact.getFile() != null && artifact.getFile().isFile();
+ }
+
+ private void installIntoProjectLocalRepository(Artifact artifact) {
+ Path target = getArtifactPath(artifact);
+ try {
+ LOGGER.info("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.error("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(version)
+ .resolve(artifactId
+ + "-" + version
+ + (classifier != null && !classifier.isEmpty() ? "-" + classifier : "")
+ + "." + extension);
+ }
+
+ private Path getProjectLocalRepo() {
+ Path root = session.getRequest().getMultiModuleProjectDirectory().toPath();
+ return root.resolve("target").resolve("project-local-repo");
Review Comment:
https://maven.apache.org/pom.html#Build
--
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
[GitHub] [maven] mthmulders commented on a diff in pull request #954: [MNG-7629] Change reactor reader to copy packaged artifacts and reuse them across builds if needed
Posted by GitBox <gi...@apache.org>.
mthmulders commented on code in PR #954:
URL: https://github.com/apache/maven/pull/954#discussion_r1072684739
##########
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:
Thanks!
--
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
[GitHub] [maven] MartinKanters commented on a diff in pull request #954: [MNG-7629] Change reactor reader to copy packaged artifacts and reuse them across builds if needed
Posted by GitBox <gi...@apache.org>.
MartinKanters commented on code in PR #954:
URL: https://github.com/apache/maven/pull/954#discussion_r1073152217
##########
maven-core/src/main/java/org/apache/maven/ReactorReader.java:
##########
@@ -324,4 +309,130 @@ 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;
+ }
+
+ /**
+ * Copy packaged and attached artifacts from this project to the
+ * project local repository.
+ * This allow a subsequent build to resume while still being able
+ * to locate attached artifacts.
+ *
+ * @param project the project to copy artifacts
Review Comment:
```suggestion
* This allows a subsequent build to resume while still being able
* to locate attached artifacts.
*
* @param project the project to copy artifacts from
```
##########
maven-core/src/main/java/org/apache/maven/ReactorReader.java:
##########
@@ -117,18 +105,19 @@ public File findArtifact(Artifact artifact) {
}
public List<String> findVersions(Artifact artifact) {
- String key = ArtifactUtils.versionlessKey(artifact.getGroupId(), artifact.getArtifactId());
-
- return Optional.ofNullable(projectsByGA.get(key)).orElse(Collections.emptyList()).stream()
- .filter(s -> Objects.nonNull(find(s, artifact)))
+ return getProjects()
+ .getOrDefault(artifact.getGroupId(), Collections.emptyMap())
+ .getOrDefault(artifact.getArtifactId(), Collections.emptyMap())
+ .values()
+ .stream()
+ .filter(p -> Objects.nonNull(find(p, artifact)))
Review Comment:
Since `find` is also not very descriptive, let's write out `p` into `project`
##########
maven-core/src/main/java/org/apache/maven/ReactorReader.java:
##########
@@ -324,4 +309,130 @@ 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;
+ }
+
+ /**
+ * Copy packaged and attached artifacts from this project to the
+ * project local repository.
+ * This allow a subsequent build to resume while still being able
+ * to locate attached artifacts.
+ *
+ * @param project the project to copy artifacts
+ */
+ private void installIntoProjectLocalRepository(MavenProject project) {
+ if (!hasBeenPackagedDuringThisSession(project)) {
+ return;
+ }
+ 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) {
Review Comment:
You could go for streams here as well, if you like.
--
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
[GitHub] [maven] gnodet commented on a diff in pull request #954: [MNG-7629] Change reactor reader to copy packaged artifacts and reuse them across builds if needed
Posted by GitBox <gi...@apache.org>.
gnodet commented on code in PR #954:
URL: https://github.com/apache/maven/pull/954#discussion_r1071816860
##########
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:
> Also, I wonder if this change allows us to remove the fallbacks that were previously on line 150 - 159. Since those lines aren't changed, it seems GitHub doesn't allow me to comment on the spot :-\.
Good question. I think part of it could be removed, we'll need to keep the `determineBuildOutputDirectoryForArtifact` in order to be able to partially run `mvn test`.
--
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
[GitHub] [maven] gnodet commented on a diff in pull request #954: [MNG-7629] Change reactor reader to copy packaged artifacts and reuse them across builds if needed
Posted by GitBox <gi...@apache.org>.
gnodet commented on code in PR #954:
URL: https://github.com/apache/maven/pull/954#discussion_r1073764622
##########
maven-core/src/main/java/org/apache/maven/ReactorReader.java:
##########
@@ -324,4 +273,185 @@ private static boolean isTestArtifact(Artifact artifact) {
return ("test-jar".equals(artifact.getProperty("type", "")))
|| ("jar".equals(artifact.getExtension()) && "tests".equals(artifact.getClassifier()));
}
+
+ private File findInProjectLocalRepository(Artifact artifact) {
+ Path target = getArtifactPath(artifact);
+ return Files.isRegularFile(target) ? target.toFile() : null;
+ }
+
+ /**
+ * Copy packaged and attached artifacts from this project to the
+ * project local repository.
+ * This allows a subsequent build to resume while still being able
+ * to locate attached artifacts.
+ *
+ * @param project the project to copy artifacts from
+ */
+ private void installIntoProjectLocalRepository(MavenProject project) {
+ if (!hasBeenPackagedDuringThisSession(project)) {
+ return;
+ }
+ getProjectArtifacts(project).filter(this::isRegularFile).forEach(this::installIntoProjectLocalRepository);
+ }
+
+ private void cleanProjectLocalRepository(MavenProject project) {
+ try {
+ Path artifactPath = getProjectLocalRepo()
+ .resolve(project.getGroupId())
+ .resolve(project.getArtifactId())
+ .resolve(project.getVersion());
+ if (Files.isDirectory(artifactPath)) {
+ try (Stream<Path> paths = Files.list(artifactPath)) {
+ for (Path path : (Iterable<Path>) paths::iterator) {
+ Files.delete(path);
+ }
+ }
+ try {
+ Files.delete(artifactPath);
+ Files.delete(artifactPath.getParent());
+ Files.delete(artifactPath.getParent().getParent());
+ } catch (DirectoryNotEmptyException e) {
+ // ignore
+ }
+ }
+ } catch (IOException e) {
+ LOGGER.error("Error while cleaning project local repository", e);
+ }
+ }
+
+ /**
+ * Retrieve a stream of the project's artifacts
+ */
+ private Stream<Artifact> getProjectArtifacts(MavenProject project) {
+ Stream<org.apache.maven.artifact.Artifact> artifacts = Stream.concat(
+ Stream.concat(
+ // pom artifact
+ Stream.of(new ProjectArtifact(project)),
+ // main project artifact if not a pom
+ "pom".equals(project.getPackaging()) ? Stream.empty() : Stream.of(project.getArtifact())),
+ // attached artifacts
+ project.getAttachedArtifacts().stream());
+ return artifacts.map(RepositoryUtils::toArtifact);
+ }
+
+ private boolean isRegularFile(Artifact artifact) {
+ return artifact.getFile() != null && artifact.getFile().isFile();
+ }
+
+ private void installIntoProjectLocalRepository(Artifact artifact) {
+ Path target = getArtifactPath(artifact);
+ try {
+ LOGGER.info("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.error("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(version)
+ .resolve(artifactId
+ + "-" + version
+ + (classifier != null && !classifier.isEmpty() ? "-" + classifier : "")
+ + "." + extension);
+ }
+
+ private Path getProjectLocalRepo() {
+ Path root = session.getRequest().getMultiModuleProjectDirectory().toPath();
+ return root.resolve("target").resolve("project-local-repo");
Review Comment:
Fixed
--
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
[GitHub] [maven] gnodet commented on a diff in pull request #954: [MNG-7629] Change reactor reader to copy packaged artifacts and reuse them across builds if needed
Posted by GitBox <gi...@apache.org>.
gnodet commented on code in PR #954:
URL: https://github.com/apache/maven/pull/954#discussion_r1073601881
##########
maven-core/src/main/java/org/apache/maven/ReactorReader.java:
##########
@@ -324,4 +309,130 @@ 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;
+ }
+
+ /**
+ * Copy packaged and attached artifacts from this project to the
+ * project local repository.
+ * This allow a subsequent build to resume while still being able
+ * to locate attached artifacts.
+ *
+ * @param project the project to copy artifacts
+ */
+ private void installIntoProjectLocalRepository(MavenProject project) {
+ if (!hasBeenPackagedDuringThisSession(project)) {
+ return;
+ }
+ 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) {
Review Comment:
Done
--
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
[GitHub] [maven] gnodet commented on a diff in pull request #954: [MNG-7629] Change reactor reader to copy packaged artifacts and reuse them across builds if needed
Posted by GitBox <gi...@apache.org>.
gnodet commented on code in PR #954:
URL: https://github.com/apache/maven/pull/954#discussion_r1073602143
##########
maven-core/src/main/java/org/apache/maven/ReactorReader.java:
##########
@@ -117,18 +105,19 @@ public File findArtifact(Artifact artifact) {
}
public List<String> findVersions(Artifact artifact) {
- String key = ArtifactUtils.versionlessKey(artifact.getGroupId(), artifact.getArtifactId());
-
- return Optional.ofNullable(projectsByGA.get(key)).orElse(Collections.emptyList()).stream()
- .filter(s -> Objects.nonNull(find(s, artifact)))
+ return getProjects()
+ .getOrDefault(artifact.getGroupId(), Collections.emptyMap())
+ .getOrDefault(artifact.getArtifactId(), Collections.emptyMap())
+ .values()
+ .stream()
+ .filter(p -> Objects.nonNull(find(p, artifact)))
Review Comment:
I've renamed a few methods.
--
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
[GitHub] [maven] gnodet merged pull request #954: [MNG-7629] Change reactor reader to copy packaged artifacts and reuse them across builds if needed
Posted by GitBox <gi...@apache.org>.
gnodet merged PR #954:
URL: https://github.com/apache/maven/pull/954
--
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
[GitHub] [maven] gnodet commented on pull request #954: [MNG-7629] Change reactor reader to copy packaged artifacts and reuse them across builds if needed
Posted by GitBox <gi...@apache.org>.
gnodet commented on PR #954:
URL: https://github.com/apache/maven/pull/954#issuecomment-1384932632
> Left a couple of questions.
>
> Also, I wonder if this change allows us to remove the fallbacks that were previously on line 150 - 159. Since those lines aren't changed, it seems GitHub doesn't allow me to comment on the spot :-\.
Good question. I think part of it could be removed, but we need to keep the `determineBuildOutputDirectoryForArtifact` in order to be able to partially run mvn test.
Another possible strategy instead of copying the artifacts would be to only record their paths.
--
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