Re: [Monotone-devel] new bisect command

[Thomas Keller]

Am 01.12.09 06:49, schrieb Derek Scherger:
> On Tue, Nov 24, 2009 at 12:59 AM, Thomas Keller <address@hidden
> <mailto:address@hidden>> wrote:

At first sorry for answering this late. Too much attention withdrawn on
other things lately...

>     Am 24.11.2009 06:57, schrieb Derek Scherger:
>     > On Fri, Nov 20, 2009 at 4:01 PM, Thomas Keller
>     <address@hidden <mailto:address@hidden>> wrote:
>     >
>     >> 1) If an update to a revision failed, f.e. because the addition /
>     >> removal of a node is blocked because of unversioned files, it
>     tells me
>     >> to re-run the process with --move-conflicting-paths - however what
>     >> should be re-run here is only the update, because the bisect
>     status has
>     >> already been updated.
> 
>  
> This turns out to be quite simple so I've allowed for
> --move-conflicting-paths in 'bisect good/bad/skip' commands, which may
> all update the workspace. I've also added a 'bisect update' command to
> be used if you notice later that status is warning about the workspace
> being at the wrong revision. More on this below.

Ok, cool.

>     I haven't looked at the implementation, but if you say this is a common
>     code path, then yes, we should change it in mainline.
> 
>     I'm not particularily fond to make every single piece of output monotone
>     gives configurable via a hook. If there's consent to reorder the output
>     of describe_revision, then we should probably just do so. Otherwise we
>     end up with a big chunk of hooks which change the output somehow which
>     nobody quite follows and which may just slow down the output (I don't
>     know how much of a time penalty there is to call into lua, but there
>     certainly is one).
> 
> 
> Agreed. I haven't made any changes here yet, but the describe_revision
> function should probably be updated to output localized dates. At the
> moment the only thing that localizes dates is the log command.

As this change can be main directly on nvm, it shouldn't affect your branch.

>     >  4) If I'm in the mid of a bisecting operation and (accidentially)
>     call
>     >
>     >> mtn up, I'm updated back to the head revision (if that is possible at
>     >> all) of the branch I'm bisecting. I can easily go back to the last
>     >> revision to test by simply reissuing mtn good or mtn bad on an
>     already
>     >> marked revision (I tested that - cool!), but somehow I wished mtn
>     bisect
>     >> status would just tell me the revision which is currently tested so I
>     >> could also update my workspace revision by hand... (this would
>     also be
>     >> handy for problem 1) when the update to a certain revision fails for
>     >> some reason)
> 
> 
> I've changed 'bisect status' to warn if the workspace is not at the
> correct revision for the current bisection and also added a 'bisect
> update' command that will update the workspace back to the next revision
> selected for bisection. The 'bisect update' command also allows for the
> --move-conflicting-paths option so it can be used to deal with a
> previously failed good/bad/skip update.

Ok, that handles the problem very well. Thank you!

>  > The simplest approach I could think of is to at least note in the output
> 
>     >> of mtn status that the parent revision is not in the same branch
>     as what
>     >> is noted in _MTN/options for the workspace. This might also be
>     handy for
>     >> people screwing around with _MTN/options to branch-off some trunk
>     for a
>     >> feature branch (the "long awaited" (tm) mtn branch command should fix
>     >> that :) and I think its reasonable to give this information there
>     >> because in my opinion mtn status is the most-used command _before_ a
>     >> commit happens.
> 
> 
> The normal status command now outputs the current revision in addition
> to the current branch and will also report the old and new branches in
> the cases where the current workspace branch does not match one of the
> parent revision's branches, regardless of whether a bisection is in
> progress or not. For example:
> 
> $ ./mtn st
> Current revision: 902eb1ca6aeb7de6d6601c20b9de735d290e3613
> Old branch: net.venge.monotone.bisect
> New branch: net.venge.monotone.bisect.asdf
> Changes against parent feb303ed0811833626f532c189e79ca1324cf45a
>   no changes
> 
> Here, the old branch (on the parent revision) is
> 'net.venge.monotone.bisect'. The new branch (from _MTN/options) will be
> 'net.venge.monotone.bisect.asdf' if revisions are committed from this
> workspace.

Very cool!

>     I wasn't sure at first how I as a user was supposed to use bisection
>     (didn't used that in git or any other vcs until now), so my personal use
>     case was at first "I found a bug, but I don't know exactly where it
>     happened. Ok, I enable debug output, start bisecting and look at the
>     output...", which obviously won't work if enabling the debug output
>     requires a code change. I learned however that bisecting is even more a
>     perfect tool for automated bug finding and that my approach might also
>     be honored with a simple patch(8) call...
> 
> 
> I've been doing quite a bit of bisecting lately, unfortunately with
> subversion, which has no native support and seems to fail to update my
> workspace about 50% of the time, even when there are no pending changes.
> This rekindled my original motivation, which was to track down some bug
> we had around the time I started the bisect branch. Even in that very
> early stage it helped me find the problem I was looking for.

Again, as I'm not used to this kind of bug finding it seems to be
perfectly ok if it works for you and others. So this issue shouldn't
block the landing either.

> One thing I haven't yet done is to deal with the case where the good/bad
> revisions are unrelated and don't constrain the revision graph to some
> subset, mainly because I'm not sure how.

Well, my blunt approach here would have been to use
database::get_common_ancestors() and look if the returned list is empty.
If yes, the revision should be part of another tree and thus not be
considered in the bisection, no?

> I haven't thought much about
> this yet though so maybe there's a simple solution. Ideally I'd like to
> get rid of the current assumption that 'good' revisions are ancestors of
> 'bad' revisions which I think is a related problem. Currently the bisect
> good/bad/skip commands allow several revisions to be specified but you
> are also able to specify several bad revisions in successive commands
> before any good revisions have been specified (or good before bad). I'm
> not sure how we would ensure that these initial sets of good and bad
> revisions formed some sensible boundary around a subset of the revision
> graph other than to run the bisection algorithm and see what remains.

If we ensure that the revisions are all in the same tree like above, we
could probably simply compare the revision heights, i.e. if we recorded
a bad revision with revision height X and another bad revision with
revision height > X should be recorded, than the latter should not be
considered because the bug seem to have been introduced in a revision
with height <= X, right? The same is true for good revisions, just that
we should not record any with a height smaller than the last good
revision we already have been recorded.

But actually I'm not sure if this is also the correct thing for
non-linear development lines, in which case we'd falsely exclude
good/bad revisions we actually need to find the bug. Unfortunately I'm
too brain dead at the moment to think about a graph which would reveal
such a misbehaviour.

Thomas.

-- 
GPG-Key 0x160D1092 | address@hidden | http://thomaskeller.biz
Please note that according to the EU law on data retention, information
on every electronic information exchange might be retained for a period
of six months or longer: http://www.vorratsdatenspeicherung.de/?lang=en

signature.asc
Description: OpenPGP digital signature