Apply unstage logic from nextflow-io#2182

Signed-off-by: Lehmann-Fabian <[email protected]>
Lehmann-Fabian · Jan 11, 2022 · 6b3621f · 6b3621f
1 parent 21d1289
commit 6b3621f
Show file tree

Hide file tree

Showing 9 changed files with 384 additions and 56 deletions.
diff --git a/modules/nextflow/src/main/groovy/nextflow/executor/ScriptOutputFiles.groovy b/modules/nextflow/src/main/groovy/nextflow/executor/ScriptOutputFiles.groovy
@@ -65,7 +65,7 @@ class ScriptOutputFiles implements Map<String,Glob> {
      *      The name containing escaping special chars to make it shell friendly
      */
     static String shellSpecialChars(String name, Glob glob) {
-        return glob ? Escape.path(removeGlobStar(name)) : Escape.wildcards(name)
+        return glob ? escapeGlob(name) : Escape.wildcards(name)
     }
 
     /**
@@ -78,36 +78,12 @@ class ScriptOutputFiles implements Map<String,Glob> {
         return loc
     }
 
-    /**
-     * Remove a glob star specifier returning the longest parent path
-     *
-     * <pre>
-     * /some/data/&#42;&#42;/file.txt  ->   /some/data
-     * /some/data/file.txt     ->   /some/data/file.txt
-     * /some&#42;&#42;                 ->   *
-     * </pre>
-     *
-     * @param path
-     * @return
-     */
-    static String removeGlobStar(String path) {
-
-        def p = path?.indexOf('**')
-        if( p == -1 )
-            return path
-
-        def slash = -1
-        for( int i=p-1; i>=0; i-- ) {
-            if( path.charAt(i) == '/' as char) {
-                slash = i
-                break
-            }
-        }
-
-        if( slash == -1 )
-            return '*'
-
-        path.substring(0,slash)
+    private static String escapeGlob( String path ){
+        String escaped = Escape.path( path, true )
+        //Bash glob behaves different than Java's Glob, if the path starts with **
+        //https://unix.stackexchange.com/questions/49913/recursive-glob
+        escaped = escaped.replace( '**', '{*,*/**/*}' )
+        escaped
     }
 
     void putName(String name, boolean glob) {
@@ -116,8 +92,8 @@ class ScriptOutputFiles implements Map<String,Glob> {
 
     String toShellEscapedNames() {
         final result = new ArrayList(target.size())
-        for( String it : target.keySet() ) {
-            result.add( tts(shellSpecialChars(it, target[it])) )
+        for( def it : target.entrySet() ) {
+            result.add( tts(shellSpecialChars( it.getKey(), it.getValue() )) )
         }
         return result.join(' ')
     }

diff --git a/modules/nextflow/src/main/groovy/nextflow/executor/SimpleFileCopyStrategy.groovy b/modules/nextflow/src/main/groovy/nextflow/executor/SimpleFileCopyStrategy.groovy
@@ -168,9 +168,14 @@ class SimpleFileCopyStrategy implements ScriptFileCopyStrategy {
         final mode = stageoutMode ?: ( workDir==targetDir ? 'copy' : 'move' )
         return """\
             IFS=\$'\\n'
-            for name in \$(eval "ls -1d ${outputFiles.toShellEscapedNames()}" | sort | uniq); do
+            shopt -s globstar extglob || true
+            pathes=`ls -1d ${outputFiles.toShellEscapedNames()} | sort | uniq`
+            shopt -u globstar extglob || true
+            set -f
+            for name in \$pathes; do
                 ${stageOutCommand('$name', targetDir, mode)} || true
             done
+            set +f
             unset IFS""".stripIndent(true)
     }
 

diff --git a/modules/nextflow/src/test/groovy/nextflow/executor/BashWrapperBuilderTest.groovy b/modules/nextflow/src/test/groovy/nextflow/executor/BashWrapperBuilderTest.groovy
@@ -408,9 +408,14 @@ class BashWrapperBuilderTest extends Specification {
         then:
         binding.unstage_outputs == '''\
                 IFS=$'\\n'
-                for name in $(eval "ls -1d test.bam test.bai" | sort | uniq); do
+                shopt -s globstar extglob || true
+                pathes=`ls -1d test.bam test.bai | sort | uniq`
+                shopt -u globstar extglob || true
+                set -f
+                for name in $pathes; do
                     nxf_fs_copy "$name" /work/dir || true
                 done
+                set +f
                 unset IFS
                 '''.stripIndent().rightTrim()
 
@@ -425,9 +430,14 @@ class BashWrapperBuilderTest extends Specification {
         then:
         binding.unstage_outputs == '''\
                 IFS=$'\\n'
-                for name in $(eval "ls -1d test.bam test.bai" | sort | uniq); do
+                shopt -s globstar extglob || true
+                pathes=`ls -1d test.bam test.bai | sort | uniq`
+                shopt -u globstar extglob || true
+                set -f
+                for name in $pathes; do
                     nxf_fs_move "$name" /another/dir || true
                 done
+                set +f
                 unset IFS
                 '''.stripIndent().rightTrim()
     }

diff --git a/modules/nextflow/src/test/groovy/nextflow/executor/ScriptOutputFilesTest.groovy b/modules/nextflow/src/test/groovy/nextflow/executor/ScriptOutputFilesTest.groovy
@@ -46,20 +46,6 @@ class ScriptOutputFilesTest extends Specification {
         'fo?.txt'   | true      | 'fo?.txt'
     }
 
-
-    def 'should remove star glob pattern'() {
-
-        expect:
-        ScriptOutputFiles.removeGlobStar('a/b/c') == 'a/b/c'
-        ScriptOutputFiles.removeGlobStar('/a/b/c') == '/a/b/c'
-        ScriptOutputFiles.removeGlobStar('some/*/path') == 'some/*/path'
-        ScriptOutputFiles.removeGlobStar('some/**/path') == 'some'
-        ScriptOutputFiles.removeGlobStar('some/**/path') == 'some'
-        ScriptOutputFiles.removeGlobStar('some*') == 'some*'
-        ScriptOutputFiles.removeGlobStar('some**') == '*'
-
-    }
-
     def 'should return shell escaped names' () {
         given:
         def files = new ScriptOutputFiles()