Use --internal-diff only for 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 754 matching lines @@
       The value of the property, which will be the empty string if the property
       is not set on the file.  If the file is not under version control, the
       empty string is also returned.
     """
     try:
       return SVN.Capture(['propget', property_name, filename], cwd)
     except subprocess2.CalledProcessError:
       return ''
 
   @staticmethod
-  def DiffItem(filename, cwd, full_move, revision):
-    """Diffs a single file.
+  def GenerateDiff(filenames, cwd, full_move, revision):
+    """Returns a string containing the diff for the given file list.
 
-    Should be simple, eh? No it isn't.
-    Be sure to be in the appropriate directory before calling to have the
-    expected relative path.
-    full_move means that move or copy operations should completely recreate the
-    files, usually in the prospect to apply the patch for a try job."""
+    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.  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:
-      # Use "svn info" output instead of os.path.isdir because the latter fails
-      # when the file is deleted.
-      return SVN._DiffItemInternal(
-          filename,
-          cwd,
-          SVN.CaptureLocalInfo([filename], cwd),
-          bogus_dir,
-          full_move,
-          revision)
-    finally:
-      gclient_utils.RemoveDirectory(bogus_dir)
+    # since we want the unified diff.  On svn >= 1.7, the "--internal-diff" flag

[M-A Ruel, 2013/04/19 16:17:08]

Could you split the comments, move the svn >= 1.7 part inside the condition and
the svn < 1.7 into the else. I think it'd be more readable.

+    # will solve this.  On svn < 1.7, however, this flag doesn't exist.  Using
+    # --diff-cmd=diff doesn't always work, since some users (e.g. Windows cmd
+    # users) may have no diff executable in their paths at all, and even people
+    # who do can have a diff executable in their path that uses unexpected line
+    # endings.  So we use a bogus temp directory as the config directory, which
+    # bypasses any user settings for the diff-cmd.
+    if SVN.AssertVersion("1.7")[0]:
+      return SVN._GenerateDiffInternal(filenames, cwd, full_move, revision,
+                                       ["diff", "--internal-diff"])
+    else:
+      bogus_dir = tempfile.mkdtemp()
+      try:
+        return SVN._GenerateDiffInternal(filenames, cwd, full_move, revision,
+                                         ["diff", "--config_dir", bogus_dir])
+      finally:
+        gclient_utils.RemoveDirectory(bogus_dir)
 
   @staticmethod
-  def _DiffItemInternal(filename, cwd, info, bogus_dir, full_move, revision):
+  def _GenerateDiffInternal(filenames, cwd, full_move, revision, 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
+    # 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])

[M-A Ruel, 2013/04/19 16:17:08]

data = dict((f, SVN.CaptureLocalInfo([f], root)) for f in filenames)

[] are unnecessary.

+    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_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.iterkeys()):

[M-A Ruel, 2013/04/19 16:17:08]

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, diff_command, full_move, revision):
     """Grabs the diff data."""
-    command = ["diff", "--config-dir", bogus_dir, filename]
+    diff_command.append(filename)
     if revision:
-      command.extend(['--revision', revision])
+      diff_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
           # return a string instead of a StringIO is that StringIO.write()
           # doesn't accept a StringIO object. *sigh*.
@@ skipping 11 matching lines @@
               data.write(GenFakeDiff(os.path.join(dirpath, f)))
           if data:
             tmp = data.getvalue()
             data.close()
             data = tmp
         else:
           data = GenFakeDiff(filename)
       else:
         if info.get("Node Kind") != "directory":
           # svn diff on a mv/cp'd file outputs nothing if there was no change.
-          data = SVN.Capture(command, cwd)
+          data = SVN.Capture(diff_command, cwd)
           if not data:
             # We put in an empty Index entry so upload.py knows about them.
             data = "Index: %s\n" % filename.replace(os.sep, '/')
         # Otherwise silently ignore directories.
     else:
       if info.get("Node Kind") != "directory":
         # Normal simple case.
         try:
-          data = SVN.Capture(command, cwd)
+          data = SVN.Capture(diff_command, cwd)
         except subprocess2.CalledProcessError:
           if revision:
             data = GenFakeDiff(filename)
           else:
             raise
       # Otherwise silently ignore directories.
     return data
 
   @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))
-    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.RemoveDirectory(bogus_dir)
-
-  @staticmethod
   def GetEmail(cwd):
     """Retrieves the svn account which we assume is an email address."""
     try:
       infos = SVN.CaptureLocalInfo([], cwd)
     except subprocess2.CalledProcessError:
       return None
 
     # Should check for uuid but it is incorrectly saved for https creds.
     root = infos['Repository Root']
     realm = root.rsplit('/', 1)[0]
@@ skipping 140 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