Merge branch 'eg/p4-submit-catch-failure' into maint
authorJeff King <peff@peff.net>
Tue, 1 Dec 2015 22:24:20 +0000 (17:24 -0500)
committerJeff King <peff@peff.net>
Tue, 1 Dec 2015 22:24:21 +0000 (17:24 -0500)
Just like the working tree is cleaned up when the user cancelled
submission in P4Submit.applyCommit(), clean up the mess if "p4
submit" fails.

* eg/p4-submit-catch-failure:
git-p4: clean up after p4 submit failure

1  2 
git-p4.py
diff --combined git-p4.py
index 4ea1553c6f943e7c8d7ccf3696d827454110ae34,d53590435a0ff29c82ac313162d3cc7de02e5f2f..a79b6d82ab5ac8def38e7a213f0b990f4688379a
+++ b/git-p4.py
@@@ -134,11 -134,13 +134,11 @@@ def read_pipe(c, ignore_error=False)
          sys.stderr.write('Reading pipe: %s\n' % str(c))
  
      expand = isinstance(c,basestring)
 -    p = subprocess.Popen(c, stdout=subprocess.PIPE, shell=expand)
 -    pipe = p.stdout
 -    val = pipe.read()
 -    if p.wait() and not ignore_error:
 -        die('Command failed: %s' % str(c))
 -
 -    return val
 +    p = subprocess.Popen(c, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=expand)
 +    (out, err) = p.communicate()
 +    if p.returncode != 0 and not ignore_error:
 +        die('Command failed: %s\nError: %s' % (str(c), err))
 +    return out
  
  def p4_read_pipe(c, ignore_error=False):
      real_cmd = p4_build_cmd(c)
@@@ -190,16 -192,14 +190,16 @@@ def p4_has_move_command()
      # assume it failed because @... was invalid changelist
      return True
  
 -def system(cmd):
 +def system(cmd, ignore_error=False):
      expand = isinstance(cmd,basestring)
      if verbose:
          sys.stderr.write("executing %s\n" % str(cmd))
      retcode = subprocess.call(cmd, shell=expand)
 -    if retcode:
 +    if retcode and not ignore_error:
          raise CalledProcessError(retcode, cmd)
  
 +    return retcode
 +
  def p4_system(cmd):
      """Specifically invoke p4 as the system command. """
      real_cmd = p4_build_cmd(cmd)
