[PATCH 1 of 5] histedit: simplify computation of `newchildren` during --continue

[Augie Fackler]

On Oct 2, 2012, at 4:42 AM, pierre-yves.david at logilab.fr wrote:

> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david at ens-lyon.org>
> # Date 1348786765 -7200
> # Node ID 1a8f44e303c07659197acf8f2e77904b70f06872
> # Parent  fa714f3ed2989aff64c267c9935251d9fc4f31ee
> histedit: simplify computation of `newchildren` during --continue
> 
> We are now checking for any changesets between the previous `parentctx` and the
> current working directory parent. If the current working directory parent is
> inconsistent, we abort.
> 
> This change is useful as it simplify the --continue process, easing upcoming

s/simplify/simplifies/


> changes.
> 
> diff --git a/hgext/histedit.py b/hgext/histedit.py
> --- a/hgext/histedit.py
> +++ b/hgext/histedit.py
> @@ -548,25 +548,23 @@ def histedit(ui, repo, *parent, **opts):
>         os.unlink(repo.sjoin('undo'))
> 
> 
> def bootstrapcontinue(ui, repo, parentctx, existing, replacemap, rules,
>                       tmpnodes, created, replaced, opts):
> -    currentparent, wantnull = repo.dirstate.parents()
> -    # existing is the list of revisions initially considered by
> -    # histedit. Here we use it to list new changesets, descendants
> -    # of parentctx without an 'existing' changeset in-between. We
> -    # also have to exclude 'existing' changesets which were
> -    # previously dropped.
> -    descendants = set(c.node() for c in
> -            repo.set('(%d::) - %d', parentctx, parentctx))
> -    notdropped = set(n for n in existing if n in descendants and
> -            (n not in replacemap or replacemap[n] in descendants))
> -    # Discover any nodes the user has added in the interim. We can
> -    # miss changesets which were dropped and recreated the same.
> -    newchildren = list(c.node() for c in repo.set(
> -        'sort(%ln - (%ln or %ln::))', descendants, existing, notdropped))
>     action, currentnode = rules.pop(0)
> +    # is there any new commit between the expected parent and "."
> +    # XXX does not take non linear new change in account (but previous
> +    # XXX implementation didn't used them anyway

I'd still like this reworked into an abort since we're going to (arguably) be broken if the user does this. I'd rather be overly defensive than let someone start depending on weird broken behavior.

Actually, looking at the code further, it looks like you are considering that case? In that case, rework the comment thus:
"""We currently can't handle nonlinear new changes. This isn't a regression, but defend against it since it could screw up obsolete markers."""

(I normally use XXX as a marker for something that's really wrong and shouldn't be finalized as stated.)

> +    newchildren = [c.node() for c in repo.set('(%d::.)', parentctx)]
> +    if not newchildren:
> +        # `parentctxnode` should match but no result. This means that
> +        # currentnode is not a descendant from parentctxnode.
> +        msg = _('working directory parent is not a descendant of %s')
> +        hint = _('update to proper revision and run "hg histedit '
> +                 '--continue" again')

Please describe for the user what a "proper revision" is. "A descendant of %s" seems reasonable to me.

> +        raise util.Abort(msg % parentctx, hint=hint)
> +    newchildren.pop(0)  # remove parentctxnode
>     if action in ('f', 'fold'):
>         tmpnodes.extend(newchildren)
>     else:
>         created.extend(newchildren)
> 
> diff --git a/tests/test-histedit-edit.t b/tests/test-histedit-edit.t
> --- a/tests/test-histedit-edit.t
> +++ b/tests/test-histedit-edit.t
> @@ -64,10 +64,23 @@ edit the history
>   $ HGEDITOR="cat \"$EDITED\" > " hg histedit 177f92b77385 2>&1 | fixbundle
>   0 files updated, 0 files merged, 2 files removed, 0 files unresolved
>   abort: Make changes as needed, you may commit or record as needed now.
>   When you are finished, run hg histedit --continue to resume.
> 
> +Go at a random point and try to continue
> +
> +  $ hg id -n
> +  3+
> +  $ hg up 0
> +  0 files updated, 0 files merged, 3 files removed, 0 files unresolved
> +  $ HGEDITOR='echo foobaz > ' hg histedit --continue
> +  abort: working directory parent is not a descendant of 055a42cdd887
> +  (update to proper revision and run "hg histedit --continue" again)
> +  [255]
> +  $ hg up 3
> +  3 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +
> commit, then edit the revision
>   $ hg ci -m 'wat'
>   created new head
>   $ echo a > e
>   $ HGEDITOR='echo foobaz > ' hg histedit --continue 2>&1 | fixbundle
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel