diff --git a/lib/galaxy/webapps/galaxy/controllers/workflow.py b/lib/galaxy/webapps/galaxy/controllers/workflow.py index 3d6dcfd732d..1aa91d2c2c9 100644 --- a/lib/galaxy/webapps/galaxy/controllers/workflow.py +++ b/lib/galaxy/webapps/galaxy/controllers/workflow.py @@ -655,8 +655,7 @@ class WorkflowController(BaseUIController, SharableMixin, UsesStoredWorkflowMixi """ if not id: if workflow_id: - workflow = trans.sa_session.query(model.Workflow).get(trans.security.decode_id(workflow_id)) - stored_workflow = workflow.stored_workflow + stored_workflow = self.app.workflow_manager.get_stored_workflow(trans, workflow_id, by_stored_id=False) self.security_check(trans, stored_workflow, True, False) stored_workflow_id = trans.security.encode_id(stored_workflow.id) return trans.response.send_redirect(f'{url_for("/")}workflow/editor?id={stored_workflow_id}') diff --git a/lib/galaxy/workflow/refactor/execute.py b/lib/galaxy/workflow/refactor/execute.py index aebf7851ac5..c823e428f34 100644 --- a/lib/galaxy/workflow/refactor/execute.py +++ b/lib/galaxy/workflow/refactor/execute.py @@ -391,6 +391,10 @@ class WorkflowRefactorExecutor: self._inject_for_updated_step(step, execution) step_def["tool_version"] = tool_version step_def["tool_state"] = step.module.get_tool_state() + if step_def.get("tool_id"): + step_def["tool_id"] = step.module.get_content_id() + if step_def.get("content_id"): + step_def["content_id"] = step.module.get_content_id() self._patch_step(execution, step, step_def) def _apply_upgrade_all_steps(self, action: UpgradeAllStepsAction, execution: RefactorActionExecution): diff --git a/lib/galaxy_test/base/workflow_fixtures.py b/lib/galaxy_test/base/workflow_fixtures.py index 5b28ee8a6d1..e6187e1d99c 100644 --- a/lib/galaxy_test/base/workflow_fixtures.py +++ b/lib/galaxy_test/base/workflow_fixtures.py @@ -423,6 +423,10 @@ steps: in: input1: nested_workflow/workflow_output queries_0|input2: nested_workflow/workflow_output + compose_text_param: + tool_id: compose_text_param + tool_version: 0.1.0 + label: compose_text_param """ WORKFLOW_WITH_OUTPUT_ACTIONS = """ diff --git a/lib/tool_shed/util/commit_util.py b/lib/tool_shed/util/commit_util.py index 845d52c441d..fcc9e988def 100644 --- a/lib/tool_shed/util/commit_util.py +++ b/lib/tool_shed/util/commit_util.py @@ -17,7 +17,7 @@ from tool_shed.util import basic_util, hg_util, shed_util_common as suc log = logging.getLogger(__name__) -UNDESIRABLE_DIRS = ['.hg', '.svn', '.git', '.cvs'] +UNDESIRABLE_DIRS = ['.hg', '.svn', '.git', '.cvs', '.idea'] UNDESIRABLE_FILES = ['.hg_archival.txt', 'hgrc', '.DS_Store', 'tool_test_output.html', 'tool_test_output.json'] @@ -155,7 +155,6 @@ def handle_directory_changes(app, host, username, repository, full_path, filenam content_alert_str = '' files_to_remove = [] filenames_in_archive = [os.path.join(full_path, name) for name in filenames_in_archive] - repo = repository.hg_repo if remove_repo_files_not_in_tar and not repository.is_new(): # We have a repository that is not new (it contains files), so discover those files that are in the # repository, but not in the uploaded archive. @@ -177,27 +176,11 @@ def handle_directory_changes(app, host, username, repository, full_path, filenam # Remove files in the repository (relative to the upload point) that are not in # the uploaded archive. try: - hg_util.remove_file(repo_path, repo_file, force=True) + hg_util.remove_path(repo_path, repo_file) except Exception as e: - log.debug(f"Error removing files using the mercurial API, so trying a different approach, the error was: {str(e)}") - relative_selected_file = repo_file.split('repo_%d' % repository.id)[1].lstrip('/') - repo.dirstate.remove(relative_selected_file) - repo.dirstate.write() - absolute_selected_file = os.path.abspath(repo_file) - if os.path.isdir(absolute_selected_file): - try: - os.rmdir(absolute_selected_file) - except OSError: - # The directory is not empty. - pass - elif os.path.isfile(absolute_selected_file): - os.remove(absolute_selected_file) - dir = os.path.split(absolute_selected_file)[0] - try: - os.rmdir(dir) - except OSError: - # The directory is not empty. - pass + error_message = (f"Error removing file {repo_file} in mercurial repo:\n{e}") + log.debug(error_message) + return 'error', error_message, files_to_remove, content_alert_str, 0, 0 # See if any admin users have chosen to receive email alerts when a repository is updated. # If so, check every uploaded file to ensure content is appropriate. check_contents = check_file_contents_for_email_alerts(app) diff --git a/lib/tool_shed/util/hg_util.py b/lib/tool_shed/util/hg_util.py index f7de9b28884..82c8feb2bd4 100644 --- a/lib/tool_shed/util/hg_util.py +++ b/lib/tool_shed/util/hg_util.py @@ -1,5 +1,6 @@ import logging import os +import shutil import subprocess import tempfile from datetime import datetime @@ -209,17 +210,24 @@ def get_rev_label_from_changeset_revision(repo, changeset_revision, include_date return rev, label -def remove_file(repo_path, selected_file, force=True): - cmd = ['hg', 'remove'] - if force: - cmd.append('--force') - cmd.append(selected_file) +def remove_path(repo_path, selected_file): + cmd = ['hg', 'remove', '--force', selected_file] try: subprocess.check_output(cmd, stderr=subprocess.STDOUT, cwd=repo_path) except Exception as e: - error_message = f"Error removing file '{selected_file}': {unicodify(e)}" + error_message = f"Error removing path '{selected_file}': {unicodify(e)}" if isinstance(e, subprocess.CalledProcessError): - error_message += f"\nOutput was:\n{unicodify(e.output)}" + output = unicodify(e.output) + if 'is untracked' in output: + # That's ok, happens if we add a new file or directory via tarball upload, + # just delete the file or dir on disk + selected_file_path = os.path.join(repo_path, selected_file) + if os.path.isdir(selected_file_path): + shutil.rmtree(selected_file_path) + else: + os.remove(selected_file_path) + return + error_message += f"\nOutput was:\n{output}" raise Exception(error_message) @@ -270,7 +278,7 @@ __all__ = ( 'get_revision_label_from_ctx', 'get_rev_label_from_changeset_revision', 'pull_repository', - 'remove_file', + 'remove_path', 'reversed_lower_upper_bounded_changelog', 'reversed_upper_bounded_changelog', 'update_repository', diff --git a/lib/tool_shed/webapp/controllers/repository.py b/lib/tool_shed/webapp/controllers/repository.py index cc519bd5f0a..83b49dbcbee 100644 --- a/lib/tool_shed/webapp/controllers/repository.py +++ b/lib/tool_shed/webapp/controllers/repository.py @@ -2157,29 +2157,11 @@ class RepositoryController(BaseUIController, ratings_util.ItemRatings): tip = repository.tip() for selected_file in selected_files_to_delete: try: - hg_util.remove_file(repo_dir, selected_file, force=True) + hg_util.remove_path(repo_dir, selected_file) except Exception as e: - log.debug("Error removing the following file using the mercurial API:\n %s", selected_file) - log.debug("The error was: %s", util.unicodify(e)) - log.debug("Attempting to remove the file using a different approach.") - relative_selected_file = selected_file.split('repo_%d' % repository.id)[1].lstrip('/') - repo.dirstate.remove(relative_selected_file) - repo.dirstate.write() - absolute_selected_file = os.path.abspath(selected_file) - if os.path.isdir(absolute_selected_file): - try: - os.rmdir(absolute_selected_file) - except OSError: - # The directory is not empty - pass - elif os.path.isfile(absolute_selected_file): - os.remove(absolute_selected_file) - dir = os.path.split(absolute_selected_file)[0] - try: - os.rmdir(dir) - except OSError: - # The directory is not empty - pass + status = 'error' + message = f"Error removing file {selected_file} in mercurial repo:\n{e}" + log.debug(message) # Commit the change set. if not commit_message: commit_message = 'Deleted selected files' diff --git a/test/integration/test_workflow_refactoring.py b/test/integration/test_workflow_refactoring.py index 17ba3819c60..719d7f87f0b 100644 --- a/test/integration/test_workflow_refactoring.py +++ b/test/integration/test_workflow_refactoring.py @@ -19,6 +19,9 @@ from galaxy.workflow.refactor.schema import ( from galaxy_test.base.populators import ( WorkflowPopulator, ) +from galaxy_test.base.uses_shed import ( + UsesShed +) from galaxy_test.base.workflow_fixtures import ( WORKFLOW_NESTED_RUNTIME_PARAMETER, WORKFLOW_NESTED_SIMPLE, @@ -42,7 +45,7 @@ steps: """ -class WorkflowRefactoringIntegrationTestCase(integration_util.IntegrationTestCase): +class WorkflowRefactoringIntegrationTestCase(integration_util.IntegrationTestCase, UsesShed): framework_tool_and_types = True @@ -693,6 +696,8 @@ steps: assert message.output_label == "outer_output" def test_upgrade_all_steps(self): + self.install_repository("iuc", "compose_text_param", "feb3acba1e0a") # 0.1.0 + self.install_repository("iuc", "compose_text_param", "e188c9826e0f") # 0.1.1 self.workflow_populator.upload_yaml_workflow(WORKFLOW_NESTED_WITH_MULTIPLE_VERSIONS_TOOL) nested_stored_workflow = self._recent_stored_workflow(2) assert self._latest_workflow.step_by_label("tool_update_step").tool_version == "0.1" @@ -716,13 +721,15 @@ steps: nested_stored_workflow = self._recent_stored_workflow(2) updated_nested_step = nested_stored_workflow.latest_workflow.step_by_label("random_lines") assert updated_nested_step.tool_inputs["num_lines"] == "2" + assert self._latest_workflow.step_by_label("compose_text_param").tool_version == '0.1.1' + assert self._latest_workflow.step_by_label("compose_text_param").tool_id == 'toolshed.g2.bx.psu.edu/repos/iuc/compose_text_param/compose_text_param/0.1.1' assert len(action_executions) == 1 messages = action_executions[0].messages assert len(messages) == 1 message = messages[0] assert message.message_type == RefactorActionExecutionMessageTypeEnum.connection_drop_forced - assert message.order_index == 1 + assert message.order_index == 2 assert message.step_label == "tool_update_step" assert message.output_name == "output" diff --git a/test/unit/shed_unit/test_hg_util.py b/test/unit/shed_unit/test_hg_util.py new file mode 100644 index 00000000000..b5d2f88b165 --- /dev/null +++ b/test/unit/shed_unit/test_hg_util.py @@ -0,0 +1,57 @@ +import pytest + +from tool_shed.util import hg_util + + +def test_init_repository(tmpdir): + hg_util.init_repository(str(tmpdir)) + assert (tmpdir / '.hg').exists() + + +def test_init_repository_fails(tmpdir): + test_init_repository(tmpdir) + with pytest.raises(Exception) as exc_info: + test_init_repository(tmpdir) + assert 'Error initializing repository' in str(exc_info.value) + + +def test_add_file_and_commmit_changeset(tmpdir): + test_init_repository(tmpdir) + path_to_add = (tmpdir / 'test.txt') + path_to_add.write('test') + hg_util.add_changeset(str(tmpdir), str(path_to_add)) + hg_util.commit_changeset(str(tmpdir), str(path_to_add), 'testuser', 'testcommit') + return path_to_add + + +def test_add_dir_and_commit_changeset(tmpdir): + test_init_repository(tmpdir) + path_to_add = tmpdir.mkdir('abc') + (path_to_add / 'test.txt').write('bla') + hg_util.add_changeset(str(tmpdir), str(path_to_add)) + hg_util.commit_changeset(str(tmpdir), str(path_to_add), 'testuser', 'testcommit') + return path_to_add + + +def test_remove_tracked_file(tmpdir): + path_to_remove = test_add_file_and_commmit_changeset(tmpdir) + hg_util.remove_path(str(tmpdir), path_to_remove) + + +def test_remove_untracked_file(tmpdir): + test_init_repository(tmpdir) + untracked_path = tmpdir / 'untracked.txt' + untracked_path.write('bla') + hg_util.remove_path(str(tmpdir), str(untracked_path)) + + +def test_remove_tracked_dir(tmpdir): + path_to_remove = test_add_dir_and_commit_changeset(tmpdir) + hg_util.remove_path(str(tmpdir), path_to_remove) + + +def test_remove_nonexistant_file_fails(tmpdir): + test_init_repository(tmpdir) + with pytest.raises(Exception) as exc_info: + hg_util.remove_path(str(tmpdir), str(tmpdir / 'some path')) + assert 'Error removing path' in str(exc_info.value)