On Tue, Sep 29, 2015 at 1:47 PM, Stefan Fuhrmann <stefan.fuhrm...@wandisco.com> wrote: > On Mon, Sep 28, 2015 at 10:55 AM, Stefan Fuhrmann > <stefan.fuhrm...@wandisco.com> wrote: >> >> On Wed, Sep 23, 2015 at 12:55 PM, Julian Foad <julianf...@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
