Edgewall Software
Modify

Opened 3 years ago

Closed 2 years ago

#12771 closed enhancement (fixed)

Simplify configuration of trac-svn-hook

Reported by: Ryan J Ollos Owned by: Ryan J Ollos
Priority: normal Milestone: 1.3.3
Component: version control Version:
Severity: normal Keywords: svn trac-svn-hook
Cc: Branch:
Release Notes:

trac-svn-hook can be configured using the hook script environment configuration, available in Subversion 1.8 and later.

API Changes:

Description (last modified by 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

    diff --git a/contrib/trac-svn-hook b/contrib/trac-svn-hook
    index 7a1416088..291f88444 100755
    a b  
    4545#         /path/to/trac-svn-hook $REPOS $REV $USER $PROPNAME
    4646#     fi
    4747#
    48 # See also http://svnbook.red-bean.com/en/1.5/svn.reposadmin.create.html#svn.reposadmin.create.hooks
     48# See also http://svnbook.red-bean.com/en/1.7/svn.reposadmin.create.html#svn.reposadmin.create.hooks
    4949#
    5050#  Platform:: Unix or Cygwin.
    5151#
     
    6060#
    6161# == Configuration
    6262#
    63 # Uncomment and adapt to your local setup:
     63# Uncomment and adapt to your local setup. In Subversion 1.8 and later, you
     64# can set these variables in a hook script environment configuration rather
     65# than editing this file.
    6466#
    65 # export TRAC_ENV=/path/to/trac-env:/path/to/another/trac-env
    66 # export PATH=/path/to/python/bin:$PATH
    67 # export LD_LIBRARY_PATH=/path/to/python/lib:$LD_LIBRARY_PATH
     67# See also http://svnbook.red-bean.com/en/1.8/svn.reposadmin.create.html#svn.reposadmin.hooks.configuration
     68#
     69# TRAC_ENV=/path/to/trac-env:/path/to/another/trac-env
     70# PYTHON_BIN=/path/to/python/bin
     71# PYTHON_LIB=/path/to/python/lib
    6872#
    6973# -----------------------------------------------------------------------------
    7074#
     
    171175#
    172176# This is the script itself, you shouldn't need to modify this part.
    173177
     178export PATH=$PYTHON_BIN:$PATH
     179export LD_LIBRARY_PATH=$PYTHON_LIB:$LD_LIBRARY_PATH
     180
    174181# -- Command line arguments (cf. usage)
    175182
    176183REPOS="$1"

Attachments (0)

Change History (15)

comment:1 by Ryan J Ollos, 3 years ago

Description: modified (diff)

comment:2 by Ryan J Ollos, 3 years ago

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

Last edited 2 years ago by Ryan J Ollos (previous) (diff)

comment:3 by Ryan J Ollos, 3 years ago

Milestone: 1.2.21.2.3

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

comment:4 by Ryan J Ollos, 2 years ago

Milestone: 1.2.31.3.3

comment:5 by Ryan J Ollos, 2 years ago

Release Notes: modified (diff)

comment:6 by Jun Omae, 2 years ago

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

comment:7 by Jun Omae, 2 years ago

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

comment:8 by Ryan J Ollos, 2 years ago

That's WIP, not ready for review.

comment:9 by Ryan J Ollos, 2 years ago

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].

Last edited 2 years ago by Ryan J Ollos (previous) (diff)

comment:10 by Ryan J Ollos, 2 years ago

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

    diff --git a/contrib/trac-svn-hook b/contrib/trac-svn-hook
    index 7a1416088..46d019a76 100755
    a b if ! python -V 2>/dev/null; then  
    191191fi
    192192
    193193if [ -z "$TRAC_ENV" ]; then
    194     echo "TRAC_ENV is not set."
     194    >&2 echo "TRAC_ENV is not set."
    195195    exit 2
    196196fi

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

comment:11 by Ryan J Ollos, 2 years ago

comment:12 by Ryan J Ollos, 2 years ago

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.

Last edited 2 years ago by Ryan J Ollos (previous) (diff)

comment:13 by Ryan J Ollos, 2 years ago

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.

comment:14 by Jun Omae, 2 years ago

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

comment:15 by Ryan J Ollos, 2 years ago

Resolution: fixed
Status: assignedclosed

Committed to trunk in r16342.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Ryan J Ollos.
The resolution will be deleted. Next status will be 'reopened'.
to as closed The owner will be changed from Ryan J Ollos to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.