You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Blair Zajac <bl...@orcaware.com> on 2002/11/09 05:53:22 UTC

svnlook diff fails on binary files:

It looks like diff is being called on for binary files in svnlook.
This will mess up our commit emails when binary files are added or
modified in the repos.

rm -fr repos r
svnadmin create repos
svn co file://`pwd`/repos r
cd r
dd if=/dev/urandom of=file bs=1024 count=1
svn add file
svn ci -m ''
cd ..
svnlook repos diff

This generates:

Checked out revision 0.
1+0 records in
1+0 records out
A  (bin)  file
Adding  (bin)  file
Transmitting file data .
Committed revision 1.
Added: file
==============================================================================
Binary files file       (original) and file differ
svn: Error calling external program
svn: /usr/bin/diff returned 2

Best,
Blair

-- 
Blair Zajac <bl...@orcaware.com>
Web and OS performance plots - http://www.orcaware.com/orca/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svnlook diff fails on binary files:

Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Blair Zajac <bl...@orcaware.com> writes:
> Well, the file would be noted as a binary file and unless there
> was a specific diff program configured for it, /usr/bin/diff
> wouldn't be run on it.

We can do that right now, it needn't wait for fully configurable diff
commands.



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svnlook diff fails on binary files:

Posted by Blair Zajac <bl...@orcaware.com>.
Karl Fogel wrote:
> 
> Blair Zajac <bl...@orcaware.com> writes:
> > If we're going to do this, then maybe we should design in a more
> > general diff scheme, where you can specify different diff programs
> > for different mime-types.
> >
> > So you could have an xmldiff for XML files, etc.
> 
> That's always been on the roadmap, for Post-1.0.  It's not a huge
> amount of work, but also not clear to me how it would help this
> particular situation (?)...

Well, the file would be noted as a binary file and unless there
was a specific diff program configured for it, /usr/bin/diff
wouldn't be run on it.

Blair

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svnlook diff fails on binary files:

Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Blair Zajac <bl...@orcaware.com> writes:
> If we're going to do this, then maybe we should design in a more
> general diff scheme, where you can specify different diff programs
> for different mime-types.
> 
> So you could have an xmldiff for XML files, etc.

That's always been on the roadmap, for Post-1.0.  It's not a huge
amount of work, but also not clear to me how it would help this
particular situation (?)...


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svnlook diff fails on binary files:

Posted by Blair Zajac <bl...@orcaware.com>.
Philip Martin wrote:
> Changing Subversion to avoid running diff on binary files will solve
> most problems, leaving only the case where Subversion classifies a
> file as text and diff classifies it as binary.  However such a case
> will probably cause problems when attempting a three-way merge during
> update, so the user is advised to make sure Subversion's file
> classification matches that of diff.

If we're going to do this, then maybe we should design in a more
general diff scheme, where you can specify different diff programs
for different mime-types.

So you could have an xmldiff for XML files, etc.

Blair

-- 
Blair Zajac <bl...@orcaware.com>
Web and OS performance plots - http://www.orcaware.com/orca/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svnlook diff fails on binary files:

Posted by Philip Martin <ph...@codematters.co.uk>.
Blair Zajac <bl...@orcaware.com> writes:

> > $ diff -v
> > diff - GNU diffutils version 2.7
> > $  diff /bin/rm /bin/mv
> > Binary files /bin/rm and /bin/mv differ
> > $ echo $?
> > 1
> > $  diff /bin/rm /bin/rm
> > $ echo $?
> > 0
> 
> Diff 2.8.1 does it differently.
> 
> % diff -v
> diff (GNU diffutils) 2.8.1
> % diff /bin/rm /bin/mv
> Binary files /bin/rm and /bin/mv differ
> % echo $?
> 2
> % diff /bin/rm /bin/rm
> % echo $?
> 0

