Patchwork [1,of,1,STABLE] dirstate: don't write repo.currenttransaction to repo.dirstate if repo is None (issue4983)

login
register
mail settings
Submitter Sietse Brouwer
Date Dec. 3, 2015, 1:22 a.m.
Message ID <CAF=dkzx8CBYXVve9r-9qgqZBWFrmpdxw_VrYc9bnyOmVDOWfEg@mail.gmail.com>
Download mbox | patch
Permalink /patch/11778/
State Accepted
Headers show

Comments

Sietse Brouwer - Dec. 3, 2015, 1:22 a.m.
This fixes issue4983 [1]: post-init and post-clone hooks always crash
(after hg init / hg clone)

I realise that Matt's e-mail backlog is terrifying right now [2]; but
the bug is a regression, and the fix is a simple one, so I thought I'd
send it in anyway.

@FUJIWARA Katsunori: this patch touches dirstate code. Does it look
all right to you?

Kind regards,

Sietse
Sietse Brouwer

[1] http://bz.mercurial-scm.org/show_bug.cgi?id=4983
[2] https://www.mercurial-scm.org/wiki/mpm/flow


# HG changeset patch
# User Sietse Brouwer <sbbrouwer@gmail.com>
# Date 1449103101 -3600
#      Thu Dec 03 01:38:21 2015 +0100
# Branch stable
# Node ID b5b96dcd27a0e89c0161f5863f4dc58c06a9519c
# Parent  7e1fac6c0a9ce6afd3edeed5e47bcca343155d8a
dirstate: don't write repo.currenttransaction to repo.dirstate if repo
is None (issue4983)

Some hooks, such as post-init and post-clone, do not get a repo parameter in
their environment. If there is no repo, there is no repo.currenttransaction();
attempting to retrieve it anyway was causing crashes. Now currenttransaction is
only retrieved and written if the repo is not None.

+  $ cat << EOF >> hgrc-with-post-init-hook
+  > [hooks]
+  > post-init = printenv.py post-init
+  > EOF
+  $ HGRCPATH=hgrc-with-post-init-hook hg init to
+  post-init hook: HG_ARGS=init to HG_OPTS={'insecure': None,
'remotecmd': '', 'ssh': ''} HG_PATS=['to'] HG_RESULT=0
+
 new commits must be visible in pretxnchangegroup (issue3428)

-  $ cd ..
-  $ hg init to
   $ echo '[hooks]' >> to/.hg/hgrc
   $ echo 'prechangegroup = hg --traceback tip' >> to/.hg/hgrc
   $ echo 'pretxnchangegroup = hg --traceback tip' >> to/.hg/hgrc
Pierre-Yves David - Dec. 3, 2015, 9:20 a.m.
On 12/02/2015 05:22 PM, Sietse Brouwer wrote:
> This fixes issue4983 [1]: post-init and post-clone hooks always crash
> (after hg init / hg clone)

Thanks, you patch had some bad damage from the your email client. 
Consider using the patchbomb extensions or the pushgate to send a patch 
next week.

I've pushed this to the clowncopter.

> I realise that Matt's e-mail backlog is terrifying right now [2]; but
> the bug is a regression, and the fix is a simple one, so I thought I'd
> send it in anyway.

Augie is the goaltender this week, so the important backlog is the 
patchwork one.

The regression is indeed pretty bad. (and the scheduled bugfixe version 
was 2 day ago…)
Sietse Brouwer - Dec. 3, 2015, 10:10 a.m.
> Thanks, you patch had some bad damage from the your email client. Consider
> using the patchbomb extensions or the pushgate to send a patch next week.

Noted. I thought sending as plain text would keep Gmail from mangling
it, but nooo. Have updated the wiki to make that clear. Excited to try
out the pushgate in future. Thank you very much for the repair work.

https://www.mercurial-scm.org/wiki/ContributingChanges#Experimental_submission_via_push_gate

>> Matt's e-mail backlog is terrifying

> Augie is the goaltender this week, so the important backlog is the patchwork one.

That's good news. Pity about missing 3.6.2, yes... ah well, such is life.

Cheers,
Sietse

On 3 December 2015 at 10:20, Pierre-Yves David
<pierre-yves.david@ens-lyon.org> wrote:
>
>
> On 12/02/2015 05:22 PM, Sietse Brouwer wrote:
>>
>> This fixes issue4983 [1]: post-init and post-clone hooks always crash
>> (after hg init / hg clone)
>
>
> Thanks, you patch had some bad damage from the your email client. Consider
> using the patchbomb extensions or the pushgate to send a patch next week.
>
> I've pushed this to the clowncopter.
>
>> I realise that Matt's e-mail backlog is terrifying right now [2]; but
>> the bug is a regression, and the fix is a simple one, so I thought I'd
>> send it in anyway.
>
>
> Augie is the goaltender this week, so the important backlog is the patchwork
> one.
>
> The regression is indeed pretty bad. (and the scheduled bugfixe version was
> 2 day ago…)
>
> --
> Pierre-Yves David
Pierre-Yves David - Dec. 3, 2015, 10:14 a.m.
On 12/03/2015 02:10 AM, Sietse Brouwer wrote:
>> Thanks, you patch had some bad damage from the your email client. Consider
>> using the patchbomb extensions or the pushgate to send a patch next week.
>
> Noted. I thought sending as plain text would keep Gmail from mangling
> it, but nooo. Have updated the wiki to make that clear. Excited to try
> out the pushgate in future. Thank you very much for the repair work.
>
> https://www.mercurial-scm.org/wiki/ContributingChanges#Experimental_submission_via_push_gate
>
>>> Matt's e-mail backlog is terrifying
>
>> Augie is the goaltender this week, so the important backlog is the patchwork one.
>
> That's good news. Pity about missing 3.6.2, yes... ah well, such is life.

