|
|
Created:
7 years, 8 months ago by Peter Kasting Modified:
7 years, 8 months ago Reviewers:
M-A Ruel CC:
chromium-reviews, Dirk Pranke, cmp-cc_chromium.org, M-A Ruel, iannucci+depot_tools_chromium.org, jam Visibility:
Public. |
DescriptionReland r195308, r195328, and r195363, which use --internal-diff on svn 1.7+.
This time, do not use the --config-dir hack on svn < 1.7 for the diff command
that runs against a remote URL. This seems to trigger auth prompts (see revert
comments on https://codereview.chromium.org/14297017/ ).
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=196263
Patch Set 1 #Patch Set 2 : #Patch Set 3 : #Messages
Total messages: 6 (0 generated)
maruel: review. Patch set 1 represents what was previously landed (and later reverted). jam: CCed in case you want to test this patch to see if it avoids the problems you were having.
lgtm
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/pkasting@chromium.org/14084009/2001
Failed to apply patch for depot_tools/scm.py: While running patch -p0 --forward --force --no-backup-if-mismatch; patching file depot_tools/scm.py Hunk #1 FAILED at 781. 1 out of 1 hunk FAILED -- saving rejects to file depot_tools/scm.py.rej Patch: depot_tools/scm.py Index: scm.py =================================================================== --- depot_tools/scm.py (revision 195901) +++ depot_tools/scm.py (working copy) @@ -781,87 +781,106 @@ 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.RemoveDirectory(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)) + # 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 - 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 _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
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/pkasting@chromium.org/14084009/10001
Message was sent while issue was closed.
Change committed as 196263 |