> -----Original Message-----
> From: Julian Foad [mailto:julianfoad_at_btopenworld.com]
> Sent: vrijdag 22 augustus 2014 13:48
> To: Bert Huijben
> Cc: dev_at_subversion.apache.org
> Subject: Re: svn commit: r1619452 - in /subversion/trunk/subversion:
> libsvn_client/diff.c tests/cmdline/diff_tests.py
>
> 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_at_head):
>
> [[[
> $ svn cp tools/hook-scripts/mailer_at_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.
+1
The interesting thing is how we should improve it further.
>
> > 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_at_10 and trunk_at_20' followed
by
> a hunk actually containing a diff between branch_at_15 and trunk_at_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?
Agree.
What we currently show in the headers is very inconsistent.
For 1.9 I tried to cleanup the code that creates these paths, but we update
the values all over the place in the diff code, that calls the diff drivers.
(That is why we have those stupid output arguments and if I remember
correctly even a callback in the runner code now... I tried to refactor
everything out, but kept finding dependencies that need updating)
Bert
>
> - Julian
Received on 2014-08-22 13:59:20 CEST