On Mon, 2010-09-20 at 18:45 +0100, Julian Foad wrote: > On Mon, 2010-09-20 at 13:17 -0400, Greg Stein wrote: > > On Thu, Sep 16, 2010 at 09:21, Julian Foad <julian.f...@wandisco.com> wrote: > > > Greg Stein wrote: > > >... > > >> Also, please note that I want to expand the presence values > > >> dramatically with this move to NODES. I suggest the following new > > >> values: > > > > > > Can you explain what these would mean, and what are the main advantages? > > > > Primarily: no need to scan to figure out what is going on. > > > > > Some of them look obvious at first glance but the details of > > > child-of-copy etc. are quite complex. If I were to start to guess... > > > > > > For op_depth = 0, we keep using the current set of 'presence' values, as > > > defined for BASE_NODE. > > > > Yes. > > > > > For op_depth > 0, these new values replace the old 'normal', > > > 'base-deleted' and 'not-present' presence values and the old > > > 'moved_here' flag. The old 'excluded' and 'incomplete' are still used. > > > > Yes. > > > > >> * copied [-here] > > >> * moved-here > > > > > > This is a root or a child of a copied/moved sub-tree. 'op_depth' > > > compared with 'local_relpath' depth indicates whether it's the root. > > > 'copyfrom_*' is non-null iff it's the root (?). > > > > Correct on all three parts. > > > > Note: the columns should be named original_* or repos_*, as discussed > > elsewhere. > > > > > Previous encoding: If it's the root, presence==normal + non-null > > > 'copyfrom_*' + 'moved_here'=FALSE/TRUE. If it's a child, > > > presence==normal and scan_addition reveals it's part of a copy/move. > > > > > > Benefits: Avoid scan_addition. Remove the 'moved_here' flag. > > > > Yes, yes. > > > > >> * moved-away (aka deleted) > > > > > > This is a root or a child of a moved-away sub-tree. The new WC > > > location (local relpath) is in 'moved_to' iff it's the root. > > > > Correct. With root detection based on op_depth and local_relpath as > > mentioned above. > > > > > Previous encoding: presence==base-deleted; if it's the root, non-null > > > 'moved_to', else scan_deletion reveals it's a move-away. > > > > The presence could be 'deleted' if you were moving a child of a > > copy/move-here. But note that wc_db does not implement moves, so we > > never got to these encoding details. > > > > > Benefits: Avoid scan_deletion. > > > > Yes. > > > > >> * deleted > > > > > > This node is deleted relative to the layer below. Previous encoding: > > > 'base-deleted' or 'not-present' depending on whether it was a child of a > > > copied subtree. > > > > Correct. > > > > >> * added > > > > > > A simple addition. Previous encoding: presence==normal, > > > copyfrom_*==null, scan_addition reveals it's a simple add. Benefits: > > > avoid scan_addition. > > > > Yes on all. > > > > >... > > > Are my notes close to what you were thinking? (I'm trying to write out > > > the states in a table to be more rigorous about it.) > > > > Spot-on :-D > > > > (part of the reason to expand the set: their semantics become obvious, > > compared to the current encoding) > > OK, good, thanks. *More* obvious, yes. (Obvious, not quite: it still > took me half a day of figuring to come up with this concise > description.) > > > FULL SET OF VALUES > > The values listed above cover most of the cases. Next we must consider > how to get a full set of values to represent all possible changes. > > The possible changes to an unoccupied path are: > > added > copied-here (as root or as child of op) > moved-here (as root or as child of op) > > and the possible changes to an occupied path are: > > [ > delete > moved-away (as root or as child of op) > ] > x > [ > <no replacement> > added > copied-here (as root or as child of op) > moved-here (as root or as child of op) > ] > > Are you planning to encode all these combinations as separate values? > > That's 11 in total, plus whatever valid combinations that involve > excluded/absent/file-external.
>From IRC: <julianf> ehu: Re. deletes: I think you're feeling, and I feel as well a bit, that "delete" and "move-away" are really operations on the layer below and thus should be indicated within the layer below: there is no perhaps need to create a new row because there is no repos-node-id or node-kind or other node information to store about a delete or move-away. The only info there is ... the fact that it's del or mv-away, and (possibly) this (not really well defined) moved_to path. <ehu> yes. that's my feeling. I understand the choices with respect to where we are today. but the question is if the single presence value in the top-level layer really does what it needs to, now that we're moving to a multi-layer model. <julianf> ehu: So maybe it makes sense to swap the layering of the "creation" part of a NODES row and the "deletion" part of a row. Let the "creation" part be considered as the lower half of the layer, and then a possible "deleted/moved-to-PATH" indication to be considered as an operation "above" the creation (in the sense of "nearer to the user's working version"). - Julian > > > If this does enable us to eliminate the use of scan_addition and > > > scan_deletion in some cases, even if they (or some simpler versions) are > > > still needed for finding the copyfrom info in other cases, that would be > > > a worthwhile change. > > > > I was originally trying to be minimalist with the presence values, but > > after use/implementation it became obvious that we can simply encode > > operations in the nodes rather than requiring a scan for them. > > > > scan_addition will receive the most benefit since it resolves an "add" > > to an "add/copy-here/move-here". Or in today's implementation add vs > > copy. The scan_deletion never really has to resolve since we don't > > truly implement moved-away. The *concepts* are still in the code > > because I think we need the design and direction rigor. > > > > For the 1.7 release, we should keep scan_* and just change their > > internals. We can rethink other APIs for future releases since these > > are internal to WC. The implementation is generally reading just a row > > or two: read the target row, examine its op_depth, and read the > > operation root's row. We don't have to scan upwards to find the > > operation root. > > > > Cheers, > > -g > >
