* [PATCH v3 0/4] Hand the repo configuration to the filters
@ 2011-03-23 10:57 mailings
2011-03-23 10:57 ` [PATCH v4 1/5] new_filter: determine extra_args from filter type mailings
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: mailings @ 2011-03-23 10:57 UTC (permalink / raw)
From: Ferry Huberts <ferry.huberts at pelagic.nl>
Changes compared to v3:
- Incorporated feedback from Lars
Changes compared to v2:
- Split out the first 2 patches into a seperate thread
- Incorporated feedback from Lars
Changes compared to v1:
- Rewritten to hand the repo configuration via environment variables
I have a server setup in which each repo has a trac instance and
for the commit filter I really need to know with which repo I'm
dealing in order to be able to resolve the #123 ticket numbers
into hyperlinks into the correct trac instance.
Lars, patch 3 has become quite a bit more complex. However, adding
environment variables is a matter of simply extending the
env_vars array in the prepare_env function; only the code
that processes that array has become a bit more complex.
Ferry Huberts (5):
new_filter: determine extra_args from filter type
cgit_open_filter: also take the repo as a parameter
cgit_open_filter: hand down repo configuration to script
filters: document environment variables in filter scripts
debug logging
cgit.c | 27 +++++++--
cgit.h | 6 ++-
cgitrc.5.txt | 47 ++++++++++++++--
filters/commit-links.sh | 11 ++++
filters/syntax-highlighting.sh | 11 ++++
shared.c | 119 +++++++++++++++++++++++++++++++++++++++-
ui-commit.c | 6 +-
ui-repolist.c | 2 +-
ui-snapshot.c | 2 +-
ui-summary.c | 2 +-
ui-tree.c | 2 +-
11 files changed, 213 insertions(+), 22 deletions(-)
--
1.7.4
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 1/5] new_filter: determine extra_args from filter type
2011-03-23 10:57 [PATCH v3 0/4] Hand the repo configuration to the filters mailings
@ 2011-03-23 10:57 ` mailings
2011-04-29 14:28 ` mailings
2011-03-23 10:57 ` [PATCH v4 2/5] cgit_open_filter: also take the repo as a parameter mailings
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: mailings @ 2011-03-23 10:57 UTC (permalink / raw)
From: Ferry Huberts <ferry.huberts at pelagic.nl>
Currently the number of extra arguments is linked hard to the type of
the filter. This is also logical since it would be confusing to have
a different number of arguments for the same type of filter depending
on the context under which the filter is run (unless ofcourse one the
parameters would make the context clear, which is currently not the
case).
Signed-off-by: Ferry Huberts <ferry.huberts at pelagic.nl>
---
cgit.c | 27 ++++++++++++++++++++-------
cgit.h | 4 ++++
2 files changed, 24 insertions(+), 7 deletions(-)
diff --git a/cgit.c b/cgit.c
index f4dd6ef..5d6e488 100644
--- a/cgit.c
+++ b/cgit.c
@@ -26,13 +26,26 @@ void add_mimetype(const char *name, const char *value)
item->util = xstrdup(value);
}
-struct cgit_filter *new_filter(const char *cmd, int extra_args)
+struct cgit_filter *new_filter(const char *cmd, filter_type filtertype)
{
struct cgit_filter *f;
+ int extra_args;
if (!cmd || !cmd[0])
return NULL;
+ switch (filtertype) {
+ case SOURCE:
+ extra_args = 1;
+ break;
+
+ case ABOUT:
+ case COMMIT:
+ default:
+ extra_args = 0;
+ break;
+ }
+
f = xmalloc(sizeof(struct cgit_filter));
f->cmd = xstrdup(cmd);
f->argv = xmalloc((2 + extra_args) * sizeof(char *));
@@ -81,11 +94,11 @@ void repo_config(struct cgit_repo *repo, const char *name, const char *value)
repo->logo_link = xstrdup(value);
else if (ctx.cfg.enable_filter_overrides) {
if (!strcmp(name, "about-filter"))
- repo->about_filter = new_filter(value, 0);
+ repo->about_filter = new_filter(value, ABOUT);
else if (!strcmp(name, "commit-filter"))
- repo->commit_filter = new_filter(value, 0);
+ repo->commit_filter = new_filter(value, COMMIT);
else if (!strcmp(name, "source-filter"))
- repo->source_filter = new_filter(value, 1);
+ repo->source_filter = new_filter(value, SOURCE);
}
}
@@ -176,9 +189,9 @@ void config_cb(const char *name, const char *value)
else if (!strcmp(name, "cache-dynamic-ttl"))
ctx.cfg.cache_dynamic_ttl = atoi(value);
else if (!strcmp(name, "about-filter"))
- ctx.cfg.about_filter = new_filter(value, 0);
+ ctx.cfg.about_filter = new_filter(value, ABOUT);
else if (!strcmp(name, "commit-filter"))
- ctx.cfg.commit_filter = new_filter(value, 0);
+ ctx.cfg.commit_filter = new_filter(value, COMMIT);
else if (!strcmp(name, "embedded"))
ctx.cfg.embedded = atoi(value);
else if (!strcmp(name, "max-atom-items"))
@@ -208,7 +221,7 @@ void config_cb(const char *name, const char *value)
else if (!strcmp(name, "section-from-path"))
ctx.cfg.section_from_path = atoi(value);
else if (!strcmp(name, "source-filter"))
- ctx.cfg.source_filter = new_filter(value, 1);
+ ctx.cfg.source_filter = new_filter(value, SOURCE);
else if (!strcmp(name, "summary-log"))
ctx.cfg.summary_log = atoi(value);
else if (!strcmp(name, "summary-branches"))
diff --git a/cgit.h b/cgit.h
index b5f00fc..1f8b1be 100644
--- a/cgit.h
+++ b/cgit.h
@@ -51,6 +51,10 @@ typedef void (*configfn)(const char *name, const char *value);
typedef void (*filepair_fn)(struct diff_filepair *pair);
typedef void (*linediff_fn)(char *line, int len);
+typedef enum {
+ ABOUT, COMMIT, SOURCE
+} filter_type;
+
struct cgit_filter {
char *cmd;
char **argv;
--
1.7.4
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 2/5] cgit_open_filter: also take the repo as a parameter
2011-03-23 10:57 [PATCH v3 0/4] Hand the repo configuration to the filters mailings
2011-03-23 10:57 ` [PATCH v4 1/5] new_filter: determine extra_args from filter type mailings
@ 2011-03-23 10:57 ` mailings
2011-03-23 10:57 ` [PATCH v4 3/5] cgit_open_filter: hand down repo configuration to script mailings
` (2 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: mailings @ 2011-03-23 10:57 UTC (permalink / raw)
From: Ferry Huberts <ferry.huberts at pelagic.nl>
To prepare for handing repo configuration to the
filter script that is executed.
Signed-off-by: Ferry Huberts <ferry.huberts at pelagic.nl>
---
cgit.h | 2 +-
shared.c | 2 +-
ui-commit.c | 6 +++---
ui-repolist.c | 2 +-
ui-snapshot.c | 2 +-
ui-summary.c | 2 +-
ui-tree.c | 2 +-
7 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/cgit.h b/cgit.h
index 1f8b1be..3b0eaf5 100644
--- a/cgit.h
+++ b/cgit.h
@@ -318,7 +318,7 @@ extern const char *cgit_repobasename(const char *reponame);
extern int cgit_parse_snapshots_mask(const char *str);
-extern int cgit_open_filter(struct cgit_filter *filter);
+extern int cgit_open_filter(struct cgit_filter *filter, struct cgit_repo * repo);
extern int cgit_close_filter(struct cgit_filter *filter);
extern int readfile(const char *path, char **buf, size_t *size);
diff --git a/shared.c b/shared.c
index 7ec2e19..e4879df 100644
--- a/shared.c
+++ b/shared.c
@@ -376,7 +376,7 @@ int cgit_parse_snapshots_mask(const char *str)
return rv;
}
-int cgit_open_filter(struct cgit_filter *filter)
+int cgit_open_filter(struct cgit_filter *filter, struct cgit_repo * repo)
{
filter->old_stdout = chk_positive(dup(STDOUT_FILENO),
diff --git a/ui-commit.c b/ui-commit.c
index 2b4f677..2da9fcf 100644
--- a/ui-commit.c
+++ b/ui-commit.c
@@ -110,7 +110,7 @@ void cgit_print_commit(char *hex, const char *prefix)
html("</table>\n");
html("<div class='commit-subject'>");
if (ctx.repo->commit_filter)
- cgit_open_filter(ctx.repo->commit_filter);
+ cgit_open_filter(ctx.repo->commit_filter, ctx.repo);
html_txt(info->subject);
if (ctx.repo->commit_filter)
cgit_close_filter(ctx.repo->commit_filter);
@@ -118,7 +118,7 @@ void cgit_print_commit(char *hex, const char *prefix)
html("</div>");
html("<div class='commit-msg'>");
if (ctx.repo->commit_filter)
- cgit_open_filter(ctx.repo->commit_filter);
+ cgit_open_filter(ctx.repo->commit_filter, ctx.repo);
html_txt(info->msg);
if (ctx.repo->commit_filter)
cgit_close_filter(ctx.repo->commit_filter);
@@ -127,7 +127,7 @@ void cgit_print_commit(char *hex, const char *prefix)
html("<div class='notes-header'>Notes</div>");
html("<div class='notes'>");
if (ctx.repo->commit_filter)
- cgit_open_filter(ctx.repo->commit_filter);
+ cgit_open_filter(ctx.repo->commit_filter, ctx.repo);
html_txt(notes.buf);
if (ctx.repo->commit_filter)
cgit_close_filter(ctx.repo->commit_filter);
diff --git a/ui-repolist.c b/ui-repolist.c
index 2c98668..05b4548 100644
--- a/ui-repolist.c
+++ b/ui-repolist.c
@@ -291,7 +291,7 @@ void cgit_print_site_readme()
if (!ctx.cfg.root_readme)
return;
if (ctx.cfg.about_filter)
- cgit_open_filter(ctx.cfg.about_filter);
+ cgit_open_filter(ctx.cfg.about_filter, NULL);
html_include(ctx.cfg.root_readme);
if (ctx.cfg.about_filter)
cgit_close_filter(ctx.cfg.about_filter);
diff --git a/ui-snapshot.c b/ui-snapshot.c
index 6e3412c..067082c 100644
--- a/ui-snapshot.c
+++ b/ui-snapshot.c
@@ -19,7 +19,7 @@ static int write_compressed_tar_archive(struct archiver_args *args,const char *f
f.argv = malloc(2 * sizeof(char *));
f.argv[0] = f.cmd;
f.argv[1] = NULL;
- cgit_open_filter(&f);
+ cgit_open_filter(&f, NULL);
rv = write_tar_archive(args);
cgit_close_filter(&f);
return rv;
diff --git a/ui-summary.c b/ui-summary.c
index 5be2545..1e9a1b6 100644
--- a/ui-summary.c
+++ b/ui-summary.c
@@ -113,7 +113,7 @@ void cgit_print_repo_readme(char *path)
*/
html("<div id='summary'>");
if (ctx.repo->about_filter)
- cgit_open_filter(ctx.repo->about_filter);
+ cgit_open_filter(ctx.repo->about_filter, ctx.repo);
if (ref)
cgit_print_file(tmp, ref);
else
diff --git a/ui-tree.c b/ui-tree.c
index 0b1b531..835c166 100644
--- a/ui-tree.c
+++ b/ui-tree.c
@@ -45,7 +45,7 @@ static void print_text_buffer(const char *name, char *buf, unsigned long size)
if (ctx.repo->source_filter) {
html("<td class='lines'><pre><code>");
ctx.repo->source_filter->argv[1] = xstrdup(name);
- cgit_open_filter(ctx.repo->source_filter);
+ cgit_open_filter(ctx.repo->source_filter, ctx.repo);
html_raw(buf, size);
cgit_close_filter(ctx.repo->source_filter);
html("</code></pre></td></tr></table>\n");
--
1.7.4
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 3/5] cgit_open_filter: hand down repo configuration to script
2011-03-23 10:57 [PATCH v3 0/4] Hand the repo configuration to the filters mailings
2011-03-23 10:57 ` [PATCH v4 1/5] new_filter: determine extra_args from filter type mailings
2011-03-23 10:57 ` [PATCH v4 2/5] cgit_open_filter: also take the repo as a parameter mailings
@ 2011-03-23 10:57 ` mailings
2011-03-23 10:57 ` [PATCH v4 4/5] filters: document environment variables in filter scripts mailings
2011-03-26 10:48 ` [PATCH v3 0/4] Hand the repo configuration to the filters hjemli
4 siblings, 0 replies; 10+ messages in thread
From: mailings @ 2011-03-23 10:57 UTC (permalink / raw)
From: Ferry Huberts <ferry.huberts at pelagic.nl>
The environment variables can be used to (for example) resolve
the following situation:
Suppose a server setup in which each repository has a trac
instance; the commit filter needs to know with which
repository it's dealing in order to be able to resolve the
#123 ticket numbers in the commit messages into hyperlinks
into the correct trac instance.
Signed-off-by: Ferry Huberts <ferry.huberts at pelagic.nl>
---
shared.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 77 insertions(+), 1 deletions(-)
diff --git a/shared.c b/shared.c
index e4879df..8c3d18a 100644
--- a/shared.c
+++ b/shared.c
@@ -7,6 +7,8 @@
*/
#include "cgit.h"
+#include <stdio.h>
+#include <linux/limits.h>
struct cgit_repolist cgit_repolist;
struct cgit_context ctx;
@@ -376,6 +378,70 @@ int cgit_parse_snapshots_mask(const char *str)
return rv;
}
+typedef struct {
+ char * name;
+ char * value;
+} cgit_env_var;
+
+static char * prepare_env(struct cgit_repo * repo) {
+ cgit_env_var env_vars[] = {
+ { .name = "CGIT_REPO_URL", .value = repo->url },
+ { .name = "CGIT_REPO_NAME", .value = repo->name },
+ { .name = "CGIT_REPO_PATH", .value = repo->path },
+ { .name = "CGIT_REPO_OWNER", .value = repo->owner },
+ { .name = "CGIT_REPO_DEFBRANCH", .value = repo->defbranch },
+ { .name = "CGIT_REPO_SECTION", .value = repo->section },
+ { .name = "CGIT_REPO_CLONE_URL", .value = repo->clone_url }
+ };
+ int env_var_count = ARRAY_SIZE(env_vars);
+ long values_space = (env_var_count * (PATH_MAX + 64));
+
+ void * buffer;
+ char ** vars;
+ char * values;
+ int vars_index = 0;
+ unsigned int chars_printed;
+
+ /* Allocate buffer for environment variables: first in the buffer is an
+ * array of pointers to argument strings, terminated with a NULL pointer.
+ * After that the argument strings are placed after each other */
+ buffer = malloc(((env_var_count + 1) * sizeof(char *)) + values_space);
+ if (!buffer)
+ return NULL;
+
+ vars = buffer;
+ values = (char *) &vars[env_var_count + 1];
+
+ /* loop over all defined environment variables and their values */
+ while (vars_index < env_var_count) {
+ char * name = env_vars[vars_index].name;
+ char * value = env_vars[vars_index].value;
+
+ if (!value)
+ value = "";
+
+ chars_printed = snprintf(values, (values_space - 1), "%s=%s", name,
+ value);
+ if (chars_printed > (values_space - 1)) {
+ /* Buffer space exhausted: stop adding variables.
+ * Not all environment variables are defined, but the best we can
+ * do is to provide the ones that _are_ defined */
+ break;
+ }
+
+ values[chars_printed] = '\0';
+ *&vars[vars_index] = values;
+ values += (chars_printed + 1);
+ values_space -= (chars_printed + 1);
+ vars_index++;
+ }
+
+ /* terminate the array with pointers */
+ *&vars[vars_index] = NULL;
+
+ return (char *) buffer;
+}
+
int cgit_open_filter(struct cgit_filter *filter, struct cgit_repo * repo)
{
@@ -384,10 +450,20 @@ int cgit_open_filter(struct cgit_filter *filter, struct cgit_repo * repo)
chk_zero(pipe(filter->pipe_fh), "Unable to create pipe to subprocess");
filter->pid = chk_non_negative(fork(), "Unable to create subprocess");
if (filter->pid == 0) {
+ char * env = NULL;
+
close(filter->pipe_fh[1]);
chk_non_negative(dup2(filter->pipe_fh[0], STDIN_FILENO),
"Unable to use pipe as STDIN");
- execvp(filter->cmd, filter->argv);
+
+ if (repo)
+ env = prepare_env(repo);
+
+ execve(filter->cmd, filter->argv, (char **)env);
+
+ if (env)
+ free(env);
+
die("Unable to exec subprocess %s: %s (%d)", filter->cmd,
strerror(errno), errno);
}
--
1.7.4
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 4/5] filters: document environment variables in filter scripts
2011-03-23 10:57 [PATCH v3 0/4] Hand the repo configuration to the filters mailings
` (2 preceding siblings ...)
2011-03-23 10:57 ` [PATCH v4 3/5] cgit_open_filter: hand down repo configuration to script mailings
@ 2011-03-23 10:57 ` mailings
2011-03-26 10:48 ` [PATCH v3 0/4] Hand the repo configuration to the filters hjemli
4 siblings, 0 replies; 10+ messages in thread
From: mailings @ 2011-03-23 10:57 UTC (permalink / raw)
From: Ferry Huberts <ferry.huberts at pelagic.nl>
Signed-off-by: Ferry Huberts <ferry.huberts at pelagic.nl>
---
cgitrc.5.txt | 47 +++++++++++++++++++++++++++++++++++----
filters/commit-links.sh | 11 +++++++++
filters/syntax-highlighting.sh | 11 +++++++++
3 files changed, 64 insertions(+), 5 deletions(-)
diff --git a/cgitrc.5.txt b/cgitrc.5.txt
index c3698a6..60539d7 100644
--- a/cgitrc.5.txt
+++ b/cgitrc.5.txt
@@ -31,7 +31,7 @@ about-filter::
about pages (both top-level and for each repository). The command will
get the content of the about-file on its STDIN, and the STDOUT from the
command will be included verbatim on the about page. Default value:
- none.
+ none. See also: "FILTER API".
agefile::
Specifies a path, relative to each repository path, which can be used
@@ -81,6 +81,7 @@ commit-filter::
The command will get the message on its STDIN, and the STDOUT from the
command will be included verbatim as the commit message, i.e. this can
be used to implement bugtracker integration. Default value: none.
+ See also: "FILTER API".
css::
Url which specifies the css document to include in all cgit pages.
@@ -316,7 +317,7 @@ source-filter::
and the name of the blob as its only command line argument. The STDOUT
from the command will be included verbatim as the blob contents, i.e.
this can be used to implement e.g. syntax highlighting. Default value:
- none.
+ none. See also: "FILTER API".
summary-branches::
Specifies the number of branches to display in the repository "summary"
@@ -349,7 +350,7 @@ REPOSITORY SETTINGS
-------------------
repo.about-filter::
Override the default about-filter. Default value: none. See also:
- "enable-filter-overrides".
+ "enable-filter-overrides". See also: "FILTER API".
repo.clone-url::
A list of space-separated urls which can be used to clone this repo.
@@ -357,7 +358,7 @@ repo.clone-url::
repo.commit-filter::
Override the default commit-filter. Default value: none. See also:
- "enable-filter-overrides".
+ "enable-filter-overrides". See also: "FILTER API".
repo.defbranch::
The name of the default branch for this repository. If no such branch
@@ -428,7 +429,7 @@ repo.section::
repo.source-filter::
Override the default source-filter. Default value: none. See also:
- "enable-filter-overrides".
+ "enable-filter-overrides". See also: "FILTER API".
repo.url::
The relative url used to access the repository. This must be the first
@@ -448,6 +449,42 @@ Note: the "repo." prefix is dropped from the option names in repo-specific
config files, e.g. "repo.desc" becomes "desc".
+FILTER API
+----------
+- about filter::
+ This filter is given no arguments.
+ The about text that is to be filtered is available on standard input and the
+ filtered text is expected on standard output.
+- commit filter::
+ This filter is given no arguments.
+ The commit message text that is to be filtered is available on standard input
+ and the filtered text is expected on standard output.
+- source filter::
+ This filter is given a single parameter: the filename of the source file to
+ filter. The filter can use the filename to determine (for example) the syntax
+ highlighting mode.
+ The contents of the source file that is to be filtered is available on
+ standard input and the filtered contents is expected on standard output.
+
+Also, all filters are handed the following environment variables:
+- CGIT_REPO_URL ( = repo.url setting )
+- CGIT_REPO_NAME ( = repo.name setting )
+- CGIT_REPO_PATH ( = repo.path setting )
+- CGIT_REPO_OWNER ( = repo.owner setting )
+- CGIT_REPO_DEFBRANCH ( = repo.defbranch setting )
+- CGIT_REPO_SECTION ( = section setting )
+- CGIT_REPO_CLONE_URL ( = repo.clone-url setting )
+
+If a setting is not defined for a repository and the corresponding global
+setting is also not defined (if applicable), then the corresponding
+environment variable will be an empty string.
+
+Note that under normal circumstance all these environment variables are
+defined. If however the total size of the defined settings exceed the
+allocated buffer within cgit then only the environment variables that fit
+in the allocated buffer are handed to the filter.
+
+
EXAMPLE CGITRC FILE
-------------------
diff --git a/filters/commit-links.sh b/filters/commit-links.sh
index 110c609..d2cd2b3 100755
--- a/filters/commit-links.sh
+++ b/filters/commit-links.sh
@@ -3,6 +3,17 @@
#
# To use this script, refer to this file with either the commit-filter or the
# repo.commit-filter options in cgitrc.
+#
+# The following environment variables can be used to retrieve the configuration
+# of the repository for which this script is called:
+# CGIT_REPO_URL ( = repo.url setting )
+# CGIT_REPO_NAME ( = repo.name setting )
+# CGIT_REPO_PATH ( = repo.path setting )
+# CGIT_REPO_OWNER ( = repo.owner setting )
+# CGIT_REPO_DEFBRANCH ( = repo.defbranch setting )
+# CGIT_REPO_SECTION ( = section setting )
+# CGIT_REPO_CLONE_URL ( = repo.clone-url setting )
+#
# This expression generates links to commits referenced by their SHA1.
regex=$regex'
diff --git a/filters/syntax-highlighting.sh b/filters/syntax-highlighting.sh
index 6b1c576..6283ce9 100755
--- a/filters/syntax-highlighting.sh
+++ b/filters/syntax-highlighting.sh
@@ -23,6 +23,17 @@
# table.blob .kwb { color:#830000; }
# table.blob .kwc { color:#000000; font-weight:bold; }
# table.blob .kwd { color:#010181; }
+#
+# The following environment variables can be used to retrieve the configuration
+# of the repository for which this script is called:
+# CGIT_REPO_URL ( = repo.url setting )
+# CGIT_REPO_NAME ( = repo.name setting )
+# CGIT_REPO_PATH ( = repo.path setting )
+# CGIT_REPO_OWNER ( = repo.owner setting )
+# CGIT_REPO_DEFBRANCH ( = repo.defbranch setting )
+# CGIT_REPO_SECTION ( = section setting )
+# CGIT_REPO_CLONE_URL ( = repo.clone-url setting )
+#
# store filename and extension in local vars
BASENAME="$1"
--
1.7.4
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 0/4] Hand the repo configuration to the filters
2011-03-23 10:57 [PATCH v3 0/4] Hand the repo configuration to the filters mailings
` (3 preceding siblings ...)
2011-03-23 10:57 ` [PATCH v4 4/5] filters: document environment variables in filter scripts mailings
@ 2011-03-26 10:48 ` hjemli
2011-03-26 10:52 ` hjemli
2011-03-26 11:45 ` mailings
4 siblings, 2 replies; 10+ messages in thread
From: hjemli @ 2011-03-26 10:48 UTC (permalink / raw)
On Wed, Mar 23, 2011 at 11:57, Ferry Huberts <mailings at hupie.com> wrote:
> Ferry Huberts (5):
> ?new_filter: determine extra_args from filter type
> ?cgit_open_filter: also take the repo as a parameter
> ?cgit_open_filter: hand down repo configuration to script
> ?filters: document environment variables in filter scripts
Thanks. I've applied this to my wip branch for now with the attached
patch on top, since the switch from execvp() to execve() made the
snapshot feature stop working (make test reported "Unable to exec
subprocess gzip").
--
larsh
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 0/4] Hand the repo configuration to the filters
2011-03-26 10:48 ` [PATCH v3 0/4] Hand the repo configuration to the filters hjemli
@ 2011-03-26 10:52 ` hjemli
2011-03-26 11:45 ` mailings
1 sibling, 0 replies; 10+ messages in thread
From: hjemli @ 2011-03-26 10:52 UTC (permalink / raw)
[* possibly gmail-wrapped patch *]
From c2b58ed8539fcfa4f05cc9da316bbc782f4b8f10 Mon Sep 17 00:00:00 2001
From: Lars Hjemli <hjemli at gmail.com>
Date: Sat, 26 Mar 2011 11:22:35 +0100
Subject: [PATCH] shared.c: use execvp() to execute filter commands
This reintroduces the use of execvp(), since the filter commands doesn't
always contain an absolute path (i.e. snapshot compression filters).
Signed-off-by: Lars Hjemli <hjemli at gmail.com>
---
shared.c | 68 ++++++++++---------------------------------------------------
1 files changed, 11 insertions(+), 57 deletions(-)
diff --git a/shared.c b/shared.c
index 8c3d18a..986f965 100644
--- a/shared.c
+++ b/shared.c
@@ -383,7 +383,7 @@ typedef struct {
char * value;
} cgit_env_var;
-static char * prepare_env(struct cgit_repo * repo) {
+static void prepare_env(struct cgit_repo * repo) {
cgit_env_var env_vars[] = {
{ .name = "CGIT_REPO_URL", .value = repo->url },
{ .name = "CGIT_REPO_NAME", .value = repo->name },
@@ -394,52 +394,14 @@ static char * prepare_env(struct cgit_repo * repo) {
{ .name = "CGIT_REPO_CLONE_URL", .value = repo->clone_url }
};
int env_var_count = ARRAY_SIZE(env_vars);
- long values_space = (env_var_count * (PATH_MAX + 64));
-
- void * buffer;
- char ** vars;
- char * values;
- int vars_index = 0;
- unsigned int chars_printed;
-
- /* Allocate buffer for environment variables: first in the buffer is an
- * array of pointers to argument strings, terminated with a
NULL pointer.
- * After that the argument strings are placed after each other */
- buffer = malloc(((env_var_count + 1) * sizeof(char *)) + values_space);
- if (!buffer)
- return NULL;
-
- vars = buffer;
- values = (char *) &vars[env_var_count + 1];
-
- /* loop over all defined environment variables and their values */
- while (vars_index < env_var_count) {
- char * name = env_vars[vars_index].name;
- char * value = env_vars[vars_index].value;
-
- if (!value)
- value = "";
-
- chars_printed = snprintf(values, (values_space - 1),
"%s=%s", name,
- value);
- if (chars_printed > (values_space - 1)) {
- /* Buffer space exhausted: stop adding variables.
- * Not all environment variables are defined,
but the best we can
- * do is to provide the ones that _are_ defined */
- break;
- }
-
- values[chars_printed] = '\0';
- *&vars[vars_index] = values;
- values += (chars_printed + 1);
- values_space -= (chars_printed + 1);
- vars_index++;
- }
-
- /* terminate the array with pointers */
- *&vars[vars_index] = NULL;
-
- return (char *) buffer;
+ cgit_env_var *p, *q;
+ static char *warn = "cgit warning: failed to set env: %s=%s\n";
+
+ p = env_vars;
+ q = p + env_var_count;
+ for (; p < q; p++)
+ if (setenv(p->name, p->value, 1))
+ fprintf(stderr, warn, p->name, p->value);
}
int cgit_open_filter(struct cgit_filter *filter, struct cgit_repo * repo)
@@ -450,20 +412,12 @@ int cgit_open_filter(struct cgit_filter *filter,
struct cgit_repo * repo)
chk_zero(pipe(filter->pipe_fh), "Unable to create pipe to subprocess");
filter->pid = chk_non_negative(fork(), "Unable to create subprocess");
if (filter->pid == 0) {
- char * env = NULL;
-
close(filter->pipe_fh[1]);
chk_non_negative(dup2(filter->pipe_fh[0], STDIN_FILENO),
"Unable to use pipe as STDIN");
-
if (repo)
- env = prepare_env(repo);
-
- execve(filter->cmd, filter->argv, (char **)env);
-
- if (env)
- free(env);
-
+ prepare_env(repo);
+ execvp(filter->cmd, filter->argv);
die("Unable to exec subprocess %s: %s (%d)", filter->cmd,
strerror(errno), errno);
}
--
1.7.4.15.g7811d
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 0/4] Hand the repo configuration to the filters
2011-03-26 10:48 ` [PATCH v3 0/4] Hand the repo configuration to the filters hjemli
2011-03-26 10:52 ` hjemli
@ 2011-03-26 11:45 ` mailings
2011-03-26 12:50 ` hjemli
1 sibling, 1 reply; 10+ messages in thread
From: mailings @ 2011-03-26 11:45 UTC (permalink / raw)
On 03/26/2011 11:48 AM, Lars Hjemli wrote:
> On Wed, Mar 23, 2011 at 11:57, Ferry Huberts <mailings at hupie.com> wrote:
>> Ferry Huberts (5):
>> new_filter: determine extra_args from filter type
>> cgit_open_filter: also take the repo as a parameter
>> cgit_open_filter: hand down repo configuration to script
>> filters: document environment variables in filter scripts
>
> Thanks. I've applied this to my wip branch for now with the attached
> patch on top, since the switch from execvp() to execve() made the
> snapshot feature stop working (make test reported "Unable to exec
> subprocess gzip").
>
> --
> larsh
looks ok, but I'm a bit worried:
Your approach would only work when parallel requests execute their own
cgit cgi instance _and_ when setenv does not add the variables to the
environment of the web server, but only to the environment of the
executing cgit cgi. it thus depends heavily on the execution model,
while my solution does not.
your solution is much simpler and when we can guarantee that the
execution model is how we expect it so that parallel requests do not
interfere with eachother then I'm all for it.
--
Ferry Huberts
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 0/4] Hand the repo configuration to the filters
2011-03-26 11:45 ` mailings
@ 2011-03-26 12:50 ` hjemli
0 siblings, 0 replies; 10+ messages in thread
From: hjemli @ 2011-03-26 12:50 UTC (permalink / raw)
On Sat, Mar 26, 2011 at 12:45, Ferry Huberts <mailings at hupie.com> wrote:
> On 03/26/2011 11:48 AM, Lars Hjemli wrote:
>> On Wed, Mar 23, 2011 at 11:57, Ferry Huberts <mailings at hupie.com> wrote:
>>> Ferry Huberts (5):
>>> ?new_filter: determine extra_args from filter type
>>> ?cgit_open_filter: also take the repo as a parameter
>>> ?cgit_open_filter: hand down repo configuration to script
>>> ?filters: document environment variables in filter scripts
>>
>> Thanks. I've applied this to my wip branch for now with the attached
>> patch on top, since the switch from execvp() to execve() made the
>> snapshot feature stop working (make test reported "Unable to exec
>> subprocess gzip").
>>
>> --
>> larsh
>
> looks ok, but I'm a bit worried:
> Your approach would only work when parallel requests execute their own
> cgit cgi instance _and_ when setenv does not add the variables to the
> environment of the web server, but only to the environment of the
> executing cgit cgi. it thus depends heavily on the execution model,
> while my solution does not.
>
> your solution is much simpler and when we can guarantee that the
> execution model is how we expect it so that parallel requests do not
> interfere with eachother then I'm all for it.
Since the calls to setenv() are performed in the subprocess (between
fork() and execvp()), this should work even in a fastcgi-version of
cgit (the only system I know about where the environment is global is
MS-DOS, which I don't consider a likely target).
--
larsh
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 1/5] new_filter: determine extra_args from filter type
2011-03-23 10:57 ` [PATCH v4 1/5] new_filter: determine extra_args from filter type mailings
@ 2011-04-29 14:28 ` mailings
0 siblings, 0 replies; 10+ messages in thread
From: mailings @ 2011-04-29 14:28 UTC (permalink / raw)
Lars,
Do you have a plan/time for merging these patches from wip into master?
It's been over a month now :-)
(I'm kind of waiting on them to appear on master)
grtz
--
Ferry Huberts
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2011-04-29 14:28 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-03-23 10:57 [PATCH v3 0/4] Hand the repo configuration to the filters mailings
2011-03-23 10:57 ` [PATCH v4 1/5] new_filter: determine extra_args from filter type mailings
2011-04-29 14:28 ` mailings
2011-03-23 10:57 ` [PATCH v4 2/5] cgit_open_filter: also take the repo as a parameter mailings
2011-03-23 10:57 ` [PATCH v4 3/5] cgit_open_filter: hand down repo configuration to script mailings
2011-03-23 10:57 ` [PATCH v4 4/5] filters: document environment variables in filter scripts mailings
2011-03-26 10:48 ` [PATCH v3 0/4] Hand the repo configuration to the filters hjemli
2011-03-26 10:52 ` hjemli
2011-03-26 11:45 ` mailings
2011-03-26 12:50 ` hjemli
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).