[Monotone-devel] restriction fixes

[Derek Scherger]

Stephen Leake wrote:
> Derek Scherger <address@hidden> writes:
> 
>> 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

Thanks!

> 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 :).

My attempt to describe it is here
   http://www.mail-archive.com/address@hidden/msg09026.html

Yes, the point is to fix a specific problem with renamed directories.
The *result* is the change from make_restricted_csets to
make_restricted_roster.

I don't recall anything specific from Richard on this. Are you talking
about the thread on commit/ChangeLog messages?

> 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.

That's the general idea yeah.

> 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

It handles them *correctly* unlike make restricted cset in some cases!
I'm not sure what would be special about what inventory needs?

> 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.

The changes on the restricted_rosters branch don't have anything to do
with performance. They are purely about correctness.

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

Assuming that inventory gets merged to mainline before the
restricted_rosters change does I'll update that branch and sort things
out there.

> 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?

Yeah, bug fixes only.

> 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.

Yup, sounds good to me.

Hope this helps.

Cheers,
Derek