Merge branch 'ld/p4-editor-multi-words'
authorJunio C Hamano <gitster@pobox.com>
Fri, 5 Jun 2015 19:17:38 +0000 (12:17 -0700)
committerJunio C Hamano <gitster@pobox.com>
Fri, 5 Jun 2015 19:17:38 +0000 (12:17 -0700)
Unlike "$EDITOR" and "$GIT_EDITOR" that can hold the path to the
command and initial options (e.g. "/path/to/emacs -nw"), 'git p4'
did not let the shell interpolate the contents of the environment
variable that name the editor "$P4EDITOR" (and "$EDITOR", too).
Make it in line with the rest of Git, as well as with Perforce.

* ld/p4-editor-multi-words:
git-p4: tests: use test-chmtime in place of touch
git-p4: fix handling of multi-word P4EDITOR
git-p4: add failing test for P4EDITOR handling

1  2 
git-p4.py
diff --combined git-p4.py
index 41a77e6648ddad9a7599452bf361fd13ff4dcf88,de06046d60f30edf615b3a6ff80c6b71b9a438ad..ca6bb95c573608f8477ed7f3388c5df1e63f8041
+++ b/git-p4.py
@@@ -368,7 -368,7 +368,7 @@@ def getP4OpenedType(file)
      # Returns the perforce file type for the given file.
  
      result = p4_read_pipe(["opened", wildcard_encode(file)])
 -    match = re.match(".*\((.+)\)\r?$", result)
 +    match = re.match(".*\((.+)\)( \*exclusive\*)?\r?$", result)
      if match:
          return match.group(1)
      else:
@@@ -502,14 -502,12 +502,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:
@@@ -742,43 -740,17 +742,43 @@@ def createOrUpdateBranchesFromOrigin(lo
  def originP4BranchesExist():
          return gitBranchExists("origin") or gitBranchExists("origin/p4") or gitBranchExists("origin/p4/master")
  
 -def p4ChangesForPaths(depotPaths, changeRange):
 +def p4ChangesForPaths(depotPaths, changeRange, block_size):
      assert depotPaths
 -    cmd = ['changes']
 -    for p in depotPaths:
 -        cmd += ["%s...%s" % (p, changeRange)]
 -    output = p4_read_pipe_lines(cmd)
 +    assert block_size
 +
 +    # Parse the change range into start and end
 +    if changeRange is None or changeRange == '':
 +        changeStart = '@1'
 +        changeEnd = '#head'
 +    else:
 +        parts = changeRange.split(',')
 +        assert len(parts) == 2
 +        changeStart = parts[0]
 +        changeEnd = parts[1]
  
 +    # Accumulate change numbers in a dictionary to avoid duplicates
      changes = {}
 -    for line in output:
 -        changeNum = int(line.split(" ")[1])
 -        changes[changeNum] = True
 +
 +    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 = []
 +            cmd = ['changes']
 +            cmd += ['-m', str(block_size)]
 +            cmd += ["%s...%s,%s" % (p, start, end)]
 +            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
  
      changelist = changes.keys()
      changelist.sort()
@@@ -1248,7 -1220,7 +1248,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.
              print "  " + self.clientPath
              print
              print "To submit, use \"p4 submit\" to write a new description,"
 -            print "or \"p4 submit -i %s\" to use the one prepared by" \
 +            print "or \"p4 submit -i <%s\" to use the one prepared by" \
                    " \"git p4\"." % fileName
              print "You can delete the file \"%s\" when finished." % fileName
  
          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:
@@@ -1942,17 -1911,11 +1942,17 @@@ class P4Sync(Command, P4UserMap)
                  optparse.make_option("--import-labels", dest="importLabels", action="store_true"),
                  optparse.make_option("--import-local", dest="importIntoRemotes", action="store_false",
                                       help="Import into refs/heads/ , not refs/remotes"),
 -                optparse.make_option("--max-changes", dest="maxChanges"),
 +                optparse.make_option("--max-changes", dest="maxChanges",
 +                                     help="Maximum number of changes to import"),
 +                optparse.make_option("--changes-block-size", dest="changes_block_size", type="int",
 +                                     help="Internal block size to use when iteratively calling p4 changes"),
                  optparse.make_option("--keep-path", dest="keepRepoPath", action='store_true',
                                       help="Keep entire BRANCH/DIR/SUBDIR prefix during import"),
                  optparse.make_option("--use-client-spec", dest="useClientSpec", action='store_true',
 -                                     help="Only sync files that are included in the Perforce Client Spec")
 +                                     help="Only sync files that are included in the Perforce Client Spec"),
 +                optparse.make_option("-/", dest="cloneExclude",
 +                                     action="append", type="string",
 +                                     help="exclude depot path"),
          ]
          self.description = """Imports from Perforce into a git repository.\n
      example:
          self.syncWithOrigin = True
          self.importIntoRemotes = True
          self.maxChanges = ""
 +        self.changes_block_size = 500
          self.keepRepoPath = False
          self.depotPaths = None
          self.p4BranchesInGit = []
          if gitConfig("git-p4.syncFromOrigin") == "false":
              self.syncWithOrigin = False
  
 +    # This is required for the "append" cloneExclude action
 +    def ensure_value(self, attr, value):
 +        if not hasattr(self, attr) or getattr(self, attr) is None:
 +            setattr(self, attr, value)
 +        return getattr(self, attr)
 +
      # Force a checkpoint in fast-import and wait for it to finish
      def checkpoint(self):
          self.gitStream.write("checkpoint\n\n")
          branchPrefix = self.depotPaths[0] + branch + "/"
          range = "@1,%s" % maxChange
          #print "prefix" + branchPrefix
 -        changes = p4ChangesForPaths([branchPrefix], range)
 +        changes = p4ChangesForPaths([branchPrefix], range, self.changes_block_size)
          if len(changes) <= 0:
              return False
          firstChange = changes[0]
                  if self.verbose:
                      print "Getting p4 changes for %s...%s" % (', '.join(self.depotPaths),
                                                                self.changeRange)
 -                changes = p4ChangesForPaths(self.depotPaths, self.changeRange)
 +                changes = p4ChangesForPaths(self.depotPaths, self.changeRange, self.changes_block_size)
  
                  if len(self.maxChanges) > 0:
                      changes = changes[:min(int(self.maxChanges), len(changes))]
@@@ -3145,6 -3101,9 +3145,6 @@@ class P4Clone(P4Sync)
              optparse.make_option("--destination", dest="cloneDestination",
                                   action='store', default=None,
                                   help="where to leave result of the clone"),
 -            optparse.make_option("-/", dest="cloneExclude",
 -                                 action="append", type="string",
 -                                 help="exclude depot path"),
              optparse.make_option("--bare", dest="cloneBare",
                                   action="store_true", default=False),
          ]
          self.needsGit = False
          self.cloneBare = False
  
 -    # This is required for the "append" cloneExclude action
 -    def ensure_value(self, attr, value):
 -        if not hasattr(self, attr) or getattr(self, attr) is None:
 -            setattr(self, attr, value)
 -        return getattr(self, attr)
 -
      def defaultDestination(self, args):
          ## TODO: use common prefix of args?
          depotPath = args[0]