From 9953d6dd7c5488a5a9b98e687b546ce1f41dcf6a Mon Sep 17 00:00:00 2001 From: bjlang <> Date: Wed, 18 Oct 2023 11:32:50 +0200 Subject: [PATCH 1/5] refactor code in preparation of a future 'subworkflows patch' command --- nf_core/components/patch.py | 221 ++++++++++++++++++++++++++++++++++++ nf_core/modules/patch.py | 209 +--------------------------------- 2 files changed, 225 insertions(+), 205 deletions(-) create mode 100644 nf_core/components/patch.py diff --git a/nf_core/components/patch.py b/nf_core/components/patch.py new file mode 100644 index 0000000000..aaa5c3dd10 --- /dev/null +++ b/nf_core/components/patch.py @@ -0,0 +1,221 @@ +import logging +import os +import shutil +import tempfile +from pathlib import Path + +import questionary + +import nf_core.utils +from nf_core.components.components_command import ComponentCommand + +from nf_core.modules.modules_differ import ModulesDiffer +from nf_core.modules.modules_json import ModulesJson + +log = logging.getLogger(__name__) + + +class ComponentPatch(ComponentCommand): + def __init__(self, pipeline_dir, component_type, remote_url=None, branch=None, no_pull=False, installed_by=False): + super().__init__(component_type, pipeline_dir, remote_url, branch, no_pull) + + self.modules_json = ModulesJson(dir) + + def _parameter_checks(self, component): + """Checks the compatibility of the supplied parameters. + + Raises: + UserWarning: if any checks fail. + """ + if not self.has_valid_directory(): + raise UserWarning("The command was not run in a valid pipeline directory.") + + components = self.modules_json.get_all_components(self.component_type).get(self.modules_repo.remote_url) + component_names = [component for _, component in components] + + if component is not None and component not in component_names: + component_dir = [dir for dir, m in components if m == component][0] + raise UserWarning(f"{self.component_type[:-1].title()} '{Path(self.component_type, component_dir, module)}' does not exist in the pipeline") + + def patch(self, component=None): + # Check modules directory structure + self.check_modules_structure() + + # Verify that 'modules.json' is consistent with the installed modules + self.modules_json.check_up_to_date() + self._parameter_checks(component) + components = self.modules_json.get_all_components(self.component_type).get(self.modules_repo.remote_url) + + if component is None: + choices = [ + component if directory == self.modules_repo.repo_path else f"{directory}/{component}" + for directory, component in components + ] + component = questionary.autocomplete( + f"{self.component_type[:-1].title()} name:", + choices=sorted(choices), + style=nf_core.utils.nfcore_question_style, + ).unsafe_ask() + component_dir = [dir for dir, m in components if m == component][0] + component_fullname = str(Path(self.component_type, self.modules_repo.repo_path, component)) + + # Verify that the component has an entry in the modules.json file + if not self.modules_json.module_present(component, self.modules_repo.remote_url, component_dir): + raise UserWarning( + f"The '{component_fullname}' {self.component_type[:-1]} does not have an entry in the 'modules.json' file. Cannot compute patch" + ) + + component_version = self.modules_json.get_component_version( + self.component_type, component, self.modules_repo.remote_url, self.modules_repo.repo_path + ) + if component_version is None: + raise UserWarning( + f"The '{component_fullname}' {self.component_type[:-1]} does not have a valid version in the 'modules.json' file. Cannot compute patch" + ) + # Get the component branch and reset it in the ModulesRepo object + component_branch = self.modules_json.get_component_branch( + self.component_type, component, self.modules_repo.remote_url, component_dir + ) + if component_branch != self.modules_repo.branch: + self.modules_repo.setup_branch(component_branch) + + # Set the diff filename based on the module name + patch_filename = f"{component.replace('/', '-')}.diff" + component_relpath = Path("modules", component_dir, component) + patch_relpath = Path(component_relpath, patch_filename) + component_current_dir = Path(self.dir, component_relpath) + patch_path = Path(self.dir, patch_relpath) + + if patch_path.exists(): + remove = questionary.confirm( + f"Patch exists for {self.component_type[:-1]} '{component_fullname}'. Do you want to regenerate it?", + style=nf_core.utils.nfcore_question_style, + ).unsafe_ask() + if remove: + os.remove(patch_path) + else: + return + + # Create a temporary directory for storing the unchanged version of the module + install_dir = tempfile.mkdtemp() + component_install_dir = Path(install_dir, component) + if not self.install_component_files(component, component_version, self.modules_repo, install_dir): + raise UserWarning( + f"Failed to install files of {self.component_type[:-1]} '{component}' from remote ({self.modules_repo.remote_url})." + ) + + # Write the patch to a temporary location (otherwise it is printed to the screen later) + patch_temp_path = tempfile.mktemp() + try: + ModulesDiffer.write_diff_file( + patch_temp_path, + component, + self.modules_repo.repo_path, + component_install_dir, + component_current_dir, + for_git=False, + dsp_from_dir=component_relpath, + dsp_to_dir=component_relpath, + ) + log.debug(f"Patch file wrote to a temporary directory {patch_temp_path}") + except UserWarning: + raise UserWarning(f"{self.component_type[:-1]} '{component_fullname}' is unchanged. No patch to compute") + + # Write changes to modules.json + self.modules_json.add_patch_entry(component, self.modules_repo.remote_url, component_dir, patch_relpath) + log.debug(f"Wrote patch path for {self.component_type[:-1]} {component} to modules.json") + + # Show the changes made to the module + ModulesDiffer.print_diff( + component, + self.modules_repo.repo_path, + component_install_dir, + component_current_dir, + dsp_from_dir=component_current_dir, + dsp_to_dir=component_current_dir, + ) + + # Finally move the created patch file to its final location + shutil.move(patch_temp_path, patch_path) + log.info(f"Patch file of '{component_fullname}' written to '{patch_path}'") + + def remove(self, component): + # Check modules directory structure + self.check_modules_structure() + + self.modules_json.check_up_to_date() + self._parameter_checks(component) + components = self.modules_json.get_all_components(self.component_type).get(self.modules_repo.remote_url) + + if component is None: + choices = [ + component if directory == self.modules_repo.repo_path else f"{directory}/{component}" + for directory, component in components + ] + component = questionary.autocomplete( + f"{self.component_type[:-1]} name:", + choices, + style=nf_core.utils.nfcore_question_style, + ).unsafe_ask() + component_dir = [dir for dir, m in components if m == component][0] + component_fullname = str(Path("modules", component_dir, component)) + + # Verify that the component has an entry in the modules.json file + if not self.modules_json.module_present(component, self.modules_repo.remote_url, component_dir): + raise UserWarning( + f"The '{component_fullname}' {self.component_type[:-1]} does not have an entry in the 'modules.json' file. Cannot compute patch" + ) + + component_version = self.modules_json.get_component_version( + self.component_type, component, self.modules_repo.remote_url, self.modules_repo.repo_path + ) + if component_version is None: + raise UserWarning( + f"The '{component_fullname}' {self.component_type[:-1]} 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 + component_branch = self.modules_json.get_component_branch( + self.component_type, component, self.modules_repo.remote_url, component_dir + ) + if component_branch != self.modules_repo.branch: + self.modules_repo.setup_branch(component_branch) + + # Set the diff filename based on the component name + patch_filename = f"{component.replace('/', '-')}.diff" + component_relpath = Path("modules", component_dir, component) + patch_relpath = Path(component_relpath, patch_filename) + patch_path = Path(self.dir, patch_relpath) + component_path = Path(self.dir, component_relpath) + + if patch_path.exists(): + remove = questionary.confirm( + f"Patch exists for {self.component_type[:-1]} '{component_fullname}'. Are you sure you want to remove?", + style=nf_core.utils.nfcore_question_style, + ).unsafe_ask() + if not remove: + return + + # Try to apply the patch in reverse and move resulting files to module dir + temp_component_dir = self.modules_json.try_apply_patch_reverse( + component, self.modules_repo.repo_path, patch_relpath, component_path + ) + try: + for file in Path(temp_component_dir).glob("*"): + file.rename(component_path.joinpath(file.name)) + os.rmdir(temp_component_dir) + except Exception as err: + raise UserWarning(f"There was a problem reverting the patched file: {err}") + + log.info(f"Patch for {component} reverted!") + # Remove patch file if we could revert the patch + patch_path.unlink() + # Write changes to module.json + self.modules_json.remove_patch_entry(component, self.modules_repo.remote_url, component_dir) + + if not all( + self.modules_repo.component_files_identical(component, component_path, component_version, "modules").values() + ): + log.error( + f"Module files do not appear to match the remote for the commit sha in the 'module.json': {component_version}\n" + f"Recommend reinstalling with 'nf-core modules install --force --sha {component_version} {module}' " + ) diff --git a/nf_core/modules/patch.py b/nf_core/modules/patch.py index 198bb70de5..b4e86f2d19 100644 --- a/nf_core/modules/patch.py +++ b/nf_core/modules/patch.py @@ -1,211 +1,10 @@ import logging -import os -import shutil -import tempfile -from pathlib import Path -import questionary - -import nf_core.utils -from nf_core.components.components_command import ComponentCommand - -from .modules_differ import ModulesDiffer -from .modules_json import ModulesJson +from nf_core.components.patch import ComponentPatch log = logging.getLogger(__name__) -class ModulePatch(ComponentCommand): - def __init__(self, dir, remote_url=None, branch=None, no_pull=False): - super().__init__("modules", dir, remote_url, branch, no_pull) - - self.modules_json = ModulesJson(dir) - - def param_check(self, module): - if not self.has_valid_directory(): - raise UserWarning() - - modules = self.modules_json.get_all_components(self.component_type)[self.modules_repo.remote_url] - module_names = [module for _, module in modules] - - if module is not None and module not in module_names: - module_dir = [dir for dir, m in modules if m == module][0] - raise UserWarning(f"Module '{Path('modules', module_dir, module)}' does not exist in the pipeline") - - def patch(self, module=None): - # Check modules directory structure - self.check_modules_structure() - - self.modules_json.check_up_to_date() - self.param_check(module) - modules = self.modules_json.get_all_components(self.component_type)[self.modules_repo.remote_url] - - if module is None: - choices = [ - module if directory == self.modules_repo.repo_path else f"{directory}/{module}" - for directory, module in modules - ] - module = questionary.autocomplete( - "Tool:", - choices, - style=nf_core.utils.nfcore_question_style, - ).unsafe_ask() - module_dir = [dir for dir, m in modules if m == module][0] - module_fullname = str(Path("modules", module_dir, module)) - - # Verify that the module has an entry in the modules.json file - if not self.modules_json.module_present(module, self.modules_repo.remote_url, module_dir): - raise UserWarning( - f"The '{module_fullname}' module does not have an entry in the 'modules.json' file. Cannot compute patch" - ) - - module_version = self.modules_json.get_module_version(module, self.modules_repo.remote_url, module_dir) - if module_version is 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_component_branch( - self.component_type, module, self.modules_repo.remote_url, module_dir - ) - 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", module_dir, module) - patch_relpath = Path(module_relpath, patch_filename) - module_current_dir = Path(self.dir, module_relpath) - patch_path = Path(self.dir, patch_relpath) - - if patch_path.exists(): - remove = questionary.confirm( - f"Patch exists for module '{module_fullname}'. Do you want to regenerate it?", - style=nf_core.utils.nfcore_question_style, - ).unsafe_ask() - if remove: - os.remove(patch_path) - else: - return - - # Create a temporary directory for storing the unchanged version of the module - install_dir = tempfile.mkdtemp() - module_install_dir = Path(install_dir, module) - if not self.install_component_files(module, module_version, self.modules_repo, install_dir): - raise UserWarning( - f"Failed to install files of module '{module}' from remote ({self.modules_repo.remote_url})." - ) - - # Write the patch to a temporary location (otherwise it is printed to the screen later) - patch_temp_path = tempfile.mktemp() - try: - ModulesDiffer.write_diff_file( - patch_temp_path, - module, - self.modules_repo.repo_path, - module_install_dir, - module_current_dir, - for_git=False, - dsp_from_dir=module_relpath, - dsp_to_dir=module_relpath, - ) - log.debug(f"Patch file wrote to a temporary directory {patch_temp_path}") - except UserWarning: - raise UserWarning(f"Module '{module_fullname}' is unchanged. No patch to compute") - - # Write changes to modules.json - self.modules_json.add_patch_entry(module, self.modules_repo.remote_url, module_dir, patch_relpath) - log.debug(f"Wrote patch path for module {module} to modules.json") - - # Show the changes made to the module - ModulesDiffer.print_diff( - module, - self.modules_repo.repo_path, - module_install_dir, - module_current_dir, - dsp_from_dir=module_current_dir, - dsp_to_dir=module_current_dir, - ) - - # Finally move the created patch file to its final location - shutil.move(patch_temp_path, patch_path) - log.info(f"Patch file of '{module_fullname}' written to '{patch_path}'") - - def remove(self, module): - # Check modules directory structure - self.check_modules_structure() - - self.modules_json.check_up_to_date() - self.param_check(module) - modules = self.modules_json.get_all_components(self.component_type)[self.modules_repo.remote_url] - - if module is None: - choices = [ - module if directory == self.modules_repo.repo_path else f"{directory}/{module}" - for directory, module in modules - ] - module = questionary.autocomplete( - "Tool:", - choices, - style=nf_core.utils.nfcore_question_style, - ).unsafe_ask() - module_dir = [dir for dir, m in modules if m == module][0] - module_fullname = str(Path("modules", module_dir, module)) - - # Verify that the module has an entry in the modules.json file - if not self.modules_json.module_present(module, self.modules_repo.remote_url, module_dir): - raise UserWarning( - f"The '{module_fullname}' module does not have an entry in the 'modules.json' file. Cannot compute patch" - ) - - module_version = self.modules_json.get_module_version(module, self.modules_repo.remote_url, module_dir) - if module_version is 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_component_branch( - self.component_type, module, self.modules_repo.remote_url, module_dir - ) - 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", module_dir, module) - patch_relpath = Path(module_relpath, patch_filename) - patch_path = Path(self.dir, patch_relpath) - module_path = Path(self.dir, module_relpath) - - if patch_path.exists(): - remove = questionary.confirm( - f"Patch exists for module '{module_fullname}'. Are you sure you want to remove?", - style=nf_core.utils.nfcore_question_style, - ).unsafe_ask() - if not remove: - return - - # Try to apply the patch in reverse and move resulting files to module dir - temp_module_dir = self.modules_json.try_apply_patch_reverse( - module, self.modules_repo.repo_path, patch_relpath, module_path - ) - try: - for file in Path(temp_module_dir).glob("*"): - file.rename(module_path.joinpath(file.name)) - os.rmdir(temp_module_dir) - except Exception as err: - raise UserWarning(f"There was a problem reverting the patched file: {err}") - - log.info(f"Patch for {module} reverted!") - # Remove patch file if we could revert the patch - patch_path.unlink() - # Write changes to module.json - self.modules_json.remove_patch_entry(module, self.modules_repo.remote_url, module_dir) - - if not all( - self.modules_repo.component_files_identical(module, module_path, module_version, "modules").values() - ): - log.error( - f"Module files do not appear to match the remote for the commit sha in the 'module.json': {module_version}\n" - f"Recommend reinstalling with 'nf-core modules install --force --sha {module_version} {module}' " - ) +class ModulePatch(ComponentPatch): + def __init__(self, pipeline_dir, remote_url=None, branch=None, no_pull=False, installed_by=False): + super().__init__(pipeline_dir, "modules", remote_url, branch, no_pull, installed_by) From 3bae03e5859c5c5f93e28997b2a2be98466235fa Mon Sep 17 00:00:00 2001 From: bjlang <> Date: Wed, 18 Oct 2023 12:34:24 +0200 Subject: [PATCH 2/5] correct paths --- nf_core/components/patch.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/nf_core/components/patch.py b/nf_core/components/patch.py index aaa5c3dd10..0f93b2081b 100644 --- a/nf_core/components/patch.py +++ b/nf_core/components/patch.py @@ -19,7 +19,7 @@ class ComponentPatch(ComponentCommand): def __init__(self, pipeline_dir, component_type, remote_url=None, branch=None, no_pull=False, installed_by=False): super().__init__(component_type, pipeline_dir, remote_url, branch, no_pull) - self.modules_json = ModulesJson(dir) + self.modules_json = ModulesJson(pipeline_dir) def _parameter_checks(self, component): """Checks the compatibility of the supplied parameters. @@ -81,7 +81,7 @@ def patch(self, component=None): # Set the diff filename based on the module name patch_filename = f"{component.replace('/', '-')}.diff" - component_relpath = Path("modules", component_dir, component) + component_relpath = Path(self.component_type, component_dir, component) patch_relpath = Path(component_relpath, patch_filename) component_current_dir = Path(self.dir, component_relpath) patch_path = Path(self.dir, patch_relpath) @@ -158,7 +158,7 @@ def remove(self, component): style=nf_core.utils.nfcore_question_style, ).unsafe_ask() component_dir = [dir for dir, m in components if m == component][0] - component_fullname = str(Path("modules", component_dir, component)) + component_fullname = str(Path(self.component_type, component_dir, component)) # Verify that the component has an entry in the modules.json file if not self.modules_json.module_present(component, self.modules_repo.remote_url, component_dir): @@ -182,7 +182,7 @@ def remove(self, component): # Set the diff filename based on the component name patch_filename = f"{component.replace('/', '-')}.diff" - component_relpath = Path("modules", component_dir, component) + component_relpath = Path(self.component_type, component_dir, component) patch_relpath = Path(component_relpath, patch_filename) patch_path = Path(self.dir, patch_relpath) component_path = Path(self.dir, component_relpath) @@ -213,7 +213,7 @@ def remove(self, component): self.modules_json.remove_patch_entry(component, self.modules_repo.remote_url, component_dir) if not all( - self.modules_repo.component_files_identical(component, component_path, component_version, "modules").values() + self.modules_repo.component_files_identical(component, component_path, component_version, self.component_type).values() ): log.error( f"Module files do not appear to match the remote for the commit sha in the 'module.json': {component_version}\n" From 5582012d318446b1130a8abd6db5e31465b3e1f3 Mon Sep 17 00:00:00 2001 From: bjlang <> Date: Wed, 18 Oct 2023 12:50:37 +0200 Subject: [PATCH 3/5] Make isort happy --- nf_core/components/patch.py | 1 - 1 file changed, 1 deletion(-) diff --git a/nf_core/components/patch.py b/nf_core/components/patch.py index 0f93b2081b..e17c05713a 100644 --- a/nf_core/components/patch.py +++ b/nf_core/components/patch.py @@ -8,7 +8,6 @@ import nf_core.utils from nf_core.components.components_command import ComponentCommand - from nf_core.modules.modules_differ import ModulesDiffer from nf_core.modules.modules_json import ModulesJson From 6c1d2700f2560f515ed8feeb1e2aa82f53967793 Mon Sep 17 00:00:00 2001 From: bjlang <> Date: Mon, 23 Oct 2023 11:51:57 +0200 Subject: [PATCH 4/5] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 04a40a48c7..0f10292826 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ ### Modules - Added stub test creation to `create_test_yml` ([#2476](https://github.com/nf-core/tools/pull/2476)) +- Replace ModulePatch by ComponentPatch ([#2482](https://github.com/nf-core/tools/pull/2482)) ### Subworkflows From bf77d8872c71e363d372a9575267a0b2e7bda921 Mon Sep 17 00:00:00 2001 From: nf-core-bot Date: Mon, 23 Oct 2023 09:58:54 +0000 Subject: [PATCH 5/5] [automated] Fix code linting --- nf_core/components/patch.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/nf_core/components/patch.py b/nf_core/components/patch.py index e17c05713a..28f2f886b1 100644 --- a/nf_core/components/patch.py +++ b/nf_core/components/patch.py @@ -34,7 +34,9 @@ def _parameter_checks(self, component): if component is not None and component not in component_names: component_dir = [dir for dir, m in components if m == component][0] - raise UserWarning(f"{self.component_type[:-1].title()} '{Path(self.component_type, component_dir, module)}' does not exist in the pipeline") + raise UserWarning( + f"{self.component_type[:-1].title()} '{Path(self.component_type, component_dir, module)}' does not exist in the pipeline" + ) def patch(self, component=None): # Check modules directory structure @@ -212,7 +214,9 @@ def remove(self, component): self.modules_json.remove_patch_entry(component, self.modules_repo.remote_url, component_dir) if not all( - self.modules_repo.component_files_identical(component, component_path, component_version, self.component_type).values() + self.modules_repo.component_files_identical( + component, component_path, component_version, self.component_type + ).values() ): log.error( f"Module files do not appear to match the remote for the commit sha in the 'module.json': {component_version}\n"