[PATCH] commit: add option to amend the working dir parent

Idan Kamara idankk86 at gmail.com
Tue Apr 17 18:21:49 CDT 2012


On Wed, Apr 18, 2012 at 2:14 AM, Augie Fackler <raf at durin42.com> wrote:

>
> On Apr 17, 2012, at 6:02 PM, Idan Kamara wrote:
>
> > On Wed, Apr 18, 2012 at 1:30 AM, Idan Kamara <idankk86 at gmail.com> wrote:
> >
> >> # HG changeset patch
> >> # User Idan Kamara <idankk86 at gmail.com>
> >> # Date 1334701216 -10800
> >> # Node ID 55982f62651f1974fcd91197f1c4801cc98a48f2
> >> # Parent  91196ebcaeed06217427e08b879e761dc79472c7
> >> commit: add option to amend the working dir parent
> >>
> >
> > [snip]
> >
> >
> >>
> >> +def amend(ui, repo, commitfunc, old, extra, pats, opts):
> >> +    ui.note(_('amending changeset %s\n') % old)
> >> +    base = old.p1()
> >> +
> >> +    wlock = repo.wlock()
> >> +    try:
> >> +        # Fix up dirstate for copies and renames
> >> +        duplicatecopies(repo, None, base.node())
> >>
> >
> > For those who are curious the problem was that this function
> > call is mutating the dirstate and apparently without a wlock
> > it caused some inconsistent results.
> >
> > It took me quite a while to spot and I think we should protect
> > ourselves from this. I wrote a tiny extension that wraps some
> > functions in dirstate such as setparents, setbranch, etc. and
> > already saw that graft is missing a wlock. There are probably
> > other casualties around the code.
> >
> > But maybe a more robust solution is a decorator
> > @ensurelock(lock=True, wlock=True), that will be used on
> > these functions that require a lock/wlock. Then when the
> > test suite is running it can translate to a test for lock existence,
> > otherwise it will do nothing and return the original function with
> > no performance hit.
> >
> > Now that I think about it though, it might be technically
> > impossible because of the time decorators are invoked.
>
> We enable it with an environment variable? Just have the decorator
> evaluate away except when HG_REQUIRE_LOCK_PEDANTRY or something is set?


That's one possibility but has a small one time cost during module import
(or whenever decorators are evaluated).

I'll try and think if there's a zero cost solution.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20120418/2572152f/attachment.html>


More information about the Mercurial-devel mailing list