From patchwork Thu Nov  5 15:33:06 2015
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: [V4] histedit: add support for exec/x command to histedit (issue4036)
From: Yuya Nishihara <yuya@tcha.org>
X-Patchwork-Id: 11298
Message-Id: <20151106003306.34f60deab5c8afb945d61d25@tcha.org>
To: liscju <piotr.listkiewicz@gmail.com>
Cc: mercurial-devel@selenic.com
Date: Fri, 6 Nov 2015 00:33:06 +0900

On Wed, 28 Oct 2015 09:15:58 +0100, liscju wrote:
> # HG changeset patch
> # User liscju <piotr.listkiewicz@gmail.com>
> # Date 1442157106 -7200
> #      Sun Sep 13 17:11:46 2015 +0200
> # Node ID 90176aa09b5619526cd8b25b23f7a3649e27f0df
> # Parent  58a309e9cf80d74d96e8c56cb95be20a4b130092
> histedit: add support for exec/x command to histedit (issue4036)

> +def _isdirtywc(repo):
> +    s = repo.status()
> +    return s.modified or s.added or s.removed or s.deleted

We might want to use repo[None].dirty(...) (= workingctx.dirty()).

> +class execute(histeditaction):
> +    def __init__(self, state, cmd):
> +        self.state = state
> +        self.cmd = cmd
> +
> +    @classmethod
> +    def fromrule(cls, state, cmd):
> +        return cls(state, cmd)
> +
> +    def run(self):
> +        repo = self.state.repo
> +        node = self.state.parentctxnode
> +
> +        hg.update(repo, repo[node].parents()[0])
> +        applychanges(repo.ui, repo, repo[node], {})
> +        self._commit()
> +
> +        lockheld = self.state.lock.releaseall()
> +        wlockheld = self.state.wlock.releaseall()
> +        try:
> +            return_code = repo.ui.system(self.cmd, cwd=repo.root)
> +        except OSError:
> +            raise error.InterventionRequired(
> +                self._getinterventionmsg(
> +                    _('Execution of "%s" failed.') % self.cmd))

Perhaps this isn't an InterventionRequired error (code=1) because the specified
command is wrong?

> +        finally:
> +            self.state.wlock = repo.wlock(times=wlockheld)
> +            self.state.lock = repo.lock(times=lockheld)
> +            repo.invalidateall()

If I understand it, new wlock (and lock) should be the same object to old one.
I think that's the reason why localrepo keeps weakref to the active lock.
(but I might be wrong.)

But here, wlock (and lock) seem to be recreated. (tested by the following
assertion.)


> +    def _getinterventionmsg(self, errormsg):
> +        return _(errormsg + "\n" +
> +              'Make changes as needed, you may commit or record as needed '
> +              'now.\nWhen you are finished, run hg histedit --continue to '
> +              'resume.')

It should be  errormsg + "\n" + _(...).

diff --git a/hgext/histedit.py b/hgext/histedit.py
--- a/hgext/histedit.py
+++ b/hgext/histedit.py
@@ -541,9 +541,11 @@ class execute(histeditaction):
                 self._getinterventionmsg(
                     _('Execution of "%s" failed.') % self.cmd))
         finally:
+            oldwlock = self.state.wlock
             self.state.wlock = repo.wlock(times=wlockheld)
             self.state.lock = repo.lock(times=lockheld)
             repo.invalidateall()
+            assert oldwlock is self.state.wlock
 
         if return_code == 0:
             if _isdirtywc(repo):