You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Marcus Comstedt <ma...@mc.pp.se> on 2002/04/11 14:57:20 UTC

GNU diff or not GNU diff, that is the question...

Hi.

I've been off the list for a while because the importer I was using to
read it broke down (I've subscribed via normal email now), so my
apologies if this has already been hashed through.

Nearly all testsuite tests now work again after the "new commit
system" hullabaloo, but diff_tests.py is still a no-go for me, and
that for a very simple reason:

START: diff_tests.py
['/usr/bin/diff: illegal option -- u\n', 'usage: diff [-bitw] [-c | -e | -f | -h | -n] file1 file2\n', '       diff [-bitw] [-C number] file1 file2\n', '       diff [-bitw] [-D string] file1 file2\n', '
diff [-bitw] [-c | -e | -f | -h | -n] [-l] [-r] [-s] [-S name] directory1 directory2\n']
FAIL: diff_tests.py 1: update a file

etc etc.  There are two possible solutions

1) If svn diff really needs GNU diff, then it should run GNU diff
   (gdiff on my system) and not any old /usr/bin/diff.

2) If svn diff doesn't really need GNU diff, then it should not assume
   GNU diff by using the -u flag without checking first.


  // Marcus



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

Re: GNU diff or not GNU diff, that is the question...

Posted by cm...@collab.net.
cmpilato@collab.net writes:

> Marcus Comstedt <ma...@mc.pp.se> writes:
> 
> > Karl Fogel <kf...@newton.ch.collab.net> writes:
> > 
> > > What makes you so sure it's a regression? :-) Maybe it's always been
> > > there...
> > 
> > No it hasn't.  My test used to be able to commit its test file this
> > way (and then it would fail later on of course), but now I had to
> > change it into committing the directory in order to reach the place
> > where it was supposed to fail.
> 
> He's right Karl.  This is a feature of the new commit system.  I'll
> look into it.
> 
> > Roger Wilco.  I don't have time to look for the bug at the mo, but a
> > patch to commit_tests.py is attached (good thing "svn diff" works
> > again... :).  It's basically the same as the old "commit_one_file"
> > test, but with the file of choice changed to one of the newly added
> > ones.  Hm, maybe there should be a test for commiting a single remove
> > as well?  Mañana...
> 
> I know exactly where the bug lies, so don't waste your time (unless
> you just want to fix it yourself before I get to it).  Thanks for the
> testing code, though!

Nailed in revision 1676.

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

Re: GNU diff or not GNU diff, that is the question...

Posted by cm...@collab.net.
Marcus Comstedt <ma...@mc.pp.se> writes:

> Karl Fogel <kf...@newton.ch.collab.net> writes:
> 
> > What makes you so sure it's a regression? :-) Maybe it's always been
> > there...
> 
> No it hasn't.  My test used to be able to commit its test file this
> way (and then it would fail later on of course), but now I had to
> change it into committing the directory in order to reach the place
> where it was supposed to fail.

He's right Karl.  This is a feature of the new commit system.  I'll
look into it.

> Roger Wilco.  I don't have time to look for the bug at the mo, but a
> patch to commit_tests.py is attached (good thing "svn diff" works
> again... :).  It's basically the same as the old "commit_one_file"
> test, but with the file of choice changed to one of the newly added
> ones.  Hm, maybe there should be a test for commiting a single remove
> as well?  Mañana...

I know exactly where the bug lies, so don't waste your time (unless
you just want to fix it yourself before I get to it).  Thanks for the
testing code, though!

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

Re: GNU diff or not GNU diff, that is the question...

Posted by Marcus Comstedt <ma...@mc.pp.se>.
Karl Fogel <kf...@newton.ch.collab.net> writes:

> What makes you so sure it's a regression? :-) Maybe it's always been
> there...

No it hasn't.  My test used to be able to commit its test file this
way (and then it would fail later on of course), but now I had to
change it into committing the directory in order to reach the place
where it was supposed to fail.


> Can you package up your reproduction recipe as a new test (i.e., a
> patch to commit_tests.py) and post it here?  (I mean, if you want to
> fix the bug too that's even better, but if not, just having the test
> case is more than half the battle for whoever does fix it.)

