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

Fix linting multiple remotes #2071

Merged
merged 15 commits into from
Dec 2, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
4 changes: 2 additions & 2 deletions nf_core/components/info.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,13 +103,13 @@ def init_mod_name(self, component):
components = self.get_components_clone_modules()
else:
components = self.modules_json.get_all_components(self.component_type).get(
self.modules_repo.remote_url
self.modules_repo.remote_url, {}
)
components = [
component if directory == self.modules_repo.repo_path else f"{directory}/{component}"
for directory, component in components
]
if components is None:
if not components:
raise UserWarning(
f"No {self.component_type[:-1]} installed from '{self.modules_repo.remote_url}'"
)
Expand Down
6 changes: 2 additions & 4 deletions nf_core/components/remove.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,10 +161,8 @@ def remove(self, component, removed_by=None, removed_components=None, force=Fals
else:
log.info(f"Removed files for '{component}'.")
else:
installed_by = modules_json.modules_json["repos"][self.modules_repo.remote_url][self.component_type][
repo_path
][component]["installed_by"]
if installed_by == self.component_type:
installed_by = modules_json.get_installed_by_entries(self.component_type, component)
if installed_by == [self.component_type]:
log.error(
f"Did not remove '{component}', because it was also manually installed. Only updated 'installed_by' entry in modules.json."
)
Expand Down
10 changes: 4 additions & 6 deletions nf_core/components/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ def update(self, component=None, silent=False, updated=None, check_diff_exist=Tr
else:
updated.append(component)
recursive_update = True
modules_to_update, subworkflows_to_update = self.get_components_to_update(component, modules_repo)
modules_to_update, subworkflows_to_update = self.get_components_to_update(component)
if not silent and len(modules_to_update + subworkflows_to_update) > 0:
log.warning(
f"All modules and subworkflows linked to the updated {self.component_type[:-1]} will be added to the same diff file.\n"
Expand Down Expand Up @@ -287,7 +287,7 @@ def update(self, component=None, silent=False, updated=None, check_diff_exist=Tr
self.modules_json.update(self.component_type, modules_repo, component, version, installed_by=None)
updated.append(component)
recursive_update = True
modules_to_update, subworkflows_to_update = self.get_components_to_update(component, modules_repo)
modules_to_update, subworkflows_to_update = self.get_components_to_update(component)
if not silent and not self.update_all and len(modules_to_update + subworkflows_to_update) > 0:
log.warning(
f"All modules and subworkflows linked to the updated {self.component_type[:-1]} will be {'asked for update' if self.show_diff else 'automatically updated'}.\n"
Expand Down Expand Up @@ -831,7 +831,7 @@ def try_apply_patch(

return True

def get_components_to_update(self, component, modules_repo):
def get_components_to_update(self, component):
"""
Get all modules and subworkflows linked to the updated component.

Expand All @@ -841,9 +841,7 @@ def get_components_to_update(self, component, modules_repo):
mods_json = self.modules_json.get_modules_json()
modules_to_update = []
subworkflows_to_update = []
installed_by = mods_json["repos"][modules_repo.remote_url][self.component_type][modules_repo.repo_path][
component
]["installed_by"]
installed_by = self.modules_json.get_installed_by_entries(self.component_type, component)

if self.component_type == "modules":
# All subworkflow names in the installed_by section of a module are subworkflows using this module
Expand Down
34 changes: 20 additions & 14 deletions nf_core/modules/lint/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,23 +89,29 @@ def __init__(
if self.repo_type == "pipeline":
modules_json = ModulesJson(self.dir)
modules_json.check_up_to_date()
all_pipeline_modules = modules_json.get_all_components(self.component_type)
if all_pipeline_modules is not None and self.modules_repo.remote_url in all_pipeline_modules:
module_dir = Path(self.dir, "modules", self.modules_repo.repo_path)
self.all_remote_modules = [
NFCoreModule(m[1], self.modules_repo.remote_url, module_dir / m[1], self.repo_type, Path(self.dir))
for m in all_pipeline_modules[self.modules_repo.remote_url]
] # m = (module_dir, module_name)
if not self.all_remote_modules:
raise LookupError(f"No modules from {self.modules_repo.remote_url} installed in pipeline.")
local_module_dir = Path(self.dir, "modules", "local")
self.all_remote_modules = []
for repo_url, components in modules_json.get_all_components(self.component_type).items():
for org, comp in components:
self.all_remote_modules.append(
NFCoreModule(
comp,
repo_url,
Path(self.dir, self.component_type, org, comp),
self.repo_type,
Path(self.dir),
)
)
if not self.all_remote_modules:
raise LookupError(f"No modules from {self.modules_repo.remote_url} installed in pipeline.")
local_module_dir = Path(self.dir, "modules", "local")
self.all_local_modules = []
if local_module_dir.exists():
self.all_local_modules = [
NFCoreModule(m, None, local_module_dir / m, self.repo_type, Path(self.dir), nf_core_module=False)
NFCoreModule(
m, None, Path(local_module_dir, m), self.repo_type, Path(self.dir), remote_module=False
)
for m in self.get_local_components()
]

else:
raise LookupError(f"No modules from {self.modules_repo.remote_url} installed in pipeline.")
else:
module_dir = Path(self.dir, self.default_modules_path)
self.all_remote_modules = [
Expand Down
9 changes: 6 additions & 3 deletions nf_core/modules/lint/module_changes.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import tempfile
from pathlib import Path

import nf_core.modules.modules_repo
from nf_core.modules.modules_differ import ModulesDiffer


Expand Down Expand Up @@ -39,10 +40,12 @@ def module_changes(module_lint_object, module):
return
else:
tempdir = module.module_dir
module.branch = module_lint_object.modules_json.get_component_branch(
"modules", module.module_name, module.repo_url, module.org
)
modules_repo = nf_core.modules.modules_repo.ModulesRepo(remote_url=module.repo_url, branch=module.branch)

for f, same in module_lint_object.modules_repo.module_files_identical(
module.module_name, tempdir, module.git_sha
).items():
for f, same in modules_repo.module_files_identical(module.module_name, tempdir, module.git_sha).items():
if same:
module.passed.append(
(
Expand Down
11 changes: 6 additions & 5 deletions nf_core/modules/lint/module_version.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,8 @@ def module_version(module_lint_object, module):
"""

modules_json_path = Path(module_lint_object.dir, "modules.json")

# Verify that a git_sha exists in the `modules.json` file for this module
version = module_lint_object.modules_json.get_module_version(
module.module_name, module_lint_object.modules_repo.remote_url, module_lint_object.modules_repo.repo_path
)
version = module_lint_object.modules_json.get_module_version(module.module_name, module.repo_url, module.org)
if version is None:
module.failed.append(("git_sha", "No git_sha entry in `modules.json`", modules_json_path))
return
Expand All @@ -36,7 +33,11 @@ def module_version(module_lint_object, module):

# Check whether a new version is available
try:
modules_repo = nf_core.modules.modules_repo.ModulesRepo()
module.branch = module_lint_object.modules_json.get_component_branch(
"modules", module.module_name, module.repo_url, module.org
)
modules_repo = nf_core.modules.modules_repo.ModulesRepo(remote_url=module.repo_url, branch=module.branch)

module_git_log = modules_repo.get_component_git_log(module.module_name, "modules")
if version == next(module_git_log)["git_sha"]:
module.passed.append(("module_version", "Module is the latest version", module.module_dir))
Expand Down
28 changes: 25 additions & 3 deletions nf_core/modules/modules_json.py
Original file line number Diff line number Diff line change
Expand Up @@ -917,8 +917,6 @@ def get_all_components(self, component_type):
if component_type in repo_entry:
for dir, components in repo_entry[component_type].items():
self.pipeline_components[repo] = [(dir, m) for m in components]
if self.pipeline_components == {}:
self.pipeline_components = None

return self.pipeline_components

Expand Down Expand Up @@ -961,14 +959,38 @@ def get_dependent_components(

return dependent_components

def get_installed_by_entries(self, component_type, name):
"""
Retrieves all entries of installed_by for a given component

Args:
component_type (str): Type of component [modules, subworkflows]
name (str): Name of the component to find dependencies for

Returns:
(list): The list of installed_by entries

"""
if self.modules_json is None:
self.load()
installed_by_entries = {}
for repo_url, repo_entry in self.modules_json.get("repos", {}).items():
if component_type in repo_entry:
for install_dir, components in repo_entry[component_type].items():
if name in components:
installed_by_entries = components[name]["installed_by"]
break

return installed_by_entries

def get_component_branch(self, component_type, component, repo_url, install_dir):
"""
Gets the branch from which the module/subworkflow was installed

Returns:
(str): The branch name
Raises:
LookupError: If their is no branch entry in the `modules.json`
LookupError: If there is no branch entry in the `modules.json`
"""
if self.modules_json is None:
self.load()
Expand Down
2 changes: 1 addition & 1 deletion nf_core/modules/modules_repo.py
Original file line number Diff line number Diff line change
Expand Up @@ -376,8 +376,8 @@ def module_files_identical(self, module_name, base_path, commit):
else:
self.checkout(commit)
module_files = ["main.nf", "meta.yml"]
module_dir = self.get_component_dir(module_name, "modules")
files_identical = {file: True for file in module_files}
module_dir = self.get_component_dir(module_name, "modules")
for file in module_files:
try:
files_identical[file] = filecmp.cmp(os.path.join(module_dir, file), os.path.join(base_path, file))
Expand Down
8 changes: 4 additions & 4 deletions nf_core/modules/nfcore_module.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ class NFCoreModule:
Includes functionality for linting
"""

def __init__(self, module_name, repo_url, module_dir, repo_type, base_dir, nf_core_module=True):
def __init__(self, module_name, repo_url, module_dir, repo_type, base_dir, remote_module=True):
"""
Initialize the object

Expand All @@ -20,7 +20,7 @@ def __init__(self, module_name, repo_url, module_dir, repo_type, base_dir, nf_co
whether the directory is a pipeline or clone
of nf-core/modules.
base_dir (Path): The absolute path to the pipeline base dir
nf_core_module (bool): Whether the module is to be treated as a
remote_module (bool): Whether the module is to be treated as a
nf-core or local module
"""
self.module_name = module_name
Expand All @@ -36,14 +36,14 @@ def __init__(self, module_name, repo_url, module_dir, repo_type, base_dir, nf_co
self.has_meta = False
self.git_sha = None
self.is_patched = False
self.is_patched = None

if nf_core_module:
if remote_module:
# Initialize the important files
self.main_nf = self.module_dir / "main.nf"
self.meta_yml = self.module_dir / "meta.yml"

repo_dir = self.module_dir.parts[: self.module_dir.parts.index(self.module_name.split("/")[0])][-1]
self.org = repo_dir
self.test_dir = Path(self.base_dir, "tests", "modules", repo_dir, self.module_name)
self.test_yml = self.test_dir / "test.yml"
self.test_main_nf = self.test_dir / "main.nf"
Expand Down
23 changes: 21 additions & 2 deletions tests/modules/lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ def test_modules_lint_new_modules(self):

def test_modules_lint_no_gitlab(self):
"""Test linting a pipeline with no modules installed"""
self.mods_remove.remove("fastqc", force=True)
self.mods_remove.remove("multiqc", force=True)
self.mods_remove.remove("custom/dumpsoftwareversions", force=True)
with pytest.raises(LookupError):
nf_core.modules.ModuleLint(dir=self.pipeline_dir, remote_url=GITLAB_URL)

Expand All @@ -68,6 +71,17 @@ def test_modules_lint_gitlab_modules(self):
assert len(module_lint.warned) >= 0


def test_modules_lint_multiple_remotes(self):
"""Lint modules from a different remote"""
self.mods_install.install("fastqc")
self.mods_install_gitlab.install("multiqc")
module_lint = nf_core.modules.ModuleLint(dir=self.pipeline_dir, remote_url=GITLAB_URL)
module_lint.lint(print_results=False, all_modules=True)
assert len(module_lint.failed) == 0
assert len(module_lint.passed) > 0
assert len(module_lint.warned) >= 0


def test_modules_lint_patched_modules(self):
"""
Test creating a patch file and applying it to a new version of the the files
Expand All @@ -81,8 +95,13 @@ def test_modules_lint_patched_modules(self):
# change temporarily working directory to the pipeline directory
# to avoid error from try_apply_patch() during linting
with set_wd(self.pipeline_dir):
module_lint = nf_core.modules.ModuleLint(dir=self.pipeline_dir, remote_url=GITLAB_URL, branch=PATCH_BRANCH)
module_lint.lint(print_results=False, all_modules=True)
module_lint = nf_core.modules.ModuleLint(
dir=self.pipeline_dir, remote_url=GITLAB_URL, branch=PATCH_BRANCH, hide_progress=True
)
module_lint.lint(
print_results=False,
all_modules=True,
)

assert len(module_lint.failed) == 0
assert len(module_lint.passed) > 0
Expand Down
1 change: 1 addition & 0 deletions tests/test_modules.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ def test_modulesrepo_class(self):
from .modules.lint import (
test_modules_lint_empty,
test_modules_lint_gitlab_modules,
test_modules_lint_multiple_remotes,
test_modules_lint_new_modules,
test_modules_lint_no_gitlab,
test_modules_lint_patched_modules,
Expand Down