Merge branch 'ls/p4-keep-empty-commits' into maint
authorJunio C Hamano <gitster@pobox.com>
Tue, 15 Dec 2015 17:34:18 +0000 (09:34 -0800)
committerJunio C Hamano <gitster@pobox.com>
Tue, 15 Dec 2015 17:34:19 +0000 (09:34 -0800)
"git p4" used to import Perforce CLs that touch only paths outside
the client spec as empty commits. It has been corrected to ignore
them instead, with a new configuration git-p4.keepEmptyCommits as a
backward compatibility knob.

* ls/p4-keep-empty-commits:
git-p4: add option to keep empty commits

1  2 
git-p4.py
diff --combined git-p4.py
index a79b6d82ab5ac8def38e7a213f0b990f4688379a,4ad7c6890cf57860a280169e73188a443cdc949c..a7ec1187066b90675f55e44103ba54e03d132caa
+++ b/git-p4.py
@@@ -43,9 -43,6 +43,9 @@@ verbose = Fals
  # Only labels/tags matching this will be imported/exported
  defaultLabelRegexp = r'[a-zA-Z0-9_\-.]+$'
  
 +# Grab changes in blocks of this many revisions, unless otherwise requested
 +defaultBlockSize = 512
 +
  def p4_build_cmd(cmd):
      """Build a suitable p4 command line.
  
@@@ -134,11 -131,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 -189,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)
@@@ -252,10 -249,6 +252,10 @@@ def p4_reopen(type, f)
  def p4_move(src, dest):
      p4_system(["move", "-k", wildcard_encode(src), wildcard_encode(dest)])
  
 +def p4_last_change():
 +    results = p4CmdList(["changes", "-m", "1"])
 +    return int(results[0]['change'])
 +
  def p4_describe(change):
      """Make sure it returns a valid result by checking for
         the presence of field "time".  Return a dict of the
@@@ -509,14 -502,12 +509,14 @@@ def p4Cmd(cmd)
  def p4Where(depotPath):
      if not depotPath.endswith("/"):
          depotPath += "/"
 -    depotPath = depotPath + "..."
 -    outputList = p4CmdList(["where", depotPath])
 +    depotPathLong = depotPath + "..."
 +    outputList = p4CmdList(["where", depotPathLong])
      output = None
      for entry in outputList:
          if "depotFile" in entry:
 -            if entry["depotFile"] == depotPath:
 +            # Search for the base client side depot path, as long as it starts with the branch's P4 path.
 +            # The base path always ends with "/...".
 +            if entry["depotFile"].find(depotPath) == 0 and entry["depotFile"][-4:] == "/...":
                  output = entry
                  break
          elif "data" in entry:
      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")
@@@ -754,77 -740,43 +754,77 @@@ def createOrUpdateBranchesFromOrigin(lo
  def originP4BranchesExist():
          return gitBranchExists("origin") or gitBranchExists("origin/p4") or gitBranchExists("origin/p4/master")
  
 -def p4ChangesForPaths(depotPaths, changeRange, block_size):
 +
 +def p4ParseNumericChangeRange(parts):
 +    changeStart = int(parts[0][1:])
 +    if parts[1] == '#head':
 +        changeEnd = p4_last_change()
 +    else:
 +        changeEnd = int(parts[1])
 +
 +    return (changeStart, changeEnd)
 +
 +def chooseBlockSize(blockSize):
 +    if blockSize:
 +        return blockSize
 +    else:
 +        return defaultBlockSize
 +
 +def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize):
      assert depotPaths
 -    assert block_size
  
 -    # Parse the change range into start and end
 +    # Parse the change range into start and end. Try to find integer
 +    # revision ranges as these can be broken up into blocks to avoid
 +    # hitting server-side limits (maxrows, maxscanresults). But if
 +    # that doesn't work, fall back to using the raw revision specifier
 +    # strings, without using block mode.
 +
      if changeRange is None or changeRange == '':
 -        changeStart = '@1'
 -        changeEnd = '#head'
 +        changeStart = 1
 +        changeEnd = p4_last_change()
 +        block_size = chooseBlockSize(requestedBlockSize)
      else:
          parts = changeRange.split(',')
          assert len(parts) == 2
 -        changeStart = parts[0]
 -        changeEnd = parts[1]
 +        try:
 +            (changeStart, changeEnd) = p4ParseNumericChangeRange(parts)
 +            block_size = chooseBlockSize(requestedBlockSize)
 +        except:
 +            changeStart = parts[0][1:]
 +            changeEnd = parts[1]
 +            if requestedBlockSize:
 +                die("cannot use --changes-block-size with non-numeric revisions")
 +            block_size = None
  
      # Accumulate change numbers in a dictionary to avoid duplicates
      changes = {}
  
      for p in depotPaths:
          # Retrieve changes a block at a time, to prevent running
 -        # into a MaxScanRows error from the server.
 -        start = changeStart
 -        end = changeEnd
 -        get_another_block = True
 -        while get_another_block:
 -            new_changes = []
 +        # into a MaxResults/MaxScanRows error from the server.
 +
 +        while True:
              cmd = ['changes']
 -            cmd += ['-m', str(block_size)]
 -            cmd += ["%s...%s,%s" % (p, start, end)]
 +
 +            if block_size:
 +                end = min(changeEnd, changeStart + block_size)
 +                revisionRange = "%d,%d" % (changeStart, end)
 +            else:
 +                revisionRange = "%s,%s" % (changeStart, changeEnd)
 +
 +            cmd += ["%s...@%s" % (p, revisionRange)]
 +
              for line in p4_read_pipe_lines(cmd):
                  changeNum = int(line.split(" ")[1])
 -                new_changes.append(changeNum)
                  changes[changeNum] = True
 -            if len(new_changes) == block_size:
 -                get_another_block = True
 -                end = '@' + str(min(new_changes))
 -            else:
 -                get_another_block = False
 +
 +            if not block_size:
 +                break
 +
 +            if end >= changeEnd:
 +                break
 +
 +            changeStart = end + 1
  
      changelist = changes.keys()
      changelist.sort()
