distinguish between update of one module and general container generation to speed things up

mashehu · mashehu · commit f34d28afd084 · 2026-05-11T15:10:19.000+02:00
diff --git a/nf_core/pipelines/containers_utils.py b/nf_core/pipelines/containers_utils.py
@@ -1,13 +1,14 @@
 import contextlib
 import logging
 import re
+from concurrent.futures import ThreadPoolExecutor, as_completed
 from pathlib import Path
 
 import yaml
 
-log = logging.getLogger(__name__)
+from nf_core.utils import read_module_name
 
-_PROCESS_NAME_RE = re.compile(r"^\s*process\s+(\w+)\s*\{", re.MULTILINE)
+log = logging.getLogger(__name__)
 
 PLATFORMS: dict[str, list[str]] = {
     "docker_amd64": ["docker", "linux/amd64", "name"],
@@ -20,6 +21,69 @@
     "conda_lock_files_arm64": ["conda", "linux/arm64", "lock_file"],
 }
 
+_CONFIG_LINE_RE = re.compile(r"withName:\s*'(\w+)'\s*\{\s*(?:container|conda)\s*=\s*'([^']+)'")
+
+
+def _container_key(platform: str) -> str:
+    return "conda" if platform.startswith("conda_lock_") else "container"
+
+
+def _parse_config_file(config_path: Path) -> dict[str, str]:
+    """Return {module_name: container} from an existing platform config file."""
+    result: dict[str, str] = {}
+    with contextlib.suppress(OSError):
+        for line in config_path.read_text().splitlines():
+            m = _CONFIG_LINE_RE.search(line)
+            if m:
+                result[m.group(1)] = m.group(2)
+    return result
+
+
+def _write_platform_config(config_path: Path, entries: dict[str, str], key: str) -> bool:
+    """Write *entries* to *config_path*, or delete it when empty.
+
+    Skips the write when content is unchanged.  Returns True if the file now exists.
+    """
+    if not entries:
+        config_path.unlink(missing_ok=True)
+        return False
+    new_content = "".join(
+        f"process {{ withName: '{name}' {{ {key} = '{container}' }} }}\n" for name, container in sorted(entries.items())
+    )
+    if not config_path.exists() or config_path.read_text() != new_content:
+        config_path.write_text(new_content)
+    return True
+
+
+def _process_meta(meta_path: Path) -> tuple[str, dict[str, str]] | None:
+    """Read one meta.yml + sibling main.nf and return (module_name, {platform: container}).
+
+    Returns None when the file should be skipped.
+    """
+    try:
+        raw = meta_path.read_bytes()
+    except OSError as e:
+        log.debug(f"Could not read {meta_path}: {e}")
+        return None
+
+    # TODO: remove this early-exit once containers are present in the majority of modules
+    if b"containers:" not in raw:
+        return None
+
+    meta = yaml.safe_load(raw)
+
+    module_name = read_module_name(meta_path.parent / "main.nf")
+    if not module_name:
+        log.debug(f"No process definition found next to {meta_path}, skipping")
+        return None
+
+    platform_containers: dict[str, str] = {}
+    for platform_name, (runtime, arch, protocol) in PLATFORMS.items():
+        with contextlib.suppress(KeyError, TypeError):
+            platform_containers[platform_name] = meta["containers"][runtime][arch][protocol]
+
+    return module_name, platform_containers
+
 
 class ContainerConfigs:
     """Generates the container configuration files for a pipeline.
@@ -28,78 +92,82 @@ class ContainerConfigs:
         workflow_directory (Path): The directory containing the workflow files.
     """
 
-    def __init__(
-        self,
-        workflow_directory: Path = Path(),
-    ):
+    def __init__(self, workflow_directory: Path = Path()) -> None:
         self.workflow_directory = workflow_directory
 
-    def generate_container_configs(
-        self, new_module_path: Path | None = None, new_module_name: str | None = None
-    ) -> set[str]:
+    def update_module_container_config(self, module_path: Path) -> None:
+        """Targeted update for a single module.
+
+        Reads the current config files, splices in (or removes) the entry for
+        the module at *module_path*, and writes back only what changed.
         """
