Re: [PATCH 6 of 6 v3] hooks: use /usr/bin/env only when needed
On 4/8/19 10:14 PM, Thomas De Schampheleire wrote:
# HG changeset patch
# User Thomas De Schampheleire
# Date 1554753972 -7200
# Mon Apr 08 22:06:12 2019 +0200
# Branch stable
# Node ID d02dee3a34716ea8930e366595e613572a3783e7
# Parent cb33fa0fa1551384df6e2f4ab9ddd00d8567c69d
hooks: use /usr/bin/env only when needed
The use of /usr/bin/env is only needed for relative arguments (or to pass
variables in the environment, which we don't do). It is thus not needed in
case the Git hook interpreter is obtained via the ini file, or via
sys.executable (both of which are expected to be absolute paths).
It would perhaps be "cleaner" to have this change first in the series.
But no big deal.
Either way: The serious looks good to me. Especially if you consider the
other comments and agree to leave out "scripts/generate-ini: also allow
setting mako variables".
/Mads
diff --git a/kallithea/model/scm.py b/kallithea/model/scm.py
--- a/kallithea/model/scm.py
+++ b/kallithea/model/scm.py
@@ -733,7 +733,9 @@ class ScmModel(object):
FIXME This may not work on Windows and may need a shell wrapper
script.
To be revisited later...
"""
-return kallithea.CONFIG.get('git_hook_interpreter') or sys.executable
or 'python2'
+return (kallithea.CONFIG.get('git_hook_interpreter')
+or sys.executable
+or '/usr/bin/env python2')
def install_git_hooks(self, repo, force_create=False):
"""
@@ -749,11 +751,11 @@ class ScmModel(object):
if not os.path.isdir(loc):
os.makedirs(loc)
-tmpl_post = "#!/usr/bin/env %s\n" % self._get_git_hook_interpreter()
+tmpl_post = "#!%s\n" % self._get_git_hook_interpreter()
tmpl_post += pkg_resources.resource_string(
'kallithea', os.path.join('config', 'post_receive_tmpl.py')
)
-tmpl_pre = "#!/usr/bin/env %s\n" % self._get_git_hook_interpreter()
+tmpl_pre = "#!%s\n" % self._get_git_hook_interpreter()
tmpl_pre += pkg_resources.resource_string(
'kallithea', os.path.join('config', 'pre_receive_tmpl.py')
)
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
https://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: [PATCH 3 of 6 v3] scripts/generate-ini: also allow setting mako variables
On 4/8/19 10:14 PM, Thomas De Schampheleire wrote:
# HG changeset patch
# User Thomas De Schampheleire
# Date 1554322593 -7200
# Wed Apr 03 22:16:33 2019 +0200
# Branch stable
# Node ID 90f704f5372af599838b4a8171790eeb89c9749f
# Parent fa06015a39a0d757790a334207b4066beefee64f
scripts/generate-ini: also allow setting mako variables
The current code only allowed to set custom textual values, not variables
used for evaluation by mako. This will be needed in a subsequent commit.
But we don't need this at all, after introducing "%if
git_hook_interpreter:"? If not specified, the value will be false, and
the .ini will not contain that setting.
diff --git a/scripts/generate-ini.py b/scripts/generate-ini.py
--- a/scripts/generate-ini.py
+++ b/scripts/generate-ini.py
@@ -47,6 +47,8 @@ ini_files = [
'level': 'DEBUG',
},
},
+{
+},
),
]
@@ -63,9 +65,9 @@ def main():
open(makofile, 'w').write(mako_marked_up)
# create ini files
-for fn, settings in ini_files:
+for fn, settings, mako_variables in ini_files:
print 'updating:', fn
-inifile.create(fn, None, settings)
+inifile.create(fn, mako_variables, settings)
if __name__ == '__main__':
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
https://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: [PATCH 2 of 6 v3] ini: introduce setting 'git_hook_interpreter'
On 4/8/19 10:14 PM, Thomas De Schampheleire wrote:
# HG changeset patch
# User Thomas De Schampheleire
# Date 1554751977 -7200
# Mon Apr 08 21:32:57 2019 +0200
# Branch stable
# Node ID fa06015a39a0d757790a334207b4066beefee64f
# Parent a4ad7b50bab39a528ac147ae3f50791434313127
ini: introduce setting 'git_hook_interpreter'
As preparation to fix issues with Git hooks that cannot be executed in all
cases due to an incorrect interpreter (issue #333), introduce a
configuration setting 'git_hook_interpreter'.
A subsequent commit will cause its value to be filled in automatically when
generating a new ini file, but an administrator can always override it.
diff --git a/development.ini b/development.ini
diff --git a/kallithea/lib/paster_commands/template.ini.mako
b/kallithea/lib/paster_commands/template.ini.mako
--- a/kallithea/lib/paster_commands/template.ini.mako
+++ b/kallithea/lib/paster_commands/template.ini.mako
@@ -211,6 +211,17 @@ use_htsts = false
<%text>## number of commits stats will parse on each iteration
commit_parse_limit = 25
+<%text>## Path to Python executable to be used for git hooks.
+<%text>## This value will be written inside the git hook scripts as the
text
+<%text>## after '#!' (shebang). When empty or not defined, the value of
+<%text>## 'sys.executable' at the time of installation of the git hooks
is
+<%text>## used, which is correct in many cases but not when using
uwsgi.
I think this should be "for example not when using uwsgi"
Also: Should this perhaps already give an example for Windows (and/or
mention why not and what to do)?
+<%text>## FIXME this setting is not yet used.
+# git_hook_interpreter = /srv/kallithea/venv/bin/python2
Hmm. That is *the* python interpreter to use. Do we really want to
constrain the name to only be for hooks, and then introduce a config use
using another name when/if we need it for other purposes?
Perhaps. Or perhaps name it `python_interpreter` instead. I just would
like it to be deliberate decision.
+%if git_hook_interpreter:
+git_hook_interpreter = ${git_hook_interpreter}
+%endif
+
<%text>## path to git executable
git_path = git
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
https://lists.sfconservancy.org/mailman/listinfo/kallithea-general
