Patchwork [1,of,4,RFC] hook: have a generic hook for transaction opening

login
register
mail settings
Submitter Pierre-Yves David
Date March 10, 2015, 8:07 a.m.
Message ID <82dea52f27319896a8d3.1425974833@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/7960/
State Superseded
Commit e9ede9b4c2f846bedc2fa23c0a5a0be14fe27d64
Headers show

Comments

Pierre-Yves David - March 10, 2015, 8:07 a.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1418264389 28800
#      Wed Dec 10 18:19:49 2014 -0800
# Node ID 82dea52f27319896a8d33a0b805f439107a94845
# Parent  4ef4e3c3c00693868ba428e21ac092e559e4fcea
hook: have a generic hook for transaction opening

We are adding generic hooking for all transaction. We do not really have any
useful information to including when opening the transaction but this is a
useful time to allow hook anyway.
Pierre-Yves David - March 10, 2015, 8:14 a.m.
On 03/10/2015 01:07 AM, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1418264389 28800
> #      Wed Dec 10 18:19:49 2014 -0800
> # Node ID 82dea52f27319896a8d33a0b805f439107a94845
> # Parent  4ef4e3c3c00693868ba428e21ac092e559e4fcea
> hook: have a generic hook for transaction opening

This is RFC because I'm unhappy with the current names, but they are 
going to be bikesheded anyway. So I would like to get the topic moving 
and code is a good way to get that.

The name in this series are:

- txnopen (can abort)
- txnclosing (can abort)
- txnclosed

The name that would fit existing hooks best will probably be:

- pretxnopen (can abort)
- pretxnclose (can abort)
- txnclose

However, I always struggle to find out which of the hook can actually 
abort a transaction. so I like the "closed" version that settle this down.

So current optimal idea is probably:

- pretxnopen
- pretxnclose
- txnclosed

But I've also to admit that "txn" is not the most sexy thing we have 
ever done.

Though?
Augie Fackler - March 10, 2015, 1:02 p.m.
On Tue, Mar 10, 2015 at 01:07:13AM -0700, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1418264389 28800
> #      Wed Dec 10 18:19:49 2014 -0800
> # Node ID 82dea52f27319896a8d33a0b805f439107a94845
> # Parent  4ef4e3c3c00693868ba428e21ac092e559e4fcea
> hook: have a generic hook for transaction opening
>
> We are adding generic hooking for all transaction. We do not really have any
> useful information to including when opening the transaction but this is a
> useful time to allow hook anyway.

If we can't provide useful information, then why bother? Do you have a
practical use case?