That said, you still caught an exception and are eligible for a 
Mercurial T-shirt. Can you send me the following information in private,

T-Shirt size (Male or Female too),
Name,
Full address,
Phone number

I'll ship you one.
Sietse Brouwer - Dec. 4, 2015, 8:54 a.m.
> That said, you still caught an exception and are eligible for a Mercurial T-shirt. Can you send me the following information in private,

Private e-mail sent. I think it's very cool that this project (a) does
not just deal in online activities, and (b) has so few regressions
that you can generally afford to send out T-shirts for them. That puts
Mercurial's reliability somewhere in the same league as TeX, which is
amazing.

Cheers,
Sietse
--
Sietse Brouwer -- sbbrouwer@gmail.com -- +31 6 13.456.848
Korte Langestraat 2 -- 2312 SK Leiden -- the Netherlands


On 3 December 2015 at 11:14, Pierre-Yves David
<pierre-yves.david@ens-lyon.org> wrote:
>
>
> On 12/03/2015 02:10 AM, Sietse Brouwer wrote:
>>>
>>> Thanks, you patch had some bad damage from the your email client.
>>> Consider
>>> using the patchbomb extensions or the pushgate to send a patch next week.
>>
>>
>> Noted. I thought sending as plain text would keep Gmail from mangling
>> it, but nooo. Have updated the wiki to make that clear. Excited to try
>> out the pushgate in future. Thank you very much for the repair work.
>>
>>
>> https://www.mercurial-scm.org/wiki/ContributingChanges#Experimental_submission_via_push_gate
>>
>>>> Matt's e-mail backlog is terrifying
>>
>>
>>> Augie is the goaltender this week, so the important backlog is the
>>> patchwork one.
>>
>>
>> That's good news. Pity about missing 3.6.2, yes... ah well, such is life.
>
>
> That said, you still caught an exception and are eligible for a Mercurial
> T-shirt. Can you send me the following information in private,
>
> T-Shirt size (Male or Female too),
> Name,
> Full address,
> Phone number
>
> I'll ship you one.
>
> --
> Pierre-Yves David
Pierre-Yves David - Dec. 4, 2015, 9:04 a.m.
On 12/04/2015 12:54 AM, Sietse Brouwer wrote:
>> That said, you still caught an exception and are eligible for a Mercurial T-shirt. Can you send me the following information in private,
>
> Private e-mail sent. I think it's very cool that this project (a) does
> not just deal in online activities, and (b) has so few regressions
> that you can generally afford to send out T-shirts for them. That puts
> Mercurial's reliability somewhere in the same league as TeX, which is
> amazing.

Even if we try hard, we are unfortunately not that regression free. This 
is why our T-Shirt program does not plan for exponential rewards.

Patch

diff --git a/mercurial/hook.py b/mercurial/hook.py
--- a/mercurial/hook.py
+++ b/mercurial/hook.py
@@ -120,10 +120,11 @@ 
     env = {}

     # make in-memory changes visible to external process
-    tr = repo.currenttransaction()
-    repo.dirstate.write(tr)
-    if tr and tr.writepending():
-        env['HG_PENDING'] = repo.root
+    if repo is not None:
+        tr = repo.currenttransaction()
+        repo.dirstate.write(tr)
+        if tr and tr.writepending():
+            env['HG_PENDING'] = repo.root

     for k, v in args.iteritems():
         if callable(v):
diff --git a/tests/test-hook.t b/tests/test-hook.t
--- a/tests/test-hook.t
+++ b/tests/test-hook.t
@@ -681,10 +681,19 @@ 
...skipping...
-    tr = repo.currenttransaction()
-    repo.dirstate.write(tr)
-    if tr and tr.writepending():
-        env['HG_PENDING'] = repo.root
+    if repo is not None:
+        tr = repo.currenttransaction()
+        repo.dirstate.write(tr)
+        if tr and tr.writepending():
+            env['HG_PENDING'] = repo.root

     for k, v in args.iteritems():
         if callable(v):
diff --git a/tests/test-hook.t b/tests/test-hook.t
--- a/tests/test-hook.t
+++ b/tests/test-hook.t
@@ -681,10 +681,19 @@ 
   $ hg tag -f foo
   ['a', 'foo', 'tip']

+post-init hooks must not crash (issue4983)
+This also creates the `to` repo for the next test block.
+
+  $ cd ..