[PATCH] mq: do not strip revision if applied mq is descendant (issue1881)

Mads Kiilerich mads at kiilerich.com
Mon Jul 5 06:07:14 CDT 2010


  Vishakh H wrote, On 07/05/2010 12:11 PM:
> # HG changeset patch
> # User Vishakh H<vsh426 at gmail.com>
> # Date 1278268639 -19800
> # Branch stable
> # Node ID fdd2d7955d697155a0449952cff5079631e2c968
> # Parent  b2468fb58b2bbbe5711119437449f253fda68318
> mq: do not strip revision if applied mq is descendant (issue1881)
>
> check if there are applied patches and whether strip rev is ancestor of
> qparent or is one of applied patches. two different messages are for the
> benefit of the user.

Thanks!

That would prevent the stupid user from this specific problem, but do we 
want such safety guards in the brutal and efficient strip command?

I would rather see more robust handling of inconsistencies.

qapplied could check that the patches really was applied with the hash 
stored in the status file - and in the right order.

The error messages could be clearer - for example by telling which patch 
caused the problem and give a hint how the problem can be resolved.

What is the recommended way to resolve such an situation? Should we tell 
about the status file and expect the user to edit it manually? Or should 
we give qpop (or some other command) a way to handle the sitation?

Both
"abort: trying to pop unknown node a1cde8e42a91" and
"mq status file refers to unknown node a1cde8e42a91" (which perhaps 
should be an abort)
could perhaps be
"abort: patch XXX in mq status file refers to unknown node a1cde8e42a91" or
"abort: mq status file refers to unknown node a1cde8e42a91 for patch XXX" or
"abort: patch XXX not found with node a1cde8e42a91"

> diff --git a/hgext/mq.py b/hgext/mq.py
> --- a/hgext/mq.py
> +++ b/hgext/mq.py
> @@ -2404,8 +2404,15 @@
>           backup = 'none'
>
>       rev = repo.lookup(rev)
> +    cl = repo.changelog
> +    q = repo.mq
> +    if q.applied:

Introducing a variable and only using it once is a bit overkill.

> +        if rev == cl.ancestor(repo.lookup('qparent'), rev):
> +            raise util.Abort(_("cannot strip revision when applied "
> +                "patches are descendants"))
> +        if rev == cl.ancestor(repo.lookup('qtip'), rev):
> +            raise util.Abort(_("cannot strip applied patch"))
>       p = repo.dirstate.parents()
> -    cl = repo.changelog
>       update = True
>       if p[0] == nullid:
>           update = False
> diff --git a/tests/test-strip-applied b/tests/test-strip-applied
> new file mode 100755
> --- /dev/null
> +++ b/tests/test-strip-applied
> @@ -0,0 +1,30 @@
> +#!/bin/sh
> +#do not strip revision if it or its descendants are applied mq patches

Wouldn't it be possible to get the same test coverage by just adding a 
couple of lines to an existing test?

/Mads


More information about the Mercurial-devel mailing list