Merge branch 'pw/maint-p4-rcs-expansion-newline' into maint
authorJunio C Hamano <gitster@pobox.com>
Wed, 28 Nov 2012 20:04:32 +0000 (12:04 -0800)
committerJunio C Hamano <gitster@pobox.com>
Wed, 28 Nov 2012 20:04:50 +0000 (12:04 -0800)
"git p4" used to try expanding malformed "$keyword$" that spans
across multiple lines.

* pw/maint-p4-rcs-expansion-newline:
git p4: RCS expansion should not span newlines

1  2 
git-p4.py
t/t9810-git-p4-rcs.sh
diff --combined git-p4.py
index 882b1bbab53c6e184c7eadb5250c128b59c93482,8dbcdc3b8b38c97ae64d17e73dda4c1de2df1660..7d6c928c3f2b3d90a754f855c074ea57d2ca1bbf
+++ b/git-p4.py
@@@ -227,7 -227,7 +227,7 @@@ def p4_keywords_regexp_for_type(base, t
          pattern = r"""
              \$              # Starts with a dollar, followed by...
              (%s)            # one of the keywords, followed by...
-             (:[^$]+)?       # possibly an old expansion, followed by...
+             (:[^$\n]+)?     # possibly an old expansion, followed by...
              \$              # another dollar
              """ % kwords
          return pattern
@@@ -844,9 -844,6 +844,9 @@@ class P4RollBack(Command)
          return True
  
  class P4Submit(Command, P4UserMap):
 +
 +    conflict_behavior_choices = ("ask", "skip", "quit")
 +
      def __init__(self):
          Command.__init__(self)
          P4UserMap.__init__(self)
                  # preserve the user, requires relevant p4 permissions
                  optparse.make_option("--preserve-user", dest="preserveUser", action="store_true"),
                  optparse.make_option("--export-labels", dest="exportLabels", action="store_true"),
 +                optparse.make_option("--dry-run", "-n", dest="dry_run", action="store_true"),
 +                optparse.make_option("--prepare-p4-only", dest="prepare_p4_only", action="store_true"),
 +                optparse.make_option("--conflict", dest="conflict_behavior",
 +                                     choices=self.conflict_behavior_choices)
          ]
          self.description = "Submit changes from git to the perforce depot."
          self.usage += " [name of git branch to submit into perforce depot]"
          self.origin = ""
          self.detectRenames = False
          self.preserveUser = gitConfig("git-p4.preserveUser").lower() == "true"
 +        self.dry_run = False
 +        self.prepare_p4_only = False
 +        self.conflict_behavior = None
          self.isWindows = (platform.system() == "Windows")
          self.exportLabels = False
          self.p4HasMoveCommand = p4_has_command("move")
                  return False
  
      def applyCommit(self, id):
 -        print "Applying %s" % (read_pipe("git log --max-count=1 --pretty=oneline %s" % id))
 +        """Apply one commit, return True if it succeeded."""
 +
 +        print "Applying", read_pipe(["git", "show", "-s",
 +                                     "--format=format:%h %s", id])
  
          (p4User, gitEmail) = self.p4UserForCommit(id)
  
                      patch_succeeded = True
  
          if not patch_succeeded:
 -            print "What do you want to do?"
 -            response = "x"
 -            while response != "s" and response != "a" and response != "w":
 -                response = raw_input("[s]kip this patch / [a]pply the patch forcibly "
 -                                     "and with .rej files / [w]rite the patch to a file (patch.txt) ")
 -            if response == "s":
 -                print "Skipping! Good luck with the next patches..."
 -                for f in editedFiles:
 -                    p4_revert(f)
 -                for f in filesToAdd:
 -                    os.remove(f)
 -                return
 -            elif response == "a":
 -                os.system(applyPatchCmd)
 -                if len(filesToAdd) > 0:
 -                    print "You may also want to call p4 add on the following files:"
 -                    print " ".join(filesToAdd)
 -                if len(filesToDelete):
 -                    print "The following files should be scheduled for deletion with p4 delete:"
 -                    print " ".join(filesToDelete)
 -                die("Please resolve and submit the conflict manually and "
 -                    + "continue afterwards with git p4 submit --continue")
 -            elif response == "w":
 -                system(diffcmd + " > patch.txt")
 -                print "Patch saved to patch.txt in %s !" % self.clientPath
 -                die("Please resolve and submit the conflict manually and "
 -                    "continue afterwards with git p4 submit --continue")
 +            for f in editedFiles:
 +                p4_revert(f)
 +            return False
  
 +        #
 +        # Apply the patch for real, and do add/delete/+x handling.
 +        #
          system(applyPatchCmd)
  
          for f in filesToAdd:
              mode = filesToChangeExecBit[f]
              setP4ExecBit(f, mode)
  
 +        #
 +        # Build p4 change description, starting with the contents
 +        # of the git commit message.
 +        #
          logMessage = extractLogMessageFromGitCommit(id)
          logMessage = logMessage.strip()
          (logMessage, jobs) = self.separate_jobs_from_description(logMessage)
          submitTemplate = self.prepareLogMessage(template, logMessage, jobs)
  
          if self.preserveUser:
 -           submitTemplate = submitTemplate + ("\n######## Actual user %s, modified after commit\n" % p4User)
 +           submitTemplate += "\n######## Actual user %s, modified after commit\n" % p4User
 +
 +        if self.checkAuthorship and not self.p4UserIsMe(p4User):
 +            submitTemplate += "######## git author %s does not match your p4 account.\n" % gitEmail
 +            submitTemplate += "######## Use option --preserve-user to modify authorship.\n"
 +            submitTemplate += "######## Variable git-p4.skipUserNameCheck hides this message.\n"
  
 +        separatorLine = "######## everything below this line is just the diff #######\n"
 +
 +        # diff
          if os.environ.has_key("P4DIFF"):
              del(os.environ["P4DIFF"])
          diff = ""
              diff += p4_read_pipe(['diff', '-du',
                                    wildcard_encode(editedFile)])
  
 +        # new file diff
          newdiff = ""
          for newFile in filesToAdd:
              newdiff += "==== new file ====\n"
                  newdiff += "+" + line
              f.close()
  
 -        if self.checkAuthorship and not self.p4UserIsMe(p4User):
 -            submitTemplate += "######## git author %s does not match your p4 account.\n" % gitEmail
 -            submitTemplate += "######## Use option --preserve-user to modify authorship.\n"
 -            submitTemplate += "######## Variable git-p4.skipUserNameCheck hides this message.\n"
 -
 -        separatorLine = "######## everything below this line is just the diff #######\n"
 -
 +        # change description file: submitTemplate, separatorLine, diff, newdiff
          (handle, fileName) = tempfile.mkstemp()
          tmpFile = os.fdopen(handle, "w+")
          if self.isWindows:
          tmpFile.write(submitTemplate + separatorLine + diff + newdiff)
          tmpFile.close()
  
 +        if self.prepare_p4_only:
 +            #
 +            # Leave the p4 tree prepared, and the submit template around
 +            # and let the user decide what to do next
 +            #
 +            print
 +            print "P4 workspace prepared for submission."
 +            print "To submit or revert, go to client workspace"
 +            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" \
 +                  " \"git p4\"." % fileName
 +            print "You can delete the file \"%s\" when finished." % fileName
 +
 +            if self.preserveUser and p4User and not self.p4UserIsMe(p4User):
 +                print "To preserve change ownership by user %s, you must\n" \
 +                      "do \"p4 change -f <change>\" after submitting and\n" \
 +                      "edit the User field."
 +            if pureRenameCopy:
 +                print "After submitting, renamed files must be re-synced."
 +                print "Invoke \"p4 sync -f\" on each of these files:"
 +                for f in pureRenameCopy:
 +                    print "  " + f
 +
 +            print
 +            print "To revert the changes, use \"p4 revert ...\", and delete"
 +            print "the submit template file \"%s\"" % fileName
 +            if filesToAdd:
 +                print "Since the commit adds new files, they must be deleted:"
 +                for f in filesToAdd:
 +                    print "  " + f
 +            print
 +            return True
 +
 +        #
 +        # 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()
  
          else:
              # 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)
  
          os.remove(fileName)
 +        return ret
  
      # Export git tags as p4 labels. Create a p4 label and then tag
      # with that.
              for mapping in clientSpec.mappings:
                  labelTemplate += "\t%s\n" % mapping.depot_side.path
  
 -            p4_write_pipe(["label", "-i"], labelTemplate)
 +            if self.dry_run:
 +                print "Would create p4 label %s for tag" % name
 +            elif self.prepare_p4_only:
 +                print "Not creating p4 label %s for tag due to option" \
 +                      " --prepare-p4-only" % name
 +            else:
 +                p4_write_pipe(["label", "-i"], labelTemplate)
  
 -            # Use the label
 -            p4_system(["tag", "-l", name] +
 -                      ["%s@%s" % (mapping.depot_side.path, changelist) for mapping in clientSpec.mappings])
 +                # Use the label
 +                p4_system(["tag", "-l", name] +
 +                          ["%s@%s" % (mapping.depot_side.path, changelist) for mapping in clientSpec.mappings])
  
 -            if verbose:
 -                print "created p4 label for tag %s" % name
 +                if verbose:
 +                    print "created p4 label for tag %s" % name
  
      def run(self, args):
          if len(args) == 0:
              if not self.canChangeChangelists():
                  die("Cannot preserve user names without p4 super-user or admin permissions")
  
 +        # if not set from the command line, try the config file
 +        if self.conflict_behavior is None:
 +            val = gitConfig("git-p4.conflict")
 +            if val:
 +                if val not in self.conflict_behavior_choices:
 +                    die("Invalid value '%s' for config git-p4.conflict" % val)
 +            else:
 +                val = "ask"
 +            self.conflict_behavior = val
 +
          if self.verbose:
              print "Origin branch is " + self.origin
  
              os.makedirs(self.clientPath)
  
          chdir(self.clientPath)
 -        print "Synchronizing p4 checkout..."
 -        if new_client_dir:
 -            # old one was destroyed, and maybe nobody told p4
 -            p4_sync("...", "-f")
 +        if self.dry_run:
 +            print "Would synchronize p4 checkout in %s" % self.clientPath
          else:
 -            p4_sync("...")
 +            print "Synchronizing p4 checkout..."
 +            if new_client_dir:
 +                # old one was destroyed, and maybe nobody told p4
 +                p4_sync("...", "-f")
 +            else:
 +                p4_sync("...")
          self.check()
  
          commits = []
          if gitConfig("git-p4.detectCopiesHarder", "--bool") == "true":
              self.diffOpts += " --find-copies-harder"
  
 -        while len(commits) > 0:
 -            commit = commits[0]
 -            commits = commits[1:]
 -            self.applyCommit(commit)
 +        #
 +        # Apply the commits, one at a time.  On failure, ask if should
 +        # continue to try the rest of the patches, or quit.
 +        #
 +        if self.dry_run:
 +            print "Would apply"
 +        applied = []
 +        last = len(commits) - 1
 +        for i, commit in enumerate(commits):
 +            if self.dry_run:
 +                print " ", read_pipe(["git", "show", "-s",
 +                                      "--format=format:%h %s", commit])
 +                ok = True
 +            else:
 +                ok = self.applyCommit(commit)
 +            if ok:
 +                applied.append(commit)
 +            else:
 +                if self.prepare_p4_only and i < last:
 +                    print "Processing only the first commit due to option" \
 +                          " --prepare-p4-only"
 +                    break
 +                if i < last:
 +                    quit = False
 +                    while True:
 +                        # prompt for what to do, or use the option/variable
 +                        if self.conflict_behavior == "ask":
 +                            print "What do you want to do?"
 +                            response = raw_input("[s]kip this commit but apply"
 +                                                 " the rest, or [q]uit? ")
 +                            if not response:
 +                                continue
 +                        elif self.conflict_behavior == "skip":
 +                            response = "s"
 +                        elif self.conflict_behavior == "quit":
 +                            response = "q"
 +                        else:
 +                            die("Unknown conflict_behavior '%s'" %
 +                                self.conflict_behavior)
 +
 +                        if response[0] == "s":
 +                            print "Skipping this commit, but applying the rest"
 +                            break
 +                        if response[0] == "q":
 +                            print "Quitting"
 +                            quit = True
 +                            break
 +                    if quit:
 +                        break
 +
 +        chdir(self.oldWorkingDirectory)
  
 -        if len(commits) == 0:
 -            print "All changes applied!"
 -            chdir(self.oldWorkingDirectory)
 +        if self.dry_run:
 +            pass
 +        elif self.prepare_p4_only:
 +            pass
 +        elif len(commits) == len(applied):
 +            print "All commits applied!"
  
              sync = P4Sync()
              sync.run([])
              rebase = P4Rebase()
              rebase.rebase()
  
 +        else:
 +            if len(applied) == 0:
 +                print "No commits applied."
 +            else:
 +                print "Applied only the commits marked with '*':"
 +                for c in commits:
 +                    if c in applied:
 +                        star = "*"
 +                    else:
 +                        star = " "
 +                    print star, read_pipe(["git", "show", "-s",
 +                                           "--format=format:%h %s",  c])
 +                print "You will have to do 'git p4 sync' and rebase."
 +
          if gitConfig("git-p4.exportLabels", "--bool") == "true":
              self.exportLabels = True
  
              missingGitTags = gitTags - p4Labels
              self.exportGitTags(missingGitTags)
  
 +        # exit with error unless everything applied perfecly
 +        if len(commits) != len(applied):
 +                sys.exit(1)
 +
          return True
  
  class View(object):
