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/12/30 20:10:24 UTC

[GitHub] [maven] rfscholte opened a new pull request #423: [MNG-7060] Let build fail fast in case any maven-gpg-plugin goal is c…

rfscholte opened a new pull request #423:
URL: https://github.com/apache/maven/pull/423


   …alled
   
   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] - 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.
    - [ ] 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven] khmarbaise commented on a change in pull request #423: [MNG-7060] Let build fail fast in case any maven-gpg-plugin goal is c…

Posted by GitBox <gi...@apache.org>.
khmarbaise commented on a change in pull request #423:
URL: https://github.com/apache/maven/pull/423#discussion_r550321423



##########
File path: maven-core/src/main/java/org/apache/maven/lifecycle/internal/builder/BuilderCommon.java
##########
@@ -105,6 +106,20 @@ public MavenExecutionPlan resolveBuildPlan( MavenSession session, MavenProject p
 
         lifecycleDebugLogger.debugProjectPlan( project, executionPlan );
 
+        // With Maven 4's build/consumer the POM will always rewrite during distribution.
+        // The maven-gpg-plugin uses the original POM, causing an invalid signature.
+        // Fail as long as there's no solution available yet
+        Optional<MojoExecution> gpgMojo = executionPlan.getMojoExecutions().stream()
+                .filter( m -> "org.apache.maven.plugins".equals( m.getGroupId() ) )
+                .filter( m -> "maven-gpg-plugin".equals( m.getArtifactId() ) )
+                .findAny();
+
+        if ( gpgMojo.isPresent() )
+        {

Review comment:
       What about adding a predicate:
   ```java
   private Predicate<MojoExecution> isMavenGpgPlugin = m -> m.getArtifactId().equals(
               "maven-gpg-plugin" ) && m.getGroupId().equals( "org.apache.maven.plugins" );
   ```
   Then you can change the whole filter via the following:
   ```
           boolean gpgMojo = executionPlan.getMojoExecutions().stream().anyMatch(isMavenGpgPlugin);
   
           if ( gpgMojo )
           {
               throw new LifecycleExecutionException( "The maven-gpg-plugin is not supported by Maven 4."
                   + " Verify if there is a compatible signing solution or use Maven 3" );
           }
   ```
   because the optional is only used as boolean.




----------------------------------------------------------------
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



[GitHub] [maven] michael-o commented on pull request #423: [MNG-7060] Let build fail fast in case any maven-gpg-plugin goal is c…

Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #423:
URL: https://github.com/apache/maven/pull/423#issuecomment-752776107


   > 
   > 
   > Good question. You will be fine if build/consumer is disabled. Do you expect this to be added to the message?
   
   But I don't expect to fail if this feature is disabled because the POM is never transformed. If the feature is enabled, the exception should say that this plugin can only be used when this feature is *disabled*.


----------------------------------------------------------------
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



[GitHub] [maven] rfscholte commented on pull request #423: [MNG-7060] Let build fail fast in case any maven-gpg-plugin goal is c…

Posted by GitBox <gi...@apache.org>.
rfscholte commented on pull request #423:
URL: https://github.com/apache/maven/pull/423#issuecomment-753607676


   Merged with https://github.com/apache/maven/commit/94612f96fa3b96ccc69072dc7b141fd0a987262f


----------------------------------------------------------------
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



[GitHub] [maven] rfscholte commented on a change in pull request #423: [MNG-7060] Let build fail fast in case any maven-gpg-plugin goal is c…

Posted by GitBox <gi...@apache.org>.
rfscholte commented on a change in pull request #423:
URL: https://github.com/apache/maven/pull/423#discussion_r550319373



##########
File path: maven-core/src/main/java/org/apache/maven/lifecycle/internal/builder/BuilderCommon.java
##########
@@ -105,6 +106,20 @@ public MavenExecutionPlan resolveBuildPlan( MavenSession session, MavenProject p
 
         lifecycleDebugLogger.debugProjectPlan( project, executionPlan );
 
+        // With Maven 4's build/consumer the POM will always rewrite during distribution.
+        // The maven-gpg-plugin uses the original POM, causing an invalid signature.
+        // Fail as long as there's no solution available yet
+        Optional<MojoExecution> gpgMojo = executionPlan.getMojoExecutions().stream()
+                .filter( m -> "org.apache.maven.plugins".equals( m.getGroupId() ) )

Review comment:
       Sure it can, but IMO this is more readable. The effect is the same.




----------------------------------------------------------------
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



[GitHub] [maven] michael-o commented on pull request #423: [MNG-7060] Let build fail fast in case any maven-gpg-plugin goal is c…

Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #423:
URL: https://github.com/apache/maven/pull/423#issuecomment-752774281


   Should this fail to when build consumer/producer feature is disabled?


----------------------------------------------------------------
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



[GitHub] [maven] slachiewicz commented on pull request #423: [MNG-7060] Let build fail fast in case any maven-gpg-plugin goal is c…

Posted by GitBox <gi...@apache.org>.
slachiewicz commented on pull request #423:
URL: https://github.com/apache/maven/pull/423#issuecomment-753512089


   only one NPE at and of build
   
   `java.lang.NullPointerException: Cannot invoke "org.apache.maven.project.MavenProject.equals(Object)" because "firstFailedProject" is null at org.apache.maven.cli.MavenCli.execute (MavenCli.java:1042) at org.apache.maven.cli.MavenCli.doMain (MavenCli.java:293) at org.apache.maven.cli.MavenCli.main (MavenCli.java:197) at`
   
   https://github.com/apache/maven/blob/538de4d1924ef982df12d32da394e375561a6e19/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java#L1034-L1047
   
   `
   [INFO] ------------------------------------------------------------------------
   [INFO] BUILD FAILURE
   [INFO] ------------------------------------------------------------------------
   [INFO] Total time:  6.503 s
   [INFO] Finished at: 2021-01-02T19:18:15+01:00
   [INFO] ------------------------------------------------------------------------
   [INFO] No failed projects found, resuming the build would not make sense.
   [ERROR] The maven-gpg-plugin is not supported by Maven 4. Verify if there is a compatible signing solution, add -Dmaven.experimental.buildconsumer=false or use Maven 3.
   org.apache.maven.lifecycle.LifecycleExecutionException: The maven-gpg-plugin is not supported by Maven 4. Verify if there is a compatible signing solution, add -Dmaven.experimental.buildconsumer=false or use Maven 3.
       at org.apache.maven.lifecycle.internal.builder.BuilderCommon.resolveBuildPlan (BuilderCommon.java:123)
       at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:109)
       at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:79)
       at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build (SingleThreadedBuilder.java:64)
       at org.apache.maven.lifecycle.internal.LifecycleStarter.execute (LifecycleStarter.java:131)
       at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:321)
       at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:206)
       at org.apache.maven.DefaultMaven.execute (DefaultMaven.java:119)
       at org.apache.maven.cli.MavenCli.execute (MavenCli.java:979)
       at org.apache.maven.cli.MavenCli.doMain (MavenCli.java:293)
       at org.apache.maven.cli.MavenCli.main (MavenCli.java:197)
       at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0 (Native Method)
       at jdk.internal.reflect.NativeMethodAccessorImpl.invoke (NativeMethodAccessorImpl.java:78)
       at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke (DelegatingMethodAccessorImpl.java:43)
       at java.lang.reflect.Method.invoke (Method.java:567)
       at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced (Launcher.java:282)
       at org.codehaus.plexus.classworlds.launcher.Launcher.launch (Launcher.java:225)
       at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode (Launcher.java:406)
       at org.codehaus.plexus.classworlds.launcher.Launcher.main (Launcher.java:347)
   [ERROR]
   [ERROR] Error executing Maven.
   java.lang.NullPointerException: Cannot invoke "org.apache.maven.project.MavenProject.equals(Object)" because "firstFailedProject" is null
       at org.apache.maven.cli.MavenCli.execute (MavenCli.java:1042)
       at org.apache.maven.cli.MavenCli.doMain (MavenCli.java:293)
       at org.apache.maven.cli.MavenCli.main (MavenCli.java:197)
       at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0 (Native Method)
       at jdk.internal.reflect.NativeMethodAccessorImpl.invoke (NativeMethodAccessorImpl.java:78)
       at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke (DelegatingMethodAccessorImpl.java:43)
       at java.lang.reflect.Method.invoke (Method.java:567)
       at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced (Launcher.java:282)
       at org.codehaus.plexus.classworlds.launcher.Launcher.launch (Launcher.java:225)
       at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode (Launcher.java:406)
       at org.codehaus.plexus.classworlds.launcher.Launcher.main (Launcher.java:347)
   `


----------------------------------------------------------------
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



[GitHub] [maven] khmarbaise commented on a change in pull request #423: [MNG-7060] Let build fail fast in case any maven-gpg-plugin goal is c…

Posted by GitBox <gi...@apache.org>.
khmarbaise commented on a change in pull request #423:
URL: https://github.com/apache/maven/pull/423#discussion_r550321423



##########
File path: maven-core/src/main/java/org/apache/maven/lifecycle/internal/builder/BuilderCommon.java
##########
@@ -105,6 +106,20 @@ public MavenExecutionPlan resolveBuildPlan( MavenSession session, MavenProject p
 
         lifecycleDebugLogger.debugProjectPlan( project, executionPlan );
 
+        // With Maven 4's build/consumer the POM will always rewrite during distribution.
+        // The maven-gpg-plugin uses the original POM, causing an invalid signature.
+        // Fail as long as there's no solution available yet
+        Optional<MojoExecution> gpgMojo = executionPlan.getMojoExecutions().stream()
+                .filter( m -> "org.apache.maven.plugins".equals( m.getGroupId() ) )
+                .filter( m -> "maven-gpg-plugin".equals( m.getArtifactId() ) )
+                .findAny();
+
+        if ( gpgMojo.isPresent() )
+        {

Review comment:
       What about adding a predicate:
   ```java
   private Predicate<MojoExecution> isMavenGpgPlugin = m -> m.getArtifactId().equals(
               "maven-gpg-plugin" ) && m.getGroupId().equals( "org.apache.maven.plugins" );
   ```
   Then you can change the whole filter via the following:
   ```java
           boolean gpgMojo = executionPlan.getMojoExecutions().stream().anyMatch(isMavenGpgPlugin);
   
           if ( gpgMojo )
           {
               throw new LifecycleExecutionException( "The maven-gpg-plugin is not supported by Maven 4."
                   + " Verify if there is a compatible signing solution or use Maven 3" );
           }
   ```
   because the optional is only used as boolean.




----------------------------------------------------------------
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



[GitHub] [maven] michael-o commented on a change in pull request #423: [MNG-7060] Let build fail fast in case any maven-gpg-plugin goal is c…

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #423:
URL: https://github.com/apache/maven/pull/423#discussion_r550321326



##########
File path: maven-core/src/main/java/org/apache/maven/lifecycle/internal/builder/BuilderCommon.java
##########
@@ -105,6 +106,20 @@ public MavenExecutionPlan resolveBuildPlan( MavenSession session, MavenProject p
 
         lifecycleDebugLogger.debugProjectPlan( project, executionPlan );
 
+        // With Maven 4's build/consumer the POM will always rewrite during distribution.
+        // The maven-gpg-plugin uses the original POM, causing an invalid signature.
+        // Fail as long as there's no solution available yet
+        Optional<MojoExecution> gpgMojo = executionPlan.getMojoExecutions().stream()
+                .filter( m -> "org.apache.maven.plugins".equals( m.getGroupId() ) )

Review comment:
       Think of it like `if (.. && ..)`.




----------------------------------------------------------------
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



[GitHub] [maven] rfscholte commented on pull request #423: [MNG-7060] Let build fail fast in case any maven-gpg-plugin goal is c…

Posted by GitBox <gi...@apache.org>.
rfscholte commented on pull request #423:
URL: https://github.com/apache/maven/pull/423#issuecomment-752899776


   When 4.0.0 will be released, this code will be touched anyway. I see this as a temporary protection (at least 2  already suffered from this issue without understanding the problem).
   Currently no reason for a test, confirmation that it works is good enough for now.


----------------------------------------------------------------
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



[GitHub] [maven] rfscholte commented on pull request #423: [MNG-7060] Let build fail fast in case any maven-gpg-plugin goal is c…

Posted by GitBox <gi...@apache.org>.
rfscholte commented on pull request #423:
URL: https://github.com/apache/maven/pull/423#issuecomment-753607036


   Looks unrelated, so I'll merge.


----------------------------------------------------------------
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



[GitHub] [maven] michael-o commented on a change in pull request #423: [MNG-7060] Let build fail fast in case any maven-gpg-plugin goal is c…

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #423:
URL: https://github.com/apache/maven/pull/423#discussion_r550326838



##########
File path: maven-core/src/main/java/org/apache/maven/lifecycle/internal/builder/BuilderCommon.java
##########
@@ -105,6 +106,20 @@ public MavenExecutionPlan resolveBuildPlan( MavenSession session, MavenProject p
 
         lifecycleDebugLogger.debugProjectPlan( project, executionPlan );
 
+        // With Maven 4's build/consumer the POM will always rewrite during distribution.
+        // The maven-gpg-plugin uses the original POM, causing an invalid signature.
+        // Fail as long as there's no solution available yet
+        Optional<MojoExecution> gpgMojo = executionPlan.getMojoExecutions().stream()
+                .filter( m -> "org.apache.maven.plugins".equals( m.getGroupId() ) )
+                .filter( m -> "maven-gpg-plugin".equals( m.getArtifactId() ) )
+                .findAny();
+
+        if ( gpgMojo.isPresent() )
+        {

Review comment:
       Let's keep it as simple as possible. Either one or two filters.




----------------------------------------------------------------
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



[GitHub] [maven] rfscholte commented on pull request #423: [MNG-7060] Let build fail fast in case any maven-gpg-plugin goal is c…

Posted by GitBox <gi...@apache.org>.
rfscholte commented on pull request #423:
URL: https://github.com/apache/maven/pull/423#issuecomment-753463579


   I'll wait for confirmation of @slachiewicz 


----------------------------------------------------------------
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



[GitHub] [maven] khmarbaise commented on a change in pull request #423: [MNG-7060] Let build fail fast in case any maven-gpg-plugin goal is c…

Posted by GitBox <gi...@apache.org>.
khmarbaise commented on a change in pull request #423:
URL: https://github.com/apache/maven/pull/423#discussion_r550322416



##########
File path: maven-core/src/main/java/org/apache/maven/lifecycle/internal/builder/BuilderCommon.java
##########
@@ -105,6 +106,20 @@ public MavenExecutionPlan resolveBuildPlan( MavenSession session, MavenProject p
 
         lifecycleDebugLogger.debugProjectPlan( project, executionPlan );
 
+        // With Maven 4's build/consumer the POM will always rewrite during distribution.
+        // The maven-gpg-plugin uses the original POM, causing an invalid signature.
+        // Fail as long as there's no solution available yet
+        Optional<MojoExecution> gpgMojo = executionPlan.getMojoExecutions().stream()
+                .filter( m -> "org.apache.maven.plugins".equals( m.getGroupId() ) )
+                .filter( m -> "maven-gpg-plugin".equals( m.getArtifactId() ) )
+                .findAny();
+
+        if ( gpgMojo.isPresent() )
+        {

Review comment:
       To make more clear the combining of G+A: you could go the path:
   Defining two predicates like:
   ```java
       private Predicate<MojoExecution> isMavenGpgGroupId = m -> m.getGroupId().equals( "org.apache.maven.plugins" );
       private Predicate<MojoExecution> isMavenGpgArtifactId = m -> m.getArtifactId().equals( "maven-gpg-plugin" );
   ```
   and later use it like this:
   ```java
   boolean gpgMojo = executionPlan.getMojoExecutions().stream().anyMatch(isMavenGpgGroupId.and(isMavenGpgArtifactId ));
   ```
   




----------------------------------------------------------------
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



[GitHub] [maven] rfscholte commented on pull request #423: [MNG-7060] Let build fail fast in case any maven-gpg-plugin goal is c…

Posted by GitBox <gi...@apache.org>.
rfscholte commented on pull request #423:
URL: https://github.com/apache/maven/pull/423#issuecomment-752775667


   Good question. You will be fine if build/consumer is disabled. Do you expect this to be added to the message?


----------------------------------------------------------------
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



[GitHub] [maven] michael-o commented on a change in pull request #423: [MNG-7060] Let build fail fast in case any maven-gpg-plugin goal is c…

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #423:
URL: https://github.com/apache/maven/pull/423#discussion_r550315149



##########
File path: maven-core/src/main/java/org/apache/maven/lifecycle/internal/builder/BuilderCommon.java
##########
@@ -105,6 +106,20 @@ public MavenExecutionPlan resolveBuildPlan( MavenSession session, MavenProject p
 
         lifecycleDebugLogger.debugProjectPlan( project, executionPlan );
 
+        // With Maven 4's build/consumer the POM will always rewrite during distribution.
+        // The maven-gpg-plugin uses the original POM, causing an invalid signature.
+        // Fail as long as there's no solution available yet
+        Optional<MojoExecution> gpgMojo = executionPlan.getMojoExecutions().stream()
+                .filter( m -> "org.apache.maven.plugins".equals( m.getGroupId() ) )

Review comment:
       Can't the filter be ANDed?




----------------------------------------------------------------
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



[GitHub] [maven] rfscholte commented on a change in pull request #423: [MNG-7060] Let build fail fast in case any maven-gpg-plugin goal is c…

Posted by GitBox <gi...@apache.org>.
rfscholte commented on a change in pull request #423:
URL: https://github.com/apache/maven/pull/423#discussion_r550335853



##########
File path: maven-core/src/main/java/org/apache/maven/lifecycle/internal/builder/BuilderCommon.java
##########
@@ -105,6 +106,20 @@ public MavenExecutionPlan resolveBuildPlan( MavenSession session, MavenProject p
 
         lifecycleDebugLogger.debugProjectPlan( project, executionPlan );
 
+        // With Maven 4's build/consumer the POM will always rewrite during distribution.
+        // The maven-gpg-plugin uses the original POM, causing an invalid signature.
+        // Fail as long as there's no solution available yet
+        Optional<MojoExecution> gpgMojo = executionPlan.getMojoExecutions().stream()
+                .filter( m -> "org.apache.maven.plugins".equals( m.getGroupId() ) )
+                .filter( m -> "maven-gpg-plugin".equals( m.getArtifactId() ) )
+                .findAny();
+
+        if ( gpgMojo.isPresent() )
+        {

Review comment:
       To me the stream is not complex enough to justify this propsed extraction.




----------------------------------------------------------------
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



[GitHub] [maven] rfscholte commented on a change in pull request #423: [MNG-7060] Let build fail fast in case any maven-gpg-plugin goal is c…

Posted by GitBox <gi...@apache.org>.
rfscholte commented on a change in pull request #423:
URL: https://github.com/apache/maven/pull/423#discussion_r550325388



##########
File path: maven-core/src/main/java/org/apache/maven/lifecycle/internal/builder/BuilderCommon.java
##########
@@ -105,6 +106,20 @@ public MavenExecutionPlan resolveBuildPlan( MavenSession session, MavenProject p
 
         lifecycleDebugLogger.debugProjectPlan( project, executionPlan );
 
+        // With Maven 4's build/consumer the POM will always rewrite during distribution.
+        // The maven-gpg-plugin uses the original POM, causing an invalid signature.
+        // Fail as long as there's no solution available yet
+        Optional<MojoExecution> gpgMojo = executionPlan.getMojoExecutions().stream()
+                .filter( m -> "org.apache.maven.plugins".equals( m.getGroupId() ) )
+                .filter( m -> "maven-gpg-plugin".equals( m.getArtifactId() ) )
+                .findAny();
+
+        if ( gpgMojo.isPresent() )
+        {

Review comment:
       So yes, there are many options to write this. My style would be: 1 simple statement per filter as it would reduce complexity (TT, TF, FT, FF vs T->T, T->F, F) and no externalized predicates if the statement is short and clear. 




----------------------------------------------------------------
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



[GitHub] [maven] khmarbaise commented on a change in pull request #423: [MNG-7060] Let build fail fast in case any maven-gpg-plugin goal is c…

Posted by GitBox <gi...@apache.org>.
khmarbaise commented on a change in pull request #423:
URL: https://github.com/apache/maven/pull/423#discussion_r550321627



##########
File path: maven-core/src/main/java/org/apache/maven/lifecycle/internal/builder/BuilderCommon.java
##########
@@ -105,6 +106,20 @@ public MavenExecutionPlan resolveBuildPlan( MavenSession session, MavenProject p
 
         lifecycleDebugLogger.debugProjectPlan( project, executionPlan );
 
+        // With Maven 4's build/consumer the POM will always rewrite during distribution.
+        // The maven-gpg-plugin uses the original POM, causing an invalid signature.
+        // Fail as long as there's no solution available yet
+        Optional<MojoExecution> gpgMojo = executionPlan.getMojoExecutions().stream()
+                .filter( m -> "org.apache.maven.plugins".equals( m.getGroupId() ) )
+                .filter( m -> "maven-gpg-plugin".equals( m.getArtifactId() ) )
+                .findAny();
+
+        if ( gpgMojo.isPresent() )
+        {

Review comment:
       You might also inline the `gpgMojo` into the if-Statement.




----------------------------------------------------------------
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



[GitHub] [maven] michael-o commented on a change in pull request #423: [MNG-7060] Let build fail fast in case any maven-gpg-plugin goal is c…

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #423:
URL: https://github.com/apache/maven/pull/423#discussion_r550349032



##########
File path: maven-core/src/main/java/org/apache/maven/lifecycle/internal/builder/BuilderCommon.java
##########
@@ -108,15 +109,20 @@ public MavenExecutionPlan resolveBuildPlan( MavenSession session, MavenProject p
         // With Maven 4's build/consumer the POM will always rewrite during distribution.
         // The maven-gpg-plugin uses the original POM, causing an invalid signature.
         // Fail as long as there's no solution available yet
-        Optional<MojoExecution> gpgMojo = executionPlan.getMojoExecutions().stream()
-                .filter( m -> "maven-gpg-plugin".equals( m.getArtifactId() ) 
-                           && "org.apache.maven.plugins".equals( m.getGroupId() ) )
-                .findAny();
-
-        if ( gpgMojo.isPresent() )
+        if ( Features.buildConsumer().isActive() )
         {
-            throw new LifecycleExecutionException( "The maven-gpg-plugin is not supported by Maven 4."
-                + " Verify if there is a compatible signing solution or use Maven 3" );
+            Optional<MojoExecution> gpgMojo = executionPlan.getMojoExecutions().stream()
+                            .filter( m -> "maven-gpg-plugin".equals( m.getArtifactId() ) 
+                                       && "org.apache.maven.plugins".equals( m.getGroupId() ) )
+                            .findAny();
+
+            if ( gpgMojo.isPresent() )
+            {
+                throw new LifecycleExecutionException( "The maven-gpg-plugin is not supported by Maven 4."
+                    + " Verify if there is a compatible signing solution"
+                    + " or add -D" + Features.buildConsumer().propertyName() + "=false"
+                    + " or use Maven 3" );

Review comment:
       Sentence ends here. Add a full stop.

##########
File path: maven-core/src/main/java/org/apache/maven/lifecycle/internal/builder/BuilderCommon.java
##########
@@ -108,15 +109,20 @@ public MavenExecutionPlan resolveBuildPlan( MavenSession session, MavenProject p
         // With Maven 4's build/consumer the POM will always rewrite during distribution.
         // The maven-gpg-plugin uses the original POM, causing an invalid signature.
         // Fail as long as there's no solution available yet
-        Optional<MojoExecution> gpgMojo = executionPlan.getMojoExecutions().stream()
-                .filter( m -> "maven-gpg-plugin".equals( m.getArtifactId() ) 
-                           && "org.apache.maven.plugins".equals( m.getGroupId() ) )
-                .findAny();
-
-        if ( gpgMojo.isPresent() )
+        if ( Features.buildConsumer().isActive() )
         {
-            throw new LifecycleExecutionException( "The maven-gpg-plugin is not supported by Maven 4."
-                + " Verify if there is a compatible signing solution or use Maven 3" );
+            Optional<MojoExecution> gpgMojo = executionPlan.getMojoExecutions().stream()
+                            .filter( m -> "maven-gpg-plugin".equals( m.getArtifactId() ) 
+                                       && "org.apache.maven.plugins".equals( m.getGroupId() ) )
+                            .findAny();
+
+            if ( gpgMojo.isPresent() )
+            {
+                throw new LifecycleExecutionException( "The maven-gpg-plugin is not supported by Maven 4."
+                    + " Verify if there is a compatible signing solution"
+                    + " or add -D" + Features.buildConsumer().propertyName() + "=false"

Review comment:
       the first "or" should be replaced with a comma.




----------------------------------------------------------------
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



[GitHub] [maven] michael-o commented on pull request #423: [MNG-7060] Let build fail fast in case any maven-gpg-plugin goal is c…

Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #423:
URL: https://github.com/apache/maven/pull/423#issuecomment-753214610


   I am good to merge this one now.


----------------------------------------------------------------
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



[GitHub] [maven] slachiewicz edited a comment on pull request #423: [MNG-7060] Let build fail fast in case any maven-gpg-plugin goal is c…

Posted by GitBox <gi...@apache.org>.
slachiewicz edited a comment on pull request #423:
URL: https://github.com/apache/maven/pull/423#issuecomment-753512089


   only one NPE at and of build
   
   `java.lang.NullPointerException: Cannot invoke "org.apache.maven.project.MavenProject.equals(Object)" because "firstFailedProject" is null at org.apache.maven.cli.MavenCli.execute (MavenCli.java:1042) at org.apache.maven.cli.MavenCli.doMain (MavenCli.java:293) at org.apache.maven.cli.MavenCli.main (MavenCli.java:197) at`
   
   https://github.com/apache/maven/blob/538de4d1924ef982df12d32da394e375561a6e19/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java#L1034-L1047
   
   ```
   [INFO] ------------------------------------------------------------------------
   [INFO] BUILD FAILURE
   [INFO] ------------------------------------------------------------------------
   [INFO] Total time:  6.503 s
   [INFO] Finished at: 2021-01-02T19:18:15+01:00
   [INFO] ------------------------------------------------------------------------
   [INFO] No failed projects found, resuming the build would not make sense.
   [ERROR] The maven-gpg-plugin is not supported by Maven 4. Verify if there is a compatible signing solution, add -Dmaven.experimental.buildconsumer=false or use Maven 3.
   org.apache.maven.lifecycle.LifecycleExecutionException: The maven-gpg-plugin is not supported by Maven 4. Verify if there is a compatible signing solution, add -Dmaven.experimental.buildconsumer=false or use Maven 3.
       at org.apache.maven.lifecycle.internal.builder.BuilderCommon.resolveBuildPlan (BuilderCommon.java:123)
       at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:109)
       at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:79)
       at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build (SingleThreadedBuilder.java:64)
       at org.apache.maven.lifecycle.internal.LifecycleStarter.execute (LifecycleStarter.java:131)
       at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:321)
       at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:206)
       at org.apache.maven.DefaultMaven.execute (DefaultMaven.java:119)
       at org.apache.maven.cli.MavenCli.execute (MavenCli.java:979)
       at org.apache.maven.cli.MavenCli.doMain (MavenCli.java:293)
       at org.apache.maven.cli.MavenCli.main (MavenCli.java:197)
       at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0 (Native Method)
       at jdk.internal.reflect.NativeMethodAccessorImpl.invoke (NativeMethodAccessorImpl.java:78)
       at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke (DelegatingMethodAccessorImpl.java:43)
       at java.lang.reflect.Method.invoke (Method.java:567)
       at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced (Launcher.java:282)
       at org.codehaus.plexus.classworlds.launcher.Launcher.launch (Launcher.java:225)
       at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode (Launcher.java:406)
       at org.codehaus.plexus.classworlds.launcher.Launcher.main (Launcher.java:347)
   [ERROR]
   [ERROR] Error executing Maven.
   java.lang.NullPointerException: Cannot invoke "org.apache.maven.project.MavenProject.equals(Object)" because "firstFailedProject" is null
       at org.apache.maven.cli.MavenCli.execute (MavenCli.java:1042)
       at org.apache.maven.cli.MavenCli.doMain (MavenCli.java:293)
       at org.apache.maven.cli.MavenCli.main (MavenCli.java:197)
       at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0 (Native Method)
       at jdk.internal.reflect.NativeMethodAccessorImpl.invoke (NativeMethodAccessorImpl.java:78)
       at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke (DelegatingMethodAccessorImpl.java:43)
       at java.lang.reflect.Method.invoke (Method.java:567)
       at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced (Launcher.java:282)
       at org.codehaus.plexus.classworlds.launcher.Launcher.launch (Launcher.java:225)
       at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode (Launcher.java:406)
       at org.codehaus.plexus.classworlds.launcher.Launcher.main (Launcher.java:347)
   ```


----------------------------------------------------------------
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



[GitHub] [maven] khmarbaise commented on a change in pull request #423: [MNG-7060] Let build fail fast in case any maven-gpg-plugin goal is c…

Posted by GitBox <gi...@apache.org>.
khmarbaise commented on a change in pull request #423:
URL: https://github.com/apache/maven/pull/423#discussion_r550328822



##########
File path: maven-core/src/main/java/org/apache/maven/lifecycle/internal/builder/BuilderCommon.java
##########
@@ -105,6 +106,20 @@ public MavenExecutionPlan resolveBuildPlan( MavenSession session, MavenProject p
 
         lifecycleDebugLogger.debugProjectPlan( project, executionPlan );
 
+        // With Maven 4's build/consumer the POM will always rewrite during distribution.
+        // The maven-gpg-plugin uses the original POM, causing an invalid signature.
+        // Fail as long as there's no solution available yet
+        Optional<MojoExecution> gpgMojo = executionPlan.getMojoExecutions().stream()
+                .filter( m -> "org.apache.maven.plugins".equals( m.getGroupId() ) )
+                .filter( m -> "maven-gpg-plugin".equals( m.getArtifactId() ) )
+                .findAny();
+
+        if ( gpgMojo.isPresent() )
+        {

Review comment:
       A separate predicate expresses better what you really mean because it gives it a readable name.
   I prefer this one:
   ```java
   boolean gpgMojo = executionPlan.getMojoExecutions().stream().anyMatch(MavenGpgPlugin);
   ```
   Can't be clearer from my POV... If you like more details jump to the predicate via IDE.
   
   Complexity? If you use stream API predicates are more less everywhere. Usually if a method is to long (too complex) you refactor out a part of it with a readable name. Here it's the same.
   
   Furthermore combined filters don't give you the chance to use `anyMatch(..)`, `.allMatch(..)` or `noneMatch(..)` which is often a good choice and often not used based on the usage of chaining several filter(..)...
   




----------------------------------------------------------------
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



[GitHub] [maven] slachiewicz commented on pull request #423: [MNG-7060] Let build fail fast in case any maven-gpg-plugin goal is c…

Posted by GitBox <gi...@apache.org>.
slachiewicz commented on pull request #423:
URL: https://github.com/apache/maven/pull/423#issuecomment-752768758


   Cool, let me try to test it again, releasing something ;-)


----------------------------------------------------------------
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



[GitHub] [maven] rfscholte closed pull request #423: [MNG-7060] Let build fail fast in case any maven-gpg-plugin goal is c…

Posted by GitBox <gi...@apache.org>.
rfscholte closed pull request #423:
URL: https://github.com/apache/maven/pull/423


   


----------------------------------------------------------------
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