Simplify configuration of trac-svn-hook

[Ryan J Ollos]

Proposed is a minor change, untested so far, to make it easier to configure trac-svn-hook without edit the file, using ​hook script environment configuration.

See TracRepositoryAdmin@45.

contrib/trac-svn-hook

@@ -45 +45 @@
 #         /path/to/trac-svn-hook $REPOS $REV $USER $PROPNAME
 #     fi
 #
-# See also http://svnbook.red-bean.com/en/1.5/svn.reposadmin.create.html#svn.reposadmin.create.hooks
+# See also http://svnbook.red-bean.com/en/1.7/svn.reposadmin.create.html#svn.reposadmin.create.hooks
 #
 #  Platform:: Unix or Cygwin.
 #
@@ -60 +60 @@
 #
 # == Configuration
 #
-# Uncomment and adapt to your local setup:
+# Uncomment and adapt to your local setup. In Subversion 1.8 and later, you
+# can set these variables in a hook script environment configuration rather
+# than editing this file.
 #
-# export TRAC_ENV=/path/to/trac-env:/path/to/another/trac-env
-# export PATH=/path/to/python/bin:$PATH
-# export LD_LIBRARY_PATH=/path/to/python/lib:$LD_LIBRARY_PATH
+# See also http://svnbook.red-bean.com/en/1.8/svn.reposadmin.create.html#svn.reposadmin.hooks.configuration
+#
+# TRAC_ENV=/path/to/trac-env:/path/to/another/trac-env
+# PYTHON_BIN=/path/to/python/bin
+# PYTHON_LIB=/path/to/python/lib
 #
 # -----------------------------------------------------------------------------
 #
@@ -171 +175 @@
 #
 # This is the script itself, you shouldn't need to modify this part.
 
+export PATH=$PYTHON_BIN:$PATH
+export LD_LIBRARY_PATH=$PYTHON_LIB:$LD_LIBRARY_PATH
+
 # -- Command line arguments (cf. usage)
 
 REPOS="$1"

[Ryan J Ollos]

DONE Minor revisions to 1.3/TracRepositoryAdmin when change is committed.

[Ryan J Ollos]

I'll propose more extensive changes after testing on Windows with Cygwin.

[Jun Omae]

I think we should check whether PYTHON_BIN and PYTHON_LIB are not empty.

-export PATH=$PYTHON_BIN:$PATH
-export LD_LIBRARY_PATH=$PYTHON_LIB:$LD_LIBRARY_PATH
+if [ -n "$PYTHON_BIN" ]; then
+    export PATH="$PYTHON_BIN:$PATH"
+fi
+if [ -n "$PYTHON_LIB" ]; then
+    export LD_LIBRARY_PATH="$PYTHON_LIB:$LD_LIBRARY_PATH"
+fi

[Jun Omae]

In [b1cc07561/rjollos.git], example settings for cygwin are changed but LD_LIBRARY_PATH doesn't exist on cygwin. Instead, we should add $PATHONHOME\scripts to PATH environment.

-# PYTHON_LIB=/C/Dev/Python261/Scripts
+# PYTHON_BIN=/C/Dev/Python261/Scripts

[Ryan J Ollos]

That's WIP, not ready for review.

[Ryan J Ollos]

The main goal I have for this ticket is to extract the variables that are appended to PATH and LD_LIBRARY_PATH so that those variables can also be specified in the hook configuration. I reconsidered the variable naming in light of some of the examples in the script, and propose to add two variables with the TRAC_ prefix: TRAC_PATH and TRAC_LD_LIBRARY_PATH. Those variables will get appended to PATH and LD_LIBRARY_PATH, respectively. The user can specify the variables directly in trac-svn-hook, in the subversion hook script (post-commit and post-revprop-change) or the environment hook configuration.

Proposed changes in [8ce98ed62/rjollos.git].

[Ryan J Ollos]

If we echo to stderr the debug would appear in the client output, which could facilitate debugging.

TRAC_ENV unset:

Committing transaction...
Committed revision 167.

Warning: post-commit hook failed (exit code 2) with no output.