@@@ -1947,41 -1818,19 +1947,41 @@@ class P4Sync(Command, P4UserMap)
          return files
  
      def stripRepoPath(self, path, prefixes):
 -        if self.useClientSpec:
 -            return self.clientSpecDirs.map_in_client(path)
 +        """When streaming files, this is called to map a p4 depot path
 +           to where it should go in git.  The prefixes are either
 +           self.depotPaths, or self.branchPrefixes in the case of
 +           branch detection."""
  
 -        if self.keepRepoPath:
 -            prefixes = [re.sub("^(//[^/]+/).*", r'\1', prefixes[0])]
 +        if self.useClientSpec:
 +            # branch detection moves files up a level (the branch name)
 +            # from what client spec interpretation gives
 +            path = self.clientSpecDirs.map_in_client(path)
 +            if self.detectBranches:
 +                for b in self.knownBranches:
 +                    if path.startswith(b + "/"):
 +                        path = path[len(b)+1:]
 +
 +        elif self.keepRepoPath:
 +            # Preserve everything in relative path name except leading
 +            # //depot/; just look at first prefix as they all should
 +            # be in the same depot.
 +            depot = re.sub("^(//[^/]+/).*", r'\1', prefixes[0])
 +            if p4PathStartsWith(path, depot):
 +                path = path[len(depot):]
  
 -        for p in prefixes:
 -            if p4PathStartsWith(path, p):
 -                path = path[len(p):]
 +        else:
 +            for p in prefixes:
 +                if p4PathStartsWith(path, p):
 +                    path = path[len(p):]
 +                    break
  
 +        path = wildcard_decode(path)
          return path
  
      def splitFilesIntoBranches(self, commit):
 +        """Look at each depotFile in the commit to figure out to what
 +           branch it belongs."""
 +
          branches = {}
          fnum = 0
          while commit.has_key("depotFile%s" % fnum):
              file["type"] = commit["type%s" % fnum]
              fnum = fnum + 1
  
 -            relPath = self.stripRepoPath(path, self.depotPaths)
 -            relPath = wildcard_decode(relPath)
 +            # start with the full relative path where this file would
 +            # go in a p4 client
 +            if self.useClientSpec:
 +                relPath = self.clientSpecDirs.map_in_client(path)
 +            else:
 +                relPath = self.stripRepoPath(path, self.depotPaths)
  
              for branch in self.knownBranches.keys():
 -
 -                # add a trailing slash so that a commit into qt/4.2foo doesn't end up in qt/4.2
 +                # add a trailing slash so that a commit into qt/4.2foo
 +                # doesn't end up in qt/4.2, e.g.
                  if relPath.startswith(branch + "/"):
                      if branch not in branches:
                          branches[branch] = []
  
      def streamOneP4File(self, file, contents):
          relPath = self.stripRepoPath(file['depotFile'], self.branchPrefixes)
 -        relPath = wildcard_decode(relPath)
          if verbose:
              sys.stderr.write("%s\n" % relPath)
  
  
      def streamOneP4Deletion(self, file):
          relPath = self.stripRepoPath(file['path'], self.branchPrefixes)
 -        relPath = wildcard_decode(relPath)
          if verbose:
              sys.stderr.write("delete %s\n" % relPath)
          self.gitStream.write("D %s\n" % relPath)
          gitStream.write(description)
          gitStream.write("\n")
  
 -    def commit(self, details, files, branch, branchPrefixes, parent = ""):
 +    def commit(self, details, files, branch, parent = ""):
          epoch = details["time"]
          author = details["user"]
 -        self.branchPrefixes = branchPrefixes
  
          if self.verbose:
              print "commit into %s" % branch
          # create a commit.
          new_files = []
          for f in files:
 -            if [p for p in branchPrefixes if p4PathStartsWith(f['path'], p)]:
 +            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'])
  
          self.gitStream.write("data <<EOT\n")
          self.gitStream.write(details["desc"])
 -        self.gitStream.write("\n[git-p4: depot-paths = \"%s\": change = %s"
 -                             % (','.join (branchPrefixes), details["change"]))
 +        self.gitStream.write("\n[git-p4: depot-paths = \"%s\": change = %s" %
 +                             (','.join(self.branchPrefixes), details["change"]))
          if len(details['options']) > 0:
              self.gitStream.write(": options = %s" % details['options'])
          self.gitStream.write("]\nEOT\n\n")
                  print "Change %s is labelled %s" % (change, labelDetails)
  
              files = p4CmdList(["files"] + ["%s...@%s" % (p, change)
 -                                                    for p in branchPrefixes])
 +                                                for p in self.branchPrefixes])
  
              if len(files) == len(labelRevisions):
  
                      for branch in branches.keys():
                          ## HACK  --hwn
                          branchPrefix = self.depotPaths[0] + branch + "/"
 +                        self.branchPrefixes = [ branchPrefix ]
  
                          parent = ""
  
                              tempBranch = os.path.join(self.tempBranchLocation, "%d" % (change))
                              if self.verbose:
                                  print "Creating temporary branch: " + tempBranch
 -                            self.commit(description, filesForCommit, tempBranch, [branchPrefix])
 +                            self.commit(description, filesForCommit, tempBranch)
                              self.tempBranches.append(tempBranch)
                              self.checkpoint()
                              blob = self.searchParent(parent, branch, tempBranch)
                          if blob:
 -                            self.commit(description, filesForCommit, branch, [branchPrefix], blob)
 +                            self.commit(description, filesForCommit, branch, blob)
                          else:
                              if self.verbose:
                                  print "Parent of %s not found. Committing into head of %s" % (branch, parent)
 -                            self.commit(description, filesForCommit, branch, [branchPrefix], parent)
 +                            self.commit(description, filesForCommit, branch, parent)
                  else:
                      files = self.extractFilesFromCommit(description)
 -                    self.commit(description, files, self.branch, self.depotPaths,
 +                    self.commit(description, files, self.branch,
                                  self.initialParent)
                      self.initialParent = ""
              except IOError:
  
          self.updateOptionDict(details)
          try:
 -            self.commit(details, self.extractFilesFromCommit(details), self.branch, self.depotPaths)
 +            self.commit(details, self.extractFilesFromCommit(details), self.branch)
          except IOError:
              print "IO error with git fast-import. Is your git version recent enough?"
              print self.gitError.read()
  
          self.depotPaths = newPaths
  
 +        # --detect-branches may change this for each branch
 +        self.branchPrefixes = self.depotPaths
 +
          self.loadUserMapFromCache()
          self.labels = {}
          if self.detectLabels:
@@@ -3144,7 -2988,7 +3144,7 @@@ def main()
  
      args = sys.argv[2:]
  
 -    options.append(optparse.make_option("--verbose", dest="verbose", action="store_true"))
 +    options.append(optparse.make_option("--verbose", "-v", dest="verbose", action="store_true"))
      if cmd.needsGit:
          options.append(optparse.make_option("--git-dir", dest="gitdir"))
  
diff --combined t/t9810-git-p4-rcs.sh
index fe30ad881f67c9f1fdcb2ee40922880126ec13cc,12b3d814499da0ebf54875157e013ca96da28f38..0c2fc3ea1a28268e216a3a93ed12b09f0cf55ef2
@@@ -155,11 -155,33 +155,30 @@@ test_expect_success 'cleanup after fail
        )
  '
  
