Reland r195308, r195328, and r195363, which use --internal-diff on svn 1.7+.

scm.py

 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """SCM-specific utility classes."""
 
 import cStringIO
 import glob
 import logging
 import os
@@ skipping 763 matching lines @@
   @staticmethod
   def GenerateDiff(filenames, cwd, full_move, revision):
     """Returns a string containing the diff for the given file list.
 
     The files in the list should either be absolute paths or relative to the
     given root. If no root directory is provided, the repository root will be
     used.
     The diff will always use relative paths.
     """
     assert isinstance(filenames, (list, tuple))
+    # If the user specified a custom diff command in their svn config file,
+    # then it'll be used when we do svn diff, which we don't want to happen
+    # since we want the unified diff.
+    if SVN.AssertVersion("1.7")[0]:
+      # On svn >= 1.7, the "--internal-diff" flag will solve this.
+      return SVN._GenerateDiffInternal(filenames, cwd, full_move, revision,
+                                       ["diff", "--internal-diff"],
+                                       ["diff", "--internal-diff"])
+    else:
+      # On svn < 1.7, the "--internal-diff" flag doesn't exist.  Using
+      # --diff-cmd=diff doesn't always work, since e.g. Windows cmd users may
+      # not have a "diff" executable in their path at all.  So we use an empty
+      # temporary directory as the config directory, which bypasses any user
+      # settings for the diff-cmd.  However, we don't pass this for the
+      # remote_safe_diff_command parameter, since when a new config-dir is
+      # specified for an svn diff against a remote URL, it triggers
+      # authentication prompts.  In this case there isn't really a good
+      # alternative to svn 1.7's --internal-diff flag.
+      bogus_dir = tempfile.mkdtemp()
+      try:
+        return SVN._GenerateDiffInternal(filenames, cwd, full_move, revision,
+                                         ["diff", "--config-dir", bogus_dir],
+                                         ["diff"])
+      finally:
+        gclient_utils.rmtree(bogus_dir)
+
+  @staticmethod
+  def _GenerateDiffInternal(filenames, cwd, full_move, revision, diff_command,
+                            remote_safe_diff_command):
     root = os.path.normcase(os.path.join(cwd, ''))
     def RelativePath(path, root):
       """We must use relative paths."""
       if os.path.normcase(path).startswith(root):
         return path[len(root):]
       return path
