From 56086b170465228dcfb4bf6e5e7b75f026c9128c Mon Sep 17 00:00:00 2001
From: Matthias Zepper <MatthiasZepper@users.noreply.github.com>
Date: Wed, 5 Jul 2023 17:38:33 +0200
Subject: [PATCH 01/17] Remove loop for resolution of container source
 priority. This is too flaky.

---
 nf_core/download.py | 142 ++++++++++++++++++++++----------------------
 1 file changed, 71 insertions(+), 71 deletions(-)

diff --git a/nf_core/download.py b/nf_core/download.py
index f049b41ab5..c7e54195b1 100644
--- a/nf_core/download.py
+++ b/nf_core/download.py
@@ -781,6 +781,13 @@ def rectify_raw_container_matches(self, raw_findings):
         """
         cleaned_matches = []
 
+        # Thanks Stack Overflow for the regex: https://stackoverflow.com/a/3809435/713980
+        url_regex = (
+            r"https?:\/\/(www\.)?[-a-zA-Z0-9@:%._\+~#=]{1,256}\.[a-zA-Z0-9()]{1,6}\b([-a-zA-Z0-9()@:%_\+.~#?&//=]*)"
+        )
+        # Thanks Stack Overflow for the regex: https://stackoverflow.com/a/39672069/713980
+        docker_regex = r"^(?:(?=[^:\/]{1,253})(?!-)[a-zA-Z0-9-]{1,63}(?<!-)(?:\.(?!-)[a-zA-Z0-9-]{1,63}(?<!-))*(?::[0-9]{1,5})?/)?((?![._-])(?:[a-z0-9._-]*)(?<![._-])(?:/(?![._-])[a-z0-9._-]*(?<![._-]))*)(?::(?![.-])[a-zA-Z0-9_.-]{1,128})?$"
+
         for _, container_value, search_space, file_path in raw_findings:
             """
             Now we need to isolate all container paths (typically quoted strings) from the raw container_value
@@ -814,20 +821,15 @@ def rectify_raw_container_matches(self, raw_findings):
             Mostly, it is a nested DSL2 string, but it may also just be a plain string.
 
             """
-            # reset for each raw_finding
-            this_container = None
 
             # first check if container_value it is a plain container URI like in DSL1 pipelines?
-            # Thanks Stack Overflow for the regex: https://stackoverflow.com/a/39672069/713980
-            docker_regex = r"^(?:(?=[^:\/]{1,253})(?!-)[a-zA-Z0-9-]{1,63}(?<!-)(?:\.(?!-)[a-zA-Z0-9-]{1,63}(?<!-))*(?::[0-9]{1,5})?/)?((?![._-])(?:[a-z0-9._-]*)(?<![._-])(?:/(?![._-])[a-z0-9._-]*(?<![._-]))*)(?::(?![.-])[a-zA-Z0-9_.-]{1,128})?$"
             docker_match = re.match(docker_regex, container_value.strip(), re.S)
             if docker_match:
-                this_container = docker_match.group(0)
-                cleaned_matches.append(this_container)
-                continue  # skip further processing
+                cleaned_matches.append(docker_match.group(0))
+                continue  # skip further processing, we already have a match
 
             """