Roger Wilco.  I don't have time to look for the bug at the mo, but a
patch to commit_tests.py is attached (good thing "svn diff" works
again... :).  It's basically the same as the old "commit_one_file"
test, but with the file of choice changed to one of the newly added
ones.  Hm, maybe there should be a test for commiting a single remove
as well?  Mañana...


  // Marcus



Re: GNU diff or not GNU diff, that is the question...

Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Marcus Comstedt <ma...@mc.pp.se> writes:
> So a new test i commit_tests.py to catch this regression would
> probably be in order.

What makes you so sure it's a regression? :-) Maybe it's always been
there...

Can you package up your reproduction recipe as a new test (i.e., a
patch to commit_tests.py) and post it here?  (I mean, if you want to
fix the bug too that's even better, but if not, just having the test
case is more than half the battle for whoever does fix it.)

-K


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

Re: GNU diff or not GNU diff, that is the question...

Posted by Marcus Comstedt <ma...@mc.pp.se>.
Karl Fogel <kf...@newton.ch.collab.net> writes:

> Okay, Marcus, try it again.  The build process should find a diff that
> supports "-u", if there is one, or error out if not.

Looking fine.

[...]
checking for diff... /pike/sw/bin/gdiff
[...]
Running all tests in diff_tests.py...success
[...]

Btw, while messing around with the testsuite (and updating my
"unneccesary conflict" test, which still fails), I noticed a rather
strange bug that the rest of the testsuite mysteriously manages to
avoid somehow (my test caught it though, purely by chance):

pelix:~% svn co -d test_wc file:///pike/data/repos/empty/ # new repos
pelix:~% cd test_wc/
pelix:~/test_wc% echo 'what now?' > brown_cow
pelix:~/test_wc% svn add brown_cow
A          brown_cow
pelix:~/test_wc% svn commit -m 'log msg' brown_cow # now check this out

subversion/libsvn_client/commit.c:590
svn_error: #21032 : <Entry has no url>
  Commit failed (details follow):