-    # If the user specified a custom diff command in their svn config file,
-    # then it'll be used when we do svn diff, which we don't want to happen
-    # since we want the unified diff.  Using --diff-cmd=diff doesn't always
-    # work, since they can have another diff executable in their path that
-    # gives different line endings.  So we use a bogus temp directory as the
-    # config directory, which gets around these problems.
-    bogus_dir = tempfile.mkdtemp()
-    try:
-      # Cleanup filenames
-      filenames = [RelativePath(f, root) for f in filenames]
-      # Get information about the modified items (files and directories)
-      data = dict([(f, SVN.CaptureLocalInfo([f], root)) for f in filenames])
-      diffs = []
-      if full_move:
-        # Eliminate modified files inside moved/copied directory.
-        for (filename, info) in data.iteritems():
-          if SVN.IsMovedInfo(info) and info.get("Node Kind") == "directory":
-            # Remove files inside the directory.
-            filenames = [f for f in filenames
-                         if not f.startswith(filename + os.path.sep)]
-        for filename in data.keys():
-          if not filename in filenames:
-            # Remove filtered out items.
-            del data[filename]
-      else:
-        metaheaders = []
-        for (filename, info) in data.iteritems():
-          if SVN.IsMovedInfo(info):
-            # for now, the most common case is a head copy,
-            # so let's just encode that as a straight up cp.
-            srcurl = info.get('Copied From URL')
-            file_root = info.get('Repository Root')
-            rev = int(info.get('Copied From Rev'))
-            assert srcurl.startswith(file_root)
-            src = srcurl[len(file_root)+1:]
-            try:
-              srcinfo = SVN.CaptureRemoteInfo(srcurl)
-            except subprocess2.CalledProcessError, e:
-              if not 'Not a valid URL' in e.stderr:
-                raise
-              # Assume the file was deleted. No idea how to figure out at which
-              # revision the file was deleted.
-              srcinfo = {'Revision': rev}
-            if (srcinfo.get('Revision') != rev and
-                SVN.Capture(['diff', '-r', '%d:head' % rev, srcurl], cwd)):
-              metaheaders.append("#$ svn cp -r %d %s %s "
-                                 "### WARNING: note non-trunk copy\n" %
-                                 (rev, src, filename))
-            else:
-              metaheaders.append("#$ cp %s %s\n" % (src,
-                                                    filename))
-
-        if metaheaders:
-          diffs.append("### BEGIN SVN COPY METADATA\n")
-          diffs.extend(metaheaders)
-          diffs.append("### END SVN COPY METADATA\n")
-      # Now ready to do the actual diff.
-      for filename in sorted(data.iterkeys()):
-        diffs.append(SVN._DiffItemInternal(
-            filename, cwd, data[filename], bogus_dir, full_move, revision))
-      # Use StringIO since it can be messy when diffing a directory move with
-      # full_move=True.
-      buf = cStringIO.StringIO()
-      for d in filter(None, diffs):
-        buf.write(d)
-      result = buf.getvalue()
-      buf.close()
-      return result
-    finally:
-      gclient_utils.rmtree(bogus_dir)
+    # Cleanup filenames
+    filenames = [RelativePath(f, root) for f in filenames]
+    # Get information about the modified items (files and directories)
+    data = dict((f, SVN.CaptureLocalInfo([f], root)) for f in filenames)
+    diffs = []
+    if full_move:
+      # Eliminate modified files inside moved/copied directory.
+      for (filename, info) in data.iteritems():
+        if SVN.IsMovedInfo(info) and info.get("Node Kind") == "directory":
+          # Remove files inside the directory.
+          filenames = [f for f in filenames
+                       if not f.startswith(filename + os.path.sep)]
+      for filename in data.keys():
+        if not filename in filenames:
+          # Remove filtered out items.
+          del data[filename]
+    else:
+      metaheaders = []
+      for (filename, info) in data.iteritems():
+        if SVN.IsMovedInfo(info):
+          # for now, the most common case is a head copy,
+          # so let's just encode that as a straight up cp.
+          srcurl = info.get('Copied From URL')
+          file_root = info.get('Repository Root')
+          rev = int(info.get('Copied From Rev'))
+          assert srcurl.startswith(file_root)
+          src = srcurl[len(file_root)+1:]
+          try:
+            srcinfo = SVN.CaptureRemoteInfo(srcurl)
+          except subprocess2.CalledProcessError, e:
+            if not 'Not a valid URL' in e.stderr:
+              raise
+            # Assume the file was deleted. No idea how to figure out at which
+            # revision the file was deleted.
+            srcinfo = {'Revision': rev}
+          if (srcinfo.get('Revision') != rev and
+              SVN.Capture(remote_safe_diff_command + ['-r', '%d:head' % rev,
+                                                      srcurl], cwd)):
+            metaheaders.append("#$ svn cp -r %d %s %s "
+                               "### WARNING: note non-trunk copy\n" %
+                               (rev, src, filename))
+          else:
+            metaheaders.append("#$ cp %s %s\n" % (src,
+                                                  filename))
+      if metaheaders:
+        diffs.append("### BEGIN SVN COPY METADATA\n")
+        diffs.extend(metaheaders)
+        diffs.append("### END SVN COPY METADATA\n")
+    # Now ready to do the actual diff.
+    for filename in sorted(data):
+      diffs.append(SVN._DiffItemInternal(
+          filename, cwd, data[filename], diff_command, full_move, revision))
+    # Use StringIO since it can be messy when diffing a directory move with
+    # full_move=True.
+    buf = cStringIO.StringIO()
+    for d in filter(None, diffs):
+      buf.write(d)
+    result = buf.getvalue()
+    buf.close()
+    return result
 
   @staticmethod
-  def _DiffItemInternal(filename, cwd, info, bogus_dir, full_move, revision):
+  def _DiffItemInternal(filename, cwd, info, diff_command, full_move, revision):
     """Grabs the diff data."""
-    command = ["diff", "--config-dir", bogus_dir, filename]
+    command = diff_command + [filename]
     if revision:
       command.extend(['--revision', revision])
     data = None
     if SVN.IsMovedInfo(info):
       if full_move:
         if info.get("Node Kind") == "directory":
           # Things become tricky here. It's a directory copy/move. We need to
           # diff all the files inside it.
           # This will put a lot of pressure on the heap. This is why StringIO
           # is used and converted back into a string at the end. The reason to
@@ skipping 192 matching lines @@
         # revert, like for properties.
         if not os.path.isdir(cwd):
           # '.' was deleted. It's not worth continuing.
           return
         try:
           SVN.Capture(['revert', file_status[1]], cwd=cwd)
         except subprocess2.CalledProcessError:
           if not os.path.exists(file_path):
             continue
           raise