You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by mi...@apache.org on 2022/07/18 11:37:51 UTC

[maven] branch master updated: [MNG-7511] Ensure the degreeOfConcurrency is a positive number in MavenExecutionRequest

This is an automated email from the ASF dual-hosted git repository.

michaelo pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven.git


The following commit(s) were added to refs/heads/master by this push:
     new 03b1faff7 [MNG-7511] Ensure the degreeOfConcurrency is a positive number in MavenExecutionRequest
03b1faff7 is described below

commit 03b1faff7eec915f8b277d307d3e3aa0d0a24fdf
Author: Josef Cacek <jo...@gmail.com>
AuthorDate: Tue Jul 12 20:15:25 2022 +0200

    [MNG-7511] Ensure the degreeOfConcurrency is a positive number in MavenExecutionRequest
    
    This closes #767
---
 .../maven/lifecycle/internal/LifecycleStarter.java |  2 +-
 .../multithreaded/MultiThreadedBuilder.java        |  2 +-
 .../maven/execution/DefaultMavenExecutionTest.java |  1 +
 .../main/java/org/apache/maven/cli/CLIManager.java |  2 +-
 .../main/java/org/apache/maven/cli/MavenCli.java   | 68 +++++++++++++++++-----
 .../java/org/apache/maven/cli/MavenCliTest.java    | 45 ++++++++++----
 6 files changed, 93 insertions(+), 27 deletions(-)

diff --git a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleStarter.java b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleStarter.java
index 260dd2554..a96c29a61 100644
--- a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleStarter.java
+++ b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleStarter.java
@@ -134,7 +134,7 @@ public class LifecycleStarter
             }
 
             int degreeOfConcurrency = session.getRequest().getDegreeOfConcurrency();
