You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/12/14 21:23:08 UTC
[GitHub] [maven] gnodet opened a new pull request, #912: [MNG-7629] Fix resolution of reactor dependencies
gnodet opened a new pull request, #912:
URL: https://github.com/apache/maven/pull/912
- Partial revert
- [MNG-7629] Change reactor reader to copy packaged artifacts and reuse them across builds if needed
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. Best practice is to use the JIRA issue
title in 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] mthmulders commented on a diff in pull request #912: [MNG-7629] Fix resolution of reactor dependencies
Posted by GitBox <gi...@apache.org>.
mthmulders commented on code in PR #912:
URL: https://github.com/apache/maven/pull/912#discussion_r1051655836
##########
maven-core/src/main/java/org/apache/maven/ReactorReader.java:
##########
@@ -64,28 +63,21 @@
class ReactorReader implements MavenWorkspaceReader {
public static final String HINT = "reactor";
- private static final Collection<String> COMPILE_PHASE_TYPES =
- Arrays.asList("jar", "ejb-client", "war", "rar", "ejb3", "par", "sar", "wsr", "har", "app-client");
-
private static final Logger LOGGER = LoggerFactory.getLogger(ReactorReader.class);
private final MavenSession session;
private final Map<String, MavenProject> projectsByGAV;
private final Map<String, List<MavenProject>> projectsByGA;
private final WorkspaceRepository repository;
- private Function<MavenProject, String> projectIntoKey =
- s -> ArtifactUtils.key(s.getGroupId(), s.getArtifactId(), s.getVersion());
-
- private Function<MavenProject, String> projectIntoVersionlessKey =
- s -> ArtifactUtils.versionlessKey(s.getGroupId(), s.getArtifactId());
-
@Inject
ReactorReader(MavenSession session) {
this.session = session;
- this.projectsByGAV = session.getAllProjects().stream().collect(toMap(projectIntoKey, identity()));
+ this.projectsByGAV = session.getAllProjects().stream()
Review Comment:
I don't see how this is necessary for this issue, but I think it is equivalent from a technical point of view.
##########
maven-core/src/main/java/org/apache/maven/ReactorReader.java:
##########
@@ -139,187 +132,75 @@ private File find(MavenProject project, Artifact artifact) {
return project.getFile();
}
- Artifact projectArtifact = findMatchingArtifact(project, artifact);
- File packagedArtifactFile = determinePreviouslyPackagedArtifactFile(project, projectArtifact);
-
- if (hasArtifactFileFromPackagePhase(projectArtifact)) {
- return projectArtifact.getFile();
- }
- // Check whether an earlier Maven run might have produced an artifact that is still on disk.
- else if (packagedArtifactFile != null
- && packagedArtifactFile.exists()
- && isPackagedArtifactUpToDate(project, packagedArtifactFile, artifact)) {
- return packagedArtifactFile;
- } else if (!hasBeenPackagedDuringThisSession(project)) {
- // fallback to loose class files only if artifacts haven't been packaged yet
- // and only for plain old jars. Not war files, not ear files, not anything else.
- return determineBuildOutputDirectoryForArtifact(project, artifact);
- }
-
- // The fall-through indicates that the artifact cannot be found;
- // for instance if package produced nothing or classifier problems.
- return null;
+ Path target = getArtifactPath(artifact);
+ return Files.isRegularFile(target) ? target.toFile() : null;
}
- private File determineBuildOutputDirectoryForArtifact(final MavenProject project, final Artifact artifact) {
- if (isTestArtifact(artifact)) {
- if (project.hasLifecyclePhase("test-compile")) {
- return new File(project.getBuild().getTestOutputDirectory());
- }
- } else {
- String type = artifact.getProperty("type", "");
- File outputDirectory = new File(project.getBuild().getOutputDirectory());
+ public void processProject(MavenProject project) {
+ List<Artifact> artifacts = new ArrayList<>();
- // Check if the project is being built during this session, and if we can expect any output.
- // There is no need to check if the build has created any outputs, see MNG-2222.
- boolean projectCompiledDuringThisSession =
- project.hasLifecyclePhase("compile") && COMPILE_PHASE_TYPES.contains(type);
-
- // Check if the project is part of the session (not filtered by -pl, -rf, etc). If so, we check
- // if a possible earlier Maven invocation produced some output for that project which we can use.
- boolean projectHasOutputFromPreviousSession =
- !session.getProjects().contains(project) && outputDirectory.exists();
-
- if (projectHasOutputFromPreviousSession || projectCompiledDuringThisSession) {
- return outputDirectory;
- }
+ artifacts.add(RepositoryUtils.toArtifact(new ProjectArtifact(project)));
+ if (!"pom".equals(project.getPackaging())) {
+ org.apache.maven.artifact.Artifact mavenMainArtifact = project.getArtifact();
+ artifacts.add(RepositoryUtils.toArtifact(mavenMainArtifact));
}
-
- // The fall-through indicates that the artifact cannot be found;
- // for instance if package produced nothing or classifier problems.
- return null;
- }
-
- private File determinePreviouslyPackagedArtifactFile(MavenProject project, Artifact artifact) {
- if (artifact == null) {
- return null;
+ for (org.apache.maven.artifact.Artifact attached : project.getAttachedArtifacts()) {
+ artifacts.add(RepositoryUtils.toArtifact(attached));
}
- String fileName = String.format("%s.%s", project.getBuild().getFinalName(), artifact.getExtension());
- return new File(project.getBuild().getDirectory(), fileName);
- }
-
- private boolean hasArtifactFileFromPackagePhase(Artifact projectArtifact) {
- return projectArtifact != null
- && projectArtifact.getFile() != null
- && projectArtifact.getFile().exists();
- }
-
- private boolean isPackagedArtifactUpToDate(MavenProject project, File packagedArtifactFile, Artifact artifact) {
- Path outputDirectory = Paths.get(project.getBuild().getOutputDirectory());
- if (!outputDirectory.toFile().exists()) {
- return true;
- }
-
- try (Stream<Path> outputFiles = Files.walk(outputDirectory)) {
- // Not using File#lastModified() to avoid a Linux JDK8 milliseconds precision bug: JDK-8177809.
- long artifactLastModified =
- Files.getLastModifiedTime(packagedArtifactFile.toPath()).toMillis();
-
- if (session.getProjectBuildingRequest().getBuildStartTime() != null) {
- long buildStartTime =
- session.getProjectBuildingRequest().getBuildStartTime().getTime();
- if (artifactLastModified > buildStartTime) {
- return true;
- }
- }
-
- Iterator<Path> iterator = outputFiles.iterator();
- while (iterator.hasNext()) {
- Path outputFile = iterator.next();
-
- if (Files.isDirectory(outputFile)) {
- continue;
- }
-
- long outputFileLastModified =
- Files.getLastModifiedTime(outputFile).toMillis();
- if (outputFileLastModified > artifactLastModified) {
- File alternative = determineBuildOutputDirectoryForArtifact(project, artifact);
- if (alternative != null) {
- LOGGER.warn(
- "File '{}' is more recent than the packaged artifact for '{}'; using '{}' instead",
- relativizeOutputFile(outputFile),
- project.getArtifactId(),
- relativizeOutputFile(alternative.toPath()));
- } else {
- LOGGER.warn(
- "File '{}' is more recent than the packaged artifact for '{}'; "
- + "cannot use the build output directory for this type of artifact",
- relativizeOutputFile(outputFile),
- project.getArtifactId());
- }
- return false;
+ for (Artifact artifact : artifacts) {
+ if (artifact.getFile() != null && artifact.getFile().isFile()) {
+ Path target = getArtifactPath(artifact);
+ try {
+ Files.createDirectories(target.getParent());
+ Files.copy(artifact.getFile().toPath(), target, StandardCopyOption.REPLACE_EXISTING);
+ } catch (IOException e) {
+ LOGGER.warn("Error while copying artifact to project local repository", e);
}
}
-
- return true;
- } catch (IOException e) {
- LOGGER.warn(
- "An I/O error occurred while checking if the packaged artifact is up-to-date "
- + "against the build output directory. "
- + "Continuing with the assumption that it is up-to-date.",
- e);
- return true;
}
}
- private boolean hasBeenPackagedDuringThisSession(MavenProject project) {
- return project.hasLifecyclePhase("package")
- || project.hasLifecyclePhase("install")
- || project.hasLifecyclePhase("deploy");
+ private Path getArtifactPath(Artifact artifact) {
+ String groupId = artifact.getGroupId();
+ String artifactId = artifact.getArtifactId();
+ String classifier = artifact.getClassifier();
+ String extension = artifact.getExtension();
+ Path root = session.getRequest().getMultiModuleProjectDirectory().toPath();
+ Path repo = root.resolve("target/project-local-repo");
Review Comment:
Will this work as expected on operating systems that have the `\` as the file separator (e.g., Windows)?
##########
maven-core/src/main/java/org/apache/maven/ReactorReader.java:
##########
@@ -139,187 +132,75 @@ private File find(MavenProject project, Artifact artifact) {
return project.getFile();
}
- Artifact projectArtifact = findMatchingArtifact(project, artifact);
- File packagedArtifactFile = determinePreviouslyPackagedArtifactFile(project, projectArtifact);
-
- if (hasArtifactFileFromPackagePhase(projectArtifact)) {
- return projectArtifact.getFile();
- }
- // Check whether an earlier Maven run might have produced an artifact that is still on disk.
- else if (packagedArtifactFile != null
- && packagedArtifactFile.exists()
- && isPackagedArtifactUpToDate(project, packagedArtifactFile, artifact)) {
- return packagedArtifactFile;
- } else if (!hasBeenPackagedDuringThisSession(project)) {
- // fallback to loose class files only if artifacts haven't been packaged yet
- // and only for plain old jars. Not war files, not ear files, not anything else.
- return determineBuildOutputDirectoryForArtifact(project, artifact);
- }
-
- // The fall-through indicates that the artifact cannot be found;
- // for instance if package produced nothing or classifier problems.
- return null;
+ Path target = getArtifactPath(artifact);
+ return Files.isRegularFile(target) ? target.toFile() : null;
}
- private File determineBuildOutputDirectoryForArtifact(final MavenProject project, final Artifact artifact) {
- if (isTestArtifact(artifact)) {
- if (project.hasLifecyclePhase("test-compile")) {
- return new File(project.getBuild().getTestOutputDirectory());
- }
- } else {
- String type = artifact.getProperty("type", "");
- File outputDirectory = new File(project.getBuild().getOutputDirectory());
+ public void processProject(MavenProject project) {
+ List<Artifact> artifacts = new ArrayList<>();
- // Check if the project is being built during this session, and if we can expect any output.
- // There is no need to check if the build has created any outputs, see MNG-2222.
- boolean projectCompiledDuringThisSession =
- project.hasLifecyclePhase("compile") && COMPILE_PHASE_TYPES.contains(type);
-
- // Check if the project is part of the session (not filtered by -pl, -rf, etc). If so, we check
- // if a possible earlier Maven invocation produced some output for that project which we can use.
- boolean projectHasOutputFromPreviousSession =
- !session.getProjects().contains(project) && outputDirectory.exists();
-
- if (projectHasOutputFromPreviousSession || projectCompiledDuringThisSession) {
- return outputDirectory;
- }
+ artifacts.add(RepositoryUtils.toArtifact(new ProjectArtifact(project)));
+ if (!"pom".equals(project.getPackaging())) {
+ org.apache.maven.artifact.Artifact mavenMainArtifact = project.getArtifact();
+ artifacts.add(RepositoryUtils.toArtifact(mavenMainArtifact));
}
-
- // The fall-through indicates that the artifact cannot be found;
- // for instance if package produced nothing or classifier problems.
- return null;
- }
-
- private File determinePreviouslyPackagedArtifactFile(MavenProject project, Artifact artifact) {
- if (artifact == null) {
- return null;
+ for (org.apache.maven.artifact.Artifact attached : project.getAttachedArtifacts()) {
+ artifacts.add(RepositoryUtils.toArtifact(attached));
}
- String fileName = String.format("%s.%s", project.getBuild().getFinalName(), artifact.getExtension());
- return new File(project.getBuild().getDirectory(), fileName);
- }
-
- private boolean hasArtifactFileFromPackagePhase(Artifact projectArtifact) {
- return projectArtifact != null
- && projectArtifact.getFile() != null
- && projectArtifact.getFile().exists();
- }
-
- private boolean isPackagedArtifactUpToDate(MavenProject project, File packagedArtifactFile, Artifact artifact) {
- Path outputDirectory = Paths.get(project.getBuild().getOutputDirectory());
- if (!outputDirectory.toFile().exists()) {
- return true;
- }
-
- try (Stream<Path> outputFiles = Files.walk(outputDirectory)) {
- // Not using File#lastModified() to avoid a Linux JDK8 milliseconds precision bug: JDK-8177809.
- long artifactLastModified =
- Files.getLastModifiedTime(packagedArtifactFile.toPath()).toMillis();
-
- if (session.getProjectBuildingRequest().getBuildStartTime() != null) {
- long buildStartTime =
- session.getProjectBuildingRequest().getBuildStartTime().getTime();
- if (artifactLastModified > buildStartTime) {
- return true;
- }
- }
-
- Iterator<Path> iterator = outputFiles.iterator();
- while (iterator.hasNext()) {
- Path outputFile = iterator.next();
-
- if (Files.isDirectory(outputFile)) {
- continue;
- }
-
- long outputFileLastModified =
- Files.getLastModifiedTime(outputFile).toMillis();
- if (outputFileLastModified > artifactLastModified) {
- File alternative = determineBuildOutputDirectoryForArtifact(project, artifact);
- if (alternative != null) {
- LOGGER.warn(
- "File '{}' is more recent than the packaged artifact for '{}'; using '{}' instead",
- relativizeOutputFile(outputFile),
- project.getArtifactId(),
- relativizeOutputFile(alternative.toPath()));
- } else {
- LOGGER.warn(
- "File '{}' is more recent than the packaged artifact for '{}'; "
- + "cannot use the build output directory for this type of artifact",
- relativizeOutputFile(outputFile),
- project.getArtifactId());
- }
- return false;
+ for (Artifact artifact : artifacts) {
+ if (artifact.getFile() != null && artifact.getFile().isFile()) {
+ Path target = getArtifactPath(artifact);
+ try {
+ Files.createDirectories(target.getParent());
+ Files.copy(artifact.getFile().toPath(), target, StandardCopyOption.REPLACE_EXISTING);
Review Comment:
Would it be worth writing a log message to signal which file is copied to where?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #912: [MNG-7629] Fix resolution of reactor dependencies
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #912:
URL: https://github.com/apache/maven/pull/912#issuecomment-1356403407
> It's going to take me a bit of time to understand, but I definitely want to review this.
I'll try to understand by end of next week.
--
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 closed pull request #912: [MNG-7629] Improve resolution of modules within a multi-module build
Posted by GitBox <gi...@apache.org>.
gnodet closed pull request #912: [MNG-7629] Improve resolution of modules within a multi-module build
URL: https://github.com/apache/maven/pull/912
--
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 #912: [MNG-7629] Fix resolution of reactor dependencies
Posted by GitBox <gi...@apache.org>.
gnodet commented on PR #912:
URL: https://github.com/apache/maven/pull/912#issuecomment-1358469551
> In a multimodule project, when I run `mvn package` on the root level, it correctly copies the JAR to **target/project-local-repo**. When I then run `mvn package` on a submodule, it fails with
>
> > Could not resolve dependencies for project <groupId>:<artifactId>:jar:0.1-SNAPSHOT: Could not find artifact <groupId>:<artifactId>:jar:0.1-SNAPSHOT
>
> From the ticket and the conversation we had, I would expect `mvn compile` not to work anymore - which I still consider a big loss! But `mvn package` was supposed to work in this scenario, right?
I've tried to improve the PR to fully fix the `mvn verify -o -f submodule` use case. It was failing in various scenarii because the workspace reader was not used during in the early reactor phase.
This happen in maven `master` if you run an initial `mvn package` but then try to `mvn validate -f maven-settings -o`:
```
➜ maven git:(MNG-7629) /opt/homebrew/bin/mvn verify -DskipTests -o -f maven-settings
[INFO] Scanning for projects...
[ERROR] [ERROR] Some problems were encountered while processing the POMs:
[ERROR] Non-resolvable import POM: Cannot access apache.snapshots (https://repository.apache.org/snapshots) in offline mode and the artifact org.apache.maven:maven-bom:pom:4.0.0-alpha-4-SNAPSHOT has not been downloaded from it before. @ org.apache.maven:maven:4.0.0-alpha-4-SNAPSHOT, /Users/gnodet/work/git/maven/pom.xml, line 186, column 19
@
[ERROR] The build could not read 1 project -> [Help 1]
[ERROR]
[ERROR] The project org.apache.maven:maven-settings:4.0.0-alpha-4-SNAPSHOT (/Users/gnodet/work/git/maven/maven-settings/pom.xml) has 1 error
[ERROR] Non-resolvable import POM: Cannot access apache.snapshots (https://repository.apache.org/snapshots) in offline mode and the artifact org.apache.maven:maven-bom:pom:4.0.0-alpha-4-SNAPSHOT has not been downloaded from it before. @ org.apache.maven:maven:4.0.0-alpha-4-SNAPSHOT, /Users/gnodet/work/git/maven/pom.xml, line 186, column 19 -> [Help 2]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/ProjectBuildingException
[ERROR] [Help 2] http://cwiki.apache.org/confluence/display/MAVEN/UnresolvableModelException
```
while with this PR, all previously built artifacts can be reused in a subsequent 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] gnodet commented on pull request #912: [MNG-7629] Improve resolution of modules within a multi-module build
Posted by GitBox <gi...@apache.org>.
gnodet commented on PR #912:
URL: https://github.com/apache/maven/pull/912#issuecomment-1361132977
I'm going to split the two issues as they can be handled separately:
* [MNG-7629](https://issues.apache.org/jira/browse/MNG-7629): cache all built artifacts in a per-multi-project repository so that they can be reused in later builds, do not point to {{target/classes}} to ensure correct build
* [MNG-7646](https://issues.apache.org/jira/browse/MNG-7646): do not parse all poms in the reactor when building a subtree
--
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 #912: [MNG-7629] Fix resolution of reactor dependencies
Posted by GitBox <gi...@apache.org>.
mthmulders commented on PR #912:
URL: https://github.com/apache/maven/pull/912#issuecomment-1352708178
It's going to take me a bit of time to understand, but I definitely want to review this.
--
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 #912: [MNG-7629] Fix resolution of reactor dependencies
Posted by GitBox <gi...@apache.org>.
gnodet commented on code in PR #912:
URL: https://github.com/apache/maven/pull/912#discussion_r1051657680
##########
maven-core/src/main/java/org/apache/maven/ReactorReader.java:
##########
@@ -139,187 +132,75 @@ private File find(MavenProject project, Artifact artifact) {
return project.getFile();
}
- Artifact projectArtifact = findMatchingArtifact(project, artifact);
- File packagedArtifactFile = determinePreviouslyPackagedArtifactFile(project, projectArtifact);
-
- if (hasArtifactFileFromPackagePhase(projectArtifact)) {
- return projectArtifact.getFile();
- }
- // Check whether an earlier Maven run might have produced an artifact that is still on disk.
- else if (packagedArtifactFile != null
- && packagedArtifactFile.exists()
- && isPackagedArtifactUpToDate(project, packagedArtifactFile, artifact)) {
- return packagedArtifactFile;
- } else if (!hasBeenPackagedDuringThisSession(project)) {
- // fallback to loose class files only if artifacts haven't been packaged yet
- // and only for plain old jars. Not war files, not ear files, not anything else.
- return determineBuildOutputDirectoryForArtifact(project, artifact);
- }
-
- // The fall-through indicates that the artifact cannot be found;
- // for instance if package produced nothing or classifier problems.
- return null;
+ Path target = getArtifactPath(artifact);
+ return Files.isRegularFile(target) ? target.toFile() : null;
}
- private File determineBuildOutputDirectoryForArtifact(final MavenProject project, final Artifact artifact) {
- if (isTestArtifact(artifact)) {
- if (project.hasLifecyclePhase("test-compile")) {
- return new File(project.getBuild().getTestOutputDirectory());
- }
- } else {
- String type = artifact.getProperty("type", "");
- File outputDirectory = new File(project.getBuild().getOutputDirectory());
+ public void processProject(MavenProject project) {
+ List<Artifact> artifacts = new ArrayList<>();
- // Check if the project is being built during this session, and if we can expect any output.
- // There is no need to check if the build has created any outputs, see MNG-2222.
- boolean projectCompiledDuringThisSession =
- project.hasLifecyclePhase("compile") && COMPILE_PHASE_TYPES.contains(type);
-
- // Check if the project is part of the session (not filtered by -pl, -rf, etc). If so, we check
- // if a possible earlier Maven invocation produced some output for that project which we can use.
- boolean projectHasOutputFromPreviousSession =
- !session.getProjects().contains(project) && outputDirectory.exists();
-
- if (projectHasOutputFromPreviousSession || projectCompiledDuringThisSession) {
- return outputDirectory;
- }
+ artifacts.add(RepositoryUtils.toArtifact(new ProjectArtifact(project)));
+ if (!"pom".equals(project.getPackaging())) {
+ org.apache.maven.artifact.Artifact mavenMainArtifact = project.getArtifact();
+ artifacts.add(RepositoryUtils.toArtifact(mavenMainArtifact));
}
-
- // The fall-through indicates that the artifact cannot be found;
- // for instance if package produced nothing or classifier problems.
- return null;
- }
-
- private File determinePreviouslyPackagedArtifactFile(MavenProject project, Artifact artifact) {
- if (artifact == null) {
- return null;
+ for (org.apache.maven.artifact.Artifact attached : project.getAttachedArtifacts()) {
+ artifacts.add(RepositoryUtils.toArtifact(attached));
}
- String fileName = String.format("%s.%s", project.getBuild().getFinalName(), artifact.getExtension());
- return new File(project.getBuild().getDirectory(), fileName);
- }
-
- private boolean hasArtifactFileFromPackagePhase(Artifact projectArtifact) {
- return projectArtifact != null
- && projectArtifact.getFile() != null
- && projectArtifact.getFile().exists();
- }
-
- private boolean isPackagedArtifactUpToDate(MavenProject project, File packagedArtifactFile, Artifact artifact) {
- Path outputDirectory = Paths.get(project.getBuild().getOutputDirectory());
- if (!outputDirectory.toFile().exists()) {
- return true;
- }
-
- try (Stream<Path> outputFiles = Files.walk(outputDirectory)) {
- // Not using File#lastModified() to avoid a Linux JDK8 milliseconds precision bug: JDK-8177809.
- long artifactLastModified =
- Files.getLastModifiedTime(packagedArtifactFile.toPath()).toMillis();
-
- if (session.getProjectBuildingRequest().getBuildStartTime() != null) {
- long buildStartTime =
- session.getProjectBuildingRequest().getBuildStartTime().getTime();
- if (artifactLastModified > buildStartTime) {
- return true;
- }
- }
-
- Iterator<Path> iterator = outputFiles.iterator();
- while (iterator.hasNext()) {
- Path outputFile = iterator.next();
-
- if (Files.isDirectory(outputFile)) {
- continue;
- }
-
- long outputFileLastModified =
- Files.getLastModifiedTime(outputFile).toMillis();
- if (outputFileLastModified > artifactLastModified) {
- File alternative = determineBuildOutputDirectoryForArtifact(project, artifact);
- if (alternative != null) {
- LOGGER.warn(
- "File '{}' is more recent than the packaged artifact for '{}'; using '{}' instead",
- relativizeOutputFile(outputFile),
- project.getArtifactId(),
- relativizeOutputFile(alternative.toPath()));
- } else {
- LOGGER.warn(
- "File '{}' is more recent than the packaged artifact for '{}'; "
- + "cannot use the build output directory for this type of artifact",
- relativizeOutputFile(outputFile),
- project.getArtifactId());
- }
- return false;
+ for (Artifact artifact : artifacts) {
+ if (artifact.getFile() != null && artifact.getFile().isFile()) {
+ Path target = getArtifactPath(artifact);
+ try {
+ Files.createDirectories(target.getParent());
+ Files.copy(artifact.getFile().toPath(), target, StandardCopyOption.REPLACE_EXISTING);
+ } catch (IOException e) {
+ LOGGER.warn("Error while copying artifact to project local repository", e);
}
}
-
- return true;
- } catch (IOException e) {
- LOGGER.warn(
- "An I/O error occurred while checking if the packaged artifact is up-to-date "
- + "against the build output directory. "
- + "Continuing with the assumption that it is up-to-date.",
- e);
- return true;
}
}
- private boolean hasBeenPackagedDuringThisSession(MavenProject project) {
- return project.hasLifecyclePhase("package")
- || project.hasLifecyclePhase("install")
- || project.hasLifecyclePhase("deploy");
+ private Path getArtifactPath(Artifact artifact) {
+ String groupId = artifact.getGroupId();
+ String artifactId = artifact.getArtifactId();
+ String classifier = artifact.getClassifier();
+ String extension = artifact.getExtension();
+ Path root = session.getRequest().getMultiModuleProjectDirectory().toPath();
+ Path repo = root.resolve("target/project-local-repo");
Review Comment:
I think it will be safer as
```
root.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] mthmulders commented on a diff in pull request #912: [MNG-7629] Fix resolution of reactor dependencies
Posted by GitBox <gi...@apache.org>.
mthmulders commented on code in PR #912:
URL: https://github.com/apache/maven/pull/912#discussion_r1051659869
##########
maven-core/src/main/java/org/apache/maven/ReactorReader.java:
##########
@@ -139,187 +132,75 @@ private File find(MavenProject project, Artifact artifact) {
return project.getFile();
}
- Artifact projectArtifact = findMatchingArtifact(project, artifact);
- File packagedArtifactFile = determinePreviouslyPackagedArtifactFile(project, projectArtifact);
-
- if (hasArtifactFileFromPackagePhase(projectArtifact)) {
- return projectArtifact.getFile();
- }
- // Check whether an earlier Maven run might have produced an artifact that is still on disk.
- else if (packagedArtifactFile != null
- && packagedArtifactFile.exists()
- && isPackagedArtifactUpToDate(project, packagedArtifactFile, artifact)) {
- return packagedArtifactFile;
- } else if (!hasBeenPackagedDuringThisSession(project)) {
- // fallback to loose class files only if artifacts haven't been packaged yet
- // and only for plain old jars. Not war files, not ear files, not anything else.
- return determineBuildOutputDirectoryForArtifact(project, artifact);
- }
-
- // The fall-through indicates that the artifact cannot be found;
- // for instance if package produced nothing or classifier problems.
- return null;
+ Path target = getArtifactPath(artifact);
+ return Files.isRegularFile(target) ? target.toFile() : null;
}
- private File determineBuildOutputDirectoryForArtifact(final MavenProject project, final Artifact artifact) {
- if (isTestArtifact(artifact)) {
- if (project.hasLifecyclePhase("test-compile")) {
- return new File(project.getBuild().getTestOutputDirectory());
- }
- } else {
- String type = artifact.getProperty("type", "");
- File outputDirectory = new File(project.getBuild().getOutputDirectory());
+ public void processProject(MavenProject project) {
+ List<Artifact> artifacts = new ArrayList<>();
- // Check if the project is being built during this session, and if we can expect any output.
- // There is no need to check if the build has created any outputs, see MNG-2222.
- boolean projectCompiledDuringThisSession =
- project.hasLifecyclePhase("compile") && COMPILE_PHASE_TYPES.contains(type);
-
- // Check if the project is part of the session (not filtered by -pl, -rf, etc). If so, we check
- // if a possible earlier Maven invocation produced some output for that project which we can use.
- boolean projectHasOutputFromPreviousSession =
- !session.getProjects().contains(project) && outputDirectory.exists();
-
- if (projectHasOutputFromPreviousSession || projectCompiledDuringThisSession) {
- return outputDirectory;
- }
+ artifacts.add(RepositoryUtils.toArtifact(new ProjectArtifact(project)));
+ if (!"pom".equals(project.getPackaging())) {
+ org.apache.maven.artifact.Artifact mavenMainArtifact = project.getArtifact();
+ artifacts.add(RepositoryUtils.toArtifact(mavenMainArtifact));
}
-
- // The fall-through indicates that the artifact cannot be found;
- // for instance if package produced nothing or classifier problems.
- return null;
- }
-
- private File determinePreviouslyPackagedArtifactFile(MavenProject project, Artifact artifact) {
- if (artifact == null) {
- return null;
+ for (org.apache.maven.artifact.Artifact attached : project.getAttachedArtifacts()) {
+ artifacts.add(RepositoryUtils.toArtifact(attached));
}
- String fileName = String.format("%s.%s", project.getBuild().getFinalName(), artifact.getExtension());
- return new File(project.getBuild().getDirectory(), fileName);
- }
-
- private boolean hasArtifactFileFromPackagePhase(Artifact projectArtifact) {
- return projectArtifact != null
- && projectArtifact.getFile() != null
- && projectArtifact.getFile().exists();
- }
-
- private boolean isPackagedArtifactUpToDate(MavenProject project, File packagedArtifactFile, Artifact artifact) {
- Path outputDirectory = Paths.get(project.getBuild().getOutputDirectory());
- if (!outputDirectory.toFile().exists()) {
- return true;
- }
-
- try (Stream<Path> outputFiles = Files.walk(outputDirectory)) {
- // Not using File#lastModified() to avoid a Linux JDK8 milliseconds precision bug: JDK-8177809.
- long artifactLastModified =
- Files.getLastModifiedTime(packagedArtifactFile.toPath()).toMillis();
-
- if (session.getProjectBuildingRequest().getBuildStartTime() != null) {
- long buildStartTime =
- session.getProjectBuildingRequest().getBuildStartTime().getTime();
- if (artifactLastModified > buildStartTime) {
- return true;
- }
- }
-
- Iterator<Path> iterator = outputFiles.iterator();
- while (iterator.hasNext()) {
- Path outputFile = iterator.next();
-
- if (Files.isDirectory(outputFile)) {
- continue;
- }
-
- long outputFileLastModified =
- Files.getLastModifiedTime(outputFile).toMillis();
- if (outputFileLastModified > artifactLastModified) {
- File alternative = determineBuildOutputDirectoryForArtifact(project, artifact);
- if (alternative != null) {
- LOGGER.warn(
- "File '{}' is more recent than the packaged artifact for '{}'; using '{}' instead",
- relativizeOutputFile(outputFile),
- project.getArtifactId(),
- relativizeOutputFile(alternative.toPath()));
- } else {
- LOGGER.warn(
- "File '{}' is more recent than the packaged artifact for '{}'; "
- + "cannot use the build output directory for this type of artifact",
- relativizeOutputFile(outputFile),
- project.getArtifactId());
- }
- return false;
+ for (Artifact artifact : artifacts) {
+ if (artifact.getFile() != null && artifact.getFile().isFile()) {
+ Path target = getArtifactPath(artifact);
+ try {
+ Files.createDirectories(target.getParent());
+ Files.copy(artifact.getFile().toPath(), target, StandardCopyOption.REPLACE_EXISTING);
Review Comment:
I was thinking about 'info', but I'm not sure about it and your question made me rethink.
Thinking out loud... Is it meaningful to the user? Not sure. Is it meaningful to Maven developers? Pretty sure. So from that POV, I think 'debug' would be the best option.
--
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 #912: [MNG-7629] Improve resolution of modules within a multi-module build
Posted by GitBox <gi...@apache.org>.
gnodet commented on PR #912:
URL: https://github.com/apache/maven/pull/912#issuecomment-1382845069
Closing this PR in favour of #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 a diff in pull request #912: [MNG-7629] Fix resolution of reactor dependencies
Posted by GitBox <gi...@apache.org>.
gnodet commented on code in PR #912:
URL: https://github.com/apache/maven/pull/912#discussion_r1051658567
##########
maven-core/src/main/java/org/apache/maven/ReactorReader.java:
##########
@@ -139,187 +132,75 @@ private File find(MavenProject project, Artifact artifact) {
return project.getFile();
}
- Artifact projectArtifact = findMatchingArtifact(project, artifact);
- File packagedArtifactFile = determinePreviouslyPackagedArtifactFile(project, projectArtifact);
-
- if (hasArtifactFileFromPackagePhase(projectArtifact)) {
- return projectArtifact.getFile();
- }
- // Check whether an earlier Maven run might have produced an artifact that is still on disk.
- else if (packagedArtifactFile != null
- && packagedArtifactFile.exists()
- && isPackagedArtifactUpToDate(project, packagedArtifactFile, artifact)) {
- return packagedArtifactFile;
- } else if (!hasBeenPackagedDuringThisSession(project)) {
- // fallback to loose class files only if artifacts haven't been packaged yet
- // and only for plain old jars. Not war files, not ear files, not anything else.
- return determineBuildOutputDirectoryForArtifact(project, artifact);
- }
-
- // The fall-through indicates that the artifact cannot be found;
- // for instance if package produced nothing or classifier problems.
- return null;
+ Path target = getArtifactPath(artifact);
+ return Files.isRegularFile(target) ? target.toFile() : null;
}
- private File determineBuildOutputDirectoryForArtifact(final MavenProject project, final Artifact artifact) {
- if (isTestArtifact(artifact)) {
- if (project.hasLifecyclePhase("test-compile")) {
- return new File(project.getBuild().getTestOutputDirectory());
- }
- } else {
- String type = artifact.getProperty("type", "");
- File outputDirectory = new File(project.getBuild().getOutputDirectory());
+ public void processProject(MavenProject project) {
+ List<Artifact> artifacts = new ArrayList<>();
- // Check if the project is being built during this session, and if we can expect any output.
- // There is no need to check if the build has created any outputs, see MNG-2222.
- boolean projectCompiledDuringThisSession =
- project.hasLifecyclePhase("compile") && COMPILE_PHASE_TYPES.contains(type);
-
- // Check if the project is part of the session (not filtered by -pl, -rf, etc). If so, we check
- // if a possible earlier Maven invocation produced some output for that project which we can use.
- boolean projectHasOutputFromPreviousSession =
- !session.getProjects().contains(project) && outputDirectory.exists();
-
- if (projectHasOutputFromPreviousSession || projectCompiledDuringThisSession) {
- return outputDirectory;
- }
+ artifacts.add(RepositoryUtils.toArtifact(new ProjectArtifact(project)));
+ if (!"pom".equals(project.getPackaging())) {
+ org.apache.maven.artifact.Artifact mavenMainArtifact = project.getArtifact();
+ artifacts.add(RepositoryUtils.toArtifact(mavenMainArtifact));
}
-
- // The fall-through indicates that the artifact cannot be found;
- // for instance if package produced nothing or classifier problems.
- return null;
- }
-
- private File determinePreviouslyPackagedArtifactFile(MavenProject project, Artifact artifact) {
- if (artifact == null) {
- return null;
+ for (org.apache.maven.artifact.Artifact attached : project.getAttachedArtifacts()) {
+ artifacts.add(RepositoryUtils.toArtifact(attached));
}
- String fileName = String.format("%s.%s", project.getBuild().getFinalName(), artifact.getExtension());
- return new File(project.getBuild().getDirectory(), fileName);
- }
-
- private boolean hasArtifactFileFromPackagePhase(Artifact projectArtifact) {
- return projectArtifact != null
- && projectArtifact.getFile() != null
- && projectArtifact.getFile().exists();
- }
-
- private boolean isPackagedArtifactUpToDate(MavenProject project, File packagedArtifactFile, Artifact artifact) {
- Path outputDirectory = Paths.get(project.getBuild().getOutputDirectory());
- if (!outputDirectory.toFile().exists()) {
- return true;
- }
-
- try (Stream<Path> outputFiles = Files.walk(outputDirectory)) {
- // Not using File#lastModified() to avoid a Linux JDK8 milliseconds precision bug: JDK-8177809.
- long artifactLastModified =
- Files.getLastModifiedTime(packagedArtifactFile.toPath()).toMillis();
-
- if (session.getProjectBuildingRequest().getBuildStartTime() != null) {
- long buildStartTime =
- session.getProjectBuildingRequest().getBuildStartTime().getTime();
- if (artifactLastModified > buildStartTime) {
- return true;
- }
- }
-
- Iterator<Path> iterator = outputFiles.iterator();
- while (iterator.hasNext()) {
- Path outputFile = iterator.next();
-
- if (Files.isDirectory(outputFile)) {
- continue;
- }
-
- long outputFileLastModified =
- Files.getLastModifiedTime(outputFile).toMillis();
- if (outputFileLastModified > artifactLastModified) {
- File alternative = determineBuildOutputDirectoryForArtifact(project, artifact);
- if (alternative != null) {
- LOGGER.warn(
- "File '{}' is more recent than the packaged artifact for '{}'; using '{}' instead",
- relativizeOutputFile(outputFile),
- project.getArtifactId(),
- relativizeOutputFile(alternative.toPath()));
- } else {
- LOGGER.warn(
- "File '{}' is more recent than the packaged artifact for '{}'; "
- + "cannot use the build output directory for this type of artifact",
- relativizeOutputFile(outputFile),
- project.getArtifactId());
- }
- return false;
+ for (Artifact artifact : artifacts) {
+ if (artifact.getFile() != null && artifact.getFile().isFile()) {
+ Path target = getArtifactPath(artifact);
+ try {
+ Files.createDirectories(target.getParent());
+ Files.copy(artifact.getFile().toPath(), target, StandardCopyOption.REPLACE_EXISTING);
Review Comment:
At debug level ? Or did you think about log level ?
--
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 #912: [MNG-7629] Fix resolution of reactor dependencies
Posted by GitBox <gi...@apache.org>.
gnodet commented on code in PR #912:
URL: https://github.com/apache/maven/pull/912#discussion_r1051658332
##########
maven-core/src/main/java/org/apache/maven/ReactorReader.java:
##########
@@ -64,28 +63,21 @@
class ReactorReader implements MavenWorkspaceReader {
public static final String HINT = "reactor";
- private static final Collection<String> COMPILE_PHASE_TYPES =
- Arrays.asList("jar", "ejb-client", "war", "rar", "ejb3", "par", "sar", "wsr", "har", "app-client");
-
private static final Logger LOGGER = LoggerFactory.getLogger(ReactorReader.class);
private final MavenSession session;
private final Map<String, MavenProject> projectsByGAV;
private final Map<String, List<MavenProject>> projectsByGA;
private final WorkspaceRepository repository;
- private Function<MavenProject, String> projectIntoKey =
- s -> ArtifactUtils.key(s.getGroupId(), s.getArtifactId(), s.getVersion());
-
- private Function<MavenProject, String> projectIntoVersionlessKey =
- s -> ArtifactUtils.versionlessKey(s.getGroupId(), s.getArtifactId());
-
@Inject
ReactorReader(MavenSession session) {
this.session = session;
- this.projectsByGAV = session.getAllProjects().stream().collect(toMap(projectIntoKey, identity()));
+ this.projectsByGAV = session.getAllProjects().stream()
Review Comment:
It's not. I'll revert this hunk to minimize the changes.
--
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 #912: [MNG-7629] Fix resolution of reactor dependencies
Posted by GitBox <gi...@apache.org>.
gnodet commented on PR #912:
URL: https://github.com/apache/maven/pull/912#issuecomment-1352250822
Some ITs failures are caused by the changes in the new reactor reader. The current behaviour is to allow inter-module dependencies at the `compile` phase. I wonder how this is actually valid ? For example some plugins do modify the _content_ of the artifacts at package phase, such as the `maven-shade-plugin` which can be used to rewrite classes. So the output of the compile phase is not always something that can be consumed. While I do agree that `mvn verify` should work, those tests kinda imply the `mvn compile` should always work, which I think is wrong.
--
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