@@@ -542,12 -542,7 +542,12 @@@ def p4Where(depotPath)
      return clientPath
  
  def currentGitBranch():
 -    return read_pipe("git name-rev HEAD").split(" ")[1].strip()
 +    retcode = system(["git", "symbolic-ref", "-q", "HEAD"], ignore_error=True)
 +    if retcode != 0:
 +        # on a detached head
 +        return None
 +    else:
 +        return read_pipe(["git", "name-rev", "HEAD"]).split(" ")[1].strip()
  
  def isValidGitDir(path):
      if (os.path.exists(path + "/HEAD")
@@@ -1543,44 -1538,47 +1543,47 @@@ class P4Submit(Command, P4UserMap)
          #
          # Let the user edit the change description, then submit it.
          #
-         if self.edit_template(fileName):
-             # read the edited message and submit
-             ret = True
-             tmpFile = open(fileName, "rb")
-             message = tmpFile.read()
-             tmpFile.close()
-             if self.isWindows:
-                 message = message.replace("\r\n", "\n")
-             submitTemplate = message[:message.index(separatorLine)]
-             p4_write_pipe(['submit', '-i'], submitTemplate)
-             if self.preserveUser:
-                 if p4User:
-                     # Get last changelist number. Cannot easily get it from
-                     # the submit command output as the output is
-                     # unmarshalled.
-                     changelist = self.lastP4Changelist()
-                     self.modifyChangelistUser(changelist, p4User)
-             # The rename/copy happened by applying a patch that created a
-             # new file.  This leaves it writable, which confuses p4.
-             for f in pureRenameCopy:
-                 p4_sync(f, "-f")
+         submitted = False
  
-         else:
+         try:
+             if self.edit_template(fileName):
+                 # read the edited message and submit
+                 tmpFile = open(fileName, "rb")
+                 message = tmpFile.read()
+                 tmpFile.close()
+                 if self.isWindows:
+                     message = message.replace("\r\n", "\n")
+                 submitTemplate = message[:message.index(separatorLine)]
+                 p4_write_pipe(['submit', '-i'], submitTemplate)
+                 if self.preserveUser:
+                     if p4User:
+                         # Get last changelist number. Cannot easily get it from
+                         # the submit command output as the output is
+                         # unmarshalled.
+                         changelist = self.lastP4Changelist()
+                         self.modifyChangelistUser(changelist, p4User)
+                 # The rename/copy happened by applying a patch that created a
+                 # new file.  This leaves it writable, which confuses p4.
+                 for f in pureRenameCopy:
+                     p4_sync(f, "-f")
+                 submitted = True
+         finally:
              # skip this patch
-             ret = False
-             print "Submission cancelled, undoing p4 changes."
-             for f in editedFiles:
-                 p4_revert(f)
-             for f in filesToAdd:
-                 p4_revert(f)
-                 os.remove(f)
-             for f in filesToDelete:
-                 p4_revert(f)
+             if not submitted:
+                 print "Submission cancelled, undoing p4 changes."
+                 for f in editedFiles:
+                     p4_revert(f)
+                 for f in filesToAdd:
+                     p4_revert(f)
+                     os.remove(f)
+                 for f in filesToDelete:
+                     p4_revert(f)
  
          os.remove(fileName)
-         return ret
+         return submitted
  
      # Export git tags as p4 labels. Create a p4 label and then tag
      # with that.
      def run(self, args):
          if len(args) == 0:
              self.master = currentGitBranch()
 -            if len(self.master) == 0 or not gitBranchExists("refs/heads/%s" % self.master):
 -                die("Detecting current git branch failed!")
          elif len(args) == 1:
              self.master = args[0]
              if not branchExists(self.master):
          else:
              return False
  
 -        allowSubmit = gitConfig("git-p4.allowSubmit")
 -        if len(allowSubmit) > 0 and not self.master in allowSubmit.split(","):
 -            die("%s is not in git-p4.allowSubmit" % self.master)
 +        if self.master:
 +            allowSubmit = gitConfig("git-p4.allowSubmit")
 +            if len(allowSubmit) > 0 and not self.master in allowSubmit.split(","):
 +                die("%s is not in git-p4.allowSubmit" % self.master)
  
          [upstream, settings] = findUpstreamBranchPoint()
          self.depotPath = settings['depot-paths'][0]
          self.check()
  
          commits = []
 -        for line in read_pipe_lines(["git", "rev-list", "--no-merges", "%s..%s" % (self.origin, self.master)]):
 +        if self.master:
 +            commitish = self.master
 +        else:
 +            commitish = 'HEAD'
 +
 +        for line in read_pipe_lines(["git", "rev-list", "--no-merges", "%s..%s" % (self.origin, commitish)]):
              commits.append(line.strip())
          commits.reverse()
  
@@@ -2202,17 -2196,10 +2205,17 @@@ class P4Sync(Command, P4UserMap)
              # them back too.  This is not needed to the cygwin windows version,
              # just the native "NT" type.
              #
 -            text = p4_read_pipe(['print', '-q', '-o', '-', "%s@%s" % (file['depotFile'], file['change']) ])
 -            if p4_version_string().find("/NT") >= 0:
 -                text = text.replace("\r\n", "\n")
 -            contents = [ text ]
 +            try:
 +                text = p4_read_pipe(['print', '-q', '-o', '-', '%s@%s' % (file['depotFile'], file['change'])])
 +            except Exception as e:
 +                if 'Translation of file content failed' in str(e):
 +                    type_base = 'binary'
 +                else:
 +                    raise e
 +            else:
 +                if p4_version_string().find('/NT') >= 0:
 +                    text = text.replace('\r\n', '\n')
 +                contents = [ text ]
  
          if type_base == "apple":
              # Apple filetype files will be streamed as a concatenation of
          else:
              return "%s <a@b>" % userid
  
 -    # Stream a p4 tag
      def streamTag(self, gitStream, labelName, labelDetails, commit, epoch):
 +        """ Stream a p4 tag.
 +        commit is either a git commit, or a fast-import mark, ":<p4commit>"
 +        """
 +
          if verbose:
              print "writing tag %s for commit %s" % (labelName, commit)
          gitStream.write("tag %s\n" % labelName)
              self.clientSpecDirs.update_client_spec_path_cache(files)
  
          self.gitStream.write("commit %s\n" % branch)
 -#        gitStream.write("mark :%s\n" % details["change"])
 +        self.gitStream.write("mark :%s\n" % details["change"])
          self.committedChanges.add(int(details["change"]))
          committer = ""
          if author not in self.users:
              if change.has_key('change'):
                  # find the corresponding git commit; take the oldest commit
                  changelist = int(change['change'])
 -                gitCommit = read_pipe(["git", "rev-list", "--max-count=1",
 -                     "--reverse", ":/\[git-p4:.*change = %d\]" % changelist])
 -                if len(gitCommit) == 0:
 -                    print "could not find git commit for changelist %d" % changelist
 -                else:
 -                    gitCommit = gitCommit.strip()
 +                if changelist in self.committedChanges:
 +                    gitCommit = ":%d" % changelist       # use a fast-import mark
                      commitFound = True
 +                else:
 +                    gitCommit = read_pipe(["git", "rev-list", "--max-count=1",
 +                        "--reverse", ":/\[git-p4:.*change = %d\]" % changelist], ignore_error=True)
 +                    if len(gitCommit) == 0:
 +                        print "importing label %s: could not find git commit for changelist %d" % (name, changelist)
 +                    else:
 +                        commitFound = True
 +                        gitCommit = gitCommit.strip()
 +
 +                if commitFound:
                      # Convert from p4 time format
                      try:
                          tmwhen = time.strptime(labelDetails['Update'], "%Y/%m/%d %H:%M:%S")