+ # perl $File:: bug check
+ test_expect_success 'ktext expansion should not expand multi-line $File::' '
+       (
+               cd "$cli" &&
+               cat >lv.pm <<-\EOF
+               my $wanted = sub { my $f = $File::Find::name;
+                                   if ( -f && $f =~ /foo/ ) {
+               EOF
+               p4 add -t ktext lv.pm &&
+               p4 submit -d "lv.pm"
+       ) &&
+       test_when_finished cleanup_git &&
+       git p4 clone --dest="$git" //depot &&
+       (
+               cd "$git" &&
+               test_cmp "$cli/lv.pm" lv.pm
+       )
+ '
  #
  # Do not scrub anything but +k or +ko files.  Sneak a change into
  # the cli file so that submit will get a conflict.  Make sure that
  # scrubbing doesn't make a mess of things.
  #
 -# Assumes that git-p4 exits leaving the p4 file open, with the
 -# conflict-generating patch unapplied.
 -#
  # This might happen only if the git repo is behind the p4 repo at
  # submit time, and there is a conflict.
  #
@@@ -178,11 -200,14 +197,11 @@@ test_expect_success 'do not scrub plai
                        sed -i "s/^line5/line5 p4 edit/" file_text &&
                        p4 submit -d "file5 p4 edit"
                ) &&
 -              ! git p4 submit &&
 +              echo s | test_expect_code 1 git p4 submit &&
                (
 -                      # exepct something like:
 -                      #    file_text - file(s) not opened on this client
 -                      # but not copious diff output
 +                      # make sure the file is not left open
                        cd "$cli" &&
 -                      p4 diff file_text >wc &&
 -                      test_line_count = 1 wc
 +                      ! p4 fstat -T action file_text
                )
        )
  '