-        Generate the container configuration files for a pipeline by scanning
-        all ``main.nf`` files under the ``modules/`` directory and reading the
-        accompanying ``meta.yml`` for container information.
+        result = _process_meta(module_path / "meta.yml")
+
+        if result is None:
+            module_name = read_module_name(module_path / "main.nf")
+            if not module_name:
+                log.debug(f"Could not determine process name for {module_path}, skipping")
+                return
+            platform_containers: dict[str, str] = {}
+        else:
+            module_name, platform_containers = result
+
+        conf_dir = self.workflow_directory / "conf"
+        for platform in PLATFORMS:
+            config_path = conf_dir / f"containers_{platform}.config"
+            entries = _parse_config_file(config_path)
+            if platform in platform_containers:
+                entries[module_name] = platform_containers[platform]
+            _write_platform_config(config_path, entries, _container_key(platform))
+
+    def generate_container_configs(self) -> set[str]:
+        """Full scan of all ``meta.yml`` files under ``modules/``.
+
+        Used by lint and bulk operations (update, remove, patch) where multiple
+        modules may have changed.
 
         Returns:
             set[str]: Names of config files written (e.g. ``{'containers_docker_amd64.config'}``).
         """
+        modules_dir = self.workflow_directory / "modules"
+        if not modules_dir.is_dir():
+            log.debug(f"No modules directory found at {modules_dir}, skipping")
+            return set()
+
         containers: dict[str, dict[str, str]] = {platform: {} for platform in PLATFORMS}
 
-        for meta_path in (self.workflow_directory / "modules").rglob("meta.yml"):
-            try:
-                content = meta_path.read_text()
-            except OSError as e:
-                log.debug(f"Could not read {meta_path}: {e}")
-                continue
-
-            # TODO: remove this early-exit once containers are present in the majority of modules
-            if "containers:" not in content:
-                continue
-
-            meta = yaml.safe_load(content)
-
-            main_nf = meta_path.parent / "main.nf"
-            try:
-                match = _PROCESS_NAME_RE.search(main_nf.read_text())
-            except OSError:
-                log.debug(f"No main.nf next to {meta_path}, skipping")
-                continue
-            if not match:
-                log.debug(f"No process definition found in {main_nf}, skipping")
-                continue
-            process_name = match.group(1)
-
-            for platform_name, (runtime, arch, protocol) in PLATFORMS.items():
-                with contextlib.suppress(KeyError, TypeError):
-                    containers[platform_name][process_name] = meta["containers"][runtime][arch][protocol]
+        with ThreadPoolExecutor() as pool:
+            futures = {pool.submit(_process_meta, p): p for p in modules_dir.rglob("meta.yml")}
+            for future in as_completed(futures):
+                result = future.result()
+                if result is None:
+                    continue
+                module_name, platform_containers = result
+                for platform_name, container in platform_containers.items():
+                    containers[platform_name][module_name] = container
 
         log.info("Generated container configs for the pipeline.")
 
-        # remove all generated config files, to handle removed modules
-        for platform in PLATFORMS:
-            (self.workflow_directory / "conf" / f"containers_{platform}.config").unlink(missing_ok=True)
-        # write config files
         written: set[str] = set()
         for platform, module_containers in containers.items():
-            if not module_containers:
-                continue
-            container_key = "conda" if platform.startswith("conda_lock_") else "container"
-            lines = [
-                f"process {{ withName: '{module_name}' {{ {container_key} = '{container}' }} }}\n"
-                for module_name, container in sorted(module_containers.items())
-            ]
             config_path = self.workflow_directory / "conf" / f"containers_{platform}.config"
-            config_path.write_text("".join(lines))
-            written.add(config_path.name)
+            if _write_platform_config(config_path, module_containers, _container_key(platform)):
+                written.add(config_path.name)
         return written
 
 
-def try_generate_container_configs(
-    directory: Path, new_module_path: Path | None = None, new_module_name: str | None = None
-) -> None:
+def try_generate_container_configs(directory: Path, module_path: Path | None = None) -> None:
+    """Regenerate container configs for *directory*.
+
+    If *module_path* is given, only that module's entries are updated (fast
+    path for single-module installs).  Otherwise a full scan of ``modules/``
+    is performed.
+    """
     try:
-        ContainerConfigs(directory).generate_container_configs(new_module_path, new_module_name)
+        configs = ContainerConfigs(directory)
+        if module_path is not None:
+            configs.update_module_container_config(module_path)
+        else:
+            configs.generate_container_configs()
     except UserWarning as e:
         log.warning(f"Could not regenerate container configuration files: {e}")
