Skip to content

Commit

Permalink
Merge pull request #2228 from anoronh4/feature/warn_multiple_remotes
Browse files Browse the repository at this point in the history
Log error when multiple remotes have the same org name
  • Loading branch information
anoronh4 authored Apr 10, 2023
2 parents f1fc15b + 3f44849 commit 6ff6dee
Show file tree
Hide file tree
Showing 13 changed files with 137 additions and 18 deletions.
16 changes: 13 additions & 3 deletions .github/workflows/create-lint-wf.yml
Original file line number Diff line number Diff line change
Expand Up @@ -107,15 +107,25 @@ jobs:
- name: nf-core modules install
run: nf-core --log-file log.txt modules install fastqc --dir nf-core-testpipeline/ --force

- name: nf-core modules install gitlab
run: nf-core --log-file log.txt modules --git-remote https://gitlab.com/nf-core/modules-test.git --branch main install fastqc --force --dir nf-core-testpipeline/

- name: nf-core modules list local
run: nf-core --log-file log.txt modules list local --dir nf-core-testpipeline/

- name: nf-core modules list remote
run: nf-core --log-file log.txt modules list remote

- name: nf-core modules remove
run: |
find nf-core-testpipeline/workflows/ -type f -exec sed -i '/^include /d' {} \;
nf-core --log-file log.txt modules remove fastqc --dir nf-core-testpipeline/
nf-core --log-file log.txt modules remove multiqc --dir nf-core-testpipeline/
nf-core --log-file log.txt modules remove custom/dumpsoftwareversions --dir nf-core-testpipeline/
- name: nf-core modules install gitlab
run: nf-core --log-file log.txt modules --git-remote https://gitlab.com/nf-core/modules-test.git --branch main install fastqc --force --dir nf-core-testpipeline/

- name: nf-core modules list local gitlab
run: nf-core --log-file log.txt modules list local --dir nf-core-testpipeline/

