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

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

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(

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)
 

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