Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update bug two #19

Merged
merged 34 commits into from
Nov 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
8f10765
wip: Add bug two
jvfe Oct 2, 2024
2ede707
refact: Try defaulting to a dict
jvfe Oct 14, 2024
7066463
fix: Dont allow remote to be unbound
jvfe Oct 14, 2024
c3b6acc
refact: Check if not equal to component type
jvfe Oct 24, 2024
484bea4
refact: Re-add isinstance dict
jvfe Oct 24, 2024
885e27b
fix: Remove mention of component_name
jvfe Oct 24, 2024
7b0f295
refact: Add special case for strings following dicts
jvfe Oct 24, 2024
c31c81b
test: Try removing branch from test case
jvfe Oct 24, 2024
a74facc
refact: Remove isinstance for str
jvfe Oct 24, 2024
7ebc6dc
refact: Use dict only with get_comps out
jvfe Oct 25, 2024
f7d3a4d
Revert "refact: Use dict only with get_comps out"
jvfe Oct 25, 2024
6982193
refact: Check if content has subwfs
jvfe Oct 25, 2024
a9b9184
refact: Always set current remote
jvfe Oct 25, 2024
046881a
refact: Check sha before resetting current_repo
jvfe Oct 28, 2024
0199e2d
refact: Do the negative test
jvfe Oct 28, 2024
955a1da
refact: Use another variable for remote
jvfe Oct 28, 2024
8ec9bbb
refact: Simplify check
jvfe Oct 28, 2024
0e35a85
refact: Expand check once again
jvfe Oct 28, 2024
959a63c
refact: Roll back to previous check
jvfe Oct 28, 2024
65aa1c4
fix: self.sha must not be none to reset to none
jvfe Oct 28, 2024
1e10b29
Revert "fix: self.sha must not be none to reset to none"
jvfe Oct 28, 2024
649fc64
Try removing the section entirely
jvfe Oct 29, 2024
c89e726
refact: Try moving check to reset
jvfe Nov 6, 2024
00996cb
refact: Remove unnecessary current_sha
jvfe Nov 7, 2024
4225272
refact: Change check in get_comps
jvfe Nov 7, 2024
e72de2f
refact: Set git remote beforehand
jvfe Nov 7, 2024
f58815e
refact: Change indent so previous check is the same
jvfe Nov 7, 2024
a956e04
refact: Remove current repo check
jvfe Nov 7, 2024
347ebcc
Revert "refact: Remove current repo check"
jvfe Nov 7, 2024
f20712c
refact: Try using name list
jvfe Nov 7, 2024
b4f9760
fix: Remove break in loop
jvfe Nov 29, 2024
38709da
refactor: Always set current_repo
jvfe Nov 29, 2024
45991f4
fix: Check if name is in updated
jvfe Nov 29, 2024
c0760a8
refactor: Remove unused sections and logs
jvfe Nov 29, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions nf_core/components/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,13 @@ def install(self, component: Union[str, Dict[str, str]], silent: bool = False) -
self.modules_repo = ModulesRepo(remote_url, branch)
component = component["name"]

if self.current_remote == self.modules_repo.remote_url and self.sha is not None:
self.current_sha = self.sha
else:
self.current_sha = None
if self.current_remote is None:
self.current_remote = self.modules_repo.remote_url

if self.current_remote == self.modules_repo.remote_url and self.sha is not None:
self.current_sha = self.sha
else:
self.current_sha = None