-            if ( degreeOfConcurrency >= 2 )
+            if ( degreeOfConcurrency > 1 )
             {
                 logger.info( "" );
                 logger.info( String.format( "Using the %s implementation with a thread count of %d",
diff --git a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/builder/multithreaded/MultiThreadedBuilder.java b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/builder/multithreaded/MultiThreadedBuilder.java
index 4bf68102e..7c5ca0076 100644
--- a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/builder/multithreaded/MultiThreadedBuilder.java
+++ b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/builder/multithreaded/MultiThreadedBuilder.java
@@ -84,7 +84,7 @@ public class MultiThreadedBuilder
         throws ExecutionException, InterruptedException
     {
         int nThreads = Math.min( session.getRequest().getDegreeOfConcurrency(), session.getProjects().size() );
-        boolean parallel = nThreads >= 2;
+        boolean parallel = nThreads > 1;
         // Propagate the parallel flag to the root session and all of the cloned sessions in each project segment
         session.setParallel( parallel );
         for ( ProjectSegment segment : projectBuilds )
diff --git a/maven-core/src/test/java/org/apache/maven/execution/DefaultMavenExecutionTest.java b/maven-core/src/test/java/org/apache/maven/execution/DefaultMavenExecutionTest.java
index 9a363eeea..f8f5e0afb 100644
--- a/maven-core/src/test/java/org/apache/maven/execution/DefaultMavenExecutionTest.java
+++ b/maven-core/src/test/java/org/apache/maven/execution/DefaultMavenExecutionTest.java
@@ -20,6 +20,7 @@ package org.apache.maven.execution;
  */
 import static org.hamcrest.Matchers.empty;
 import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNotSame;
 import static org.hamcrest.MatcherAssert.assertThat;
diff --git a/maven-embedder/src/main/java/org/apache/maven/cli/CLIManager.java b/maven-embedder/src/main/java/org/apache/maven/cli/CLIManager.java
index 45e322fcb..51824ce45 100644
--- a/maven-embedder/src/main/java/org/apache/maven/cli/CLIManager.java
+++ b/maven-embedder/src/main/java/org/apache/maven/cli/CLIManager.java
@@ -153,7 +153,7 @@ public class CLIManager
         options.addOption( Option.builder( Character.toString( SHOW_VERSION ) ).longOpt( "show-version" ).desc( "Display version information WITHOUT stopping build" ).build() );
         options.addOption( Option.builder( ENCRYPT_MASTER_PASSWORD ).longOpt( "encrypt-master-password" ).hasArg().optionalArg( true ).desc( "Encrypt master security password" ).build() );
         options.addOption( Option.builder( ENCRYPT_PASSWORD ).longOpt( "encrypt-password" ).hasArg().optionalArg( true ).desc( "Encrypt server password" ).build() );
-        options.addOption( Option.builder( THREADS ).longOpt( "threads" ).hasArg().desc( "Thread count, for instance 2.0C where C is core multiplied" ).build() );
+        options.addOption( Option.builder( THREADS ).longOpt( "threads" ).hasArg().desc( "Thread count, for instance 4 (int) or 2C/2.5C (int/float) where C is core multiplied" ).build() );
         options.addOption( Option.builder( LEGACY_LOCAL_REPOSITORY ).longOpt( "legacy-local-repository" ).desc( "Use Maven 2 Legacy Local Repository behaviour, ie no use of _remote.repositories. Can also be activated by using -Dmaven.legacyLocalRepo=true" ).build() );
         options.addOption( Option.builder( BUILDER ).longOpt( "builder" ).hasArg().desc( "The id of the build strategy to use" ).build() );
         options.addOption( Option.builder( NO_TRANSFER_PROGRESS ).longOpt( "no-transfer-progress" ).desc( "Do not display transfer progress when downloading or uploading" ).build() );
diff --git a/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java b/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java
index ffb9ea830..27e726dd5 100644
--- a/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java
+++ b/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java
@@ -24,6 +24,7 @@ import org.apache.commons.cli.CommandLine;
 import org.apache.commons.cli.Option;
 import org.apache.commons.cli.ParseException;
 import org.apache.commons.cli.UnrecognizedOptionException;
+import org.apache.commons.lang3.math.NumberUtils;
 import org.apache.maven.BuildAbort;
 import org.apache.maven.InternalErrorException;
 import org.apache.maven.Maven;
@@ -1421,18 +1422,11 @@ public class MavenCli
 
         if ( threadConfiguration != null )
         {
-            //
-            // Default to the standard multithreaded builder
-            //
-            request.setBuilderId( "multithreaded" );
-
-            if ( threadConfiguration.contains( "C" ) )
-            {
-                request.setDegreeOfConcurrency( calculateDegreeOfConcurrencyWithCoreMultiplier( threadConfiguration ) );
-            }
-            else
+            int degreeOfConcurrency = calculateDegreeOfConcurrency( threadConfiguration );
+            if ( degreeOfConcurrency > 1 )
             {
-                request.setDegreeOfConcurrency( Integer.parseInt( threadConfiguration ) );
+                request.setBuilderId( "multithreaded" );
+                request.setDegreeOfConcurrency( degreeOfConcurrency );
             }
         }
 
@@ -1698,10 +1692,56 @@ public class MavenCli
         }
     }
 
-    int calculateDegreeOfConcurrencyWithCoreMultiplier( String threadConfiguration )
+    int calculateDegreeOfConcurrency( String threadConfiguration )
     {
-        int procs = Runtime.getRuntime().availableProcessors();
-        return (int) ( Float.parseFloat( threadConfiguration.replace( "C", "" ) ) * procs );
+        if ( threadConfiguration.endsWith( "C" ) )
+        {
+            threadConfiguration = threadConfiguration.substring( 0, threadConfiguration.length() - 1 );
+
+            if ( !NumberUtils.isParsable( threadConfiguration ) )
+            {
+                throw new IllegalArgumentException( "Invalid threads core multiplier value: '" + threadConfiguration
+                        + "C'. Supported are int and float values ending with C." );
+            }
+
+            float coreMultiplier = Float.parseFloat( threadConfiguration );
+
+            if ( coreMultiplier <= 0.0f )
+            {
+                throw new IllegalArgumentException( "Invalid threads core multiplier value: '" + threadConfiguration
+                        + "C'. Value must be positive." );
+            }
+
+            int procs = Runtime.getRuntime().availableProcessors();
+            int threads = (int) ( coreMultiplier * procs );
+            return threads == 0 ? 1 : threads;
+        }
+        else
+        {
+            if ( !NumberUtils.isParsable( threadConfiguration ) )
+            {
+                throw new IllegalArgumentException( "Invalid threads value: '" + threadConfiguration
+                        + "'. Supported are int values." );
+            }
+
+            try
+            {
+                int threads = Integer.parseInt( threadConfiguration );
+
+                if ( threads <= 0 )
+                {
+                    throw new IllegalArgumentException( "Invalid threads value: '" + threadConfiguration
+                            + "'. Value must be positive." );
+                }
+
+                return threads;
+            }
+            catch ( NumberFormatException e )
+            {
+                throw new IllegalArgumentException( "Invalid threads value: '" + threadConfiguration
+                        + "'. Supported are integer values." );
+            }
+        }
     }
 
     // ----------------------------------------------------------------------
diff --git a/maven-embedder/src/test/java/org/apache/maven/cli/MavenCliTest.java b/maven-embedder/src/test/java/org/apache/maven/cli/MavenCliTest.java
index 3d7e385dc..5c2132f5a 100644
--- a/maven-embedder/src/test/java/org/apache/maven/cli/MavenCliTest.java
+++ b/maven-embedder/src/test/java/org/apache/maven/cli/MavenCliTest.java
@@ -146,18 +146,43 @@ public class MavenCliTest
     }
 
     @Test
-    public void testCalculateDegreeOfConcurrencyWithCoreMultiplier()
+    public void testCalculateDegreeOfConcurrency()
     {
-        int cores = Runtime.getRuntime().availableProcessors();
-        // -T2.2C
-        assertEquals( (int) ( cores * 2.2 ), cli.calculateDegreeOfConcurrencyWithCoreMultiplier( "C2.2" ) );
-        // -TC2.2
-        assertEquals( (int) ( cores * 2.2 ), cli.calculateDegreeOfConcurrencyWithCoreMultiplier( "2.2C" ) );
+        assertThrows( IllegalArgumentException.class,
+                () -> cli.calculateDegreeOfConcurrency( "0" ) );
+        assertThrows( IllegalArgumentException.class,
+                () -> cli.calculateDegreeOfConcurrency( "-1" ) );
+        assertThrows( IllegalArgumentException.class,
+                () -> cli.calculateDegreeOfConcurrency( "0x4" ) );
+        assertThrows( IllegalArgumentException.class,
+                () -> cli.calculateDegreeOfConcurrency( "1.0" ) );
+        assertThrows( IllegalArgumentException.class,
+                () -> cli.calculateDegreeOfConcurrency( "1." ) );
+        assertThrows( IllegalArgumentException.class,
+                () -> cli.calculateDegreeOfConcurrency( "AA" ) );
+        assertThrows( IllegalArgumentException.class,
+                () -> cli.calculateDegreeOfConcurrency( "C" ) );
+        assertThrows( IllegalArgumentException.class,
+                () -> cli.calculateDegreeOfConcurrency( "C2.2C" ) );
+        assertThrows( IllegalArgumentException.class,
+                () -> cli.calculateDegreeOfConcurrency( "C2.2" ) );
+        assertThrows( IllegalArgumentException.class,
+                () -> cli.calculateDegreeOfConcurrency( "2C2" ) );
+        assertThrows( IllegalArgumentException.class,
+                () -> cli.calculateDegreeOfConcurrency( "CXXX" ) );
+        assertThrows( IllegalArgumentException.class,
+                () -> cli.calculateDegreeOfConcurrency( "XXXC" ) );
+
+        int cpus = Runtime.getRuntime().availableProcessors();
+        assertEquals( (int) ( cpus * 2.2 ), cli.calculateDegreeOfConcurrency( "2.2C" ) );
+        assertEquals( 1, cli.calculateDegreeOfConcurrency( "0.0001C" ) );
+        assertThrows( IllegalArgumentException.class,
+                () -> cli.calculateDegreeOfConcurrency( "2.C" ) );
+        assertThrows( IllegalArgumentException.class,
+                () -> cli.calculateDegreeOfConcurrency( "-2.2C" ) );
+        assertThrows( IllegalArgumentException.class,
+                () -> cli.calculateDegreeOfConcurrency( "0C" ) );
 
-        assertThrows(
-                NumberFormatException.class,
-                () -> cli.calculateDegreeOfConcurrencyWithCoreMultiplier( "CXXX" ),
-                "Should have failed with a NumberFormatException" );
     }
 
     @Test