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
Received on 2015-09-29 15:10:09 CEST