-            # no plain string, we likely need to break it up further
+            no plain string, we likely need to break it up further
 
             [^\"\'] makes sure that the outermost quote character is not matched.
             (?P<quote>(?<![\\])[\'\"]) again captures the quote character, but not if it is preceded by an escape sequence (?<![\\])
@@ -838,80 +840,78 @@ def rectify_raw_container_matches(self, raw_findings):
                 r"[^\"\'](?P<quote>(?<![\\])[\'\"])(?P<param>(?:.(?!(?<![\\])\1))*.?)\1", container_value
             )
 
+            """
+            eliminate known false positives and create plain list out of the tuples returned by the regex above
+            example result:
+            ['https://depot.galaxyproject.org/singularity/scanpy:1.7.2--pyhdfd78af_0', 'biocontainers/scanpy:1.7.2--pyhdfd78af_0']
+            """
+            container_value_defs = [
+                capture for _, capture in container_value_defs[:] if not capture in ["singularity", "apptainer"]
+            ]
+
             """
             For later DSL2 syntax, container_value_defs should contain both, the download URL and the Docker URI.
-            By breaking the loop upon finding a download URL, we avoid duplication
-            (Container is downloaded directly as well as pulled and converted from Docker)
+            For earlier DSL2, both end up in different raw_findings, so a subsequent deduplication is necessary anyway
+            to not pull a container that is already downloaded directly.
 
-            For earlier DSL2, both end up in different raw_findings, so a deduplication is necessary
-            when the outer loop has finished.
+            At this point, we just add everything that is either a URL or a Docker URI to cleaned matches.
             """
 
-            for _, capture in container_value_defs:
-                # common false positive(s)
-                if capture in ["singularity", "apptainer"]:
-                    continue
+            either_url_or_docker = re.compile(f"{url_regex}|{docker_regex}")
+            valid_containers = list(filter(either_url_or_docker.match, container_value_defs))
 
-                # Look for a http download URL.
-                # Thanks Stack Overflow for the regex: https://stackoverflow.com/a/3809435/713980
-                url_regex = r"https?:\/\/(www\.)?[-a-zA-Z0-9@:%._\+~#=]{1,256}\.[a-zA-Z0-9()]{1,6}\b([-a-zA-Z0-9()@:%_\+.~#?&//=]*)"
-                url_match = re.search(url_regex, capture, re.S)
-                if url_match:
-                    this_container = url_match.group(0)
-                    break  # Prioritise http, exit loop as soon as we find it
-
-                # No https download, is the entire container string a docker URI?
-                # Thanks Stack Overflow for the regex: https://stackoverflow.com/a/39672069/713980
-                docker_regex = r"^(?:(?=[^:\/]{1,253})(?!-)[a-zA-Z0-9-]{1,63}(?<!-)(?:\.(?!-)[a-zA-Z0-9-]{1,63}(?<!-))*(?::[0-9]{1,5})?/)?((?![._-])(?:[a-z0-9._-]*)(?<![._-])(?:/(?![._-])[a-z0-9._-]*(?<![._-]))*)(?::(?![.-])[a-zA-Z0-9_.-]{1,128})?$"
-                docker_match = re.match(docker_regex, capture, re.S)
-                if docker_match:
-                    this_container = docker_match.group(0)
-                    break
+            if valid_containers:
+                cleaned_matches = cleaned_matches + valid_containers
+                # Yeah, we have successfully extracted something from this raw_finding, so move on.
+                continue
 
-            else:
-                """
-                Some modules declare the container as separate variable. This entails that " instead of ' is used,
-                so container_value_defs will not contain it and above loop will not break.
+            """
+            Neither a plain Docker URI nor a DSL2-like definition was found. This is a tricky case, then.
 
-                Therefore, we need to repeat the search over the contents, extract the variable name, and use it inside a new regex.
-                This is why the raw search_space is still needed at this level.
+            Some modules declare the container as separate variable. This entails that " instead of ' is used,
+            so container_value will not contain it.
 
-                To get the variable name ( ${container_id} in above example ), we match the literal word "container" and use lookbehind (reset the match).
-                Then we skip [^${}]+ everything that is not $ or curly braces. The next capture group is
-                ${ followed by any characters that are not curly braces [^{}]+ and ended by a closing curly brace (}),
-                but only if it's not followed by any other curly braces (?![^{]*}). The latter ensures we capture the innermost
-                variable name.
-                """
-                container_definition = re.search(r"(?<=container)[^\${}]+\${([^{}]+)}(?![^{]*})", str(search_space))
-
-                if bool(container_definition) and bool(container_definition.group(1)):
-                    pattern = re.escape(container_definition.group(1))
-                    # extract the quoted string(s) following the variable assignment
-                    container_names = re.findall(r"%s\s*=\s*[\"\']([^\"\']+)[\"\']" % pattern, search_space)
-
-                    if bool(container_names):
-                        if isinstance(container_names, str):
-                            this_container = f"https://depot.galaxyproject.org/singularity/{container_names}"
-
-                        elif isinstance(container_names, list):
-                            # this deliberately appends container_names[-1] twice to cleaned_matches
-                            # but deduplication is performed anyway and just setting this_container
-                            # here as well allows for an easy check to see if parsing succeeded.
-                            for container_name in container_names:
-                                this_container = f"https://depot.galaxyproject.org/singularity/{container_name}"
-                                cleaned_matches.append(this_container)
-                        else:
-                            # didn't find valid container declaration, but parsing succeeded.
-                            this_container = None
+            Therefore, we need to repeat the search over the raw contents, extract the variable name, and use it inside a new regex.
+            This is why the raw search_space is still needed at this level.
 
-            # If we have a this_container, parsing succeeded.
-            if this_container:
-                cleaned_matches.append(this_container)
-            else:
-                log.error(
-                    f"[red]Cannot parse container string in '{file_path}':\n\n{textwrap.indent(container_value, '    ')}\n\n:warning: Skipping this singularity image."
-                )
+            To get the variable name ( ${container_id} in above example ), we match the literal word "container" and use lookbehind (reset the match).
+            Then we skip [^${}]+ everything that is not $ or curly braces. The next capture group is
+            ${ followed by any characters that are not curly braces [^{}]+ and ended by a closing curly brace (}),
+            but only if it's not followed by any other curly braces (?![^{]*}). The latter ensures we capture the innermost
+            variable name.
+            """
+            import pdb
+
+            pdb.set_trace()
+
+            container_definition = re.search(r"(?<=container)[^\${}]+\${([^{}]+)}(?![^{]*})", str(search_space))
+
+            if bool(container_definition) and bool(container_definition.group(1)):
+                pattern = re.escape(container_definition.group(1))
+                # extract the quoted string(s) following the variable assignment
+                container_names = re.findall(r"%s\s*=\s*[\"\']([^\"\']+)[\"\']" % pattern, search_space)
+
+                if bool(container_names):
+                    if isinstance(container_names, str):
+                        cleaned_matches.append(f"https://depot.galaxyproject.org/singularity/{container_names}")
+
+                    elif isinstance(container_names, list):
+                        # this deliberately appends container_names[-1] twice to cleaned_matches
+                        # but deduplication is performed anyway and just setting this_container
+                        # here as well allows for an easy check to see if parsing succeeded.
+                        for container_name in container_names:
+                            cleaned_matches.append(f"https://depot.galaxyproject.org/singularity/{container_name}")
+
+                    continue
+
+            # all implemented options exhausted. Nothing left to be done:
+            log.error(
+                f"[red]Cannot parse container string in '{file_path}':\n\n{textwrap.indent(container_value, '    ')}\n\n:warning: Skipping this singularity image."
+            )
+
+        import pdb
 
+        pdb.set_trace()
         return cleaned_matches
 
     def get_singularity_images(self, current_revision=""):

From 60358e99a4438e233b780c6f693e86c73761a197 Mon Sep 17 00:00:00 2001
From: Matthias Zepper <MatthiasZepper@users.noreply.github.com>
Date: Wed, 5 Jul 2023 19:05:19 +0200
Subject: [PATCH 02/17] New helper function to prioritize direct download URLs,
 since previous solution was prone to false positives and negatives.

---
 nf_core/download.py | 54 +++++++++++++++++++++++++++++++++++----------
 1 file changed, 42 insertions(+), 12 deletions(-)

diff --git a/nf_core/download.py b/nf_core/download.py
index c7e54195b1..89bdb78d1f 100644
--- a/nf_core/download.py
+++ b/nf_core/download.py
@@ -702,10 +702,12 @@ def find_container_images(self, workflow_directory):
                     for finding in config_findings_dsl2:
                         config_findings.append((finding + (self.nf_config, "Nextflow configs")))
                 else:  # no regex match, likely just plain string
-                    # Append string also as finding-like tuple for consistency
-                    # because all will run through rectify_raw_container_matches()
-                    # self.nf_config is needed, because we need to restart search over raw input
-                    # if no proper container matches are found.
+                    """
+                    Append string also as finding-like tuple for consistency
+                    because all will run through rectify_raw_container_matches()
+                    self.nf_config is needed, because we need to restart search over raw input
+                    if no proper container matches are found.
+                    """
                     config_findings.append((k, v.strip('"').strip("'"), self.nf_config, "Nextflow configs"))
 
         # rectify the container paths found in the config
@@ -748,8 +750,8 @@ def find_container_images(self, workflow_directory):
         # Like above run on shallow copy, because length may change at runtime.
         module_findings = self.rectify_raw_container_matches(module_findings[:])
 
-        # Remove duplicates and sort
-        self.containers = sorted(list(set(previous_findings + config_findings + module_findings)))
+        # Again clean list, in case config declares Docker URI but module or previous finding already had the http:// download
+        self.containers = self.prioritize_direct_download(previous_findings + config_findings + module_findings)
 
     def rectify_raw_container_matches(self, raw_findings):
         """Helper function to rectify the raw extracted container matches into fully qualified container names.
@@ -880,9 +882,6 @@ def rectify_raw_container_matches(self, raw_findings):
             but only if it's not followed by any other curly braces (?![^{]*}). The latter ensures we capture the innermost
             variable name.
             """
-            import pdb
-
-            pdb.set_trace()
 
             container_definition = re.search(r"(?<=container)[^\${}]+\${([^{}]+)}(?![^{]*})", str(search_space))
 
@@ -909,10 +908,41 @@ def rectify_raw_container_matches(self, raw_findings):
                 f"[red]Cannot parse container string in '{file_path}':\n\n{textwrap.indent(container_value, '    ')}\n\n:warning: Skipping this singularity image."
             )
 
-        import pdb
+        """
+        Loop has finished, now we need to remove duplicates and prioritize direct downloads over containers pulled from the registries
+        """
+        return self.prioritize_direct_download(cleaned_matches)
+
+    def prioritize_direct_download(self, container_list, d={}):
+        """
+        Helper function that takes a list of container images (URLs and Docker URIs),
+        eliminates all Docker URIs for which also a URL is contained and returns the
+        cleaned and also deduplicated list.
+
+        Conceptually, this works like so:
+
+        Everything after the last Slash should be identical, e.g. "scanpy:1.7.2--pyhdfd78af_0" in
+        ['https://depot.galaxyproject.org/singularity/scanpy:1.7.2--pyhdfd78af_0', 'biocontainers/scanpy:1.7.2--pyhdfd78af_0']
+
+
+        re.sub('.*/(.*)','\\1',c) will drop everything up to the last slash from c (container_id)
 
-        pdb.set_trace()
-        return cleaned_matches
+        d.get(k:=re.sub('.*/(.*)','\\1',c),'') assigns the truncated string to k (key) and gets the
+        corresponding value from the dict if present or else defaults to "".
+
+        If the regex pattern matches, the original container_id will be assigned to the dict with the k key.
+        r"^$|(?!^http)" matches an empty string (we didn't have it in the dict yet and want to keep it in either case) or
+        any string that does not start with http. Because if our current dict value already starts with http,
+        we want to keep it and not replace with with whatever we have now (which might be the Docker URI).
+
+        A regex that matches http, r"^$|^http" could thus be used to prioritize the Docker URIs over http Downloads
+        """
+        for c in container_list:
+            log.info(c)
+            if re.match(r"^$|(?!^http)", d.get(k := re.sub(".*/(.*)", "\\1", c), "")):
+                log.debug(f"{c} matches and will be saved as {k}")
+                d[k] = c
+        return sorted(list(d.values()))
 
     def get_singularity_images(self, current_revision=""):
         """Loop through container names and download Singularity images"""

From d7784e3772ff25af6295a8ee73141c50a02c2da0 Mon Sep 17 00:00:00 2001
From: Matthias Zepper <MatthiasZepper@users.noreply.github.com>
Date: Wed, 5 Jul 2023 21:44:39 +0200
Subject: [PATCH 03/17] New test allows to test specific container notations in
 mock  modules.

---
 .../modules/mock_docker_single_quay_io.nf     |  8 +++
 .../modules/mock_dsl2_apptainer_var1.nf       | 10 +++
 .../modules/mock_dsl2_apptainer_var2.nf       | 10 +++
 .../modules/mock_dsl2_current.nf              | 10 +++
 .../modules/mock_dsl2_current_inverted.nf     | 10 +++
 .../modules/mock_dsl2_old.nf                  | 12 ++++
 .../modules/mock_dsl2_variable.nf             | 19 ++++++
 tests/test_download.py                        | 66 +++++++++++++++++--
 8 files changed, 141 insertions(+), 4 deletions(-)
 create mode 100644 tests/data/mock_module_containers/modules/mock_docker_single_quay_io.nf
 create mode 100644 tests/data/mock_module_containers/modules/mock_dsl2_apptainer_var1.nf
 create mode 100644 tests/data/mock_module_containers/modules/mock_dsl2_apptainer_var2.nf
 create mode 100644 tests/data/mock_module_containers/modules/mock_dsl2_current.nf
 create mode 100644 tests/data/mock_module_containers/modules/mock_dsl2_current_inverted.nf
 create mode 100644 tests/data/mock_module_containers/modules/mock_dsl2_old.nf
 create mode 100644 tests/data/mock_module_containers/modules/mock_dsl2_variable.nf

diff --git a/tests/data/mock_module_containers/modules/mock_docker_single_quay_io.nf b/tests/data/mock_module_containers/modules/mock_docker_single_quay_io.nf
new file mode 100644
index 0000000000..185c178ae4
--- /dev/null
+++ b/tests/data/mock_module_containers/modules/mock_docker_single_quay_io.nf
@@ -0,0 +1,8 @@
+process MOCK {
+    label 'process_fake'
+    
+    conda     (params.enable_conda ? "bioconda::singlequay=1.9" : null)
+    container "quay.io/biocontainers/singlequay:1.9--pyh9f0ad1d_0"
+
+    // truncated
+}
diff --git a/tests/data/mock_module_containers/modules/mock_dsl2_apptainer_var1.nf b/tests/data/mock_module_containers/modules/mock_dsl2_apptainer_var1.nf
new file mode 100644
index 0000000000..c92f69b42c
--- /dev/null
+++ b/tests/data/mock_module_containers/modules/mock_dsl2_apptainer_var1.nf
@@ -0,0 +1,10 @@
+process MOCK {
+    label 'process_fake'
+
+    conda "bioconda::dsltwoapptainervarone=1.1.0"
+    container "${ (workflow.containerEngine == 'singularity' || workflow.containerEngine == 'apptainer') && !task.ext.singularity_pull_docker_container ?
+        'https://depot.galaxyproject.org/singularity/dsltwoapptainervarone:1.1.0--py38h7be5676_2':
+        'biocontainers/dsltwoapptainervarone:1.1.0--py38h7be5676_2' }"
+
+    // truncated
+}
diff --git a/tests/data/mock_module_containers/modules/mock_dsl2_apptainer_var2.nf b/tests/data/mock_module_containers/modules/mock_dsl2_apptainer_var2.nf
new file mode 100644
index 0000000000..412c73d285
--- /dev/null
+++ b/tests/data/mock_module_containers/modules/mock_dsl2_apptainer_var2.nf
@@ -0,0 +1,10 @@
+process MOCK {
+    label 'process_fake'
+
+    conda "bioconda::dsltwoapptainervartwo=1.1.0"
+    container "${ ['singularity', 'apptainer'].contains(workflow.containerEngine) && !task.ext.singularity_pull_docker_container ?
+        'https://depot.galaxyproject.org/singularity/dsltwoapptainervartwo:1.1.0--hdfd78af_0':
+        'biocontainers/dsltwoapptainervartwo:1.1.0--hdfd78af_0' }"
+
+    // truncated
+}
diff --git a/tests/data/mock_module_containers/modules/mock_dsl2_current.nf b/tests/data/mock_module_containers/modules/mock_dsl2_current.nf
new file mode 100644
index 0000000000..65cd8086ac
--- /dev/null
+++ b/tests/data/mock_module_containers/modules/mock_dsl2_current.nf
@@ -0,0 +1,10 @@
+process MOCK {
+    label 'process_fake'
+
+    conda "bioconda::dsltwocurrent=1.2.1"
+    container "${ workflow.containerEngine == 'singularity' && !task.ext.singularity_pull_docker_container ?
+        'https://depot.galaxyproject.org/singularity/dsltwocurrent:1.2.1--pyhdfd78af_0':
+        'biocontainers/dsltwocurrent:1.2.1--pyhdfd78af_0' }"
+
+    // truncated
+}
diff --git a/tests/data/mock_module_containers/modules/mock_dsl2_current_inverted.nf b/tests/data/mock_module_containers/modules/mock_dsl2_current_inverted.nf
new file mode 100644
index 0000000000..d5a369c742
--- /dev/null
+++ b/tests/data/mock_module_containers/modules/mock_dsl2_current_inverted.nf
@@ -0,0 +1,10 @@
+process MOCK {
+    label 'process_fake'
+
+    conda "bioconda::dsltwocurrentinv=3.3.2"
+    container "${ !workflow.containerEngine == 'singularity' && task.ext.singularity_pull_docker_container ?
+        'biocontainers/dsltwocurrentinv:3.3.2--h1b792b2_1' :
+        'https://depot.galaxyproject.org/singularity/dsltwocurrentinv:3.3.2--h1b792b2_1' }"
+
+    // truncated
+}
diff --git a/tests/data/mock_module_containers/modules/mock_dsl2_old.nf b/tests/data/mock_module_containers/modules/mock_dsl2_old.nf
new file mode 100644
index 0000000000..11eace3b1c
--- /dev/null
+++ b/tests/data/mock_module_containers/modules/mock_dsl2_old.nf
@@ -0,0 +1,12 @@
+process MOCK {
+    label 'process_fake'
+
+    conda (params.enable_conda ? "bioconda::dsltwoold=0.23.0" : null)
+    if (workflow.containerEngine == 'singularity' && !params.singularity_pull_docker_container) {
+        container "https://depot.galaxyproject.org/singularity/dsltwoold:0.23.0--0"
+    } else {
+        container "quay.io/biocontainers/dsltwoold:0.23.0--0"
+    }
+
+    // truncated
+}
diff --git a/tests/data/mock_module_containers/modules/mock_dsl2_variable.nf b/tests/data/mock_module_containers/modules/mock_dsl2_variable.nf
new file mode 100644
index 0000000000..561254069a
--- /dev/null
+++ b/tests/data/mock_module_containers/modules/mock_dsl2_variable.nf
@@ -0,0 +1,19 @@
+process STAR_ALIGN {
+    // from rnaseq 3.7
+    label 'process_fake'
+
+    conda (params.enable_conda ? conda_str : null)
+    container "${ workflow.containerEngine == 'singularity' && !task.ext.singularity_pull_docker_container ?
+        "https://depot.galaxyproject.org/singularity/${container_id}" :
+        "quay.io/biocontainers/${container_id}" }"
+
+
+    // Note: 2.7X indices incompatible with AWS iGenomes so use older STAR version
+    conda_str = "bioconda::star=2.7.10a bioconda::samtools=1.15.1 conda-forge::gawk=5.1.0"
+    container_id = 'mulled-v2-1fa26d1ce03c295fe2fdcf85831a92fbcbd7e8c2:afaaa4c6f5b308b4b6aa2dd8e99e1466b2a6b0cd-0'
+    if (is_aws_igenome) {
+        conda_str = "bioconda::star=2.6.1d bioconda::samtools=1.10 conda-forge::gawk=5.1.0"
+        container_id = 'mulled-v2-1fa26d1ce03c295fe2fdcf85831a92fbcbd7e8c2:59cdd445419f14abac76b31dd0d71217994cbcc9-0'
+    }
+
+}
diff --git a/tests/test_download.py b/tests/test_download.py
index dd226d9dae..dfd78adcfb 100644
--- a/tests/test_download.py
+++ b/tests/test_download.py
@@ -174,10 +174,68 @@ def test__find_container_images_config_nextflow(self, tmp_path, mock_fetch_wf_co
         assert "nfcore/methylseq:1.4" in download_obj.containers
         assert "nfcore/sarek:dev" in download_obj.containers
         assert "https://depot.galaxyproject.org/singularity/r-shinyngs:1.7.1--r42hdfd78af_1" in download_obj.containers
-        # does not yet pick up nfcore/sarekvep:dev.${params.genome}, because successfully detecting "nfcore/sarek:dev"
-        # breaks the loop already. However, this loop-breaking is needed to stop iterating over DSL2 syntax if a
-        # direct download link has been found. Unless we employ a better deduplication, support for this kind of
-        # free-style if-else switches will sadly remain insufficient.
+        # does not yet pick up nfcore/sarekvep:dev.${params.genome}, because that is no valid URL or Docker URI.
+
+    #
+    # Test for 'find_container_images' in modules
+    #
+    @with_temporary_folder
+    @mock.patch("nf_core.utils.fetch_wf_config")
+    def test_find_container_images_modules(self, tmp_path, mock_fetch_wf_config):
+        download_obj = DownloadWorkflow(pipeline="dummy", outdir=tmp_path)
+        mock_fetch_wf_config.return_value = {}
+        download_obj.find_container_images(Path(__file__).resolve().parent / "data/mock_module_containers")
+
+        # mock_docker_single_quay_io.nf
+        assert "quay.io/biocontainers/singlequay:1.9--pyh9f0ad1d_0" in download_obj.containers
+
+        # mock_dsl2_apptainer_var1.nf (possible future convention?)
+        assert (
+            "https://depot.galaxyproject.org/singularity/dsltwoapptainervarone:1.1.0--py38h7be5676_2"
+            in download_obj.containers
+        )
+        assert "biocontainers/dsltwoapptainervarone:1.1.0--py38h7be5676_2" not in download_obj.containers
+
+        # mock_dsl2_apptainer_var2.nf (possible future convention?)
+        assert (
+            "https://depot.galaxyproject.org/singularity/dsltwoapptainervartwo:1.1.0--hdfd78af_0"
+            in download_obj.containers
+        )
+        assert "biocontainers/dsltwoapptainervartwo:1.1.0--hdfd78af_0" not in download_obj.containers
+
+        # mock_dsl2_current_inverted.nf (new implementation supports if the direct download URL is listed after Docker URI)
+        assert (
+            "https://depot.galaxyproject.org/singularity/dsltwocurrentinv:3.3.2--h1b792b2_1" in download_obj.containers
+        )
+        assert "biocontainers/dsltwocurrentinv:3.3.2--h1b792b2_1" not in download_obj.containers
+
+        # mock_dsl2_current.nf (main nf-core convention, should be the one in far the most modules)
+        assert (
+            "https://depot.galaxyproject.org/singularity/dsltwocurrent:1.2.1--pyhdfd78af_0" in download_obj.containers
+        )
+        assert "biocontainers/dsltwocurrent:1.2.1--pyhdfd78af_0" not in download_obj.containers
+
+        # mock_dsl2_old.nf (initial DSL2 convention)
+        assert "https://depot.galaxyproject.org/singularity/dsltwoold:0.23.0--0" in download_obj.containers
+        assert "quay.io/biocontainers/dsltwoold:0.23.0--0" not in download_obj.containers
+
+        # mock_dsl2_variable.nf (currently the edgiest edge case supported)
+        assert (
+            "https://depot.galaxyproject.org/singularity/mulled-v2-1fa26d1ce03c295fe2fdcf85831a92fbcbd7e8c2:59cdd445419f14abac76b31dd0d71217994cbcc9-0"
+            in download_obj.containers
+        )
+        assert (
+            "https://depot.galaxyproject.org/singularity/mulled-v2-1fa26d1ce03c295fe2fdcf85831a92fbcbd7e8c2:afaaa4c6f5b308b4b6aa2dd8e99e1466b2a6b0cd-0"
+            in download_obj.containers
+        )
+        assert (
+            "quay.io/biocontainers/mulled-v2-1fa26d1ce03c295fe2fdcf85831a92fbcbd7e8c2:59cdd445419f14abac76b31dd0d71217994cbcc9-0"
+            not in download_obj.containers
+        )
+        assert (
+            "quay.io/biocontainers/mulled-v2-1fa26d1ce03c295fe2fdcf85831a92fbcbd7e8c2:afaaa4c6f5b308b4b6aa2dd8e99e1466b2a6b0cd-0"
+            not in download_obj.containers
+        )
 
     #
     # Tests for 'singularity_pull_image'

From d8fadaefba32c38009e3fd1ce7b5066895278f13 Mon Sep 17 00:00:00 2001
From: Matthias Zepper <MatthiasZepper@users.noreply.github.com>
Date: Wed, 5 Jul 2023 22:18:27 +0200
Subject: [PATCH 04/17] Old DSL2 modules resolve correctly for the first time

---
 nf_core/download.py | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/nf_core/download.py b/nf_core/download.py
index 89bdb78d1f..04eefce53b 100644
--- a/nf_core/download.py
+++ b/nf_core/download.py
@@ -790,6 +790,9 @@ def rectify_raw_container_matches(self, raw_findings):
         # Thanks Stack Overflow for the regex: https://stackoverflow.com/a/39672069/713980
         docker_regex = r"^(?:(?=[^:\/]{1,253})(?!-)[a-zA-Z0-9-]{1,63}(?<!-)(?:\.(?!-)[a-zA-Z0-9-]{1,63}(?<!-))*(?::[0-9]{1,5})?/)?((?![._-])(?:[a-z0-9._-]*)(?<![._-])(?:/(?![._-])[a-z0-9._-]*(?<![._-]))*)(?::(?![.-])[a-zA-Z0-9_.-]{1,128})?$"
 
+        # at this point, we don't have to distinguish anymore, because we will later prioritize direct downloads over Docker URIs.
+        either_url_or_docker = re.compile(f"{url_regex}|{docker_regex}", re.S)
+
         for _, container_value, search_space, file_path in raw_findings:
             """
             Now we need to isolate all container paths (typically quoted strings) from the raw container_value
@@ -822,13 +825,15 @@ def rectify_raw_container_matches(self, raw_findings):
 
             Mostly, it is a nested DSL2 string, but it may also just be a plain string.
 
-            """
 
-            # first check if container_value it is a plain container URI like in DSL1 pipelines?
-            docker_match = re.match(docker_regex, container_value.strip(), re.S)
-            if docker_match:
-                cleaned_matches.append(docker_match.group(0))
-                continue  # skip further processing, we already have a match
+            First check if container_value it is a plain container URI like in DSL1 pipelines
+            or a plain URL like in the old DSL2 convention
+
+            """
+            direct_match = re.match(either_url_or_docker, container_value.strip())
+            if direct_match:
+                cleaned_matches.append(direct_match.group(0))
+                continue  # oh yes, that was plain sailing
 
             """
             no plain string, we likely need to break it up further
@@ -859,7 +864,6 @@ def rectify_raw_container_matches(self, raw_findings):
             At this point, we just add everything that is either a URL or a Docker URI to cleaned matches.
             """
 
-            either_url_or_docker = re.compile(f"{url_regex}|{docker_regex}")
             valid_containers = list(filter(either_url_or_docker.match, container_value_defs))
 
             if valid_containers:

From c3523c92025ffe17dc82e2ebd557d150e11e1bfb Mon Sep 17 00:00:00 2001
From: Matthias Zepper <MatthiasZepper@users.noreply.github.com>
Date: Wed, 5 Jul 2023 22:29:06 +0200
Subject: [PATCH 05/17] Got rid of a DeprecationWarning, that was triggered due
 to a multi-line string which I abused as comment.

---
 CHANGELOG.md        |  2 ++
 nf_core/download.py | 10 +++++-----
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 426225685a..f91434137f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,8 @@
 
 ### Template
 
+### Download
+- Improved container image resolution and prioritization of http downloads over Docker URIs.
 ### Linting
 
 ### Modules
diff --git a/nf_core/download.py b/nf_core/download.py
index 04eefce53b..0882fe0eb2 100644
--- a/nf_core/download.py
+++ b/nf_core/download.py
@@ -693,7 +693,7 @@ def find_container_images(self, workflow_directory):
 
                 # for DSL2 syntax in process scope of configs
                 config_regex = re.compile(
-                    r"[\s{}=$]*(?P<quote>(?<![\\])[\'\"])(?P<param>(?:.(?!(?<![\\])\1))*.?)\1[\s}]*"
+                    r"[\\s{}=$]*(?P<quote>(?<![\\])[\'\"])(?P<param>(?:.(?!(?<![\\])\1))*.?)\1[\\s}]*"
                 )
                 config_findings_dsl2 = re.findall(config_regex, v)
 
@@ -724,18 +724,18 @@ def find_container_images(self, workflow_directory):
                         search_space = fh.read()
                         """
                         Figure out which quotes were used and match everything until the closing quote.
-                        Since the other quote typically appears inside, a simple r"container\s*[\"\']([^\"\']*)[\"\']" unfortunately abridges the matches.
+                        Since the other quote typically appears inside, a simple r"container\\s*[\"\']([^\"\']*)[\"\']" unfortunately abridges the matches.
 
-                        container\s+[\s{}$=]* matches the literal word "container" followed by whitespace, brackets, equal or variable names.
+                        container\\s+[\\s{}$=]* matches the literal word "container" followed by whitespace, brackets, equal or variable names.
                         (?P<quote>[\'\"]) The quote character is captured into the quote group \1.
                         The pattern (?:.(?!\1))*.? is used to match any character (.) not followed by the closing quote character (?!\1).
                         This capture happens greedy *, but we add a .? to ensure that we don't match the whole file until the last occurrence
                         of the closing quote character, but rather stop at the first occurrence. \1 inserts the matched quote character into the regex, either " or '.
-                        It may be followed by whitespace or closing bracket [\s}]*
+                        It may be followed by whitespace or closing bracket [\\s}]*
                         re.DOTALL is used to account for the string to be spread out across multiple lines.
                         """
                         container_regex = re.compile(
-                            r"container\s+[\s{}=$]*(?P<quote>[\'\"])(?P<param>(?:.(?!\1))*.?)\1[\s}]*", re.DOTALL
+                            r"container\s+[\\s{}=$]*(?P<quote>[\'\"])(?P<param>(?:.(?!\1))*.?)\1[\\s}]*", re.DOTALL
                         )
 
                         local_module_findings = re.findall(container_regex, search_space)

From 60c9cefa2c00ccdf4daf5b4074e4eeb26d26af12 Mon Sep 17 00:00:00 2001
From: Matthias Zepper <MatthiasZepper@users.noreply.github.com>
Date: Wed, 5 Jul 2023 23:07:00 +0200
Subject: [PATCH 06/17] Changelog changes.

---
 CHANGELOG.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index f91434137f..c7d2c9c672 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,7 +5,7 @@
 ### Template
 
 ### Download
-- Improved container image resolution and prioritization of http downloads over Docker URIs.
+- Improved container image resolution and prioritization of http downloads over Docker URIs ([#2364](https://github.com/nf-core/tools/pull/2364)).
 ### Linting
 
 ### Modules

From 1ab228b2b4256c0493a630cbe747c3d63a5a8b15 Mon Sep 17 00:00:00 2001
From: Matthias Zepper <MatthiasZepper@users.noreply.github.com>
Date: Thu, 6 Jul 2023 16:35:28 +0200
Subject: [PATCH 07/17] I had never really realized that the functions in the
 DownloadWorkflow class are stateful, i.e. if the function is called without a
 parameter that was provided in a previous call, then it is still present.

---
 nf_core/download.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/nf_core/download.py b/nf_core/download.py
index 0882fe0eb2..7e7b45756d 100644
--- a/nf_core/download.py
+++ b/nf_core/download.py
@@ -917,7 +917,7 @@ def rectify_raw_container_matches(self, raw_findings):
         """
         return self.prioritize_direct_download(cleaned_matches)
 
-    def prioritize_direct_download(self, container_list, d={}):
+    def prioritize_direct_download(self, container_list):
         """
         Helper function that takes a list of container images (URLs and Docker URIs),
         eliminates all Docker URIs for which also a URL is contained and returns the
@@ -941,6 +941,7 @@ def prioritize_direct_download(self, container_list, d={}):
 
         A regex that matches http, r"^$|^http" could thus be used to prioritize the Docker URIs over http Downloads
         """
+        d = {}
         for c in container_list:
             log.info(c)
             if re.match(r"^$|(?!^http)", d.get(k := re.sub(".*/(.*)", "\\1", c), "")):

From 5a6de123b444030d2f42e559997d01e62c11dd6c Mon Sep 17 00:00:00 2001
From: Matthias Zepper <MatthiasZepper@users.noreply.github.com>
Date: Thu, 6 Jul 2023 17:08:30 +0200
Subject: [PATCH 08/17] Remove trailing whitespace from mock module so
 editorconfig-checker is happy.

---
 .../modules/mock_docker_single_quay_io.nf                       | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/data/mock_module_containers/modules/mock_docker_single_quay_io.nf b/tests/data/mock_module_containers/modules/mock_docker_single_quay_io.nf
index 185c178ae4..fad0e26e3d 100644
--- a/tests/data/mock_module_containers/modules/mock_docker_single_quay_io.nf
+++ b/tests/data/mock_module_containers/modules/mock_docker_single_quay_io.nf
@@ -1,6 +1,6 @@
 process MOCK {
     label 'process_fake'
-    
+
     conda     (params.enable_conda ? "bioconda::singlequay=1.9" : null)
     container "quay.io/biocontainers/singlequay:1.9--pyh9f0ad1d_0"
 

From 47388c36173decf992fdad9cb8f4c227d807d62d Mon Sep 17 00:00:00 2001
From: Matthias Zepper <MatthiasZepper@users.noreply.github.com>
Date: Thu, 6 Jul 2023 17:14:31 +0200
Subject: [PATCH 09/17] Run prettier on CHANGELOG.

---
 CHANGELOG.md | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index c7d2c9c672..a03ba8af70 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,7 +5,9 @@
 ### Template
 
 ### Download
+
 - Improved container image resolution and prioritization of http downloads over Docker URIs ([#2364](https://github.com/nf-core/tools/pull/2364)).
+
 ### Linting
 
 ### Modules

From e459c661e313ae8096914ff1039995175f3cc722 Mon Sep 17 00:00:00 2001
From: Matthias Zepper <MatthiasZepper@users.noreply.github.com>
Date: Fri, 14 Jul 2023 15:03:51 +0200
Subject: [PATCH 10/17] Commit review suggestions by @mirpedrol.

---
 nf_core/download.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/nf_core/download.py b/nf_core/download.py
index 7e7b45756d..cf95d27a14 100644
--- a/nf_core/download.py
+++ b/nf_core/download.py
@@ -826,7 +826,7 @@ def rectify_raw_container_matches(self, raw_findings):
             Mostly, it is a nested DSL2 string, but it may also just be a plain string.
 
 
-            First check if container_value it is a plain container URI like in DSL1 pipelines
+            First, check if container_value is a plain container URI like in DSL1 pipelines
             or a plain URL like in the old DSL2 convention
 
             """
@@ -943,7 +943,6 @@ def prioritize_direct_download(self, container_list):
         """
         d = {}
         for c in container_list:
-            log.info(c)
             if re.match(r"^$|(?!^http)", d.get(k := re.sub(".*/(.*)", "\\1", c), "")):
                 log.debug(f"{c} matches and will be saved as {k}")
                 d[k] = c

From dc426c9739b5ee60fe595336367314e5912fe942 Mon Sep 17 00:00:00 2001
From: mirpedrol <mirp.julia@gmail.com>
Date: Mon, 17 Jul 2023 13:17:48 +0200
Subject: [PATCH 11/17] fix prompt pipeline revision during launch

---
 nf_core/launch.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/nf_core/launch.py b/nf_core/launch.py
index 648c8775f8..363506c448 100644
--- a/nf_core/launch.py
+++ b/nf_core/launch.py
@@ -216,7 +216,7 @@ def get_pipeline_schema(self):
                     log.error(e)
                     return False
 
-                self.pipeline_revision = nf_core.utils.prompt_pipeline_release_branch(wf_releases, wf_branches)
+                self.pipeline_revision, _ = nf_core.utils.prompt_pipeline_release_branch(wf_releases, wf_branches)
             self.nextflow_cmd += f" -r {self.pipeline_revision}"
 
         # Get schema from name, load it and lint it

From e21220385836074e31ad66a13a5c05d0a23364f1 Mon Sep 17 00:00:00 2001
From: mirpedrol <mirp.julia@gmail.com>
Date: Mon, 17 Jul 2023 13:19:20 +0200
Subject: [PATCH 12/17] update changelog

---
 CHANGELOG.md | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a03ba8af70..0afac4e62e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -16,6 +16,8 @@
 
 ### General
 
+- Fix prompt pipeline revision during launch ([#2375](https://github.com/nf-core/tools/pull/2375))
+
 # [v2.9 - Chromium Falcon](https://github.com/nf-core/tools/releases/tag/2.9) + [2023-06-29]
 
 ### Template

From 97f10433a7b33af6a1203cd58336ecd3715beb68 Mon Sep 17 00:00:00 2001
From: mirpedrol <mirp.julia@gmail.com>
Date: Mon, 17 Jul 2023 14:23:37 +0200
Subject: [PATCH 13/17] remove default false from json schema

---
 nf_core/pipeline-template/nextflow_schema.json | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/nf_core/pipeline-template/nextflow_schema.json b/nf_core/pipeline-template/nextflow_schema.json
index 319503c37f..49549d0464 100644
--- a/nf_core/pipeline-template/nextflow_schema.json
+++ b/nf_core/pipeline-template/nextflow_schema.json
@@ -175,14 +175,12 @@
                     "type": "boolean",
                     "description": "Display help text.",
                     "fa_icon": "fas fa-question-circle",
-                    "default": false,
                     "hidden": true
                 },
                 "version": {
                     "type": "boolean",
                     "description": "Display version and exit.",
                     "fa_icon": "fas fa-question-circle",
-                    "default": false,
                     "hidden": true
                 },
                 "publish_dir_mode": {
@@ -206,7 +204,6 @@
                     "type": "boolean",
                     "description": "Send plain-text email instead of HTML.",
                     "fa_icon": "fas fa-remove-format",
-                    "default": false,
                     "hidden": true
                 },
                 "max_multiqc_email_size": {
@@ -221,7 +218,6 @@
                     "type": "boolean",
                     "description": "Do not use coloured log outputs.",
                     "fa_icon": "fas fa-palette",
-                    "default": false,
                     "hidden": true
                 },
                 "hook_url": {
@@ -260,7 +256,6 @@
                     "type": "boolean",
                     "fa_icon": "far fa-eye-slash",
                     "description": "Show all params when using `--help`",
-                    "default": false,
                     "hidden": true,
                     "help_text": "By default, parameters set as _hidden_ in the schema are not shown on the command line when a user runs with `--help`. Specifying this option will tell the pipeline to show all parameters."
                 },
@@ -268,7 +263,6 @@
                     "type": "boolean",
                     "fa_icon": "far fa-check-circle",
                     "description": "Validation of parameters fails when an unrecognised parameter is found.",
-                    "default": false,
                     "hidden": true,
                     "help_text": "By default, when an unrecognised parameter is found, it returns a warinig."
                 },
@@ -276,7 +270,6 @@
                     "type": "boolean",
                     "fa_icon": "far fa-check-circle",
                     "description": "Validation of parameters in lenient more.",
-                    "default": false,
                     "hidden": true,
                     "help_text": "Allows string values that are parseable as numbers or booleans. For further information see [JSONSchema docs](https://github.com/everit-org/json-schema#lenient-mode)."
                 }

From 61f0216df3611b1ae9c18f204397a2977dbc173b Mon Sep 17 00:00:00 2001
From: mirpedrol <mirp.julia@gmail.com>
Date: Mon, 17 Jul 2023 14:25:48 +0200
Subject: [PATCH 14/17] update changelog

---
 CHANGELOG.md | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a03ba8af70..cabb9a9c50 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,8 @@
 
 ### Template
 
+- Remove default false from nextflow_schema.json ([#2375](https://github.com/nf-core/tools/pull/2375))
+
 ### Download
 
 - Improved container image resolution and prioritization of http downloads over Docker URIs ([#2364](https://github.com/nf-core/tools/pull/2364)).

From 24f5236fc7a5c03fd9f34adc8e500d64f6d282f2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=BAlia=20Mir=20Pedrol?= <mirp.julia@gmail.com>
Date: Mon, 17 Jul 2023 14:29:08 +0200
Subject: [PATCH 15/17] Update CHANGELOG.md

---
 CHANGELOG.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index cabb9a9c50..54b08db292 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,7 +4,7 @@
 
 ### Template
 
-- Remove default false from nextflow_schema.json ([#2375](https://github.com/nf-core/tools/pull/2375))
+- Remove default false from nextflow_schema.json ([#2376](https://github.com/nf-core/tools/pull/2376))
 
 ### Download
 

From e414cc8f3f3c389556ff8aa108ee9e13e2b7159f Mon Sep 17 00:00:00 2001
From: mirpedrol <mirp.julia@gmail.com>
Date: Mon, 17 Jul 2023 14:31:50 +0200
Subject: [PATCH 16/17] add multiqc to modules.config

---
 nf_core/pipeline-template/conf/modules.config | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/nf_core/pipeline-template/conf/modules.config b/nf_core/pipeline-template/conf/modules.config
index da58a5d881..39e8138653 100644
--- a/nf_core/pipeline-template/conf/modules.config
+++ b/nf_core/pipeline-template/conf/modules.config
@@ -38,4 +38,13 @@ process {
         ]
     }
 
+    withName: 'MULTIQC' {
+        ext.args   = params.multiqc_title ? "--title \"$params.multiqc_title\"" : ''
+        publishDir = [
+            path: { "${params.outdir}/multiqc" },
+            mode: params.publish_dir_mode,
+            saveAs: { filename -> filename.equals('versions.yml') ? null : filename }
+        ]
+    }
+
 }

From 7fffb4ba2baa8dd59a80fbb926fedfa428d2ae79 Mon Sep 17 00:00:00 2001
From: mirpedrol <mirp.julia@gmail.com>
Date: Mon, 17 Jul 2023 14:32:55 +0200
Subject: [PATCH 17/17] update changelog

---
 CHANGELOG.md | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a03ba8af70..55dfa2a81d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,8 @@
 
 ### Template
 
+- Add module MULTIQC to modules.config ([#2377](https://github.com/nf-core/tools/pull/2377))
+
 ### Download
 
 - Improved container image resolution and prioritization of http downloads over Docker URIs ([#2364](https://github.com/nf-core/tools/pull/2364)).