Re: [PATCH 3 of 4 V2] phase: add a dedicated txnclose-phase hook

2017-10-19 Thread Boris Feld 
On Thu, 2017-10-19 at 08:57 +0200, Boris Feld wrote:
> On Wed, 2017-10-18 at 10:51 -0500, Kevin Bullock wrote:
> > > On Oct 14, 2017, at 10:46, Boris Feld 
> > > wrote:
> > > 
> > > # HG changeset patch
> > > # User Boris Feld 
> > > # Date 1507477846 -7200
> > > #  Sun Oct 08 17:50:46 2017 +0200
> > > # Node ID 06eeae1828afe149886e18e6b4fc8180a3803fdb
> > > # Parent  e1aa5834b2423fd04b32965075e5dee2204768c2
> > > # EXP-Topic b2.phases.hooks
> > > # Available At https://bitbucket.org/octobus/mercurial-devel/
> > > #  hg pull https://bitbucket.org/octobus/mercurial-de
> > > ve
> > > l/ -r 06eeae1828af
> > > phase: add a dedicated txnclose-phase hook
> > > 
> > 
> > [snip]
> > > diff --git a/tests/test-phases.t b/tests/test-phases.t
> > > --- a/tests/test-phases.t
> > > +++ b/tests/test-phases.t
> > > @@ -2,6 +2,8 @@
> > >   $ cat >> $HGRCPATH << EOF
> > > > [extensions]
> > > > phasereport=$TESTDIR/testlib/ext-phase-report.py
> > > 
> > > +  > [hooks]
> > > +  > txnclose-phase.test = echo "test-hook-close-phase:
> > > \$HG_NODE:  \$HG_OLDPHASE -> \$HG_PHASE"
> > > > EOF
> > > 
> > >   $ hglog() { hg log --template "{rev} {phaseidx} {desc}\n" $*; }
> > > @@ -26,6 +28,7 @@
> > > 
> > >   $ mkcommit A
> > >   test-debug-phase: new rev 0:  x -> 1
> > > +  test-hook-close-phase:
> > > 4a2df7238c3b48766b5e22fafbb8a2f506ec8256:   -> 1
> > 
> > I don't think there's any other place where we expose numeric
> > values
> > for phase, and I'm not keen to start doing so now. Could you send a
> > follow-up that changes these to the normal
> > 'secret'/'draft'/'public'
> > labels?
> 
> I think the pushkey hooks are using numeric values. As we might have
> people migrating from pushkey hooks to new phases hooks, I think we
> should keep the numeric values but send the human phase
> representation
> in additional environment variables, like PHASESTR, OLDPHASESTR.
> 
> Also, numeric values are useful for making comparison so I think we
> should keep them.
> 
> What do you think? Should I send a follow-up?

After re-reading the output of existing puskey hooks, I realized that
the variables are not the same anyway so people will need to rewrite
their phase hooks for the new variable.

But, I think that we should send numeric values in additional
variables. We have the {phase} template keyword and the {phaseidx}
template keyword. Kevin, I've seen your follow-up changing phase and
oldphase to send phase name instead of phase value, I will send a
follow-up sending also phaseidx and oldphase in hooks with their
numeric values.

> 
> > 
> > pacem in terris / мир / शान्ति / ‎‫سَلاَم‬ / 平和
> > Kevin R. Bullock
> > 
> 
> ___
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 3 of 4 V2] phase: add a dedicated txnclose-phase hook

