Merge branch 'release_21.09' into dev

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}')

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):

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 = """

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)

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',

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'

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"
 

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)