TRAC_ENV unset with proposed changes:

Committing transaction...
Committed revision 168.

Warning: post-commit hook failed (exit code 2) with output:
TRAC_ENV is not set.

contrib/trac-svn-hook

@@ -191 +191 @@
 fi
 
 if [ -z "$TRAC_ENV" ]; then
-    echo "TRAC_ENV is not set."
+    >&2 echo "TRAC_ENV is not set."
     exit 2
 fi

… and presumably similar changes for other echoed error messages, but I haven't tested yet.

[Ryan J Ollos]

Proposed changes in rjollos.git:t12771_simplify_svn_hook.2.

[Ryan J Ollos]

Side note, I see several places in the SVN documentation stating that hooks are run in an empty environment (e.g. ​here):

Before Subversion calls a hook script, it removes all variables — including $PATH on Unix, and %PATH% on Windows — from the environment. Therefore, your script can only run another program if you spell out that program's absolute name.

Similarly, trac-svn-hook documentation says:

# This folder is typically the same as your Python installation bin/ folder.
# If this is /usr/bin, then you probably don't need to put it in TRAC_PATH.

which seems to be correct, but implies that /usr/bin is on PATH.

I noticed though, if I tried to set PATH in hooks-env, which presumably overwrites the existing PATH environment variable,

[default]
TRAC_ENV=/Users/rjollos/Documents/Workspace/trac-dev/tracenvs/proj-1.3
PATH=/Users/rjollos/Documents/Workspace/trac-dev/pve/bin

the following is found in trac-svn-.log:

/Users/rjollos/Documents/Workspace/trac-dev/repos/svn/proj1/hooks/trac-svn-hook: line 242: nohup: command not found

So the script doesn't see nohup in /usr/bin:

$ which nohup
/usr/bin/nohup

If I put the following in the top of trac-svn-hook,

>&2 echo "$PATH"
exit 2

the output is:

Warning: post-commit hook failed (exit code 2) with output:
/usr/gnu/bin:/usr/local/bin:/bin:/usr/bin:.

Or putting >&2 echo `printenv`; exit 2 after all the export statements in trac-svn-hook:

TRAC_ENV=/Users/rjollos/Documents/Workspace/trac-dev/tracenvs/proj-1.3 
PATH=/Users/rjollos/Documents/Workspace/trac-dev/pve/bin:/usr/gnu/bin:/usr/local/bin:/bin:/usr/bin:.
PWD=/Users/rjollos/Documents/Workspace/trac-dev/repos/svn-proj1-wc/trunk
SHLVL=2
TRAC_PATH=/Users/rjollos/Documents/Workspace/trac-dev/pve/bin
_=/usr/bin/printenv

It seems like the environment is not entirely empty.

[Ryan J Ollos]

While it appears to be harmless, I also don't see the need to export TRAC_ENV from trac-svn-hook since the value isn't used outside of that script. It would of course be necessary to export TRAC_ENV if setting the value in post-commit or post-revprop-change.

[Jun Omae]

The PATH environment is set to default value when it is not exported.

$ env - /bin/sh -c 'echo "= export"; export; echo "= set"; set; echo "="; type nohup'
= export
export PWD='/home/jun66j5/src/tracdev/git'
= set
IFS='
'
OPTIND='1'
PATH='/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin'
PPID='1589'
PS1='$ '
PS2='> '
PS4='+ '
PWD='/home/jun66j5/src/tracdev/git'
=
nohup is /usr/bin/nohup

When PATH is exported:

$ env - PATH=$HOME/bin /bin/sh -c 'echo "= export"; export; echo "= set"; set; echo "="; type nohup'
= export
export PATH='/home/jun66j5/bin'
export PWD='/home/jun66j5/src/tracdev/git'
= set
IFS='
'
OPTIND='1'
PATH='/home/jun66j5/bin'
PPID='1589'
PS1='$ '
PS2='> '
PS4='+ '
PWD='/home/jun66j5/src/tracdev/git'
=
nohup: not found

[Ryan J Ollos]

Committed to trunk in r16342.