>
> diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
> --- a/mercurial/help/config.txt
> +++ b/mercurial/help/config.txt
> @@ -806,10 +806,14 @@ variables it is passed are listed with n
>    Run before creating a tag. Exit status 0 allows the tag to be
>    created. Non-zero status will cause the tag to fail. ID of
>    changeset to tag is in ``$HG_NODE``. Name of tag is in ``$HG_TAG``. Tag is
>    local if ``$HG_LOCAL=1``, in repository if ``$HG_LOCAL=0``.
>
> +``txnopen``
> +  Run before any new repository transaction is open. Reason for the transaction
> +  opening will be in ``$HG_TXNNAME``. This hook can abort transaction opening.
> +
>  ``pretxnchangegroup``
>    Run after a changegroup has been added via push, pull or unbundle,
>    but before the transaction has been committed. Changegroup is
>    visible to hook program. This lets you validate incoming changes
>    before accepting them. Passed the ID of the first new changeset in
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -906,10 +906,12 @@ class localrepository(object):
>          if self.svfs.exists("journal"):
>              raise error.RepoError(
>                  _("abandoned transaction found"),
>                  hint=_("run 'hg recover' to clean up transaction"))
>
> +        self.hook('txnopen', throw=True, txnname=desc)
> +
>          self._writejournal(desc)
>          renames = [(vfs, x, undoname(x)) for vfs, x in self._journalfiles()]
>          rp = report and report or self.ui.warn
>          vfsmap = {'plain': self.vfs} # root of .hg/
>          tr = transaction.transaction(rp, self.svfs, vfsmap,
> diff --git a/tests/test-hook.t b/tests/test-hook.t
> --- a/tests/test-hook.t
> +++ b/tests/test-hook.t
> @@ -10,15 +10,17 @@ commit hooks can see env vars
>    > pretxncommit = sh -c "HG_LOCAL= HG_TAG= python \"$TESTDIR/printenv.py\" pretxncommit"
>    > pretxncommit.tip = hg -q tip
>    > pre-identify = python "$TESTDIR/printenv.py" pre-identify 1
>    > pre-cat = python "$TESTDIR/printenv.py" pre-cat
>    > post-cat = python "$TESTDIR/printenv.py" post-cat
> +  > txnopen = sh -c "HG_LOCAL= HG_TAG= python \"$TESTDIR/printenv.py\" txnopen"
>    > EOF
>    $ echo a > a
>    $ hg add a
>    $ hg commit -m a
>    precommit hook: HG_PARENT1=0000000000000000000000000000000000000000
> +  txnopen hook: HG_TXNNAME=commit
>    pretxncommit hook: HG_NODE=cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b HG_PARENT1=0000000000000000000000000000000000000000 HG_PENDING=$TESTTMP/a
>    0:cb9a9f314b8b
>    commit hook: HG_NODE=cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b HG_PARENT1=0000000000000000000000000000000000000000
>    commit.b hook: HG_NODE=cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b HG_PARENT1=0000000000000000000000000000000000000000
>
> @@ -40,30 +42,33 @@ pretxncommit and commit hooks can see bo
>
>    $ cd ../a
>    $ echo b >> a
>    $ hg commit -m a1 -d "1 0"
>    precommit hook: HG_PARENT1=cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b
> +  txnopen hook: HG_TXNNAME=commit
>    pretxncommit hook: HG_NODE=ab228980c14deea8b9555d91c9581127383e40fd HG_PARENT1=cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b HG_PENDING=$TESTTMP/a
>    1:ab228980c14d
>    commit hook: HG_NODE=ab228980c14deea8b9555d91c9581127383e40fd HG_PARENT1=cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b
>    commit.b hook: HG_NODE=ab228980c14deea8b9555d91c9581127383e40fd HG_PARENT1=cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b
>    $ hg update -C 0
>    1 files updated, 0 files merged, 0 files removed, 0 files unresolved
>    $ echo b > b
>    $ hg add b
>    $ hg commit -m b -d '1 0'
>    precommit hook: HG_PARENT1=cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b
> +  txnopen hook: HG_TXNNAME=commit
>    pretxncommit hook: HG_NODE=ee9deb46ab31e4cc3310f3cf0c3d668e4d8fffc2 HG_PARENT1=cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b HG_PENDING=$TESTTMP/a
>    2:ee9deb46ab31
>    commit hook: HG_NODE=ee9deb46ab31e4cc3310f3cf0c3d668e4d8fffc2 HG_PARENT1=cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b
>    commit.b hook: HG_NODE=ee9deb46ab31e4cc3310f3cf0c3d668e4d8fffc2 HG_PARENT1=cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b
>    created new head
>    $ hg merge 1
>    1 files updated, 0 files merged, 0 files removed, 0 files unresolved
>    (branch merge, don't forget to commit)
>    $ hg commit -m merge -d '2 0'
>    precommit hook: HG_PARENT1=ee9deb46ab31e4cc3310f3cf0c3d668e4d8fffc2 HG_PARENT2=ab228980c14deea8b9555d91c9581127383e40fd
> +  txnopen hook: HG_TXNNAME=commit
>    pretxncommit hook: HG_NODE=07f3376c1e655977439df2a814e3cc14b27abac2 HG_PARENT1=ee9deb46ab31e4cc3310f3cf0c3d668e4d8fffc2 HG_PARENT2=ab228980c14deea8b9555d91c9581127383e40fd HG_PENDING=$TESTTMP/a
>    3:07f3376c1e65
>    commit hook: HG_NODE=07f3376c1e655977439df2a814e3cc14b27abac2 HG_PARENT1=ee9deb46ab31e4cc3310f3cf0c3d668e4d8fffc2 HG_PARENT2=ab228980c14deea8b9555d91c9581127383e40fd
>    commit.b hook: HG_NODE=07f3376c1e655977439df2a814e3cc14b27abac2 HG_PARENT1=ee9deb46ab31e4cc3310f3cf0c3d668e4d8fffc2 HG_PARENT2=ab228980c14deea8b9555d91c9581127383e40fd
>
> @@ -101,10 +106,11 @@ tag hooks can see env vars
>    > tag = sh -c "HG_PARENT1= HG_PARENT2= python \"$TESTDIR/printenv.py\" tag"
>    > EOF
>    $ hg tag -d '3 0' a
>    pretag hook: HG_LOCAL=0 HG_NODE=07f3376c1e655977439df2a814e3cc14b27abac2 HG_TAG=a
>    precommit hook: HG_PARENT1=07f3376c1e655977439df2a814e3cc14b27abac2
> +  txnopen hook: HG_TXNNAME=commit
>    pretxncommit hook: HG_NODE=539e4b31b6dc99b3cfbaa6b53cbc1c1f9a1e3a10 HG_PARENT1=07f3376c1e655977439df2a814e3cc14b27abac2 HG_PENDING=$TESTTMP/a
>    4:539e4b31b6dc
>    tag hook: HG_LOCAL=0 HG_NODE=07f3376c1e655977439df2a814e3cc14b27abac2 HG_TAG=a
>    commit hook: HG_NODE=539e4b31b6dc99b3cfbaa6b53cbc1c1f9a1e3a10 HG_PARENT1=07f3376c1e655977439df2a814e3cc14b27abac2
>    commit.b hook: HG_NODE=539e4b31b6dc99b3cfbaa6b53cbc1c1f9a1e3a10 HG_PARENT1=07f3376c1e655977439df2a814e3cc14b27abac2
> @@ -135,10 +141,11 @@ more there after
>    $ hg add z
>    $ hg -q tip
>    4:539e4b31b6dc
>    $ hg commit -m 'fail' -d '4 0'
>    precommit hook: HG_PARENT1=539e4b31b6dc99b3cfbaa6b53cbc1c1f9a1e3a10
> +  txnopen hook: HG_TXNNAME=commit
>    pretxncommit hook: HG_NODE=6f611f8018c10e827fee6bd2bc807f937e761567 HG_PARENT1=539e4b31b6dc99b3cfbaa6b53cbc1c1f9a1e3a10 HG_PENDING=$TESTTMP/a
>    5:6f611f8018c1
>    5:6f611f8018c1
>    pretxncommit.forbid hook: HG_NODE=6f611f8018c10e827fee6bd2bc807f937e761567 HG_PARENT1=539e4b31b6dc99b3cfbaa6b53cbc1c1f9a1e3a10 HG_PENDING=$TESTTMP/a
>    transaction abort!
> @@ -196,10 +203,11 @@ pushkey hook
>    $ hg bookmark -r null foo
>    $ hg push -B foo ../a
>    pushing to ../a
>    searching for changes
>    no changes found
> +  txnopen hook: HG_TXNNAME=bookmarks
>    pushkey hook: HG_KEY=foo HG_NAMESPACE=bookmarks HG_NEW=0000000000000000000000000000000000000000 HG_RET=1
>    exporting bookmark foo
>    [1]
>    $ cd ../a
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Augie Fackler - March 10, 2015, 1:04 p.m.
On Tue, Mar 10, 2015 at 01:14:28AM -0700, Pierre-Yves David wrote:
>
>
> On 03/10/2015 01:07 AM, Pierre-Yves David wrote:
> ># HG changeset patch
> ># User Pierre-Yves David <pierre-yves.david@fb.com>
> ># Date 1418264389 28800
> >#      Wed Dec 10 18:19:49 2014 -0800
> ># Node ID 82dea52f27319896a8d33a0b805f439107a94845
> ># Parent  4ef4e3c3c00693868ba428e21ac092e559e4fcea
> >hook: have a generic hook for transaction opening
>
> This is RFC because I'm unhappy with the current names, but they are going
> to be bikesheded anyway. So I would like to get the topic moving and code is
> a good way to get that.
>
> The name in this series are:
>
> - txnopen (can abort)
> - txnclosing (can abort)
> - txnclosed

I have no particular recommendations for better names (sorry), but I'm
not sure what the merit of txnopen is given that we can't provide much
information. A sample use case would help there, I guess.

As for the others, we could probably rework some existing
functionality to be examples. verify-after-push could be done as a
txnclosing, and txnclosed could be used by the notify extension, so I
see merit in both of those.

>
> The name that would fit existing hooks best will probably be:
>
> - pretxnopen (can abort)
> - pretxnclose (can abort)
> - txnclose
>
> However, I always struggle to find out which of the hook can actually abort
> a transaction. so I like the "closed" version that settle this down.
>
> So current optimal idea is probably:
>
> - pretxnopen
> - pretxnclose
> - txnclosed
>
> But I've also to admit that "txn" is not the most sexy thing we have ever
> done.
>
> Though?
>
> --
> Pierre-Yves David
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Pierre-Yves David - March 10, 2015, 4:15 p.m.
On 03/10/2015 06:04 AM, Augie Fackler wrote:
> On Tue, Mar 10, 2015 at 01:14:28AM -0700, Pierre-Yves David wrote:
>>
>>
>> On 03/10/2015 01:07 AM, Pierre-Yves David wrote:
>>> # HG changeset patch
>>> # User Pierre-Yves David <pierre-yves.david@fb.com>
>>> # Date 1418264389 28800
>>> #      Wed Dec 10 18:19:49 2014 -0800
>>> # Node ID 82dea52f27319896a8d33a0b805f439107a94845
>>> # Parent  4ef4e3c3c00693868ba428e21ac092e559e4fcea
>>> hook: have a generic hook for transaction opening
>>
>> This is RFC because I'm unhappy with the current names, but they are going
>> to be bikesheded anyway. So I would like to get the topic moving and code is
>> a good way to get that.
>>
>> The name in this series are:
>>
>> - txnopen (can abort)
>> - txnclosing (can abort)
>> - txnclosed
>
> I have no particular recommendations for better names (sorry), but I'm
> not sure what the merit of txnopen is given that we can't provide much
> information. A sample use case would help there, I guess.

I've no sample usecase for txnopen, but it is very cheap to add and I 
guess some extension//baroque setup will be very happy to have some 
official way to control//record transaction opening.

We could also move it a bit further to right after the transaction have 
been openened (or add a new hooks for that). So that its easy to add 
data in the transaction after it is open.

Maybe we should have a hook for aborted transaction too. But yet again 
I've no specific usecase in mind.

> As for the others, we could probably rework some existing
> functionality to be examples. verify-after-push could be done as a
> txnclosing, and txnclosed could be used by the notify extension, so I
> see merit in both of those.

txnclosing and txnclosed are -must-have-. None of the current hook allow 
to catch a full transaction run with all its data. This is extremely 
annoying for validation or synchronization.
Ryan McElroy - March 11, 2015, 4:17 p.m.
On 3/10/2015 9:15 AM, Pierre-Yves David wrote:
>
>
> On 03/10/2015 06:04 AM, Augie Fackler wrote:
>> On Tue, Mar 10, 2015 at 01:14:28AM -0700, Pierre-Yves David wrote:
>>> This is RFC because I'm unhappy with the current names, but they are 
>>> going
>>> to be bikesheded anyway. So I would like to get the topic moving and 
>>> code is
>>> a good way to get that.
>>>
>>> The name in this series are:
>>>
>>> - txnopen (can abort)
>>> - txnclosing (can abort)
>>> - txnclosed
>>
>> I have no particular recommendations for better names (sorry), but I'm
>> not sure what the merit of txnopen is given that we can't provide much
>> information. A sample use case would help there, I guess.
>
> I've no sample usecase for txnopen, but it is very cheap to add and I 
> guess some extension//baroque setup will be very happy to have some 
> official way to control//record transaction opening.
>
> We could also move it a bit further to right after the transaction 
> have been openened (or add a new hooks for that). So that its easy to 
> add data in the transaction after it is open.
>
> Maybe we should have a hook for aborted transaction too. But yet again 
> I've no specific usecase in mind.
>
>> As for the others, we could probably rework some existing
>> functionality to be examples. verify-after-push could be done as a
>> txnclosing, and txnclosed could be used by the notify extension, so I
>> see merit in both of those.
>
> txnclosing and txnclosed are -must-have-. None of the current hook 
> allow to catch a full transaction run with all its data. This is 
> extremely annoying for validation or synchronization.
>
The only idea I have on naming is "trx" instead of "txn", which is what 
I remember using back in my MySQL days. This is purely bikeshedding, and 
I feel bad about it, but you did ask... sooo...

I prefer the "optimal" version you suggested with trx:

- pretrxopen
- pretrxclose
- trxclosed

That being said, the functionality here matters more than naming, and I 
don't think any of the names proposed so far would be hard to understand 
what is going on.
Pierre-Yves David - March 11, 2015, 9:34 p.m.
On 03/11/2015 09:17 AM, Ryan McElroy wrote:
> On 3/10/2015 9:15 AM, Pierre-Yves David wrote:
>>
>>
>> On 03/10/2015 06:04 AM, Augie Fackler wrote:
>>> On Tue, Mar 10, 2015 at 01:14:28AM -0700, Pierre-Yves David wrote:
>>>> This is RFC because I'm unhappy with the current names, but they are
>>>> going
>>>> to be bikesheded anyway. So I would like to get the topic moving and
>>>> code is
>>>> a good way to get that.
>>>>
>>>> The name in this series are:
>>>>
>>>> - txnopen (can abort)
>>>> - txnclosing (can abort)
>>>> - txnclosed
>>>
>>> I have no particular recommendations for better names (sorry), but I'm
>>> not sure what the merit of txnopen is given that we can't provide much
>>> information. A sample use case would help there, I guess.
>>
>> I've no sample usecase for txnopen, but it is very cheap to add and I
>> guess some extension//baroque setup will be very happy to have some
>> official way to control//record transaction opening.
>>
>> We could also move it a bit further to right after the transaction
>> have been openened (or add a new hooks for that). So that its easy to
>> add data in the transaction after it is open.
>>
>> Maybe we should have a hook for aborted transaction too. But yet again
>> I've no specific usecase in mind.
>>
>>> As for the others, we could probably rework some existing
>>> functionality to be examples. verify-after-push could be done as a
>>> txnclosing, and txnclosed could be used by the notify extension, so I
>>> see merit in both of those.
>>
>> txnclosing and txnclosed are -must-have-. None of the current hook
>> allow to catch a full transaction run with all its data. This is
>> extremely annoying for validation or synchronization.
>>
> The only idea I have on naming is "trx" instead of "txn", which is what
> I remember using back in my MySQL days. This is purely bikeshedding, and
> I feel bad about it, but you did ask... sooo...
>
> I prefer the "optimal" version you suggested with trx:
>
> - pretrxopen
> - pretrxclose
> - trxclosed

The thing is that we already have two hooks that use txn:

- pretxnchangegroup
- pretxncommit

another option is to go explicit and say pretransactionclose as the 
important part is "transaction"

Full list of currently documented hooks:

- hooks
- changegroup
- commit
- incoming
- outgoing
- post-<command>
- pre-<command>
- prechangegroup
- precommit
- prelistkeys
- preoutgoing
- prepushkey
- pretag
- pretxnchangegroup
- pretxncommit
- preupdate
- listkeys
- pushkey
- tag
- update
Matt Mackall - March 11, 2015, 10:28 p.m.
On Wed, 2015-03-11 at 09:17 -0700, Ryan McElroy wrote:
> On 3/10/2015 9:15 AM, Pierre-Yves David wrote:
> >
> >
> > On 03/10/2015 06:04 AM, Augie Fackler wrote:
> >> On Tue, Mar 10, 2015 at 01:14:28AM -0700, Pierre-Yves David wrote:
> >>> This is RFC because I'm unhappy with the current names, but they are 
> >>> going
> >>> to be bikesheded anyway. So I would like to get the topic moving and 
> >>> code is
> >>> a good way to get that.
> >>>
> >>> The name in this series are:
> >>>
> >>> - txnopen (can abort)
> >>> - txnclosing (can abort)
> >>> - txnclosed
> >>
> >> I have no particular recommendations for better names (sorry), but I'm
> >> not sure what the merit of txnopen is given that we can't provide much
> >> information. A sample use case would help there, I guess.
> >
> > I've no sample usecase for txnopen, but it is very cheap to add and I 
> > guess some extension//baroque setup will be very happy to have some 
> > official way to control//record transaction opening.
> >
> > We could also move it a bit further to right after the transaction 
> > have been openened (or add a new hooks for that). So that its easy to 
> > add data in the transaction after it is open.
> >
> > Maybe we should have a hook for aborted transaction too. But yet again 
> > I've no specific usecase in mind.
> >
> >> As for the others, we could probably rework some existing
> >> functionality to be examples. verify-after-push could be done as a
> >> txnclosing, and txnclosed could be used by the notify extension, so I
> >> see merit in both of those.
> >
> > txnclosing and txnclosed are -must-have-. None of the current hook 
> > allow to catch a full transaction run with all its data. This is 
> > extremely annoying for validation or synchronization.
> >
> The only idea I have on naming is "trx" instead of "txn", which is what 
> I remember using back in my MySQL days. This is purely bikeshedding, and 
> I feel bad about it, but you did ask... sooo...

We've already got pretxncommit and pretxnchangegroup, so we're stuck
with the 'txn' and 'pretxn' bits.

I think pretxnclose and txnclose are the most consistent with our
current nomenclature.
Matt Mackall - March 11, 2015, 10:31 p.m.
On Tue, 2015-03-10 at 01:07 -0700, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1418264389 28800
> #      Wed Dec 10 18:19:49 2014 -0800
> # Node ID 82dea52f27319896a8d33a0b805f439107a94845
> # Parent  4ef4e3c3c00693868ba428e21ac092e559e4fcea
> hook: have a generic hook for transaction opening
> 
> We are adding generic hooking for all transaction. We do not really have any
> useful information to including when opening the transaction but this is a
> useful time to allow hook anywa

I'm inclined to hold off on this until we identify a user.

(As aborting a transaction at pretxnclose time should be equivalent to
never starting it, it shouldn't be essential strictly speaking.)
Pierre-Yves David - March 11, 2015, 10:48 p.m.
On 03/11/2015 03:31 PM, Matt Mackall wrote:
> On Tue, 2015-03-10 at 01:07 -0700, Pierre-Yves David wrote:
>> # HG changeset patch
>> # User Pierre-Yves David <pierre-yves.david@fb.com>
>> # Date 1418264389 28800
>> #      Wed Dec 10 18:19:49 2014 -0800
>> # Node ID 82dea52f27319896a8d33a0b805f439107a94845
>> # Parent  4ef4e3c3c00693868ba428e21ac092e559e4fcea
>> hook: have a generic hook for transaction opening
>>
>> We are adding generic hooking for all transaction. We do not really have any
>> useful information to including when opening the transaction but this is a
>> useful time to allow hook anywa
>
> I'm inclined to hold off on this until we identify a user.

okay

> (As aborting a transaction at pretxnclose time should be equivalent to
> never starting it, it shouldn't be essential strictly speaking.)

It is equivalent if you disregard the 5 minutes spent processing this 
transaction ;-)

If you want to disallow any explicit bookmark movement by user X, you 
could use this hook.
Pierre-Yves David - March 11, 2015, 10:49 p.m.
On 03/11/2015 03:28 PM, Matt Mackall wrote:
> On Wed, 2015-03-11 at 09:17 -0700, Ryan McElroy wrote:
>> On 3/10/2015 9:15 AM, Pierre-Yves David wrote:
>>>
>>>
>>> On 03/10/2015 06:04 AM, Augie Fackler wrote:
>>>> On Tue, Mar 10, 2015 at 01:14:28AM -0700, Pierre-Yves David wrote:
>>>>> This is RFC because I'm unhappy with the current names, but they are
>>>>> going
>>>>> to be bikesheded anyway. So I would like to get the topic moving and
>>>>> code is
>>>>> a good way to get that.
>>>>>
>>>>> The name in this series are:
>>>>>
>>>>> - txnopen (can abort)
>>>>> - txnclosing (can abort)
>>>>> - txnclosed
>>>>
>>>> I have no particular recommendations for better names (sorry), but I'm
>>>> not sure what the merit of txnopen is given that we can't provide much
>>>> information. A sample use case would help there, I guess.
>>>
>>> I've no sample usecase for txnopen, but it is very cheap to add and I
>>> guess some extension//baroque setup will be very happy to have some
>>> official way to control//record transaction opening.
>>>
>>> We could also move it a bit further to right after the transaction
>>> have been openened (or add a new hooks for that). So that its easy to
>>> add data in the transaction after it is open.
>>>
>>> Maybe we should have a hook for aborted transaction too. But yet again
>>> I've no specific usecase in mind.
>>>
>>>> As for the others, we could probably rework some existing
>>>> functionality to be examples. verify-after-push could be done as a
>>>> txnclosing, and txnclosed could be used by the notify extension, so I
>>>> see merit in both of those.
>>>
>>> txnclosing and txnclosed are -must-have-. None of the current hook
>>> allow to catch a full transaction run with all its data. This is
>>> extremely annoying for validation or synchronization.
>>>
>> The only idea I have on naming is "trx" instead of "txn", which is what
>> I remember using back in my MySQL days. This is purely bikeshedding, and
>> I feel bad about it, but you did ask... sooo...
>
> We've already got pretxncommit and pretxnchangegroup, so we're stuck
> with the 'txn' and 'pretxn' bits.
>
> I think pretxnclose and txnclose are the most consistent with our
> current nomenclature.

What are my odd to convince you to go for pretxnclose and txnclosed for 
explicitness?
Matt Mackall - March 11, 2015, 10:59 p.m.
On Wed, 2015-03-11 at 15:48 -0700, Pierre-Yves David wrote:
> 
> On 03/11/2015 03:31 PM, Matt Mackall wrote:
> > On Tue, 2015-03-10 at 01:07 -0700, Pierre-Yves David wrote:
> >> # HG changeset patch
> >> # User Pierre-Yves David <pierre-yves.david@fb.com>
> >> # Date 1418264389 28800
> >> #      Wed Dec 10 18:19:49 2014 -0800
> >> # Node ID 82dea52f27319896a8d33a0b805f439107a94845
> >> # Parent  4ef4e3c3c00693868ba428e21ac092e559e4fcea
> >> hook: have a generic hook for transaction opening
> >>
> >> We are adding generic hooking for all transaction. We do not really have any
> >> useful information to including when opening the transaction but this is a
> >> useful time to allow hook anywa
> >
> > I'm inclined to hold off on this until we identify a user.
> 
> okay
> 
> > (As aborting a transaction at pretxnclose time should be equivalent to
> > never starting it, it shouldn't be essential strictly speaking.)
> 
> It is equivalent if you disregard the 5 minutes spent processing this 
> transaction ;-)
> 
> If you want to disallow any explicit bookmark movement by user X, you 
> could use this hook.

Ok, fair enough.

pretxnopen
pretxnclose
txnclose (no d for consistency with our existing non-past-tense names)
Gregory Szorc - March 14, 2015, 5:42 p.m.
On Tue, Mar 10, 2015 at 9:15 AM, Pierre-Yves David <
pierre-yves.david@ens-lyon.org> wrote:

>
> I've no sample usecase for txnopen, but it is very cheap to add and I
> guess some extension//baroque setup will be very happy to have some
> official way to control//record transaction opening.
>

Assuming all repository mutations occur inside transactions, a "transaction
opening" hook would be a great place to put a "the repo is
closed/read-only" hook, no? Today, you have to hook multiple places:
changegroup and pushkey. I would welcome a single hook.


> We could also move it a bit further to right after the transaction have
> been openened (or add a new hooks for that). So that its easy to add data
> in the transaction after it is open.
>
> Maybe we should have a hook for aborted transaction too. But yet again
> I've no specific usecase in mind.
>

Mozilla's pushlog extension uses a SQLite database to record the who, what,
and when of a push from the server's perspective. We currently do an
internal "hook" of transaction abort so we can roll back the SQL
transaction. While our code is an extension and not using hooks, presumably
an out-of-process hook could want similar behavior. Although, I concede
it's likely of marginal utility.
Pierre-Yves David - March 24, 2015, 11:40 p.m.
On 03/14/2015 10:42 AM, Gregory Szorc wrote:
> On Tue, Mar 10, 2015 at 9:15 AM, Pierre-Yves David
> <pierre-yves.david@ens-lyon.org <mailto:pierre-yves.david@ens-lyon.org>>
> wrote:
>
>
>     I've no sample usecase for txnopen, but it is very cheap to add and
>     I guess some extension//baroque setup will be very happy to have
>     some official way to control//record transaction opening.
>
>
> Assuming all repository mutations occur inside transactions, a
> "transaction opening" hook would be a great place to put a "the repo is
> closed/read-only" hook, no? Today, you have to hook multiple places:
> changegroup and pushkey. I would welcome a single hook.
>
>     We could also move it a bit further to right after the transaction
>     have been openened (or add a new hooks for that). So that its easy
>     to add data in the transaction after it is open.
>
>     Maybe we should have a hook for aborted transaction too. But yet
>     again I've no specific usecase in mind.
>
>
> Mozilla's pushlog extension uses a SQLite database to record the who,
> what, and when of a push from the server's perspective. We currently do
> an internal "hook" of transaction abort so we can roll back the SQL
> transaction. While our code is an extension and not using hooks,
> presumably an out-of-process hook could want similar behavior. Although,
> I concede it's likely of marginal utility.

Such hook would probably makes a lot of sense. But in that case we 
probably need to pass some kind of transaction identifier in such hook 
to lets hook call to relate to each other.

Patch

diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
--- a/mercurial/help/config.txt
+++ b/mercurial/help/config.txt
@@ -806,10 +806,14 @@  variables it is passed are listed with n
   Run before creating a tag. Exit status 0 allows the tag to be
   created. Non-zero status will cause the tag to fail. ID of
   changeset to tag is in ``$HG_NODE``. Name of tag is in ``$HG_TAG``. Tag is
   local if ``$HG_LOCAL=1``, in repository if ``$HG_LOCAL=0``.
 
+``txnopen``
+  Run before any new repository transaction is open. Reason for the transaction
+  opening will be in ``$HG_TXNNAME``. This hook can abort transaction opening.
+
 ``pretxnchangegroup``
   Run after a changegroup has been added via push, pull or unbundle,
   but before the transaction has been committed. Changegroup is
   visible to hook program. This lets you validate incoming changes
   before accepting them. Passed the ID of the first new changeset in
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -906,10 +906,12 @@  class localrepository(object):
         if self.svfs.exists("journal"):
             raise error.RepoError(
                 _("abandoned transaction found"),
                 hint=_("run 'hg recover' to clean up transaction"))
 
+        self.hook('txnopen', throw=True, txnname=desc)
+
         self._writejournal(desc)
         renames = [(vfs, x, undoname(x)) for vfs, x in self._journalfiles()]
         rp = report and report or self.ui.warn
         vfsmap = {'plain': self.vfs} # root of .hg/
         tr = transaction.transaction(rp, self.svfs, vfsmap,
diff --git a/tests/test-hook.t b/tests/test-hook.t
--- a/tests/test-hook.t
+++ b/tests/test-hook.t
@@ -10,15 +10,17 @@  commit hooks can see env vars
   > pretxncommit = sh -c "HG_LOCAL= HG_TAG= python \"$TESTDIR/printenv.py\" pretxncommit"
   > pretxncommit.tip = hg -q tip
   > pre-identify = python "$TESTDIR/printenv.py" pre-identify 1
   > pre-cat = python "$TESTDIR/printenv.py" pre-cat
   > post-cat = python "$TESTDIR/printenv.py" post-cat
+  > txnopen = sh -c "HG_LOCAL= HG_TAG= python \"$TESTDIR/printenv.py\" txnopen"
   > EOF
   $ echo a > a
   $ hg add a
   $ hg commit -m a
   precommit hook: HG_PARENT1=0000000000000000000000000000000000000000
+  txnopen hook: HG_TXNNAME=commit
   pretxncommit hook: HG_NODE=cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b HG_PARENT1=0000000000000000000000000000000000000000 HG_PENDING=$TESTTMP/a
   0:cb9a9f314b8b
   commit hook: HG_NODE=cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b HG_PARENT1=0000000000000000000000000000000000000000
   commit.b hook: HG_NODE=cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b HG_PARENT1=0000000000000000000000000000000000000000
 
@@ -40,30 +42,33 @@  pretxncommit and commit hooks can see bo
 
   $ cd ../a
   $ echo b >> a
   $ hg commit -m a1 -d "1 0"
   precommit hook: HG_PARENT1=cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b
+  txnopen hook: HG_TXNNAME=commit
   pretxncommit hook: HG_NODE=ab228980c14deea8b9555d91c9581127383e40fd HG_PARENT1=cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b HG_PENDING=$TESTTMP/a
   1:ab228980c14d
   commit hook: HG_NODE=ab228980c14deea8b9555d91c9581127383e40fd HG_PARENT1=cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b
   commit.b hook: HG_NODE=ab228980c14deea8b9555d91c9581127383e40fd HG_PARENT1=cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b
   $ hg update -C 0
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ echo b > b
   $ hg add b
   $ hg commit -m b -d '1 0'
   precommit hook: HG_PARENT1=cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b
+  txnopen hook: HG_TXNNAME=commit
   pretxncommit hook: HG_NODE=ee9deb46ab31e4cc3310f3cf0c3d668e4d8fffc2 HG_PARENT1=cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b HG_PENDING=$TESTTMP/a
   2:ee9deb46ab31
   commit hook: HG_NODE=ee9deb46ab31e4cc3310f3cf0c3d668e4d8fffc2 HG_PARENT1=cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b
   commit.b hook: HG_NODE=ee9deb46ab31e4cc3310f3cf0c3d668e4d8fffc2 HG_PARENT1=cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b
   created new head
   $ hg merge 1
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved
   (branch merge, don't forget to commit)
   $ hg commit -m merge -d '2 0'
   precommit hook: HG_PARENT1=ee9deb46ab31e4cc3310f3cf0c3d668e4d8fffc2 HG_PARENT2=ab228980c14deea8b9555d91c9581127383e40fd
+  txnopen hook: HG_TXNNAME=commit
   pretxncommit hook: HG_NODE=07f3376c1e655977439df2a814e3cc14b27abac2 HG_PARENT1=ee9deb46ab31e4cc3310f3cf0c3d668e4d8fffc2 HG_PARENT2=ab228980c14deea8b9555d91c9581127383e40fd HG_PENDING=$TESTTMP/a
   3:07f3376c1e65
   commit hook: HG_NODE=07f3376c1e655977439df2a814e3cc14b27abac2 HG_PARENT1=ee9deb46ab31e4cc3310f3cf0c3d668e4d8fffc2 HG_PARENT2=ab228980c14deea8b9555d91c9581127383e40fd
   commit.b hook: HG_NODE=07f3376c1e655977439df2a814e3cc14b27abac2 HG_PARENT1=ee9deb46ab31e4cc3310f3cf0c3d668e4d8fffc2 HG_PARENT2=ab228980c14deea8b9555d91c9581127383e40fd
 
@@ -101,10 +106,11 @@  tag hooks can see env vars
   > tag = sh -c "HG_PARENT1= HG_PARENT2= python \"$TESTDIR/printenv.py\" tag"
   > EOF
   $ hg tag -d '3 0' a
   pretag hook: HG_LOCAL=0 HG_NODE=07f3376c1e655977439df2a814e3cc14b27abac2 HG_TAG=a
   precommit hook: HG_PARENT1=07f3376c1e655977439df2a814e3cc14b27abac2
+  txnopen hook: HG_TXNNAME=commit
   pretxncommit hook: HG_NODE=539e4b31b6dc99b3cfbaa6b53cbc1c1f9a1e3a10 HG_PARENT1=07f3376c1e655977439df2a814e3cc14b27abac2 HG_PENDING=$TESTTMP/a
   4:539e4b31b6dc
   tag hook: HG_LOCAL=0 HG_NODE=07f3376c1e655977439df2a814e3cc14b27abac2 HG_TAG=a
   commit hook: HG_NODE=539e4b31b6dc99b3cfbaa6b53cbc1c1f9a1e3a10 HG_PARENT1=07f3376c1e655977439df2a814e3cc14b27abac2
   commit.b hook: HG_NODE=539e4b31b6dc99b3cfbaa6b53cbc1c1f9a1e3a10 HG_PARENT1=07f3376c1e655977439df2a814e3cc14b27abac2
@@ -135,10 +141,11 @@  more there after
   $ hg add z
   $ hg -q tip
   4:539e4b31b6dc
   $ hg commit -m 'fail' -d '4 0'
   precommit hook: HG_PARENT1=539e4b31b6dc99b3cfbaa6b53cbc1c1f9a1e3a10
+  txnopen hook: HG_TXNNAME=commit
   pretxncommit hook: HG_NODE=6f611f8018c10e827fee6bd2bc807f937e761567 HG_PARENT1=539e4b31b6dc99b3cfbaa6b53cbc1c1f9a1e3a10 HG_PENDING=$TESTTMP/a
   5:6f611f8018c1
   5:6f611f8018c1
   pretxncommit.forbid hook: HG_NODE=6f611f8018c10e827fee6bd2bc807f937e761567 HG_PARENT1=539e4b31b6dc99b3cfbaa6b53cbc1c1f9a1e3a10 HG_PENDING=$TESTTMP/a
   transaction abort!
@@ -196,10 +203,11 @@  pushkey hook
   $ hg bookmark -r null foo
   $ hg push -B foo ../a
   pushing to ../a
   searching for changes
   no changes found
+  txnopen hook: HG_TXNNAME=bookmarks
   pushkey hook: HG_KEY=foo HG_NAMESPACE=bookmarks HG_NEW=0000000000000000000000000000000000000000 HG_RET=1
   exporting bookmark foo
   [1]
   $ cd ../a