@@@ -1294,7 -1246,7 +1294,7 @@@ class P4Submit(Command, P4UserMap)
              editor = os.environ.get("P4EDITOR")
          else:
              editor = read_pipe("git var GIT_EDITOR").strip()
 -        system([editor, template_file])
 +        system(["sh", "-c", ('%s "$@"' % editor), editor, template_file])
  
          # If the file was not saved, prompt to see if this patch should
          # be skipped.  But skip this verification step if configured so.
          #
          # 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]
          if self.useClientSpec:
              self.clientSpecDirs = getClientSpec()
  
 -        if self.useClientSpec:
 +        # Check for the existance of P4 branches
 +        branchesDetected = (len(p4BranchesInGit().keys()) > 1)
 +
 +        if self.useClientSpec and not branchesDetected:
              # all files are relative to the client spec
              self.clientPath = getClientRoot()
          else:
          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()
  
@@@ -1962,14 -1904,10 +1962,14 @@@ class View(object)
              if "unmap" in res:
                  # it will list all of them, but only one not unmap-ped
                  continue
 +            if gitConfigBool("core.ignorecase"):
 +                res['depotFile'] = res['depotFile'].lower()
              self.client_spec_path_cache[res['depotFile']] = self.convert_client_path(res["clientFile"])
  
          # not found files or unmap files set to ""
          for depotFile in fileArgs:
 +            if gitConfigBool("core.ignorecase"):
 +                depotFile = depotFile.lower()
              if depotFile not in self.client_spec_path_cache:
                  self.client_spec_path_cache[depotFile] = ""
  
             depot file should live.  Returns "" if the file should
             not be mapped in the client."""
  
 +        if gitConfigBool("core.ignorecase"):
 +            depot_path = depot_path.lower()
 +
          if depot_path in self.client_spec_path_cache:
              return self.client_spec_path_cache[depot_path]
  
@@@ -2034,7 -1969,7 +2034,7 @@@ class P4Sync(Command, P4UserMap)
          self.syncWithOrigin = True
          self.importIntoRemotes = True
          self.maxChanges = ""
 -        self.changes_block_size = 500
 +        self.changes_block_size = None
          self.keepRepoPath = False
          self.depotPaths = None
          self.p4BranchesInGit = []
              # them back too.  This is not needed to the cygwin windows version,
              # just the native "NT" type.
              #
 -            text = p4_read_pipe(['print', '-q', '-o', '-', file['depotFile']])
 -            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
          filesToDelete = []
  
          for f in files:
-             # if using a client spec, only add the files that have
-             # a path in the client
-             if self.clientSpecDirs:
-                 if self.clientSpecDirs.map_in_client(f['path']) == "":
-                     continue
              filesForCommit.append(f)
              if f['action'] in self.delete_actions:
                  filesToDelete.append(f)
          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)
          gitStream.write(description)
          gitStream.write("\n")
  
+     def inClientSpec(self, path):
+         if not self.clientSpecDirs:
+             return True
+         inClientSpec = self.clientSpecDirs.map_in_client(path)
+         if not inClientSpec and self.verbose:
+             print('Ignoring file outside of client spec: {0}'.format(path))
+         return inClientSpec
+     def hasBranchPrefix(self, path):
+         if not self.branchPrefixes:
+             return True
+         hasPrefix = [p for p in self.branchPrefixes
+                         if p4PathStartsWith(path, p)]
+         if hasPrefix and self.verbose:
+             print('Ignoring file outside of prefix: {0}'.format(path))
+         return hasPrefix
      def commit(self, details, files, branch, parent = ""):
          epoch = details["time"]
          author = details["user"]
  
          if self.verbose:
-             print "commit into %s" % branch
-         # start with reading files; if that fails, we should not
-         # create a commit.
-         new_files = []
-         for f in files:
-             if [p for p in self.branchPrefixes if p4PathStartsWith(f['path'], p)]:
-                 new_files.append (f)
-             else:
-                 sys.stderr.write("Ignoring file outside of prefix: %s\n" % f['path'])
+             print('commit into {0}'.format(branch))
  
          if self.clientSpecDirs:
              self.clientSpecDirs.update_client_spec_path_cache(files)
  
+         files = [f for f in files
+             if self.inClientSpec(f['path']) and self.hasBranchPrefix(f['path'])]
+         if not files and not gitConfigBool('git-p4.keepEmptyCommits'):
+             print('Ignoring revision {0} as it would produce an empty commit.'
+                 .format(details['change']))
+             return
          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:
                  print "parent %s" % parent
              self.gitStream.write("from %s\n" % parent)
  
-         self.streamP4Files(new_files)
+         self.streamP4Files(files)
          self.gitStream.write("\n")
  
          change = int(details["change"])
              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")