- name: nf-core modules list remote gitlab
run: nf-core --log-file log.txt modules --git-remote https://gitlab.com/nf-core/modules-test.git list remote

Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
- Add the Nextflow version to Gitpod container matching the minimal Nextflow version for nf-core (according to `nextflow.config`) ([#2196](https://github.com/nf-core/tools/pull/2196))
- Use `nfcore/gitpod:dev` container in the dev branch ([#2196](https://github.com/nf-core/tools/pull/2196))
- Replace requests_mock with responses in test mocks ([#2165](https://github.com/nf-core/tools/pull/2165)).
- Add warning when installing a module from an `org_path` that exists in multiple remotes in `modules.json` ([#2228](https://github.com/nf-core/tools/pull/2228)).

## [v2.7.2 - Mercury Eagle Patch](https://github.com/nf-core/tools/releases/tag/2.7.2) - [2022-12-19]

Expand Down
23 changes: 23 additions & 0 deletions nf_core/components/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,12 @@ def install(self, component, silent=False):
if not silent:
modules_json.check_up_to_date()

# Verify that the remote repo's org_path does not match the org_path of any alternate repo among the installed modules
if self.check_alternate_remotes(modules_json):
err_msg = f"You are trying to install {self.component_type} from different repositories with the same organization name '{self.modules_repo.repo_path}' (set in the `.nf-core.yml` file in the `org_path` field).\nThis is not supported, and will likely cause problems. org_path should be set to the github account/organization name."
log.error(err_msg)
return False

# Verify SHA
if not self.modules_repo.verify_sha(self.prompt, self.sha):
return False
Expand Down Expand Up @@ -264,3 +270,20 @@ def clean_modules_json(self, component, modules_repo, modules_json):
self.component_type, component, repo_to_remove, modules_repo.repo_path
)
return component_values["installed_by"]

def check_alternate_remotes(self, modules_json):
"""
Check whether there are previously installed components with the same org_path but different remote urls
Log error if multiple remotes exist.
Return:
True: if problematic components are found
False: if problematic components are not found
"""
modules_json.load()
for repo_url, repo_content in modules_json.modules_json.get("repos", dict()).items():
for component_type in repo_content:
for dir in repo_content.get(component_type, dict()).keys():
if dir == self.modules_repo.repo_path and repo_url != self.modules_repo.remote_url:
return True
return False
10 changes: 10 additions & 0 deletions tests/modules/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
GITLAB_BRANCH_TEST_BRANCH,
GITLAB_REPO,
GITLAB_URL,
remove_template_modules,
with_temporary_folder,
)

Expand Down Expand Up @@ -49,6 +50,7 @@ def test_modules_install_trimgalore_twice(self):

def test_modules_install_from_gitlab(self):
"""Test installing a module from GitLab"""
remove_template_modules(self)
assert self.mods_install_gitlab.install("fastqc") is True


Expand All @@ -61,6 +63,7 @@ def test_modules_install_different_branch_fail(self):

def test_modules_install_different_branch_succeed(self):
"""Test installing a module from a different branch"""
remove_template_modules(self)
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
Expand All @@ -83,3 +86,10 @@ def test_modules_install_tracking(self):
assert mod_json["repos"]["https://github.com/nf-core/modules.git"]["modules"]["nf-core"]["trimgalore"][
"installed_by"
] == ["modules"]


def test_modules_install_alternate_remote(self):
"""Test installing a module from a different remote with the same organization path"""
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 False
6 changes: 4 additions & 2 deletions tests/modules/lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import nf_core.modules

from ..utils import GITLAB_URL, set_wd
from ..utils import GITLAB_URL, remove_template_modules, set_wd
from .patch import BISMARK_ALIGN, CORRECT_SHA, PATCH_BRANCH, REPO_NAME, modify_main_nf


Expand Down Expand Up @@ -62,6 +62,7 @@ def test_modules_lint_no_gitlab(self):

def test_modules_lint_gitlab_modules(self):
"""Lint modules from a different remote"""
remove_template_modules(self)
self.mods_install_gitlab.install("fastqc")
self.mods_install_gitlab.install("multiqc")
module_lint = nf_core.modules.ModuleLint(dir=self.pipeline_dir, remote_url=GITLAB_URL)
Expand All @@ -73,7 +74,7 @@ def test_modules_lint_gitlab_modules(self):

def test_modules_lint_multiple_remotes(self):
"""Lint modules from a different remote"""
self.mods_install.install("fastqc")
remove_template_modules(self)
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)
Expand All @@ -86,6 +87,7 @@ def test_modules_lint_patched_modules(self):
"""
Test creating a patch file and applying it to a new version of the the files
"""
remove_template_modules(self)
setup_patch(self.pipeline_dir, True)

# Create a patch file
Expand Down
43 changes: 35 additions & 8 deletions tests/modules/patch.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import nf_core.components.components_command
import nf_core.modules

from ..utils import GITLAB_URL
from ..utils import GITLAB_URL, remove_template_modules

"""
Test the 'nf-core modules patch' command
Expand All @@ -26,7 +26,7 @@
REPO_URL = "https://gitlab.com/nf-core/modules-test.git"


def setup_patch(pipeline_dir, modify_module):
def setup_patch(pipeline_dir, modify_module, pipeline_name):
install_obj = nf_core.modules.ModuleInstall(
pipeline_dir, prompt=False, force=False, remote_url=GITLAB_URL, branch=PATCH_BRANCH, sha=ORG_SHA
)
Expand All @@ -40,6 +40,15 @@ def setup_patch(pipeline_dir, modify_module):
modify_main_nf(module_path / "main.nf")


def modify_workflow_nf(path):
with open(path, "r") as fh:
lines = fh.readlines()
with open(path, "w") as fh:
for line in lines:
if not line.startswith("include {"):
fh.write(line)


def modify_main_nf(path):
"""Modify a file to test patch creation"""
with open(path, "r") as fh:
Expand All @@ -60,7 +69,10 @@ def modify_main_nf(path):

def test_create_patch_no_change(self):
"""Test creating a patch when there is no change to the module"""
setup_patch(self.pipeline_dir, False)
# Remove modules that may cause org_path conflict
remove_template_modules(self)

setup_patch(self.pipeline_dir, False, self.pipeline_name)

# Try creating a patch file
patch_obj = nf_core.modules.ModulePatch(self.pipeline_dir, GITLAB_URL, PATCH_BRANCH)
Expand All @@ -79,7 +91,10 @@ def test_create_patch_no_change(self):

def test_create_patch_change(self):
"""Test creating a patch when there is a change to the module"""
setup_patch(self.pipeline_dir, True)
# Remove modules that may cause org_path conflict
remove_template_modules(self)

setup_patch(self.pipeline_dir, True, self.pipeline_name)

# Try creating a patch file
patch_obj = nf_core.modules.ModulePatch(self.pipeline_dir, GITLAB_URL, PATCH_BRANCH)
Expand Down Expand Up @@ -112,7 +127,10 @@ def test_create_patch_try_apply_successful(self):
"""
Test creating a patch file and applying it to a new version of the the files
"""
setup_patch(self.pipeline_dir, True)
# Remove modules that may cause org_path conflict
remove_template_modules(self)

setup_patch(self.pipeline_dir, True, self.pipeline_name)
module_relpath = Path("modules", REPO_NAME, BISMARK_ALIGN)
module_path = Path(self.pipeline_dir, module_relpath)

Expand Down Expand Up @@ -178,7 +196,10 @@ def test_create_patch_try_apply_failed(self):
"""
Test creating a patch file and applying it to a new version of the the files
"""
setup_patch(self.pipeline_dir, True)
# Remove modules that may cause org_path conflict
remove_template_modules(self)

setup_patch(self.pipeline_dir, True, self.pipeline_name)
module_relpath = Path("modules", REPO_NAME, BISMARK_ALIGN)
module_path = Path(self.pipeline_dir, module_relpath)

Expand Down Expand Up @@ -216,7 +237,10 @@ def test_create_patch_update_success(self):
Should have the same effect as 'test_create_patch_try_apply_successful'
but uses higher level api
"""
setup_patch(self.pipeline_dir, True)
# Remove modules that may cause org_path conflict
remove_template_modules(self)

setup_patch(self.pipeline_dir, True, self.pipeline_name)
module_path = Path(self.pipeline_dir, "modules", REPO_NAME, BISMARK_ALIGN)

# Try creating a patch file
Expand Down Expand Up @@ -277,7 +301,10 @@ def test_create_patch_update_fail(self):
"""
Test creating a patch file and updating a module when there is a diff conflict
"""
setup_patch(self.pipeline_dir, True)
# Remove modules that may cause org_path conflict
remove_template_modules(self)

setup_patch(self.pipeline_dir, True, self.pipeline_name)
module_path = Path(self.pipeline_dir, "modules", REPO_NAME, BISMARK_ALIGN)

# Try creating a patch file
Expand Down
5 changes: 5 additions & 0 deletions tests/modules/remove.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import os

from ..utils import remove_template_modules


def test_modules_remove_trimgalore(self):
"""Test removing TrimGalore! module after installing it"""
Expand All @@ -16,6 +18,9 @@ def test_modules_remove_trimgalore_uninstalled(self):

def test_modules_remove_multiqc_from_gitlab(self):
"""Test removing multiqc module after installing it from an alternative source"""
# Remove modules that may cause org_path conflict
remove_template_modules(self)

self.mods_install_gitlab.install("multiqc")
module_path = os.path.join(self.mods_install_gitlab.dir, "modules", "nf-core", "multiqc")
assert self.mods_remove_gitlab.remove("multiqc", force=True)
Expand Down
10 changes: 10 additions & 0 deletions tests/modules/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
GITLAB_URL,
OLD_TRIMGALORE_BRANCH,
OLD_TRIMGALORE_SHA,
remove_template_modules,
)


Expand All @@ -44,6 +45,7 @@ def test_install_and_update(self):

def test_install_at_hash_and_update(self):
"""Installs an old version of a module in the pipeline and updates it"""
remove_template_modules(self)
assert self.mods_install_old.install("trimgalore")
update_obj = ModuleUpdate(
self.pipeline_dir, show_diff=False, update_deps=True, remote_url=GITLAB_URL, branch=OLD_TRIMGALORE_BRANCH
Expand All @@ -69,6 +71,7 @@ def test_install_at_hash_and_update(self):

def test_install_at_hash_and_update_and_save_diff_to_file(self):
"""Installs an old version of a module in the pipeline and updates it"""
remove_template_modules(self)
self.mods_install_old.install("trimgalore")
patch_path = os.path.join(self.pipeline_dir, "trimgalore.patch")
update_obj = ModuleUpdate(
Expand Down Expand Up @@ -109,6 +112,7 @@ def test_update_all(self):

def test_update_with_config_fixed_version(self):
"""Try updating when there are entries in the .nf-core.yml"""
remove_template_modules(self)
# Install trimgalore at the latest version
assert self.mods_install_trimgalore.install("trimgalore")

Expand All @@ -134,6 +138,7 @@ def test_update_with_config_fixed_version(self):

def test_update_with_config_dont_update(self):
"""Try updating when module is to be ignored"""
remove_template_modules(self)
# Install an old version of trimgalore
self.mods_install_old.install("trimgalore")

Expand Down Expand Up @@ -164,6 +169,7 @@ def test_update_with_config_dont_update(self):

def test_update_with_config_fix_all(self):
"""Fix the version of all nf-core modules"""
remove_template_modules(self)
self.mods_install_trimgalore.install("trimgalore")

# Fix the version of all nf-core modules in the .nf-core.yml to an old version
Expand All @@ -187,6 +193,7 @@ def test_update_with_config_fix_all(self):

def test_update_with_config_no_updates(self):
"""Don't update any nf-core modules"""
remove_template_modules(self)
self.mods_install_old.install("trimgalore")
old_mod_json = ModulesJson(self.pipeline_dir).get_modules_json()

Expand Down Expand Up @@ -220,6 +227,7 @@ def test_update_with_config_no_updates(self):

def test_update_different_branch_single_module(self):
"""Try updating a module in a specific branch"""
remove_template_modules(self)
install_obj = ModuleInstall(
self.pipeline_dir,
prompt=False,
Expand All @@ -246,6 +254,7 @@ def test_update_different_branch_single_module(self):

def test_update_different_branch_mixed_modules_main(self):
"""Try updating all modules where MultiQC is installed from main branch"""
remove_template_modules(self)
# Install fastp
assert self.mods_install_gitlab_old.install("fastp")

Expand All @@ -272,6 +281,7 @@ def test_update_different_branch_mixed_modules_main(self):

def test_update_different_branch_mix_modules_branch_test(self):
"""Try updating all modules where MultiQC is installed from branch-test branch"""
remove_template_modules(self)
# Install multiqc from the branch-test branch
assert self.mods_install_gitlab_old.install(
"multiqc"
Expand Down
8 changes: 8 additions & 0 deletions tests/subworkflows/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
GITLAB_REPO,
GITLAB_SUBWORKFLOWS_BRANCH,
GITLAB_URL,
remove_template_modules,
with_temporary_folder,
)

Expand Down Expand Up @@ -62,6 +63,7 @@ def test_subworkflows_install_bam_sort_stats_samtools_twice(self):

def test_subworkflows_install_from_gitlab(self):
"""Test installing a subworkflow from GitLab"""
remove_template_modules(self)
assert self.subworkflow_install_gitlab.install("bam_stats_samtools") is True
# Verify that the branch entry was added correctly
modules_json = ModulesJson(self.pipeline_dir)
Expand Down Expand Up @@ -140,3 +142,9 @@ def test_subworkflows_install_tracking_added_super_subworkflow(self):
"installed_by"
]
) == sorted(["subworkflows", "bam_sort_stats_samtools"])


def test_subworkflows_install_alternate_remote(self):
"""Test installing a subworkflow from a different remote with the same organization path"""
self.subworkflow_install.install("bam_sort_stats_samtools")
assert self.subworkflow_install_gitlab.install("bam_stats_samtools") is False
3 changes: 2 additions & 1 deletion tests/subworkflows/list.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import nf_core.subworkflows

from ..utils import GITLAB_SUBWORKFLOWS_BRANCH, GITLAB_URL
from ..utils import GITLAB_SUBWORKFLOWS_BRANCH, GITLAB_URL, remove_template_modules


def test_subworkflows_list_remote(self):
Expand Down Expand Up @@ -40,6 +40,7 @@ def test_subworkflows_install_and_list_subworkflows(self):

def test_subworkflows_install_gitlab_and_list_subworkflows(self):
"""Test listing locally installed subworkflows"""
remove_template_modules(self)
self.subworkflow_install_gitlab.install("bam_sort_stats_samtools")
subworkflows_list = nf_core.subworkflows.SubworkflowList(self.pipeline_dir, remote=False)
listed_subworkflows = subworkflows_list.list_components()
Expand Down
Loading

0 comments on commit 6ff6dee

Please sign in to comment.