2017-10-19 Thread Boris Feld 
On Wed, 2017-10-18 at 10:51 -0500, Kevin Bullock wrote:
> > On Oct 14, 2017, at 10:46, Boris Feld 
> > wrote:
> > 
> > # HG changeset patch
> > # User Boris Feld 
> > # Date 1507477846 -7200
> > #  Sun Oct 08 17:50:46 2017 +0200
> > # Node ID 06eeae1828afe149886e18e6b4fc8180a3803fdb
> > # Parent  e1aa5834b2423fd04b32965075e5dee2204768c2
> > # EXP-Topic b2.phases.hooks
> > # Available At https://bitbucket.org/octobus/mercurial-devel/
> > #  hg pull https://bitbucket.org/octobus/mercurial-deve
> > l/ -r 06eeae1828af
> > phase: add a dedicated txnclose-phase hook
> > 
> 
> [snip]
> > diff --git a/tests/test-phases.t b/tests/test-phases.t
> > --- a/tests/test-phases.t
> > +++ b/tests/test-phases.t
> > @@ -2,6 +2,8 @@
> >   $ cat >> $HGRCPATH << EOF
> > > [extensions]
> > > phasereport=$TESTDIR/testlib/ext-phase-report.py
> > 
> > +  > [hooks]
> > +  > txnclose-phase.test = echo "test-hook-close-phase:
> > \$HG_NODE:  \$HG_OLDPHASE -> \$HG_PHASE"
> > > EOF
> > 
> >   $ hglog() { hg log --template "{rev} {phaseidx} {desc}\n" $*; }
> > @@ -26,6 +28,7 @@
> > 
> >   $ mkcommit A
> >   test-debug-phase: new rev 0:  x -> 1
> > +  test-hook-close-phase:
> > 4a2df7238c3b48766b5e22fafbb8a2f506ec8256:   -> 1
> 
> I don't think there's any other place where we expose numeric values
> for phase, and I'm not keen to start doing so now. Could you send a
> follow-up that changes these to the normal 'secret'/'draft'/'public'
> labels?

I think the pushkey hooks are using numeric values. As we might have
people migrating from pushkey hooks to new phases hooks, I think we
should keep the numeric values but send the human phase representation
in additional environment variables, like PHASESTR, OLDPHASESTR.

Also, numeric values are useful for making comparison so I think we
should keep them.

What do you think? Should I send a follow-up?

> 
> pacem in terris / мир / शान्ति / ‎‫سَلاَم‬ / 平和
> Kevin R. Bullock
> 
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 3 of 4 V2] phase: add a dedicated txnclose-phase hook

2017-10-18 Thread Martin von Zweigbergk via Mercurial-devel 
On Wed, Oct 18, 2017 at 8:51 AM, Kevin Bullock <
kbullock+mercur...@ringworld.org> wrote:

> > On Oct 14, 2017, at 10:46, Boris Feld  wrote:
> >
> > # HG changeset patch
> > # User Boris Feld 
> > # Date 1507477846 -7200
> > #  Sun Oct 08 17:50:46 2017 +0200
> > # Node ID 06eeae1828afe149886e18e6b4fc8180a3803fdb
> > # Parent  e1aa5834b2423fd04b32965075e5dee2204768c2
> > # EXP-Topic b2.phases.hooks
> > # Available At https://bitbucket.org/octobus/mercurial-devel/
> > #  hg pull https://bitbucket.org/octobus/mercurial-devel/
> -r 06eeae1828af
> > phase: add a dedicated txnclose-phase hook
> >
> [snip]
> > diff --git a/tests/test-phases.t b/tests/test-phases.t
> > --- a/tests/test-phases.t
> > +++ b/tests/test-phases.t
> > @@ -2,6 +2,8 @@
> >   $ cat >> $HGRCPATH << EOF
> >> [extensions]
> >> phasereport=$TESTDIR/testlib/ext-phase-report.py
> > +  > [hooks]
> > +  > txnclose-phase.test = echo "test-hook-close-phase: \$HG_NODE:
> \$HG_OLDPHASE -> \$HG_PHASE"
> >> EOF
> >
> >   $ hglog() { hg log --template "{rev} {phaseidx} {desc}\n" $*; }
> > @@ -26,6 +28,7 @@
> >
> >   $ mkcommit A
> >   test-debug-phase: new rev 0:  x -> 1
> > +  test-hook-close-phase: 4a2df7238c3b48766b5e22fafbb8a2f506ec8256:
>  -> 1
>
> I don't think there's any other place where we expose numeric values for
> phase, and I'm not keen to start doing so now. Could you send a follow-up
> that changes these to the normal 'secret'/'draft'/'public' labels?
>

