=== modified file 'NEWS' --- NEWS 2006-06-13 16:26:09 +0000 +++ NEWS 2006-06-16 23:49:40 +0000 @@ -87,6 +87,10 @@ that do not use '.bzr' to store their data - i.e. '.svn', '.hg' etc. (Robert Collins, Jelmer Vernooij). + * bzrlib.diff.external_diff can be redirected to any filelike object that + has a file descriptor. Use subprocess instead of spawnvp. + (#4047, #48914, James Henstridge, John Arbash Meinel) + bzr 0.8.2 2006-05-17 BUG FIXES: === modified file 'bzrlib/diff.py' --- bzrlib/diff.py 2006-06-06 22:59:58 +0000 +++ bzrlib/diff.py 2006-06-16 23:44:47 +0000 @@ -1,19 +1,24 @@ # Copyright (C) 2004, 2005, 2006 Canonical Ltd. - +# # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by # the Free Software Foundation; either version 2 of the License, or # (at your option) any later version. - +# # This program is distributed in the hope that it will be useful, # but WITHOUT ANY WARRANTY; without even the implied warranty of # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the # GNU General Public License for more details. - +# # You should have received a copy of the GNU General Public License # along with this program; if not, write to the Free Software # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA +import errno +import os +import subprocess +import sys +import tempfile import time from bzrlib.delta import compare_trees @@ -23,7 +28,7 @@ import bzrlib.patiencediff from bzrlib.symbol_versioning import * from bzrlib.textfile import check_text_lines -from bzrlib.trace import mutter +from bzrlib.trace import mutter, warning # TODO: Rather than building a changeset object, we should probably @@ -81,20 +86,17 @@ def external_diff(old_filename, oldlines, new_filename, newlines, to_file, diff_opts): """Display a diff by calling out to the external diff program.""" - import sys + if not hasattr(to_file, 'fileno'): + raise NotImplementedError("sorry, can't send external diff other " + "than to a file descriptor", to_file) - if to_file != sys.stdout: - raise NotImplementedError("sorry, can't send external diff other than to stdout yet", - to_file) - # make sure our own output is properly ordered before the diff to_file.flush() - from tempfile import NamedTemporaryFile - import os - - oldtmpf = NamedTemporaryFile() - newtmpf = NamedTemporaryFile() + oldtmp_fd, old_abspath = tempfile.mkstemp(prefix='bzr-diff-old-') + newtmp_fd, new_abspath = tempfile.mkstemp(prefix='bzr-diff-new-') + oldtmpf = os.fdopen(oldtmp_fd, 'wb') + newtmpf = os.fdopen(newtmp_fd, 'wb') try: # TODO: perhaps a special case for comparing to or from the empty @@ -107,16 +109,16 @@ oldtmpf.writelines(oldlines) newtmpf.writelines(newlines) - oldtmpf.flush() - newtmpf.flush() + oldtmpf.close() + newtmpf.close() if not diff_opts: diff_opts = [] diffcmd = ['diff', '--label', old_filename, - oldtmpf.name, + old_abspath, '--label', new_filename, - newtmpf.name] + new_abspath] # diff only allows one style to be specified; they don't override. # note that some of these take optargs, and the optargs can be @@ -142,7 +144,11 @@ if diff_opts: diffcmd.extend(diff_opts) - rc = os.spawnvp(os.P_WAIT, 'diff', diffcmd) + pipe = subprocess.Popen(diffcmd, + stdin=subprocess.PIPE, + stdout=to_file) + pipe.stdin.close() + rc = pipe.wait() if rc != 0 and rc != 1: # returns 1 if files differ; that's OK @@ -155,6 +161,21 @@ finally: oldtmpf.close() # and delete newtmpf.close() + # Clean up. Warn in case the files couldn't be deleted + # (in case windows still holds the file open, but not + # if the files have already been deleted) + try: + os.remove(old_abspath) + except OSError, e: + if e.errno not in (errno.ENOENT,): + warning('Failed to delete temporary file: %s %s', + old_abspath, e) + try: + os.remove(new_abspath) + except OSError: + if e.errno not in (errno.ENOENT,): + warning('Failed to delete temporary file: %s %s', + new_abspath, e) @deprecated_function(zero_eight) @@ -174,7 +195,6 @@ supplies any two trees. """ if output is None: - import sys output = sys.stdout if from_spec is None: @@ -221,7 +241,6 @@ The more general form is show_diff_trees(), where the caller supplies any two trees. """ - import sys output = sys.stdout def spec_tree(spec): revision_id = spec.in_store(tree.branch).rev_id === modified file 'bzrlib/tests/test_diff.py' --- bzrlib/tests/test_diff.py 2006-06-06 22:59:58 +0000 +++ bzrlib/tests/test_diff.py 2006-06-16 23:44:47 +0000 @@ -16,12 +16,14 @@ import os from cStringIO import StringIO +import errno +from tempfile import TemporaryFile -from bzrlib.diff import internal_diff, show_diff_trees +from bzrlib.diff import internal_diff, external_diff, show_diff_trees from bzrlib.errors import BinaryFile import bzrlib.patiencediff -from bzrlib.tests import TestCase, TestCaseWithTransport, TestCaseInTempDir -from bzrlib.tests import TestCase, TestCaseInTempDir +from bzrlib.tests import (TestCase, TestCaseWithTransport, + TestCaseInTempDir, TestSkipped) def udiff_lines(old, new, allow_binary=False): @@ -31,6 +33,20 @@ return output.readlines() +def external_udiff_lines(old, new): + output = TemporaryFile() + try: + external_diff('old', old, 'new', new, output, diff_opts=['-u']) + except OSError, e: + # if the diff program could not be found, skip the test + if e.errno == errno.ENOENT: + raise TestSkipped + output.seek(0, 0) + lines = output.readlines() + output.close() + return lines + + class TestDiff(TestCase): def test_add_nl(self): @@ -78,6 +94,10 @@ udiff_lines([1023 * 'a' + '\x00'], [], allow_binary=True) udiff_lines([], [1023 * 'a' + '\x00'], allow_binary=True) + def test_external_diff(self): + lines = external_udiff_lines(['boo\n'], ['goo\n']) + self.check_patch(lines) + def test_internal_diff_default(self): # Default internal diff encoding is utf8 output = StringIO()