From 3467f5833062f2c6da28aafc01b78c4f8903884a Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Thu, 10 Nov 2022 17:08:08 +0100 Subject: [PATCH 01/31] first refactoring of update commands --- nf_core/components/update.py | 704 ++++++++++++++++++++++++++++++++ nf_core/modules/modules_json.py | 23 ++ nf_core/modules/update.py | 702 +------------------------------ nf_core/subworkflows/update.py | 30 ++ 4 files changed, 771 insertions(+), 688 deletions(-) create mode 100644 nf_core/components/update.py create mode 100644 nf_core/subworkflows/update.py diff --git a/nf_core/components/update.py b/nf_core/components/update.py new file mode 100644 index 0000000000..7ba6cff81e --- /dev/null +++ b/nf_core/components/update.py @@ -0,0 +1,704 @@ +import logging +import os +import shutil +import tempfile +from pathlib import Path + +import questionary + +import nf_core.modules.modules_utils +import nf_core.utils +from nf_core.components.components_command import ComponentCommand +from nf_core.components.components_utils import prompt_component_version_sha +from nf_core.modules.modules_differ import ModulesDiffer +from nf_core.modules.modules_json import ModulesJson +from nf_core.modules.modules_repo import ModulesRepo +from nf_core.utils import plural_es, plural_s, plural_y + +log = logging.getLogger(__name__) + + +class ComponentUpdate(ComponentCommand): + def __init__( + self, + pipeline_dir, + component_type, + force=False, + prompt=False, + sha=None, + update_all=False, + show_diff=None, + save_diff_fn=None, + remote_url=None, + branch=None, + no_pull=False, + ): + super().__init__(component_type, pipeline_dir, remote_url, branch, no_pull) + self.force = force + self.prompt = prompt + self.sha = sha + self.update_all = update_all + self.show_diff = show_diff + self.save_diff_fn = save_diff_fn + self.module = None + self.update_config = None + self.modules_json = ModulesJson(self.dir) + self.branch = branch + + def _parameter_checks(self): + """Checks the compatibilty of the supplied parameters. + + Raises: + UserWarning: if any checks fail. + """ + + if self.save_diff_fn and self.show_diff: + raise UserWarning("Either `--preview` or `--save_diff` can be specified, not both.") + + if self.update_all and self.module: + raise UserWarning("Either a module or the '--all' flag can be specified, not both.") + + if self.repo_type == "modules": + raise UserWarning("Modules in clones of nf-core/modules can not be updated.") + + if self.prompt and self.sha is not None: + raise UserWarning("Cannot use '--sha' and '--prompt' at the same time.") + + if not self.has_valid_directory(): + raise UserWarning("The command was not run in a valid pipeline directory.") + + def update(self, module=None): + """Updates a specified module or all modules modules in a pipeline. + + Args: + module (str): The name of the module to update. + + Returns: + bool: True if the update was successful, False otherwise. + """ + self.module = module + + tool_config = nf_core.utils.load_tools_config(self.dir) + self.update_config = tool_config.get("update", {}) + + self._parameter_checks() + + # 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() + + if not self.update_all and module is None: + choices = ["All modules", "Named module"] + self.update_all = ( + questionary.select( + "Update all modules or a single named module?", + choices=choices, + style=nf_core.utils.nfcore_question_style, + ).unsafe_ask() + == "All modules" + ) + + # Verify that the provided SHA exists in the repo + if self.sha is not None and not self.modules_repo.sha_exists_on_branch(self.sha): + log.error(f"Commit SHA '{self.sha}' doesn't exist in '{self.modules_repo.remote_url}'") + return False + + # Get the list of modules to update, and their version information + modules_info = self.get_all_modules_info() if self.update_all else [self.get_single_module_info(module)] + + # Save the current state of the modules.json + old_modules_json = self.modules_json.get_modules_json() + + # Ask if we should show the diffs (unless a filename was already given on the command line) + if not self.save_diff_fn and self.show_diff is None: + diff_type = questionary.select( + "Do you want to view diffs of the proposed changes?", + choices=[ + {"name": "No previews, just update everything", "value": 0}, + {"name": "Preview diff in terminal, choose whether to update files", "value": 1}, + {"name": "Just write diffs to a patch file", "value": 2}, + ], + style=nf_core.utils.nfcore_question_style, + ).unsafe_ask() + + self.show_diff = diff_type == 1 + self.save_diff_fn = diff_type == 2 + + if self.save_diff_fn: # True or a string + self.setup_diff_file() + + # Loop through all modules to be updated + # and do the requested action on them + exit_value = True + all_patches_successful = True + for modules_repo, module, sha, patch_relpath in modules_info: + module_fullname = str(Path("modules", modules_repo.repo_path, module)) + # Are we updating the files in place or not? + dry_run = self.show_diff or self.save_diff_fn + + current_version = self.modules_json.get_module_version( + module, modules_repo.remote_url, modules_repo.repo_path + ) + + # Set the temporary installation folder + install_tmp_dir = Path(tempfile.mkdtemp()) + module_install_dir = install_tmp_dir / module + + # Compute the module directory + module_dir = os.path.join(self.dir, "modules", modules_repo.repo_path, module) + + if sha is not None: + version = sha + elif self.prompt: + version = prompt_component_version_sha( + module, "modules", modules_repo=modules_repo, installed_sha=current_version + ) + else: + version = modules_repo.get_latest_component_version(module, self.component_type) + + if current_version is not None and not self.force: + if current_version == version: + if self.sha or self.prompt: + log.info(f"'{module_fullname}' is already installed at {version}") + else: + log.info(f"'{module_fullname}' is already up to date") + continue + + # Download module files + if not self.install_component_files(module, version, modules_repo, install_tmp_dir): + exit_value = False + continue + + if patch_relpath is not None: + patch_successful = self.try_apply_patch( + module, modules_repo.repo_path, patch_relpath, module_dir, module_install_dir + ) + if patch_successful: + log.info(f"Module '{module_fullname}' patched successfully") + else: + log.warning(f"Failed to patch module '{module_fullname}'. Will proceed with unpatched files.") + all_patches_successful &= patch_successful + + if dry_run: + if patch_relpath is not None: + if patch_successful: + log.info("Current installation is compared against patched version in remote.") + else: + log.warning("Current installation is compared against unpatched version in remote.") + # Compute the diffs for the module + if self.save_diff_fn: + log.info(f"Writing diff file for module '{module_fullname}' to '{self.save_diff_fn}'") + ModulesDiffer.write_diff_file( + self.save_diff_fn, + module, + modules_repo.repo_path, + module_dir, + module_install_dir, + current_version, + version, + dsp_from_dir=module_dir, + dsp_to_dir=module_dir, + ) + + elif self.show_diff: + ModulesDiffer.print_diff( + module, + modules_repo.repo_path, + module_dir, + module_install_dir, + current_version, + version, + dsp_from_dir=module_dir, + dsp_to_dir=module_dir, + ) + + # Ask the user if they want to install the module + dry_run = not questionary.confirm( + f"Update module '{module}'?", default=False, style=nf_core.utils.nfcore_question_style + ).unsafe_ask() + + if not dry_run: + # Clear the module directory and move the installed files there + self.move_files_from_tmp_dir(module, install_tmp_dir, modules_repo.repo_path, version) + # Update modules.json with newly installed module + self.modules_json.update(modules_repo, module, version, self.component_type) + else: + # Don't save to a file, just iteratively update the variable + self.modules_json.update(modules_repo, module, version, self.component_type, write_file=False) + + if self.save_diff_fn: + # Write the modules.json diff to the file + ModulesDiffer.append_modules_json_diff( + self.save_diff_fn, + old_modules_json, + self.modules_json.get_modules_json(), + Path(self.dir, "modules.json"), + ) + if exit_value: + log.info( + f"[bold magenta italic] TIP! [/] If you are happy with the changes in '{self.save_diff_fn}', you " + "can apply them by running the command :point_right:" + f" [bold magenta italic]git apply {self.save_diff_fn} [/]" + ) + elif not all_patches_successful: + log.info(f"Updates complete. Please apply failed patch{plural_es(modules_info)} manually") + else: + log.info("Updates complete :sparkles:") + + return exit_value + + def get_single_module_info(self, module): + """Collects the module repository, version and sha for a module. + + Information about the module version in the '.nf-core.yml' overrides + the '--sha' option + + Args: + module_name (str): The name of the module to get info for. + + Returns: + (ModulesRepo, str, str): The modules repo containing the module, + the module name, and the module version. + + Raises: + LookupError: If the module is not found either in the pipeline or the modules repo. + UserWarning: If the '.nf-core.yml' entry is not valid. + """ + # Check if there are any modules installed from the repo + repo_url = self.modules_repo.remote_url + modules = self.modules_json.get_all_modules().get(repo_url) + choices = [module if dir == "nf-core" else f"{dir}/{module}" for dir, module in modules] + if repo_url not in self.modules_json.get_all_modules(): + raise LookupError(f"No modules installed from '{repo_url}'") + + if module is None: + module = questionary.autocomplete( + "Tool name:", + choices=choices, + style=nf_core.utils.nfcore_question_style, + ).unsafe_ask() + + # Get module installation directory + install_dir = [dir for dir, m in modules if module == m][0] + + # Check if module is installed before trying to update + if module not in choices: + raise LookupError(f"Module '{module}' is not installed in pipeline and could therefore not be updated") + + # Check that the supplied name is an available module + if module and module not in self.modules_repo.get_avail_components(self.component_type): + raise LookupError( + f"Module '{module}' not found in list of available modules." + f"Use the command 'nf-core modules list remote' to view available software" + ) + + sha = self.sha + if module in self.update_config.get(self.modules_repo.remote_url, {}).get(install_dir, {}): + # If the module to update is in .nf-core.yml config file + config_entry = self.update_config[self.modules_repo.remote_url][install_dir].get(module) + if config_entry is not None and config_entry is not True: + if config_entry is False: + raise UserWarning("Module's update entry in '.nf-core.yml' is set to False") + if not isinstance(config_entry, str): + raise UserWarning("Module's update entry in '.nf-core.yml' is of wrong type") + + sha = config_entry + if self.sha is not None: + log.warning( + f"Found entry in '.nf-core.yml' for module '{module}' " + "which will override version specified with '--sha'" + ) + else: + log.info(f"Found entry in '.nf-core.yml' for module '{module}'") + log.info(f"Updating module to ({sha})") + + # Check if the update branch is the same as the installation branch + current_branch = self.modules_json.get_component_branch( + self.component_type, module, self.modules_repo.remote_url, install_dir + ) + new_branch = self.modules_repo.branch + if current_branch != new_branch: + log.warning( + f"You are trying to update the '{Path(install_dir, module)}' module from " + f"the '{new_branch}' branch. This module was installed from the '{current_branch}'" + ) + switch = questionary.confirm(f"Do you want to update using the '{current_branch}' instead?").unsafe_ask() + if switch: + # Change the branch + self.modules_repo.setup_branch(current_branch) + + # If there is a patch file, get its filename + patch_fn = self.modules_json.get_patch_fn(module, self.modules_repo.remote_url, install_dir) + + return (self.modules_repo, module, sha, patch_fn) + + def get_all_modules_info(self, branch=None): + """Collects the module repository, version and sha for all modules. + + Information about the module version in the '.nf-core.yml' overrides the '--sha' option. + + Returns: + [(ModulesRepo, str, str)]: A list of tuples containing a ModulesRepo object, + the module name, and the module version. + """ + if branch is not None: + use_branch = questionary.confirm( + "'--branch' was specified. Should this branch be used to update all modules?", default=False + ) + if not use_branch: + branch = None + skipped_repos = [] + skipped_modules = [] + overridden_repos = [] + overridden_modules = [] + modules_info = {} + # Loop through all the modules in the pipeline + # and check if they have an entry in the '.nf-core.yml' file + for repo_name, modules in self.modules_json.get_all_modules().items(): + if repo_name not in self.update_config or self.update_config[repo_name] is True: + # There aren't restrictions for the repository in .nf-core.yml file + modules_info[repo_name] = {} + for module_dir, module in modules: + try: + modules_info[repo_name][module_dir].append( + ( + module, + self.sha, + self.modules_json.get_component_branch( + self.component_type, module, repo_name, module_dir + ), + ) + ) + except KeyError: + modules_info[repo_name][module_dir] = [ + ( + module, + self.sha, + self.modules_json.get_component_branch( + self.component_type, module, repo_name, module_dir + ), + ) + ] + elif isinstance(self.update_config[repo_name], dict): + # If it is a dict, then there are entries for individual modules or module directories + for module_dir in set([dir for dir, _ in modules]): + if isinstance(self.update_config[repo_name][module_dir], str): + # If a string is given it is the commit SHA to which we should update to + custom_sha = self.update_config[repo_name][module_dir] + modules_info[repo_name] = {} + for dir, module in modules: + if module_dir == dir: + try: + modules_info[repo_name][module_dir].append( + ( + module, + custom_sha, + self.modules_json.get_component_branch( + self.component_type, module, repo_name, module_dir + ), + ) + ) + except KeyError: + modules_info[repo_name][module_dir] = [ + ( + module, + custom_sha, + self.modules_json.get_component_branch( + self.component_type, module, repo_name, module_dir + ), + ) + ] + if self.sha is not None: + overridden_repos.append(repo_name) + elif self.update_config[repo_name][module_dir] is False: + for dir, module in modules: + if dir == module_dir: + skipped_modules.append(f"{module_dir}/{module}") + elif isinstance(self.update_config[repo_name][module_dir], dict): + # If it's a dict, there are entries for individual modules + dir_config = self.update_config[repo_name][module_dir] + modules_info[repo_name] = {} + for module_dir, module in modules: + if module not in dir_config or dir_config[module] is True: + try: + modules_info[repo_name][module_dir].append( + ( + module, + self.sha, + self.modules_json.get_component_branch( + self.component_type, module, repo_name, module_dir + ), + ) + ) + except KeyError: + modules_info[repo_name][module_dir] = [ + ( + module, + self.sha, + self.modules_json.get_component_branch( + self.component_type, module, repo_name, module_dir + ), + ) + ] + elif isinstance(dir_config[module], str): + # If a string is given it is the commit SHA to which we should update to + custom_sha = dir_config[module] + try: + modules_info[repo_name][module_dir].append( + ( + module, + custom_sha, + self.modules_json.get_component_branch( + self.component_type, module, repo_name, module_dir + ), + ) + ) + except KeyError: + modules_info[repo_name][module_dir] = [ + ( + module, + custom_sha, + self.modules_json.get_component_branch( + self.component_type, module, repo_name, module_dir + ), + ) + ] + if self.sha is not None: + overridden_modules.append(module) + elif dir_config[module] is False: + # Otherwise the entry must be 'False' and we should ignore the module + skipped_modules.append(f"{module_dir}/{module}") + else: + raise UserWarning( + f"Module '{module}' in '{module_dir}' has an invalid entry in '.nf-core.yml'" + ) + elif isinstance(self.update_config[repo_name], str): + # If a string is given it is the commit SHA to which we should update to + custom_sha = self.update_config[repo_name] + modules_info[repo_name] = {} + for module_dir, module in modules: + try: + modules_info[repo_name][module_dir].append( + ( + module, + custom_sha, + self.modules_json.get_component_branch( + self.component_type, module, repo_name, module_dir + ), + ) + ) + except KeyError: + modules_info[repo_name][module_dir] = [ + ( + module, + custom_sha, + self.modules_json.get_component_branch( + self.component_type, module, repo_name, module_dir + ), + ) + ] + if self.sha is not None: + overridden_repos.append(repo_name) + elif self.update_config[repo_name] is False: + skipped_repos.append(repo_name) + else: + raise UserWarning(f"Repo '{repo_name}' has an invalid entry in '.nf-core.yml'") + + if skipped_repos: + skipped_str = "', '".join(skipped_repos) + log.info(f"Skipping modules in repositor{plural_y(skipped_repos)}: '{skipped_str}'") + + if skipped_modules: + skipped_str = "', '".join(skipped_modules) + log.info(f"Skipping module{plural_s(skipped_modules)}: '{skipped_str}'") + + if overridden_repos: + overridden_str = "', '".join(overridden_repos) + log.info( + f"Overriding '--sha' flag for modules in repositor{plural_y(overridden_repos)} " + f"with '.nf-core.yml' entry: '{overridden_str}'" + ) + + if overridden_modules: + overridden_str = "', '".join(overridden_modules) + log.info( + f"Overriding '--sha' flag for module{plural_s(overridden_modules)} with " + f"'.nf-core.yml' entry: '{overridden_str}'" + ) + # Loop through modules_info and create on ModulesRepo object per remote and branch + repos_and_branches = {} + for repo_name, repo_content in modules_info.items(): + for module_dir, mods in repo_content.items(): + for mod, sha, mod_branch in mods: + if branch is not None: + mod_branch = branch + if (repo_name, mod_branch) not in repos_and_branches: + repos_and_branches[(repo_name, mod_branch)] = [] + repos_and_branches[(repo_name, mod_branch)].append((mod, sha)) + + # Create ModulesRepo objects + repo_objs_mods = [] + for (repo_url, branch), mods_shas in repos_and_branches.items(): + try: + modules_repo = ModulesRepo(remote_url=repo_url, branch=branch) + except LookupError as e: + log.warning(e) + log.info(f"Skipping modules in '{repo_url}'") + else: + repo_objs_mods.append((modules_repo, mods_shas)) + + # Flatten the list + modules_info = [(repo, mod, sha) for repo, mods_shas in repo_objs_mods for mod, sha in mods_shas] + + # Verify that that all modules and shas exist in their respective ModulesRepo, + # don't try to update those that don't + i = 0 + while i < len(modules_info): + repo, module, sha = modules_info[i] + if not repo.component_exists(module, self.component_type): + log.warning(f"Module '{module}' does not exist in '{repo.remote_url}'. Skipping...") + modules_info.pop(i) + elif sha is not None and not repo.sha_exists_on_branch(sha): + log.warning( + f"Git sha '{sha}' does not exists on the '{repo.branch}' of '{repo.remote_url}'. Skipping module '{module}'" + ) + modules_info.pop(i) + else: + i += 1 + + # Add patch filenames to the modules that have them + modules_info = [ + (repo, mod, sha, self.modules_json.get_patch_fn(mod, repo.remote_url, repo.repo_path)) + for repo, mod, sha in modules_info + ] + + return modules_info + + def setup_diff_file(self): + """Sets up the diff file. + + If the save diff option was chosen interactively, the user is asked to supply a name for the diff file. + + Then creates the file for saving the diff. + """ + if self.save_diff_fn is True: + # From questionary - no filename yet + self.save_diff_fn = questionary.path( + "Enter the filename: ", style=nf_core.utils.nfcore_question_style + ).unsafe_ask() + + self.save_diff_fn = Path(self.save_diff_fn) + + # Check if filename already exists (questionary or cli) + while self.save_diff_fn.exists(): + if questionary.confirm(f"'{self.save_diff_fn}' exists. Remove file?").unsafe_ask(): + os.remove(self.save_diff_fn) + break + self.save_diff_fn = questionary.path( + "Enter a new filename: ", + style=nf_core.utils.nfcore_question_style, + ).unsafe_ask() + self.save_diff_fn = Path(self.save_diff_fn) + + # This guarantees that the file exists after calling the function + self.save_diff_fn.touch() + + def move_files_from_tmp_dir(self, module, install_folder, repo_path, new_version): + """Move the files from the temporary to the installation directory. + + Args: + module (str): The module name. + install_folder [str]: The path to the temporary installation directory. + repo_path (str): The name of the directory where modules are installed + new_version (str): The version of the module that was installed. + """ + temp_module_dir = os.path.join(install_folder, module) + files = os.listdir(temp_module_dir) + pipeline_path = os.path.join(self.dir, "modules", repo_path, module) + + log.debug(f"Removing old version of module '{module}'") + self.clear_component_dir(module, pipeline_path) + + os.makedirs(pipeline_path) + for file in files: + path = os.path.join(temp_module_dir, file) + if os.path.exists(path): + shutil.move(path, os.path.join(pipeline_path, file)) + + log.info(f"Updating '{repo_path}/{module}'") + log.debug(f"Updating module '{module}' to {new_version} from {repo_path}") + + def try_apply_patch(self, module, repo_path, patch_relpath, module_dir, module_install_dir): + """ + Try applying a patch file to the new module files + + + Args: + module (str): The name of the module + repo_path (str): The name of the repository where the module resides + patch_relpath (Path | str): The path to patch file in the pipeline + module_dir (Path | str): The module directory in the pipeline + module_install_dir (Path | str): The directory where the new module + file have been installed + + Returns: + (bool): Whether the patch application was successful + """ + module_fullname = str(Path(repo_path, module)) + log.info(f"Found patch for module '{module_fullname}'. Trying to apply it to new files") + + patch_path = Path(self.dir / patch_relpath) + module_relpath = Path("modules", repo_path, module) + + # Check that paths in patch file are updated + self.check_patch_paths(patch_path, module) + + # Copy the installed files to a new temporary directory to save them for later use + temp_dir = Path(tempfile.mkdtemp()) + temp_module_dir = temp_dir / module + shutil.copytree(module_install_dir, temp_module_dir) + + try: + new_files = ModulesDiffer.try_apply_patch(module, repo_path, patch_path, temp_module_dir) + except LookupError: + # Patch failed. Save the patch file by moving to the install dir + shutil.move(patch_path, Path(module_install_dir, patch_path.relative_to(module_dir))) + log.warning( + f"Failed to apply patch for module '{module_fullname}'. You will have to apply the patch manually" + ) + return False + + # Write the patched files to a temporary directory + log.debug("Writing patched files") + for file, new_content in new_files.items(): + fn = temp_module_dir / file + with open(fn, "w") as fh: + fh.writelines(new_content) + + # Create the new patch file + log.debug("Regenerating patch file") + ModulesDiffer.write_diff_file( + Path(temp_module_dir, patch_path.relative_to(module_dir)), + module, + repo_path, + module_install_dir, + temp_module_dir, + file_action="w", + for_git=False, + dsp_from_dir=module_relpath, + dsp_to_dir=module_relpath, + ) + + # Move the patched files to the install dir + log.debug("Overwriting installed files installed files with patched files") + shutil.rmtree(module_install_dir) + shutil.copytree(temp_module_dir, module_install_dir) + + # Add the patch file to the modules.json file + self.modules_json.add_patch_entry( + module, self.modules_repo.remote_url, repo_path, patch_relpath, write_file=True + ) + + return True diff --git a/nf_core/modules/modules_json.py b/nf_core/modules/modules_json.py index cc52ee896c..e6f4e03571 100644 --- a/nf_core/modules/modules_json.py +++ b/nf_core/modules/modules_json.py @@ -815,6 +815,29 @@ def get_modules_json(self): self.load() return copy.deepcopy(self.modules_json) + def get_component_version(self, component_type, component_name, repo_url, install_dir): + """ + Returns the version of a module or subworkflow + + Args: + component_name (str): Name of the module/subworkflow + repo_url (str): URL of the repository + install_dir (str): Name of the directory where modules/subworkflows are installed + + Returns: + (str): The git SHA of the module/subworkflow if it exists, None otherwise + """ + if self.modules_json is None: + self.load() + return ( + self.modules_json.get("repos", {}) + .get(repo_url, {}) + .get(component_type, {}) + .get(install_dir, {}) + .get(component_name, {}) + .get("git_sha", None) + ) + def get_module_version(self, module_name, repo_url, install_dir): """ Returns the version of a module diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index 297e893401..1d1ef95379 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -1,25 +1,7 @@ -import logging -import os -import shutil -import tempfile -from pathlib import Path +from nf_core.components.update import ComponentUpdate -import questionary -import nf_core.modules.modules_utils -import nf_core.utils -from nf_core.components.components_command import ComponentCommand -from nf_core.components.components_utils import prompt_component_version_sha -from nf_core.utils import plural_es, plural_s, plural_y - -from .modules_differ import ModulesDiffer -from .modules_json import ModulesJson -from .modules_repo import ModulesRepo - -log = logging.getLogger(__name__) - - -class ModuleUpdate(ComponentCommand): +class ModuleUpdate(ComponentUpdate): def __init__( self, pipeline_dir, @@ -33,672 +15,16 @@ def __init__( branch=None, no_pull=False, ): - super().__init__("modules", pipeline_dir, remote_url, branch, no_pull) - self.force = force - self.prompt = prompt - self.sha = sha - self.update_all = update_all - self.show_diff = show_diff - self.save_diff_fn = save_diff_fn - self.module = None - self.update_config = None - self.modules_json = ModulesJson(self.dir) - self.branch = branch - - def _parameter_checks(self): - """Checks the compatibilty of the supplied parameters. - - Raises: - UserWarning: if any checks fail. - """ - - if self.save_diff_fn and self.show_diff: - raise UserWarning("Either `--preview` or `--save_diff` can be specified, not both.") - - if self.update_all and self.module: - raise UserWarning("Either a module or the '--all' flag can be specified, not both.") - - if self.repo_type == "modules": - raise UserWarning("Modules in clones of nf-core/modules can not be updated.") - - if self.prompt and self.sha is not None: - raise UserWarning("Cannot use '--sha' and '--prompt' at the same time.") - - if not self.has_valid_directory(): - raise UserWarning("The command was not run in a valid pipeline directory.") - - def update(self, module=None): - """Updates a specified module or all modules modules in a pipeline. - - Args: - module (str): The name of the module to update. - - Returns: - bool: True if the update was successful, False otherwise. - """ - self.module = module - - tool_config = nf_core.utils.load_tools_config(self.dir) - self.update_config = tool_config.get("update", {}) - - self._parameter_checks() - - # 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() - - if not self.update_all and module is None: - choices = ["All modules", "Named module"] - self.update_all = ( - questionary.select( - "Update all modules or a single named module?", - choices=choices, - style=nf_core.utils.nfcore_question_style, - ).unsafe_ask() - == "All modules" - ) - - # Verify that the provided SHA exists in the repo - if self.sha is not None and not self.modules_repo.sha_exists_on_branch(self.sha): - log.error(f"Commit SHA '{self.sha}' doesn't exist in '{self.modules_repo.remote_url}'") - return False - - # Get the list of modules to update, and their version information - modules_info = self.get_all_modules_info() if self.update_all else [self.get_single_module_info(module)] - - # Save the current state of the modules.json - old_modules_json = self.modules_json.get_modules_json() - - # Ask if we should show the diffs (unless a filename was already given on the command line) - if not self.save_diff_fn and self.show_diff is None: - diff_type = questionary.select( - "Do you want to view diffs of the proposed changes?", - choices=[ - {"name": "No previews, just update everything", "value": 0}, - {"name": "Preview diff in terminal, choose whether to update files", "value": 1}, - {"name": "Just write diffs to a patch file", "value": 2}, - ], - style=nf_core.utils.nfcore_question_style, - ).unsafe_ask() - - self.show_diff = diff_type == 1 - self.save_diff_fn = diff_type == 2 - - if self.save_diff_fn: # True or a string - self.setup_diff_file() - - # Loop through all modules to be updated - # and do the requested action on them - exit_value = True - all_patches_successful = True - for modules_repo, module, sha, patch_relpath in modules_info: - module_fullname = str(Path("modules", modules_repo.repo_path, module)) - # Are we updating the files in place or not? - dry_run = self.show_diff or self.save_diff_fn - - current_version = self.modules_json.get_module_version( - module, modules_repo.remote_url, modules_repo.repo_path - ) - - # Set the temporary installation folder - install_tmp_dir = Path(tempfile.mkdtemp()) - module_install_dir = install_tmp_dir / module - - # Compute the module directory - module_dir = os.path.join(self.dir, "modules", modules_repo.repo_path, module) - - if sha is not None: - version = sha - elif self.prompt: - version = prompt_component_version_sha( - module, "modules", modules_repo=modules_repo, installed_sha=current_version - ) - else: - version = modules_repo.get_latest_component_version(module, self.component_type) - - if current_version is not None and not self.force: - if current_version == version: - if self.sha or self.prompt: - log.info(f"'{module_fullname}' is already installed at {version}") - else: - log.info(f"'{module_fullname}' is already up to date") - continue - - # Download module files - if not self.install_component_files(module, version, modules_repo, install_tmp_dir): - exit_value = False - continue - - if patch_relpath is not None: - patch_successful = self.try_apply_patch( - module, modules_repo.repo_path, patch_relpath, module_dir, module_install_dir - ) - if patch_successful: - log.info(f"Module '{module_fullname}' patched successfully") - else: - log.warning(f"Failed to patch module '{module_fullname}'. Will proceed with unpatched files.") - all_patches_successful &= patch_successful - - if dry_run: - if patch_relpath is not None: - if patch_successful: - log.info("Current installation is compared against patched version in remote.") - else: - log.warning("Current installation is compared against unpatched version in remote.") - # Compute the diffs for the module - if self.save_diff_fn: - log.info(f"Writing diff file for module '{module_fullname}' to '{self.save_diff_fn}'") - ModulesDiffer.write_diff_file( - self.save_diff_fn, - module, - modules_repo.repo_path, - module_dir, - module_install_dir, - current_version, - version, - dsp_from_dir=module_dir, - dsp_to_dir=module_dir, - ) - - elif self.show_diff: - ModulesDiffer.print_diff( - module, - modules_repo.repo_path, - module_dir, - module_install_dir, - current_version, - version, - dsp_from_dir=module_dir, - dsp_to_dir=module_dir, - ) - - # Ask the user if they want to install the module - dry_run = not questionary.confirm( - f"Update module '{module}'?", default=False, style=nf_core.utils.nfcore_question_style - ).unsafe_ask() - - if not dry_run: - # Clear the module directory and move the installed files there - self.move_files_from_tmp_dir(module, install_tmp_dir, modules_repo.repo_path, version) - # Update modules.json with newly installed module - self.modules_json.update(modules_repo, module, version, self.component_type) - else: - # Don't save to a file, just iteratively update the variable - self.modules_json.update(modules_repo, module, version, self.component_type, write_file=False) - - if self.save_diff_fn: - # Write the modules.json diff to the file - ModulesDiffer.append_modules_json_diff( - self.save_diff_fn, - old_modules_json, - self.modules_json.get_modules_json(), - Path(self.dir, "modules.json"), - ) - if exit_value: - log.info( - f"[bold magenta italic] TIP! [/] If you are happy with the changes in '{self.save_diff_fn}', you " - "can apply them by running the command :point_right:" - f" [bold magenta italic]git apply {self.save_diff_fn} [/]" - ) - elif not all_patches_successful: - log.info(f"Updates complete. Please apply failed patch{plural_es(modules_info)} manually") - else: - log.info("Updates complete :sparkles:") - - return exit_value - - def get_single_module_info(self, module): - """Collects the module repository, version and sha for a module. - - Information about the module version in the '.nf-core.yml' overrides - the '--sha' option - - Args: - module_name (str): The name of the module to get info for. - - Returns: - (ModulesRepo, str, str): The modules repo containing the module, - the module name, and the module version. - - Raises: - LookupError: If the module is not found either in the pipeline or the modules repo. - UserWarning: If the '.nf-core.yml' entry is not valid. - """ - # Check if there are any modules installed from the repo - repo_url = self.modules_repo.remote_url - modules = self.modules_json.get_all_modules().get(repo_url) - choices = [module if dir == "nf-core" else f"{dir}/{module}" for dir, module in modules] - if repo_url not in self.modules_json.get_all_modules(): - raise LookupError(f"No modules installed from '{repo_url}'") - - if module is None: - module = questionary.autocomplete( - "Tool name:", - choices=choices, - style=nf_core.utils.nfcore_question_style, - ).unsafe_ask() - - # Get module installation directory - install_dir = [dir for dir, m in modules if module == m][0] - - # Check if module is installed before trying to update - if module not in choices: - raise LookupError(f"Module '{module}' is not installed in pipeline and could therefore not be updated") - - # Check that the supplied name is an available module - if module and module not in self.modules_repo.get_avail_components(self.component_type): - raise LookupError( - f"Module '{module}' not found in list of available modules." - f"Use the command 'nf-core modules list remote' to view available software" - ) - - sha = self.sha - if module in self.update_config.get(self.modules_repo.remote_url, {}).get(install_dir, {}): - # If the module to update is in .nf-core.yml config file - config_entry = self.update_config[self.modules_repo.remote_url][install_dir].get(module) - if config_entry is not None and config_entry is not True: - if config_entry is False: - raise UserWarning("Module's update entry in '.nf-core.yml' is set to False") - if not isinstance(config_entry, str): - raise UserWarning("Module's update entry in '.nf-core.yml' is of wrong type") - - sha = config_entry - if self.sha is not None: - log.warning( - f"Found entry in '.nf-core.yml' for module '{module}' " - "which will override version specified with '--sha'" - ) - else: - log.info(f"Found entry in '.nf-core.yml' for module '{module}'") - log.info(f"Updating module to ({sha})") - - # Check if the update branch is the same as the installation branch - current_branch = self.modules_json.get_component_branch( - self.component_type, module, self.modules_repo.remote_url, install_dir + super().__init__( + pipeline_dir, + "modules", + force, + prompt, + sha, + update_all, + show_diff, + save_diff_fn, + remote_url, + branch, + no_pull, ) - new_branch = self.modules_repo.branch - if current_branch != new_branch: - log.warning( - f"You are trying to update the '{Path(install_dir, module)}' module from " - f"the '{new_branch}' branch. This module was installed from the '{current_branch}'" - ) - switch = questionary.confirm(f"Do you want to update using the '{current_branch}' instead?").unsafe_ask() - if switch: - # Change the branch - self.modules_repo.setup_branch(current_branch) - - # If there is a patch file, get its filename - patch_fn = self.modules_json.get_patch_fn(module, self.modules_repo.remote_url, install_dir) - - return (self.modules_repo, module, sha, patch_fn) - - def get_all_modules_info(self, branch=None): - """Collects the module repository, version and sha for all modules. - - Information about the module version in the '.nf-core.yml' overrides the '--sha' option. - - Returns: - [(ModulesRepo, str, str)]: A list of tuples containing a ModulesRepo object, - the module name, and the module version. - """ - if branch is not None: - use_branch = questionary.confirm( - "'--branch' was specified. Should this branch be used to update all modules?", default=False - ) - if not use_branch: - branch = None - skipped_repos = [] - skipped_modules = [] - overridden_repos = [] - overridden_modules = [] - modules_info = {} - # Loop through all the modules in the pipeline - # and check if they have an entry in the '.nf-core.yml' file - for repo_name, modules in self.modules_json.get_all_modules().items(): - if repo_name not in self.update_config or self.update_config[repo_name] is True: - # There aren't restrictions for the repository in .nf-core.yml file - modules_info[repo_name] = {} - for module_dir, module in modules: - try: - modules_info[repo_name][module_dir].append( - ( - module, - self.sha, - self.modules_json.get_component_branch( - self.component_type, module, repo_name, module_dir - ), - ) - ) - except KeyError: - modules_info[repo_name][module_dir] = [ - ( - module, - self.sha, - self.modules_json.get_component_branch( - self.component_type, module, repo_name, module_dir - ), - ) - ] - elif isinstance(self.update_config[repo_name], dict): - # If it is a dict, then there are entries for individual modules or module directories - for module_dir in set([dir for dir, _ in modules]): - if isinstance(self.update_config[repo_name][module_dir], str): - # If a string is given it is the commit SHA to which we should update to - custom_sha = self.update_config[repo_name][module_dir] - modules_info[repo_name] = {} - for dir, module in modules: - if module_dir == dir: - try: - modules_info[repo_name][module_dir].append( - ( - module, - custom_sha, - self.modules_json.get_component_branch( - self.component_type, module, repo_name, module_dir - ), - ) - ) - except KeyError: - modules_info[repo_name][module_dir] = [ - ( - module, - custom_sha, - self.modules_json.get_component_branch( - self.component_type, module, repo_name, module_dir - ), - ) - ] - if self.sha is not None: - overridden_repos.append(repo_name) - elif self.update_config[repo_name][module_dir] is False: - for dir, module in modules: - if dir == module_dir: - skipped_modules.append(f"{module_dir}/{module}") - elif isinstance(self.update_config[repo_name][module_dir], dict): - # If it's a dict, there are entries for individual modules - dir_config = self.update_config[repo_name][module_dir] - modules_info[repo_name] = {} - for module_dir, module in modules: - if module not in dir_config or dir_config[module] is True: - try: - modules_info[repo_name][module_dir].append( - ( - module, - self.sha, - self.modules_json.get_component_branch( - self.component_type, module, repo_name, module_dir - ), - ) - ) - except KeyError: - modules_info[repo_name][module_dir] = [ - ( - module, - self.sha, - self.modules_json.get_component_branch( - self.component_type, module, repo_name, module_dir - ), - ) - ] - elif isinstance(dir_config[module], str): - # If a string is given it is the commit SHA to which we should update to - custom_sha = dir_config[module] - try: - modules_info[repo_name][module_dir].append( - ( - module, - custom_sha, - self.modules_json.get_component_branch( - self.component_type, module, repo_name, module_dir - ), - ) - ) - except KeyError: - modules_info[repo_name][module_dir] = [ - ( - module, - custom_sha, - self.modules_json.get_component_branch( - self.component_type, module, repo_name, module_dir - ), - ) - ] - if self.sha is not None: - overridden_modules.append(module) - elif dir_config[module] is False: - # Otherwise the entry must be 'False' and we should ignore the module - skipped_modules.append(f"{module_dir}/{module}") - else: - raise UserWarning( - f"Module '{module}' in '{module_dir}' has an invalid entry in '.nf-core.yml'" - ) - elif isinstance(self.update_config[repo_name], str): - # If a string is given it is the commit SHA to which we should update to - custom_sha = self.update_config[repo_name] - modules_info[repo_name] = {} - for module_dir, module in modules: - try: - modules_info[repo_name][module_dir].append( - ( - module, - custom_sha, - self.modules_json.get_component_branch( - self.component_type, module, repo_name, module_dir - ), - ) - ) - except KeyError: - modules_info[repo_name][module_dir] = [ - ( - module, - custom_sha, - self.modules_json.get_component_branch( - self.component_type, module, repo_name, module_dir - ), - ) - ] - if self.sha is not None: - overridden_repos.append(repo_name) - elif self.update_config[repo_name] is False: - skipped_repos.append(repo_name) - else: - raise UserWarning(f"Repo '{repo_name}' has an invalid entry in '.nf-core.yml'") - - if skipped_repos: - skipped_str = "', '".join(skipped_repos) - log.info(f"Skipping modules in repositor{plural_y(skipped_repos)}: '{skipped_str}'") - - if skipped_modules: - skipped_str = "', '".join(skipped_modules) - log.info(f"Skipping module{plural_s(skipped_modules)}: '{skipped_str}'") - - if overridden_repos: - overridden_str = "', '".join(overridden_repos) - log.info( - f"Overriding '--sha' flag for modules in repositor{plural_y(overridden_repos)} " - f"with '.nf-core.yml' entry: '{overridden_str}'" - ) - - if overridden_modules: - overridden_str = "', '".join(overridden_modules) - log.info( - f"Overriding '--sha' flag for module{plural_s(overridden_modules)} with " - f"'.nf-core.yml' entry: '{overridden_str}'" - ) - # Loop through modules_info and create on ModulesRepo object per remote and branch - repos_and_branches = {} - for repo_name, repo_content in modules_info.items(): - for module_dir, mods in repo_content.items(): - for mod, sha, mod_branch in mods: - if branch is not None: - mod_branch = branch - if (repo_name, mod_branch) not in repos_and_branches: - repos_and_branches[(repo_name, mod_branch)] = [] - repos_and_branches[(repo_name, mod_branch)].append((mod, sha)) - - # Create ModulesRepo objects - repo_objs_mods = [] - for (repo_url, branch), mods_shas in repos_and_branches.items(): - try: - modules_repo = ModulesRepo(remote_url=repo_url, branch=branch) - except LookupError as e: - log.warning(e) - log.info(f"Skipping modules in '{repo_url}'") - else: - repo_objs_mods.append((modules_repo, mods_shas)) - - # Flatten the list - modules_info = [(repo, mod, sha) for repo, mods_shas in repo_objs_mods for mod, sha in mods_shas] - - # Verify that that all modules and shas exist in their respective ModulesRepo, - # don't try to update those that don't - i = 0 - while i < len(modules_info): - repo, module, sha = modules_info[i] - if not repo.component_exists(module, self.component_type): - log.warning(f"Module '{module}' does not exist in '{repo.remote_url}'. Skipping...") - modules_info.pop(i) - elif sha is not None and not repo.sha_exists_on_branch(sha): - log.warning( - f"Git sha '{sha}' does not exists on the '{repo.branch}' of '{repo.remote_url}'. Skipping module '{module}'" - ) - modules_info.pop(i) - else: - i += 1 - - # Add patch filenames to the modules that have them - modules_info = [ - (repo, mod, sha, self.modules_json.get_patch_fn(mod, repo.remote_url, repo.repo_path)) - for repo, mod, sha in modules_info - ] - - return modules_info - - def setup_diff_file(self): - """Sets up the diff file. - - If the save diff option was chosen interactively, the user is asked to supply a name for the diff file. - - Then creates the file for saving the diff. - """ - if self.save_diff_fn is True: - # From questionary - no filename yet - self.save_diff_fn = questionary.path( - "Enter the filename: ", style=nf_core.utils.nfcore_question_style - ).unsafe_ask() - - self.save_diff_fn = Path(self.save_diff_fn) - - # Check if filename already exists (questionary or cli) - while self.save_diff_fn.exists(): - if questionary.confirm(f"'{self.save_diff_fn}' exists. Remove file?").unsafe_ask(): - os.remove(self.save_diff_fn) - break - self.save_diff_fn = questionary.path( - "Enter a new filename: ", - style=nf_core.utils.nfcore_question_style, - ).unsafe_ask() - self.save_diff_fn = Path(self.save_diff_fn) - - # This guarantees that the file exists after calling the function - self.save_diff_fn.touch() - - def move_files_from_tmp_dir(self, module, install_folder, repo_path, new_version): - """Move the files from the temporary to the installation directory. - - Args: - module (str): The module name. - install_folder [str]: The path to the temporary installation directory. - repo_path (str): The name of the directory where modules are installed - new_version (str): The version of the module that was installed. - """ - temp_module_dir = os.path.join(install_folder, module) - files = os.listdir(temp_module_dir) - pipeline_path = os.path.join(self.dir, "modules", repo_path, module) - - log.debug(f"Removing old version of module '{module}'") - self.clear_component_dir(module, pipeline_path) - - os.makedirs(pipeline_path) - for file in files: - path = os.path.join(temp_module_dir, file) - if os.path.exists(path): - shutil.move(path, os.path.join(pipeline_path, file)) - - log.info(f"Updating '{repo_path}/{module}'") - log.debug(f"Updating module '{module}' to {new_version} from {repo_path}") - - def try_apply_patch(self, module, repo_path, patch_relpath, module_dir, module_install_dir): - """ - Try applying a patch file to the new module files - - - Args: - module (str): The name of the module - repo_path (str): The name of the repository where the module resides - patch_relpath (Path | str): The path to patch file in the pipeline - module_dir (Path | str): The module directory in the pipeline - module_install_dir (Path | str): The directory where the new module - file have been installed - - Returns: - (bool): Whether the patch application was successful - """ - module_fullname = str(Path(repo_path, module)) - log.info(f"Found patch for module '{module_fullname}'. Trying to apply it to new files") - - patch_path = Path(self.dir / patch_relpath) - module_relpath = Path("modules", repo_path, module) - - # Check that paths in patch file are updated - self.check_patch_paths(patch_path, module) - - # Copy the installed files to a new temporary directory to save them for later use - temp_dir = Path(tempfile.mkdtemp()) - temp_module_dir = temp_dir / module - shutil.copytree(module_install_dir, temp_module_dir) - - try: - new_files = ModulesDiffer.try_apply_patch(module, repo_path, patch_path, temp_module_dir) - except LookupError: - # Patch failed. Save the patch file by moving to the install dir - shutil.move(patch_path, Path(module_install_dir, patch_path.relative_to(module_dir))) - log.warning( - f"Failed to apply patch for module '{module_fullname}'. You will have to apply the patch manually" - ) - return False - - # Write the patched files to a temporary directory - log.debug("Writing patched files") - for file, new_content in new_files.items(): - fn = temp_module_dir / file - with open(fn, "w") as fh: - fh.writelines(new_content) - - # Create the new patch file - log.debug("Regenerating patch file") - ModulesDiffer.write_diff_file( - Path(temp_module_dir, patch_path.relative_to(module_dir)), - module, - repo_path, - module_install_dir, - temp_module_dir, - file_action="w", - for_git=False, - dsp_from_dir=module_relpath, - dsp_to_dir=module_relpath, - ) - - # Move the patched files to the install dir - log.debug("Overwriting installed files installed files with patched files") - shutil.rmtree(module_install_dir) - shutil.copytree(temp_module_dir, module_install_dir) - - # Add the patch file to the modules.json file - self.modules_json.add_patch_entry( - module, self.modules_repo.remote_url, repo_path, patch_relpath, write_file=True - ) - - return True diff --git a/nf_core/subworkflows/update.py b/nf_core/subworkflows/update.py new file mode 100644 index 0000000000..5272304c60 --- /dev/null +++ b/nf_core/subworkflows/update.py @@ -0,0 +1,30 @@ +from nf_core.components.update import ComponentUpdate + + +class SubworkflowUpdate(ComponentUpdate): + def __init__( + self, + pipeline_dir, + force=False, + prompt=False, + sha=None, + update_all=False, + show_diff=None, + save_diff_fn=None, + remote_url=None, + branch=None, + no_pull=False, + ): + super().__init__( + pipeline_dir, + "subworkflows", + force, + prompt, + sha, + update_all, + show_diff, + save_diff_fn, + remote_url, + branch, + no_pull, + ) From bde678c5adeef1d32553f667756992c61dbff8ea Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Thu, 10 Nov 2022 17:09:53 +0100 Subject: [PATCH 02/31] refactor components update --- nf_core/components/update.py | 404 ++++++++++++++++++----------------- 1 file changed, 212 insertions(+), 192 deletions(-) diff --git a/nf_core/components/update.py b/nf_core/components/update.py index 7ba6cff81e..a9db42e509 100644 --- a/nf_core/components/update.py +++ b/nf_core/components/update.py @@ -40,7 +40,7 @@ def __init__( self.update_all = update_all self.show_diff = show_diff self.save_diff_fn = save_diff_fn - self.module = None + self.component = None self.update_config = None self.modules_json = ModulesJson(self.dir) self.branch = branch @@ -55,11 +55,11 @@ def _parameter_checks(self): if self.save_diff_fn and self.show_diff: raise UserWarning("Either `--preview` or `--save_diff` can be specified, not both.") - if self.update_all and self.module: - raise UserWarning("Either a module or the '--all' flag can be specified, not both.") + if self.update_all and self.component: + raise UserWarning(f"Either a {self.component_type[:-1]} or the '--all' flag can be specified, not both.") if self.repo_type == "modules": - raise UserWarning("Modules in clones of nf-core/modules can not be updated.") + raise UserWarning(f"{self.component_type.title()} in clones of nf-core/modules can not be updated.") if self.prompt and self.sha is not None: raise UserWarning("Cannot use '--sha' and '--prompt' at the same time.") @@ -67,16 +67,19 @@ def _parameter_checks(self): if not self.has_valid_directory(): raise UserWarning("The command was not run in a valid pipeline directory.") - def update(self, module=None): - """Updates a specified module or all modules modules in a pipeline. + def update(self, component=None): + """Updates a specified module/subworkflow or all modules/subworkflows in a pipeline. + + If updating a subworkflow: updates all modules used in that subworkflow. + If updating a module: updates all subworkflows that use the module. Args: - module (str): The name of the module to update. + component (str): The name of the module/subworkflow to update. Returns: bool: True if the update was successful, False otherwise. """ - self.module = module + self.component = component tool_config = nf_core.utils.load_tools_config(self.dir) self.update_config = tool_config.get("update", {}) @@ -89,15 +92,15 @@ def update(self, module=None): # Verify that 'modules.json' is consistent with the installed modules self.modules_json.check_up_to_date() - if not self.update_all and module is None: - choices = ["All modules", "Named module"] + if not self.update_all and component is None: + choices = [f"All {self.component_type}", f"Named {self.component_type[:-1]}"] self.update_all = ( questionary.select( - "Update all modules or a single named module?", + f"Update all {self.component_type} or a single named {self.component_type[:-1]}?", choices=choices, style=nf_core.utils.nfcore_question_style, ).unsafe_ask() - == "All modules" + == f"All {self.component_type}" ) # Verify that the provided SHA exists in the repo @@ -105,8 +108,10 @@ def update(self, module=None): log.error(f"Commit SHA '{self.sha}' doesn't exist in '{self.modules_repo.remote_url}'") return False - # Get the list of modules to update, and their version information - modules_info = self.get_all_modules_info() if self.update_all else [self.get_single_module_info(module)] + # Get the list of modules/subworkflows to update, and their version information + components_info = ( + self.get_all_components_info() if self.update_all else [self.get_single_component_info(component)] + ) # Save the current state of the modules.json old_modules_json = self.modules_json.get_modules_json() @@ -129,56 +134,58 @@ def update(self, module=None): if self.save_diff_fn: # True or a string self.setup_diff_file() - # Loop through all modules to be updated + # Loop through all components to be updated # and do the requested action on them exit_value = True all_patches_successful = True - for modules_repo, module, sha, patch_relpath in modules_info: - module_fullname = str(Path("modules", modules_repo.repo_path, module)) + for modules_repo, component, sha, patch_relpath in components_info: + component_fullname = str(Path(self.component_type, modules_repo.repo_path, component)) # Are we updating the files in place or not? dry_run = self.show_diff or self.save_diff_fn - current_version = self.modules_json.get_module_version( - module, modules_repo.remote_url, modules_repo.repo_path + current_version = self.modules_json.get_component_version( + self.component_type, component, modules_repo.remote_url, modules_repo.repo_path ) # Set the temporary installation folder install_tmp_dir = Path(tempfile.mkdtemp()) - module_install_dir = install_tmp_dir / module + component_install_dir = install_tmp_dir / component - # Compute the module directory - module_dir = os.path.join(self.dir, "modules", modules_repo.repo_path, module) + # Compute the component directory + component_dir = os.path.join(self.dir, self.component_type, modules_repo.repo_path, component) if sha is not None: version = sha elif self.prompt: version = prompt_component_version_sha( - module, "modules", modules_repo=modules_repo, installed_sha=current_version + component, self.component_type, modules_repo=modules_repo, installed_sha=current_version ) else: - version = modules_repo.get_latest_component_version(module, self.component_type) + version = modules_repo.get_latest_component_version(component, self.component_type) if current_version is not None and not self.force: if current_version == version: if self.sha or self.prompt: - log.info(f"'{module_fullname}' is already installed at {version}") + log.info(f"'{component_fullname}' is already installed at {version}") else: - log.info(f"'{module_fullname}' is already up to date") + log.info(f"'{component_fullname}' is already up to date") continue - # Download module files - if not self.install_component_files(module, version, modules_repo, install_tmp_dir): + # Download component files + if not self.install_component_files(component, version, modules_repo, install_tmp_dir): exit_value = False continue if patch_relpath is not None: patch_successful = self.try_apply_patch( - module, modules_repo.repo_path, patch_relpath, module_dir, module_install_dir + component, modules_repo.repo_path, patch_relpath, component_dir, component_install_dir ) if patch_successful: - log.info(f"Module '{module_fullname}' patched successfully") + log.info(f"{self.component_type[:-1].title()} '{component_fullname}' patched successfully") else: - log.warning(f"Failed to patch module '{module_fullname}'. Will proceed with unpatched files.") + log.warning( + f"Failed to patch {self.component_type[:-1]} '{component_fullname}'. Will proceed with unpatched files." + ) all_patches_successful &= patch_successful if dry_run: @@ -187,46 +194,50 @@ def update(self, module=None): log.info("Current installation is compared against patched version in remote.") else: log.warning("Current installation is compared against unpatched version in remote.") - # Compute the diffs for the module + # Compute the diffs for the component if self.save_diff_fn: - log.info(f"Writing diff file for module '{module_fullname}' to '{self.save_diff_fn}'") + log.info( + f"Writing diff file for {self.component_type[:-1]} '{component_fullname}' to '{self.save_diff_fn}'" + ) ModulesDiffer.write_diff_file( self.save_diff_fn, - module, + component, modules_repo.repo_path, - module_dir, - module_install_dir, + component_dir, + component_install_dir, current_version, version, - dsp_from_dir=module_dir, - dsp_to_dir=module_dir, + dsp_from_dir=component_dir, + dsp_to_dir=component_dir, ) elif self.show_diff: ModulesDiffer.print_diff( - module, - modules_repo.repo_path, - module_dir, - module_install_dir, + component, + components_repo.repo_path, + component_dir, + component_install_dir, current_version, version, - dsp_from_dir=module_dir, - dsp_to_dir=module_dir, + dsp_from_dir=component_dir, + dsp_to_dir=component_dir, ) - # Ask the user if they want to install the module + # Ask the user if they want to install the component dry_run = not questionary.confirm( - f"Update module '{module}'?", default=False, style=nf_core.utils.nfcore_question_style + f"Update {self.component_type[:-1]} '{component}'?", + default=False, + style=nf_core.utils.nfcore_question_style, ).unsafe_ask() if not dry_run: - # Clear the module directory and move the installed files there - self.move_files_from_tmp_dir(module, install_tmp_dir, modules_repo.repo_path, version) - # Update modules.json with newly installed module - self.modules_json.update(modules_repo, module, version, self.component_type) + # Clear the component directory and move the installed files there + self.move_files_from_tmp_dir(component, install_tmp_dir, modules_repo.repo_path, version) + # Update modules.json with newly installed component + self.modules_json.update(modules_repo, component, version, self.component_type) else: # Don't save to a file, just iteratively update the variable - self.modules_json.update(modules_repo, module, version, self.component_type, write_file=False) + self.modules_json.update(modules_repo, component, version, self.component_type, write_file=False) if self.save_diff_fn: # Write the modules.json diff to the file @@ -243,86 +254,92 @@ def update(self, module=None): f" [bold magenta italic]git apply {self.save_diff_fn} [/]" ) elif not all_patches_successful: - log.info(f"Updates complete. Please apply failed patch{plural_es(modules_info)} manually") + log.info(f"Updates complete. Please apply failed patch{plural_es(components_info)} manually") else: log.info("Updates complete :sparkles:") return exit_value - def get_single_module_info(self, module): - """Collects the module repository, version and sha for a module. + def get_single_component_info(self, component): + """Collects the modules repository, version and sha for a component. - Information about the module version in the '.nf-core.yml' overrides + Information about the component version in the '.nf-core.yml' overrides the '--sha' option Args: - module_name (str): The name of the module to get info for. + component (str): The name of the module/subworkflow to get info for. Returns: - (ModulesRepo, str, str): The modules repo containing the module, - the module name, and the module version. + (ModulesRepo, str, str): The modules repo containing the component, + the component name, and the component version. Raises: - LookupError: If the module is not found either in the pipeline or the modules repo. + LookupError: If the component is not found either in the pipeline or the modules repo. UserWarning: If the '.nf-core.yml' entry is not valid. """ - # Check if there are any modules installed from the repo + # Check if there are any modules/subworkflows installed from the repo repo_url = self.modules_repo.remote_url - modules = self.modules_json.get_all_modules().get(repo_url) - choices = [module if dir == "nf-core" else f"{dir}/{module}" for dir, module in modules] - if repo_url not in self.modules_json.get_all_modules(): - raise LookupError(f"No modules installed from '{repo_url}'") - - if module is None: - module = questionary.autocomplete( - "Tool name:", + components = self.modules_json.get_all_components(self.component_type).get(repo_url) + choices = [component if dir == "nf-core" else f"{dir}/{component}" for dir, component in components] + if repo_url not in self.modules_json.get_all_components(self.component_type): + raise LookupError(f"No {self.component_type} installed from '{repo_url}'") + + if component is None: + component = questionary.autocomplete( + f"{self.component_type[:-1].title()} name:", choices=choices, style=nf_core.utils.nfcore_question_style, ).unsafe_ask() - # Get module installation directory - install_dir = [dir for dir, m in modules if module == m][0] + # Get component installation directory + install_dir = [dir for dir, m in components if component == m][0] - # Check if module is installed before trying to update - if module not in choices: - raise LookupError(f"Module '{module}' is not installed in pipeline and could therefore not be updated") + # Check if component is installed before trying to update + if component not in choices: + raise LookupError( + f"{self.component_type[:-1].title()} '{component}' is not installed in pipeline and could therefore not be updated" + ) - # Check that the supplied name is an available module - if module and module not in self.modules_repo.get_avail_components(self.component_type): + # Check that the supplied name is an available module/subworkflow + if component and component not in self.modules_repo.get_avail_components(self.component_type): raise LookupError( - f"Module '{module}' not found in list of available modules." - f"Use the command 'nf-core modules list remote' to view available software" + f"{self.component_type[:-1].title()} '{component}' not found in list of available {self.component_type}." + f"Use the command 'nf-core {self.component_type} list remote' to view available software" ) sha = self.sha - if module in self.update_config.get(self.modules_repo.remote_url, {}).get(install_dir, {}): - # If the module to update is in .nf-core.yml config file - config_entry = self.update_config[self.modules_repo.remote_url][install_dir].get(module) + if component in self.update_config.get(self.modules_repo.remote_url, {}).get(install_dir, {}): + # If the component to update is in .nf-core.yml config file + config_entry = self.update_config[self.modules_repo.remote_url][install_dir].get(component) if config_entry is not None and config_entry is not True: if config_entry is False: - raise UserWarning("Module's update entry in '.nf-core.yml' is set to False") + raise UserWarning( + f"{self.component_type[:-1].title()}'s update entry in '.nf-core.yml' is set to False" + ) if not isinstance(config_entry, str): - raise UserWarning("Module's update entry in '.nf-core.yml' is of wrong type") + raise UserWarning( + f"{self.component_type[:-1].title()}'s update entry in '.nf-core.yml' is of wrong type" + ) sha = config_entry if self.sha is not None: log.warning( - f"Found entry in '.nf-core.yml' for module '{module}' " + f"Found entry in '.nf-core.yml' for {self.component_type[:-1]} '{component}' " "which will override version specified with '--sha'" ) else: - log.info(f"Found entry in '.nf-core.yml' for module '{module}'") - log.info(f"Updating module to ({sha})") + log.info(f"Found entry in '.nf-core.yml' for {self.component_type[:-1]} '{component}'") + log.info(f"Updating component to ({sha})") # Check if the update branch is the same as the installation branch current_branch = self.modules_json.get_component_branch( - self.component_type, module, self.modules_repo.remote_url, install_dir + self.component_type, component, self.modules_repo.remote_url, install_dir ) new_branch = self.modules_repo.branch if current_branch != new_branch: log.warning( - f"You are trying to update the '{Path(install_dir, module)}' module from " - f"the '{new_branch}' branch. This module was installed from the '{current_branch}'" + f"You are trying to update the '{Path(install_dir, component)}' {self.component_type[:-1]} from " + f"the '{new_branch}' branch. This {self.component_type[:-1]} was installed from the '{current_branch}'" ) switch = questionary.confirm(f"Do you want to update using the '{current_branch}' instead?").unsafe_ask() if switch: @@ -330,172 +347,173 @@ def get_single_module_info(self, module): self.modules_repo.setup_branch(current_branch) # If there is a patch file, get its filename - patch_fn = self.modules_json.get_patch_fn(module, self.modules_repo.remote_url, install_dir) + patch_fn = self.modules_json.get_patch_fn(component, self.modules_repo.remote_url, install_dir) - return (self.modules_repo, module, sha, patch_fn) + return (self.modules_repo, component, sha, patch_fn) - def get_all_modules_info(self, branch=None): - """Collects the module repository, version and sha for all modules. + def get_all_components_info(self, branch=None): + """Collects the modules repository, version and sha for all modules/subworkflows. - Information about the module version in the '.nf-core.yml' overrides the '--sha' option. + Information about the module/subworkflow version in the '.nf-core.yml' overrides the '--sha' option. Returns: [(ModulesRepo, str, str)]: A list of tuples containing a ModulesRepo object, - the module name, and the module version. + the component name, and the component version. """ if branch is not None: use_branch = questionary.confirm( - "'--branch' was specified. Should this branch be used to update all modules?", default=False + f"'--branch' was specified. Should this branch be used to update all {self.component_type}?", + default=False, ) if not use_branch: branch = None skipped_repos = [] - skipped_modules = [] + skipped_components = [] overridden_repos = [] - overridden_modules = [] - modules_info = {} - # Loop through all the modules in the pipeline + overridden_components = [] + components_info = {} + # Loop through all the modules/subworkflows in the pipeline # and check if they have an entry in the '.nf-core.yml' file - for repo_name, modules in self.modules_json.get_all_modules().items(): + for repo_name, components in self.modules_json.get_all_components(self.component_type).items(): if repo_name not in self.update_config or self.update_config[repo_name] is True: # There aren't restrictions for the repository in .nf-core.yml file - modules_info[repo_name] = {} - for module_dir, module in modules: + components_info[repo_name] = {} + for component_dir, component in components: try: - modules_info[repo_name][module_dir].append( + components_info[repo_name][component_dir].append( ( - module, + component, self.sha, self.modules_json.get_component_branch( - self.component_type, module, repo_name, module_dir + self.component_type, component, repo_name, component_dir ), ) ) except KeyError: - modules_info[repo_name][module_dir] = [ + components_info[repo_name][component_dir] = [ ( - module, + component, self.sha, self.modules_json.get_component_branch( - self.component_type, module, repo_name, module_dir + self.component_type, component, repo_name, component_dir ), ) ] elif isinstance(self.update_config[repo_name], dict): - # If it is a dict, then there are entries for individual modules or module directories - for module_dir in set([dir for dir, _ in modules]): - if isinstance(self.update_config[repo_name][module_dir], str): + # If it is a dict, then there are entries for individual components or component directories + for component_dir in set([dir for dir, _ in components]): + if isinstance(self.update_config[repo_name][component_dir], str): # If a string is given it is the commit SHA to which we should update to - custom_sha = self.update_config[repo_name][module_dir] - modules_info[repo_name] = {} - for dir, module in modules: - if module_dir == dir: + custom_sha = self.update_config[repo_name][component_dir] + components_info[repo_name] = {} + for dir, component in components: + if component_dir == dir: try: - modules_info[repo_name][module_dir].append( + components_info[repo_name][component_dir].append( ( - module, + component, custom_sha, self.modules_json.get_component_branch( - self.component_type, module, repo_name, module_dir + self.component_type, component, repo_name, component_dir ), ) ) except KeyError: - modules_info[repo_name][module_dir] = [ + components_info[repo_name][component_dir] = [ ( - module, + component, custom_sha, self.modules_json.get_component_branch( - self.component_type, module, repo_name, module_dir + self.component_type, component, repo_name, component_dir ), ) ] if self.sha is not None: overridden_repos.append(repo_name) - elif self.update_config[repo_name][module_dir] is False: - for dir, module in modules: - if dir == module_dir: - skipped_modules.append(f"{module_dir}/{module}") - elif isinstance(self.update_config[repo_name][module_dir], dict): - # If it's a dict, there are entries for individual modules - dir_config = self.update_config[repo_name][module_dir] - modules_info[repo_name] = {} - for module_dir, module in modules: - if module not in dir_config or dir_config[module] is True: + elif self.update_config[repo_name][component_dir] is False: + for dir, component in components: + if dir == component_dir: + skipped_components.append(f"{component_dir}/{components}") + elif isinstance(self.update_config[repo_name][component_dir], dict): + # If it's a dict, there are entries for individual components + dir_config = self.update_config[repo_name][component_dir] + components_info[repo_name] = {} + for component_dir, component in components: + if component not in dir_config or dir_config[component] is True: try: - modules_info[repo_name][module_dir].append( + components_info[repo_name][component_dir].append( ( - module, + component, self.sha, self.modules_json.get_component_branch( - self.component_type, module, repo_name, module_dir + self.component_type, component, repo_name, component_dir ), ) ) except KeyError: - modules_info[repo_name][module_dir] = [ + components_info[repo_name][component_dir] = [ ( - module, + component, self.sha, self.modules_json.get_component_branch( - self.component_type, module, repo_name, module_dir + self.component_type, component, repo_name, component_dir ), ) ] - elif isinstance(dir_config[module], str): + elif isinstance(dir_config[component], str): # If a string is given it is the commit SHA to which we should update to - custom_sha = dir_config[module] + custom_sha = dir_config[component] try: - modules_info[repo_name][module_dir].append( + components_info[repo_name][component_dir].append( ( - module, + component, custom_sha, self.modules_json.get_component_branch( - self.component_type, module, repo_name, module_dir + self.component_type, component, repo_name, component_dir ), ) ) except KeyError: - modules_info[repo_name][module_dir] = [ + components_info[repo_name][component_dir] = [ ( - module, + component, custom_sha, self.modules_json.get_component_branch( - self.component_type, module, repo_name, module_dir + self.component_type, component, repo_name, component_dir ), ) ] if self.sha is not None: - overridden_modules.append(module) - elif dir_config[module] is False: - # Otherwise the entry must be 'False' and we should ignore the module - skipped_modules.append(f"{module_dir}/{module}") + overridden_components.append(component) + elif dir_config[component] is False: + # Otherwise the entry must be 'False' and we should ignore the component + skipped_components.append(f"{component_dir}/{component}") else: raise UserWarning( - f"Module '{module}' in '{module_dir}' has an invalid entry in '.nf-core.yml'" + f"{self.component_type[:-1].title()} '{component}' in '{component_dir}' has an invalid entry in '.nf-core.yml'" ) elif isinstance(self.update_config[repo_name], str): # If a string is given it is the commit SHA to which we should update to custom_sha = self.update_config[repo_name] - modules_info[repo_name] = {} - for module_dir, module in modules: + components_info[repo_name] = {} + for component_dir, component in components: try: - modules_info[repo_name][module_dir].append( + components_info[repo_name][component_dir].append( ( - module, + component, custom_sha, self.modules_json.get_component_branch( - self.component_type, module, repo_name, module_dir + self.component_type, component, repo_name, component_dir ), ) ) except KeyError: - modules_info[repo_name][module_dir] = [ + components_info[repo_name][component_dir] = [ ( - module, + component, custom_sha, self.modules_json.get_component_branch( - self.component_type, module, repo_name, module_dir + self.component_type, component, repo_name, component_dir ), ) ] @@ -508,73 +526,75 @@ def get_all_modules_info(self, branch=None): if skipped_repos: skipped_str = "', '".join(skipped_repos) - log.info(f"Skipping modules in repositor{plural_y(skipped_repos)}: '{skipped_str}'") + log.info(f"Skipping {self.component_type} in repositor{plural_y(skipped_repos)}: '{skipped_str}'") - if skipped_modules: - skipped_str = "', '".join(skipped_modules) - log.info(f"Skipping module{plural_s(skipped_modules)}: '{skipped_str}'") + if skipped_components: + skipped_str = "', '".join(skipped_components) + log.info(f"Skipping {self.component_type[:-1]}{plural_s(skipped_components)}: '{skipped_str}'") if overridden_repos: overridden_str = "', '".join(overridden_repos) log.info( - f"Overriding '--sha' flag for modules in repositor{plural_y(overridden_repos)} " + f"Overriding '--sha' flag for {self.component_type} in repositor{plural_y(overridden_repos)} " f"with '.nf-core.yml' entry: '{overridden_str}'" ) - if overridden_modules: - overridden_str = "', '".join(overridden_modules) + if overridden_components: + overridden_str = "', '".join(overridden_components) log.info( - f"Overriding '--sha' flag for module{plural_s(overridden_modules)} with " + f"Overriding '--sha' flag for {self.component_type[:-1]}{plural_s(overridden_components)} with " f"'.nf-core.yml' entry: '{overridden_str}'" ) - # Loop through modules_info and create on ModulesRepo object per remote and branch + # Loop through components_info and create on ModulesRepo object per remote and branch repos_and_branches = {} - for repo_name, repo_content in modules_info.items(): - for module_dir, mods in repo_content.items(): - for mod, sha, mod_branch in mods: + for repo_name, repo_content in components_info.items(): + for component_dir, comps in repo_content.items(): + for comp, sha, comp_branch in comps: if branch is not None: - mod_branch = branch - if (repo_name, mod_branch) not in repos_and_branches: - repos_and_branches[(repo_name, mod_branch)] = [] - repos_and_branches[(repo_name, mod_branch)].append((mod, sha)) + comp_branch = branch + if (repo_name, comp_branch) not in repos_and_branches: + repos_and_branches[(repo_name, comp_branch)] = [] + repos_and_branches[(repo_name, comp_branch)].append((comp, sha)) # Create ModulesRepo objects - repo_objs_mods = [] - for (repo_url, branch), mods_shas in repos_and_branches.items(): + repo_objs_comps = [] + for (repo_url, branch), comps_shas in repos_and_branches.items(): try: modules_repo = ModulesRepo(remote_url=repo_url, branch=branch) except LookupError as e: log.warning(e) - log.info(f"Skipping modules in '{repo_url}'") + log.info(f"Skipping {self.component_type} in '{repo_url}'") else: - repo_objs_mods.append((modules_repo, mods_shas)) + repo_objs_comps.append((modules_repo, comps_shas)) # Flatten the list - modules_info = [(repo, mod, sha) for repo, mods_shas in repo_objs_mods for mod, sha in mods_shas] + components_info = [(repo, comp, sha) for repo, comps_shas in repo_objs_comps for comp, sha in comps_shas] - # Verify that that all modules and shas exist in their respective ModulesRepo, + # Verify that that all components and shas exist in their respective ModulesRepo, # don't try to update those that don't i = 0 - while i < len(modules_info): - repo, module, sha = modules_info[i] - if not repo.component_exists(module, self.component_type): - log.warning(f"Module '{module}' does not exist in '{repo.remote_url}'. Skipping...") - modules_info.pop(i) + while i < len(components_info): + repo, component, sha = components_info[i] + if not repo.component_exists(component, self.component_type): + log.warning( + f"{self.component_type[:-1].title()} '{component}' does not exist in '{repo.remote_url}'. Skipping..." + ) + components_info.pop(i) elif sha is not None and not repo.sha_exists_on_branch(sha): log.warning( - f"Git sha '{sha}' does not exists on the '{repo.branch}' of '{repo.remote_url}'. Skipping module '{module}'" + f"Git sha '{sha}' does not exists on the '{repo.branch}' of '{repo.remote_url}'. Skipping {self.component_type[:-1]} '{component}'" ) - modules_info.pop(i) + components_info.pop(i) else: i += 1 - # Add patch filenames to the modules that have them - modules_info = [ - (repo, mod, sha, self.modules_json.get_patch_fn(mod, repo.remote_url, repo.repo_path)) - for repo, mod, sha in modules_info + # Add patch filenames to the components that have them + components_info = [ + (repo, comp, sha, self.modules_json.get_patch_fn(comp, repo.remote_url, repo.repo_path)) + for repo, comp, sha in components_info ] - return modules_info + return components_info def setup_diff_file(self): """Sets up the diff file. From c8f24948e9862bda64afde94753e2db4a60d5633 Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Fri, 11 Nov 2022 09:35:13 +0100 Subject: [PATCH 03/31] more refactoring of update command --- nf_core/components/update.py | 76 ++++++++++++++++++------------------ 1 file changed, 38 insertions(+), 38 deletions(-) diff --git a/nf_core/components/update.py b/nf_core/components/update.py index a9db42e509..17a583ac58 100644 --- a/nf_core/components/update.py +++ b/nf_core/components/update.py @@ -625,100 +625,100 @@ def setup_diff_file(self): # This guarantees that the file exists after calling the function self.save_diff_fn.touch() - def move_files_from_tmp_dir(self, module, install_folder, repo_path, new_version): + def move_files_from_tmp_dir(self, component, install_folder, repo_path, new_version): """Move the files from the temporary to the installation directory. Args: - module (str): The module name. + component (str): The module/subworkflow name. install_folder [str]: The path to the temporary installation directory. - repo_path (str): The name of the directory where modules are installed - new_version (str): The version of the module that was installed. + repo_path (str): The name of the directory where modules/subworkflows are installed + new_version (str): The version of the module/subworkflow that was installed. """ - temp_module_dir = os.path.join(install_folder, module) - files = os.listdir(temp_module_dir) - pipeline_path = os.path.join(self.dir, "modules", repo_path, module) + temp_component_dir = os.path.join(install_folder, component) + files = os.listdir(temp_component_dir) + pipeline_path = os.path.join(self.dir, self.component_type, repo_path, component) - log.debug(f"Removing old version of module '{module}'") - self.clear_component_dir(module, pipeline_path) + log.debug(f"Removing old version of {self.component_type[:-1]} '{component}'") + self.clear_component_dir(component, pipeline_path) os.makedirs(pipeline_path) for file in files: - path = os.path.join(temp_module_dir, file) + path = os.path.join(temp_component_dir, file) if os.path.exists(path): shutil.move(path, os.path.join(pipeline_path, file)) - log.info(f"Updating '{repo_path}/{module}'") - log.debug(f"Updating module '{module}' to {new_version} from {repo_path}") + log.info(f"Updating '{repo_path}/{component}'") + log.debug(f"Updating {self.component_type[:-1]} '{component}' to {new_version} from {repo_path}") - def try_apply_patch(self, module, repo_path, patch_relpath, module_dir, module_install_dir): + def try_apply_patch(self, component, repo_path, patch_relpath, component_dir, component_install_dir): """ - Try applying a patch file to the new module files + Try applying a patch file to the new module/subworkflow files Args: - module (str): The name of the module - repo_path (str): The name of the repository where the module resides + component (str): The name of the module/subworkflow + repo_path (str): The name of the repository where the module/subworkflow resides patch_relpath (Path | str): The path to patch file in the pipeline - module_dir (Path | str): The module directory in the pipeline - module_install_dir (Path | str): The directory where the new module - file have been installed + component_dir (Path | str): The module/subworkflow directory in the pipeline + component_install_dir (Path | str): The directory where the new component + file have been installed Returns: (bool): Whether the patch application was successful """ - module_fullname = str(Path(repo_path, module)) - log.info(f"Found patch for module '{module_fullname}'. Trying to apply it to new files") + component_fullname = str(Path(repo_path, component)) + log.info(f"Found patch for {self.component_type[:-1]} '{component_fullname}'. Trying to apply it to new files") patch_path = Path(self.dir / patch_relpath) - module_relpath = Path("modules", repo_path, module) + component_relpath = Path(self.component_type, repo_path, component) # Check that paths in patch file are updated - self.check_patch_paths(patch_path, module) + self.check_patch_paths(patch_path, component) # Copy the installed files to a new temporary directory to save them for later use temp_dir = Path(tempfile.mkdtemp()) - temp_module_dir = temp_dir / module - shutil.copytree(module_install_dir, temp_module_dir) + temp_component_dir = temp_dir / component + shutil.copytree(component_install_dir, temp_component_dir) try: - new_files = ModulesDiffer.try_apply_patch(module, repo_path, patch_path, temp_module_dir) + new_files = ModulesDiffer.try_apply_patch(component, repo_path, patch_path, temp_component_dir) except LookupError: # Patch failed. Save the patch file by moving to the install dir - shutil.move(patch_path, Path(module_install_dir, patch_path.relative_to(module_dir))) + shutil.move(patch_path, Path(component_install_dir, patch_path.relative_to(component_dir))) log.warning( - f"Failed to apply patch for module '{module_fullname}'. You will have to apply the patch manually" + f"Failed to apply patch for {self.component_type[:-1]} '{component_fullname}'. You will have to apply the patch manually" ) return False # Write the patched files to a temporary directory log.debug("Writing patched files") for file, new_content in new_files.items(): - fn = temp_module_dir / file + fn = temp_component_dir / file with open(fn, "w") as fh: fh.writelines(new_content) # Create the new patch file log.debug("Regenerating patch file") ModulesDiffer.write_diff_file( - Path(temp_module_dir, patch_path.relative_to(module_dir)), - module, + Path(temp_component_dir, patch_path.relative_to(component_dir)), + component, repo_path, - module_install_dir, - temp_module_dir, + component_install_dir, + temp_component_dir, file_action="w", for_git=False, - dsp_from_dir=module_relpath, - dsp_to_dir=module_relpath, + dsp_from_dir=component_relpath, + dsp_to_dir=component_relpath, ) # Move the patched files to the install dir log.debug("Overwriting installed files installed files with patched files") - shutil.rmtree(module_install_dir) - shutil.copytree(temp_module_dir, module_install_dir) + shutil.rmtree(component_install_dir) + shutil.copytree(temp_component_dir, component_install_dir) # Add the patch file to the modules.json file self.modules_json.add_patch_entry( - module, self.modules_repo.remote_url, repo_path, patch_relpath, write_file=True + component, self.modules_repo.remote_url, repo_path, patch_relpath, write_file=True ) return True From f29249cb4603426cf72a7e08151ab8ec0accbc0d Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Fri, 11 Nov 2022 09:47:43 +0100 Subject: [PATCH 04/31] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a4dd46decb..a8e6733429 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,7 @@ - `check_up_to_date()` function from `modules_json` also checks for subworkflows. - Update subworkflows install so it installs also imported modules and subworkflows ([#1904](https://github.com/nf-core/tools/pull/1904)) - Function create() from modules_json.py adds also subworkflows to modules.json file ([#2005](https://github.com/nf-core/tools/pull/2005)) +- Add `nf-core subworkflows update` command ([#2019](https://github.com/nf-core/tools/pull/2019)) ## [v2.6 - Tin Octopus](https://github.com/nf-core/tools/releases/tag/2.6) - [2022-10-04] From 3376ed22429bbc890df673de24dc2cda1fae28af Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Fri, 11 Nov 2022 13:24:27 +0100 Subject: [PATCH 05/31] add subworkflows update command to main --- nf_core/__main__.py | 57 ++++++++++++++++++++++++++++++++ nf_core/subworkflows/__init__.py | 1 + 2 files changed, 58 insertions(+) diff --git a/nf_core/__main__.py b/nf_core/__main__.py index e7087b4931..377f006f2a 100755 --- a/nf_core/__main__.py +++ b/nf_core/__main__.py @@ -1056,6 +1056,63 @@ def local(ctx, keywords, json, dir): # pylint: disable=redefined-builtin sys.exit(1) +# nf-core subworkflows update +@subworkflows.command() +@click.pass_context +@click.argument("subworkflow", type=str, required=False, metavar="subworkflow name") +@click.option( + "-d", + "--dir", + type=click.Path(exists=True), + default=".", + help=r"Pipeline directory. [dim]\[default: current working directory][/]", +) +@click.option("-f", "--force", is_flag=True, default=False, help="Force update of subworkflow") +@click.option("-p", "--prompt", is_flag=True, default=False, help="Prompt for the version of the subworkflow") +@click.option("-s", "--sha", type=str, metavar="", help="Install subworkflow at commit SHA") +@click.option("-a", "--all", is_flag=True, default=False, help="Update all subworkflow installed in pipeline") +@click.option( + "-x/-y", + "--preview/--no-preview", + is_flag=True, + default=None, + help="Preview / no preview of changes before applying", +) +@click.option( + "-D", + "--save-diff", + type=str, + metavar="", + default=None, + help="Save diffs to a file instead of updating in place", +) +def update(ctx, subworkflow, dir, force, prompt, sha, all, preview, save_diff): + """ + Update DSL2 subworkflow within a pipeline. + + Fetches and updates subworkflow files from a remote repo e.g. nf-core/modules. + """ + try: + subworkflow_install = nf_core.subworkflows.SubworkflowUpdate( + dir, + force, + prompt, + sha, + all, + preview, + save_diff, + ctx.obj["modules_repo_url"], + ctx.obj["modules_repo_branch"], + ctx.obj["modules_repo_no_pull"], + ) + exit_status = subworkflow_install.update(subworkflow) + if not exit_status and all: + sys.exit(1) + except (UserWarning, LookupError) as e: + log.error(e) + sys.exit(1) + + # nf-core schema subcommands @nf_core_cli.group() def schema(): diff --git a/nf_core/subworkflows/__init__.py b/nf_core/subworkflows/__init__.py index f10c850d8d..3a672c5a15 100644 --- a/nf_core/subworkflows/__init__.py +++ b/nf_core/subworkflows/__init__.py @@ -3,3 +3,4 @@ from .list import SubworkflowList from .subworkflows_test import SubworkflowsTest from .test_yml_builder import SubworkflowTestYmlBuilder +from .update import SubworkflowUpdate From bf1eb285ee70cd4c27382c488898a211ab3064ab Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Fri, 11 Nov 2022 15:39:04 +0100 Subject: [PATCH 06/31] update modules and subworkflows recursively --- nf_core/__main__.py | 1 + nf_core/components/update.py | 63 +++++++++++++++++++++++++-- nf_core/modules/install.py | 4 +- nf_core/modules/modules_json.py | 77 ++++++++------------------------- nf_core/subworkflows/install.py | 11 +++-- 5 files changed, 88 insertions(+), 68 deletions(-) diff --git a/nf_core/__main__.py b/nf_core/__main__.py index 377f006f2a..1f6d266d19 100755 --- a/nf_core/__main__.py +++ b/nf_core/__main__.py @@ -1109,6 +1109,7 @@ def update(ctx, subworkflow, dir, force, prompt, sha, all, preview, save_diff): if not exit_status and all: sys.exit(1) except (UserWarning, LookupError) as e: + raise log.error(e) sys.exit(1) diff --git a/nf_core/components/update.py b/nf_core/components/update.py index 17a583ac58..05349231f8 100644 --- a/nf_core/components/update.py +++ b/nf_core/components/update.py @@ -214,7 +214,7 @@ def update(self, component=None): elif self.show_diff: ModulesDiffer.print_diff( component, - components_repo.repo_path, + modules_repo.repo_path, component_dir, component_install_dir, current_version, @@ -234,10 +234,14 @@ def update(self, component=None): # Clear the component directory and move the installed files there self.move_files_from_tmp_dir(component, install_tmp_dir, modules_repo.repo_path, version) # Update modules.json with newly installed component - self.modules_json.update(modules_repo, component, version, self.component_type) + self.modules_json.update(self.component_type, modules_repo, component, version, self.component_type) + # Update linked components + self.update_linked_components(component, modules_repo.repo_path) else: # Don't save to a file, just iteratively update the variable - self.modules_json.update(modules_repo, component, version, self.component_type, write_file=False) + self.modules_json.update( + self.component_type, modules_repo, component, version, self.component_type, write_file=False + ) if self.save_diff_fn: # Write the modules.json diff to the file @@ -722,3 +726,56 @@ def try_apply_patch(self, component, repo_path, patch_relpath, component_dir, co ) return True + + def get_modules_subworkflows_to_update(self, component, install_dir): + """Get all modules and subworkflows linked to the updated component.""" + mods_json = self.modules_json.get_modules_json() + modules_to_update = [] + subworkflows_to_update = [] + installed_by = mods_json["repos"][self.modules_repo.remote_url][self.component_type][install_dir][component][ + "installed_by" + ] + + if self.component_type == "modules": + # All subworkflow names in the installed_by section of a module are subworkflows using this module + # We need to update them too + subworkflows_to_update = [subworkflow for subworkflow in installed_by if subworkflow != self.component_type] + elif self.component_type == "subworkflows": + for repo, repo_content in mods_json["repos"].items(): + for component_type, dir_content in repo_content.items(): + for dir, components in dir_content.items(): + for comp, comp_content in components.items(): + # If the updated subworkflow name appears in the installed_by section of the checked component + # The checked component is used by the updated subworkflow + # We need to update it too + if component in comp_content["installed_by"]: + if component_type == "modules": + modules_to_update.append(comp) + elif component_type == "subworkflows": + subworkflows_to_update.append(comp) + + return modules_to_update, subworkflows_to_update + + def update_linked_components(self, component, install_dir): + """ + Update modules and subworkflows linked to the component being updated. + """ + modules_to_update, subworkflows_to_update = self.get_modules_subworkflows_to_update(component, install_dir) + for s_update in subworkflows_to_update: + original_component_type = self._change_component_type("subworkflows") + self.update(s_update) + self._reset_component_type(original_component_type) + for m_update in modules_to_update: + original_component_type = self._change_component_type("modules") + self.update(m_update) + self._reset_component_type(original_component_type) + + def _change_component_type(self, new_component_type): + original_component_type = self.component_type + self.component_type = new_component_type + self.modules_json.pipeline_components = None + return original_component_type + + def _reset_component_type(self, original_component_type): + self.component_type = original_component_type + self.modules_json.pipeline_components = None diff --git a/nf_core/modules/install.py b/nf_core/modules/install.py index 2a38f16c4c..346157d7a5 100644 --- a/nf_core/modules/install.py +++ b/nf_core/modules/install.py @@ -76,7 +76,7 @@ def install(self, module, silent=False): f"Module is already installed and force is not set.\nAdding the new installation source {self.installed_by} for module {module} to 'modules.json' without installing the module." ) modules_json.load() - modules_json.update(self.modules_repo, module, current_version, self.installed_by) + modules_json.update(self.component_type, self.modules_repo, module, current_version, self.installed_by) return False version = nf_core.components.components_install.get_version( @@ -110,5 +110,5 @@ def install(self, module, silent=False): # Update module.json with newly installed module modules_json.load() - modules_json.update(self.modules_repo, module, version, self.installed_by, install_track) + modules_json.update(self.component_type, self.modules_repo, module, version, self.installed_by, install_track) return True diff --git a/nf_core/modules/modules_json.py b/nf_core/modules/modules_json.py index e6f4e03571..7444e0e653 100644 --- a/nf_core/modules/modules_json.py +++ b/nf_core/modules/modules_json.py @@ -587,55 +587,15 @@ def load(self): except FileNotFoundError: raise UserWarning("File 'modules.json' is missing") - def update(self, modules_repo, module_name, module_version, installed_by, installed_by_log=None, write_file=True): + def update(self, component_type, modules_repo, component_name, component_version, installed_by, installed_by_log=None, write_file=True): """ - Updates the 'module.json' file with new module info + Updates the 'module.json' file with new module/subworkflow info Args: - modules_repo (ModulesRepo): A ModulesRepo object configured for the new module - module_name (str): Name of new module - module_version (str): git SHA for the new module entry - installed_by_log (list): previous tracing of installed_by that needs to be added to 'modules.json' - write_file (bool): whether to write the updated modules.json to a file. - """ - if installed_by_log is None: - installed_by_log = [] - - if self.modules_json is None: - self.load() - repo_name = modules_repo.repo_path - remote_url = modules_repo.remote_url - branch = modules_repo.branch - if remote_url not in self.modules_json["repos"]: - self.modules_json["repos"][remote_url] = {"modules": {repo_name: {}}} - repo_modules_entry = self.modules_json["repos"][remote_url]["modules"][repo_name] - if module_name not in repo_modules_entry: - repo_modules_entry[module_name] = {} - repo_modules_entry[module_name]["git_sha"] = module_version - repo_modules_entry[module_name]["branch"] = branch - try: - if installed_by not in repo_modules_entry[module_name]["installed_by"]: - repo_modules_entry[module_name]["installed_by"].append(installed_by) - except KeyError: - repo_modules_entry[module_name]["installed_by"] = [installed_by] - finally: - repo_modules_entry[module_name]["installed_by"].extend(installed_by_log) - - # Sort the 'modules.json' repo entries - self.modules_json["repos"] = nf_core.utils.sort_dictionary(self.modules_json["repos"]) - if write_file: - self.dump() - - def update_subworkflow( - self, modules_repo, subworkflow_name, subworkflow_version, installed_by, installed_by_log=None, write_file=True - ): - """ - Updates the 'module.json' file with new subworkflow info - - Args: - modules_repo (ModulesRepo): A ModulesRepo object configured for the new subworkflow - subworkflow_name (str): Name of new subworkflow - subworkflow_version (str): git SHA for the new subworkflow entry + component_type (str): modules or subworkflows + modules_repo (ModulesRepo): A ModulesRepo object configured for the new module/subworkflow + component_name (str): Name of new module/subworkflow + component_version (str): git SHA for the new module/subworkflow entry installed_by_log (list): previous tracing of installed_by that needs to be added to 'modules.json' write_file (bool): whether to write the updated modules.json to a file. """ @@ -648,22 +608,21 @@ def update_subworkflow( remote_url = modules_repo.remote_url branch = modules_repo.branch if remote_url not in self.modules_json["repos"]: - self.modules_json["repos"][remote_url] = {"subworkflows": {repo_name: {}}} - if "subworkflows" not in self.modules_json["repos"][remote_url]: - # It's the first subworkflow installed in the pipeline! - self.modules_json["repos"][remote_url]["subworkflows"] = {repo_name: {}} - repo_subworkflows_entry = self.modules_json["repos"][remote_url]["subworkflows"][repo_name] - if subworkflow_name not in repo_subworkflows_entry: - repo_subworkflows_entry[subworkflow_name] = {} - repo_subworkflows_entry[subworkflow_name]["git_sha"] = subworkflow_version - repo_subworkflows_entry[subworkflow_name]["branch"] = branch + self.modules_json["repos"][remote_url] = {component_type: {repo_name: {}}} + if component_type not in self.modules_json["repos"][remote_url]: + self.modules_json["repos"][remote_url][component_type] = {repo_name: {}} + repo_component_entry = self.modules_json["repos"][remote_url][component_type][repo_name] + if component_name not in repo_component_entry: + repo_component_entry[component_name] = {} + repo_component_entry[component_name]["git_sha"] = component_version + repo_component_entry[component_name]["branch"] = branch try: - if installed_by not in repo_subworkflows_entry[subworkflow_name]["installed_by"]: - repo_subworkflows_entry[subworkflow_name]["installed_by"].append(installed_by) + if installed_by not in repo_component_entry[component_name]["installed_by"]: + repo_component_entry[component_name]["installed_by"].append(installed_by) except KeyError: - repo_subworkflows_entry[subworkflow_name]["installed_by"] = [installed_by] + repo_component_entry[component_name]["installed_by"] = [installed_by] finally: - repo_subworkflows_entry[subworkflow_name]["installed_by"].extend(installed_by_log) + repo_component_entry[component_name]["installed_by"].extend(installed_by_log) # Sort the 'modules.json' repo entries self.modules_json["repos"] = nf_core.utils.sort_dictionary(self.modules_json["repos"]) diff --git a/nf_core/subworkflows/install.py b/nf_core/subworkflows/install.py index 2df0c48970..d16e37e6c1 100644 --- a/nf_core/subworkflows/install.py +++ b/nf_core/subworkflows/install.py @@ -82,7 +82,7 @@ def install(self, subworkflow, silent=False): f"Subworkflow is already installed and force is not set.\nAdding the new installation source {self.installed_by} for subworkflow {subworkflow} to 'modules.json' without installing the subworkflow." ) modules_json.load() - modules_json.update(self.modules_repo, subworkflow, current_version, self.installed_by) + modules_json.update(self.component_type, self.modules_repo, subworkflow, current_version, self.installed_by) return False version = nf_core.components.components_install.get_version( @@ -115,6 +115,12 @@ def install(self, subworkflow, silent=False): if not self.install_component_files(subworkflow, version, self.modules_repo, install_folder): return False + # Update module.json with newly installed subworkflow + modules_json.load() + modules_json.update( + self.component_type, self.modules_repo, subworkflow, version, self.installed_by, install_track + ) + # Install included modules and subworkflows self.install_included_components(subworkflow_dir) @@ -128,9 +134,6 @@ def install(self, subworkflow, silent=False): if os.path.isfile(subworkflow_config): log.info(f"Subworkflow config include statement: includeConfig '{subworkflow_config}'") - # Update module.json with newly installed subworkflow - modules_json.load() - modules_json.update_subworkflow(self.modules_repo, subworkflow, version, self.installed_by, install_track) return True def get_modules_subworkflows_to_install(self, subworkflow_dir): From 6f335f3986c911d142e6cb1b088fcf2fc54f6287 Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Fri, 11 Nov 2022 16:17:09 +0100 Subject: [PATCH 07/31] add silent and updated arguments and add prompt --- nf_core/components/update.py | 37 ++++++++++++++++++++++++++++-------- 1 file changed, 29 insertions(+), 8 deletions(-) diff --git a/nf_core/components/update.py b/nf_core/components/update.py index 05349231f8..3b56a2d865 100644 --- a/nf_core/components/update.py +++ b/nf_core/components/update.py @@ -67,7 +67,7 @@ def _parameter_checks(self): if not self.has_valid_directory(): raise UserWarning("The command was not run in a valid pipeline directory.") - def update(self, component=None): + def update(self, component=None, silent=False, updated=None): """Updates a specified module/subworkflow or all modules/subworkflows in a pipeline. If updating a subworkflow: updates all modules used in that subworkflow. @@ -80,6 +80,8 @@ def update(self, component=None): bool: True if the update was successful, False otherwise. """ self.component = component + if updated is None: + updated = [] tool_config = nf_core.utils.load_tools_config(self.dir) self.update_config = tool_config.get("update", {}) @@ -235,8 +237,22 @@ def update(self, component=None): self.move_files_from_tmp_dir(component, install_tmp_dir, modules_repo.repo_path, version) # Update modules.json with newly installed component self.modules_json.update(self.component_type, modules_repo, component, version, self.component_type) - # Update linked components - self.update_linked_components(component, modules_repo.repo_path) + updated.append(component) + recursive_update = True + if not silent: + log.info( + f"All modules and subworkflows linked to the updated {self.component_type[:-1]} will be automatically updated." + "It is advised to keep all your modules and subworkflows up to date." + "It is not guaranteed that a subworkflow will continue working as expected if all modules/subworkflows used in it are not up to date." + ) + recursive_update = questionary.confirm( + "Would you like to continue updating all modules and subworkflows?", + default=True, + style=nf_core.utils.nfcore_question_style, + ).unsafe_ask() + if recursive_update: + # Update linked components + self.update_linked_components(component, modules_repo.repo_path, updated) else: # Don't save to a file, just iteratively update the variable self.modules_json.update( @@ -257,9 +273,9 @@ def update(self, component=None): "can apply them by running the command :point_right:" f" [bold magenta italic]git apply {self.save_diff_fn} [/]" ) - elif not all_patches_successful: + elif not all_patches_successful and not silent: log.info(f"Updates complete. Please apply failed patch{plural_es(components_info)} manually") - else: + elif not silent: log.info("Updates complete :sparkles:") return exit_value @@ -756,18 +772,23 @@ def get_modules_subworkflows_to_update(self, component, install_dir): return modules_to_update, subworkflows_to_update - def update_linked_components(self, component, install_dir): + def update_linked_components(self, component, install_dir, updated=None): """ Update modules and subworkflows linked to the component being updated. """ modules_to_update, subworkflows_to_update = self.get_modules_subworkflows_to_update(component, install_dir) for s_update in subworkflows_to_update: + if s_update in updated: + continue original_component_type = self._change_component_type("subworkflows") - self.update(s_update) + self.update(s_update, silent=True, updated=updated) self._reset_component_type(original_component_type) + for m_update in modules_to_update: + if m_update in updated: + continue original_component_type = self._change_component_type("modules") - self.update(m_update) + self.update(m_update, silent=True, updated=updated) self._reset_component_type(original_component_type) def _change_component_type(self, new_component_type): From 03f5cdb2b8088ca343b64c5502c984a4e09ea68e Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Fri, 11 Nov 2022 16:59:39 +0100 Subject: [PATCH 08/31] add recursivity and warnings also for show diff and add diff to file --- nf_core/components/update.py | 72 +++++++++++++++++++++++++----------- 1 file changed, 50 insertions(+), 22 deletions(-) diff --git a/nf_core/components/update.py b/nf_core/components/update.py index 3b56a2d865..10c3ff256d 100644 --- a/nf_core/components/update.py +++ b/nf_core/components/update.py @@ -67,7 +67,7 @@ def _parameter_checks(self): if not self.has_valid_directory(): raise UserWarning("The command was not run in a valid pipeline directory.") - def update(self, component=None, silent=False, updated=None): + def update(self, component=None, silent=False, updated=None, check_diff_exist=True): """Updates a specified module/subworkflow or all modules/subworkflows in a pipeline. If updating a subworkflow: updates all modules used in that subworkflow. @@ -134,7 +134,7 @@ def update(self, component=None, silent=False, updated=None): self.save_diff_fn = diff_type == 2 if self.save_diff_fn: # True or a string - self.setup_diff_file() + self.setup_diff_file(check_diff_exist) # Loop through all components to be updated # and do the requested action on them @@ -201,17 +201,41 @@ def update(self, component=None, silent=False, updated=None): log.info( f"Writing diff file for {self.component_type[:-1]} '{component_fullname}' to '{self.save_diff_fn}'" ) - ModulesDiffer.write_diff_file( - self.save_diff_fn, - component, - modules_repo.repo_path, - component_dir, - component_install_dir, - current_version, - version, - dsp_from_dir=component_dir, - dsp_to_dir=component_dir, - ) + try: + ModulesDiffer.write_diff_file( + self.save_diff_fn, + component, + modules_repo.repo_path, + component_dir, + component_install_dir, + current_version, + version, + dsp_from_dir=component_dir, + dsp_to_dir=component_dir, + ) + updated.append(component) + except UserWarning as e: + if str(e) != "Module is unchanged": + raise + else: + updated.append(component) + recursive_update = True + if not silent: + log.warning( + f"All modules and subworkflows linked to the updated {self.component_type[:-1]} will be added to the same diff file.\n" + "It is advised to keep all your modules and subworkflows up to date.\n" + "It is not guaranteed that a subworkflow will continue working as expected if all modules/subworkflows used in it are not up to date.\n" + ) + recursive_update = questionary.confirm( + "Would you like to continue adding all modules and subworkflows differences?", + default=True, + style=nf_core.utils.nfcore_question_style, + ).unsafe_ask() + if recursive_update: + # Write all the differences of linked componenets to a diff file + self.update_linked_components( + component, modules_repo.repo_path, updated, check_diff_exist=False + ) elif self.show_diff: ModulesDiffer.print_diff( @@ -240,10 +264,10 @@ def update(self, component=None, silent=False, updated=None): updated.append(component) recursive_update = True if not silent: - log.info( - f"All modules and subworkflows linked to the updated {self.component_type[:-1]} will be automatically updated." - "It is advised to keep all your modules and subworkflows up to date." - "It is not guaranteed that a subworkflow will continue working as expected if all modules/subworkflows used in it are not up to date." + 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" + "It is advised to keep all your modules and subworkflows up to date.\n" + "It is not guaranteed that a subworkflow will continue working as expected if all modules/subworkflows used in it are not up to date.\n" ) recursive_update = questionary.confirm( "Would you like to continue updating all modules and subworkflows?", @@ -267,7 +291,7 @@ def update(self, component=None, silent=False, updated=None): self.modules_json.get_modules_json(), Path(self.dir, "modules.json"), ) - if exit_value: + if exit_value and not silent: log.info( f"[bold magenta italic] TIP! [/] If you are happy with the changes in '{self.save_diff_fn}', you " "can apply them by running the command :point_right:" @@ -616,7 +640,7 @@ def get_all_components_info(self, branch=None): return components_info - def setup_diff_file(self): + def setup_diff_file(self, check_diff_exist=True): """Sets up the diff file. If the save diff option was chosen interactively, the user is asked to supply a name for the diff file. @@ -631,6 +655,10 @@ def setup_diff_file(self): self.save_diff_fn = Path(self.save_diff_fn) + if not check_diff_exist: + # This guarantees that the file exists after calling the function + self.save_diff_fn.touch() + return # Check if filename already exists (questionary or cli) while self.save_diff_fn.exists(): if questionary.confirm(f"'{self.save_diff_fn}' exists. Remove file?").unsafe_ask(): @@ -772,7 +800,7 @@ def get_modules_subworkflows_to_update(self, component, install_dir): return modules_to_update, subworkflows_to_update - def update_linked_components(self, component, install_dir, updated=None): + def update_linked_components(self, component, install_dir, updated=None, check_diff_exist=True): """ Update modules and subworkflows linked to the component being updated. """ @@ -781,14 +809,14 @@ def update_linked_components(self, component, install_dir, updated=None): if s_update in updated: continue original_component_type = self._change_component_type("subworkflows") - self.update(s_update, silent=True, updated=updated) + self.update(s_update, silent=True, updated=updated, check_diff_exist=check_diff_exist) self._reset_component_type(original_component_type) for m_update in modules_to_update: if m_update in updated: continue original_component_type = self._change_component_type("modules") - self.update(m_update, silent=True, updated=updated) + self.update(m_update, silent=True, updated=updated, check_diff_exist=check_diff_exist) self._reset_component_type(original_component_type) def _change_component_type(self, new_component_type): From 841869097caa236d361dedc089705209095a510d Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Fri, 11 Nov 2022 17:05:48 +0100 Subject: [PATCH 09/31] handle update all --- nf_core/components/update.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/nf_core/components/update.py b/nf_core/components/update.py index 10c3ff256d..60de4a5aeb 100644 --- a/nf_core/components/update.py +++ b/nf_core/components/update.py @@ -808,23 +808,27 @@ def update_linked_components(self, component, install_dir, updated=None, check_d for s_update in subworkflows_to_update: if s_update in updated: continue - original_component_type = self._change_component_type("subworkflows") + original_component_type, original_update_all = self._change_component_type("subworkflows") self.update(s_update, silent=True, updated=updated, check_diff_exist=check_diff_exist) - self._reset_component_type(original_component_type) + self._reset_component_type(original_component_type, original_update_all) for m_update in modules_to_update: if m_update in updated: continue - original_component_type = self._change_component_type("modules") + original_component_type, original_update_all = self._change_component_type("modules") self.update(m_update, silent=True, updated=updated, check_diff_exist=check_diff_exist) - self._reset_component_type(original_component_type) + self._reset_component_type(original_component_type, original_update_all) def _change_component_type(self, new_component_type): original_component_type = self.component_type self.component_type = new_component_type self.modules_json.pipeline_components = None - return original_component_type + # also reset update_all in case it's set + original_update_all = self.update_all + self.update_all = False + return original_component_type, original_update_all - def _reset_component_type(self, original_component_type): + def _reset_component_type(self, original_component_type, original_update_all): self.component_type = original_component_type self.modules_json.pipeline_components = None + self.update_all = original_update_all From 370c6cb212aa708e5ff74149790b70b8bdc87927 Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Fri, 11 Nov 2022 17:11:13 +0100 Subject: [PATCH 10/31] don't prompt warning if update all --- nf_core/components/update.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nf_core/components/update.py b/nf_core/components/update.py index 60de4a5aeb..a1fc4142f4 100644 --- a/nf_core/components/update.py +++ b/nf_core/components/update.py @@ -263,7 +263,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, self.component_type) updated.append(component) recursive_update = True - if not silent: + if not silent and not self.update_all: 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" "It is advised to keep all your modules and subworkflows up to date.\n" From 80fd5f6b72cdd49d5b3a8c0af522d83210d00854 Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Mon, 14 Nov 2022 09:30:07 +0100 Subject: [PATCH 11/31] run black --- nf_core/modules/modules_json.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/nf_core/modules/modules_json.py b/nf_core/modules/modules_json.py index 7444e0e653..ee7051a691 100644 --- a/nf_core/modules/modules_json.py +++ b/nf_core/modules/modules_json.py @@ -587,7 +587,16 @@ def load(self): except FileNotFoundError: raise UserWarning("File 'modules.json' is missing") - def update(self, component_type, modules_repo, component_name, component_version, installed_by, installed_by_log=None, write_file=True): + def update( + self, + component_type, + modules_repo, + component_name, + component_version, + installed_by, + installed_by_log=None, + write_file=True, + ): """ Updates the 'module.json' file with new module/subworkflow info From c99f34f7fff74f8984807e439ea18a71447d7ce3 Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Mon, 14 Nov 2022 10:30:47 +0100 Subject: [PATCH 12/31] take into account different repos --- nf_core/components/update.py | 19 +++++++++---------- tests/modules/modules_json.py | 4 ++-- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/nf_core/components/update.py b/nf_core/components/update.py index a1fc4142f4..04eca86df0 100644 --- a/nf_core/components/update.py +++ b/nf_core/components/update.py @@ -233,9 +233,7 @@ def update(self, component=None, silent=False, updated=None, check_diff_exist=Tr ).unsafe_ask() if recursive_update: # Write all the differences of linked componenets to a diff file - self.update_linked_components( - component, modules_repo.repo_path, updated, check_diff_exist=False - ) + self.update_linked_components(component, modules_repo, updated, check_diff_exist=False) elif self.show_diff: ModulesDiffer.print_diff( @@ -276,7 +274,7 @@ def update(self, component=None, silent=False, updated=None, check_diff_exist=Tr ).unsafe_ask() if recursive_update: # Update linked components - self.update_linked_components(component, modules_repo.repo_path, updated) + self.update_linked_components(component, modules_repo, updated) else: # Don't save to a file, just iteratively update the variable self.modules_json.update( @@ -771,19 +769,20 @@ def try_apply_patch(self, component, repo_path, patch_relpath, component_dir, co return True - def get_modules_subworkflows_to_update(self, component, install_dir): + def get_modules_subworkflows_to_update(self, component, modules_repo): """Get all modules and subworkflows linked to the updated component.""" mods_json = self.modules_json.get_modules_json() modules_to_update = [] subworkflows_to_update = [] - installed_by = mods_json["repos"][self.modules_repo.remote_url][self.component_type][install_dir][component][ - "installed_by" - ] + installed_by = mods_json["repos"][modules_repo.remote_url][self.component_type][modules_repo.repo_path][ + component + ]["installed_by"] if self.component_type == "modules": # All subworkflow names in the installed_by section of a module are subworkflows using this module # We need to update them too subworkflows_to_update = [subworkflow for subworkflow in installed_by if subworkflow != self.component_type] + print(subworkflows_to_update) elif self.component_type == "subworkflows": for repo, repo_content in mods_json["repos"].items(): for component_type, dir_content in repo_content.items(): @@ -800,11 +799,11 @@ def get_modules_subworkflows_to_update(self, component, install_dir): return modules_to_update, subworkflows_to_update - def update_linked_components(self, component, install_dir, updated=None, check_diff_exist=True): + def update_linked_components(self, component, modules_repo, updated=None, check_diff_exist=True): """ Update modules and subworkflows linked to the component being updated. """ - modules_to_update, subworkflows_to_update = self.get_modules_subworkflows_to_update(component, install_dir) + modules_to_update, subworkflows_to_update = self.get_modules_subworkflows_to_update(component, modules_repo) for s_update in subworkflows_to_update: if s_update in updated: continue diff --git a/tests/modules/modules_json.py b/tests/modules/modules_json.py index 20eee54e30..67ee44f973 100644 --- a/tests/modules/modules_json.py +++ b/tests/modules/modules_json.py @@ -32,7 +32,7 @@ def test_mod_json_update(self): mod_json_obj = ModulesJson(self.pipeline_dir) # Update the modules.json file mod_repo_obj = ModulesRepo() - mod_json_obj.update(mod_repo_obj, "MODULE_NAME", "GIT_SHA", "modules", write_file=False) + mod_json_obj.update("modules", mod_repo_obj, "MODULE_NAME", "GIT_SHA", "modules", write_file=False) mod_json = mod_json_obj.get_modules_json() assert "MODULE_NAME" in mod_json["repos"][NF_CORE_MODULES_REMOTE]["modules"]["nf-core"] assert "git_sha" in mod_json["repos"][NF_CORE_MODULES_REMOTE]["modules"]["nf-core"]["MODULE_NAME"] @@ -155,7 +155,7 @@ def test_mod_json_up_to_date_reinstall_fails(self): mod_json_obj = ModulesJson(self.pipeline_dir) # Update the fastqc module entry to an invalid git_sha - mod_json_obj.update(ModulesRepo(), "fastqc", "INVALID_GIT_SHA", "modules", write_file=True) + mod_json_obj.update("modules", ModulesRepo(), "fastqc", "INVALID_GIT_SHA", "modules", write_file=True) # Remove the fastqc module fastqc_path = os.path.join(self.pipeline_dir, "modules", NF_CORE_MODULES_NAME, "fastqc") From 615b975afa179486c8d99a1ce17f3a7d85e5fc8f Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Mon, 14 Nov 2022 10:57:02 +0100 Subject: [PATCH 13/31] add argument to update recursively without prompts --- nf_core/__main__.py | 21 ++++++++++++++++++--- nf_core/components/update.py | 28 ++++++++++++++++++---------- nf_core/modules/modules_json.py | 2 ++ nf_core/modules/update.py | 2 ++ nf_core/subworkflows/update.py | 2 ++ tests/modules/patch.py | 4 ++-- tests/modules/update.py | 6 ++++-- 7 files changed, 48 insertions(+), 17 deletions(-) diff --git a/nf_core/__main__.py b/nf_core/__main__.py index 1f6d266d19..a42776d685 100755 --- a/nf_core/__main__.py +++ b/nf_core/__main__.py @@ -554,7 +554,14 @@ def install(ctx, tool, dir, prompt, force, sha): default=None, help="Save diffs to a file instead of updating in place", ) -def update(ctx, tool, dir, force, prompt, sha, all, preview, save_diff): +@click.option( + "-r", + "--recursive", + is_flat=True, + default=False, + help="Automatically update all linked modules and subworkflows without asking for confirmation", +) +def update(ctx, tool, dir, force, prompt, sha, all, preview, save_diff, recursive): """ Update DSL2 modules within a pipeline. @@ -569,6 +576,7 @@ def update(ctx, tool, dir, force, prompt, sha, all, preview, save_diff): all, preview, save_diff, + recursive, ctx.obj["modules_repo_url"], ctx.obj["modules_repo_branch"], ctx.obj["modules_repo_no_pull"], @@ -1086,7 +1094,14 @@ def local(ctx, keywords, json, dir): # pylint: disable=redefined-builtin default=None, help="Save diffs to a file instead of updating in place", ) -def update(ctx, subworkflow, dir, force, prompt, sha, all, preview, save_diff): +@click.option( + "-r", + "--recursive", + is_flat=True, + default=False, + help="Automatically update all linked modules and subworkflows without asking for confirmation", +) +def update(ctx, subworkflow, dir, force, prompt, sha, all, preview, save_diff, recursive): """ Update DSL2 subworkflow within a pipeline. @@ -1101,6 +1116,7 @@ def update(ctx, subworkflow, dir, force, prompt, sha, all, preview, save_diff): all, preview, save_diff, + recursive, ctx.obj["modules_repo_url"], ctx.obj["modules_repo_branch"], ctx.obj["modules_repo_no_pull"], @@ -1109,7 +1125,6 @@ def update(ctx, subworkflow, dir, force, prompt, sha, all, preview, save_diff): if not exit_status and all: sys.exit(1) except (UserWarning, LookupError) as e: - raise log.error(e) sys.exit(1) diff --git a/nf_core/components/update.py b/nf_core/components/update.py index 04eca86df0..e5c9d667cf 100644 --- a/nf_core/components/update.py +++ b/nf_core/components/update.py @@ -29,6 +29,7 @@ def __init__( update_all=False, show_diff=None, save_diff_fn=None, + recursive=False, remote_url=None, branch=None, no_pull=False, @@ -40,6 +41,7 @@ def __init__( self.update_all = update_all self.show_diff = show_diff self.save_diff_fn = save_diff_fn + self.recursive = recursive self.component = None self.update_config = None self.modules_json = ModulesJson(self.dir) @@ -226,11 +228,14 @@ def update(self, component=None, silent=False, updated=None, check_diff_exist=Tr "It is advised to keep all your modules and subworkflows up to date.\n" "It is not guaranteed that a subworkflow will continue working as expected if all modules/subworkflows used in it are not up to date.\n" ) - recursive_update = questionary.confirm( - "Would you like to continue adding all modules and subworkflows differences?", - default=True, - style=nf_core.utils.nfcore_question_style, - ).unsafe_ask() + if self.recursive: + recursive_update = True + else: + recursive_update = questionary.confirm( + "Would you like to continue adding all modules and subworkflows differences?", + default=True, + style=nf_core.utils.nfcore_question_style, + ).unsafe_ask() if recursive_update: # Write all the differences of linked componenets to a diff file self.update_linked_components(component, modules_repo, updated, check_diff_exist=False) @@ -267,11 +272,14 @@ def update(self, component=None, silent=False, updated=None, check_diff_exist=Tr "It is advised to keep all your modules and subworkflows up to date.\n" "It is not guaranteed that a subworkflow will continue working as expected if all modules/subworkflows used in it are not up to date.\n" ) - recursive_update = questionary.confirm( - "Would you like to continue updating all modules and subworkflows?", - default=True, - style=nf_core.utils.nfcore_question_style, - ).unsafe_ask() + if self.recursive: + recursive_update = True + else: + recursive_update = questionary.confirm( + "Would you like to continue updating all modules and subworkflows?", + default=True, + style=nf_core.utils.nfcore_question_style, + ).unsafe_ask() if recursive_update: # Update linked components self.update_linked_components(component, modules_repo, updated) diff --git a/nf_core/modules/modules_json.py b/nf_core/modules/modules_json.py index ee7051a691..f636ac562d 100644 --- a/nf_core/modules/modules_json.py +++ b/nf_core/modules/modules_json.py @@ -459,6 +459,8 @@ def has_git_url_and_modules(self): """ for repo_url, repo_entry in self.modules_json.get("repos", {}).items(): if "modules" not in repo_entry: + if "subworkflows" in repo_entry: + continue log.warning(f"modules.json entry {repo_entry} does not have a modules entry") return False elif ( diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index 1d1ef95379..350401c43e 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -11,6 +11,7 @@ def __init__( update_all=False, show_diff=None, save_diff_fn=None, + recursive=False, remote_url=None, branch=None, no_pull=False, @@ -24,6 +25,7 @@ def __init__( update_all, show_diff, save_diff_fn, + recursive, remote_url, branch, no_pull, diff --git a/nf_core/subworkflows/update.py b/nf_core/subworkflows/update.py index 5272304c60..200ade9c50 100644 --- a/nf_core/subworkflows/update.py +++ b/nf_core/subworkflows/update.py @@ -11,6 +11,7 @@ def __init__( update_all=False, show_diff=None, save_diff_fn=None, + recursive=False, remote_url=None, branch=None, no_pull=False, @@ -24,6 +25,7 @@ def __init__( update_all, show_diff, save_diff_fn, + recursive, remote_url, branch, no_pull, diff --git a/tests/modules/patch.py b/tests/modules/patch.py index f1597cd776..100afe8731 100644 --- a/tests/modules/patch.py +++ b/tests/modules/patch.py @@ -235,7 +235,7 @@ def test_create_patch_update_success(self): # Update the module update_obj = nf_core.modules.ModuleUpdate( - self.pipeline_dir, sha=SUCCEED_SHA, show_diff=False, remote_url=GITLAB_URL, branch=PATCH_BRANCH + self.pipeline_dir, sha=SUCCEED_SHA, show_diff=False, recursive=True, remote_url=GITLAB_URL, branch=PATCH_BRANCH ) assert update_obj.update(BISMARK_ALIGN) @@ -294,7 +294,7 @@ def test_create_patch_update_fail(self): patch_contents = fh.read() update_obj = nf_core.modules.ModuleUpdate( - self.pipeline_dir, sha=FAIL_SHA, show_diff=False, remote_url=GITLAB_URL, branch=PATCH_BRANCH + self.pipeline_dir, sha=FAIL_SHA, show_diff=False, recursive=True, remote_url=GITLAB_URL, branch=PATCH_BRANCH ) update_obj.update(BISMARK_ALIGN) diff --git a/tests/modules/update.py b/tests/modules/update.py index 05f026fbbd..60f869ae4c 100644 --- a/tests/modules/update.py +++ b/tests/modules/update.py @@ -41,7 +41,9 @@ 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""" assert self.mods_install_old.install("trimgalore") - update_obj = ModuleUpdate(self.pipeline_dir, show_diff=False, remote_url=GITLAB_URL, branch=OLD_TRIMGALORE_BRANCH) + update_obj = ModuleUpdate( + self.pipeline_dir, show_diff=False, recursive=True, remote_url=GITLAB_URL, branch=OLD_TRIMGALORE_BRANCH + ) # Copy the module files and check that they are affected by the update tmpdir = tempfile.mkdtemp() @@ -225,7 +227,7 @@ def test_update_different_branch_single_module(self): assert install_obj.install("fastp") update_obj = ModuleUpdate( - self.pipeline_dir, remote_url=GITLAB_URL, branch=GITLAB_BRANCH_TEST_BRANCH, show_diff=False + self.pipeline_dir, recursive=True, remote_url=GITLAB_URL, branch=GITLAB_BRANCH_TEST_BRANCH, show_diff=False ) update_obj.update("fastp") From 544a4ff7b821f2d53ad4c074c3760390b07ac34c Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Mon, 14 Nov 2022 10:58:32 +0100 Subject: [PATCH 14/31] remove print --- nf_core/components/update.py | 1 - 1 file changed, 1 deletion(-) diff --git a/nf_core/components/update.py b/nf_core/components/update.py index e5c9d667cf..582484589d 100644 --- a/nf_core/components/update.py +++ b/nf_core/components/update.py @@ -790,7 +790,6 @@ def get_modules_subworkflows_to_update(self, component, modules_repo): # All subworkflow names in the installed_by section of a module are subworkflows using this module # We need to update them too subworkflows_to_update = [subworkflow for subworkflow in installed_by if subworkflow != self.component_type] - print(subworkflows_to_update) elif self.component_type == "subworkflows": for repo, repo_content in mods_json["repos"].items(): for component_type, dir_content in repo_content.items(): From 5874a0b65f1ed704f3506ebf18b07b3bba399a5d Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Mon, 14 Nov 2022 11:11:21 +0100 Subject: [PATCH 15/31] merge from dev --- nf_core/components/update.py | 11 +- tests/subworkflows/update.py | 296 +++++++++++++++++++++++++++++++++++ 2 files changed, 304 insertions(+), 3 deletions(-) create mode 100644 tests/subworkflows/update.py diff --git a/nf_core/components/update.py b/nf_core/components/update.py index 582484589d..4648a31875 100644 --- a/nf_core/components/update.py +++ b/nf_core/components/update.py @@ -182,7 +182,12 @@ def update(self, component=None, silent=False, updated=None, check_diff_exist=Tr if patch_relpath is not None: patch_successful = self.try_apply_patch( - component, modules_repo.repo_path, patch_relpath, component_dir, component_install_dir + component, + modules_repo.repo_path, + patch_relpath, + component_dir, + component_install_dir, + write_file=False, ) if patch_successful: log.info(f"{self.component_type[:-1].title()} '{component_fullname}' patched successfully") @@ -704,7 +709,7 @@ def move_files_from_tmp_dir(self, component, install_folder, repo_path, new_vers log.info(f"Updating '{repo_path}/{component}'") log.debug(f"Updating {self.component_type[:-1]} '{component}' to {new_version} from {repo_path}") - def try_apply_patch(self, component, repo_path, patch_relpath, component_dir, component_install_dir): + def try_apply_patch(self, component, repo_path, patch_relpath, component_dir, component_install_dir, write_file=True): """ Try applying a patch file to the new module/subworkflow files @@ -772,7 +777,7 @@ def try_apply_patch(self, component, repo_path, patch_relpath, component_dir, co # Add the patch file to the modules.json file self.modules_json.add_patch_entry( - component, self.modules_repo.remote_url, repo_path, patch_relpath, write_file=True + component, self.modules_repo.remote_url, repo_path, patch_relpath, write_file=write_file ) return True diff --git a/tests/subworkflows/update.py b/tests/subworkflows/update.py new file mode 100644 index 0000000000..60f869ae4c --- /dev/null +++ b/tests/subworkflows/update.py @@ -0,0 +1,296 @@ +import filecmp +import os +import shutil +import tempfile + +import yaml + +import nf_core.utils +from nf_core.modules.install import ModuleInstall +from nf_core.modules.modules_json import ModulesJson +from nf_core.modules.modules_repo import NF_CORE_MODULES_NAME, NF_CORE_MODULES_REMOTE +from nf_core.modules.update import ModuleUpdate + +from ..utils import ( + GITLAB_BRANCH_TEST_BRANCH, + GITLAB_BRANCH_TEST_NEW_SHA, + GITLAB_BRANCH_TEST_OLD_SHA, + GITLAB_DEFAULT_BRANCH, + GITLAB_REPO, + GITLAB_URL, + OLD_TRIMGALORE_BRANCH, + OLD_TRIMGALORE_SHA, +) + + +def test_install_and_update(self): + """Installs a module in the pipeline and updates it (no change)""" + self.mods_install.install("trimgalore") + update_obj = ModuleUpdate(self.pipeline_dir, show_diff=False) + + # Copy the module files and check that they are unaffected by the update + tmpdir = tempfile.mkdtemp() + trimgalore_tmpdir = os.path.join(tmpdir, "trimgalore") + trimgalore_path = os.path.join(self.pipeline_dir, "modules", NF_CORE_MODULES_NAME, "trimgalore") + shutil.copytree(trimgalore_path, trimgalore_tmpdir) + + assert update_obj.update("trimgalore") is True + assert cmp_module(trimgalore_tmpdir, trimgalore_path) is True + + +def test_install_at_hash_and_update(self): + """Installs an old version of a module in the pipeline and updates it""" + assert self.mods_install_old.install("trimgalore") + update_obj = ModuleUpdate( + self.pipeline_dir, show_diff=False, recursive=True, remote_url=GITLAB_URL, branch=OLD_TRIMGALORE_BRANCH + ) + + # Copy the module files and check that they are affected by the update + tmpdir = tempfile.mkdtemp() + trimgalore_tmpdir = os.path.join(tmpdir, "trimgalore") + trimgalore_path = os.path.join(self.pipeline_dir, "modules", GITLAB_REPO, "trimgalore") + shutil.copytree(trimgalore_path, trimgalore_tmpdir) + + assert update_obj.update("trimgalore") is True + assert cmp_module(trimgalore_tmpdir, trimgalore_path) is False + + # Check that the modules.json is correctly updated + mod_json_obj = ModulesJson(self.pipeline_dir) + mod_json = mod_json_obj.get_modules_json() + # Get the up-to-date git_sha for the module from the ModulesRepo object + correct_git_sha = update_obj.modules_repo.get_latest_component_version("trimgalore", "modules") + current_git_sha = mod_json["repos"][GITLAB_URL]["modules"][GITLAB_REPO]["trimgalore"]["git_sha"] + assert correct_git_sha == current_git_sha + + +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""" + self.mods_install_old.install("trimgalore") + patch_path = os.path.join(self.pipeline_dir, "trimgalore.patch") + update_obj = ModuleUpdate( + self.pipeline_dir, + save_diff_fn=patch_path, + sha=OLD_TRIMGALORE_SHA, + remote_url=GITLAB_URL, + branch=OLD_TRIMGALORE_BRANCH, + ) + + # Copy the module files and check that they are affected by the update + tmpdir = tempfile.mkdtemp() + trimgalore_tmpdir = os.path.join(tmpdir, "trimgalore") + trimgalore_path = os.path.join(self.pipeline_dir, "modules", GITLAB_REPO, "trimgalore") + shutil.copytree(trimgalore_path, trimgalore_tmpdir) + + assert update_obj.update("trimgalore") is True + assert cmp_module(trimgalore_tmpdir, trimgalore_path) is True + + # TODO: Apply the patch to the module + + +def test_update_all(self): + """Updates all modules present in the pipeline""" + update_obj = ModuleUpdate(self.pipeline_dir, update_all=True, show_diff=False) + # Get the current modules.json + assert update_obj.update() is True + + # We must reload the modules.json to get the updated version + mod_json_obj = ModulesJson(self.pipeline_dir) + mod_json = mod_json_obj.get_modules_json() + # Loop through all modules and check that they are updated (according to the modules.json file) + for mod in mod_json["repos"][NF_CORE_MODULES_REMOTE]["modules"][NF_CORE_MODULES_NAME]: + correct_git_sha = list(update_obj.modules_repo.get_component_git_log(mod, "modules", depth=1))[0]["git_sha"] + current_git_sha = mod_json["repos"][NF_CORE_MODULES_REMOTE]["modules"][NF_CORE_MODULES_NAME][mod]["git_sha"] + assert correct_git_sha == current_git_sha + + +def test_update_with_config_fixed_version(self): + """Try updating when there are entries in the .nf-core.yml""" + # Install trimgalore at the latest version + assert self.mods_install_trimgalore.install("trimgalore") + + # Fix the trimgalore version in the .nf-core.yml to an old version + update_config = {GITLAB_URL: {GITLAB_REPO: {"trimgalore": OLD_TRIMGALORE_SHA}}} + tools_config = nf_core.utils.load_tools_config(self.pipeline_dir) + tools_config["update"] = update_config + with open(os.path.join(self.pipeline_dir, ".nf-core.yml"), "w") as f: + yaml.dump(tools_config, f) + + # Update all modules in the pipeline + update_obj = ModuleUpdate( + self.pipeline_dir, update_all=True, show_diff=False, remote_url=GITLAB_URL, branch=OLD_TRIMGALORE_BRANCH + ) + assert update_obj.update() is True + + # Check that the git sha for trimgalore is correctly downgraded + mod_json = ModulesJson(self.pipeline_dir).get_modules_json() + assert "trimgalore" in mod_json["repos"][GITLAB_URL]["modules"][GITLAB_REPO] + assert "git_sha" in mod_json["repos"][GITLAB_URL]["modules"][GITLAB_REPO]["trimgalore"] + assert mod_json["repos"][GITLAB_URL]["modules"][GITLAB_REPO]["trimgalore"]["git_sha"] == OLD_TRIMGALORE_SHA + + +def test_update_with_config_dont_update(self): + """Try updating when module is to be ignored""" + # Install an old version of trimgalore + self.mods_install_old.install("trimgalore") + + # Set the trimgalore field to no update in the .nf-core.yml + update_config = {GITLAB_URL: {GITLAB_REPO: {"trimgalore": False}}} + tools_config = nf_core.utils.load_tools_config(self.pipeline_dir) + tools_config["update"] = update_config + with open(os.path.join(self.pipeline_dir, ".nf-core.yml"), "w") as f: + yaml.dump(tools_config, f) + + # Update all modules in the pipeline + update_obj = ModuleUpdate( + self.pipeline_dir, + update_all=True, + show_diff=False, + sha=OLD_TRIMGALORE_SHA, + remote_url=GITLAB_URL, + branch=OLD_TRIMGALORE_BRANCH, + ) + assert update_obj.update() is True + + # Check that the git sha for trimgalore is correctly downgraded + mod_json = ModulesJson(self.pipeline_dir).get_modules_json() + assert "trimgalore" in mod_json["repos"][GITLAB_URL]["modules"][GITLAB_REPO] + assert "git_sha" in mod_json["repos"][GITLAB_URL]["modules"][GITLAB_REPO]["trimgalore"] + assert mod_json["repos"][GITLAB_URL]["modules"][GITLAB_REPO]["trimgalore"]["git_sha"] == OLD_TRIMGALORE_SHA + + +def test_update_with_config_fix_all(self): + """Fix the version of all nf-core modules""" + self.mods_install_trimgalore.install("trimgalore") + + # Fix the version of all nf-core modules in the .nf-core.yml to an old version + update_config = {GITLAB_URL: OLD_TRIMGALORE_SHA} + tools_config = nf_core.utils.load_tools_config(self.pipeline_dir) + tools_config["update"] = update_config + with open(os.path.join(self.pipeline_dir, ".nf-core.yml"), "w") as f: + yaml.dump(tools_config, f) + + # Update all modules in the pipeline + update_obj = ModuleUpdate( + self.pipeline_dir, update_all=True, show_diff=False, remote_url=GITLAB_URL, branch=OLD_TRIMGALORE_BRANCH + ) + assert update_obj.update() is True + + # Check that the git sha for trimgalore is correctly downgraded + mod_json = ModulesJson(self.pipeline_dir).get_modules_json() + assert "git_sha" in mod_json["repos"][GITLAB_URL]["modules"][GITLAB_REPO]["trimgalore"] + assert mod_json["repos"][GITLAB_URL]["modules"][GITLAB_REPO]["trimgalore"]["git_sha"] == OLD_TRIMGALORE_SHA + + +def test_update_with_config_no_updates(self): + """Don't update any nf-core modules""" + self.mods_install_old.install("trimgalore") + old_mod_json = ModulesJson(self.pipeline_dir).get_modules_json() + + # Fix the version of all nf-core modules in the .nf-core.yml to an old version + update_config = {GITLAB_URL: False} + tools_config = nf_core.utils.load_tools_config(self.pipeline_dir) + tools_config["update"] = update_config + with open(os.path.join(self.pipeline_dir, ".nf-core.yml"), "w") as f: + yaml.dump(tools_config, f) + + # Update all modules in the pipeline + update_obj = ModuleUpdate( + self.pipeline_dir, + update_all=True, + show_diff=False, + sha=OLD_TRIMGALORE_SHA, + remote_url=GITLAB_URL, + branch=OLD_TRIMGALORE_BRANCH, + ) + assert update_obj.update() is True + + # Check that the git sha for trimgalore is correctly downgraded and none of the modules has changed + mod_json = ModulesJson(self.pipeline_dir).get_modules_json() + for module in mod_json["repos"][GITLAB_URL]["modules"][GITLAB_REPO]: + assert "git_sha" in mod_json["repos"][GITLAB_URL]["modules"][GITLAB_REPO][module] + assert ( + mod_json["repos"][GITLAB_URL]["modules"][GITLAB_REPO][module]["git_sha"] + == old_mod_json["repos"][GITLAB_URL]["modules"][GITLAB_REPO][module]["git_sha"] + ) + + +def test_update_different_branch_single_module(self): + """Try updating a module in a specific branch""" + install_obj = ModuleInstall( + self.pipeline_dir, + prompt=False, + force=False, + remote_url=GITLAB_URL, + branch=GITLAB_BRANCH_TEST_BRANCH, + sha=GITLAB_BRANCH_TEST_OLD_SHA, + ) + assert install_obj.install("fastp") + + update_obj = ModuleUpdate( + self.pipeline_dir, recursive=True, remote_url=GITLAB_URL, branch=GITLAB_BRANCH_TEST_BRANCH, show_diff=False + ) + update_obj.update("fastp") + + # Verify that the branch entry was updated correctly + modules_json = ModulesJson(self.pipeline_dir) + assert ( + modules_json.get_component_branch(self.component_type, "fastp", GITLAB_URL, GITLAB_REPO) + == GITLAB_BRANCH_TEST_BRANCH + ) + assert modules_json.get_module_version("fastp", GITLAB_URL, GITLAB_REPO) == GITLAB_BRANCH_TEST_NEW_SHA + + +def test_update_different_branch_mixed_modules_main(self): + """Try updating all modules where MultiQC is installed from main branch""" + # Install fastp + assert self.mods_install_gitlab_old.install("fastp") + + # Install MultiQC from gitlab default branch + assert self.mods_install_gitlab.install("multiqc") + + # Try updating + update_obj = ModuleUpdate(self.pipeline_dir, update_all=True, show_diff=False) + assert update_obj.update() is True + + modules_json = ModulesJson(self.pipeline_dir) + # Verify that the branch entry was updated correctly + assert ( + modules_json.get_component_branch(self.component_type, "fastp", GITLAB_URL, GITLAB_REPO) + == GITLAB_BRANCH_TEST_BRANCH + ) + assert modules_json.get_module_version("fastp", GITLAB_URL, GITLAB_REPO) == GITLAB_BRANCH_TEST_NEW_SHA + # MultiQC is present in both branches but should've been updated using the 'main' branch + assert ( + modules_json.get_component_branch(self.component_type, "multiqc", GITLAB_URL, GITLAB_REPO) + == GITLAB_DEFAULT_BRANCH + ) + + +def test_update_different_branch_mix_modules_branch_test(self): + """Try updating all modules where MultiQC is installed from branch-test branch""" + # Install multiqc from the branch-test branch + assert self.mods_install_gitlab_old.install( + "multiqc" + ) # Force as the same module is installed from github nf-core modules repo + modules_json = ModulesJson(self.pipeline_dir) + update_obj = ModuleUpdate( + self.pipeline_dir, + update_all=True, + show_diff=False, + remote_url=GITLAB_URL, + branch=GITLAB_BRANCH_TEST_BRANCH, + sha=GITLAB_BRANCH_TEST_NEW_SHA, + ) + assert update_obj.update() + + assert ( + modules_json.get_component_branch(self.component_type, "multiqc", GITLAB_URL, GITLAB_REPO) + == GITLAB_BRANCH_TEST_BRANCH + ) + assert modules_json.get_module_version("multiqc", GITLAB_URL, GITLAB_REPO) == GITLAB_BRANCH_TEST_NEW_SHA + + +def cmp_module(dir1, dir2): + """Compare two versions of the same module""" + files = ["main.nf", "meta.yml"] + return all(filecmp.cmp(os.path.join(dir1, f), os.path.join(dir2, f), shallow=False) for f in files) From 6a8af3d6dc357b7a10baaf5c519d8d8801b25460 Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Mon, 14 Nov 2022 13:03:20 +0100 Subject: [PATCH 16/31] fix typo and modules.json update when new component type --- nf_core/__main__.py | 4 ++-- nf_core/modules/modules_json.py | 30 +++++++++++++++++++++++------- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/nf_core/__main__.py b/nf_core/__main__.py index a42776d685..3d1c5cba91 100755 --- a/nf_core/__main__.py +++ b/nf_core/__main__.py @@ -557,7 +557,7 @@ def install(ctx, tool, dir, prompt, force, sha): @click.option( "-r", "--recursive", - is_flat=True, + is_flag=True, default=False, help="Automatically update all linked modules and subworkflows without asking for confirmation", ) @@ -1097,7 +1097,7 @@ def local(ctx, keywords, json, dir): # pylint: disable=redefined-builtin @click.option( "-r", "--recursive", - is_flat=True, + is_flag=True, default=False, help="Automatically update all linked modules and subworkflows without asking for confirmation", ) diff --git a/nf_core/modules/modules_json.py b/nf_core/modules/modules_json.py index f636ac562d..987045f43c 100644 --- a/nf_core/modules/modules_json.py +++ b/nf_core/modules/modules_json.py @@ -1042,11 +1042,27 @@ def components_with_repos(): if install_dir in install_directories ][0] repo_entry = self.determine_branches_and_shas(component_type, install_dir, remote_url, components) - if remote_url in self.modules_json["repos"]: + try: self.modules_json["repos"][remote_url][component_type][install_dir].update(repo_entry) - else: - self.modules_json["repos"][remote_url] = { - component_type: { - install_dir: repo_entry, - } - } + except KeyError: + try: + self.modules_json["repos"][remote_url][component_type].update({install_dir: repo_entry}) + except KeyError: + try: + self.modules_json["repos"][remote_url].update( + { + component_type: { + install_dir: repo_entry, + } + } + ) + except KeyError: + self.modules_json["repos"].update( + { + remote_url: { + component_type: { + install_dir: repo_entry, + } + } + } + ) From 7f69ac349d410797c9900740a47bb760f0ab1463 Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Mon, 14 Nov 2022 13:09:42 +0100 Subject: [PATCH 17/31] remove duplicated sw install objects --- tests/subworkflows/install.py | 32 ++++++++++++++++---------------- tests/test_subworkflows.py | 6 ------ 2 files changed, 16 insertions(+), 22 deletions(-) diff --git a/tests/subworkflows/install.py b/tests/subworkflows/install.py index 94b2d79ba0..f394ef4066 100644 --- a/tests/subworkflows/install.py +++ b/tests/subworkflows/install.py @@ -16,35 +16,35 @@ def test_subworkflow_install_nopipeline(self): """Test installing a subworkflow - no pipeline given""" - self.subworkflow_install.dir = None - assert self.subworkflow_install.install("foo") is False + self.sw_install.dir = None + assert self.sw_install.install("foo") is False @with_temporary_folder def test_subworkflows_install_emptypipeline(self, tmpdir): """Test installing a subworkflow - empty dir given""" os.mkdir(os.path.join(tmpdir, "nf-core-pipe")) - self.subworkflow_install.dir = os.path.join(tmpdir, "nf-core-pipe") + self.sw_install.dir = os.path.join(tmpdir, "nf-core-pipe") with pytest.raises(UserWarning) as excinfo: - self.subworkflow_install.install("foo") + self.sw_install.install("foo") assert "Could not find a 'main.nf' or 'nextflow.config' file" in str(excinfo.value) def test_subworkflows_install_nosubworkflow(self): """Test installing a subworkflow - unrecognised subworkflow given""" - assert self.subworkflow_install.install("foo") is False + assert self.sw_install.install("foo") is False def test_subworkflows_install_bam_sort_stats_samtools(self): """Test installing a subworkflow - bam_sort_stats_samtools""" - assert self.subworkflow_install.install("bam_sort_stats_samtools") is not False - subworkflow_path = os.path.join(self.subworkflow_install.dir, "subworkflows", "nf-core", "bam_sort_stats_samtools") - sub_subworkflow_path = os.path.join(self.subworkflow_install.dir, "subworkflows", "nf-core", "bam_stats_samtools") - samtools_index_path = os.path.join(self.subworkflow_install.dir, "modules", "nf-core", "samtools", "index") - samtools_sort_path = os.path.join(self.subworkflow_install.dir, "modules", "nf-core", "samtools", "sort") - samtools_stats_path = os.path.join(self.subworkflow_install.dir, "modules", "nf-core", "samtools", "stats") - samtools_idxstats_path = os.path.join(self.subworkflow_install.dir, "modules", "nf-core", "samtools", "idxstats") - samtools_flagstat_path = os.path.join(self.subworkflow_install.dir, "modules", "nf-core", "samtools", "flagstat") + assert self.sw_install.install("bam_sort_stats_samtools") is not False + subworkflow_path = os.path.join(self.sw_install.dir, "subworkflows", "nf-core", "bam_sort_stats_samtools") + sub_subworkflow_path = os.path.join(self.sw_install.dir, "subworkflows", "nf-core", "bam_stats_samtools") + samtools_index_path = os.path.join(self.sw_install.dir, "modules", "nf-core", "samtools", "index") + samtools_sort_path = os.path.join(self.sw_install.dir, "modules", "nf-core", "samtools", "sort") + samtools_stats_path = os.path.join(self.sw_install.dir, "modules", "nf-core", "samtools", "stats") + samtools_idxstats_path = os.path.join(self.sw_install.dir, "modules", "nf-core", "samtools", "idxstats") + samtools_flagstat_path = os.path.join(self.sw_install.dir, "modules", "nf-core", "samtools", "flagstat") assert os.path.exists(subworkflow_path) assert os.path.exists(sub_subworkflow_path) assert os.path.exists(samtools_index_path) @@ -56,13 +56,13 @@ def test_subworkflows_install_bam_sort_stats_samtools(self): def test_subworkflows_install_bam_sort_stats_samtools_twice(self): """Test installing a subworkflow - bam_sort_stats_samtools already there""" - self.subworkflow_install.install("bam_sort_stats_samtools") - assert self.subworkflow_install.install("bam_sort_stats_samtools") is False + self.sw_install.install("bam_sort_stats_samtools") + assert self.sw_install.install("bam_sort_stats_samtools") is False def test_subworkflows_install_from_gitlab(self): """Test installing a subworkflow from GitLab""" - assert self.subworkflow_install_gitlab.install("bam_stats_samtools") is True + assert self.sw_install_gitlab.install("bam_stats_samtools") is True # Verify that the branch entry was added correctly modules_json = ModulesJson(self.pipeline_dir) assert ( diff --git a/tests/test_subworkflows.py b/tests/test_subworkflows.py index be15e2f15b..5f2b0440a3 100644 --- a/tests/test_subworkflows.py +++ b/tests/test_subworkflows.py @@ -52,12 +52,6 @@ def setUp(self): "mypipeline", "it is mine", "me", no_git=True, outdir=self.pipeline_dir, plain=True ).init_pipeline() - # Set up install objects - self.subworkflow_install = nf_core.subworkflows.SubworkflowInstall(self.pipeline_dir, prompt=False, force=False) - self.subworkflow_install_gitlab = nf_core.subworkflows.SubworkflowInstall( - self.pipeline_dir, prompt=False, force=False, remote_url=GITLAB_URL, branch=GITLAB_SUBWORKFLOWS_BRANCH - ) - # Set up the nf-core/modules repo dummy self.nfcore_modules = create_modules_repo_dummy(self.tmp_dir) From 4fd9b8cef09e46d10689a153561d0fc751790fd5 Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Mon, 14 Nov 2022 15:55:33 +0100 Subject: [PATCH 18/31] add tests for subworkflows update --- nf_core/components/update.py | 5 +- tests/subworkflows/update.py | 385 ++++++++++++++++++----------------- tests/test_subworkflows.py | 25 ++- tests/utils.py | 2 + 4 files changed, 227 insertions(+), 190 deletions(-) diff --git a/nf_core/components/update.py b/nf_core/components/update.py index 4648a31875..178f2eb25c 100644 --- a/nf_core/components/update.py +++ b/nf_core/components/update.py @@ -113,6 +113,7 @@ def update(self, component=None, silent=False, updated=None, check_diff_exist=Tr return False # Get the list of modules/subworkflows to update, and their version information + print(self.update_all) components_info = ( self.get_all_components_info() if self.update_all else [self.get_single_component_info(component)] ) @@ -709,7 +710,9 @@ def move_files_from_tmp_dir(self, component, install_folder, repo_path, new_vers log.info(f"Updating '{repo_path}/{component}'") log.debug(f"Updating {self.component_type[:-1]} '{component}' to {new_version} from {repo_path}") - def try_apply_patch(self, component, repo_path, patch_relpath, component_dir, component_install_dir, write_file=True): + def try_apply_patch( + self, component, repo_path, patch_relpath, component_dir, component_install_dir, write_file=True + ): """ Try applying a patch file to the new module/subworkflow files diff --git a/tests/subworkflows/update.py b/tests/subworkflows/update.py index 60f869ae4c..182ed00d19 100644 --- a/tests/subworkflows/update.py +++ b/tests/subworkflows/update.py @@ -1,296 +1,305 @@ import filecmp -import os import shutil import tempfile +from pathlib import Path import yaml import nf_core.utils -from nf_core.modules.install import ModuleInstall from nf_core.modules.modules_json import ModulesJson from nf_core.modules.modules_repo import NF_CORE_MODULES_NAME, NF_CORE_MODULES_REMOTE from nf_core.modules.update import ModuleUpdate +from nf_core.subworkflows.update import SubworkflowUpdate -from ..utils import ( - GITLAB_BRANCH_TEST_BRANCH, - GITLAB_BRANCH_TEST_NEW_SHA, - GITLAB_BRANCH_TEST_OLD_SHA, - GITLAB_DEFAULT_BRANCH, - GITLAB_REPO, - GITLAB_URL, - OLD_TRIMGALORE_BRANCH, - OLD_TRIMGALORE_SHA, -) +from ..utils import OLD_SUBWORKFLOWS_SHA def test_install_and_update(self): - """Installs a module in the pipeline and updates it (no change)""" - self.mods_install.install("trimgalore") - update_obj = ModuleUpdate(self.pipeline_dir, show_diff=False) + """Installs a subworkflow in the pipeline and updates it (no change)""" + self.sw_install.install("bam_stats_samtools") + update_obj = SubworkflowUpdate(self.pipeline_dir, show_diff=False) - # Copy the module files and check that they are unaffected by the update + # Copy the sw files and check that they are unaffected by the update tmpdir = tempfile.mkdtemp() - trimgalore_tmpdir = os.path.join(tmpdir, "trimgalore") - trimgalore_path = os.path.join(self.pipeline_dir, "modules", NF_CORE_MODULES_NAME, "trimgalore") - shutil.copytree(trimgalore_path, trimgalore_tmpdir) + shutil.rmtree(tmpdir) + sw_path = Path(self.pipeline_dir, "subworkflows", NF_CORE_MODULES_NAME, "bam_stats_samtools") + shutil.copytree(sw_path, tmpdir) - assert update_obj.update("trimgalore") is True - assert cmp_module(trimgalore_tmpdir, trimgalore_path) is True + assert update_obj.update("bam_stats_samtools") is True + assert cmp_component(tmpdir, sw_path) is True def test_install_at_hash_and_update(self): - """Installs an old version of a module in the pipeline and updates it""" - assert self.mods_install_old.install("trimgalore") - update_obj = ModuleUpdate( - self.pipeline_dir, show_diff=False, recursive=True, remote_url=GITLAB_URL, branch=OLD_TRIMGALORE_BRANCH - ) + """Installs an old version of a subworkflow in the pipeline and updates it""" + assert self.sw_install_old.install("fastq_align_bowtie2") + update_obj = SubworkflowUpdate(self.pipeline_dir, show_diff=False, recursive=True) + old_mod_json = ModulesJson(self.pipeline_dir).get_modules_json() + print(old_mod_json) - # Copy the module files and check that they are affected by the update + # Copy the sw files and check that they are affected by the update tmpdir = tempfile.mkdtemp() - trimgalore_tmpdir = os.path.join(tmpdir, "trimgalore") - trimgalore_path = os.path.join(self.pipeline_dir, "modules", GITLAB_REPO, "trimgalore") - shutil.copytree(trimgalore_path, trimgalore_tmpdir) + shutil.rmtree(tmpdir) + sw_path = Path(self.pipeline_dir, "subworkflows", NF_CORE_MODULES_NAME, "fastq_align_bowtie2") + shutil.copytree(sw_path, tmpdir) - assert update_obj.update("trimgalore") is True - assert cmp_module(trimgalore_tmpdir, trimgalore_path) is False + assert update_obj.update("fastq_align_bowtie2") is True + assert cmp_component(tmpdir, sw_path) is False # Check that the modules.json is correctly updated - mod_json_obj = ModulesJson(self.pipeline_dir) - mod_json = mod_json_obj.get_modules_json() - # Get the up-to-date git_sha for the module from the ModulesRepo object - correct_git_sha = update_obj.modules_repo.get_latest_component_version("trimgalore", "modules") - current_git_sha = mod_json["repos"][GITLAB_URL]["modules"][GITLAB_REPO]["trimgalore"]["git_sha"] - assert correct_git_sha == current_git_sha + mod_json = ModulesJson(self.pipeline_dir).get_modules_json() + print(mod_json) + # Get the up-to-date git_sha for the sw from the ModulesRepo object + assert ( + old_mod_json["repos"][NF_CORE_MODULES_REMOTE]["subworkflows"][NF_CORE_MODULES_NAME]["fastq_align_bowtie2"][ + "git_sha" + ] + != mod_json["repos"][NF_CORE_MODULES_REMOTE]["subworkflows"][NF_CORE_MODULES_NAME]["fastq_align_bowtie2"][ + "git_sha" + ] + ) 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""" - self.mods_install_old.install("trimgalore") - patch_path = os.path.join(self.pipeline_dir, "trimgalore.patch") - update_obj = ModuleUpdate( - self.pipeline_dir, - save_diff_fn=patch_path, - sha=OLD_TRIMGALORE_SHA, - remote_url=GITLAB_URL, - branch=OLD_TRIMGALORE_BRANCH, - ) + """Installs an old version of a sw in the pipeline and updates it. Save differences to a file.""" + assert self.sw_install_old.install("fastq_align_bowtie2") + patch_path = Path(self.pipeline_dir, "fastq_align_bowtie2.patch") + update_obj = SubworkflowUpdate(self.pipeline_dir, save_diff_fn=patch_path, recursive=True) - # Copy the module files and check that they are affected by the update + # Copy the sw files and check that they are affected by the update tmpdir = tempfile.mkdtemp() - trimgalore_tmpdir = os.path.join(tmpdir, "trimgalore") - trimgalore_path = os.path.join(self.pipeline_dir, "modules", GITLAB_REPO, "trimgalore") - shutil.copytree(trimgalore_path, trimgalore_tmpdir) + shutil.rmtree(tmpdir) + sw_path = Path(self.pipeline_dir, "subworkflows", NF_CORE_MODULES_NAME, "fastq_align_bowtie2") + shutil.copytree(sw_path, tmpdir) - assert update_obj.update("trimgalore") is True - assert cmp_module(trimgalore_tmpdir, trimgalore_path) is True + assert update_obj.update("fastq_align_bowtie2") is True + assert cmp_component(tmpdir, sw_path) is True - # TODO: Apply the patch to the module + with open(patch_path, "r") as fh: + line = fh.readline() + assert line.startswith( + "Changes in module 'nf-core/fastq_align_bowtie2' between (f3c078809a2513f1c95de14f6633fe1f03572fdb) and" + ) def test_update_all(self): - """Updates all modules present in the pipeline""" - update_obj = ModuleUpdate(self.pipeline_dir, update_all=True, show_diff=False) - # Get the current modules.json + """Updates all subworkflows present in the pipeline""" + # Install subworkflows fastq_align_bowtie2, bam_sort_stats_samtools, bam_stats_samtools + self.sw_install.install("fastq_align_bowtie2") + # Update all subworkflows + update_obj = SubworkflowUpdate(self.pipeline_dir, update_all=True, show_diff=False) assert update_obj.update() is True # We must reload the modules.json to get the updated version mod_json_obj = ModulesJson(self.pipeline_dir) mod_json = mod_json_obj.get_modules_json() - # Loop through all modules and check that they are updated (according to the modules.json file) - for mod in mod_json["repos"][NF_CORE_MODULES_REMOTE]["modules"][NF_CORE_MODULES_NAME]: - correct_git_sha = list(update_obj.modules_repo.get_component_git_log(mod, "modules", depth=1))[0]["git_sha"] - current_git_sha = mod_json["repos"][NF_CORE_MODULES_REMOTE]["modules"][NF_CORE_MODULES_NAME][mod]["git_sha"] + print(mod_json) + # Loop through all subworkflows and check that they are updated (according to the modules.json file) + for sw in mod_json["repos"][NF_CORE_MODULES_REMOTE]["subworkflows"][NF_CORE_MODULES_NAME]: + correct_git_sha = list(update_obj.modules_repo.get_component_git_log(sw, "subworkflows", depth=1))[0]["git_sha"] + current_git_sha = mod_json["repos"][NF_CORE_MODULES_REMOTE]["subworkflows"][NF_CORE_MODULES_NAME][sw]["git_sha"] assert correct_git_sha == current_git_sha def test_update_with_config_fixed_version(self): """Try updating when there are entries in the .nf-core.yml""" - # Install trimgalore at the latest version - assert self.mods_install_trimgalore.install("trimgalore") + # Install subworkflow at the latest version + assert self.sw_install.install("fastq_align_bowtie2") - # Fix the trimgalore version in the .nf-core.yml to an old version - update_config = {GITLAB_URL: {GITLAB_REPO: {"trimgalore": OLD_TRIMGALORE_SHA}}} + # Fix the subworkflow version in the .nf-core.yml to an old version + update_config = {NF_CORE_MODULES_REMOTE: {NF_CORE_MODULES_NAME: {"fastq_align_bowtie2": OLD_SUBWORKFLOWS_SHA}}} tools_config = nf_core.utils.load_tools_config(self.pipeline_dir) tools_config["update"] = update_config - with open(os.path.join(self.pipeline_dir, ".nf-core.yml"), "w") as f: + with open(Path(self.pipeline_dir, ".nf-core.yml"), "w") as f: yaml.dump(tools_config, f) - # Update all modules in the pipeline - update_obj = ModuleUpdate( - self.pipeline_dir, update_all=True, show_diff=False, remote_url=GITLAB_URL, branch=OLD_TRIMGALORE_BRANCH - ) + # Update all subworkflows in the pipeline + update_obj = SubworkflowUpdate(self.pipeline_dir, update_all=True, show_diff=False) assert update_obj.update() is True - # Check that the git sha for trimgalore is correctly downgraded + # Check that the git sha for fastq_align_bowtie2 is correctly downgraded mod_json = ModulesJson(self.pipeline_dir).get_modules_json() - assert "trimgalore" in mod_json["repos"][GITLAB_URL]["modules"][GITLAB_REPO] - assert "git_sha" in mod_json["repos"][GITLAB_URL]["modules"][GITLAB_REPO]["trimgalore"] - assert mod_json["repos"][GITLAB_URL]["modules"][GITLAB_REPO]["trimgalore"]["git_sha"] == OLD_TRIMGALORE_SHA + assert "fastq_align_bowtie2" in mod_json["repos"][NF_CORE_MODULES_REMOTE]["subworkflows"][NF_CORE_MODULES_NAME] + assert ( + "git_sha" + in mod_json["repos"][NF_CORE_MODULES_REMOTE]["subworkflows"][NF_CORE_MODULES_NAME]["fastq_align_bowtie2"] + ) + assert ( + mod_json["repos"][NF_CORE_MODULES_REMOTE]["subworkflows"][NF_CORE_MODULES_NAME]["fastq_align_bowtie2"][ + "git_sha" + ] + == OLD_SUBWORKFLOWS_SHA + ) def test_update_with_config_dont_update(self): - """Try updating when module is to be ignored""" - # Install an old version of trimgalore - self.mods_install_old.install("trimgalore") + """Try updating when sw is to be ignored""" + # Install an old version of fastq_align_bowtie2 + self.sw_install_old.install("fastq_align_bowtie2") - # Set the trimgalore field to no update in the .nf-core.yml - update_config = {GITLAB_URL: {GITLAB_REPO: {"trimgalore": False}}} + # Set the fastq_align_bowtie2 field to no update in the .nf-core.yml + update_config = {NF_CORE_MODULES_REMOTE: {NF_CORE_MODULES_NAME: {"fastq_align_bowtie2": False}}} tools_config = nf_core.utils.load_tools_config(self.pipeline_dir) tools_config["update"] = update_config - with open(os.path.join(self.pipeline_dir, ".nf-core.yml"), "w") as f: + with open(Path(self.pipeline_dir, ".nf-core.yml"), "w") as f: yaml.dump(tools_config, f) # Update all modules in the pipeline - update_obj = ModuleUpdate( - self.pipeline_dir, - update_all=True, - show_diff=False, - sha=OLD_TRIMGALORE_SHA, - remote_url=GITLAB_URL, - branch=OLD_TRIMGALORE_BRANCH, - ) + update_obj = SubworkflowUpdate(self.pipeline_dir, update_all=True, show_diff=False) assert update_obj.update() is True - # Check that the git sha for trimgalore is correctly downgraded + # Check that the git sha for fastq_align_bowtie2 is correctly downgraded mod_json = ModulesJson(self.pipeline_dir).get_modules_json() - assert "trimgalore" in mod_json["repos"][GITLAB_URL]["modules"][GITLAB_REPO] - assert "git_sha" in mod_json["repos"][GITLAB_URL]["modules"][GITLAB_REPO]["trimgalore"] - assert mod_json["repos"][GITLAB_URL]["modules"][GITLAB_REPO]["trimgalore"]["git_sha"] == OLD_TRIMGALORE_SHA + assert "fastq_align_bowtie2" in mod_json["repos"][NF_CORE_MODULES_REMOTE]["subworkflows"][NF_CORE_MODULES_NAME] + assert ( + "git_sha" + in mod_json["repos"][NF_CORE_MODULES_REMOTE]["subworkflows"][NF_CORE_MODULES_NAME]["fastq_align_bowtie2"] + ) + assert ( + mod_json["repos"][NF_CORE_MODULES_REMOTE]["subworkflows"][NF_CORE_MODULES_NAME]["fastq_align_bowtie2"][ + "git_sha" + ] + == OLD_SUBWORKFLOWS_SHA + ) def test_update_with_config_fix_all(self): - """Fix the version of all nf-core modules""" - self.mods_install_trimgalore.install("trimgalore") + """Fix the version of all nf-core subworkflows""" + # Install subworkflow at the latest version + assert self.sw_install.install("fastq_align_bowtie2") - # Fix the version of all nf-core modules in the .nf-core.yml to an old version - update_config = {GITLAB_URL: OLD_TRIMGALORE_SHA} + # Fix the version of all nf-core subworkflows in the .nf-core.yml to an old version + update_config = {NF_CORE_MODULES_REMOTE: OLD_SUBWORKFLOWS_SHA} tools_config = nf_core.utils.load_tools_config(self.pipeline_dir) tools_config["update"] = update_config - with open(os.path.join(self.pipeline_dir, ".nf-core.yml"), "w") as f: + with open(Path(self.pipeline_dir, ".nf-core.yml"), "w") as f: yaml.dump(tools_config, f) - # Update all modules in the pipeline - update_obj = ModuleUpdate( - self.pipeline_dir, update_all=True, show_diff=False, remote_url=GITLAB_URL, branch=OLD_TRIMGALORE_BRANCH - ) + # Update all subworkflows in the pipeline + update_obj = SubworkflowUpdate(self.pipeline_dir, update_all=True, show_diff=False) assert update_obj.update() is True - # Check that the git sha for trimgalore is correctly downgraded + # Check that the git sha for fastq_align_bowtie2 is correctly downgraded mod_json = ModulesJson(self.pipeline_dir).get_modules_json() - assert "git_sha" in mod_json["repos"][GITLAB_URL]["modules"][GITLAB_REPO]["trimgalore"] - assert mod_json["repos"][GITLAB_URL]["modules"][GITLAB_REPO]["trimgalore"]["git_sha"] == OLD_TRIMGALORE_SHA + assert ( + "git_sha" + in mod_json["repos"][NF_CORE_MODULES_REMOTE]["subworkflows"][NF_CORE_MODULES_NAME]["fastq_align_bowtie2"] + ) + assert ( + mod_json["repos"][NF_CORE_MODULES_REMOTE]["subworkflows"][NF_CORE_MODULES_NAME]["fastq_align_bowtie2"][ + "git_sha" + ] + == OLD_SUBWORKFLOWS_SHA + ) def test_update_with_config_no_updates(self): - """Don't update any nf-core modules""" - self.mods_install_old.install("trimgalore") + """Don't update any nf-core subworkflows""" + # Install an old version of fastq_align_bowtie2 + self.sw_install_old.install("fastq_align_bowtie2") old_mod_json = ModulesJson(self.pipeline_dir).get_modules_json() - # Fix the version of all nf-core modules in the .nf-core.yml to an old version - update_config = {GITLAB_URL: False} + # Set all repository updates to False + update_config = {NF_CORE_MODULES_REMOTE: False} tools_config = nf_core.utils.load_tools_config(self.pipeline_dir) tools_config["update"] = update_config - with open(os.path.join(self.pipeline_dir, ".nf-core.yml"), "w") as f: + with open(Path(self.pipeline_dir, ".nf-core.yml"), "w") as f: yaml.dump(tools_config, f) - # Update all modules in the pipeline - update_obj = ModuleUpdate( - self.pipeline_dir, - update_all=True, - show_diff=False, - sha=OLD_TRIMGALORE_SHA, - remote_url=GITLAB_URL, - branch=OLD_TRIMGALORE_BRANCH, - ) + # Update all subworkflows in the pipeline + update_obj = SubworkflowUpdate(self.pipeline_dir, update_all=True, show_diff=False) assert update_obj.update() is True - # Check that the git sha for trimgalore is correctly downgraded and none of the modules has changed + # Check that the git sha for fastq_align_bowtie2 is correctly downgraded and none of the subworkflows has changed mod_json = ModulesJson(self.pipeline_dir).get_modules_json() - for module in mod_json["repos"][GITLAB_URL]["modules"][GITLAB_REPO]: - assert "git_sha" in mod_json["repos"][GITLAB_URL]["modules"][GITLAB_REPO][module] + for sw in mod_json["repos"][NF_CORE_MODULES_REMOTE]["subworkflows"][NF_CORE_MODULES_NAME]: + assert "git_sha" in mod_json["repos"][NF_CORE_MODULES_REMOTE]["subworkflows"][NF_CORE_MODULES_NAME][sw] assert ( - mod_json["repos"][GITLAB_URL]["modules"][GITLAB_REPO][module]["git_sha"] - == old_mod_json["repos"][GITLAB_URL]["modules"][GITLAB_REPO][module]["git_sha"] + mod_json["repos"][NF_CORE_MODULES_REMOTE]["subworkflows"][NF_CORE_MODULES_NAME][sw]["git_sha"] + == old_mod_json["repos"][NF_CORE_MODULES_REMOTE]["subworkflows"][NF_CORE_MODULES_NAME][sw]["git_sha"] ) -def test_update_different_branch_single_module(self): - """Try updating a module in a specific branch""" - install_obj = ModuleInstall( - self.pipeline_dir, - prompt=False, - force=False, - remote_url=GITLAB_URL, - branch=GITLAB_BRANCH_TEST_BRANCH, - sha=GITLAB_BRANCH_TEST_OLD_SHA, - ) - assert install_obj.install("fastp") +def test_update_all_linked_components_from_subworkflow(self): + """Update a subworkflow and all modules and subworkflows used on it""" + # Install an old version of fastq_align_bowtie2 + self.sw_install_old.install("fastq_align_bowtie2") + old_mod_json = ModulesJson(self.pipeline_dir).get_modules_json() - update_obj = ModuleUpdate( - self.pipeline_dir, recursive=True, remote_url=GITLAB_URL, branch=GITLAB_BRANCH_TEST_BRANCH, show_diff=False - ) - update_obj.update("fastp") + # Copy the sw files and check that they are affected by the update + tmpdir = tempfile.mkdtemp() + shutil.rmtree(tmpdir) + subworkflows_path = Path(self.pipeline_dir, "subworkflows", NF_CORE_MODULES_NAME) + modules_path = Path(self.pipeline_dir, "modules", NF_CORE_MODULES_NAME) + shutil.copytree(subworkflows_path, Path(tmpdir, "subworkflows")) + shutil.copytree(modules_path, Path(tmpdir, "modules")) + + # Update fastq_align_bowtie2 and all modules and subworkflows used by that + update_obj = SubworkflowUpdate(self.pipeline_dir, recursive=True, show_diff=False) + assert update_obj.update("fastq_align_bowtie2") is True - # Verify that the branch entry was updated correctly - modules_json = ModulesJson(self.pipeline_dir) + mod_json = ModulesJson(self.pipeline_dir).get_modules_json() + # Loop through all modules and subworkflows used in fastq_align_bowtie2 + # check that they are updated (according to the modules.json file) + for sw in ["fastq_align_bowtie2", "bam_sort_stats_samtools", "bam_stats_samtools"]: + assert ( + old_mod_json["repos"][NF_CORE_MODULES_REMOTE]["subworkflows"][NF_CORE_MODULES_NAME][sw]["git_sha"] + != mod_json["repos"][NF_CORE_MODULES_REMOTE]["subworkflows"][NF_CORE_MODULES_NAME][sw]["git_sha"] + ) + for mod in [ + "bowtie2/align", + "samtools/index", + "samtools/sort", + "samtools/flagstat", + "samtools/idxstats", + "samtools/stats", + ]: + assert ( + old_mod_json["repos"][NF_CORE_MODULES_REMOTE]["modules"][NF_CORE_MODULES_NAME][mod]["git_sha"] + != mod_json["repos"][NF_CORE_MODULES_REMOTE]["modules"][NF_CORE_MODULES_NAME][mod]["git_sha"] + ) + # Check that the subworkflow files are updated assert ( - modules_json.get_component_branch(self.component_type, "fastp", GITLAB_URL, GITLAB_REPO) - == GITLAB_BRANCH_TEST_BRANCH + cmp_component( + Path(tmpdir, "subworkflows", "fastq_align_bowtie2"), Path(subworkflows_path, "fastq_align_bowtie2") + ) + is False ) - assert modules_json.get_module_version("fastp", GITLAB_URL, GITLAB_REPO) == GITLAB_BRANCH_TEST_NEW_SHA -def test_update_different_branch_mixed_modules_main(self): - """Try updating all modules where MultiQC is installed from main branch""" - # Install fastp - assert self.mods_install_gitlab_old.install("fastp") +def test_update_all_subworkflows_from_module(self): + """Update a module and all subworkflows that use this module""" + # Install an old version of fastq_align_bowtie2 and thus all modules used by it (bowtie2/align) + self.sw_install_old.install("fastq_align_bowtie2") + old_mod_json = ModulesJson(self.pipeline_dir).get_modules_json() - # Install MultiQC from gitlab default branch - assert self.mods_install_gitlab.install("multiqc") + # Copy the sw files and check that they are affected by the update + tmpdir = tempfile.mkdtemp() + shutil.rmtree(tmpdir) + sw_path = Path(self.pipeline_dir, "subworkflows", NF_CORE_MODULES_NAME, "fastq_align_bowtie2") + shutil.copytree(sw_path, Path(tmpdir, "fastq_align_bowtie2")) - # Try updating - update_obj = ModuleUpdate(self.pipeline_dir, update_all=True, show_diff=False) - assert update_obj.update() is True + # Update bowtie2/align and all subworkflows using it + update_obj = ModuleUpdate(self.pipeline_dir, recursive=True, show_diff=False) + assert update_obj.update("bowtie2/align") is True - modules_json = ModulesJson(self.pipeline_dir) - # Verify that the branch entry was updated correctly - assert ( - modules_json.get_component_branch(self.component_type, "fastp", GITLAB_URL, GITLAB_REPO) - == GITLAB_BRANCH_TEST_BRANCH - ) - assert modules_json.get_module_version("fastp", GITLAB_URL, GITLAB_REPO) == GITLAB_BRANCH_TEST_NEW_SHA - # MultiQC is present in both branches but should've been updated using the 'main' branch + mod_json = ModulesJson(self.pipeline_dir).get_modules_json() + # Check that bowtie2/align and fastq_align_bowtie2 are updated assert ( - modules_json.get_component_branch(self.component_type, "multiqc", GITLAB_URL, GITLAB_REPO) - == GITLAB_DEFAULT_BRANCH - ) - - -def test_update_different_branch_mix_modules_branch_test(self): - """Try updating all modules where MultiQC is installed from branch-test branch""" - # Install multiqc from the branch-test branch - assert self.mods_install_gitlab_old.install( - "multiqc" - ) # Force as the same module is installed from github nf-core modules repo - modules_json = ModulesJson(self.pipeline_dir) - update_obj = ModuleUpdate( - self.pipeline_dir, - update_all=True, - show_diff=False, - remote_url=GITLAB_URL, - branch=GITLAB_BRANCH_TEST_BRANCH, - sha=GITLAB_BRANCH_TEST_NEW_SHA, + old_mod_json["repos"][NF_CORE_MODULES_REMOTE]["subworkflows"][NF_CORE_MODULES_NAME]["fastq_align_bowtie2"][ + "git_sha" + ] + != mod_json["repos"][NF_CORE_MODULES_REMOTE]["subworkflows"][NF_CORE_MODULES_NAME]["fastq_align_bowtie2"][ + "git_sha" + ] ) - assert update_obj.update() - + assert cmp_component(Path(tmpdir, "fastq_align_bowtie2"), sw_path) is False assert ( - modules_json.get_component_branch(self.component_type, "multiqc", GITLAB_URL, GITLAB_REPO) - == GITLAB_BRANCH_TEST_BRANCH + old_mod_json["repos"][NF_CORE_MODULES_REMOTE]["modules"][NF_CORE_MODULES_NAME]["bowtie2/align"]["git_sha"] + != mod_json["repos"][NF_CORE_MODULES_REMOTE]["modules"][NF_CORE_MODULES_NAME]["bowtie2/align"]["git_sha"] ) - assert modules_json.get_module_version("multiqc", GITLAB_URL, GITLAB_REPO) == GITLAB_BRANCH_TEST_NEW_SHA -def cmp_module(dir1, dir2): - """Compare two versions of the same module""" +def cmp_component(dir1, dir2): + """Compare two versions of the same component""" files = ["main.nf", "meta.yml"] - return all(filecmp.cmp(os.path.join(dir1, f), os.path.join(dir2, f), shallow=False) for f in files) + return all(filecmp.cmp(Path(dir1, f), Path(dir2, f), shallow=False) for f in files) diff --git a/tests/test_subworkflows.py b/tests/test_subworkflows.py index 5f2b0440a3..abdd550ecb 100644 --- a/tests/test_subworkflows.py +++ b/tests/test_subworkflows.py @@ -11,7 +11,12 @@ import nf_core.modules import nf_core.subworkflows -from .utils import GITLAB_SUBWORKFLOWS_BRANCH, GITLAB_URL, mock_api_calls +from .utils import ( + GITLAB_SUBWORKFLOWS_BRANCH, + GITLAB_URL, + OLD_SUBWORKFLOWS_SHA, + mock_api_calls, +) def create_modules_repo_dummy(tmp_dir): @@ -60,6 +65,12 @@ def setUp(self): self.sw_install_gitlab = nf_core.subworkflows.SubworkflowInstall( self.pipeline_dir, prompt=False, force=False, remote_url=GITLAB_URL, branch=GITLAB_SUBWORKFLOWS_BRANCH ) + self.sw_install_old = nf_core.subworkflows.SubworkflowInstall( + self.pipeline_dir, + prompt=False, + force=False, + sha=OLD_SUBWORKFLOWS_SHA, + ) ############################################ # Test of the individual modules commands. # @@ -90,3 +101,15 @@ def setUp(self): test_subworkflows_test_no_installed_subworkflows, test_subworkflows_test_no_name_no_prompts, ) + from .subworkflows.update import ( + test_install_and_update, + test_install_at_hash_and_update, + test_install_at_hash_and_update_and_save_diff_to_file, + test_update_all, + test_update_all_linked_components_from_subworkflow, + test_update_all_subworkflows_from_module, + test_update_with_config_dont_update, + test_update_with_config_fix_all, + test_update_with_config_fixed_version, + test_update_with_config_no_updates, + ) diff --git a/tests/utils.py b/tests/utils.py index 65c7d48758..abe1626496 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -16,12 +16,14 @@ GITLAB_REPO = "nf-core" GITLAB_DEFAULT_BRANCH = "main-restructure" GITLAB_SUBWORKFLOWS_BRANCH = "subworkflows" +OLD_SUBWORKFLOWS_SHA = "f3c078809a2513f1c95de14f6633fe1f03572fdb" # Branch test stuff GITLAB_BRANCH_TEST_BRANCH = "branch-tester-restructure" GITLAB_BRANCH_TEST_OLD_SHA = "bce3f17980b8d1beae5e917cfd3c65c0c69e04b5" GITLAB_BRANCH_TEST_NEW_SHA = "2f5f180f6e705bb81d6e7742dc2f24bf4a0c721e" + def with_temporary_folder(func): """ Call the decorated funtion under the tempfile.TemporaryDirectory From 30f2c07c8d9ef4c7f4e24ee1e0e30f7cb6c1aeed Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Mon, 14 Nov 2022 16:35:11 +0100 Subject: [PATCH 19/31] all subworkflows tests pass --- nf_core/components/update.py | 43 +++++++++++++++++++----------------- tests/modules/update.py | 1 - tests/subworkflows/update.py | 3 --- 3 files changed, 23 insertions(+), 24 deletions(-) diff --git a/nf_core/components/update.py b/nf_core/components/update.py index 178f2eb25c..d59f94610e 100644 --- a/nf_core/components/update.py +++ b/nf_core/components/update.py @@ -113,7 +113,6 @@ def update(self, component=None, silent=False, updated=None, check_diff_exist=Tr return False # Get the list of modules/subworkflows to update, and their version information - print(self.update_all) components_info = ( self.get_all_components_info() if self.update_all else [self.get_single_component_info(component)] ) @@ -364,28 +363,32 @@ def get_single_component_info(self, component): ) sha = self.sha - if component in self.update_config.get(self.modules_repo.remote_url, {}).get(install_dir, {}): + config_entry = None + if isinstance(self.update_config.get(self.modules_repo.remote_url, {}), str): + # If the repo entry is a string, it's the sha to update to + config_entry = self.update_config.get(self.modules_repo.remote_url, {}) + elif component in self.update_config.get(self.modules_repo.remote_url, {}).get(install_dir, {}): # If the component to update is in .nf-core.yml config file config_entry = self.update_config[self.modules_repo.remote_url][install_dir].get(component) - if config_entry is not None and config_entry is not True: - if config_entry is False: - raise UserWarning( - f"{self.component_type[:-1].title()}'s update entry in '.nf-core.yml' is set to False" - ) - if not isinstance(config_entry, str): - raise UserWarning( - f"{self.component_type[:-1].title()}'s update entry in '.nf-core.yml' is of wrong type" - ) + if config_entry is not None and config_entry is not True: + if config_entry is False: + raise UserWarning( + f"{self.component_type[:-1].title()}'s update entry in '.nf-core.yml' is set to False" + ) + if not isinstance(config_entry, str): + raise UserWarning( + f"{self.component_type[:-1].title()}'s update entry in '.nf-core.yml' is of wrong type" + ) - sha = config_entry - if self.sha is not None: - log.warning( - f"Found entry in '.nf-core.yml' for {self.component_type[:-1]} '{component}' " - "which will override version specified with '--sha'" - ) - else: - log.info(f"Found entry in '.nf-core.yml' for {self.component_type[:-1]} '{component}'") - log.info(f"Updating component to ({sha})") + sha = config_entry + if self.sha is not None: + log.warning( + f"Found entry in '.nf-core.yml' for {self.component_type[:-1]} '{component}' " + "which will override version specified with '--sha'" + ) + else: + log.info(f"Found entry in '.nf-core.yml' for {self.component_type[:-1]} '{component}'") + log.info(f"Updating component to ({sha})") # Check if the update branch is the same as the installation branch current_branch = self.modules_json.get_component_branch( diff --git a/tests/modules/update.py b/tests/modules/update.py index 2620aa5930..a4121f917c 100644 --- a/tests/modules/update.py +++ b/tests/modules/update.py @@ -357,7 +357,6 @@ def test_update_only_show_differences_when_patch(self, mock_prompt): for mod in ["custom/dumpsoftwareversions", "fastqc"]: correct_git_sha = list(update_obj.modules_repo.get_component_git_log(mod, "modules", depth=1))[0]["git_sha"] current_git_sha = mod_json["repos"][NF_CORE_MODULES_REMOTE]["modules"][NF_CORE_MODULES_NAME][mod]["git_sha"] - print(correct_git_sha, current_git_sha) assert correct_git_sha != current_git_sha diff --git a/tests/subworkflows/update.py b/tests/subworkflows/update.py index 182ed00d19..0b061e0acf 100644 --- a/tests/subworkflows/update.py +++ b/tests/subworkflows/update.py @@ -34,7 +34,6 @@ def test_install_at_hash_and_update(self): assert self.sw_install_old.install("fastq_align_bowtie2") update_obj = SubworkflowUpdate(self.pipeline_dir, show_diff=False, recursive=True) old_mod_json = ModulesJson(self.pipeline_dir).get_modules_json() - print(old_mod_json) # Copy the sw files and check that they are affected by the update tmpdir = tempfile.mkdtemp() @@ -47,7 +46,6 @@ def test_install_at_hash_and_update(self): # Check that the modules.json is correctly updated mod_json = ModulesJson(self.pipeline_dir).get_modules_json() - print(mod_json) # Get the up-to-date git_sha for the sw from the ModulesRepo object assert ( old_mod_json["repos"][NF_CORE_MODULES_REMOTE]["subworkflows"][NF_CORE_MODULES_NAME]["fastq_align_bowtie2"][ @@ -92,7 +90,6 @@ def test_update_all(self): # We must reload the modules.json to get the updated version mod_json_obj = ModulesJson(self.pipeline_dir) mod_json = mod_json_obj.get_modules_json() - print(mod_json) # Loop through all subworkflows and check that they are updated (according to the modules.json file) for sw in mod_json["repos"][NF_CORE_MODULES_REMOTE]["subworkflows"][NF_CORE_MODULES_NAME]: correct_git_sha = list(update_obj.modules_repo.get_component_git_log(sw, "subworkflows", depth=1))[0]["git_sha"] From 3e6660e957e4f5930df9482b024f5d8054fdddd7 Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Mon, 14 Nov 2022 16:41:40 +0100 Subject: [PATCH 20/31] run black --- tests/utils.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/utils.py b/tests/utils.py index abe1626496..07143171fd 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -23,7 +23,6 @@ GITLAB_BRANCH_TEST_NEW_SHA = "2f5f180f6e705bb81d6e7742dc2f24bf4a0c721e" - def with_temporary_folder(func): """ Call the decorated funtion under the tempfile.TemporaryDirectory From 98ffb73ca423587842982ec3429f8da3efbd7093 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Mir=20Pedrol?= Date: Wed, 16 Nov 2022 09:15:03 +0100 Subject: [PATCH 21/31] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Matthias Hörtenhuber --- nf_core/components/update.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/nf_core/components/update.py b/nf_core/components/update.py index d59f94610e..47f9cc1258 100644 --- a/nf_core/components/update.py +++ b/nf_core/components/update.py @@ -61,7 +61,7 @@ def _parameter_checks(self): raise UserWarning(f"Either a {self.component_type[:-1]} or the '--all' flag can be specified, not both.") if self.repo_type == "modules": - raise UserWarning(f"{self.component_type.title()} in clones of nf-core/modules can not be updated.") + raise UserWarning(f"{self.component_type.title()} can not be updated in clones of the nf-core/modules repository.") if self.prompt and self.sha is not None: raise UserWarning("Cannot use '--sha' and '--prompt' at the same time.") @@ -79,7 +79,7 @@ def update(self, component=None, silent=False, updated=None, check_diff_exist=Tr component (str): The name of the module/subworkflow to update. Returns: - bool: True if the update was successful, False otherwise. + (bool): True if the update was successful, False otherwise. """ self.component = component if updated is None: @@ -222,7 +222,7 @@ def update(self, component=None, silent=False, updated=None, check_diff_exist=Tr ) updated.append(component) except UserWarning as e: - if str(e) != "Module is unchanged": + if str(e) != "{self.component_type[:-1].title()} is unchanged": raise else: updated.append(component) @@ -309,7 +309,7 @@ def update(self, component=None, silent=False, updated=None, check_diff_exist=Tr f" [bold magenta italic]git apply {self.save_diff_fn} [/]" ) elif not all_patches_successful and not silent: - log.info(f"Updates complete. Please apply failed patch{plural_es(components_info)} manually") + log.info(f"Updates complete. Please apply failed patch{plural_es(components_info)} manually.") elif not silent: log.info("Updates complete :sparkles:") From 449885f04de8f809683cbfee2ea09f5fdef748d7 Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Wed, 16 Nov 2022 09:17:40 +0100 Subject: [PATCH 22/31] add command to rich-click --- nf_core/__main__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nf_core/__main__.py b/nf_core/__main__.py index 3d1c5cba91..3420af0483 100755 --- a/nf_core/__main__.py +++ b/nf_core/__main__.py @@ -58,7 +58,7 @@ "nf-core subworkflows": [ { "name": "For pipelines", - "commands": ["install"], + "commands": ["install", "update"], }, { "name": "Developing new subworkflows", From 9383c77fcc18d094321746d28dc5bb78252f3f3b Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Wed, 16 Nov 2022 09:23:27 +0100 Subject: [PATCH 23/31] add default to questionary.select --- nf_core/components/update.py | 1 + 1 file changed, 1 insertion(+) diff --git a/nf_core/components/update.py b/nf_core/components/update.py index d59f94610e..e878162ff8 100644 --- a/nf_core/components/update.py +++ b/nf_core/components/update.py @@ -129,6 +129,7 @@ def update(self, component=None, silent=False, updated=None, check_diff_exist=Tr {"name": "Preview diff in terminal, choose whether to update files", "value": 1}, {"name": "Just write diffs to a patch file", "value": 2}, ], + default={"name": "No previews, just update everything", "value": 0}, style=nf_core.utils.nfcore_question_style, ).unsafe_ask() From 33ef4b64af350c144ad5f431b1548dd6393288eb Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Wed, 16 Nov 2022 09:31:09 +0100 Subject: [PATCH 24/31] change --recursive by --update-deps --- nf_core/__main__.py | 16 ++++++++-------- nf_core/components/update.py | 8 ++++---- nf_core/modules/update.py | 4 ++-- nf_core/subworkflows/update.py | 4 ++-- tests/modules/patch.py | 9 +++++++-- tests/modules/update.py | 4 ++-- tests/subworkflows/update.py | 8 ++++---- 7 files changed, 29 insertions(+), 24 deletions(-) diff --git a/nf_core/__main__.py b/nf_core/__main__.py index 3420af0483..59ba06790e 100755 --- a/nf_core/__main__.py +++ b/nf_core/__main__.py @@ -555,13 +555,13 @@ def install(ctx, tool, dir, prompt, force, sha): help="Save diffs to a file instead of updating in place", ) @click.option( - "-r", - "--recursive", + "-u", + "--update-deps", is_flag=True, default=False, help="Automatically update all linked modules and subworkflows without asking for confirmation", ) -def update(ctx, tool, dir, force, prompt, sha, all, preview, save_diff, recursive): +def update(ctx, tool, dir, force, prompt, sha, all, preview, save_diff, update_deps): """ Update DSL2 modules within a pipeline. @@ -576,7 +576,7 @@ def update(ctx, tool, dir, force, prompt, sha, all, preview, save_diff, recursiv all, preview, save_diff, - recursive, + update - deps, ctx.obj["modules_repo_url"], ctx.obj["modules_repo_branch"], ctx.obj["modules_repo_no_pull"], @@ -1095,13 +1095,13 @@ def local(ctx, keywords, json, dir): # pylint: disable=redefined-builtin help="Save diffs to a file instead of updating in place", ) @click.option( - "-r", - "--recursive", + "-u", + "--update-deps", is_flag=True, default=False, help="Automatically update all linked modules and subworkflows without asking for confirmation", ) -def update(ctx, subworkflow, dir, force, prompt, sha, all, preview, save_diff, recursive): +def update(ctx, subworkflow, dir, force, prompt, sha, all, preview, save_diff, update_deps): """ Update DSL2 subworkflow within a pipeline. @@ -1116,7 +1116,7 @@ def update(ctx, subworkflow, dir, force, prompt, sha, all, preview, save_diff, r all, preview, save_diff, - recursive, + update - deps, ctx.obj["modules_repo_url"], ctx.obj["modules_repo_branch"], ctx.obj["modules_repo_no_pull"], diff --git a/nf_core/components/update.py b/nf_core/components/update.py index e878162ff8..08277d571f 100644 --- a/nf_core/components/update.py +++ b/nf_core/components/update.py @@ -29,7 +29,7 @@ def __init__( update_all=False, show_diff=None, save_diff_fn=None, - recursive=False, + update_deps=False, remote_url=None, branch=None, no_pull=False, @@ -41,7 +41,7 @@ def __init__( self.update_all = update_all self.show_diff = show_diff self.save_diff_fn = save_diff_fn - self.recursive = recursive + self.update_deps = update - deps self.component = None self.update_config = None self.modules_json = ModulesJson(self.dir) @@ -234,7 +234,7 @@ def update(self, component=None, silent=False, updated=None, check_diff_exist=Tr "It is advised to keep all your modules and subworkflows up to date.\n" "It is not guaranteed that a subworkflow will continue working as expected if all modules/subworkflows used in it are not up to date.\n" ) - if self.recursive: + if self.update - deps: recursive_update = True else: recursive_update = questionary.confirm( @@ -278,7 +278,7 @@ def update(self, component=None, silent=False, updated=None, check_diff_exist=Tr "It is advised to keep all your modules and subworkflows up to date.\n" "It is not guaranteed that a subworkflow will continue working as expected if all modules/subworkflows used in it are not up to date.\n" ) - if self.recursive: + if self.update - deps: recursive_update = True else: recursive_update = questionary.confirm( diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index 350401c43e..d07b9482ce 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -11,7 +11,7 @@ def __init__( update_all=False, show_diff=None, save_diff_fn=None, - recursive=False, + update_deps=False, remote_url=None, branch=None, no_pull=False, @@ -25,7 +25,7 @@ def __init__( update_all, show_diff, save_diff_fn, - recursive, + update - deps, remote_url, branch, no_pull, diff --git a/nf_core/subworkflows/update.py b/nf_core/subworkflows/update.py index 200ade9c50..17a3cd39f7 100644 --- a/nf_core/subworkflows/update.py +++ b/nf_core/subworkflows/update.py @@ -11,7 +11,7 @@ def __init__( update_all=False, show_diff=None, save_diff_fn=None, - recursive=False, + update_deps=False, remote_url=None, branch=None, no_pull=False, @@ -25,7 +25,7 @@ def __init__( update_all, show_diff, save_diff_fn, - recursive, + update - deps, remote_url, branch, no_pull, diff --git a/tests/modules/patch.py b/tests/modules/patch.py index 100afe8731..8bad4c59a3 100644 --- a/tests/modules/patch.py +++ b/tests/modules/patch.py @@ -235,7 +235,12 @@ def test_create_patch_update_success(self): # Update the module update_obj = nf_core.modules.ModuleUpdate( - self.pipeline_dir, sha=SUCCEED_SHA, show_diff=False, recursive=True, remote_url=GITLAB_URL, branch=PATCH_BRANCH + self.pipeline_dir, + sha=SUCCEED_SHA, + show_diff=False, + update_deps=True, + remote_url=GITLAB_URL, + branch=PATCH_BRANCH, ) assert update_obj.update(BISMARK_ALIGN) @@ -294,7 +299,7 @@ def test_create_patch_update_fail(self): patch_contents = fh.read() update_obj = nf_core.modules.ModuleUpdate( - self.pipeline_dir, sha=FAIL_SHA, show_diff=False, recursive=True, remote_url=GITLAB_URL, branch=PATCH_BRANCH + self.pipeline_dir, sha=FAIL_SHA, show_diff=False, update_deps=True, remote_url=GITLAB_URL, branch=PATCH_BRANCH ) update_obj.update(BISMARK_ALIGN) diff --git a/tests/modules/update.py b/tests/modules/update.py index a4121f917c..466bd61185 100644 --- a/tests/modules/update.py +++ b/tests/modules/update.py @@ -46,7 +46,7 @@ def test_install_at_hash_and_update(self): """Installs an old version of a module in the pipeline and updates it""" assert self.mods_install_old.install("trimgalore") update_obj = ModuleUpdate( - self.pipeline_dir, show_diff=False, recursive=True, remote_url=GITLAB_URL, branch=OLD_TRIMGALORE_BRANCH + self.pipeline_dir, show_diff=False, update_deps=True, remote_url=GITLAB_URL, branch=OLD_TRIMGALORE_BRANCH ) # Copy the module files and check that they are affected by the update @@ -231,7 +231,7 @@ def test_update_different_branch_single_module(self): assert install_obj.install("fastp") update_obj = ModuleUpdate( - self.pipeline_dir, recursive=True, remote_url=GITLAB_URL, branch=GITLAB_BRANCH_TEST_BRANCH, show_diff=False + self.pipeline_dir, update_deps=True, remote_url=GITLAB_URL, branch=GITLAB_BRANCH_TEST_BRANCH, show_diff=False ) update_obj.update("fastp") diff --git a/tests/subworkflows/update.py b/tests/subworkflows/update.py index 0b061e0acf..8626385072 100644 --- a/tests/subworkflows/update.py +++ b/tests/subworkflows/update.py @@ -32,7 +32,7 @@ def test_install_and_update(self): def test_install_at_hash_and_update(self): """Installs an old version of a subworkflow in the pipeline and updates it""" assert self.sw_install_old.install("fastq_align_bowtie2") - update_obj = SubworkflowUpdate(self.pipeline_dir, show_diff=False, recursive=True) + update_obj = SubworkflowUpdate(self.pipeline_dir, show_diff=False, update_deps=True) old_mod_json = ModulesJson(self.pipeline_dir).get_modules_json() # Copy the sw files and check that they are affected by the update @@ -61,7 +61,7 @@ def test_install_at_hash_and_update_and_save_diff_to_file(self): """Installs an old version of a sw in the pipeline and updates it. Save differences to a file.""" assert self.sw_install_old.install("fastq_align_bowtie2") patch_path = Path(self.pipeline_dir, "fastq_align_bowtie2.patch") - update_obj = SubworkflowUpdate(self.pipeline_dir, save_diff_fn=patch_path, recursive=True) + update_obj = SubworkflowUpdate(self.pipeline_dir, save_diff_fn=patch_path, update_deps=True) # Copy the sw files and check that they are affected by the update tmpdir = tempfile.mkdtemp() @@ -231,7 +231,7 @@ def test_update_all_linked_components_from_subworkflow(self): shutil.copytree(modules_path, Path(tmpdir, "modules")) # Update fastq_align_bowtie2 and all modules and subworkflows used by that - update_obj = SubworkflowUpdate(self.pipeline_dir, recursive=True, show_diff=False) + update_obj = SubworkflowUpdate(self.pipeline_dir, update_deps=True, show_diff=False) assert update_obj.update("fastq_align_bowtie2") is True mod_json = ModulesJson(self.pipeline_dir).get_modules_json() @@ -276,7 +276,7 @@ def test_update_all_subworkflows_from_module(self): shutil.copytree(sw_path, Path(tmpdir, "fastq_align_bowtie2")) # Update bowtie2/align and all subworkflows using it - update_obj = ModuleUpdate(self.pipeline_dir, recursive=True, show_diff=False) + update_obj = ModuleUpdate(self.pipeline_dir, update_deps=True, show_diff=False) assert update_obj.update("bowtie2/align") is True mod_json = ModulesJson(self.pipeline_dir).get_modules_json() From 637a036c27355e0cfff0b3e181b9713ece27cc04 Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Wed, 16 Nov 2022 09:33:49 +0100 Subject: [PATCH 25/31] fix lint mess --- nf_core/components/update.py | 6 +++--- nf_core/modules/update.py | 2 +- nf_core/subworkflows/update.py | 2 +- tests/test_subworkflows.py | 7 +------ 4 files changed, 6 insertions(+), 11 deletions(-) diff --git a/nf_core/components/update.py b/nf_core/components/update.py index 08277d571f..99c8f85349 100644 --- a/nf_core/components/update.py +++ b/nf_core/components/update.py @@ -41,7 +41,7 @@ def __init__( self.update_all = update_all self.show_diff = show_diff self.save_diff_fn = save_diff_fn - self.update_deps = update - deps + self.update_deps = update_deps self.component = None self.update_config = None self.modules_json = ModulesJson(self.dir) @@ -234,7 +234,7 @@ def update(self, component=None, silent=False, updated=None, check_diff_exist=Tr "It is advised to keep all your modules and subworkflows up to date.\n" "It is not guaranteed that a subworkflow will continue working as expected if all modules/subworkflows used in it are not up to date.\n" ) - if self.update - deps: + if self.update_deps: recursive_update = True else: recursive_update = questionary.confirm( @@ -278,7 +278,7 @@ def update(self, component=None, silent=False, updated=None, check_diff_exist=Tr "It is advised to keep all your modules and subworkflows up to date.\n" "It is not guaranteed that a subworkflow will continue working as expected if all modules/subworkflows used in it are not up to date.\n" ) - if self.update - deps: + if self.update_deps: recursive_update = True else: recursive_update = questionary.confirm( diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index d07b9482ce..9d53bf2017 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -25,7 +25,7 @@ def __init__( update_all, show_diff, save_diff_fn, - update - deps, + update_deps, remote_url, branch, no_pull, diff --git a/nf_core/subworkflows/update.py b/nf_core/subworkflows/update.py index 17a3cd39f7..3cd4ad59fd 100644 --- a/nf_core/subworkflows/update.py +++ b/nf_core/subworkflows/update.py @@ -25,7 +25,7 @@ def __init__( update_all, show_diff, save_diff_fn, - update - deps, + update_deps, remote_url, branch, no_pull, diff --git a/tests/test_subworkflows.py b/tests/test_subworkflows.py index abdd550ecb..d3eaa14a86 100644 --- a/tests/test_subworkflows.py +++ b/tests/test_subworkflows.py @@ -11,12 +11,7 @@ import nf_core.modules import nf_core.subworkflows -from .utils import ( - GITLAB_SUBWORKFLOWS_BRANCH, - GITLAB_URL, - OLD_SUBWORKFLOWS_SHA, - mock_api_calls, -) +from .utils import GITLAB_SUBWORKFLOWS_BRANCH, GITLAB_URL, OLD_SUBWORKFLOWS_SHA def create_modules_repo_dummy(tmp_dir): From 0cd49960836a11d9416df9848c907586cc5b64ca Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Wed, 16 Nov 2022 09:38:30 +0100 Subject: [PATCH 26/31] only warn about other modules/subworkflows to update if there are some --- nf_core/components/update.py | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/nf_core/components/update.py b/nf_core/components/update.py index 99c8f85349..f3a11e8c30 100644 --- a/nf_core/components/update.py +++ b/nf_core/components/update.py @@ -228,7 +228,10 @@ def update(self, component=None, silent=False, updated=None, check_diff_exist=Tr else: updated.append(component) recursive_update = True - if not silent: + modules_to_update, subworkflows_to_update = self.get_modules_subworkflows_to_update( + component, modules_repo + ) + 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" "It is advised to keep all your modules and subworkflows up to date.\n" @@ -242,9 +245,11 @@ def update(self, component=None, silent=False, updated=None, check_diff_exist=Tr default=True, style=nf_core.utils.nfcore_question_style, ).unsafe_ask() - if recursive_update: + if recursive_update and len(modules_to_update + subworkflows_to_update) > 0: # Write all the differences of linked componenets to a diff file - self.update_linked_components(component, modules_repo, updated, check_diff_exist=False) + self.update_linked_components( + modules_to_update, subworkflows_to_update, updated, check_diff_exist=False + ) elif self.show_diff: ModulesDiffer.print_diff( @@ -272,7 +277,10 @@ def update(self, component=None, silent=False, updated=None, check_diff_exist=Tr self.modules_json.update(self.component_type, modules_repo, component, version, self.component_type) updated.append(component) recursive_update = True - if not silent and not self.update_all: + modules_to_update, subworkflows_to_update = self.get_modules_subworkflows_to_update( + component, modules_repo + ) + 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" "It is advised to keep all your modules and subworkflows up to date.\n" @@ -286,9 +294,9 @@ def update(self, component=None, silent=False, updated=None, check_diff_exist=Tr default=True, style=nf_core.utils.nfcore_question_style, ).unsafe_ask() - if recursive_update: + if recursive_update and len(modules_to_update + subworkflows_to_update) > 0: # Update linked components - self.update_linked_components(component, modules_repo, updated) + self.update_linked_components(modules_to_update, subworkflows_to_update, updated) else: # Don't save to a file, just iteratively update the variable self.modules_json.update( @@ -818,11 +826,10 @@ def get_modules_subworkflows_to_update(self, component, modules_repo): return modules_to_update, subworkflows_to_update - def update_linked_components(self, component, modules_repo, updated=None, check_diff_exist=True): + def update_linked_components(self, modules_to_update, subworkflows_to_update, updated=None, check_diff_exist=True): """ Update modules and subworkflows linked to the component being updated. """ - modules_to_update, subworkflows_to_update = self.get_modules_subworkflows_to_update(component, modules_repo) for s_update in subworkflows_to_update: if s_update in updated: continue From 6b311cd0363eaf37380fa1000a174304f0d30ef2 Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Wed, 16 Nov 2022 09:43:57 +0100 Subject: [PATCH 27/31] change sw_install by subworkflow_install --- tests/subworkflows/install.py | 32 ++++++++++++++++---------------- tests/subworkflows/list.py | 4 ++-- tests/subworkflows/update.py | 20 ++++++++++---------- tests/test_subworkflows.py | 6 +++--- 4 files changed, 31 insertions(+), 31 deletions(-) diff --git a/tests/subworkflows/install.py b/tests/subworkflows/install.py index f394ef4066..94b2d79ba0 100644 --- a/tests/subworkflows/install.py +++ b/tests/subworkflows/install.py @@ -16,35 +16,35 @@ def test_subworkflow_install_nopipeline(self): """Test installing a subworkflow - no pipeline given""" - self.sw_install.dir = None - assert self.sw_install.install("foo") is False + self.subworkflow_install.dir = None + assert self.subworkflow_install.install("foo") is False @with_temporary_folder def test_subworkflows_install_emptypipeline(self, tmpdir): """Test installing a subworkflow - empty dir given""" os.mkdir(os.path.join(tmpdir, "nf-core-pipe")) - self.sw_install.dir = os.path.join(tmpdir, "nf-core-pipe") + self.subworkflow_install.dir = os.path.join(tmpdir, "nf-core-pipe") with pytest.raises(UserWarning) as excinfo: - self.sw_install.install("foo") + self.subworkflow_install.install("foo") assert "Could not find a 'main.nf' or 'nextflow.config' file" in str(excinfo.value) def test_subworkflows_install_nosubworkflow(self): """Test installing a subworkflow - unrecognised subworkflow given""" - assert self.sw_install.install("foo") is False + assert self.subworkflow_install.install("foo") is False def test_subworkflows_install_bam_sort_stats_samtools(self): """Test installing a subworkflow - bam_sort_stats_samtools""" - assert self.sw_install.install("bam_sort_stats_samtools") is not False - subworkflow_path = os.path.join(self.sw_install.dir, "subworkflows", "nf-core", "bam_sort_stats_samtools") - sub_subworkflow_path = os.path.join(self.sw_install.dir, "subworkflows", "nf-core", "bam_stats_samtools") - samtools_index_path = os.path.join(self.sw_install.dir, "modules", "nf-core", "samtools", "index") - samtools_sort_path = os.path.join(self.sw_install.dir, "modules", "nf-core", "samtools", "sort") - samtools_stats_path = os.path.join(self.sw_install.dir, "modules", "nf-core", "samtools", "stats") - samtools_idxstats_path = os.path.join(self.sw_install.dir, "modules", "nf-core", "samtools", "idxstats") - samtools_flagstat_path = os.path.join(self.sw_install.dir, "modules", "nf-core", "samtools", "flagstat") + assert self.subworkflow_install.install("bam_sort_stats_samtools") is not False + subworkflow_path = os.path.join(self.subworkflow_install.dir, "subworkflows", "nf-core", "bam_sort_stats_samtools") + sub_subworkflow_path = os.path.join(self.subworkflow_install.dir, "subworkflows", "nf-core", "bam_stats_samtools") + samtools_index_path = os.path.join(self.subworkflow_install.dir, "modules", "nf-core", "samtools", "index") + samtools_sort_path = os.path.join(self.subworkflow_install.dir, "modules", "nf-core", "samtools", "sort") + samtools_stats_path = os.path.join(self.subworkflow_install.dir, "modules", "nf-core", "samtools", "stats") + samtools_idxstats_path = os.path.join(self.subworkflow_install.dir, "modules", "nf-core", "samtools", "idxstats") + samtools_flagstat_path = os.path.join(self.subworkflow_install.dir, "modules", "nf-core", "samtools", "flagstat") assert os.path.exists(subworkflow_path) assert os.path.exists(sub_subworkflow_path) assert os.path.exists(samtools_index_path) @@ -56,13 +56,13 @@ def test_subworkflows_install_bam_sort_stats_samtools(self): def test_subworkflows_install_bam_sort_stats_samtools_twice(self): """Test installing a subworkflow - bam_sort_stats_samtools already there""" - self.sw_install.install("bam_sort_stats_samtools") - assert self.sw_install.install("bam_sort_stats_samtools") is False + self.subworkflow_install.install("bam_sort_stats_samtools") + assert self.subworkflow_install.install("bam_sort_stats_samtools") is False def test_subworkflows_install_from_gitlab(self): """Test installing a subworkflow from GitLab""" - assert self.sw_install_gitlab.install("bam_stats_samtools") is True + 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) assert ( diff --git a/tests/subworkflows/list.py b/tests/subworkflows/list.py index 8daf5fb599..c65999d42c 100644 --- a/tests/subworkflows/list.py +++ b/tests/subworkflows/list.py @@ -29,7 +29,7 @@ def test_subworkflows_list_remote_gitlab(self): def test_subworkflows_install_and_list_subworkflows(self): """Test listing locally installed subworkflows""" - self.sw_install.install("bam_sort_stats_samtools") + self.subworkflow_install.install("bam_sort_stats_samtools") subworkflows_list = nf_core.subworkflows.SubworkflowList(self.pipeline_dir, remote=False) listed_subworkflows = subworkflows_list.list_components() console = Console(record=True) @@ -40,7 +40,7 @@ def test_subworkflows_install_and_list_subworkflows(self): def test_subworkflows_install_gitlab_and_list_subworkflows(self): """Test listing locally installed subworkflows""" - self.sw_install_gitlab.install("bam_sort_stats_samtools") + 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() console = Console(record=True) diff --git a/tests/subworkflows/update.py b/tests/subworkflows/update.py index 8626385072..549f9366d8 100644 --- a/tests/subworkflows/update.py +++ b/tests/subworkflows/update.py @@ -16,7 +16,7 @@ def test_install_and_update(self): """Installs a subworkflow in the pipeline and updates it (no change)""" - self.sw_install.install("bam_stats_samtools") + self.subworkflow_install.install("bam_stats_samtools") update_obj = SubworkflowUpdate(self.pipeline_dir, show_diff=False) # Copy the sw files and check that they are unaffected by the update @@ -31,7 +31,7 @@ def test_install_and_update(self): def test_install_at_hash_and_update(self): """Installs an old version of a subworkflow in the pipeline and updates it""" - assert self.sw_install_old.install("fastq_align_bowtie2") + assert self.subworkflow_install_old.install("fastq_align_bowtie2") update_obj = SubworkflowUpdate(self.pipeline_dir, show_diff=False, update_deps=True) old_mod_json = ModulesJson(self.pipeline_dir).get_modules_json() @@ -59,7 +59,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 sw in the pipeline and updates it. Save differences to a file.""" - assert self.sw_install_old.install("fastq_align_bowtie2") + assert self.subworkflow_install_old.install("fastq_align_bowtie2") patch_path = Path(self.pipeline_dir, "fastq_align_bowtie2.patch") update_obj = SubworkflowUpdate(self.pipeline_dir, save_diff_fn=patch_path, update_deps=True) @@ -82,7 +82,7 @@ def test_install_at_hash_and_update_and_save_diff_to_file(self): def test_update_all(self): """Updates all subworkflows present in the pipeline""" # Install subworkflows fastq_align_bowtie2, bam_sort_stats_samtools, bam_stats_samtools - self.sw_install.install("fastq_align_bowtie2") + self.subworkflow_install.install("fastq_align_bowtie2") # Update all subworkflows update_obj = SubworkflowUpdate(self.pipeline_dir, update_all=True, show_diff=False) assert update_obj.update() is True @@ -100,7 +100,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""" # Install subworkflow at the latest version - assert self.sw_install.install("fastq_align_bowtie2") + assert self.subworkflow_install.install("fastq_align_bowtie2") # Fix the subworkflow version in the .nf-core.yml to an old version update_config = {NF_CORE_MODULES_REMOTE: {NF_CORE_MODULES_NAME: {"fastq_align_bowtie2": OLD_SUBWORKFLOWS_SHA}}} @@ -131,7 +131,7 @@ def test_update_with_config_fixed_version(self): def test_update_with_config_dont_update(self): """Try updating when sw is to be ignored""" # Install an old version of fastq_align_bowtie2 - self.sw_install_old.install("fastq_align_bowtie2") + self.subworkflow_install_old.install("fastq_align_bowtie2") # Set the fastq_align_bowtie2 field to no update in the .nf-core.yml update_config = {NF_CORE_MODULES_REMOTE: {NF_CORE_MODULES_NAME: {"fastq_align_bowtie2": False}}} @@ -162,7 +162,7 @@ def test_update_with_config_dont_update(self): def test_update_with_config_fix_all(self): """Fix the version of all nf-core subworkflows""" # Install subworkflow at the latest version - assert self.sw_install.install("fastq_align_bowtie2") + assert self.subworkflow_install.install("fastq_align_bowtie2") # Fix the version of all nf-core subworkflows in the .nf-core.yml to an old version update_config = {NF_CORE_MODULES_REMOTE: OLD_SUBWORKFLOWS_SHA} @@ -192,7 +192,7 @@ def test_update_with_config_fix_all(self): def test_update_with_config_no_updates(self): """Don't update any nf-core subworkflows""" # Install an old version of fastq_align_bowtie2 - self.sw_install_old.install("fastq_align_bowtie2") + self.subworkflow_install_old.install("fastq_align_bowtie2") old_mod_json = ModulesJson(self.pipeline_dir).get_modules_json() # Set all repository updates to False @@ -219,7 +219,7 @@ def test_update_with_config_no_updates(self): def test_update_all_linked_components_from_subworkflow(self): """Update a subworkflow and all modules and subworkflows used on it""" # Install an old version of fastq_align_bowtie2 - self.sw_install_old.install("fastq_align_bowtie2") + self.subworkflow_install_old.install("fastq_align_bowtie2") old_mod_json = ModulesJson(self.pipeline_dir).get_modules_json() # Copy the sw files and check that they are affected by the update @@ -266,7 +266,7 @@ def test_update_all_linked_components_from_subworkflow(self): def test_update_all_subworkflows_from_module(self): """Update a module and all subworkflows that use this module""" # Install an old version of fastq_align_bowtie2 and thus all modules used by it (bowtie2/align) - self.sw_install_old.install("fastq_align_bowtie2") + self.subworkflow_install_old.install("fastq_align_bowtie2") old_mod_json = ModulesJson(self.pipeline_dir).get_modules_json() # Copy the sw files and check that they are affected by the update diff --git a/tests/test_subworkflows.py b/tests/test_subworkflows.py index d3eaa14a86..56b4549cd0 100644 --- a/tests/test_subworkflows.py +++ b/tests/test_subworkflows.py @@ -56,11 +56,11 @@ def setUp(self): self.nfcore_modules = create_modules_repo_dummy(self.tmp_dir) # Set up install objects - self.sw_install = nf_core.subworkflows.SubworkflowInstall(self.pipeline_dir, prompt=False, force=False) - self.sw_install_gitlab = nf_core.subworkflows.SubworkflowInstall( + self.subworkflow_install = nf_core.subworkflows.SubworkflowInstall(self.pipeline_dir, prompt=False, force=False) + self.subworkflow_install_gitlab = nf_core.subworkflows.SubworkflowInstall( self.pipeline_dir, prompt=False, force=False, remote_url=GITLAB_URL, branch=GITLAB_SUBWORKFLOWS_BRANCH ) - self.sw_install_old = nf_core.subworkflows.SubworkflowInstall( + self.subworkflow_install_old = nf_core.subworkflows.SubworkflowInstall( self.pipeline_dir, prompt=False, force=False, From 166ad3e57ab00f7e57271ddaad380a89ec41c334 Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Wed, 16 Nov 2022 09:47:41 +0100 Subject: [PATCH 28/31] modify wrong variable names --- nf_core/__main__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nf_core/__main__.py b/nf_core/__main__.py index 59ba06790e..7e377ae965 100755 --- a/nf_core/__main__.py +++ b/nf_core/__main__.py @@ -576,7 +576,7 @@ def update(ctx, tool, dir, force, prompt, sha, all, preview, save_diff, update_d all, preview, save_diff, - update - deps, + update_deps, ctx.obj["modules_repo_url"], ctx.obj["modules_repo_branch"], ctx.obj["modules_repo_no_pull"], @@ -1116,7 +1116,7 @@ def update(ctx, subworkflow, dir, force, prompt, sha, all, preview, save_diff, u all, preview, save_diff, - update - deps, + update_deps, ctx.obj["modules_repo_url"], ctx.obj["modules_repo_branch"], ctx.obj["modules_repo_no_pull"], From cf31aabf16140a56ff39f058af293c2b62b57e27 Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Wed, 16 Nov 2022 09:52:21 +0100 Subject: [PATCH 29/31] improve error message --- nf_core/components/update.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/nf_core/components/update.py b/nf_core/components/update.py index f3a11e8c30..575f8c6885 100644 --- a/nf_core/components/update.py +++ b/nf_core/components/update.py @@ -356,7 +356,10 @@ def get_single_component_info(self, component): ).unsafe_ask() # Get component installation directory - install_dir = [dir for dir, m in components if component == m][0] + try: + install_dir = [dir for dir, m in components if component == m][0] + except IndexError: + raise UserWarning(f"{self.component_type[:-1].title()} '{component}' not found in 'modules.json'.") # Check if component is installed before trying to update if component not in choices: From 5c518221abe45e2dc4d2e7f5172a08f617622c14 Mon Sep 17 00:00:00 2001 From: nf-core-bot Date: Wed, 16 Nov 2022 09:03:07 +0000 Subject: [PATCH 30/31] [automated] Fix code linting --- nf_core/components/update.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/nf_core/components/update.py b/nf_core/components/update.py index e40e6c543c..3336b2f324 100644 --- a/nf_core/components/update.py +++ b/nf_core/components/update.py @@ -61,7 +61,9 @@ def _parameter_checks(self): raise UserWarning(f"Either a {self.component_type[:-1]} or the '--all' flag can be specified, not both.") if self.repo_type == "modules": - raise UserWarning(f"{self.component_type.title()} can not be updated in clones of the nf-core/modules repository.") + raise UserWarning( + f"{self.component_type.title()} can not be updated in clones of the nf-core/modules repository." + ) if self.prompt and self.sha is not None: raise UserWarning("Cannot use '--sha' and '--prompt' at the same time.") From 2e07990a8e5e028d56efd1f0d41b7fbdc070fbe3 Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Wed, 16 Nov 2022 10:48:54 +0100 Subject: [PATCH 31/31] revert suggestion that makes tests fail --- nf_core/components/update.py | 6 ++++-- tests/test_subworkflows.py | 6 +++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/nf_core/components/update.py b/nf_core/components/update.py index e40e6c543c..4e2025a158 100644 --- a/nf_core/components/update.py +++ b/nf_core/components/update.py @@ -61,7 +61,9 @@ def _parameter_checks(self): raise UserWarning(f"Either a {self.component_type[:-1]} or the '--all' flag can be specified, not both.") if self.repo_type == "modules": - raise UserWarning(f"{self.component_type.title()} can not be updated in clones of the nf-core/modules repository.") + raise UserWarning( + f"{self.component_type.title()} can not be updated in clones of the nf-core/modules repository." + ) if self.prompt and self.sha is not None: raise UserWarning("Cannot use '--sha' and '--prompt' at the same time.") @@ -223,7 +225,7 @@ def update(self, component=None, silent=False, updated=None, check_diff_exist=Tr ) updated.append(component) except UserWarning as e: - if str(e) != "{self.component_type[:-1].title()} is unchanged": + if str(e) != "Module is unchanged": raise else: updated.append(component) diff --git a/tests/test_subworkflows.py b/tests/test_subworkflows.py index 56b4549cd0..bd6ab0f9fa 100644 --- a/tests/test_subworkflows.py +++ b/tests/test_subworkflows.py @@ -67,9 +67,9 @@ def setUp(self): sha=OLD_SUBWORKFLOWS_SHA, ) - ############################################ - # Test of the individual modules commands. # - ############################################ + ################################################ + # Test of the individual subworkflow commands. # + ################################################ from .subworkflows.create import ( test_subworkflows_create_fail_exists,