Thanks for the comments, Stefan. Stefan Fuhrmann wrote: > On Mon, Sep 28, 2015, Stefan Fuhrmann wrote: >> 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).
When you say "not happening ATM", are you referring to 1.9 or all released versions of FSFS or something else? > * SVN versions file trees and the decision how to represent > them (e.g. as delta) is an implementation detail. Yes, this is a key point. It's not quite as simple as that, since we do need to distinguish whether the node at /foo is a continuation of the previous node at /foo or is a new node that replaces it, and a similar thing with identifying copies. But to a large extent, yes, the system is designed to behave as storing a sequence of tree snapshots. > * 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. An svn_delta_editor can implicitly describe all sorts of no-op changes. For example, if I add svn_delta__get_debug_editor() into svnmucc's commit, we can make no-op changes to a file's text and properties as follows: $ svnmucc -U $REPO -m "" put repo/format foo DBG: open_root : 6 DBG: open_file : 'foo':6 DBG: apply_textdelta : (null) DBG: close_file : 1dcca23355272056f04fe8bf20edfce0 DBG: close_directory DBG: close_edit r7 committed ... $ svnmucc -U $REPO -m "" propset p v foo DBG: open_root : 7 DBG: open_file : 'foo':7 DBG: change_file_prop : p -> v DBG: close_file : (null) DBG: close_directory DBG: close_edit r8 committed ... $ svn log -v --xml -r7:8 $REPO | grep "revision\|action\|mods" revision="7"> action="M" prop-mods="false" text-mods="true" revision="8"> action="M" prop-mods="true" text-mods="false" $ svn diff -c7 $REPO $ svn diff -c8 $REPO And an editor can describe many more kinds of no-op change too, such as "add /foo; delete /foo" and so on. It's up to the edit receiver to decide whether to collapse 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. When you say "list of changes", I agree if you mean the "difference between snapshots" in principle, rather than the specific list that's stored at the end of a revision file. > * 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. What's a "plain node modification (no text nor prop change)"? Ambiguous w.r.t. no-op changes. > 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. The problem is, we don't have an agreed definition of an 'M' change. > * 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. I'm not sure to what you're referring here. The 'strict' flag in the text and props comparison functions? That would omit no-op changes. > No-op changes are rare, hence including them into a dump file > does not significantly increase its size or processing time. Ack. > 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 don't see that we've decided whether we want to preserve no-op text changes, no-op prop changes, no-op node changes, and/or other. It looks like your patch is aimed at preserving the no-op node change. This correlates with an 'M' line in the 'svn log -v' output. Do we also care about no-op text and property changes, such as are reported in 'svn log -v --xml'? More precisely, we should define what we care about in terms of an API. - Julian
