Re: [Monotone-devel] 0.26 status update

[Nathaniel Smith]

On Tue, Apr 04, 2006 at 04:07:13AM -0700, Nathaniel Smith wrote:
>   -- Oddity -- there's some ugliness to roster_t's handling of
>      redundant operations.  It's a little technical, I'll, uh, just
>      append the big comment I wrote, down below where only people who
>      are interested have to read it :-).

Down below in _another message_, of course!  For even more convenience
separating it out.

  // This requires some explanation.  There is a particular kind of
  // nonsensical behavior which we wish to discourage -- when a node is
  // detached from some location, and then re-attached at that same location
  // (or similarly, if a new node is created, then immediately deleted -- this
  // is like the previous case, if you think of "does not exist" as a
  // location).  In particular, we _must_ error out if a cset attempts to do
  // this, because it indicates that the cset had something non-normalized,
  // like "rename a a" in it, and that is illegal.  There are two options for
  // detecting this.  The more natural approach, perhaps, is to keep a chunk
  // of state around while performing any particular operation (like cset
  // application) for which we wish to detect these kinds of redundant
  // computations.  The other option is to keep this state directly within the
  // roster, at all times.  In the first case, we explicitly turn on checking
  // when we want it; the the latter, we must explicitly turn _off_ checking
  // when we _don't_ want it.  We choose the latter, because it is more
  // conservative --- perhaps it will turn out that it is _too_ conservative
  // and causes problems, in which case we should probably switch to the
  // former.
  // 
  // FIXME: This _is_ all a little nasty, because this can be a source of
  // abstraction leak -- for instance, roster_merge's contract is that nodes
  // involved in name-related will be detached in the roster it returns.
  // Those nodes really should be allowed to be attached anywhere, or dropped,
  // which is not actually expressible right now.  Worse, whether or not they
  // are in old_locations map is an implementation detail of roster_merge --
  // it may temporarily attach and then detach the nodes it creates, but this
  // is not deterministic or part of its interface.  The main time this would
  // be a _problem_ is if we add interactive resolution of tree rearrangement
  // conflicts -- if someone resolves a rename conflict by saying that one
  // side wins, or by deleting one of the conflicting nodes, and this all
  // happens in memory, then it may trigger a spurious invariant failure here.
  // If anyone ever decides to add this kind of functionality, then it would
  // definitely make sense to move this checking into editable_tree.  For now,
  // though, no such functionality is planned, so we'll see what happens.
  //
  // The implementation itself uses the map old_locations.  A node can be in
  // the following states:
  //   -- attached, no entry in old_locations map
  //   -- detached, no entry in old_locations map
  //      -- create_dir_node, create_file_node put a node into this state
  //      -- a node in this state can be attached, anywhere, but may not be
  //         deleted.
  //   -- detached, an entry in old_locations map
  //      -- detach_node puts a node into this state
  //      -- a node in this state can be attached anywhere _except_ the
  //         (parent, basename) entry given in the map, or may be deleted.
  std::map<node_id, std::pair<node_id, path_component> > old_locations;

-- Nathaniel

-- 
"Of course, the entire effort is to put oneself
 Outside the ordinary range
 Of what are called statistics."
  -- Stephan Spender