subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Julian Foad <julianf...@btopenworld.com>
Subject Re: svn commit: r1619452 - in /subversion/trunk/subversion: libsvn_client/diff.c tests/cmdline/diff_tests.py
Date Fri, 22 Aug 2014 11:47:55 GMT
Bert Huijben wrote:

> Julian Foad wrote:
>>  URL: http://svn.apache.org/r1619452
>>  Log:
>>  Fix the copy-from revision number reported in a diff header. It previously
>>  reported 'nonexistent' instead of the copy-from revision in some cases.
>> 
>>  This bug seems to be a regression; 1.8.x releases show the correct revision.
> 
> 1.8.x just reported the original/left revision in a lot of cases where it 
> didn't know the revision.

You're right -- 1.8.x inconsistently showed the copy-from revision in only some cases, and
"(revision 0)" in some cases, and possibly other things in other cases, when diffing a copied
node. For example (running trunk@head):

[[[
$ svn cp tools/hook-scripts/mailer@1619388 hs

  # remove the uninteresting mergeinfo addition
$ svn propdel svn:mergeinfo hs

$ svn propedit p-new hs hs/mailer.py

$ svn18 diff hs/
Index: hs/mailer.py
===================================================================
--- hs/mailer.py    (revision 1619388)
+++ hs/mailer.py    (working copy)

Property changes on: hs/mailer.py
___________________________________________________________________
Added: p-new
## -0,0 +1 ##
+v
Index: hs
===================================================================
--- hs    (revision 0)
+++ hs    (working copy)

Property changes on: hs
___________________________________________________________________
Added: svn:ignore
## -0,0 +1 ##
+mailer.conf
Added: p-new
## -0,0 +1 ##
+v
]]]

So I shouldn't describe this as a regression.

> Especially on directories where 1.8.x where the old internal diff callbacks 
> didn't even report any revision... where the diff output just guessed the 
> revision based on the revision of the complete diff (or whatever it expected it 
> to be).
> 
> 
> I don't think we ever reported the copy from revision, as the actual 
> left-src revision of the diff...

Yes we did -- at least in cases like 'mailer.py' in the example above.

> And I'm not sure if reporting copy from is really valid either, without 
> reporting that it is a copy and probably more importantly where the copy is 
> from.
> 
> The copy can be from any repository path at the specified revision... 
> So now you can no longer trust the revision to just specify that the path 
> existed at the current path in that revision.

Well, yes... It would be better if the rev and the path matched each other.

It's all quite a mess. I think it now always shows the copy-from rev when diffing a copy (and
'nonexistent' when showing a copy as an add). It's now more consistent than it was, in that
way.

It's stupid to show a diff whose header doesn't match the diff hunks -- a header that says
'this is a diff between trunk@10 and trunk@20' followed by a hunk actually containing a diff
between branch@15 and trunk@20.

We should show header info (paths and revisions) that matches the diff hunks. So, when we
are displaying diffs against copy-from, the 'left' header should show the copy-from. We haven't
done this before but we should make it so.

Agree?

- Julian

Mime
View raw message