From 585b1bceb73958b2f80bc2578e0103c972253a1b Mon Sep 17 00:00:00 2001 From: asp8200 Date: Thu, 29 Feb 2024 16:10:29 +0000 Subject: [PATCH 01/10] Have component_files_identical check for new optional files in modules --- nf_core/synced_repo.py | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/nf_core/synced_repo.py b/nf_core/synced_repo.py index 5c31e9691..936c92bab 100644 --- a/nf_core/synced_repo.py +++ b/nf_core/synced_repo.py @@ -314,15 +314,25 @@ def component_files_identical(self, component_name, base_path, commit, component self.checkout_branch() else: self.checkout(commit) - component_files = ["main.nf", "meta.yml"] + required_files = ["main.nf", "meta.yml"] + optional_files = ["environment.yml", "tests/main.nf.test", "tests/main.nf.test.snap", "tests/tags.yml"] + component_files = required_files + optional_files files_identical = {file: True for file in component_files} component_dir = self.get_component_dir(component_name, component_type) for file in component_files: + component_file = os.path.join(component_dir, file) + base_file = os.path.join(base_path, file) + if (file in optional_files) and (not os.path.exists(component_file)) and (not os.path.exists(base_file)): + log.debug(f'The optional file "{file}" was not present.') + # TO-DO: Not sure we need to log this, but if we do what should the msg be? + # In this case, `files_identical[file]` will remain `True`. Is that what we want? + continue try: - files_identical[file] = filecmp.cmp(os.path.join(component_dir, file), os.path.join(base_path, file)) + files_identical[file] = filecmp.cmp(component_file, base_file) except FileNotFoundError: - log.debug(f"Could not open file: {os.path.join(component_dir, file)}") - continue + errmsg = f"Could not open file: {os.path.join(component_dir, file)}" + log.error(errmsg) + raise UserWarning(errmsg) self.checkout_branch() return files_identical From a9cd3256ff1e7ff71896c7ac6ac504be6a1b8e62 Mon Sep 17 00:00:00 2001 From: Anders Sune Pedersen <37172585+asp8200@users.noreply.github.com> Date: Fri, 1 Mar 2024 13:42:29 +0100 Subject: [PATCH 02/10] Update nf_core/synced_repo.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Matthias Hörtenhuber --- nf_core/synced_repo.py | 1 - 1 file changed, 1 deletion(-) diff --git a/nf_core/synced_repo.py b/nf_core/synced_repo.py index 936c92bab..63cd1ac09 100644 --- a/nf_core/synced_repo.py +++ b/nf_core/synced_repo.py @@ -331,7 +331,6 @@ def component_files_identical(self, component_name, base_path, commit, component files_identical[file] = filecmp.cmp(component_file, base_file) except FileNotFoundError: errmsg = f"Could not open file: {os.path.join(component_dir, file)}" - log.error(errmsg) raise UserWarning(errmsg) self.checkout_branch() return files_identical From 18497c2c6056f0f52dd0638e78af05f322a66390 Mon Sep 17 00:00:00 2001 From: Anders Sune Pedersen <37172585+asp8200@users.noreply.github.com> Date: Fri, 1 Mar 2024 13:42:48 +0100 Subject: [PATCH 03/10] Update nf_core/synced_repo.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Matthias Hörtenhuber --- nf_core/synced_repo.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nf_core/synced_repo.py b/nf_core/synced_repo.py index 63cd1ac09..28e08f171 100644 --- a/nf_core/synced_repo.py +++ b/nf_core/synced_repo.py @@ -320,7 +320,7 @@ def component_files_identical(self, component_name, base_path, commit, component files_identical = {file: True for file in component_files} component_dir = self.get_component_dir(component_name, component_type) for file in component_files: - component_file = os.path.join(component_dir, file) + component_file = Path(component_dir, file) base_file = os.path.join(base_path, file) if (file in optional_files) and (not os.path.exists(component_file)) and (not os.path.exists(base_file)): log.debug(f'The optional file "{file}" was not present.') From c611a3507e13ad0c6eb9dbb8bacef2cde1034810 Mon Sep 17 00:00:00 2001 From: Anders Sune Pedersen <37172585+asp8200@users.noreply.github.com> Date: Fri, 1 Mar 2024 13:42:59 +0100 Subject: [PATCH 04/10] Update nf_core/synced_repo.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Matthias Hörtenhuber --- nf_core/synced_repo.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nf_core/synced_repo.py b/nf_core/synced_repo.py index 28e08f171..a3ae881ab 100644 --- a/nf_core/synced_repo.py +++ b/nf_core/synced_repo.py @@ -321,7 +321,7 @@ def component_files_identical(self, component_name, base_path, commit, component component_dir = self.get_component_dir(component_name, component_type) for file in component_files: component_file = Path(component_dir, file) - base_file = os.path.join(base_path, file) + base_file = Path(base_path, file) if (file in optional_files) and (not os.path.exists(component_file)) and (not os.path.exists(base_file)): log.debug(f'The optional file "{file}" was not present.') # TO-DO: Not sure we need to log this, but if we do what should the msg be? From 437ec9eae00e37c88338594fc2ed6c6303375959 Mon Sep 17 00:00:00 2001 From: Anders Sune Pedersen <37172585+asp8200@users.noreply.github.com> Date: Fri, 1 Mar 2024 13:43:17 +0100 Subject: [PATCH 05/10] Update nf_core/synced_repo.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Matthias Hörtenhuber --- nf_core/synced_repo.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nf_core/synced_repo.py b/nf_core/synced_repo.py index a3ae881ab..4f9ea8b24 100644 --- a/nf_core/synced_repo.py +++ b/nf_core/synced_repo.py @@ -322,7 +322,7 @@ def component_files_identical(self, component_name, base_path, commit, component for file in component_files: component_file = Path(component_dir, file) base_file = Path(base_path, file) - if (file in optional_files) and (not os.path.exists(component_file)) and (not os.path.exists(base_file)): + if (file in optional_files) and (not component_file.exists()) and (not base_file.exists()): log.debug(f'The optional file "{file}" was not present.') # TO-DO: Not sure we need to log this, but if we do what should the msg be? # In this case, `files_identical[file]` will remain `True`. Is that what we want? From b72f6cf5dc780b44260a20bf5f190b0c8e103cd2 Mon Sep 17 00:00:00 2001 From: asp8200 Date: Fri, 1 Mar 2024 13:02:54 +0000 Subject: [PATCH 06/10] Better error msg from fct component_files_identical --- nf_core/synced_repo.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/nf_core/synced_repo.py b/nf_core/synced_repo.py index 4f9ea8b24..41cba8037 100644 --- a/nf_core/synced_repo.py +++ b/nf_core/synced_repo.py @@ -329,9 +329,8 @@ def component_files_identical(self, component_name, base_path, commit, component continue try: files_identical[file] = filecmp.cmp(component_file, base_file) - except FileNotFoundError: - errmsg = f"Could not open file: {os.path.join(component_dir, file)}" - raise UserWarning(errmsg) + except FileNotFoundError as e: + raise UserWarning(f"Could not open file: {e.filename}") self.checkout_branch() return files_identical From 0a3a600eab78504cdfcc502353bd46edc88224b1 Mon Sep 17 00:00:00 2001 From: asp8200 Date: Fri, 1 Mar 2024 13:05:52 +0000 Subject: [PATCH 07/10] No entry in dict files_identical for missing optional files --- nf_core/synced_repo.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/nf_core/synced_repo.py b/nf_core/synced_repo.py index 41cba8037..2fe42f937 100644 --- a/nf_core/synced_repo.py +++ b/nf_core/synced_repo.py @@ -317,15 +317,13 @@ def component_files_identical(self, component_name, base_path, commit, component required_files = ["main.nf", "meta.yml"] optional_files = ["environment.yml", "tests/main.nf.test", "tests/main.nf.test.snap", "tests/tags.yml"] component_files = required_files + optional_files - files_identical = {file: True for file in component_files} + files_identical = {} component_dir = self.get_component_dir(component_name, component_type) for file in component_files: component_file = Path(component_dir, file) base_file = Path(base_path, file) if (file in optional_files) and (not component_file.exists()) and (not base_file.exists()): log.debug(f'The optional file "{file}" was not present.') - # TO-DO: Not sure we need to log this, but if we do what should the msg be? - # In this case, `files_identical[file]` will remain `True`. Is that what we want? continue try: files_identical[file] = filecmp.cmp(component_file, base_file) From 80748f3272a8c620f9053cd0ec8bb4d2520bdbca Mon Sep 17 00:00:00 2001 From: mashehu Date: Tue, 12 Mar 2024 10:14:41 +0100 Subject: [PATCH 08/10] copy all files when reverting a patch --- nf_core/modules/modules_json.py | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/nf_core/modules/modules_json.py b/nf_core/modules/modules_json.py index f68c27b2d..326d94872 100644 --- a/nf_core/modules/modules_json.py +++ b/nf_core/modules/modules_json.py @@ -799,7 +799,9 @@ def get_patch_fn(self, module_name, repo_url, install_dir): ) return Path(path) if path is not None else None - def try_apply_patch_reverse(self, module, repo_name, patch_relpath, module_dir): + def try_apply_patch_reverse( + self, module: str, repo_name: str, patch_relpath: Union[Path, str], module_dir: Union[Path, str] + ) -> Path: """ Try reverse applying a patch file to the modified module files @@ -822,17 +824,30 @@ def try_apply_patch_reverse(self, module, repo_name, patch_relpath, module_dir): new_files = ModulesDiffer.try_apply_patch(module, repo_name, patch_path, module_dir, reverse=True) except LookupError as e: raise LookupError(f"Failed to apply patch in reverse for module '{module_fullname}' due to: {e}") - - # Write the patched files to a temporary directory - log.debug("Writing patched files to tmpdir") + # get all files of the module + + module_files = list(Path(module_dir).rglob("*")) + # exclude the patch file + log.info(f"Excluding patch file '{patch_path}' from the patched files") + unpatched_module_files = [f for f in module_files if f != patch_path] + # Write the patched files and rest of the files to a temporary directory + log.info("Writing patched files to tmpdir") temp_dir = Path(tempfile.mkdtemp()) temp_module_dir = temp_dir / module temp_module_dir.mkdir(parents=True, exist_ok=True) + for file, new_content in new_files.items(): fn = temp_module_dir / file with open(fn, "w") as fh: fh.writelines(new_content) + # copy old files to the temp dir + for file in unpatched_module_files: + if file.is_file(): + shutil.copy(file, temp_module_dir / file.relative_to(module_dir)) + else: + shutil.copytree(file, temp_module_dir / file.relative_to(module_dir)) + return temp_module_dir def repo_present(self, repo_name): From d2c00fe50391222a039a52aa7bd0fabd430ff8ac Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Tue, 12 Mar 2024 14:16:01 +0100 Subject: [PATCH 09/10] be file agnostic when checking that all component files are identical --- nf_core/modules/modules_json.py | 12 ------------ nf_core/synced_repo.py | 12 +++++------- 2 files changed, 5 insertions(+), 19 deletions(-) diff --git a/nf_core/modules/modules_json.py b/nf_core/modules/modules_json.py index 326d94872..3c024aac4 100644 --- a/nf_core/modules/modules_json.py +++ b/nf_core/modules/modules_json.py @@ -824,12 +824,7 @@ def try_apply_patch_reverse( new_files = ModulesDiffer.try_apply_patch(module, repo_name, patch_path, module_dir, reverse=True) except LookupError as e: raise LookupError(f"Failed to apply patch in reverse for module '{module_fullname}' due to: {e}") - # get all files of the module - module_files = list(Path(module_dir).rglob("*")) - # exclude the patch file - log.info(f"Excluding patch file '{patch_path}' from the patched files") - unpatched_module_files = [f for f in module_files if f != patch_path] # Write the patched files and rest of the files to a temporary directory log.info("Writing patched files to tmpdir") temp_dir = Path(tempfile.mkdtemp()) @@ -841,13 +836,6 @@ def try_apply_patch_reverse( with open(fn, "w") as fh: fh.writelines(new_content) - # copy old files to the temp dir - for file in unpatched_module_files: - if file.is_file(): - shutil.copy(file, temp_module_dir / file.relative_to(module_dir)) - else: - shutil.copytree(file, temp_module_dir / file.relative_to(module_dir)) - return temp_module_dir def repo_present(self, repo_name): diff --git a/nf_core/synced_repo.py b/nf_core/synced_repo.py index 2fe42f937..23966be13 100644 --- a/nf_core/synced_repo.py +++ b/nf_core/synced_repo.py @@ -314,16 +314,14 @@ def component_files_identical(self, component_name, base_path, commit, component self.checkout_branch() else: self.checkout(commit) - required_files = ["main.nf", "meta.yml"] - optional_files = ["environment.yml", "tests/main.nf.test", "tests/main.nf.test.snap", "tests/tags.yml"] - component_files = required_files + optional_files files_identical = {} component_dir = self.get_component_dir(component_name, component_type) + component_files = [file.relative_to(component_dir) for file in Path(component_dir).rglob("*") if file.is_file()] for file in component_files: - component_file = Path(component_dir, file) - base_file = Path(base_path, file) - if (file in optional_files) and (not component_file.exists()) and (not base_file.exists()): - log.debug(f'The optional file "{file}" was not present.') + component_file = component_dir / file + base_file = base_path / file + if not base_file.exists(): + files_identical[file] = False continue try: files_identical[file] = filecmp.cmp(component_file, base_file) From 848948aca90d353780b2c4cfbbd84e0fd533e86e Mon Sep 17 00:00:00 2001 From: Anders Sune Pedersen <37172585+asp8200@users.noreply.github.com> Date: Fri, 15 Mar 2024 16:12:44 +0100 Subject: [PATCH 10/10] Update nf_core/modules/modules_json.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Matthias Hörtenhuber --- nf_core/modules/modules_json.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nf_core/modules/modules_json.py b/nf_core/modules/modules_json.py index 3c024aac4..3233f6948 100644 --- a/nf_core/modules/modules_json.py +++ b/nf_core/modules/modules_json.py @@ -826,7 +826,7 @@ def try_apply_patch_reverse( raise LookupError(f"Failed to apply patch in reverse for module '{module_fullname}' due to: {e}") # Write the patched files and rest of the files to a temporary directory - log.info("Writing patched files to tmpdir") + log.debug("Writing patched files to tmpdir") temp_dir = Path(tempfile.mkdtemp()) temp_module_dir = temp_dir / module temp_module_dir.mkdir(parents=True, exist_ok=True)