From 54d48766ae177f2d8d75704962f4e90cc35fb95d Mon Sep 17 00:00:00 2001 From: Darragh Bailey Date: Wed, 28 Sep 2016 12:12:12 +0100 Subject: [PATCH] Always have rebase perform finish To ensure consistent behaviour around encountering conflicts whether in interactive mode or not, or whether there are changes to be carried or not, always have rebase perform the finish step whenever the merge behaviour is enabled. Change-Id: I8f72ebb9fb0fa9fff4943e732cbf4f7d33672837 Related-Bug: #1625876 --- git_upstream/commands/import.py | 41 +++++++++---------- git_upstream/lib/importupstream.py | 10 ++++- git_upstream/lib/rebaseeditor.py | 6 ++- .../tests/commands/import/test_import.py | 5 +++ 4 files changed, 38 insertions(+), 24 deletions(-) diff --git a/git_upstream/commands/import.py b/git_upstream/commands/import.py index f382542..f223e71 100644 --- a/git_upstream/commands/import.py +++ b/git_upstream/commands/import.py @@ -118,25 +118,19 @@ class ImportCommand(LogDedentMixin, GitUpstreamCommand): else: self.args.real_upstream_branch = self.args.upstream_branch - def _finish(self, import_upstream): - self.log.notice("Merging import to requested branch '%s'", - self.args.branch) - if import_upstream.finish(): - self.log.notice( - """ - Successfully finished import: - target branch: '%s' - upstream branch: '%s' - import branch: '%s'""", - self.args.branch, self.args.real_upstream_branch, - import_upstream.import_branch) - if self.args.branches: - for branch in self.args.branches: - self.log.notice(" extra branch: '%s'", branch, - dedent=False) - return True - else: - return False + def _finish_report(self, import_upstream): + self.log.notice( + """ + Successfully finished import: + target branch: '%s' + upstream branch: '%s' + import branch: '%s'""", + self.args.branch, self.args.real_upstream_branch, + import_upstream.import_branch) + if self.args.branches: + for branch in self.args.branches: + self.log.notice(" extra branch: '%s'", branch, + dedent=False) def execute(self): @@ -174,7 +168,11 @@ class ImportCommand(LogDedentMixin, GitUpstreamCommand): # finish and return if thats all if self.args.finish: - return self._finish(import_upstream) + if import_upstream.finish(): + self._finish_report(import_upstream) + return True + else: + return False # otherwise perform fresh import self.log.notice("Starting import of upstream") @@ -205,6 +203,7 @@ class ImportCommand(LogDedentMixin, GitUpstreamCommand): """, self.args.branch) return True - return self._finish(import_upstream) + self._finish_report(import_upstream) + return True # vim:sw=4:sts=4:ts=4:et: diff --git a/git_upstream/lib/importupstream.py b/git_upstream/lib/importupstream.py index 91c7c39..09c7e2e 100644 --- a/git_upstream/lib/importupstream.py +++ b/git_upstream/lib/importupstream.py @@ -271,7 +271,11 @@ class ImportUpstream(LogDedentMixin, GitMixin): if len(commit_list) == 0: self.log.notice("All carried changes gone upstream") self._set_branch(self.import_branch, self.upstream, force=True) - return True + # no resume_cmdline means to skip merge + if resume_cmdline is None: + return True + # otherwise perform the finish + return self.finish() self.log.debug( """ @@ -351,6 +355,7 @@ class ImportUpstream(LogDedentMixin, GitMixin): return False self.log.notice("Successfully applied all locally carried changes") + self.git.checkout(self.branch) else: self.log.warning("Warning, nothing to do: locally carried " + "changes already rebased onto " + self.upstream) @@ -366,6 +371,8 @@ class ImportUpstream(LogDedentMixin, GitMixin): Finish the import by merging the import branch to the target while performing suitable verification checks. """ + self.log.notice("Merging import to requested branch '%s'", + self.branch) self.log.info("No verification checks enabled") in_rebase = False if self.is_detached(): @@ -413,6 +420,7 @@ class ImportUpstream(LogDedentMixin, GitMixin): raise ImportUpstreamError( "Resulting tree does not match import") if in_rebase: + self.log.info("Code thinks we're in the middle of a rebase") self.git.checkout(target_sha) except (GitCommandError, ImportUpstreamError) as e: self.log.error( diff --git a/git_upstream/lib/rebaseeditor.py b/git_upstream/lib/rebaseeditor.py index 44b0fab..3a74fa8 100644 --- a/git_upstream/lib/rebaseeditor.py +++ b/git_upstream/lib/rebaseeditor.py @@ -195,6 +195,10 @@ class RebaseEditor(GitMixin, LogDedentMixin): cmd = ['git', 'rebase', '--interactive'] cmd.extend(self.git.transform_kwargs(**kwargs)) cmd.extend(args) + + # ensure that the finish will always be called + self._insert_exec_to_todo() + mode = os.environ.get('TEST_GIT_UPSTREAM_REBASE_EDITOR', "") if mode.lower() == "debug": # In general it's not recommended to run rebase in direct @@ -239,8 +243,6 @@ class RebaseEditor(GitMixin, LogDedentMixin): finally: self.cleanup() else: - self._insert_exec_to_todo() - cmd.append(environ) os.execlpe('git', *cmd) diff --git a/git_upstream/tests/commands/import/test_import.py b/git_upstream/tests/commands/import/test_import.py index f317c48..66554a4 100644 --- a/git_upstream/tests/commands/import/test_import.py +++ b/git_upstream/tests/commands/import/test_import.py @@ -40,6 +40,11 @@ class TestImportCommand(TestWithScenarios, BaseTestCase): self.addDetail('description', text_content(self.desc)) self.commands, self.parser = main.build_parsers() + + script_cmdline = self.parser.get_default('script_cmdline') + script_cmdline[-1] = os.path.join(os.getcwd(), main.__file__) + self.parser.set_defaults(script_cmdline=script_cmdline) + # builds the tree to be tested super(TestImportCommand, self).setUp()