Patchwork [04,of,11,RFC] histedit: add execute function (issue4036)

login
register
mail settings
Submitter Olle Lundberg
Date March 5, 2014, 4:21 p.m.
Message ID <1856c7395b2b4e748719.1394036510@SE-C02KQ0DADR55>
Download mbox | patch
Permalink /patch/3863/
State Changes Requested
Headers show

Comments

Olle Lundberg - March 5, 2014, 4:21 p.m.
# HG changeset patch
# User Olle Lundberg <geek@nerd.sh>
# Date 1393978358 -3600
#      Wed Mar 05 01:12:38 2014 +0100
# Node ID 1856c7395b2b4e748719c0e97ac0b181253c376b
# Parent  125d473d0d3982c8a01c94acbcec350522d9555e
histedit: add execute function (issue4036)

The basic contract is that it receives a clean working copy and
is expected to leave a clean working copy if it exits 0.
If either the command leaves the working copy dirty, or it exits non-0,
histedit aborts.
David Soria Parra - March 5, 2014, 6 p.m.
Olle Lundberg <olle.lundberg@gmail.com> writes:
>  
> +def execute(ui, repo, ctx, cmd, opts):
> +    hg.update(repo, ctx.node())
> +    rc = util.system(cmd)

I think it would be nice to populate a HGREVISION and HGNODE environment
variable at this point.
Olle Lundberg - March 5, 2014, 6:51 p.m.
On Wed, Mar 5, 2014 at 7:00 PM, David Soria Parra <dsp@experimentalworks.net
> wrote:

> Olle Lundberg <olle.lundberg@gmail.com> writes:
> >
> > +def execute(ui, repo, ctx, cmd, opts):
> > +    hg.update(repo, ctx.node())
> > +    rc = util.system(cmd)
>
> I think it would be nice to populate a HGREVISION and HGNODE environment
> variable at this point.
>

Ok. Will add that.

The issue with running commands from within mercurial is that mercurial
holds an exclusive lock over the whole repo. Which means that a tool can
not do any commits of their own. They can just leave the wd in a dirty
state and the user have to commit and --continue by herself.


> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>
Olle Lundberg - March 5, 2014, 8:11 p.m.
On Wed, Mar 5, 2014 at 7:56 PM, David Soria Parra <dsp@experimentalworks.net
> wrote:

> Olle <olle.lundberg@gmail.com> writes:
>
> > On Wed, Mar 5, 2014 at 7:00 PM, David Soria Parra
> > <dsp@experimentalworks.net> wrote:
> >
> >     Olle Lundberg <olle.lundberg@gmail.com> writes:
> >     >
> >     > +def execute(ui, repo, ctx, cmd, opts):
> >     > + hg.update(repo, ctx.node())
> >     > + rc = util.system(cmd)
> >
> >
> >     I think it would be nice to populate a HGREVISION and HGNODE
> >     environment
> >     variable at this point.
> >
> > Ok. Will add that.
> >
> > The issue with running commands from within mercurial is that
> > mercurial holds an exclusive lock over the whole repo. Which means
> > that a tool can not do any commits of their own. They can just leave
> > the wd in a dirty state and the user have to commit and --continue by
> > herself.
> >
>
> I think you want to drop locks before util.system and reaquire them
> afterwards. Reaquiring locks will also properly invalidate dirstate
> and manifest.
>

Re-sending your reply to the list.

Patch

diff --git a/hgext/histedit.py b/hgext/histedit.py
--- a/hgext/histedit.py
+++ b/hgext/histedit.py
@@ -333,10 +333,23 @@ 
     applychanges(ui, repo, oldctx, opts)
     raise error.InterventionRequired(
         _('Make changes as needed, you may commit or record as needed now.\n'
           'When you are finished, run hg histedit --continue to resume.'))
 
+def execute(ui, repo, ctx, cmd, opts):
+    hg.update(repo, ctx.node())
+    rc = util.system(cmd)
+    if rc != 0:
+        raise error.InterventionRequired(
+            _('Command exited with %i. Fix up the change and run '
+            'hg histedit --continue') % rc)
+    if util.any(repo.status()[:4]):
+        raise error.InterventionRequired(
+            _('Working copy dirty, please check the files listed above.\n'
+              'When you are finished, run hg histedit --continue to resume.'))
+    return ctx, []
+
 def fold(ui, repo, ctx, ha, opts):
     oldctx = repo[ha]
     hg.update(repo, ctx.node())
     stats = applychanges(ui, repo, oldctx, opts)
     if stats and stats[3] > 0: