Merge pull request #2071 from mashehu/fix-linting-multiple-remotes

Fix linting multiple remotes

nf_core/components/info.py

@@ -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}'"
                         )

nf_core/components/remove.py

@@ -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."
                 )

nf_core/components/update.py

@@ -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"
@@ -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"
@@ -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.
 
@@ -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

nf_core/modules/lint/__init__.py

@@ -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 = [

nf_core/modules/lint/module_changes.py

@@ -5,6 +5,7 @@
 import tempfile
 from pathlib import Path
 
+import nf_core.modules.modules_repo
 from nf_core.modules.modules_differ import ModulesDiffer
 
 
@@ -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(
                 (

nf_core/modules/lint/module_version.py

@@ -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
@@ -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))

nf_core/modules/modules_json.py

@@ -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
 
@@ -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()

nf_core/modules/modules_repo.py

@@ -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))

nf_core/modules/nfcore_module.py

@@ -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
 
@@ -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
@@ -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"

tests/modules/lint.py

@@ -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)
 
@@ -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
@@ -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

tests/test_modules.py

@@ -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,