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.foad_at_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
>
>
Received on 2010-09-20 20:08:28 CEST