diff --git a/nf_core/utils.py b/nf_core/utils.py
@@ -369,6 +369,18 @@ def check_nextflow_version(minimal_nf_version: tuple[int, int, int, bool], silen
     return nf_version >= minimal_nf_version
 
 
+_NF_PROCESS_NAME_RE = re.compile(r"^\s*process\s+(\w+)\s*\{", re.MULTILINE)
+
+
+def read_module_name(main_nf: Path) -> str | None:
+    """Return the process name declared in a Nextflow ``main.nf`` file, or ``None``."""
+    try:
+        match = _NF_PROCESS_NAME_RE.search(main_nf.read_text())
+        return match.group(1) if match else None
+    except OSError:
+        return None
+
+
 def fetch_wf_config(wf_path: Path, cache_config: bool = True) -> dict:
     """Uses Nextflow to retrieve the the configuration variables
     from a Nextflow workflow.
diff --git a/tests/pipelines/test_container_configs.py b/tests/pipelines/test_container_configs.py
@@ -1,6 +1,6 @@
 """Tests for the ContainerConfigs helper used by pipelines."""
 
-from pathlib import Path
+import shutil
 
 import ruamel.yaml
 
@@ -53,11 +53,7 @@ def test_generate_container_configs_newly_installed_module(self) -> None:
         with open(self.pipeline_dir / "modules" / "nf-core" / "fastqc" / "meta.yml") as fh:
             fastqc_meta_yml = yaml.load(fh)
 
-        # new_module_path/name are kept for backward compat but the scan finds the module regardless
-        self.container_configs.generate_container_configs(
-            new_module_path=Path("modules/nf-core/fastqc"),
-            new_module_name="fastqc",
-        )
+        self.container_configs.generate_container_configs()
 
         conf_dir = self.pipeline_dir / "conf"
         for p_name, (runtime, arch, protocol) in PLATFORMS.items():
@@ -82,3 +78,47 @@ def test_generate_container_configs_removes_stale_entries(self) -> None:
                 assert stale_line not in cfg_path.read_text(), (
                     f"{cfg_path.name} still contains stale entry after regeneration"
                 )
+
+    def test_generate_container_configs_no_modules_dir(self) -> None:
+        """Returns an empty set immediately when there is no modules/ directory."""
+        shutil.rmtree(self.pipeline_dir / "modules", ignore_errors=True)
+        result = self.container_configs.generate_container_configs()
+        assert result == set()
+
+    def test_generate_container_configs_skips_meta_without_containers(self) -> None:
+        """meta.yml files without a containers key are silently ignored."""
+        module_dir = self.pipeline_dir / "modules" / "local" / "fake"
+        module_dir.mkdir(parents=True, exist_ok=True)
+        (module_dir / "meta.yml").write_text("name: fake\ndescription: no containers here\n")
+        (module_dir / "main.nf").write_text('process FAKE {\n  script:\n  """\n  echo hello\n  """\n}\n')
+
+        self.container_configs.generate_container_configs()
+
+        conf_dir = self.pipeline_dir / "conf"
+        for p_name in PLATFORMS:
+            cfg_path = conf_dir / f"containers_{p_name}.config"
+            if cfg_path.exists():
+                assert "FAKE" not in cfg_path.read_text(), f"{cfg_path.name} contains entry for FAKE module"
+
+    def test_generate_container_configs_skips_unchanged_write(self) -> None:
+        """Config files are not rewritten when content has not changed."""
+        mods_install = ModuleInstall(
+            self.pipeline_dir, prompt=False, force=False, sha="79b36b51048048374b642289bfe9e591ef56fe05"
+        )
+        mods_install.install("fastqc")
+
+        self.container_configs.generate_container_configs()
+
+        conf_dir = self.pipeline_dir / "conf"
+        mtimes = {
+            p: (conf_dir / f"containers_{p}.config").stat().st_mtime_ns
+            for p in PLATFORMS
+            if (conf_dir / f"containers_{p}.config").exists()
+        }
+        assert mtimes, "Expected at least one config file to be generated on the first run"
+
+        self.container_configs.generate_container_configs()
+
+        for p_name, mtime_before in mtimes.items():
+            mtime_after = (conf_dir / f"containers_{p_name}.config").stat().st_mtime_ns
+            assert mtime_after == mtime_before, f"conf/{p_name}.config was rewritten unnecessarily"
