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 2020/02/16 21:25:59 UTC
[GitHub] [maven] MartinKanters opened a new pull request #329: Mng 5760
resume from without args
MartinKanters opened a new pull request #329: Mng 5760 resume from without args
URL: https://github.com/apache/maven/pull/329
Following this checklist to help us incorporate your
contribution quickly and easily:
- [x ] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/MNG-5760) 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.
- [x] Each commit in the pull request should have a meaningful subject line and body.
- [x] Format the pull request title like `[MNG-XXX] - Fixes bug in ApproximateQuantiles`,
where you replace `MNG-XXX` 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.
- [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [x] Run `mvn clean verify` to make sure basic checks pass. A more thorough check will
be performed on your pull request automatically.
- [x] 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.
- [x] 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)
- [x] In any other case, please file an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
This PR solves MNG-5760 which allows for resuming the build from the last failed module.
It works by saving a file in the build directory of the root pom including that module as file content.
Relevant integration test:
https://github.com/apache/maven-integration-testing/pull/56
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [maven] MartinKanters commented on a change in pull request #329:
[MNG-5760] resume from without args
Posted by GitBox <gi...@apache.org>.
MartinKanters commented on a change in pull request #329: [MNG-5760] resume from without args
URL: https://github.com/apache/maven/pull/329#discussion_r381663751
##########
File path: maven-embedder/src/main/java/org/apache/maven/cli/CLIManager.java
##########
@@ -134,7 +134,7 @@ public CLIManager()
options.addOption( Option.builder( FAIL_FAST ).longOpt( "fail-fast" ).desc( "Stop at first failure in reactorized builds" ).build() );
options.addOption( Option.builder( FAIL_AT_END ).longOpt( "fail-at-end" ).desc( "Only fail the build afterwards; allow all non-impacted builds to continue" ).build() );
options.addOption( Option.builder( FAIL_NEVER ).longOpt( "fail-never" ).desc( "NEVER fail the build, regardless of project result" ).build() );
- options.addOption( Option.builder( RESUME_FROM ).longOpt( "resume-from" ).hasArg().desc( "Resume reactor from specified project" ).build() );
+ options.addOption( Option.builder( RESUME_FROM ).longOpt( "resume-from" ).hasArg().optionalArg( true ).desc( "Resume reactor from last failed project or specified project" ).build() );
Review comment:
'Project' was already there, I've at least wanted to keep it consistent. Let me know if we want module instead, I'll change it gladly
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [maven] MartinKanters commented on issue #329: Mng 5760 resume from
without args
Posted by GitBox <gi...@apache.org>.
MartinKanters commented on issue #329: Mng 5760 resume from without args
URL: https://github.com/apache/maven/pull/329#issuecomment-586756340
I'm interested in knowing your opinions on what the behavior should be when someone tries to run `-rf` (without args) on a project which does not have the cached file there. Currently it fails hard, but I think it might be better to just include all projects in the reactor and have a warning log say that `-rf` did not work.
Also I'm interested in a good place to store the constant for the filename (currently called resume-from-cache) which is currently copied around in a couple places.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [maven] rfscholte commented on a change in pull request #329:
[MNG-5760] resume from without args
Posted by GitBox <gi...@apache.org>.
rfscholte commented on a change in pull request #329: [MNG-5760] resume from without args
URL: https://github.com/apache/maven/pull/329#discussion_r382199516
##########
File path: maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java
##########
@@ -1079,6 +1084,33 @@ private String getResumeFrom( List<MavenProject> mavenProjects, MavenProject fai
return ":" + failedProject.getArtifactId();
}
+ private boolean createResumeFromCacheFile( MavenExecutionResult result, String resumeFromProject )
+ {
+ String buildDirectory = result.getProject().getBuild().getDirectory();
+ File buildDirectoryFile = new File( buildDirectory );
+ boolean isBuildDirectoryPresent = ( buildDirectoryFile.exists() || buildDirectoryFile.mkdirs() );
+ if ( isBuildDirectoryPresent )
+ {
+ Path resumeFromCache = Paths.get( buildDirectory, "resume-from-cache" );
+ try
+ {
+ Files.write( resumeFromCache, resumeFromProject.getBytes() );
+ return true;
+ }
+ catch ( IOException e )
+ {
+ slf4jLogger.debug( "Failed writing last failed project to resume-from-cache file at: {}",
+ resumeFromCache, e );
+ return false;
+ }
+ }
+ else
+ {
+ slf4jLogger.debug( "Failed creating build directory for root project to store resume-from-cache file" );
Review comment:
If we say that it should be possible to run a build without warnings, then this message should be info, because it doesn't effect the build itself, only the efficiency of the (next) build. (I know this message is only shown after an error, but still...)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [maven] MartinKanters commented on a change in pull request #329:
[MNG-5760] resume from without args
Posted by GitBox <gi...@apache.org>.
MartinKanters commented on a change in pull request #329: [MNG-5760] resume from without args
URL: https://github.com/apache/maven/pull/329#discussion_r381664122
##########
File path: maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java
##########
@@ -1079,6 +1084,33 @@ private String getResumeFrom( List<MavenProject> mavenProjects, MavenProject fai
return ":" + failedProject.getArtifactId();
}
+ private boolean createResumeFromCacheFile( MavenExecutionResult result, String resumeFromProject )
+ {
+ String buildDirectory = result.getProject().getBuild().getDirectory();
+ File buildDirectoryFile = new File( buildDirectory );
+ boolean isBuildDirectoryPresent = ( buildDirectoryFile.exists() || buildDirectoryFile.mkdirs() );
+ if ( isBuildDirectoryPresent )
+ {
+ Path resumeFromCache = Paths.get( buildDirectory, "resume-from-cache" );
+ try
+ {
+ Files.write( resumeFromCache, resumeFromProject.getBytes() );
+ return true;
+ }
+ catch ( IOException e )
+ {
+ slf4jLogger.debug( "Failed writing last failed project to resume-from-cache file at: {}",
+ resumeFromCache, e );
+ return false;
+ }
+ }
+ else
+ {
+ slf4jLogger.debug( "Failed creating build directory for root project to store resume-from-cache file" );
Review comment:
Sure, I was doubting about it. Anyway, I made them warnings now. @rfscholte do you think this is the proper logging severity?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [maven] rfscholte commented on a change in pull request #329:
[MNG-5760] resume from without args
Posted by GitBox <gi...@apache.org>.
rfscholte commented on a change in pull request #329: [MNG-5760] resume from without args
URL: https://github.com/apache/maven/pull/329#discussion_r382200097
##########
File path: maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java
##########
@@ -1079,6 +1084,33 @@ private String getResumeFrom( List<MavenProject> mavenProjects, MavenProject fai
return ":" + failedProject.getArtifactId();
}
+ private boolean createResumeFromCacheFile( MavenExecutionResult result, String resumeFromProject )
+ {
+ String buildDirectory = result.getProject().getBuild().getDirectory();
+ File buildDirectoryFile = new File( buildDirectory );
+ boolean isBuildDirectoryPresent = ( buildDirectoryFile.exists() || buildDirectoryFile.mkdirs() );
+ if ( isBuildDirectoryPresent )
+ {
+ Path resumeFromCache = Paths.get( buildDirectory, "resume-from-cache" );
+ try
+ {
+ Files.write( resumeFromCache, resumeFromProject.getBytes() );
Review comment:
Maven doesn't have a json parser. Properties would make more sense here.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [maven] MartinKanters commented on a change in pull request #329:
[MNG-5760] resume from without args
Posted by GitBox <gi...@apache.org>.
MartinKanters commented on a change in pull request #329: [MNG-5760] resume from without args
URL: https://github.com/apache/maven/pull/329#discussion_r381662939
##########
File path: maven-core/src/main/java/org/apache/maven/execution/DefaultMavenExecutionRequest.java
##########
@@ -118,6 +118,8 @@
private String resumeFrom;
Review comment:
Alright! I've just changed this behavior.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [maven] rfscholte commented on a change in pull request #329: Mng
5760 resume from without args
Posted by GitBox <gi...@apache.org>.
rfscholte commented on a change in pull request #329: Mng 5760 resume from without args
URL: https://github.com/apache/maven/pull/329#discussion_r380322605
##########
File path: maven-embedder/src/main/java/org/apache/maven/cli/CLIManager.java
##########
@@ -134,7 +134,7 @@ public CLIManager()
options.addOption( Option.builder( FAIL_FAST ).longOpt( "fail-fast" ).desc( "Stop at first failure in reactorized builds" ).build() );
options.addOption( Option.builder( FAIL_AT_END ).longOpt( "fail-at-end" ).desc( "Only fail the build afterwards; allow all non-impacted builds to continue" ).build() );
options.addOption( Option.builder( FAIL_NEVER ).longOpt( "fail-never" ).desc( "NEVER fail the build, regardless of project result" ).build() );
- options.addOption( Option.builder( RESUME_FROM ).longOpt( "resume-from" ).hasArg().desc( "Resume reactor from specified project" ).build() );
+ options.addOption( Option.builder( RESUME_FROM ).longOpt( "resume-from" ).hasArg().optionalArg( true ).desc( "Resume reactor from last failed project or specified project" ).build() );
Review comment:
Module might cause more confusion due to JPMS. However it is a Maven MultiModule project... sorry, didn't answer your question ;)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [maven] MartinKanters commented on a change in pull request #329:
[MNG-5760] resume from without args
Posted by GitBox <gi...@apache.org>.
MartinKanters commented on a change in pull request #329: [MNG-5760] resume from without args
URL: https://github.com/apache/maven/pull/329#discussion_r381663300
##########
File path: maven-core/src/main/java/org/apache/maven/graph/DefaultGraphBuilder.java
##########
@@ -282,15 +287,42 @@ else if ( StringUtils.isNotEmpty( request.getMakeBehavior() ) )
{
List<MavenProject> result = projects;
- if ( StringUtils.isNotEmpty( request.getResumeFrom() ) )
+ if ( StringUtils.isNotEmpty( request.getResumeFrom() ) || request.isResumeFromLastFailedProject() )
{
File reactorDirectory = null;
if ( request.getBaseDirectory() != null )
{
reactorDirectory = new File( request.getBaseDirectory() );
}
- String selector = request.getResumeFrom();
+ String selector;
+
+ if ( StringUtils.isNotEmpty( request.getResumeFrom() ) )
+ {
+ selector = request.getResumeFrom();
+ }
+ else
+ {
+ String buildDirectory = projects.stream()
Review comment:
I have refactored this in a method now, but I called it `getLastFailedProject`. What do you think?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [maven] mthmulders commented on a change in pull request #329: Mng
5760 resume from without args
Posted by GitBox <gi...@apache.org>.
mthmulders commented on a change in pull request #329: Mng 5760 resume from without args
URL: https://github.com/apache/maven/pull/329#discussion_r380069017
##########
File path: maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java
##########
@@ -1079,6 +1084,33 @@ private String getResumeFrom( List<MavenProject> mavenProjects, MavenProject fai
return ":" + failedProject.getArtifactId();
}
+ private boolean createResumeFromCacheFile( MavenExecutionResult result, String resumeFromProject )
+ {
+ String buildDirectory = result.getProject().getBuild().getDirectory();
+ File buildDirectoryFile = new File( buildDirectory );
+ boolean isBuildDirectoryPresent = ( buildDirectoryFile.exists() || buildDirectoryFile.mkdirs() );
+ if ( isBuildDirectoryPresent )
+ {
+ Path resumeFromCache = Paths.get( buildDirectory, "resume-from-cache" );
+ try
+ {
+ Files.write( resumeFromCache, resumeFromProject.getBytes() );
+ return true;
+ }
+ catch ( IOException e )
+ {
+ slf4jLogger.debug( "Failed writing last failed project to resume-from-cache file at: {}",
+ resumeFromCache, e );
+ return false;
+ }
+ }
+ else
+ {
+ slf4jLogger.debug( "Failed creating build directory for root project to store resume-from-cache file" );
Review comment:
Wouldn't it make sense to use a little more severe message? Most uses will not see them, so they will not know they are missing out on this great new feature :-)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [maven] mthmulders commented on issue #329: Mng 5760 resume from
without args
Posted by GitBox <gi...@apache.org>.
mthmulders commented on issue #329: Mng 5760 resume from without args
URL: https://github.com/apache/maven/pull/329#issuecomment-586899735
I'm impressed by how you implemented this feature, @MartinKanters. Nice work!
As for the file name, you might be able to introduce a constant for it. I'm all for failing "hard" when a user does `-rf` without a selector. It's an invalid use of the switch.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [maven] mthmulders commented on a change in pull request #329: Mng
5760 resume from without args
Posted by GitBox <gi...@apache.org>.
mthmulders commented on a change in pull request #329: Mng 5760 resume from without args
URL: https://github.com/apache/maven/pull/329#discussion_r380066969
##########
File path: maven-core/src/main/java/org/apache/maven/graph/DefaultGraphBuilder.java
##########
@@ -282,15 +287,42 @@ else if ( StringUtils.isNotEmpty( request.getMakeBehavior() ) )
{
List<MavenProject> result = projects;
- if ( StringUtils.isNotEmpty( request.getResumeFrom() ) )
+ if ( StringUtils.isNotEmpty( request.getResumeFrom() ) || request.isResumeFromLastFailedProject() )
{
File reactorDirectory = null;
if ( request.getBaseDirectory() != null )
{
reactorDirectory = new File( request.getBaseDirectory() );
}
- String selector = request.getResumeFrom();
+ String selector;
+
+ if ( StringUtils.isNotEmpty( request.getResumeFrom() ) )
+ {
+ selector = request.getResumeFrom();
+ }
+ else
+ {
+ String buildDirectory = projects.stream()
Review comment:
I would suggest to refactor this into a dedicated method, something like `determineWhereToResumeBuild()`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [maven] MartinKanters commented on a change in pull request #329:
[MNG-5760] resume from without args
Posted by GitBox <gi...@apache.org>.
MartinKanters commented on a change in pull request #329: [MNG-5760] resume from without args
URL: https://github.com/apache/maven/pull/329#discussion_r382211868
##########
File path: maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java
##########
@@ -1079,6 +1084,33 @@ private String getResumeFrom( List<MavenProject> mavenProjects, MavenProject fai
return ":" + failedProject.getArtifactId();
}
+ private boolean createResumeFromCacheFile( MavenExecutionResult result, String resumeFromProject )
+ {
+ String buildDirectory = result.getProject().getBuild().getDirectory();
+ File buildDirectoryFile = new File( buildDirectory );
+ boolean isBuildDirectoryPresent = ( buildDirectoryFile.exists() || buildDirectoryFile.mkdirs() );
+ if ( isBuildDirectoryPresent )
+ {
+ Path resumeFromCache = Paths.get( buildDirectory, "resume-from-cache" );
+ try
+ {
+ Files.write( resumeFromCache, resumeFromProject.getBytes() );
+ return true;
+ }
+ catch ( IOException e )
+ {
+ slf4jLogger.debug( "Failed writing last failed project to resume-from-cache file at: {}",
+ resumeFromCache, e );
+ return false;
+ }
+ }
+ else
+ {
+ slf4jLogger.debug( "Failed creating build directory for root project to store resume-from-cache file" );
Review comment:
I like that reasoning, I'll change it to info then!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [maven] MartinKanters commented on a change in pull request #329:
[MNG-5760] resume from without args
Posted by GitBox <gi...@apache.org>.
MartinKanters commented on a change in pull request #329: [MNG-5760] resume from without args
URL: https://github.com/apache/maven/pull/329#discussion_r382212314
##########
File path: maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java
##########
@@ -1079,6 +1084,33 @@ private String getResumeFrom( List<MavenProject> mavenProjects, MavenProject fai
return ":" + failedProject.getArtifactId();
}
+ private boolean createResumeFromCacheFile( MavenExecutionResult result, String resumeFromProject )
+ {
+ String buildDirectory = result.getProject().getBuild().getDirectory();
+ File buildDirectoryFile = new File( buildDirectory );
+ boolean isBuildDirectoryPresent = ( buildDirectoryFile.exists() || buildDirectoryFile.mkdirs() );
+ if ( isBuildDirectoryPresent )
+ {
+ Path resumeFromCache = Paths.get( buildDirectory, "resume-from-cache" );
+ try
+ {
+ Files.write( resumeFromCache, resumeFromProject.getBytes() );
Review comment:
Ok, but do you think we should implement this already, or only when we need more data there?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [maven] mthmulders commented on a change in pull request #329: Mng
5760 resume from without args
Posted by GitBox <gi...@apache.org>.
mthmulders commented on a change in pull request #329: Mng 5760 resume from without args
URL: https://github.com/apache/maven/pull/329#discussion_r380067529
##########
File path: maven-embedder/src/main/java/org/apache/maven/cli/CLIManager.java
##########
@@ -134,7 +134,7 @@ public CLIManager()
options.addOption( Option.builder( FAIL_FAST ).longOpt( "fail-fast" ).desc( "Stop at first failure in reactorized builds" ).build() );
options.addOption( Option.builder( FAIL_AT_END ).longOpt( "fail-at-end" ).desc( "Only fail the build afterwards; allow all non-impacted builds to continue" ).build() );
options.addOption( Option.builder( FAIL_NEVER ).longOpt( "fail-never" ).desc( "NEVER fail the build, regardless of project result" ).build() );
- options.addOption( Option.builder( RESUME_FROM ).longOpt( "resume-from" ).hasArg().desc( "Resume reactor from specified project" ).build() );
+ options.addOption( Option.builder( RESUME_FROM ).longOpt( "resume-from" ).hasArg().optionalArg( true ).desc( "Resume reactor from last failed project or specified project" ).build() );
Review comment:
@rfscholte, should the message read "last failed **project**" or "last failed **module**"? I still find myself struggling with terminology every now and then...
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [maven] mthmulders commented on a change in pull request #329: Mng
5760 resume from without args
Posted by GitBox <gi...@apache.org>.
mthmulders commented on a change in pull request #329: Mng 5760 resume from without args
URL: https://github.com/apache/maven/pull/329#discussion_r380068389
##########
File path: maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java
##########
@@ -1079,6 +1084,33 @@ private String getResumeFrom( List<MavenProject> mavenProjects, MavenProject fai
return ":" + failedProject.getArtifactId();
}
+ private boolean createResumeFromCacheFile( MavenExecutionResult result, String resumeFromProject )
+ {
+ String buildDirectory = result.getProject().getBuild().getDirectory();
+ File buildDirectoryFile = new File( buildDirectory );
+ boolean isBuildDirectoryPresent = ( buildDirectoryFile.exists() || buildDirectoryFile.mkdirs() );
Review comment:
Nitpicking: no brackets needed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [maven] mthmulders commented on a change in pull request #329:
[MNG-5760] resume from without args
Posted by GitBox <gi...@apache.org>.
mthmulders commented on a change in pull request #329: [MNG-5760] resume from without args
URL: https://github.com/apache/maven/pull/329#discussion_r382200161
##########
File path: maven-embedder/src/main/java/org/apache/maven/cli/CLIManager.java
##########
@@ -134,7 +134,7 @@ public CLIManager()
options.addOption( Option.builder( FAIL_FAST ).longOpt( "fail-fast" ).desc( "Stop at first failure in reactorized builds" ).build() );
options.addOption( Option.builder( FAIL_AT_END ).longOpt( "fail-at-end" ).desc( "Only fail the build afterwards; allow all non-impacted builds to continue" ).build() );
options.addOption( Option.builder( FAIL_NEVER ).longOpt( "fail-never" ).desc( "NEVER fail the build, regardless of project result" ).build() );
- options.addOption( Option.builder( RESUME_FROM ).longOpt( "resume-from" ).hasArg().desc( "Resume reactor from specified project" ).build() );
+ options.addOption( Option.builder( RESUME_FROM ).longOpt( "resume-from" ).hasArg().optionalArg( true ).desc( "Resume reactor from last failed project or specified project" ).build() );
Review comment:
In that case I would vote to stay with "project".
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [maven] rfscholte commented on a change in pull request #329: Mng
5760 resume from without args
Posted by GitBox <gi...@apache.org>.
rfscholte commented on a change in pull request #329: Mng 5760 resume from without args
URL: https://github.com/apache/maven/pull/329#discussion_r380321406
##########
File path: maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java
##########
@@ -1079,6 +1084,33 @@ private String getResumeFrom( List<MavenProject> mavenProjects, MavenProject fai
return ":" + failedProject.getArtifactId();
}
+ private boolean createResumeFromCacheFile( MavenExecutionResult result, String resumeFromProject )
+ {
+ String buildDirectory = result.getProject().getBuild().getDirectory();
+ File buildDirectoryFile = new File( buildDirectory );
+ boolean isBuildDirectoryPresent = ( buildDirectoryFile.exists() || buildDirectoryFile.mkdirs() );
+ if ( isBuildDirectoryPresent )
+ {
+ Path resumeFromCache = Paths.get( buildDirectory, "resume-from-cache" );
+ try
+ {
+ Files.write( resumeFromCache, resumeFromProject.getBytes() );
Review comment:
Will work for now, but the MavenExecutionRequest might contain more information worth storing
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [maven] rfscholte commented on a change in pull request #329: Mng
5760 resume from without args
Posted by GitBox <gi...@apache.org>.
rfscholte commented on a change in pull request #329: Mng 5760 resume from without args
URL: https://github.com/apache/maven/pull/329#discussion_r379941499
##########
File path: maven-core/src/main/java/org/apache/maven/execution/DefaultMavenExecutionRequest.java
##########
@@ -118,6 +118,8 @@
private String resumeFrom;
Review comment:
I don't think we should split the resume-from into 2 fields.
Just add a clear javadoc saying: `null` is unused, empty unspecified.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [maven] MartinKanters edited a comment on issue #329: Mng 5760
resume from without args
Posted by GitBox <gi...@apache.org>.
MartinKanters edited a comment on issue #329: Mng 5760 resume from without args
URL: https://github.com/apache/maven/pull/329#issuecomment-586756340
I'm interested in knowing your opinions on what the behavior should be when someone tries to run `-rf` (without args) on a project which does not have the cached file there. Currently it fails hard, but I think it might be better to just include all projects in the reactor and have a warning log say that `-rf` did not take effect.
Also I'm interested in a good place to store the constant for the filename (currently called resume-from-cache) which is currently copied around in a couple places.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [maven] MartinKanters commented on a change in pull request #329:
[MNG-5760] resume from without args
Posted by GitBox <gi...@apache.org>.
MartinKanters commented on a change in pull request #329: [MNG-5760] resume from without args
URL: https://github.com/apache/maven/pull/329#discussion_r381664490
##########
File path: maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java
##########
@@ -1079,6 +1084,33 @@ private String getResumeFrom( List<MavenProject> mavenProjects, MavenProject fai
return ":" + failedProject.getArtifactId();
}
+ private boolean createResumeFromCacheFile( MavenExecutionResult result, String resumeFromProject )
+ {
+ String buildDirectory = result.getProject().getBuild().getDirectory();
+ File buildDirectoryFile = new File( buildDirectory );
+ boolean isBuildDirectoryPresent = ( buildDirectoryFile.exists() || buildDirectoryFile.mkdirs() );
+ if ( isBuildDirectoryPresent )
+ {
+ Path resumeFromCache = Paths.get( buildDirectory, "resume-from-cache" );
+ try
+ {
+ Files.write( resumeFromCache, resumeFromProject.getBytes() );
Review comment:
Right, do you think we should make it in a more extensible format like json already? Or should we refactor it when we see the need for other bits of info?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [maven] rfscholte commented on a change in pull request #329:
[MNG-5760] resume from without args
Posted by GitBox <gi...@apache.org>.
rfscholte commented on a change in pull request #329: [MNG-5760] resume from without args
URL: https://github.com/apache/maven/pull/329#discussion_r382196607
##########
File path: maven-core/src/main/java/org/apache/maven/graph/DefaultGraphBuilder.java
##########
@@ -282,15 +287,42 @@ else if ( StringUtils.isNotEmpty( request.getMakeBehavior() ) )
{
List<MavenProject> result = projects;
- if ( StringUtils.isNotEmpty( request.getResumeFrom() ) )
+ if ( StringUtils.isNotEmpty( request.getResumeFrom() ) || request.isResumeFromLastFailedProject() )
{
File reactorDirectory = null;
if ( request.getBaseDirectory() != null )
{
reactorDirectory = new File( request.getBaseDirectory() );
}
- String selector = request.getResumeFrom();
+ String selector;
+
+ if ( StringUtils.isNotEmpty( request.getResumeFrom() ) )
+ {
+ selector = request.getResumeFrom();
+ }
+ else
+ {
+ String buildDirectory = projects.stream()
Review comment:
Yes, I prefer the `getLastFailedProject`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services