subversion/libsvn_client/commit_util.c:412
svn_error: #21032 : <Entry has no url>
  Entry for `/export/spare/pike/home/marcus/test_wc/brown_cow' has no URL.  Perhaps you're committing inside of an unversioned (or not-yet-versioned) directory?
pelix:~/test_wc% svn commit -m 'log msg' . # this works though
Adding    brown_cow
Transmitting file data .
Committed revision 1.
pelix:~/test_wc% echo 'eat chow' >> brown_cow
pelix:~/test_wc% svn commit -m 'log msg' brown_cow # and now this works too
Sending   brown_cow
Transmitting file data .
Committed revision 2.
pelix:~/test_wc%

So a new test i commit_tests.py to catch this regression would
probably be in order.


  // Marcus



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

Re: GNU diff or not GNU diff, that is the question...

Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Okay, Marcus, try it again.  The build process should find a diff that
supports "-u", if there is one, or error out if not.

-Karl

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

Re: GNU diff or not GNU diff, that is the question...

Posted by cm...@collab.net.
Ben Collins-Sussman <su...@collab.net> writes:

> So yes, we should either stop passing -u (thereby forcing users to
> pass -u in via -X, either manually, or config file), or just make our
> configuration look for GNU diff.
> 
> I prefer the latter option, really.  If a system is required to have
> GNU diff3, then it will *always* have GNU diff as well.  They're both
> built and installed by the diffutils package.  Better to go for
> consistency.

Yes yes yes.  Consistency, and -u as a default, are both good things.

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

Re: GNU diff or not GNU diff, that is the question...

Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Branko Čibej <br...@xbc.nu> writes:
> Well, it would make sense (to me) to use diff and diff3 from the same
> package. And you only have to get check-diff3 prepend its own location
> to the searck list for check-diff.

I'm deep into text-base checksumming, but feel free if you want...

-K

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

Re: GNU diff or not GNU diff, that is the question...

Posted by Branko Čibej <br...@xbc.nu>.
Karl Fogel wrote:

>Branko Čibej <br...@xbc.nu> writes:
>
>>Wouldn't it make sense to first look for diff where we found diff3,
>>and try that, and only look farther if that fails?
>>
>
>Yes, in some sense.  But this is code that gets run at build-time.  I
>don't see much point gaining a tiny bit of efficiency at the cost of
>extra complexity (and it would be more complex -- we'd be adding a new
>bit of code that looks in one specific place, but the default loop in
>check-diff.sh would have to stay, just in case we fail to find diff in
>that place).
>
>Doesn't seem worth it to me... (?)
>
Well, it would make sense (to me) to use diff and diff3 from the same 
package. And you only have to get check-diff3 prepend its own location 
to the searck list for check-diff.


-- 
Brane Čibej   <br...@xbc.nu>   http://www.xbc.nu/brane/




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

Re: GNU diff or not GNU diff, that is the question...

Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Branko Čibej <br...@xbc.nu> writes:
> Wouldn't it make sense to first look for diff where we found diff3,
> and try that, and only look farther if that fails?

Yes, in some sense.  But this is code that gets run at build-time.  I
don't see much point gaining a tiny bit of efficiency at the cost of
extra complexity (and it would be more complex -- we'd be adding a new
bit of code that looks in one specific place, but the default loop in
check-diff.sh would have to stay, just in case we fail to find diff in
that place).

Doesn't seem worth it to me... (?)

-K

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

Re: GNU diff or not GNU diff, that is the question...

Posted by Branko Čibej <br...@xbc.nu>.
Karl Fogel wrote:

>Karl Fogel <kf...@newton.ch.collab.net> writes:
>
>>I'll make it use gnu diff again, then we'll
>>
>
>Excuse me -- what I meant was, I'll make it find a diff that supports
>the "-u" flag.  Whether that's GNU diff or not, we don't care. :-)
>
>Testing the change now...
>

Wouldn't it make sense to first look for diff where we found diff3, and 
try that, and only look farther if that fails?


-- 
Brane Čibej   <br...@xbc.nu>   http://www.xbc.nu/brane/




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

Re: GNU diff or not GNU diff, that is the question...

Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Karl Fogel <kf...@newton.ch.collab.net> writes:
> I'll make it use gnu diff again, then we'll

Excuse me -- what I meant was, I'll make it find a diff that supports
the "-u" flag.  Whether that's GNU diff or not, we don't care. :-)

Testing the change now...

-K

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

Re: GNU diff or not GNU diff, that is the question...

Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Ben Collins-Sussman <su...@collab.net> writes:
> I prefer the latter option, really.  If a system is required to have
> GNU diff3, then it will *always* have GNU diff as well.  They're both
> built and installed by the diffutils package.  Better to go for
> consistency.

Ah, found it: rev 1503.  I'll make it use gnu diff again, then we'll
see if that solves Marcus's problem.

-K

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

Re: GNU diff or not GNU diff, that is the question...

Posted by Ben Collins-Sussman <su...@collab.net>.
Kevin Pilch-Bisson <ke...@pilch-bisson.net> writes:

> Maybe sussman remembers better, but what I thought happened was that
> when we switched to using diff3, we decided that we still needed gnu
> diff (since diff3 runs it under the hood), but we could you any diff
> for svn diff.  Hence the problem.

Exactly.  Our ac-helper scripts look specifically for GNU diff3, but
look for any old program called 'diff'.  diff3 is used for merging,
updates, and conflict markers -- the real work.  diff is used *only*
to display stuff on the screen when you run 'svn diff'.  Hence the lax
requirement.

My mistake here, however, was thinking that every version of 'diff'
out there supports the -u switch.  Apparently not!

So yes, we should either stop passing -u (thereby forcing users to
pass -u in via -X, either manually, or config file), or just make our
configuration look for GNU diff.

I prefer the latter option, really.  If a system is required to have
GNU diff3, then it will *always* have GNU diff as well.  They're both
built and installed by the diffutils package.  Better to go for
consistency.

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

Re: GNU diff or not GNU diff, that is the question...

Posted by Alan Shutko <at...@acm.org>.
Marcus Comstedt <ma...@mc.pp.se> writes:

> Karl Fogel <kf...@newton.ch.collab.net> writes:
>
>> If "-c" is common to all diffs, then we could use that instead of
>> "-u".
>
> AFAIK, it is (I checked HP-UX, IRIX, AIX, and UNICOS).

It is part of the Single Unix Specification (v2 is what I have) and is
on pretty much every Unix I've seen (including old SCO).

-- 
Alan Shutko <at...@acm.org> - In a variety of flavors!
Stupidity got us into this mess -- why can't it get us out?

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

Re: GNU diff or not GNU diff, that is the question...

Posted by Marcus Comstedt <ma...@mc.pp.se>.
Karl Fogel <kf...@newton.ch.collab.net> writes:

> If "-c" is common to all diffs, then we could use that instead of
> "-u".

AFAIK, it is (I checked HP-UX, IRIX, AIX, and UNICOS).  But I agree
with the others that as long as GNU diffutils is required anyway, it's
probably better to just use GNU diff and get the extra consistency.
(I also happen to prefer unified over context diffs. :)


  // Marcus



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

Re: GNU diff or not GNU diff, that is the question...

Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Kevin Pilch-Bisson <ke...@pilch-bisson.net> writes:
> I think we are on the right track and what is needed is a removal of the -u
> passed to to diff, and a way to specify in ~/.subversion/options to pass
> -X"-u" to svn diff by default.

If "-c" is common to all diffs, then we could use that instead of
"-u".  It was sort of a toss-up when we implemented it, if I recall
correctly.  Although a significant majority preferred -u, nearly all
qualified their preference as "mild".

But then again, if we're still requiring GNU diff *anyway*...

> Maybe sussman remembers better, but what I thought happened was that
> when we switched to using diff3, we decided that we still needed gnu
> diff (since diff3 runs it under the hood), but we could you any diff
> for svn diff.  Hence the problem.

... then we might as well just use it for "svn diff" too, and stick
with -u.  Ben (Collins-Sussman), do you remember what we changed such
that "svn diff" now might run something other than GNU diff?

Either way, it's important that the output of "svn diff" default to
one of the human-friendly formats (context or unified).

-K

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

Re: GNU diff or not GNU diff, that is the question...

Posted by Kevin Pilch-Bisson <ke...@pilch-bisson.net>.
On Thu, Apr 11, 2002 at 10:17:03AM -0500, cmpilato@collab.net wrote:
> Marcus Comstedt <ma...@mc.pp.se> writes:
> 
> > START: diff_tests.py
> > ['/usr/bin/diff: illegal option -- u\n', 'usage: diff [-bitw] [-c | -e | -f | -h | -n] file1 file2\n', '       diff [-bitw] [-C number] file1 file2\n', '       diff [-bitw] [-D string] file1 file2\n', '
> > diff [-bitw] [-c | -e | -f | -h | -n] [-l] [-r] [-s] [-S name] directory1 directory2\n']
> > FAIL: diff_tests.py 1: update a file
> > 
> > etc etc.  There are two possible solutions
> > 
> > 1) If svn diff really needs GNU diff, then it should run GNU diff
> >    (gdiff on my system) and not any old /usr/bin/diff.
> > 
> > 2) If svn diff doesn't really need GNU diff, then it should not assume
> >    GNU diff by using the -u flag without checking first.
> 
> We *do* require GNU diff, and `configure' should be testing that you
> have such.  Wonder what's going on...

