Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 17 commits into from
Aug 5, 2022
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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]

Expand Down
2 changes: 1 addition & 1 deletion nf_core/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)


Expand Down
7 changes: 5 additions & 2 deletions nf_core/lint/modules_json.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 3 additions & 1 deletion nf_core/modules/list.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
563 changes: 323 additions & 240 deletions nf_core/modules/modules_json.py

Large diffs are not rendered by default.

27 changes: 27 additions & 0 deletions nf_core/modules/modules_repo.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import logging
import os
import shutil
from pathlib import Path

import git
import rich.progress
Expand All @@ -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):
Expand Down Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions nf_core/modules/patch.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,12 @@ 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.branch = module_branch
self.modules_repo.setup_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)
Expand Down
93 changes: 71 additions & 22 deletions nf_core/modules/update.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import enum
import json
import logging
import os
import shutil
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -316,12 +317,26 @@ 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 which was installed from the '{current_branch}'"
ErikDanielsson marked this conversation as resolved.
Show resolved Hide resolved
)
switch = questionary.confirm(f"Do you want to update using the '{current_branch}' instead?").unsafe_ask()
if switch:
# Change the branch
self.modules_repo.branch = current_branch
self.modules_repo.setup_branch()
ErikDanielsson marked this conversation as resolved.
Show resolved Hide resolved

# 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.
Expand All @@ -330,6 +345,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 = []
Expand All @@ -339,18 +360,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:
Expand All @@ -361,7 +388,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:
Expand Down Expand Up @@ -390,32 +420,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):
ErikDanielsson marked this conversation as resolved.
Show resolved Hide resolved
log.warning(
f"Git sha '{sha}' does not exists on the '{branch}' of '{repo.fullname}'. Skipping module '{mod}'"
ErikDanielsson marked this conversation as resolved.
Show resolved Hide resolved
)
modules_info.pop(i)
else:
i += 1

# Add patch filenames to the modules that have them
modules_info = [
Expand All @@ -427,7 +476,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.
"""
Expand Down
9 changes: 6 additions & 3 deletions nf_core/pipeline-template/modules.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
}
}
Expand Down
12 changes: 12 additions & 0 deletions nf_core/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
32 changes: 25 additions & 7 deletions tests/modules/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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
3 changes: 3 additions & 0 deletions tests/modules/modules_json.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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):
Expand All @@ -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):
Expand Down
Loading