The 'revert' to something like '1.8' also introduces most changes on 1.9.x 
behavior. If you look at it that way, you have more to review, the more you 
revert to 1.8.

I'm still not convinced that the 1.8 behavior is really the behavior we want 
*everywhere*. Julian's great document outlines the different ways we look at 
this in different scenarios.

Noting everything as a change even if there is no actual delta... still looks 
like a bug we should fix (and we did) instead of something that we should 
restore.

For the reasoning on why:
For blame we are really interested if there are deltas... and if there aren't 
any changes the revision is not 'interesting'. (See docs of the api).

Blame with mergeinfo is just a hack, which has all kinds of assumptions on how 
closely all merged branches are related. We simply don't store the information 
to really obtain where the change originated*. The way this code (build as a 
summer of code project if I remember correct) assumed that every revision was 
reported as interesting is just one of the many limitations.

Keeping this broken code working is not a goal by itself, that should force us 
to keep our apis inefficient.

Bert

* I think we once (far before 1.5) had a branch that started to really store 
what was needed here. Perhaps Julian's current work will improve things later 
on.

Sent from Mail for Windows 10



From: Evgeny Kotkov
Sent: woensdag 21 oktober 2015 14:22
To: Stefan Fuhrmann
Cc: Johan Corveleyn;Julian Foad;dev
Subject: Re: No-op changes no longer dumped by 'svnadmin dump' in 1.9


Stefan Fuhrmann <stefan.fuhrm...@wandisco.com> writes:

> Could you at least use the new API in svn_repos__compare_files instead
> of re-implementing parts of the FS back-end but worse? I know this is
> the code as it has been in 1.8 but that does not make the it any better.

Speaking of /branches/1.9.x, I would like to nominate this change as is.
It should be easier to review, because we would be restoring things to a
known previous state, instead of mixing new with old.

As for /trunk, I think that we could do that, so I sketched this option in
the attached patch.  Currently I am not sure that there are no subtle but
important differences between two implementations, so doing this is going
to require a bit more time.  Hopefully, I would be able to sort it out after
we're done with the backport.


Regards,
Evgeny Kotkov


Reply via email to