This makes diff 2.8.1 very hard to use on text files :-(

The change is not documented in the NEWS/man/info files as far as I
can tell, it's hidden away in this cryptic changelog

   2002-02-28  Paul Eggert  <eg...@twinsun.com>
        (Binary): Differing binary files are trouble unless the user asked for
        brief output.

where "trouble" is a reference to the use of that word in the return
value documentation.  One is obviously expected to be able to recall
the GNU info pages word-for-word!

This means that for diff 2.8.1 the only way to tell whether

      'diff foo bar'

failed due to a write error is to examine the output text, text which
Subversion has directed straight to stdout.  (Well there is another
way but one has to know which algorithm diff uses to determine whether
a file is binary.)

Changing Subversion to avoid running diff on binary files will solve
most problems, leaving only the case where Subversion classifies a
file as text and diff classifies it as binary.  However such a case
will probably cause problems when attempting a three-way merge during
update, so the user is advised to make sure Subversion's file
classification matches that of diff.

-- 
Philip Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svnlook diff fails on binary files:

Posted by Blair Zajac <bl...@orcaware.com>.
Philip Martin wrote:
> 
> cmpilato@collab.net writes:
> 
> > Blair Zajac <bl...@orcaware.com> writes:
> >
> > > Just to be beat this into the ground, when diff returns 2, svnlook
> > > quits immediately and doesn't generate the entire diff for the
> > > revision, hence our commit email will be lacking info.
> >
> > I don't understand.  Every time I've ever run diff on a binary file,
> > it either prints nothing (no diff) or "Binary files foo and bar
> > differ".  Is this not true for you?
> >
> > Oh, I see.
> >
> >    % cd /usr/share/pixmaps
> >    % diff tycoon.png ximian_button_pill.png
> >    Binary files tycoon.png and ximian_button_pill.png differ
> >    % echo $?
> >    2
> 
> Good old diff!
> 
> $ diff -v
> diff - GNU diffutils version 2.7
> $  diff /bin/rm /bin/mv
> Binary files /bin/rm and /bin/mv differ
> $ echo $?
> 1
> $  diff /bin/rm /bin/rm
> $ echo $?
> 0

Diff 2.8.1 does it differently.

% diff -v
diff (GNU diffutils) 2.8.1
% diff /bin/rm /bin/mv
Binary files /bin/rm and /bin/mv differ
% echo $?
2
% diff /bin/rm /bin/rm
% echo $?
0

Blair

-- 
Blair Zajac <bl...@orcaware.com>
Web and OS performance plots - http://www.orcaware.com/orca/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svnlook diff fails on binary files:

Posted by Philip Martin <ph...@codematters.co.uk>.
cmpilato@collab.net writes:

> Blair Zajac <bl...@orcaware.com> writes:
> 
> > Just to be beat this into the ground, when diff returns 2, svnlook
> > quits immediately and doesn't generate the entire diff for the
> > revision, hence our commit email will be lacking info.
> 
> I don't understand.  Every time I've ever run diff on a binary file,
> it either prints nothing (no diff) or "Binary files foo and bar
> differ".  Is this not true for you?
> 
> Oh, I see.
> 
>    % cd /usr/share/pixmaps
>    % diff tycoon.png ximian_button_pill.png 
>    Binary files tycoon.png and ximian_button_pill.png differ
>    % echo $?
>    2

Good old diff!

$ diff -v
diff - GNU diffutils version 2.7
$  diff /bin/rm /bin/mv
Binary files /bin/rm and /bin/mv differ
$ echo $?
1
$  diff /bin/rm /bin/rm
$ echo $?
0

I changed the Subversion code so that a diff return value of 2 was an
error as that's what happens when diff has a write failure.  If there
is a diff write failure then the diff will be incomplete, so we don't
really want Subversion to say that everything worked.

-- 
Philip Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svnlook diff fails on binary files:

Posted by cm...@collab.net.
Blair Zajac <bl...@orcaware.com> writes:

> > So, the problem isn't so much that diff is run on binary files, it's
> > that we aren't discerning the meaning of the 2 error code in this
> > case.  And since error code 2 is a really generic error code for diff,
> > the best thing to do is just to not diff binary files.  Am I on track?
> 
> I'm more concerned that something has changed and we don't know it.
> All of the svn logo's got checked in a while ago and we didn't see
> svnlook fail on the diffs.

The only important thing that has changed in this area is that
svn_io_run_diff didn't use to return the error code 2 to its caller.

   ------------------------------------------------------------------------
   rev 3421:  philip | 2002-10-20 09:45:59 -0500 (Sun, 20 Oct 2002) | 5 lines

   Fix part of issue 779.

   * subversion/libsvn_subr/io.c (svn_io_run_diff): Treat a return value
     of 2 from the diff process as an error. 
   ------------------------------------------------------------------------

> I don't know why svnlook diff is running diff on binary files in the
> first place.

Because I never added code to check the properties of the node for
binariness.  If you'd like to do this, I would recommend:

   - adding a flag to the svn_repos_node_t: 'is_binary'
   - in the relevant places of libsvn_repos/node_tree.c, check for the
     svn:mime-type property on the nodes
   - in svnlook, have it check node->is_binary before running diff.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svnlook diff fails on binary files:

Posted by Blair Zajac <bl...@orcaware.com>.
cmpilato@collab.net wrote:
> 
> Blair Zajac <bl...@orcaware.com> writes:
> 
> > Just to be beat this into the ground, when diff returns 2, svnlook
> > quits immediately and doesn't generate the entire diff for the
> > revision, hence our commit email will be lacking info.
> 
> I don't understand.  Every time I've ever run diff on a binary file,
> it either prints nothing (no diff) or "Binary files foo and bar
> differ".  Is this not true for you?
> 
> Oh, I see.
> 
>    % cd /usr/share/pixmaps
>    % diff tycoon.png ximian_button_pill.png
>    Binary files tycoon.png and ximian_button_pill.png differ
>    % echo $?
>    2
> 
> So, the problem isn't so much that diff is run on binary files, it's
> that we aren't discerning the meaning of the 2 error code in this
> case.  And since error code 2 is a really generic error code for diff,
> the best thing to do is just to not diff binary files.  Am I on track?

I'm more concerned that something has changed and we don't know it.
All of the svn logo's got checked in a while ago and we didn't see
svnlook fail on the diffs.

I don't know why svnlook diff is running diff on binary files in the
first place.

Here's a commit on a logo that was updated

http://subversion.tigris.org/servlets/ReadMsg?list=svn&msgId=189515

It's not clear to me, is the message

Binary files /tmp/.svnlook/trunk/notes/logo/256-colour/subversion_logo_hor-237x32.png.64128.00001(null) and trunk/notes/logo/256-colour/subversion_logo_hor-237x32.png differ

from diff or from svn and svn was smart enough not to diff the file?

Blair

-- 
Blair Zajac <bl...@orcaware.com>
Web and OS performance plots - http://www.orcaware.com/orca/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svnlook diff fails on binary files:

Posted by cm...@collab.net.
Blair Zajac <bl...@orcaware.com> writes:

> Just to be beat this into the ground, when diff returns 2, svnlook
> quits immediately and doesn't generate the entire diff for the
> revision, hence our commit email will be lacking info.

I don't understand.  Every time I've ever run diff on a binary file,
it either prints nothing (no diff) or "Binary files foo and bar
differ".  Is this not true for you?

Oh, I see.

   % cd /usr/share/pixmaps
   % diff tycoon.png ximian_button_pill.png 
   Binary files tycoon.png and ximian_button_pill.png differ
   % echo $?
   2

So, the problem isn't so much that diff is run on binary files, it's
that we aren't discerning the meaning of the 2 error code in this
case.  And since error code 2 is a really generic error code for diff,
the best thing to do is just to not diff binary files.  Am I on track?

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svnlook diff fails on binary files:

Posted by Blair Zajac <bl...@orcaware.com>.
Just to be beat this into the ground, when diff returns 2, svnlook
quits immediately and doesn't generate the entire diff for the
revision, hence our commit email will be lacking info.

dd if=/dev/urandom of=file bs=1024 count=1
diff /dev/null file; echo $?

This always returns 2.

Blair

Blair Zajac wrote:
> 
> It looks like diff is being called on for binary files in svnlook.
> This will mess up our commit emails when binary files are added or
> modified in the repos.
> 
> rm -fr repos r
> svnadmin create repos
> svn co file://`pwd`/repos r
> cd r
> dd if=/dev/urandom of=file bs=1024 count=1
> svn add file
> svn ci -m ''
> cd ..
> svnlook repos diff
> 
> This generates:
> 
> Checked out revision 0.
> 1+0 records in
> 1+0 records out
> A  (bin)  file
> Adding  (bin)  file
> Transmitting file data .
> Committed revision 1.
> Added: file
> ==============================================================================
> Binary files file       (original) and file differ
> svn: Error calling external program
> svn: /usr/bin/diff returned 2
> 
> Best,
> Blair
> 
> --
> Blair Zajac <bl...@orcaware.com>
> Web and OS performance plots - http://www.orcaware.com/orca/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org