Commit 07258f5a authored by pkasting@chromium.org's avatar pkasting@chromium.org

Second take at handling users with locally overridden diff-cmds.

The first take at https://chromiumcodereview.appspot.com/14130006 tried to pass
"--diff-cmd diff", which doesn't work for e.g. Windows cmd users who don't have
a "diff" executable installed.

This take instead passes the "--internal-diff" switch which tells svn to ignore
any diff-cmd set locally and use its internal diff engine at all times.

While implementing this I found that the existing code tried to work around this
problem in a different way, by setting up a bogus config dir.  Since that
doesn't seem to work for me, and shouldn't be necessary with this patch anyway,
removed that code.

Review URL: https://chromiumcodereview.appspot.com/14050007

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@194990 0039d316-1c4b-4281-b951-d872f2087c98
parent b3b5201b
...@@ -10,7 +10,6 @@ import logging ...@@ -10,7 +10,6 @@ import logging
import os import os
import re import re
import sys import sys
import tempfile
import time import time
from xml.etree import ElementTree from xml.etree import ElementTree
...@@ -780,30 +779,19 @@ class SVN(object): ...@@ -780,30 +779,19 @@ class SVN(object):
expected relative path. expected relative path.
full_move means that move or copy operations should completely recreate the 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.""" files, usually in the prospect to apply the patch for a try job."""
# 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 # Use "svn info" output instead of os.path.isdir because the latter fails
# when the file is deleted. # when the file is deleted.
return SVN._DiffItemInternal( return SVN._DiffItemInternal(
filename, filename,
cwd, cwd,
SVN.CaptureLocalInfo([filename], cwd), SVN.CaptureLocalInfo([filename], cwd),
bogus_dir,
full_move, full_move,
revision) revision)
finally:
gclient_utils.RemoveDirectory(bogus_dir)
@staticmethod @staticmethod
def _DiffItemInternal(filename, cwd, info, bogus_dir, full_move, revision): def _DiffItemInternal(filename, cwd, info, full_move, revision):
"""Grabs the diff data.""" """Grabs the diff data."""
command = ["diff", "--config-dir", bogus_dir, filename] command = ["diff", "--internal-diff", filename]
if revision: if revision:
command.extend(['--revision', revision]) command.extend(['--revision', revision])
data = None data = None
...@@ -871,14 +859,6 @@ class SVN(object): ...@@ -871,14 +859,6 @@ class SVN(object):
if os.path.normcase(path).startswith(root): if os.path.normcase(path).startswith(root):
return path[len(root):] return path[len(root):]
return path 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 # Cleanup filenames
filenames = [RelativePath(f, root) for f in filenames] filenames = [RelativePath(f, root) for f in filenames]
# Get information about the modified items (files and directories) # Get information about the modified items (files and directories)
...@@ -915,13 +895,13 @@ class SVN(object): ...@@ -915,13 +895,13 @@ class SVN(object):
# revision the file was deleted. # revision the file was deleted.
srcinfo = {'Revision': rev} srcinfo = {'Revision': rev}
if (srcinfo.get('Revision') != rev and if (srcinfo.get('Revision') != rev and
SVN.Capture(['diff', '-r', '%d:head' % rev, srcurl], cwd)): SVN.Capture(['diff', '--internal-diff', '-r', '%d:head' % rev,
srcurl], cwd)):
metaheaders.append("#$ svn cp -r %d %s %s " metaheaders.append("#$ svn cp -r %d %s %s "
"### WARNING: note non-trunk copy\n" % "### WARNING: note non-trunk copy\n" %
(rev, src, filename)) (rev, src, filename))
else: else:
metaheaders.append("#$ cp %s %s\n" % (src, metaheaders.append("#$ cp %s %s\n" % (src, filename))
filename))
if metaheaders: if metaheaders:
diffs.append("### BEGIN SVN COPY METADATA\n") diffs.append("### BEGIN SVN COPY METADATA\n")
...@@ -929,8 +909,8 @@ class SVN(object): ...@@ -929,8 +909,8 @@ class SVN(object):
diffs.append("### END SVN COPY METADATA\n") diffs.append("### END SVN COPY METADATA\n")
# Now ready to do the actual diff. # Now ready to do the actual diff.
for filename in sorted(data.iterkeys()): for filename in sorted(data.iterkeys()):
diffs.append(SVN._DiffItemInternal( diffs.append(SVN._DiffItemInternal(filename, cwd, data[filename],
filename, cwd, data[filename], bogus_dir, full_move, revision)) full_move, revision))
# Use StringIO since it can be messy when diffing a directory move with # Use StringIO since it can be messy when diffing a directory move with
# full_move=True. # full_move=True.
buf = cStringIO.StringIO() buf = cStringIO.StringIO()
...@@ -939,8 +919,6 @@ class SVN(object): ...@@ -939,8 +919,6 @@ class SVN(object):
result = buf.getvalue() result = buf.getvalue()
buf.close() buf.close()
return result return result
finally:
gclient_utils.RemoveDirectory(bogus_dir)
@staticmethod @staticmethod
def GetEmail(cwd): def GetEmail(cwd):
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment