refactored

Signed-off-by: munishchouhan <hrma017@gmail.com>

Author: munishchouhan

app/src/main/java/io/seqera/wave/cli/App.java

@@ -132,7 +132,7 @@ public class App implements Runnable {
     @Option(names = {"--freeze", "-F"}, paramLabel = "false",  description = "Request a container freeze.")
     private boolean freeze;
 
-    @Option(names = {"--platform"}, paramLabel = "''", description = "Platform to be used for the container build. One of: linux/amd64, linux/arm64, linux/amd64,linux/arm64 for multi-arch builds, or 'all'.")
+    @Option(names = {"--platform"}, paramLabel = "''", description = "Platform to be used for the container build. One of: linux/amd64, linux/arm64. Use 'all' for multi-arch builds.")
     private String platform;
 
     @Option(names = {"--await"}, paramLabel = "false", arity = "0..1", description = "Await the container build to be available. you can provide a timeout like --await 10m or 2s, by default its 15 minutes.")
@@ -426,17 +426,12 @@ protected void validateArgs() {
         if( dryRun && await != null )
             throw new IllegalCliArgumentException("Options --dry-run and --await conflicts each other");
 
-        if( !isEmpty(platform) )
-            validatePlatform(platform);
-
-    }
+        if( !isEmpty(platform) ) {
+            platform = normalizePlatform(platform);
+            if( platform.contains(",") )
+                validateMultiArch();
+        }
 
-    protected void validatePlatform(String platform) {
-        // normalizePlatform validates each platform value via canonicalPlatform
-        normalizePlatform(platform);
-        // check multi-arch specific constraints
-        if( PLATFORM_ALL.equals(platform) || platform.contains(",") )
-            validateMultiArch();
     }
 
     private void validateMultiArch() {
@@ -455,7 +450,7 @@ protected SubmitContainerTokenRequest createRequest() {
                 .withContainerImage(image)
                 .withContainerFile(containerFileBase64())
                 .withPackages(packagesSpec())
-                .withContainerPlatform(normalizePlatform(platform))
+                .withContainerPlatform(platform)
                 .withTimestamp(OffsetDateTime.now())
                 .withBuildRepository(buildRepository)
                 .withCacheRepository(cacheRepository)
@@ -484,19 +479,20 @@ protected String normalizePlatform(String platform) {
             return MULTI_ARCH_PLATFORM;
         if( platform.contains(",") ) {
             return Arrays.stream(platform.split(","))
-                    .map(String::trim)
                     .map(App::canonicalPlatform)
+                    .distinct()
                     .collect(Collectors.joining(","));
         }
         return canonicalPlatform(platform);
     }
 
     private static String canonicalPlatform(String value) {
-        if( AMD64_ALIASES.contains(value) )
+        final String v = value.trim();
+        if( AMD64_ALIASES.contains(v) )
             return "linux/amd64";
-        if( ARM64_ALIASES.contains(value) )
+        if( ARM64_ALIASES.contains(v) )
             return "linux/arm64";
-        throw new IllegalCliArgumentException(String.format("Unsupported container platform: '%s'", value));
+        throw new IllegalCliArgumentException(String.format("Unsupported container platform: '%s'", v));
     }
 
     BuildCompression compression(BuildCompression.Mode mode) {

app/src/test/groovy/io/seqera/wave/cli/AppTest.groovy

@@ -388,16 +388,16 @@ class AppTest extends Specification {
         and:
         app.validateArgs()
         then:
-        app.@platform == PLATFORM
+        app.@platform == EXPECTED
 
         where:
-        PLATFORM        || _
-        'amd64'         || _
-        'x86_64'        || _
-        'arm64'         || _
-        'linux/amd64'   || _
-        'linux/x86_64'  || _
-        'linux/arm64'   || _
+        PLATFORM        || EXPECTED
+        'amd64'         || 'linux/amd64'
+        'x86_64'        || 'linux/amd64'
+        'arm64'         || 'linux/arm64'
+        'linux/amd64'   || 'linux/amd64'
+        'linux/x86_64'  || 'linux/amd64'
+        'linux/arm64'   || 'linux/arm64'
     }
 
     @Unroll
@@ -786,6 +786,77 @@ class AppTest extends Specification {
         noExceptionThrown()
     }
 
+    def 'should normalize platform with whitespace in comma-separated values' () {
+        given:
+        def folder = Files.createTempDirectory('test')
+        def containerFile = folder.resolve('Dockerfile')
+        containerFile.text = 'FROM ubuntu:latest'
+        and:
+        def app = new App()
+        String[] args = ["-f", containerFile.toString(), "--platform", "linux/amd64, linux/arm64"]
+
+        when:
+        new CommandLine(app).parseArgs(args)
+        and:
+        app.validateArgs()
+        then:
+        noExceptionThrown()
+
+        when:
+        def req = app.createRequest()
+        then:
+        req.containerPlatform == 'linux/amd64,linux/arm64'
+
+        cleanup:
+        folder?.deleteDir()
+    }
+
+    def 'should fail platform all with singularity' () {
+        given:
+        def folder = Files.createTempDirectory('test')
+        def containerFile = folder.resolve('Dockerfile')
+        containerFile.text = 'FROM ubuntu:latest'
+        and:
+        def app = new App()
+        String[] args = ["-f", containerFile.toString(), "--platform", "all", "--singularity", "--freeze", "--build-repo", "docker.io/foo", "--tower-token", "xyz"]
+
+        when:
+        new CommandLine(app).parseArgs(args)
+        and:
+        app.validateArgs()
+        then:
+        def e = thrown(IllegalCliArgumentException)
+        e.message == 'Multi-arch build is not compatible with Singularity format'
+
+        cleanup:
+        folder?.deleteDir()
+    }
+
+    def 'should deduplicate repeated platforms' () {
+        given:
+        def folder = Files.createTempDirectory('test')
+        def containerFile = folder.resolve('Dockerfile')
+        containerFile.text = 'FROM ubuntu:latest'
+        and:
+        def app = new App()
+        String[] args = ["-f", containerFile.toString(), "--platform", "amd64,amd64"]
+
+        when:
+        new CommandLine(app).parseArgs(args)
+        and:
+        app.validateArgs()
+        then:
+        noExceptionThrown()
+
+        when:
+        def req = app.createRequest()
+        then:
+        req.containerPlatform == 'linux/amd64'
+
+        cleanup:
+        folder?.deleteDir()
+    }
+
     def 'should fail when comma-separated platform contains invalid value' () {
         given:
         def folder = Files.createTempDirectory('test')