Stephen Leake wrote: > Derek Scherger <[EMAIL PROTECTED]> 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/[email protected]/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 _______________________________________________ Monotone-devel mailing list [email protected] http://lists.nongnu.org/mailman/listinfo/monotone-devel