if self.repo_type == "modules":
log.error(f"You cannot install a {component} in a clone of nf-core/modules")
Expand Down
60 changes: 48 additions & 12 deletions nf_core/components/update.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import logging
import os
import re
import shutil
import tempfile
from pathlib import Path
Expand Down Expand Up @@ -41,6 +42,8 @@ def __init__(
limit_output=False,
):
super().__init__(component_type, pipeline_dir, remote_url, branch, no_pull)
self.current_remote = remote_url
self.branch = branch
self.force = force
self.prompt = prompt
self.sha = sha
Expand Down Expand Up @@ -92,6 +95,13 @@ def update(self, component=None, silent=False, updated=None, check_diff_exist=Tr
Returns:
(bool): True if the update was successful, False otherwise.
"""
if isinstance(component, dict):
# Override modules_repo when the component to install is a dependency from a subworkflow.
remote_url = component.get("git_remote", self.current_remote)
branch = component.get("branch", self.branch)
self.modules_repo = ModulesRepo(remote_url, branch)
component = component["name"]

self.component = component
if updated is None:
updated = []
Expand Down Expand Up @@ -868,7 +878,17 @@ def get_components_to_update(self, component):
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]
git_remote = self.current_remote
for subworkflow in installed_by:
if subworkflow != component:
for remote_url, content in mods_json["repos"].items():
if (all_subworkflows := content.get("subworkflows")) is not None:
for _, details in all_subworkflows.items():
if subworkflow in details:
git_remote = remote_url
if subworkflow != self.component_type:
subworkflows_to_update.append({"name": subworkflow, "git_remote": git_remote})

elif self.component_type == "subworkflows":
for repo, repo_content in mods_json["repos"].items():
for component_type, dir_content in repo_content.items():
Expand All @@ -879,9 +899,9 @@ def get_components_to_update(self, component):
# We need to update it too
if component in comp_content["installed_by"]:
if component_type == "modules":
modules_to_update.append(comp)
modules_to_update.append({"name": comp, "git_remote": repo, "org_path": dir})
elif component_type == "subworkflows":
subworkflows_to_update.append(comp)
subworkflows_to_update.append({"name": comp, "git_remote": repo, "org_path": dir})

return modules_to_update, subworkflows_to_update

Expand All @@ -896,7 +916,7 @@ def update_linked_components(
Update modules and subworkflows linked to the component being updated.
"""
for s_update in subworkflows_to_update:
if s_update in updated:
if s_update["name"] in updated:
continue
original_component_type, original_update_all = self._change_component_type("subworkflows")
self.update(
Expand All @@ -908,7 +928,7 @@ def update_linked_components(
self._reset_component_type(original_component_type, original_update_all)

for m_update in modules_to_update:
if m_update in updated:
if m_update["name"] in updated:
continue
original_component_type, original_update_all = self._change_component_type("modules")
try:
Expand All @@ -931,28 +951,42 @@ def update_linked_components(
def manage_changes_in_linked_components(self, component, modules_to_update, subworkflows_to_update):
"""Check for linked components added or removed in the new subworkflow version"""
if self.component_type == "subworkflows":
subworkflow_directory = Path(self.directory, self.component_type, self.modules_repo.repo_path, component)
org_path_match = re.search(r"(?:https://|git@)[\w\.]+[:/](.*?)/", self.current_remote)
if org_path_match:
org_path = org_path_match.group(1)

subworkflow_directory = Path(self.directory, self.component_type, org_path, component)
included_modules, included_subworkflows = get_components_to_install(subworkflow_directory)
# If a module/subworkflow has been removed from the subworkflow
for module in modules_to_update:
if module not in included_modules:
module = module["name"]
included_modules_names = [m["name"] for m in included_modules]
if module not in included_modules_names:
log.info(f"Removing module '{module}' which is not included in '{component}' anymore.")
remove_module_object = ComponentRemove("modules", self.directory)
remove_module_object.remove(module, removed_by=component)
for subworkflow in subworkflows_to_update:
if subworkflow not in included_subworkflows:
subworkflow = subworkflow["name"]
included_subworkflow_names = [m["name"] for m in included_subworkflows]
if subworkflow not in included_subworkflow_names:
log.info(f"Removing subworkflow '{subworkflow}' which is not included in '{component}' anymore.")
remove_subworkflow_object = ComponentRemove("subworkflows", self.directory)
remove_subworkflow_object.remove(subworkflow, removed_by=component)
# If a new module/subworkflow is included in the subworklfow and wasn't included before
for module in included_modules:
if module not in modules_to_update:
log.info(f"Installing newly included module '{module}' for '{component}'")
module_name = module["name"]
module["git_remote"] = module.get("git_remote", self.current_remote)
module["branch"] = module.get("branch", self.branch)
if module_name not in modules_to_update:
log.info(f"Installing newly included module '{module_name}' for '{component}'")
install_module_object = ComponentInstall(self.directory, "modules", installed_by=component)
install_module_object.install(module, silent=True)
for subworkflow in included_subworkflows:
if subworkflow not in subworkflows_to_update:
log.info(f"Installing newly included subworkflow '{subworkflow}' for '{component}'")
subworkflow_name = subworkflow["name"]
subworkflow["git_remote"] = subworkflow.get("git_remote", self.current_remote)
subworkflow["branch"] = subworkflow.get("branch", self.branch)
if subworkflow_name not in subworkflows_to_update:
log.info(f"Installing newly included subworkflow '{subworkflow_name}' for '{component}'")
install_subworkflow_object = ComponentInstall(
self.directory, "subworkflows", installed_by=component
)
Expand All @@ -971,3 +1005,5 @@ 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
if self.current_remote is None:
self.current_remote = self.modules_repo.remote_url
4 changes: 1 addition & 3 deletions tests/subworkflows/test_update.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
from nf_core.subworkflows.update import SubworkflowUpdate

from ..test_subworkflows import TestSubworkflows
from ..utils import CROSS_ORGANIZATION_BRANCH, CROSS_ORGANIZATION_URL, OLD_SUBWORKFLOWS_SHA, cmp_component
from ..utils import CROSS_ORGANIZATION_URL, OLD_SUBWORKFLOWS_SHA, cmp_component


class TestSubworkflowsUpdate(TestSubworkflows):
Expand Down Expand Up @@ -379,7 +379,6 @@ def test_update_subworkflow_across_orgs(self):
install_obj = SubworkflowInstall(
self.pipeline_dir,
remote_url=CROSS_ORGANIZATION_URL,
branch=CROSS_ORGANIZATION_BRANCH,
sha="b7dc6a4fcfdf780c9228b3abf6bd821b466c2f81",
)
# The fastq_trim_fastp_fastqc subworkflow contains the cross-org fastqc module, not the nf-core one
Expand All @@ -388,7 +387,6 @@ def test_update_subworkflow_across_orgs(self):
update_obj = SubworkflowUpdate(
self.pipeline_dir,
remote_url=CROSS_ORGANIZATION_URL,
branch=CROSS_ORGANIZATION_BRANCH,
update_all=False,
update_deps=True,
show_diff=False,
Expand Down
Loading