+1

I had not seen Kevin's comment when I was reviewing the queued patches just
now and looked up this thread just to make the same comment :-)


>
> pacem in terris / мир / शान्ति / ‎‫سَلاَم‬ / 平和
> Kevin R. Bullock
>
> ___
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 3 of 4 V2] phase: add a dedicated txnclose-phase hook

2017-10-18 Thread Kevin Bullock 
> On Oct 14, 2017, at 10:46, Boris Feld  wrote:
> 
> # HG changeset patch
> # User Boris Feld 
> # Date 1507477846 -7200
> #  Sun Oct 08 17:50:46 2017 +0200
> # Node ID 06eeae1828afe149886e18e6b4fc8180a3803fdb
> # Parent  e1aa5834b2423fd04b32965075e5dee2204768c2
> # EXP-Topic b2.phases.hooks
> # Available At https://bitbucket.org/octobus/mercurial-devel/
> #  hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 
> 06eeae1828af
> phase: add a dedicated txnclose-phase hook
> 
[snip]
> diff --git a/tests/test-phases.t b/tests/test-phases.t
> --- a/tests/test-phases.t
> +++ b/tests/test-phases.t
> @@ -2,6 +2,8 @@
>   $ cat >> $HGRCPATH << EOF
>> [extensions]
>> phasereport=$TESTDIR/testlib/ext-phase-report.py
> +  > [hooks]
> +  > txnclose-phase.test = echo "test-hook-close-phase: \$HG_NODE:  
> \$HG_OLDPHASE -> \$HG_PHASE"
>> EOF
> 
>   $ hglog() { hg log --template "{rev} {phaseidx} {desc}\n" $*; }
> @@ -26,6 +28,7 @@
> 
>   $ mkcommit A
>   test-debug-phase: new rev 0:  x -> 1
> +  test-hook-close-phase: 4a2df7238c3b48766b5e22fafbb8a2f506ec8256:   -> 1

I don't think there's any other place where we expose numeric values for phase, 
and I'm not keen to start doing so now. Could you send a follow-up that changes 
these to the normal 'secret'/'draft'/'public' labels?

pacem in terris / мир / शान्ति / ‎‫سَلاَم‬ / 平和
Kevin R. Bullock

___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

[PATCH 3 of 4 V2] phase: add a dedicated txnclose-phase hook

2017-10-14 Thread Boris Feld 
# HG changeset patch
# User Boris Feld 
# Date 1507477846 -7200
#  Sun Oct 08 17:50:46 2017 +0200
# Node ID 06eeae1828afe149886e18e6b4fc8180a3803fdb
# Parent  e1aa5834b2423fd04b32965075e5dee2204768c2
# EXP-Topic b2.phases.hooks
# Available At https://bitbucket.org/octobus/mercurial-devel/
#  hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 
06eeae1828af
phase: add a dedicated txnclose-phase hook

The new 'txnclose-phase' hook expose the phase movement information stored in
'tr.changes['phases]'. To provide a simple and straightforward hook API to the
users, we introduce a new hook called for each revision affected.  Since a
transaction can affect the phase of multiple changesets, updating the existing
'txnclose' hook to expose that information would be more complex. The data for
all moves will not fit in environment variables and iterations over each move
would be cumbersome. So the introduction of a new dedicated hook is
preferred in this changesets.

This does not exclude the addition of the full phase movement information to
the existing 'txnclose' in the future to help write more complex hooks.

diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
--- a/mercurial/help/config.txt
+++ b/mercurial/help/config.txt
@@ -1002,6 +1002,16 @@
   is released. See :hg:`help config.hooks.pretxnclose-bookmark` for details
   about available variables.
 
