On 4/1/19 10:25 PM, Thomas De Schampheleire wrote:
# HG changeset patch
# User Thomas De Schampheleire <[email protected]>
# Date 1554150088 -7200
# Mon Apr 01 22:21:28 2019 +0200
# Branch stable
# Node ID 83678cb1c0fa84473f686d94867c777c2671632b
# Parent 953047e8c88a9c5ccaa4ddc1bb417622de37f01f
hooks: fix potentially invalid interpreter in git hooks (Issue #333)
Commit 5e501b6ee639c2cf25080ba8c3d53f4e4b6ad32e introduced the use of
(Generally, I suggest using short 12 char hash prefixes for Mercurial.)
'sys.executable' as interpreter for git hooks instead of 'python2' with the
following argument:
"Windows doesn't necessarily have "python2" available in $PATH, but we
still want to make sure we don't end up invoking a python3. Using the
absolute path seems more safe."
But, sys.executable does not necessarily point to Python. When Kallithea is
started under uWSGI, sys.executable points to the uwsgi executable. As a
result, the git hooks did not work in this case:
$ git push
Password for 'http://user@localhost:5050':
Enumerating objects: 3, done.
Counting objects: 100% (3/3), done.
Writing objects: 100% (3/3), 241 bytes | 241.00 KiB/s, done.
Total 3 (delta 0), reused 0 (delta 0)
remote: unable to load configuration from hooks/pre-receive
To http://localhost:5050/gitrepo-new
! [remote rejected] master -> master (pre-receive hook declined)
error: failed to push some refs to 'http://user@localhost:5050/gitrepo-new'
Perhaps more interesting than this high level failure, mention what
hash-bang line would be inserted into the hook scripts.
Try to determine a better interpreter for the git hooks by covering multiple
cases.
In the case of Windows without virtualenv, we are not 100% sure that the
value will be valid. More heuristics could be added there, but if we can
assume that most installations on Windows will effectively use a virtualenv,
which have their own handling in the new code, it may not be worth the
complexity.
The heuristics can probably never be perfect. Should we have a .ini
configure option or environment variable (PYTHON_SYS_EXECUTABLE?) for
overriding it?
diff --git a/kallithea/model/scm.py b/kallithea/model/scm.py
--- a/kallithea/model/scm.py
+++ b/kallithea/model/scm.py
@@ -720,6 +720,30 @@ class ScmModel(object):
return choices, hist_l
+ def _get_python_executable(self):
+ """Return a Python executable for use in hooks
+
+ This is not simply sys.executable as that points to uwsgi if using
+ uwsgi.
+
+ The returned string is valid when run in the current PATH, it is not
+ necessarily a full path by itself.
+ """
+ # If we are running in a virtualenv, the python in the PATH is the
+ # right one
+ if 'VIRTUAL_ENV' in os.environ:
+ return 'python'
+
+ # On Windows, there is no 'python2' executable. We'll use 'python' and
+ # hope that the user has set their PATH correctly so it points to
+ # Python 2, not Python 3.
+ if kallithea.is_windows:
+ return 'python'
+
+ # On Unix, outside of a virtualenv, there should be a 'python2'
+ # executable in the PATH
+ return 'python2'
Perhaps also see Mercurial `_usecorrectpython` and `hgcmd` and
`sysexecutable` and `PYTHON_SYS_EXECUTABLE`. But I guess they don't add
much info.
This is something that will have to be revisited when switching to
Python3, but probably hard to be forward-compatible already.
def install_git_hooks(self, repo, force_create=False):
"""
Creates a kallithea hook inside a git repository
@@ -734,11 +758,11 @@ class ScmModel(object):
if not os.path.isdir(loc):
os.makedirs(loc)
- tmpl_post = "#!/usr/bin/env %s\n" % sys.executable or 'python2'
+ tmpl_post = "#!/usr/bin/env %s\n" % self._get_python_executable()
It seems a bit brutal to just ignore sys.executable completely while
moving code out. (Perhaps first add the wrapper function without
changing the heuristics - then change the heuristics with a simple,
explicit and readable change.
This change should probably also be tested on Windows - I am not sure it
worked correctly before in most common Windows setups. (Where we
probably must assume that there is no .cmd wrapper script ...)
Other than that (and without any blockers here), this series looks good
to me.
/Mads
_______________________________________________
kallithea-general mailing list
[email protected]
https://lists.sfconservancy.org/mailman/listinfo/kallithea-general
