Merge pull request #1738 from ErikDanielsson/branch-modules-json

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

@@ -44,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.
@@ -141,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
@@ -315,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.
@@ -329,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 = []
@@ -338,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:
@@ -360,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:
@@ -389,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 = [
@@ -426,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):