+``txnclose-phase``
+  Run after any phase change has been committed. At this point, the
+  transaction can no longer be rolled back. The hook will run after the lock
+  is released.
+  The affected node is available in ``$HG_NODE``, the new phase will be
+  available in ``$HG_PHASE`` while the previous phase will be available in
+  ``$HG_OLDPHASE``. In case of new node, ``$HG_OLDPHASE`` will be empty.  In
+  addition, the reason for the transaction opening will be in ``$HG_TXNNAME``,
+  and a unique identifier for the transaction will be in ``HG_TXNID``.
+
 ``txnabort``
   Run when a transaction is aborted. See :hg:`help config.hooks.pretxnclose`
   for details about available variables.
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -1299,6 +1299,16 @@
 repo.hook('txnclose-bookmark', throw=False,
   txnname=desc, **pycompat.strkwargs(args))
 
+if hook.hashook(repo.ui, 'txnclose-phase'):
+cl = repo.unfiltered().changelog
+phasemv = sorted(tr.changes['phases'].items())
+for rev, (old, new) in phasemv:
+args = tr.hookargs.copy()
+node = hex(cl.node(rev))
+args.update(phases.preparehookargs(node, old, new))
+repo.hook('txnclose-phase', throw=False, txnname=desc,
+  **pycompat.strkwargs(args))
+
 repo.hook('txnclose', throw=False, txnname=desc,
   **pycompat.strkwargs(hookargs))
 reporef()._afterlock(hookfunc)
diff --git a/mercurial/phases.py b/mercurial/phases.py
--- a/mercurial/phases.py
+++ b/mercurial/phases.py
@@ -632,3 +632,12 @@
 def hassecret(repo):
 """utility function that check if a repo have any secret changeset."""
 return bool(repo._phasecache.phaseroots[2])
+
+def preparehookargs(node, old, new):
+if old is None:
+old = ''
+else:
+old = '%s' % old
+return {'node': node,
+'oldphase': old,
+'phase': '%s' % new}
diff --git a/tests/test-phases.t b/tests/test-phases.t
--- a/tests/test-phases.t
+++ b/tests/test-phases.t
@@ -2,6 +2,8 @@
   $ cat >> $HGRCPATH << EOF
   > [extensions]
   > phasereport=$TESTDIR/testlib/ext-phase-report.py
+  > [hooks]
+  > txnclose-phase.test = echo "test-hook-close-phase: \$HG_NODE:  
\$HG_OLDPHASE -> \$HG_PHASE"
   > EOF
 
   $ hglog() { hg log --template "{rev} {phaseidx} {desc}\n" $*; }
@@ -26,6 +28,7 @@
 
   $ mkcommit A
   test-debug-phase: new rev 0:  x -> 1
+  test-hook-close-phase: 4a2df7238c3b48766b5e22fafbb8a2f506ec8256:   -> 1
 
 New commit are draft by default
 
@@ -36,6 +39,7 @@
 
   $ mkcommit B
   test-debug-phase: new rev 1:  x -> 1
+  test-hook-close-phase: 27547f69f25460a52fff66ad004e58da7ad3fb56:   -> 1
 
   $ hglog
   1 1 B
@@ -46,6 +50,8 @@
   $ hg phase --public .
   test-debug-phase: move rev 0: 1 -> 0
   test-debug-phase: move rev 1: 1 -> 0
+  test-hook-close-phase: 4a2df7238c3b48766b5e22fafbb8a2f506ec8256:  1 -> 0
+  test-hook-close-phase: 27547f69f25460a52fff66ad004e58da7ad3fb56:  1 -> 0
   $ hg phase
   1: public
   $ hglog
@@ -54,8 +60,10 @@
 
   $ mkcommit C
   test-debug-phase: new rev 2:  x -> 1
+  test-hook-close-phase: f838bfaca5c7226600ebcfd84f3c3c13a28d3757:   -> 1
   $ mkcommit D
   test-debug-phase: new rev 3:  x -> 1
+  test-hook-close-phase: b3325c91a4d916bcc4cdc83ea3fe4ece46a42f6e:   -> 1