using --diff-options to exclude a file

[John Arbash Meinel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Jamie Wilkinson wrote:
> Hey,
> 
> I'm trying to generate a diff between two branches, but I want to exclude
> .bzrignore from the diff because it's not relevant to upstream.
> 
> bzr diff --diff-options='-x .bzrignore' ../upstream ./
> seems like the right thing to do, but this dies with the following error:
> 
> dawn% bzr diff --diff-options='-x .bzrignore' ../upstream ./
> === added file '.bzrignore'
> diff: standard output: Bad file descriptor
> bzr: ERROR: external diff failed with 2; command: ['diff', '--label',
> u'.bzrignore\t1970-01-01 00:00:00 +0000', '/tmp/bzr-diff-old-8CUmJB',
> '--label', u'.bzrignore\t2006-07-25 02:16:36 +0000',
> '/tmp/bzr-diff-new-Wv8Nqa', '--binary', '-u', '-x', '.bzrignore']
> 
> Is this a bug or am I using --diff-options the wrong way?

There seems to be a bug, and I know a way to work around it, but I am
mystified as to *why* it is a bug.

Basically, if a file has a 'fileno()' member, we pass the file handle
directly to the subprocess.Popen(), which should mean that the spawned
diff writes directly to sys.stdout.

I looked into it, and we do indeed have a fileno(), and it evaluates to
1, which should be sys.stdout. But when we send that into Popen(), it
raises 'standard output: Bad file descriptor'.
Why would stdout be a bad file descriptor?

The workaround is to call Popen(stdout=subprocess.PIPE), and then read
the output, and write it back on the requested file.. There are ways to
prevent this from blocking.

If you want to see if this fixes your problem, do this:
=== modified file 'bzrlib/diff.py'
- --- bzrlib/diff.py      2006-07-12 12:36:57 +0000
+++ bzrlib/diff.py      2006-07-25 18:57:06 +0000
@@ -88,12 +88,8 @@
 def external_diff(old_filename, oldlines, new_filename, newlines, to_file,
                   diff_opts):
     """Display a diff by calling out to the external diff program."""
- -    if hasattr(to_file, 'fileno'):
- -        out_file = to_file
- -        have_fileno = True
- -    else:
- -        out_file = subprocess.PIPE
- -        have_fileno = False
+    out_file = subprocess.PIPE
+    have_fileno = False

     # make sure our own output is properly ordered before the diff
     to_file.flush()


We already are able to use PIPE in case we don't have a fileno (such as
passing in a StringIO).

Doing that fixed it for me, but I think we should investigate why
sys.stdout is failing before we really use that.

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.2 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFExmrHJdeBCYSNAAMRAoqJAJ988vJtlqn/U/fJnfZrpWpuAM+BMgCgy1/H
SDGZIzp+6q+72395SALyTpo=
=qpPU
-----END PGP SIGNATURE-----