Merge pull request #2216 from awgymer/modules-remove-patch

Modules remove patch

nf_core/__main__.py

@@ -618,7 +618,8 @@ def update(ctx, tool, dir, force, prompt, sha, all, preview, save_diff, update_d
     default=".",
     help=r"Pipeline directory. [dim]\[default: current working directory][/]",
 )
-def patch(ctx, tool, dir):
+@click.option("-r", "--remove", is_flag=True, default=False)
+def patch(ctx, tool, dir, remove):
     """
     Create a patch file for minor changes in a module
 
@@ -632,7 +633,10 @@ def patch(ctx, tool, dir):
             ctx.obj["modules_repo_branch"],
             ctx.obj["modules_repo_no_pull"],
         )
-        module_patch.patch(tool)
+        if remove:
+            module_patch.remove(tool)
+        else:
+            module_patch.patch(tool)
     except (UserWarning, LookupError) as e:
         log.error(e)
         sys.exit(1)

nf_core/modules/modules_json.py

@@ -745,6 +745,16 @@ def add_patch_entry(self, module_name, repo_url, install_dir, patch_filename, wr
         if write_file:
             self.dump()
 
+    def remove_patch_entry(self, module_name, repo_url, install_dir, write_file=True):
+        if self.modules_json is None:
+            self.load()
+        try:
+            del self.modules_json["repos"][repo_url]["modules"][install_dir][module_name]["patch"]
+        except KeyError:
+            log.warning("No patch entry in 'modules.json' to remove")
+        if write_file:
+            self.dump()
+
     def get_patch_fn(self, module_name, repo_url, install_dir):
         """
         Get the patch filename of a module

nf_core/modules/patch.py

@@ -130,3 +130,80 @@ def patch(self, module=None):
         # 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.module_files_identical(module, module_path, module_version).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}' "
+            )

tests/modules/patch.py

@@ -1,6 +1,7 @@
 import os
 import tempfile
 from pathlib import Path
+from unittest import mock
 
 import pytest
 
@@ -326,3 +327,34 @@ def test_create_patch_update_fail(self):
     with open(module_path / patch_fn, "r") as fh:
         new_patch_contents = fh.read()
     assert patch_contents == new_patch_contents
+
+
+def test_remove_patch(self):
+    """Test creating a patch when there is no change to the module"""
+    setup_patch(self.pipeline_dir, True)
+
+    # Try creating a patch file
+    patch_obj = nf_core.modules.ModulePatch(self.pipeline_dir, GITLAB_URL, PATCH_BRANCH)
+    patch_obj.patch(BISMARK_ALIGN)
+
+    module_path = Path(self.pipeline_dir, "modules", REPO_NAME, BISMARK_ALIGN)
+
+    # Check that a patch file with the correct name has been created
+    patch_fn = f"{'-'.join(BISMARK_ALIGN.split('/'))}.diff"
+    assert set(os.listdir(module_path)) == {"main.nf", "meta.yml", patch_fn}
+
+    # Check the 'modules.json' contains a patch file for the module
+    modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir)
+    assert modules_json_obj.get_patch_fn(BISMARK_ALIGN, REPO_URL, REPO_NAME) == Path(
+        "modules", REPO_NAME, BISMARK_ALIGN, patch_fn
+    )
+
+    with mock.patch.object(nf_core.create.questionary, "confirm") as mock_questionary:
+        mock_questionary.unsafe_ask.return_value = True
+        patch_obj.remove(BISMARK_ALIGN)
+    # Check that the diff file has been removed
+    assert set(os.listdir(module_path)) == {"main.nf", "meta.yml"}
+
+    # Check that the 'modules.json' entry has been removed
+    modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir)
+    assert modules_json_obj.get_patch_fn(BISMARK_ALIGN, REPO_URL, REPO_NAME) is None

tests/test_modules.py

@@ -204,6 +204,7 @@ def test_modulesrepo_class(self):
         test_create_patch_try_apply_successful,
         test_create_patch_update_fail,
         test_create_patch_update_success,
+        test_remove_patch,
     )
     from .modules.remove import (
         test_modules_remove_multiqc_from_gitlab,