Add ability to define templates library at module level (#2332)

Signed-off-by: Manuele Simi <[email protected]>

Author: pditommaso

.github/workflows/build.yml

@@ -102,9 +102,9 @@ jobs:
         AZURE_STORAGE_ACCOUNT_KEY: ${{ secrets.AZURE_STORAGE_ACCOUNT_KEY }}
         AZURE_BATCH_ACCOUNT_KEY: ${{ secrets.AZURE_BATCH_ACCOUNT_KEY }}
 
-    - name: Publish
+    - name: Publish tests report
       if: failure()
       uses: actions/upload-artifact@v2
       with:
-        name: test-reports
-        path: "*build/reports/tests/test"
+        name: test-reports-jdk-${{ matrix.java_version }}
+        path: "**/build/reports/tests/test"

docs/dsl2.rst

@@ -354,7 +354,7 @@ Multiple inclusions
 -------------------
 
 A Nextflow script allows the inclusion of any number of modules. When multiple
-components need to be included from the some module script, the component names can be
+components need to be included from the same module script, the component names can be
 specified in the same inclusion using the curly brackets notation as shown below::
 
     include { foo; bar } from './some/module'
@@ -447,6 +447,66 @@ The above snippet prints::
 Finally the include option ``params`` allows the specification of one or more parameters without
 inheriting any value from the external environment. 
 
+.. _module-templates:
+
+Module templates
+-----------------
+The module script can be defined in an external :ref:`template <process-template>` file. With DSL2 the template file
+can be placed under the ``templates`` directory where the module script is located.
+
+For example, let's suppose to have a project L with a module script defining 2 processes (P1 and P2) and both use templates.
+The template files can be made available under the local ``templates`` directory::
+
+	Project L
+		|-myModules.nf
+		|-templates
+			|-P1-template.sh
+			|-P2-template.sh
+
+Then, we have a second project A with a workflow that includes P1 and P2::
+
+	Pipeline A
+		|-main.nf
+
+Finally, we have a third project B with a workflow that includes again P1 and P2::
+
+	Pipeline B
+		|-main.nf
+
+With the possibility to keep the template files inside the project L, A and B can use the modules defined in L without any changes.
+A future prject C would do the same, just cloning L (if not available on the system) and including its module script.
+
+Beside promoting sharing modules across pipelines, there are several advantages in keeping the module template under the script path::
+1 - module components are *self-contained*
+2 - module components can be tested independently from the pipeline(s) importing them
+3 - it is possible to create libraries of module components
+
+Ultimately, having multiple template locations allows a more structured organization within the same project. If a project
+has several module components, and all them use templates, the project could group module scripts and their templates as needed. For example::
+
+	baseDir
+		|-main.nf
+		|-Phase0-Modules
+			|-mymodules1.nf
+			|-mymodules2.nf
+			|-templates
+				|-P1-template.sh
+				|-P2-template.sh
+		|-Phase1-Modules
+			|-mymodules3.nf
+			|-mymodules4.nf
+			|-templates
+				|-P3-template.sh
+				|-P4-template.sh
+		|-Phase2-Modules
+			|-mymodules5.nf
+			|-mymodules6.nf
+			|-templates
+				|-P5-template.sh
+				|-P6-template.sh
+				|-P7-template.sh
+
+
 Channel forking
 ===============
 

docs/process.rst

@@ -235,6 +235,11 @@ as shown below::
 Nextflow looks for the ``my_script.sh`` template file in the directory ``templates`` that must exist in the same folder
 where the Nextflow script file is located (any other location can be provided by using an absolute template path).
 
+.. note::
+  When using :ref:`DSL2 <dsl2-page>` Nextflow looks for the specified file name also in the ``templates`` directory
+  located in the same folder where the module script is placed. See :ref:`module templates <module-templates>`.
+
+
 The template script can contain any piece of code that can be executed by the underlying system. For example::
 
   #!/bin/bash

modules/nextflow/src/main/groovy/nextflow/datasource/SraExplorer.groovy

@@ -109,7 +109,14 @@ class SraExplorer {
         return target
     }
 
-    protected Map getEnv() { System.getenv() }
+    protected Map env() {
+        return System.getenv()
+    }
+
+    protected Map config() {
+        final session = Global.session as Session
+        return session.getConfig()
+    }
 
     protected Path getCacheFolder() {
         if( cacheFolder )
@@ -120,10 +127,9 @@ class SraExplorer {
     }
 
     protected String getConfigApiKey() {
-        def session = Global.session as Session
-        def result = session ?.config ?. navigate('ncbi.apiKey')
+        def result = config().navigate('ncbi.apiKey')
         if( !result )
-            result = getEnv().get('NCBI_API_KEY')
+            result = env().get('NCBI_API_KEY')
         if( !result )
             log.warn1("Define the NCBI_API_KEY env variable to use NCBI search service -- Read more https://ncbiinsights.ncbi.nlm.nih.gov/2017/11/02/new-api-keys-for-the-e-utilities/")
         return result

modules/nextflow/src/main/groovy/nextflow/processor/TaskContext.groovy

@@ -17,8 +17,12 @@
 
 package nextflow.processor
 
+import nextflow.NF
+import nextflow.script.ScriptMeta
+
 import java.nio.file.Path
 import java.nio.file.Paths
+import java.nio.file.Files
 import java.util.concurrent.atomic.AtomicBoolean
 
 import com.esotericsoftware.kryo.io.Input
@@ -303,14 +307,22 @@ class TaskContext implements Map<String,Object>, Cloneable {
         if( !path )
             throw new ProcessException("Process `$name` missing template name")
 
-        if( !(path instanceof Path) )
+        if( path !instanceof Path )
             path = Paths.get(path.toString())
 
         // if the path is already absolute just return it
         if( path.isAbsolute() )
             return path
 
-        // otherwise make
+        // make from the module dir
+        def module = NF.isDsl2Final() ? ScriptMeta.get(this.script)?.getModuleDir() : null
+        if( module ) {
+            def target = module.resolve('templates').resolve(path)
+            if (Files.exists(target))
+                return target
+        }
+
+        // otherwise make from the base dir
         def base = Global.session.baseDir
         if( base )
             return base.resolve('templates').resolve(path)

modules/nextflow/src/main/groovy/nextflow/script/BaseScript.groovy

@@ -100,7 +100,7 @@ abstract class BaseScript extends Script implements ExecutionContext {
         binding.setVariable( 'workflow', session.workflowMetadata )
         binding.setVariable( 'nextflow', NextflowMeta.instance )
         binding.setVariable('launchDir', Paths.get('./').toRealPath())
-        binding.setVariable('moduleDir', meta.scriptPath?.parent )
+        binding.setVariable('moduleDir', meta.moduleDir )
     }
 
     protected process( String name, Closure<BodyDef> body ) {

modules/nextflow/src/main/groovy/nextflow/script/ScriptMeta.groovy

@@ -47,7 +47,7 @@ class ScriptMeta {
 
     static ScriptMeta get(BaseScript script) {
         if( !script ) throw new IllegalStateException("Missing current script context")
-        REGISTRY.get(script)
+        return REGISTRY.get(script)
     }
 
     static Set<String> allProcessNames() {
@@ -71,7 +71,7 @@ class ScriptMeta {
     /** the script {@link Class} object */
     private Class<? extends BaseScript> clazz
 
-    /** The location path from there the script has been loaded */
+    /** The location path from where the script has been loaded */
     private Path scriptPath
 
     /** The list of function, procs and workflow defined in this script */
@@ -87,6 +87,8 @@ class ScriptMeta {
 
     Path getScriptPath() { scriptPath }
 
+    Path getModuleDir () { scriptPath?.parent }
+
     String getScriptName() { clazz.getName() }
 
     boolean isModule() { module }

modules/nextflow/src/test/groovy/nextflow/datasource/SraExplorerTest.groovy

@@ -197,7 +197,8 @@ class SraExplorerTest extends Specification {
         when:
         def result = slurper.getConfigApiKey()
         then:
-        1 * slurper.getEnv() >> [NCBI_API_KEY: '1bc']
+        1 * slurper.config() >> [:]
+        1 * slurper.env() >> [NCBI_API_KEY: '1bc']
         then:
         result == '1bc'
     }

modules/nextflow/src/test/groovy/nextflow/processor/TaskContextTest.groovy

@@ -17,13 +17,21 @@
 
 package nextflow.processor
 
+import java.nio.file.Files
 import java.nio.file.Paths
 
+import groovy.runtime.metaclass.DelegatingPlugin
+import groovy.runtime.metaclass.NextflowDelegatingMetaClass
 import groovy.transform.InheritConstructors
+import nextflow.Global
+import nextflow.NF
+import nextflow.NextflowMeta
+import nextflow.Session
 import nextflow.script.BaseScript
+import nextflow.script.BodyDef
 import nextflow.script.ProcessConfig
 import nextflow.script.ScriptBinding
-import nextflow.script.BodyDef
+import nextflow.script.ScriptMeta
 import nextflow.util.BlankSeparatedList
 import nextflow.util.Duration
 import nextflow.util.MemoryUnit
@@ -34,6 +42,10 @@ import spock.lang.Specification
  */
 class TaskContextTest extends Specification {
 
+    def setupSpec() {
+        NF.init()
+    }
+    
     def 'should save and read TaskContext object' () {
 
         setup:
@@ -76,8 +88,8 @@ class TaskContextTest extends Specification {
 
         setup:
         def bind = new ScriptBinding([x:1, y:2])
-        def script = new MockScript(bind)
-
+        def script = Mock(BaseScript) { getBinding() >> bind }
+        and:
         def local = [p:3, q:4, path: Paths.get('some/path')]
         def delegate = new TaskContext( script, local, 'hola' )
 
@@ -139,6 +151,66 @@ class TaskContextTest extends Specification {
 
     }
 
+
+    def 'should resolve absolute paths as template paths' () {
+        given:
+        def temp = Files.createTempDirectory('test')
+        and:
+        Global.session = Mock(Session) { getBaseDir() >> temp  }
+        and:
+        def holder = [:]
+        def script = Mock(BaseScript)
+        TaskContext context = Spy(TaskContext, constructorArgs: [script, holder, 'proc_1'])
+
+        when:
+        // an absolute path is specified
+        def absolutePath = Paths.get('/some/template.txt')
+        def result = context.template(absolutePath)
+        then:
+        // the path is returned
+        result == absolutePath
+
+        when:
+        // an absolute string path is specified
+        result = context.template('/some/template.txt')
+        then:
+        // the path is returned
+        result == absolutePath
+
+        when:
+        // when a rel file path is matched against the project
+        // template dir, it's returned as the template file
+        temp.resolve('templates').mkdirs()
+        temp.resolve('templates/foo.txt').text = 'echo hello'
+        and:
+        result = context.template('foo.txt')
+        then:
+        result == temp.resolve('templates/foo.txt')
+
+        when:
+        // it's a DSL2 module
+        NextflowMeta.instance.enableDsl2()
+        NextflowDelegatingMetaClass.plugin = Mock(DelegatingPlugin) { operatorNames() >> new HashSet<String>() }
+        def meta = ScriptMeta.register(script)
+        meta.setScriptPath(temp.resolve('modules/my-module/main.nf'))
+        and:
+        // the module has a nested `templates` directory
+        temp.resolve('modules/my-module/templates').mkdirs()
+        temp.resolve('modules/my-module/templates/foo.txt').text = 'echo Hola'
+        and:
+        // the template is a relative path
+        result = context.template('foo.txt')
+        then:
+        // the path is resolved against the module templates
+        result == temp.resolve('modules/my-module/templates/foo.txt')
+
+        cleanup:
+        NextflowDelegatingMetaClass.plugin = null
+        NextflowMeta.instance.disableDsl2()
+        temp?.deleteDir()
+    }
+
+
 }
 
 @InheritConstructors