From 1b333b75830c4630782fc89d79d15102a60971c2 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Sat, 4 Jul 2026 14:26:22 +0200 Subject: [PATCH] shed install populate: resolve new tool paths the way discovery does MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The partial populate filters discovered tools on exact path strings. _collect_new_tool_paths joined the shed conf dict's raw tool_path — relative for typical shed_tool_conf files — while discovery resolves a relative tool_path against the conf file's directory and returns an absolute path. The strings never matched, so the enriched conf-driven discovery was filtered out and the ad-hoc synthesis populated a metadata-poor entry instead: the panel served fastp without tool_shed_repository (test_only_latest_version_in_panel_fastp), and the store grew a duplicate row under the unresolved path string. Route both through discover.resolve_tool_path (now public) and thread each tool's guid so even a genuinely-unreachable path yields a guid-keyed entry. --- .../tools/tool_panel_manager.py | 43 ++++++++++++------- lib/galaxy/tool_source_store/discover.py | 4 +- .../unit/tool_shed/test_tool_panel_manager.py | 33 ++++++++++++++ 3 files changed, 62 insertions(+), 18 deletions(-) diff --git a/lib/galaxy/tool_shed/galaxy_install/tools/tool_panel_manager.py b/lib/galaxy/tool_shed/galaxy_install/tools/tool_panel_manager.py index a150a7b21da..7ee64a66ec4 100644 --- a/lib/galaxy/tool_shed/galaxy_install/tools/tool_panel_manager.py +++ b/lib/galaxy/tool_shed/galaxy_install/tools/tool_panel_manager.py @@ -23,28 +23,38 @@ from galaxy.util.tool_shed.xml_util import parse_xml log = logging.getLogger(__name__) -def _collect_new_tool_paths(elem_list, tool_path: str) -> list[str]: - """Walk ``elem_list`` and return the absolute paths of every ````. +def _collect_new_tool_paths(elem_list, tool_path: str, shed_tool_conf: str) -> dict[str, str | None]: + """Walk ``elem_list`` and map each new ````'s absolute path to its guid. ``elem_list`` is the freshly-generated panel additions for a shed install — either top-level ```` elements or ``
`` elements with - nested ```` children. Paths are ``os.path.join(tool_path, file)``, - matching what ``galaxy.tool_source_store.discover.discover_tools`` yields - after the conf is rewritten on disk. + nested ```` children. Paths must match what + ``galaxy.tool_source_store.discover.discover_tools`` yields for the + rewritten conf byte-for-byte (the partial populate filters on the string): + a relative ``tool_path`` resolves against the conf file's directory, not + the process cwd, so route through the same ``resolve_tool_path``. """ - new_paths: list[str] = [] + # Local import: keeps galaxy.tool_source_store out of the eager + # tool-shed install path's module graph. + from galaxy.tool_source_store.discover import resolve_tool_path + + resolved_base = resolve_tool_path(tool_path, shed_tool_conf) + path_guids: dict[str, str | None] = {} + + def _add(tool_elem) -> None: + relative = tool_elem.get("file") + if relative: + path = os.path.normpath(os.path.join(resolved_base, relative)) + path_guids[path] = tool_elem.get("guid") + for elem in elem_list: if elem.tag == "tool": - relative = elem.get("file") - if relative: - new_paths.append(os.path.normpath(os.path.join(tool_path, relative))) + _add(elem) elif elem.tag == "section": for child in elem: if child.tag == "tool": - relative = child.get("file") - if relative: - new_paths.append(os.path.normpath(os.path.join(tool_path, relative))) - return new_paths + _add(child) + return path_guids class ToolPanelManager: @@ -155,8 +165,8 @@ class ToolPanelManager: shed_tool_conf_dict["config_elems"] = config_elems self.app.toolbox.update_shed_config(shed_tool_conf_dict) self.add_to_shed_tool_config(shed_tool_conf_dict, elem_list) - new_paths = _collect_new_tool_paths(elem_list, tool_path) - if new_paths: + new_path_guids = _collect_new_tool_paths(elem_list, tool_path, shed_tool_conf_dict["config_filename"]) + if new_path_guids: from galaxy.tool_source_store.populator import populate_for_paths populate_for_paths( @@ -164,8 +174,9 @@ class ToolPanelManager: # Lazy mode implies a full Galaxy app, which carries # ``model`` beyond the InstallationTarget protocol. self.app.model.context, # type: ignore[attr-defined] - paths=new_paths, + paths=list(new_path_guids), rebuild_whoosh=True, + path_guids=new_path_guids, ) # Refresh THIS process synchronously; the AMQP broadcast # above only reaches peers asynchronously, but the install diff --git a/lib/galaxy/tool_source_store/discover.py b/lib/galaxy/tool_source_store/discover.py index d6847137608..57b3577e181 100644 --- a/lib/galaxy/tool_source_store/discover.py +++ b/lib/galaxy/tool_source_store/discover.py @@ -96,7 +96,7 @@ def get_tool_configs(config: "GalaxyAppConfiguration") -> list[str]: return configs -def _resolve_tool_path(tool_path: str | None, config_filename: str, root_dir: str | None = None) -> str: +def resolve_tool_path(tool_path: str | None, config_filename: str, root_dir: str | None = None) -> str: """ Resolve the tool_path to an absolute directory path. @@ -210,7 +210,7 @@ def discover_tools_from_config( return tool_path = tool_conf_source.parse_tool_path() - resolved_tool_path = _resolve_tool_path(tool_path, config_filename, root_dir) + resolved_tool_path = resolve_tool_path(tool_path, config_filename, root_dir) is_shed_conf = tool_conf_source.is_shed_tool_conf() # Match what AbstractToolBox._path_template_kwds does for ToolBox: tool diff --git a/test/unit/tool_shed/test_tool_panel_manager.py b/test/unit/tool_shed/test_tool_panel_manager.py index b5b738e6973..91baf7ec17d 100644 --- a/test/unit/tool_shed/test_tool_panel_manager.py +++ b/test/unit/tool_shed/test_tool_panel_manager.py @@ -1,4 +1,8 @@ import os +from xml.etree.ElementTree import ( + Element, + SubElement, +) from galaxy.app_unittest_utils.toolbox_support import ( BaseToolBoxTestCase, @@ -226,3 +230,32 @@ class TestToolPanelManager(BaseToolBoxTestCase): @property def tvm(self): return tool_version_manager.ToolVersionManager(self.ts_app) + + +GUID_V2 = DEFAULT_GUID + "v/2" + + +def _new_install_elem_list(): + section = Element("section", {"id": "sec", "name": "Sec", "version": ""}) + SubElement(section, "tool", {"file": "repos/iuc/fastp/abc/fastp/fastp.xml", "guid": DEFAULT_GUID}) + top = Element("tool", {"file": "repos/iuc/other/def/other/other.xml", "guid": GUID_V2}) + return [section, top] + + +def test_collect_new_tool_paths_resolves_relative_tool_path_against_conf_dir(tmp_path): + conf = tmp_path / "config" / "shed_tool_conf.xml" + path_guids = tool_panel_manager._collect_new_tool_paths(_new_install_elem_list(), "../shed_tools", str(conf)) + base = str(tmp_path / "shed_tools") + assert path_guids == { + f"{base}/repos/iuc/fastp/abc/fastp/fastp.xml": DEFAULT_GUID, + f"{base}/repos/iuc/other/def/other/other.xml": GUID_V2, + } + + +def test_collect_new_tool_paths_absolute_tool_path(tmp_path): + base = str(tmp_path / "shed_tools") + path_guids = tool_panel_manager._collect_new_tool_paths( + _new_install_elem_list(), base, str(tmp_path / "shed_tool_conf.xml") + ) + assert all(p.startswith(f"{base}/") for p in path_guids) + assert set(path_guids.values()) == {DEFAULT_GUID, GUID_V2}