Re: [Monotone-devel] automate inventory

[Stephen Leake]

Derek Scherger <address@hidden> writes:

> I vaguely recall some mention of a restricted inventory case where only
> one side of a rename was listed in the inventory. Is there a test for
> this? 

Yes; see tests/automate_inventory_restricted/__driver__.lua, search
for the comment:

-- Rename a file from dir_a to dir_b, bookkeep-only; inventory dir_a


> Is it still a problem?

It reports the correct results.

> I wonder if this case is the one that is fixed in the
> net.venge.monotone.experiment.restricted_rosters branch. 

I fixed this purely in the automate inventory code; we just had to
pay attention to this case. See automate.cc, inventory_rosters; there
is a comment:

          // There is no new node available; this is either a drop or a
          // rename to outside the current path restriction.

and later a similar comment for the reverse case:

          // There is no old node available; this is either added or a
          // rename from outside the current path restriction.


> BTW, has anyone looked at this branch? I think it's an improvement
> and fixes a couple of invariant failures. Please see my earlier
> emails for details.

I just looked thru the log and the code. I gather the main point is
the function make_restricted_roster. There are no comments saying what
it does or why it is there (vis-a-vis Richard's recent complaint :).

I'm guessing it builds a roster representing the changes from 'from' to
'to', but including only those nodes that match 'mask'; is that right?
The names certainly suggest that, and if in general names in monotone
are accurate, perhaps it does not need a comment saying this.

It may be that automate inventory could use make_restricted_roster
rather than doing the same work itself. It depends on whether the
make_restriced_roster handles the renames (and other off-nominal)
cases the way inventory needs to. That definitely should be stated in
a comment; "both sides of renames to/from outside the restriction are
mentioned in the `restricted' roster" (or "not mentioned", as the case
may be).

make_restricted_roster appears to be more efficient than the automate
inventory code; make_restricted_roster does one pass thru the parents,
and one thru the result, while automate inventory does several loops
thru several lists. But there may be hidden costs that I don't see;
I'd have to run some timing tests to tell which is better.

It also detects (and fails on) some "problems" that inventory ignores;
I'm not clear whether inventory should ignore them.

I see make_restricted_roster is used in diff and merge, as an
intermediate to makeing the restriced csets. I guess the new code is
faster? Or fixes some bugs?

In any case, I think the way forward here is to merge
basic_io.inventory to main, then propagate main to
experiment.restricted_rosters, and then try using
make_restricted_roster in automate inventory in that branch.

-- 
-- Stephe