diff --git a/lib/galaxy/tool_shed/tools/data_table_manager.py b/lib/galaxy/tool_shed/tools/data_table_manager.py index f2d02a4d96a6..4ca5dcfdc5d1 100644 --- a/lib/galaxy/tool_shed/tools/data_table_manager.py +++ b/lib/galaxy/tool_shed/tools/data_table_manager.py @@ -163,16 +163,17 @@ def install_tool_data_tables(self, tool_shed_repository, tool_index_sample_files if os.path.exists(tool_data_table_conf_filename): tree, error_message = xml_util.parse_xml(tool_data_table_conf_filename) if tree: - for elem in tree.getroot(): - # Append individual table elems or other elemes, but not tables elems. - if elem.tag == "tables": - # TODO: this code need to be revised - for _table_elem in elems: - elems.append(elem) - else: - elems.append(elem) + root = tree.getroot() + if root.tag == "tables": + elems = list(root) + else: + log.warning( + "The '%s' data table file has '%s' instead of as root element, skipping.", + tool_data_table_conf_filename, + root.tag, + ) else: - log.debug( + log.warning( "The '%s' data table file was not found, but was expected to be copied from '%s' during repository installation.", tool_data_table_conf_filename, TOOL_DATA_TABLE_FILE_SAMPLE_NAME, diff --git a/lib/galaxy/tool_shed/util/utility_container_manager.py b/lib/galaxy/tool_shed/util/utility_container_manager.py index 433b18df4218..07582e5e8362 100644 --- a/lib/galaxy/tool_shed/util/utility_container_manager.py +++ b/lib/galaxy/tool_shed/util/utility_container_manager.py @@ -58,9 +58,11 @@ def contains_repository_dependency(self, repository_dependency): def remove_repository_dependency(self, repository_dependency): listified_repository_dependency = repository_dependency.listify - for contained_repository_dependency in self.repository_dependencies: - if contained_repository_dependency.listify == listified_repository_dependency: - self.repository_dependencies.remove(contained_repository_dependency) + self.repository_dependencies = [ + contained_repository_dependency + for contained_repository_dependency in self.repository_dependencies + if contained_repository_dependency.listify != listified_repository_dependency + ] def to_dict(self): folders = [] diff --git a/lib/galaxy/tool_util/data/__init__.py b/lib/galaxy/tool_util/data/__init__.py index fb39d6a56719..77f03cf9c7e0 100644 --- a/lib/galaxy/tool_util/data/__init__.py +++ b/lib/galaxy/tool_util/data/__init__.py @@ -1101,10 +1101,7 @@ def to_xml_file( except Exception as e: out_elems = [] log.debug("Could not parse existing tool data table config, assume no existing elements: %s", e) - for elem in remove_elems: - # handle multiple occurrences of remove elem in existing elems - while elem in out_elems: - remove_elems.remove(elem) + out_elems = [elem for elem in out_elems if elem not in remove_elems] # add new elems out_elems.extend(new_elems) out_path_is_new = not os.path.exists(full_path) diff --git a/lib/galaxy/tool_util/toolbox/base.py b/lib/galaxy/tool_util/toolbox/base.py index 9b9f532d497d..a146c93cde26 100644 --- a/lib/galaxy/tool_util/toolbox/base.py +++ b/lib/galaxy/tool_util/toolbox/base.py @@ -261,14 +261,14 @@ def _init_tools_from_configs(self, config_filenames): execution_timer = ExecutionTimer() self._tool_tag_manager.reset_tags() config_filenames = listify(config_filenames) - for config_filename in config_filenames: - if os.path.isdir(config_filename): - directory_contents = sorted(os.listdir(config_filename)) - directory_config_files = [ - config_file for config_file in directory_contents if config_file.endswith(".xml") - ] - config_filenames.remove(config_filename) - config_filenames.extend(directory_config_files) + config_directories = [config_filename for config_filename in config_filenames if os.path.isdir(config_filename)] + config_filenames = [ + config_filename for config_filename in config_filenames if config_filename not in config_directories + ] + for config_directory in config_directories: + directory_contents = sorted(os.listdir(config_directory)) + directory_config_files = [config_file for config_file in directory_contents if config_file.endswith(".xml")] + config_filenames.extend(directory_config_files) for config_filename in config_filenames: if not self.can_load_config_file(config_filename): continue diff --git a/lib/galaxy/tool_util/toolbox/watcher.py b/lib/galaxy/tool_util/toolbox/watcher.py index b91751e0ac00..e570bb58198d 100644 --- a/lib/galaxy/tool_util/toolbox/watcher.py +++ b/lib/galaxy/tool_util/toolbox/watcher.py @@ -136,7 +136,7 @@ def check(self): # thread to die in these cases. if path in drop_now: log.warning("'%s' could not be read, removing from watched files", path) - del paths[path] + del self.paths[path] if path in hashes: del hashes[path] else: diff --git a/lib/galaxy/util/topsort.py b/lib/galaxy/util/topsort.py index 64a1338fa1a4..38bdf40c4bd4 100644 --- a/lib/galaxy/util/topsort.py +++ b/lib/galaxy/util/topsort.py @@ -168,7 +168,7 @@ def topsort(pairlist): for y in successors[x]: numpreds[y] = numpreds[y] - 1 if numpreds[y] == 0: - answer.append(y) + answer.append(y) # noqa: B038 # following "del" isn't needed; just makes # CycleError details easier to grasp del successors[x] diff --git a/lib/galaxy/util/xml_macros.py b/lib/galaxy/util/xml_macros.py index b52e2bc08f8e..6ffeb12dfb3c 100644 --- a/lib/galaxy/util/xml_macros.py +++ b/lib/galaxy/util/xml_macros.py @@ -283,13 +283,6 @@ def _load_macro_file(path: StrPath, xml_base_dir, macros) -> List[str]: return _load_macros(root, xml_base_dir, macros) -def _xml_set_children(element, new_children): - for old_child in element: - element.remove(old_child) - for i, new_child in enumerate(new_children): - element.insert(i, new_child) - - def _xml_replace(query, targets): parent_el = query.find("..") matching_index = -1 diff --git a/test/unit/app/test_galaxy_install.py b/test/unit/app/test_galaxy_install.py index f0898a79d348..13ef2a5e81e4 100644 --- a/test/unit/app/test_galaxy_install.py +++ b/test/unit/app/test_galaxy_install.py @@ -20,8 +20,8 @@ def test_against_production_shed(tmp_path: Path): repo_owner = "iuc" - repo_name = "collection_column_join" - repo_revision = "dfde09461b1e" + repo_name = "featurecounts" + repo_revision = "f9d49f5cb597" install_target: InstallationTarget = StandaloneInstallationTarget(tmp_path) install_manager = InstallRepositoryManager(install_target) @@ -33,10 +33,13 @@ def test_against_production_shed(tmp_path: Path): repo_revision, # revision 2, a known installable revision install_options, ) + tool_guid = f"toolshed.g2.bx.psu.edu/repos/{repo_owner}/{repo_name}/featurecounts/2.0.3+galaxy2" with open(tmp_path / "shed_conf.xml") as f: - assert "toolshed.g2.bx.psu.edu/repos/iuc/collection_column_join/collection_column_join/0.0.2" in f.read() + assert tool_guid in f.read() repo_path = tmp_path / "tools" / "toolshed.g2.bx.psu.edu" / "repos" / repo_owner / repo_name / repo_revision assert repo_path.exists() + tool_data_table_path = tmp_path / "tool_data" / "toolshed.g2.bx.psu.edu" / "repos" / repo_owner / repo_name / repo_revision / "tool_data_table_conf.xml" + assert tool_data_table_path.exists() install_model_context = install_target.install_model.context query = install_model_context.query(ToolShedRepository).where(ToolShedRepository.name == repo_name) @@ -54,7 +57,7 @@ def test_against_production_shed(tmp_path: Path): assert not errors with open(tmp_path / "shed_conf.xml") as f: - assert "toolshed.g2.bx.psu.edu/repos/iuc/collection_column_join/collection_column_join/0.0.2" not in f.read() + assert tool_guid not in f.read() - repo_path = tmp_path / "tools" / "toolshed.g2.bx.psu.edu" / "repos" / repo_owner / repo_name / repo_revision assert not repo_path.exists() + # Tool data tables are not removed when uninstalling a repository