On Tue, Sep 29, 2015 at 1:47 PM, Stefan Fuhrmann
<stefan.fuhrmann_at_wandisco.com> wrote:
> On Mon, Sep 28, 2015 at 10:55 AM, Stefan Fuhrmann
> <stefan.fuhrmann_at_wandisco.com> wrote:
>>
>> On Wed, Sep 23, 2015 at 12:55 PM, Julian Foad <julianfoad_at_btopenworld.com>
>> wrote:
>>>
>>> > Johan Corveleyn wrote:
>>> >> [...] stefan2 told me in person that that part of the
>>> >> change in r1572363 was unintentional :-). IIUC, he didn't realize that
>>> >> it would have this effect on the output of dump.
>>> [...]
>>> >>>> I think the dump.c part of r1572363 and r1573111 should be reverted
>>> >>>> /
>>> >>>> fixed so that we get the previous behaviour again, and this should
>>> >>>> be
>>> >>>> backported to 1.9. At this point, IMO 'svnadmin dump' is broken in
>>> >>>> 1.9.
>>>
>>> 1. I pretty much agree now that we should revert the change.
>>
>>
>> I suggest that we apply the patch below. It is more efficient
>> that the original behaviour but ensures that "no-op" changes
>> will be retained.
>
>
> Now that I've read through the whole thread, here my view
> on this problem plus an explanation why I think my patch is
> the right approach.
>
> First some facts and things that I believe:
>
> * Internally, FSFS can record no-op prop and no-op content
> changes (the delta between new representation and the
> respective proplist / text of its predecessor is empty). There
> is even the possibility of a no-op node change, i.e. a new
> node without touching text nor props (not happening atm).
> * SVN versions file trees and the decision how to represent
> them (e.g. as delta) is an implementation detail.
> * Relying on implementation details of a lower layer is bad.
>
> * Via FS and Repos layer API, new instances of no-op changes
> to either text and properties can be created.
> * Dump files (produced by 3rd party tools or non-incremental
> svnadmin dumps) are another source of those no-op changes.
> * SVN editors don't produce no-op changes.
>
> * A commit links user-controlled revision meta data (e.g. comment)
> to a user-controlled list of changes. That link shall not be
> lost when tweak implementation details.
> * Dump / load is a compatibility interface between SVN versions
> and allows for forward and backward migration.
> * A plain node modification (no text nor prop change) will be
> ignored by "svnadmin load". Even if we changed that on /trunk,
> it would not fix old releases.
>
> From that, we can derive a few requirements:
>
> * If the a revision contains a "M" change, svnadmin must produce
> a dump file that causes previous releases to produce the "M"
> change - within reason.
> * Load shall produce no-op changes for the same data as in previous
> releases - within reason. If the FS API would no longer support
> creating no-op changes at some point in the future, the load process
> should issue a warning.
> * The dump file contents shall be independent of FS implementation
> details, i.e. use "strict" FS API functions.
>
> No-op changes are rare, hence including them into a dump file
> does not significantly increase its size or processing time. The
> text / proplist asymmetry in the patch is due to the fact that all
> files due have a text but maybe no proplist representation. So,
> dumping the text is always safe.
>
> V2 of the patch also covers no-op prop changes to directories.
I haven't reviewed the patch, but I agree with all of your points above.
Thanks for taking this on (both you and Julian, and others
participating in the discussion).
--
Johan
Received on 2015-09-29 15:06:33 CEST