Patchwork [2,of,2] hooks: allow Unix style environment variables on external Windows hooks

login
register
mail settings
Submitter Matt Harbison
Date June 27, 2018, 12:44 p.m.
Message ID <6d206e4742d6f31246d0.1530103467@Envy>
Download mbox | patch
Permalink /patch/32459/
State Accepted
Headers show

Comments

Matt Harbison - June 27, 2018, 12:44 p.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1498969929 14400
#      Sun Jul 02 00:32:09 2017 -0400
# Node ID 6d206e4742d6f31246d0e234add03436ac429ebf
# Parent  7ac9de5a8826fc95864ee4ba844eb8b5c9e71332
hooks: allow Unix style environment variables on external Windows hooks

This will help making common hooks between Windows and non-Windows platforms.

Having to build the shellenviron dict here and in procutil.system() is a bit
unfortunate, but the only other option is to fix up the command inside
procutil.system().  It seems more important that the note about the hook being
run reflects what is actually run.

The patch from last summer added the hooks on the command line, but it looks
like HG_ARGS has since learned about --config args, and the output was just
confusing.  Therefore, it's now loaded from a file in the histedit test for
clarity.
Yuya Nishihara - June 28, 2018, 11:53 a.m.
On Wed, 27 Jun 2018 08:44:27 -0400, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1498969929 14400
> #      Sun Jul 02 00:32:09 2017 -0400
> # Node ID 6d206e4742d6f31246d0e234add03436ac429ebf
> # Parent  7ac9de5a8826fc95864ee4ba844eb8b5c9e71332
> hooks: allow Unix style environment variables on external Windows hooks

> +    if pycompat.iswindows:
> +        environ = procutil.shellenviron(env)
> +        cmd = util.platform.shelltocmdexe(cmd, environ)

util.platform isn't intended to be a re-export of the platform module.
Instead, we can add a wrapper function to procutil:

  if windows:
      def shelltonative(cmd, env):
          return platform.shelltocmdexe(cmd, shellenviron(env))
  else:
      def shelltonative(cmd, env):
          return cmd

Can you send a follow up?

Patch

diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
--- a/mercurial/help/config.txt
+++ b/mercurial/help/config.txt
@@ -886,6 +886,12 @@  They contain the type of hook which trig
 of the hook in the config, respectively. In the example above, this will
 be ``$HG_HOOKTYPE=incoming`` and ``$HG_HOOKNAME=incoming.email``.
 
+.. container:: windows
+
+  Some basic Unix syntax is supported for portability, including ``$VAR``
+  and ``${VAR}`` style variables.  To use a literal ``$``, it must be
+  escaped with a back slash or inside of a strong quote.
+
 ``changegroup``
   Run after a changegroup has been added via push, pull or unbundle.  The ID of
   the first new changeset is in ``$HG_NODE`` and last is in ``$HG_NODE_LAST``.
diff --git a/mercurial/hook.py b/mercurial/hook.py
--- a/mercurial/hook.py
+++ b/mercurial/hook.py
@@ -120,8 +120,6 @@  def pythonhook(ui, repo, htype, hname, f
     return r, False
 
 def _exthook(ui, repo, htype, name, cmd, args, throw):
-    ui.note(_("running hook %s: %s\n") % (name, cmd))
-
     starttime = util.timer()
     env = {}
 
@@ -141,6 +139,12 @@  def _exthook(ui, repo, htype, name, cmd,
             v = stringutil.pprint(v)
         env['HG_' + k.upper()] = v
 
+    if pycompat.iswindows:
+        environ = procutil.shellenviron(env)
+        cmd = util.platform.shelltocmdexe(cmd, environ)
+
+    ui.note(_("running hook %s: %s\n") % (name, cmd))
+
     if repo:
         cwd = repo.root
     else:
diff --git a/tests/test-histedit-fold.t b/tests/test-histedit-fold.t
--- a/tests/test-histedit-fold.t
+++ b/tests/test-histedit-fold.t
@@ -478,14 +478,7 @@  This is an excuse to test hook with hist
   1:199b6bb90248 b
   0:6c795aa153cb a
 
-Setup the proper environment variable symbol for the platform, to be subbed
-into the hook command.
-#if windows
-  $ NODE="%HG_NODE%"
-#else
-  $ NODE="\$HG_NODE"
-#endif
-  $ hg histedit 6c795aa153cb --config hooks.commit="echo commit $NODE" --commands - 2>&1 << EOF | fixbundle
+  $ hg histedit 6c795aa153cb --config hooks.commit='echo commit $HG_NODE' --commands - 2>&1 << EOF | fixbundle
   > pick 199b6bb90248 b
   > fold a1a953ffb4b0 c
   > pick 6c795aa153cb a
@@ -496,8 +489,24 @@  into the hook command.
   1:9599899f62c0 a
   0:79b99e9c8e49 b
 
+Test unix -> windows style variable substitution in external hooks.
+
+  $ cat > $TESTTMP/tmp.hgrc <<'EOF'
+  > [hooks]
+  > pre-add = echo no variables
+  > post-add = echo ran $HG_ARGS, literal \$non-var, 'also $non-var', $HG_RESULT
+  > EOF
+
+TODO: Windows should output double quotes around "also $non-var"
   $ echo "foo" > amended.txt
-  $ hg add amended.txt
+  $ HGRCPATH=$TESTTMP/tmp.hgrc hg add -v amended.txt
+  running hook pre-add: echo no variables
+  no variables
+  adding amended.txt
+  running hook post-add: echo ran %HG_ARGS%, literal $non-var, 'also $non-var', %HG_RESULT% (windows !)
+  running hook post-add: echo ran $HG_ARGS, literal \$non-var, 'also $non-var', $HG_RESULT (no-windows !)
+  ran add -v amended.txt, literal $non-var, 'also $non-var', 0 (windows !)
+  ran add -v amended.txt, literal $non-var, also $non-var, 0 (no-windows !)
   $ hg ci -q --config extensions.largefiles= --amend -I amended.txt
   The fsmonitor extension is incompatible with the largefiles extension and has been disabled. (fsmonitor !)
 
diff --git a/tests/test-rebase-interruptions.t b/tests/test-rebase-interruptions.t
--- a/tests/test-rebase-interruptions.t
+++ b/tests/test-rebase-interruptions.t
@@ -333,12 +333,7 @@  Test rebase interrupted by hooks
 
   $ cp -R a3 hook-pretxncommit
   $ cd hook-pretxncommit
-#if windows
-  $ NODE="%HG_NODE%"
-#else
-  $ NODE="\$HG_NODE"
-#endif
-  $ hg rebase --source 2 --dest 5 --tool internal:other --config "hooks.pretxncommit=hg log -r $NODE | grep \"summary:     C\""
+  $ hg rebase --source 2 --dest 5 --tool internal:other --config 'hooks.pretxncommit=hg log -r $HG_NODE | grep "summary:     C"'
   rebasing 2:965c486023db "C"
   summary:     C
   rebasing 6:a0b2430ebfb8 "F" (tip)