Add branch field to module entries in modules.json to record what branch a module comes from

CHANGELOG.md

@@ -12,6 +12,7 @@
 - Remove call to `getGenomeAttribute` in `main.nf` when running `nf-core create` without iGenomes ([#1670](https://github.com/nf-core/tools/issues/1670))
 - Make `nf-core create` fail if Git default branch name is dev or TEMPLATE ([#1705](https://github.com/nf-core/tools/pull/1705))
 - Convert `console` snippets to `bash` snippets in the template where applicable ([#1729](https://github.com/nf-core/tools/pull/1729))
+- Add `branch` field to module entries in `modules.json` to record what branch a module was installed from ([#1728](https://github.com/nf-core/tools/issues/1728))
 
 ### Linting
 
@@ -68,6 +69,7 @@
 - Make `nf-core modules` commands work with arbitrary git remotes ([#1721](https://github.com/nf-core/tools/issues/1721))
 - Add links in `README.md` for `info` and `patch` commands ([#1722](https://github.com/nf-core/tools/issues/1722)])
 - Fix misc. issues with `--branch` and `--base-path` ([#1726](https://github.com/nf-core/tools/issues/1726))
+- Add `branch` field to module entries in `modules.json` to record what branch a module was installed from ([#1728](https://github.com/nf-core/tools/issues/1728))
 
 ## [v2.4.1 - Cobolt Koala Patch](https://github.com/nf-core/tools/releases/tag/2.4) - [2022-05-16]
 

nf_core/__main__.py

@@ -437,7 +437,7 @@ def local(ctx, keywords, json, dir):
         )
         print(module_list.list_modules(keywords, json))
     except (UserWarning, LookupError) as e:
-        log.critical(e)
+        log.error(e)
         sys.exit(1)
 
 

nf_core/lint/modules_json.py

@@ -36,13 +36,16 @@ def modules_json(self):
                 )
                 continue
 
-            for module in modules_json_dict["repos"][repo]["modules"]:
+            for module, module_entry in modules_json_dict["repos"][repo]["modules"].items():
                 if not Path(modules_dir, repo, module).exists():
                     failed.append(
                         f"Entry for `{Path(repo, module)}` found in `modules.json` but module is not installed in pipeline."
                     )
                     all_modules_passed = False
-
+                if module_entry.get("branch") is None:
+                    failed.append(f"Entry for `{Path(repo, module)}` is missing branch information.")
+                if module_entry.get("git_sha") is None:
+                    failed.append(f"Entry for `{Path(repo, module)}` is missing version information.")
         if all_modules_passed:
             passed.append("Only installed modules found in `modules.json`")
     else:

nf_core/modules/list.py

@@ -102,7 +102,9 @@ def pattern_msg(keywords):
                         try:
                             # pass repo_name to get info on modules even outside nf-core/modules
                             message, date = ModulesRepo(
-                                remote_url=repo_entry["git_url"], base_path=repo_entry["base_path"]
+                                remote_url=repo_entry["git_url"],
+                                base_path=repo_entry["base_path"],
+                                branch=module_entry["branch"],
                             ).get_commit_info(version_sha)
                         except LookupError as e:
                             log.warning(e)

nf_core/modules/modules_repo.py

@@ -2,6 +2,7 @@
 import logging
 import os
 import shutil
+from pathlib import Path
 
 import git
 import rich.progress
@@ -16,6 +17,7 @@
 NF_CORE_MODULES_NAME = "nf-core/modules"
 NF_CORE_MODULES_REMOTE = "https://github.com/nf-core/modules.git"
 NF_CORE_MODULES_BASE_PATH = "modules"
+NF_CORE_MODULES_DEFAULT_BRANCH = "master"
 
 
 class RemoteProgressbar(git.RemoteProgress):
@@ -84,6 +86,31 @@ def update_local_repo_status(repo_name, up_to_date):
         """
         ModulesRepo.local_repo_statuses[repo_name] = up_to_date
 
+    @staticmethod
+    def get_remote_branches(remote_url):
+        """
+        Get all branches from a remote repository
+
+        Args:
+            remote_url (str): The git url to the remote repository
+
+        Returns:
+            (set[str]): All branches found in the remote
+        """
+        try:
+            unparsed_branches = git.Git().ls_remote(remote_url)
+        except git.GitCommandError:
+            raise LookupError(f"Was unable to fetch branches from '{remote_url}'")
+        else:
+            branches = {}
+            for branch_info in unparsed_branches.split("\n"):
+                sha, name = branch_info.split("\t")
+                if name != "HEAD":
+                    # The remote branches are shown as 'ref/head/branch'
+                    branch_name = Path(name).stem
+                    branches[sha] = branch_name
+            return set(branches.values())
+
     def __init__(self, remote_url=None, branch=None, no_pull=False, base_path=None, no_progress=False):
         """
         Initializes the object and clones the git repository if it is not already present

nf_core/modules/patch.py

@@ -51,6 +51,11 @@ def patch(self, module=None):
             raise UserWarning(
                 f"The '{module_fullname}' module does not have a valid version in the 'modules.json' file. Cannot compute patch"
             )
+        # Get the module branch and reset it in the ModulesRepo object
+        module_branch = self.modules_json.get_module_branch(module, self.modules_repo.fullname)
+        if module_branch != self.modules_repo.branch:
+            self.modules_repo.setup_branch(module_branch)
+
         # Set the diff filename based on the module name
         patch_filename = f"{module.replace('/', '-')}.diff"
         module_relpath = Path("modules", self.modules_repo.fullname, module)

nf_core/modules/update.py

@@ -1,5 +1,4 @@
 import enum
-import json
 import logging
 import os
 import shutil
@@ -45,6 +44,7 @@ def __init__(
         self.module = None
         self.update_config = None
         self.modules_json = ModulesJson(self.dir)
+        self.branch = branch
 
     class DiffEnum(enum.Enum):
         """Enumeration to keeping track of file diffs.
@@ -142,8 +142,9 @@ def update(self, module=None):
         # and do the requested action on them
         exit_value = True
         all_patches_successful = True
+        print(modules_info)
         for modules_repo, module, sha, patch_relpath in modules_info:
-
+            print(sha)
             module_fullname = str(Path(modules_repo.fullname, module))
             # Are we updating the files in place or not?
             dry_run = self.show_diff or self.save_diff_fn
@@ -316,12 +317,25 @@ def get_single_module_info(self, module):
                     log.info(f"Found entry in '.nf-core.yml' for module '{module}'")
                 log.info(f"Updating module to ({sha})")
 
+        # Check if the update branch is the same as the installation branch
+        current_branch = self.modules_json.get_module_branch(module, self.modules_repo.fullname)
+        new_branch = self.modules_repo.branch
+        if current_branch != new_branch:
+            log.warning(
+                f"You are trying to update the '{Path(self.modules_repo.fullname, module)}' module from "
+                f"the '{new_branch}' branch. This module was installed from the '{current_branch}'"
+            )
+            switch = questionary.confirm(f"Do you want to update using the '{current_branch}' instead?").unsafe_ask()
+            if switch:
+                # Change the branch
+                self.modules_repo.setup_branch(current_branch)
+
         # If there is a patch file, get its filename
         patch_fn = self.modules_json.get_patch_fn(module, self.modules_repo.fullname)
 
         return (self.modules_repo, module, sha, patch_fn)
 
-    def get_all_modules_info(self):
+    def get_all_modules_info(self, branch=None):
         """Collects the module repository, version and sha for all modules.
 
         Information about the module version in the '.nf-core.yml' overrides the '--sha' option.
@@ -330,6 +344,12 @@ def get_all_modules_info(self):
             [(ModulesRepo, str, str)]: A list of tuples containing a ModulesRepo object,
             the module name, and the module version.
         """
+        if branch is not None:
+            use_branch = questionary.confirm(
+                "'--branch' was specified. Should this branch be used to update all modules?", default=False
+            )
+            if not use_branch:
+                branch = None
         skipped_repos = []
         skipped_modules = []
         overridden_repos = []
@@ -339,18 +359,24 @@ def get_all_modules_info(self):
         # and check if they have an entry in the '.nf-core.yml' file
         for repo_name, modules in self.modules_json.get_all_modules().items():
             if repo_name not in self.update_config or self.update_config[repo_name] is True:
-                modules_info[repo_name] = [(module, self.sha) for module in modules]
+                modules_info[repo_name] = [
+                    (module, self.sha, self.modules_json.get_module_branch(module, repo_name)) for module in modules
+                ]
             elif isinstance(self.update_config[repo_name], dict):
                 # If it is a dict, then there are entries for individual modules
                 repo_config = self.update_config[repo_name]
                 modules_info[repo_name] = []
                 for module in modules:
                     if module not in repo_config or repo_config[module] is True:
-                        modules_info[repo_name].append((module, self.sha))
+                        modules_info[repo_name].append(
+                            (module, self.sha, self.modules_json.get_module_branch(module, repo_name))
+                        )
                     elif isinstance(repo_config[module], str):
                         # If a string is given it is the commit SHA to which we should update to
                         custom_sha = repo_config[module]
-                        modules_info[repo_name].append((module, custom_sha))
+                        modules_info[repo_name].append(
+                            (module, custom_sha, self.modules_json.get_module_branch(module, repo_name))
+                        )
                         if self.sha is not None:
                             overridden_modules.append(module)
                     elif repo_config[module] is False:
@@ -361,7 +387,10 @@ def get_all_modules_info(self):
             elif isinstance(self.update_config[repo_name], str):
                 # If a string is given it is the commit SHA to which we should update to
                 custom_sha = self.update_config[repo_name]
-                modules_info[repo_name] = [(module_name, custom_sha) for module_name in modules]
+                modules_info[repo_name] = [
+                    (module_name, custom_sha, self.modules_json.get_module_branch(module_name, repo_name))
+                    for module_name in modules
+                ]
                 if self.sha is not None:
                     overridden_repos.append(repo_name)
             elif self.update_config[repo_name] is False:
@@ -390,32 +419,51 @@ def get_all_modules_info(self):
                 f"Overriding '--sha' flag for module{plural_s(overridden_modules)} with "
                 f"'.nf-core.yml' entry: '{overridden_str}'"
             )
+        # Loop through modules_info and create on ModulesRepo object per remote and branch
+        repos_and_branches = {}
+        for repo_name, mods in modules_info.items():
+            for mod, sha, mod_branch in mods:
+                if branch is not None:
+                    mod_branch = branch
+                if (repo_name, mod_branch) not in repos_and_branches:
+                    repos_and_branches[(repo_name, mod_branch)] = []
+                repos_and_branches[(repo_name, mod_branch)].append((mod, sha))
 
         # Get the git urls from the modules.json
-        modules_info = [
-            (self.modules_json.get_git_url(repo_name), self.modules_json.get_base_path(repo_name), mods_shas)
-            for repo_name, mods_shas in modules_info.items()
-        ]
+        modules_info = (
+            (self.modules_json.get_git_url(repo_name), branch, self.modules_json.get_base_path(repo_name), mods_shas)
+            for (repo_name, branch), mods_shas in repos_and_branches.items()
+        )
 
         # Create ModulesRepo objects
-        modules_info = [
-            (ModulesRepo(remote_url=repo_url, base_path=base_path), mods_shas)
-            for repo_url, base_path, mods_shas in modules_info
-        ]
+        repo_objs_mods = []
+        for repo_url, branch, base_path, mods_shas in modules_info:
+            try:
+                modules_repo = ModulesRepo(remote_url=repo_url, branch=branch, base_path=base_path)
+            except LookupError as e:
+                log.warning(e)
+                log.info(f"Skipping modules in '{modules_repo.fullname}'")
+            else:
+                repo_objs_mods.append((modules_repo, mods_shas))
 
-        # Flatten and return the list
-        modules_info = [(repo, mod, sha) for repo, mods_shas in modules_info for mod, sha in mods_shas]
+        # Flatten the list
+        modules_info = [(repo, mod, sha) for repo, mods_shas in repo_objs_mods for mod, sha in mods_shas]
 
-        # Verify that that all modules exist in their respective ModulesRepo,
+        # Verify that that all modules and shas exist in their respective ModulesRepo,
         # don't try to update those that don't
         i = 0
         while i < len(modules_info):
             repo, module, _ = modules_info[i]
-            if repo.module_exists(module):
-                i += 1
-            else:
+            if not repo.module_exists(module):
                 log.warning(f"Module '{module}' does not exist in '{repo.fullname}'. Skipping...")
                 modules_info.pop(i)
+            elif sha is not None and not repo.sha_exists_on_branch(sha):
+                log.warning(
+                    f"Git sha '{sha}' does not exists on the '{branch}' of '{repo.fullname}'. Skipping module '{mod}'"
+                )
+                modules_info.pop(i)
+            else:
+                i += 1
 
         # Add patch filenames to the modules that have them
         modules_info = [
@@ -427,7 +475,7 @@ def get_all_modules_info(self):
     def setup_diff_file(self):
         """Sets up the diff file.
 
-        If the save diff option was choosen interactively, the user is asked to supply a name for the diff file.
+        If the save diff option was chosen interactively, the user is asked to supply a name for the diff file.
 
         Then creates the file for saving the diff.
         """

nf_core/pipeline-template/modules.json

@@ -7,13 +7,16 @@
             "git_url": "https://github.com/nf-core/modules.git",
             "modules": {
                 "custom/dumpsoftwareversions": {
-                    "git_sha": "e745e167c1020928ef20ea1397b6b4d230681b4d"
+                    "git_sha": "e745e167c1020928ef20ea1397b6b4d230681b4d",
+                    "branch": "master"
                 },
                 "fastqc": {
-                    "git_sha": "e745e167c1020928ef20ea1397b6b4d230681b4d"
+                    "git_sha": "e745e167c1020928ef20ea1397b6b4d230681b4d",
+                    "branch": "master"
                 },
                 "multiqc": {
-                    "git_sha": "e745e167c1020928ef20ea1397b6b4d230681b4d"
+                    "git_sha": "e745e167c1020928ef20ea1397b6b4d230681b4d",
+                    "branch": "master"
                 }
             }
         }

nf_core/utils.py

@@ -1006,3 +1006,15 @@ def strip_ansi_codes(string, replace_with=""):
     From Stack Overflow: https://stackoverflow.com/a/14693789/713980
     """
     return ANSI_ESCAPE_RE.sub(replace_with, string)
+
+
+def is_relative_to(path1, path2):
+    """
+    Checks if a path is relative to another.
+
+    Should mimic Path.is_relative_to which not available in Python < 3.9
+
+    path1 (Path | str): The path that could be a subpath
+    path2 (Path | str): The path the could be the superpath
+    """
+    return str(path1).startswith(str(path2) + os.sep)

tests/modules/install.py

@@ -2,7 +2,15 @@
 
 import pytest
 
-from ..utils import with_temporary_folder
+from nf_core.modules.install import ModuleInstall
+from nf_core.modules.modules_json import ModulesJson
+
+from ..utils import (
+    GITLAB_BRANCH_TEST_BRANCH,
+    GITLAB_REPO,
+    GITLAB_URL,
+    with_temporary_folder,
+)
 
 
 def test_modules_install_nopipeline(self):
@@ -43,9 +51,19 @@ def test_modules_install_from_gitlab(self):
     assert self.mods_install_gitlab.install("fastqc") is True
 
 
-# TODO Remove comments once external repository to have same structure as nf-core/modules
-# def test_modules_install_trimgalore_alternative_source(self):
-#     """Test installing a module from a different source repository - TrimGalore!"""
-#     assert self.mods_install_alt.install("trimgalore") is not False
-#     module_path = os.path.join(self.mods_install.dir, "modules", "ewels", "nf-core-modules", "trimgalore")
-#     assert os.path.exists(module_path)
+def test_modules_install_different_branch_fail(self):
+    """Test installing a module from a different branch"""
+    install_obj = ModuleInstall(self.pipeline_dir, remote_url=GITLAB_URL, branch=GITLAB_BRANCH_TEST_BRANCH)
+    # The FastQC module does not exists in the branch-test branch
+    assert install_obj.install("fastqc") is False
+
+
+def test_modules_install_different_branch_succeed(self):
+    """Test installing a module from a different branch"""
+    install_obj = ModuleInstall(self.pipeline_dir, remote_url=GITLAB_URL, branch=GITLAB_BRANCH_TEST_BRANCH)
+    # The fastp module does exists in the branch-test branch
+    assert install_obj.install("fastp") is True
+
+    # Verify that the branch entry was added correctly
+    modules_json = ModulesJson(self.pipeline_dir)
+    assert modules_json.get_module_branch("fastp", GITLAB_REPO) == GITLAB_BRANCH_TEST_BRANCH

tests/modules/modules_json.py

@@ -5,6 +5,7 @@
 from nf_core.modules.modules_json import ModulesJson
 from nf_core.modules.modules_repo import (
     NF_CORE_MODULES_BASE_PATH,
+    NF_CORE_MODULES_DEFAULT_BRANCH,
     NF_CORE_MODULES_NAME,
     NF_CORE_MODULES_REMOTE,
     ModulesRepo,
@@ -34,6 +35,7 @@ def test_mod_json_update(self):
     assert "MODULE_NAME" in mod_json["repos"][NF_CORE_MODULES_NAME]["modules"]
     assert "git_sha" in mod_json["repos"][NF_CORE_MODULES_NAME]["modules"]["MODULE_NAME"]
     assert "GIT_SHA" == mod_json["repos"][NF_CORE_MODULES_NAME]["modules"]["MODULE_NAME"]["git_sha"]
+    assert NF_CORE_MODULES_DEFAULT_BRANCH == mod_json["repos"][NF_CORE_MODULES_NAME]["modules"]["MODULE_NAME"]["branch"]
 
 
 def test_mod_json_create(self):
@@ -57,6 +59,7 @@ def test_mod_json_create(self):
     for mod in mods:
         assert mod in mod_json["repos"][NF_CORE_MODULES_NAME]["modules"]
         assert "git_sha" in mod_json["repos"][NF_CORE_MODULES_NAME]["modules"][mod]
+        assert "branch" in mod_json["repos"][NF_CORE_MODULES_NAME]["modules"][mod]
 
 
 def test_mod_json_up_to_date(self):