Maybe sussman remembers better, but what I thought happened was that when we
switched to using diff3, we decided that we still needed gnu diff (since diff3
runs it under the hood), but we could you any diff for svn diff.  Hence the
problem.

I think we are on the right track and what is needed is a removal of the -u
passed to to diff, and a way to specify in ~/.subversion/options to pass
-X"-u" to svn diff by default.


-- 
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Kevin Pilch-Bisson                    http://www.pilch-bisson.net
     "Historically speaking, the presences of wheels in Unix
     has never precluded their reinvention." - Larry Wall
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Re: GNU diff or not GNU diff, that is the question...

Posted by cm...@collab.net.
Marcus Comstedt <ma...@mc.pp.se> writes:

> START: diff_tests.py
> ['/usr/bin/diff: illegal option -- u\n', 'usage: diff [-bitw] [-c | -e | -f | -h | -n] file1 file2\n', '       diff [-bitw] [-C number] file1 file2\n', '       diff [-bitw] [-D string] file1 file2\n', '
> diff [-bitw] [-c | -e | -f | -h | -n] [-l] [-r] [-s] [-S name] directory1 directory2\n']
> FAIL: diff_tests.py 1: update a file
> 
> etc etc.  There are two possible solutions
> 
> 1) If svn diff really needs GNU diff, then it should run GNU diff
>    (gdiff on my system) and not any old /usr/bin/diff.
> 
> 2) If svn diff doesn't really need GNU diff, then it should not assume
>    GNU diff by using the -u flag without checking first.

We *do* require GNU diff, and `configure' should be testing that you
have such.  Wonder what's going on...

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