@@@ -337,6 -362,44 +356,6 @@@ test_expect_failure 'Add keywords in gi
        )
  '
  
 -# Check that the existing merge conflict handling still works.
 -# Modify kwfile1.c in git, and delete in p4. We should be able
 -# to skip the git commit.
 -#
 -test_expect_success 'merge conflict handling still works' '
 -      test_when_finished cleanup_git &&
 -      (
 -              cd "$cli" &&
 -              echo "Hello:\$Id\$" >merge2.c &&
 -              echo "World" >>merge2.c &&
 -              p4 add -t ktext merge2.c &&
 -              p4 submit -d "add merge test file"
 -      ) &&
 -      git p4 clone --dest="$git" //depot &&
 -      (
 -              cd "$git" &&
 -              sed -e "/Hello/d" merge2.c >merge2.c.tmp &&
 -              mv merge2.c.tmp merge2.c &&
 -              git add merge2.c &&
 -              git commit -m "Modifying merge2.c"
 -      ) &&
 -      (
 -              cd "$cli" &&
 -              p4 delete merge2.c &&
 -              p4 submit -d "remove merge test file"
 -      ) &&
 -      (
 -              cd "$git" &&
 -              test -f merge2.c &&
 -              git config git-p4.skipSubmitEdit true &&
 -              git config git-p4.attemptRCSCleanup true &&
 -              !(echo "s" | git p4 submit) &&
 -              git rebase --skip &&
 -              ! test -f merge2.c
 -      )
 -'
 -
 -
  test_expect_success 'kill p4d' '
        kill_p4d
  '