[PATCH] Makefile: fix parallel "make test"
When building the "test" target we depend on both cgit and building the Git tools. By doing this with two targets we end up running make in the git/ directory twice, concurrently if using parallel make, which causes us to build more than we need and potentially builds incorrectly if multi-step build-then-move operations overlap. Fix this by instead calling back into the makefile so that we alter the "cgit" target to also build the Git tools. Signed-off-by: John Keeping --- Makefile | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index 77c676d..0fe0bc2 100644 --- a/Makefile +++ b/Makefile @@ -64,12 +64,10 @@ endif all:: cgit cgit: - $(QUIET_SUBDIR0)git $(QUIET_SUBDIR1) -f ../cgit.mk ../cgit NO_CURL=1 + $(QUIET_SUBDIR0)git $(QUIET_SUBDIR1) -f ../cgit.mk ../cgit $(EXTRA_GIT_TARGETS) NO_CURL=1 -git: - $(QUIET_SUBDIR0)git $(QUIET_SUBDIR1) NO_CURL=1 - -test: all git +test: + @$(MAKE) --no-print-directory cgit EXTRA_GIT_TARGETS=all $(QUIET_SUBDIR0)tests $(QUIET_SUBDIR1) all install: all -- 1.8.3.rc2.285.gfc18c2c ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH] ui-log: add around commit decorations
This helps projects that have a large number of tags to display them all
using custom CSS.
The default stylesheet has not been updated since what is useful for
projects with a lot of tags is not the same as what is useful for
projects with only a small number of decorations per commit.
Suggested-by: Konstantin Ryabitsev
Signed-off-by: John Keeping
---
ui-log.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/ui-log.c b/ui-log.c
index 2aa12c3..6f1249b 100644
--- a/ui-log.c
+++ b/ui-log.c
@@ -61,6 +61,7 @@ void show_commit_decorations(struct commit *commit)
buf[sizeof(buf) - 1] = 0;
deco = lookup_decoration(&name_decoration, &commit->object);
+ html("");
while (deco) {
if (!prefixcmp(deco->name, "refs/heads/")) {
strncpy(buf, deco->name + 11, sizeof(buf) - 1);
@@ -94,6 +95,7 @@ void show_commit_decorations(struct commit *commit)
next:
deco = deco->next;
}
+ html("");
}
static void print_commit(struct commit *commit, struct rev_info *revs)
--
1.8.3.rc2.285.gfc18c2c
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH] html.c: die when write fails
If we fail to write HTML output once, there's no point carrying on so
just write a failure message once and die. By using Git's die_errno
function we also let the user know in what way the write failed.
Signed-off-by: John Keeping
---
html.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/html.c b/html.c
index f7772dc..03277db 100644
--- a/html.c
+++ b/html.c
@@ -78,7 +78,7 @@ char *fmtalloc(const char *format, ...)
void html_raw(const char *data, size_t size)
{
if (write(htmlfd, data, size) != size)
- fprintf(stderr, "[html.c] html output truncated.\n");
+ die_errno("write error on html output");
}
void html(const char *txt)
--
1.8.3.rc2.285.gfc18c2c
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
Re: [RFC] Relocate repos when using scan-path
On Mon, May 13, 2013 at 03:56:13PM +0200, Florian Pritz wrote: > Having now seen section-from-path now I think this feature is pretty > useful, but to fully utilize it I'd have to move around some repos for a > proper directory structure. > > Since I want to keep URLs working for existing clones I thought about > placing symlinks in the old location, but then cgit will display each > repo twice. > > Should there be an option to ignore symlinks, a (per repo) list of valid > paths for the index or something else to keep cgit's index list clean? I think what you want here is to return a "301 Moved Permanently" response on the old URLs. I considered adding this to CGit but I'm not sure we should add yet more configuration options when this can already be achieved with mod_rewrite or ngx_http_rewrite_module. ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH] html.c: die when write fails
On Sat, May 18, 2013 at 05:05:21PM +0200, Jason A. Donenfeld wrote: > It seems reasonable to die like this. However, if we're going to start > using git's die_errno, I'd also like to use that in other places > instead of die(). Yeah, there are probably a lot of places that would be helped by this. This was just the one I noticed when I mis-clicked and then clicked again aborting the first request, which caused a spew of "html output truncated" messages as CGit carried on generating the page. ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH] shared.c: use die_errno() where appropriate
This replaces some code that is re-implementing die_errno by just
calling the function.
Signed-off-by: John Keeping
---
>From a quick grep, these are the only places where it makes sense to
replace die with die_errno.
shared.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/shared.c b/shared.c
index 4369378..919a99e 100644
--- a/shared.c
+++ b/shared.c
@@ -15,21 +15,21 @@ struct cgit_context ctx;
int chk_zero(int result, char *msg)
{
if (result != 0)
- die("%s: %s", msg, strerror(errno));
+ die_errno("%s", msg);
return result;
}
int chk_positive(int result, char *msg)
{
if (result <= 0)
- die("%s: %s", msg, strerror(errno));
+ die_errno("%s", msg);
return result;
}
int chk_non_negative(int result, char *msg)
{
if (result < 0)
- die("%s: %s", msg, strerror(errno));
+ die_errno("%s", msg);
return result;
}
@@ -468,8 +468,7 @@ int cgit_open_filter(struct cgit_filter *filter)
chk_non_negative(dup2(filter->pipe_fh[0], STDIN_FILENO),
"Unable to use pipe as STDIN");
execvp(filter->cmd, filter->argv);
- die("Unable to exec subprocess %s: %s (%d)", filter->cmd,
- strerror(errno), errno);
+ die_errno("Unable to exec subprocess %s", filter->cmd);
}
close(filter->pipe_fh[0]);
chk_non_negative(dup2(filter->pipe_fh[1], STDOUT_FILENO),
--
1.8.3.rc2.285.gfc18c2c
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
Re: [RFC] Relocate repos when using scan-path
On Sat, May 18, 2013 at 05:46:35PM +0200, Jason A. Donenfeld wrote:
> On Sat, May 18, 2013 at 5:27 PM, Florian Pritz wrote:
> > The point is that I want to keep already cloned repositories (as in "git
> > clone git://foo" via git-daemon) working and mod_rewrite won't help with
> > that even though it would probably work fine with the http clones.
>
> Make a directory elsewhere on the file system.
> Symlink everything you need into it.
> Point git-daemon at that.
That might still break SSH; for a proper solution I think we need
something like this (untested).
It might be good to force USE_WILDCARD=YesPlease in the makefile on top
of this patch, which will replace fnmatch with Git's wildmatch, adding
support for "**" to match any number of directory levels.
-- >8 --
Subject: [PATCH] Add scan-exclude option to filter out repositories
Signed-off-by: John Keeping
---
cgit.c | 4
cgit.h | 1 +
cgitrc.5.txt | 7 +++
scan-tree.c | 15 +++
4 files changed, 27 insertions(+)
diff --git a/cgit.c b/cgit.c
index 6f44ef2..c621439 100644
--- a/cgit.c
+++ b/cgit.c
@@ -229,6 +229,8 @@ static void config_cb(const char *name, const char *value)
ctx.cfg.max_commit_count = atoi(value);
else if (!strcmp(name, "project-list"))
ctx.cfg.project_list = xstrdup(expand_macros(value));
+ else if (!strcmp(name, "scan-exclude"))
+ string_list_append(&ctx.cfg.scan_exclude, expand_macros(value));
else if (!strcmp(name, "scan-path"))
if (!ctx.cfg.nocache && ctx.cfg.cache_size)
process_cached_repolist(expand_macros(value));
@@ -405,6 +407,8 @@ static void prepare_context(struct cgit_context *ctx)
ctx->page.expires = ctx->page.modified;
ctx->page.etag = NULL;
memset(&ctx->cfg.mimetypes, 0, sizeof(struct string_list));
+ memset(&ctx->cfg.scan_exclude, 0, sizeof(struct string_list));
+ ctx->cfg.scan_exclude.strdup_strings = 1;
if (ctx->env.script_name)
ctx->cfg.script_name = xstrdup(ctx->env.script_name);
if (ctx->env.query_string)
diff --git a/cgit.h b/cgit.h
index 850b792..ee6a545 100644
--- a/cgit.h
+++ b/cgit.h
@@ -238,6 +238,7 @@ struct cgit_config {
int branch_sort;
int commit_sort;
struct string_list mimetypes;
+ struct string_list scan_exclude;
struct cgit_filter *about_filter;
struct cgit_filter *commit_filter;
struct cgit_filter *source_filter;
diff --git a/cgitrc.5.txt b/cgitrc.5.txt
index 9b803b3..71db425 100644
--- a/cgitrc.5.txt
+++ b/cgitrc.5.txt
@@ -329,6 +329,13 @@ root-title::
Text printed as heading on the repository index page. Default value:
"Git Repository Browser".
+scan-exclude::
+ Specified a pattern to be used to exclude repositories found by
+ "scan-path". This option may be specified more than once, with each
+ pattern being compared to discovered repositories with fnmatch(3).
+ Any matches will cause the repository to be omitted from CGit's index.
+ This must be defined prior to scan-path. See also: scan-path.
+
scan-hidden-path::
If set to "1" and scan-path is enabled, scan-path will recurse into
directories whose name starts with a period ('.'). Otherwise,
diff --git a/scan-tree.c b/scan-tree.c
index a1ec8fb..6ab49e5 100644
--- a/scan-tree.c
+++ b/scan-tree.c
@@ -75,6 +75,18 @@ static char *xstrrchr(char *s, char *from, int c)
return from < s ? NULL : from;
}
+static int exclude_repo(const char *path)
+{
+ int i;
+ struct string_list *exclude = &ctx.cfg.scan_exclude;
+
+ for (i = 0; i < exclude->nr; i++)
+ if (!fnmatch(exclude->items[i].string, path, FNM_PATHNAME))
+ return 1;
+
+ return 0;
+}
+
static void add_repo(const char *base, struct strbuf *path, repo_config_fn fn)
{
struct stat st;
@@ -91,6 +103,9 @@ static void add_repo(const char *base, struct strbuf *path,
repo_config_fn fn)
return;
}
+ if (exclude_repo(path->buf))
+ return;
+
strbuf_addch(path, '/');
pathlen = path->len;
--
1.8.3.rc2.285.gfc18c2c
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
Re: [RFC] Relocate repos when using scan-path
On Sat, May 18, 2013 at 06:20:56PM +0200, Jason A. Donenfeld wrote: > On Sat, May 18, 2013 at 6:06 PM, John Keeping wrote: > > That might still break SSH; for a proper solution I think we need > > something like this (untested). > > Why? What's git-daemon have anything to do with SSH? Nothing, but the same problem of moving repositories applies. We can solve HTTP with a 301 redirect but not git:// or ssh:// and people seem to use absolute paths with SSH if they're not using Gitolite or an equivalent, which makes it harder to move repositories accessed in that way. ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH] cache.c: fix cache_ls
Commit fb3655d (use struct strbuf instead of static buffers, 2013-04-06)
broke the logic in cache.c::cache_ls by failing to set slot->cache_name
before calling open_slot.
While fixing this, also free the strbufs added by that commit once we're
done with them.
Signed-off-by: John Keeping
---
cache.c | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/cache.c b/cache.c
index c1d777b..74a1795 100644
--- a/cache.c
+++ b/cache.c
@@ -316,6 +316,7 @@ int cache_process(int size, const char *path, const char
*key, int ttl,
struct strbuf filename = STRBUF_INIT;
struct strbuf lockname = STRBUF_INIT;
struct cache_slot slot;
+ int result;
/* If the cache is disabled, just generate the content */
if (size <= 0) {
@@ -343,11 +344,15 @@ int cache_process(int size, const char *path, const char
*key, int ttl,
slot.fn = fn;
slot.cbdata = cbdata;
slot.ttl = ttl;
- slot.cache_name = strbuf_detach(&filename, NULL);
- slot.lock_name = strbuf_detach(&lockname, NULL);
+ slot.cache_name = filename.buf;
+ slot.lock_name = lockname.buf;
slot.key = key;
slot.keylen = strlen(key);
- return process_slot(&slot);
+ result = process_slot(&slot);
+
+ strbuf_release(&filename);
+ strbuf_release(&lockname);
+ return result;
}
/* Return a strftime formatted date/time
@@ -393,6 +398,7 @@ int cache_ls(const char *path)
continue;
strbuf_setlen(&fullname, prefixlen);
strbuf_addstr(&fullname, ent->d_name);
+ slot.cache_name = fullname.buf;
if ((err = open_slot(&slot)) != 0) {
cache_log("[cgit] unable to open path %s: %s (%d)\n",
fullname.buf, strerror(err), err);
@@ -406,8 +412,8 @@ int cache_ls(const char *path)
slot.buf);
close_slot(&slot);
}
- slot.cache_name = strbuf_detach(&fullname, NULL);
closedir(dir);
+ strbuf_release(&fullname);
return 0;
}
--
1.8.3.rc2.285.gfc18c2c
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH 1/2] tests: introduce strip_header() helper function
This means that we can avoid hardcoding the number of headers we expect
CGit to generate in test cases and simply remove whatever headers happen
to by there when we are checking body content.
Signed-off-by: John Keeping
---
This was previously sent with a different command message and
justification[1] but wasn't picked up. I still think this is a useful
function to have in the test suite and it's used by patch 2/2 here.
tests/setup.sh | 8
tests/t0107-snapshot.sh | 4 ++--
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/tests/setup.sh b/tests/setup.sh
index a573444..1d8677a 100755
--- a/tests/setup.sh
+++ b/tests/setup.sh
@@ -98,4 +98,12 @@ cgit_url()
CGIT_CONFIG="$PWD/cgitrc" QUERY_STRING="url=$1" cgit
}
+strip_headers () {
+ while read -r line
+ do
+ test -z "$line" && break
+ done
+ cat
+}
+
test -z "$CGIT_TEST_NO_CREATE_REPOS" && setup_repos
diff --git a/tests/t0107-snapshot.sh b/tests/t0107-snapshot.sh
index 053062c..6cf7aaa 100755
--- a/tests/t0107-snapshot.sh
+++ b/tests/t0107-snapshot.sh
@@ -16,7 +16,7 @@ test_expect_success 'check html headers' '
'
test_expect_success 'strip off the header lines' '
- tail -n +6 tmp > master.tar.gz
+ strip_headers master.tar.gz
'
test_expect_success 'verify gzip format' '
@@ -51,7 +51,7 @@ test_expect_success 'check HTML headers (zip)' '
'
test_expect_success 'strip off the header lines (zip)' '
- tail -n +6 tmp >master.zip
+ strip_headers master.zip
'
if test -n "$(which unzip 2>/dev/null)"; then
--
1.8.3.rc2.285.gfc18c2c
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH 2/2] cache.c: cache ls_cache output properly
By using the standard library's printf, cache_ls does not redirect its
output to the cache when we change the process' stdout file descriptor
to point to the cache file. Fix this by using "htmlf" in the same way
that we do for writing HTTP headers.
Signed-off-by: John Keeping
---
cache.c | 13 +++--
tests/t0020-validate-cache.sh | 8 +++-
2 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/cache.c b/cache.c
index c1d777b..265d392 100644
--- a/cache.c
+++ b/cache.c
@@ -15,6 +15,7 @@
#include "cgit.h"
#include "cache.h"
+#include "html.h"
#define CACHE_BUFSIZE (1024 * 4)
@@ -398,12 +399,12 @@ int cache_ls(const char *path)
fullname.buf, strerror(err), err);
continue;
}
- printf("%s %s %10"PRIuMAX" %s\n",
- fullname.buf,
- sprintftime("%Y-%m-%d %H:%M:%S",
- slot.cache_st.st_mtime),
- (uintmax_t)slot.cache_st.st_size,
- slot.buf);
+ htmlf("%s %s %10"PRIuMAX" %s\n",
+ fullname.buf,
+ sprintftime("%Y-%m-%d %H:%M:%S",
+ slot.cache_st.st_mtime),
+ (uintmax_t)slot.cache_st.st_size,
+ slot.buf);
close_slot(&slot);
}
slot.cache_name = strbuf_detach(&fullname, NULL);
diff --git a/tests/t0020-validate-cache.sh b/tests/t0020-validate-cache.sh
index 7e7379a..657765d 100755
--- a/tests/t0020-validate-cache.sh
+++ b/tests/t0020-validate-cache.sh
@@ -66,7 +66,13 @@ test_expect_success 'verify cache-size=1021' '
cgit_url "bar/diff" &&
cgit_url "bar/patch" &&
ls cache >output &&
- test_line_count = 13 output
+ test_line_count = 13 output &&
+ cgit_url "foo/ls_cache" >output.full &&
+ strip_headers output &&
+ test_line_count = 13 output &&
+ # Check that ls_cache output is cached correctly
+ cgit_url "foo/ls_cache" >output.second &&
+ test_cmp output.full output.second
'
test_done
--
1.8.3.rc2.285.gfc18c2c
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH 1/2] tests: introduce strip_header() helper function
On Sat, May 18, 2013 at 06:46:38PM +0100, John Keeping wrote:
> This means that we can avoid hardcoding the number of headers we expect
> CGit to generate in test cases and simply remove whatever headers happen
> to by there when we are checking body content.
>
> Signed-off-by: John Keeping
> ---
> This was previously sent with a different command message and
> justification[1] but wasn't picked up. I still think this is a useful
> function to have in the test suite and it's used by patch 2/2 here.
Should have included the reference here...
[1] http://article.gmane.org/gmane.comp.version-control.cgit/1349
> tests/setup.sh | 8
> tests/t0107-snapshot.sh | 4 ++--
> 2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/tests/setup.sh b/tests/setup.sh
> index a573444..1d8677a 100755
> --- a/tests/setup.sh
> +++ b/tests/setup.sh
> @@ -98,4 +98,12 @@ cgit_url()
> CGIT_CONFIG="$PWD/cgitrc" QUERY_STRING="url=$1" cgit
> }
>
> +strip_headers () {
> + while read -r line
> + do
> + test -z "$line" && break
> + done
> + cat
> +}
> +
> test -z "$CGIT_TEST_NO_CREATE_REPOS" && setup_repos
> diff --git a/tests/t0107-snapshot.sh b/tests/t0107-snapshot.sh
> index 053062c..6cf7aaa 100755
> --- a/tests/t0107-snapshot.sh
> +++ b/tests/t0107-snapshot.sh
> @@ -16,7 +16,7 @@ test_expect_success 'check html headers' '
> '
>
> test_expect_success 'strip off the header lines' '
> - tail -n +6 tmp > master.tar.gz
> + strip_headers master.tar.gz
> '
>
> test_expect_success 'verify gzip format' '
> @@ -51,7 +51,7 @@ test_expect_success 'check HTML headers (zip)' '
> '
>
> test_expect_success 'strip off the header lines (zip)' '
> - tail -n +6 tmp >master.zip
> + strip_headers master.zip
> '
>
> if test -n "$(which unzip 2>/dev/null)"; then
> --
> 1.8.3.rc2.285.gfc18c2c
>
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH] git: update to 1.8.3
No changes required, just bump the submodule and Makefile versions. Signed-off-by: John Keeping --- Makefile | 2 +- git | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 77c676d..6eddca1 100644 --- a/Makefile +++ b/Makefile @@ -14,7 +14,7 @@ htmldir = $(docdir) pdfdir = $(docdir) mandir = $(prefix)/share/man SHA1_HEADER = -GIT_VER = 1.8.2.2 +GIT_VER = 1.8.3 GIT_URL = https://git-core.googlecode.com/files/git-$(GIT_VER).tar.gz INSTALL = install MAN5_TXT = $(wildcard *.5.txt) diff --git a/git b/git index 4a9a4f0..edca415 16 --- a/git +++ b/git @@ -1 +1 @@ -Subproject commit 4a9a4f0ec1daaccb88cdf507375ca64696dfe167 +Subproject commit edca4152560522a431a51fc0a06147fc680b5b18 -- 1.8.3.rc3.438.gb3e4ae3 ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: readme & about-filter improvements: auto selection
On Sat, May 25, 2013 at 05:21:05PM +0200, Jason A. Donenfeld wrote: > * On readme=, multiple items may be specified, separated by a space, > and if multiple items exist, cgit will display the first one that it's > able to find. Why not make this a multi-valued key instead? That way we don't have to worry about escaping for people who have whitespace in file names. > Combined, they allow for the following setup. In cgitrc, I have these > specified: > > about-filter=/var/www/uwsgi/cgit/filters/about-selector.sh > readme=:README.md :readme.md :README.mkd :readme.mkd :README.rst > :readme.rst :README.txt :readme.txt :README :readme :INSTALL.txt > :install.txt :INSTALL :install Then this becomes: about-filter=/var/www/uwsgi/cgit/filters/about-selector.sh readme=:README.md readme=:readme.md readme=:README.mkd ... Which is also a lot more readable when there is a long lost of file names. ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: RFE: download patch between arbitrary revisions
On Sun, Jun 02, 2013 at 09:19:11PM -0400, Konstantin Ryabitsev wrote:
> There is currently a way to render a diff between two arbitrary objects,
> e.g.:
>
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/diff/?id=v3.10-rc4&id2=v3.10-rc3
>
> However, there doesn't appear to be a way to download a patch in the
> same way -- it will only make patch against id's parent. E.g.:
>
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/patch/?id=v3.10-rc4&id2=v3.10-rc3
>
> Any way we can make the behaviour of patch match that of diff?
I had a quick look at this and changing the patch output is really easy,
but the top of the output is now completely wrong since it displays the
message from the "id" commit. I'm not sure what to do about this;
clearly it is useful to be able to get the patch output between two
arbitrary points in a raw format (not HTML) but I don't know what to do
about the commit message and headers. Perhaps we can do something like:
if "id2" is specified:
print shortlog output
else:
do what we currently do
but I don't know how much code would be needed for that.
Here's the patch in case anyone wants to try it (slightly bigger than
strictly necessary because I renamed hex -> new_rev for consistency with
cgit_print_diff):
-- >8 --
diff --git a/cmd.c b/cmd.c
index abe8e46..7e8b168 100644
--- a/cmd.c
+++ b/cmd.c
@@ -93,7 +93,7 @@ static void repolist_fn(struct cgit_context *ctx)
static void patch_fn(struct cgit_context *ctx)
{
- cgit_print_patch(ctx->qry.sha1, ctx->qry.path);
+ cgit_print_patch(ctx->qry.sha1, ctx->qry.sha2, ctx->qry.path);
}
static void plain_fn(struct cgit_context *ctx)
diff --git a/ui-patch.c b/ui-patch.c
index fbb92cc..ff8ee34 100644
--- a/ui-patch.c
+++ b/ui-patch.c
@@ -83,28 +83,30 @@ static void filepair_cb(struct diff_filepair *pair)
html("Binary files differ\n");
}
-void cgit_print_patch(char *hex, const char *prefix)
+void cgit_print_patch(const char *new_rev, const char *old_rev, const char
*prefix)
{
struct commit *commit;
struct commitinfo *info;
unsigned char sha1[20], old_sha1[20];
char *patchname;
- if (!hex)
- hex = ctx.qry.head;
+ if (!new_rev)
+ new_rev = ctx.qry.head;
- if (get_sha1(hex, sha1)) {
- cgit_print_error("Bad object id: %s", hex);
+ if (get_sha1(new_rev, sha1)) {
+ cgit_print_error("Bad object id: %s", new_rev);
return;
}
commit = lookup_commit_reference(sha1);
if (!commit) {
- cgit_print_error("Bad commit reference: %s", hex);
+ cgit_print_error("Bad commit reference: %s", new_rev);
return;
}
info = cgit_parse_commit(commit);
- if (commit->parents && commit->parents->item)
+ if (old_rev)
+ get_sha1(old_rev, old_sha1);
+ else if (commit->parents && commit->parents->item)
hashcpy(old_sha1, commit->parents->item->object.sha1);
else
hashclr(old_sha1);
diff --git a/ui-patch.h b/ui-patch.h
index 1641cea..e833042 100644
--- a/ui-patch.h
+++ b/ui-patch.h
@@ -1,6 +1,6 @@
#ifndef UI_PATCH_H
#define UI_PATCH_H
-extern void cgit_print_patch(char *hex, const char *prefix);
+extern void cgit_print_patch(const char *new_rev, const char *old_rev, const
char *prefix);
#endif /* UI_PATCH_H */
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH] Display the most recent modtime for the repolist
On Thu, Aug 22, 2013 at 09:05:33PM +0200, Fabien Parent wrote:
> Instead of using the modtime of the master branch cgit now uses the modtime
> of the lastest ref updated.
> ---
> ui-repolist.c | 32 +---
> 1 file changed, 29 insertions(+), 3 deletions(-)
>
>
> diff --git a/ui-repolist.c b/ui-repolist.c
> index 2ab6e9e..dcd74dc 100644
> --- a/ui-repolist.c
> +++ b/ui-repolist.c
> @@ -13,6 +13,11 @@
> #include "ui-shared.h"
> #include
>
> +struct ref_mod_time {
> + struct cgit_repo *repo;
> + time_t mod_time;
> +};
> +
> static time_t read_agefile(char *path)
> {
> time_t result;
> @@ -74,11 +79,32 @@ end:
> return (r->mtime != 0);
> }
>
> +static int get_ref_mod_time(const char *refname, const unsigned char *sha1,
> + int flags, void *cb_data)
> +{
> + struct ref_mod_time *rmt = (struct ref_mod_time*) cb_data;
> + char *defbranch_old;
> +
> + defbranch_old = rmt->repo->defbranch;
> + rmt->repo->defbranch = (char*) refname;
> + get_repo_modtime(rmt->repo, &rmt->mod_time);
By doing this for each ref, don't we end up with repo->modtime pointing
at which ever branch is processed last? There doesn't seem to be an
attempt to update it below.
Also this doesn't change the behaviour to what people normally want - by
sticking with get_repo_modtime you're still just stat'ing the ref file
(assuming it's not packed). For repositories that spend a long time
idle, a GC or other operation may well change the mtime of the file
without any actual change to the repository.
> + rmt->repo->defbranch = defbranch_old;
> + return 0;
> +}
> +
> static void print_modtime(struct cgit_repo *repo)
> {
> - time_t t;
> - if (get_repo_modtime(repo, &t))
> - cgit_print_age(t, -1, NULL);
> + struct ref_mod_time rmt = { .repo = repo };
> + struct ref_mod_time last_mod_ref = {0};
> +
> + /* Get the time of the most recent update to the repo */
> + setenv("GIT_DIR", repo->path, 1);
> + for_each_branch_ref(get_ref_mod_time, &rmt);
So we unconditionally examine ever ref here? When a user's configured
an age file that will return the same value every time -
get_repo_modtime will never actually inspect the branch.
Also, by putting the change only in print_modtime this is going to
become inconsistent with the sort order when get_repo_modtime is used.
If we want to do a change like this, then it should be contained in
get_repo_modtime (probably with a new helper function, but it shouldn't
need to touch any other functions here). We probably also want it
controlled by a config variable to let users with a lot of repositories
and refs avoid paying the cost of examining all of them.
> + if (rmt.mod_time > last_mod_ref.mod_time)
> + last_mod_ref = rmt;
> +
> + if (last_mod_ref.mod_time)
> + cgit_print_age(last_mod_ref.mod_time, -1, NULL);
> }
>
> static int is_match(struct cgit_repo *repo)
> --
> 1.8.4.rc1
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH] git: update to 1.8.4
No code changes required, just bump the submodule and makefile versions. Signed-off-by: John Keeping --- Makefile | 2 +- git | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 49b6b0c..82d5b58 100644 --- a/Makefile +++ b/Makefile @@ -14,7 +14,7 @@ htmldir = $(docdir) pdfdir = $(docdir) mandir = $(prefix)/share/man SHA1_HEADER = -GIT_VER = 1.8.3 +GIT_VER = 1.8.4 GIT_URL = https://git-core.googlecode.com/files/git-$(GIT_VER).tar.gz INSTALL = install COPYTREE = cp -r diff --git a/git b/git index edca415..e230c56 16 --- a/git +++ b/git @@ -1 +1 @@ -Subproject commit edca4152560522a431a51fc0a06147fc680b5b18 +Subproject commit e230c568c4b9a991e3175e5f65171a566fd8e39c -- 1.8.4.rc3.504.gcab7572 ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: HEAD tag when branch viewed
On Mon, Sep 16, 2013 at 06:35:43PM +, Smith, Eric wrote: > Is this the right place to ask a cgit question? Yes :-) > Shouldn't the red "HEAD" indicator reflect the selected branch? > > Using cgit v0.9.2 the Summary display of the 'master' branch correctly > displays the red "HEAD" indicator next to the green "master" > indicator. > > But when I use the 'switch' dropdown to display the 'develop' branch > the red "HEAD" indicator remains where it was, even though the green > "develop" indicator is now (correctly) displayed on a different > commit. > > The 'git show-ref' does have different commit shas for > refs/head/develop and refs/head/master. The HEAD indicator is showing where the HEAD symref on the server points. By default this is refs/heads/master. You don't normally see a remote's HEAD ref although it does affect the default branch checked out by "git clone", but you will see it if you use "git ls-remote". ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: Encoding problem
On Sat, Sep 28, 2013 at 12:19:38AM +0100, Jorge Bastos wrote: > Is it possible to define charset in cgitrc? > > I'm having encoding problems in the frontend, in the latest version 1.8.4 > from version 0.9.2, and now non-ascii chars are shown with ?? or some other > char instead of the correct one. > > > > Is there a charset option for cgit ? I can't find it. The charset is hardcoded to "UTF-8", which should be the default encoding for Git commit messages and CGit does attempt to transcode Git messages to the correct encoding. Are you seeing '??' in the commit message or in blob/tree content? Do you have a public repository that is exhibiting these symptoms? ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: Encoding problem
On Sat, Oct 05, 2013 at 11:32:54AM +0100, Jorge Bastos wrote:
> > On Sat, Sep 28, 2013 at 12:19:38AM +0100, Jorge Bastos wrote:
> > > Is it possible to define charset in cgitrc?
> > >
> > > I'm having encoding problems in the frontend, in the latest version
> > > 1.8.4 from version 0.9.2, and now non-ascii chars are shown with ??
> > or
> > > some other char instead of the correct one.
> > >
> > >
> > >
> > > Is there a charset option for cgit ? I can't find it.
> >
> > The charset is hardcoded to "UTF-8", which should be the default
> > encoding for Git commit messages and CGit does attempt to transcode Git
> > messages to the correct encoding.
> >
> > Are you seeing '??' in the commit message or in blob/tree content?
> >
> > Do you have a public repository that is exhibiting these symptoms?
>
> I was checking and the file in question was indeed in ANSI, changed the file
> encoding to utf8 and it's OK.
> Anyway, I have gitweb install side-by-side, and in gitweb it was shown
> correctly.
>
> I have other places where chars are not shown OK but didn't get any
> conclution about the file encoding, I'll tell you later,
I've had another look at this, and Gitweb is doing this for all data it
outputs:
# decode sequences of octets in utf8 into Perl's internal form,
# which is utf-8 with utf8 flag set if needed. gitweb writes out
# in utf-8 thanks to "binmode STDOUT, ':utf8'" at beginning
sub to_utf8 {
my $str = shift;
return undef unless defined $str;
if (utf8::is_utf8($str) || utf8::decode($str)) {
return $str;
} else {
return decode($fallback_encoding, $str, Encode::FB_DEFAULT);
}
}
Do you know what the fallback encoding on your Gitweb installation is?
(The default is 'latin1').
If you're not using any other source filter with CGit, you should get
the same result by configuring the following script as "source-filter"
in your cgitrc file.
We'll still get it wrong in "plain" view though, since we
unconditionally set the charset to UTF-8 there and dump the content out
raw; that can be tweaked in the config file but it looks like we get
that wrong and unconditionally append a "charset=" to the MIME type even
for binary types.
-- >8 --
#!/usr/bin/perl
use strict;
use warnings;
use Encode;
binmode STDOUT, ':utf8';
my $str = do { local $/; };
if (utf8::decode($str)) {
print $str;
} else {
print decode('latin1', $str, Encode::FB_DEFAULT);
}
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH] plain: don't append charset for binary MIME types
When outputting the Content-Type HTTP header we print the MIME type and
then append "; charset=" if the charset variable is non-null.
We don't want a charset when we have selected "application/octet-stream"
or when the user has specified a custom MIME type, since they may have
specified their own charset. To avoid this, make sure we set the page's
charset to NULL in ui-plain before we generate the HTTP headers.
Signed-off-by: John Keeping
---
ui-plain.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/ui-plain.c b/ui-plain.c
index 9c86542..23a2b6d 100644
--- a/ui-plain.c
+++ b/ui-plain.c
@@ -83,16 +83,20 @@ static int print_object(const unsigned char *sha1, const
char *path)
mime = string_list_lookup(&ctx.cfg.mimetypes, ext);
if (mime) {
ctx.page.mimetype = (char *)mime->util;
+ ctx.page.charset = NULL;
} else {
ctx.page.mimetype =
get_mimetype_from_file(ctx.cfg.mimetype_file, ext);
- if (ctx.page.mimetype)
+ if (ctx.page.mimetype) {
freemime = 1;
+ ctx.page.charset = NULL;
+ }
}
}
if (!ctx.page.mimetype) {
- if (buffer_is_binary(buf, size))
+ if (buffer_is_binary(buf, size)) {
ctx.page.mimetype = "application/octet-stream";
- else
+ ctx.page.charset = NULL;
+ } else
ctx.page.mimetype = "text/plain";
}
ctx.page.filename = path;
--
1.8.4.566.g73d370b
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
Re: Remove a few loops in cgit for a large number of repos.
On Thu, Oct 10, 2013 at 10:44:55AM -0400, Michael Hess wrote: > I am looking into using cgit for Drupal.org's repos, and wondering if we > could remove code like this: > http://git.zx2c4.com/cgit/tree/shared.c#n79 > > > We have almost 10,000 repos and are worried about the load from loops, and > building the index it loops over. All of the repos are under a directory > (in 2 different sub directories), so I was hoping we could just validate > the directory path (making sure someone is not trying to do a ../../,etc) > and allow it? > > Could that be done? Please let me know your thoughts. Have you actually seen this causing excessive load, or is it only a theoretical issue? I expect it would be possible, in the case of scan-path, to load repos from disk lazily, but that will probably add quite a lot of complexity and I'm not convinced it's worthwhile. That particular loop will only be executed once and I suspect it is dwarfed by the time spent loading and parsing the config (cached project list if you're using scan-path). ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH] Return HTTP status codes for error conditions for various commands
On Fri, Nov 15, 2013 at 03:01:25PM +0100, Dirk Best wrote:
> This returns 404 Not found for most errors, the only exception is for the
> stats, where it will return 400
> Bad request for unknown statistics types. Note that this patch also fixes
> various incomplete header errors,
> which manifest themselves like this in the web server error log: "Premature
> end of script headers:
> cgit.cgi".
>
> Signed-off-by: Dirk Best
> ---
This change looks fairly sensible from a quick scan through, but there
are several different changes rolled into a single patch here.
At the very least I think there are three commits in here:
1/3 Add cgit_print_pagestart() helper function
Refactor existing code to reduce duplication.
2/3 Remove want_layout from command structure
Move print_pagestart and print_docend into individual commands,
no change in functionality.
3/3 Add error messages
Change in functionality.
> cgit.c | 33 +
> cmd.c| 58 ++
> cmd.h| 1 -
> ui-commit.c | 11 ++-
> ui-diff.c| 20 +---
> ui-diff.h| 2 +-
> ui-patch.c | 8
> ui-shared.c | 16
> ui-shared.h | 3 +++
> ui-stats.c | 7 +++
> ui-summary.c | 5 +
> ui-tag.c | 16 +++-
> ui-tree.c| 8
> 13 files changed, 129 insertions(+), 59 deletions(-)
>
> diff --git a/cgit.c b/cgit.c
> index 861352a..f1ba6dd 100644
> --- a/cgit.c
> +++ b/cgit.c
> @@ -575,9 +575,7 @@ static int prepare_repo_cmd(struct cgit_context *ctx)
> ctx->page.title = fmtalloc("%s - %s", ctx->cfg.root_title,
> "config error");
> ctx->repo = NULL;
> - cgit_print_http_headers(ctx);
> - cgit_print_docstart(ctx);
> - cgit_print_pageheader(ctx);
> + cgit_print_pagestart(ctx);
> cgit_print_error("Failed to open %s: %s", name,
>rc ? strerror(rc) : "Not a valid git
> repository");
> cgit_print_docend();
> @@ -594,9 +592,7 @@ static int prepare_repo_cmd(struct cgit_context *ctx)
> }
>
> if (!ctx->qry.head) {
> - cgit_print_http_headers(ctx);
> - cgit_print_docstart(ctx);
> - cgit_print_pageheader(ctx);
> + cgit_print_pagestart(ctx);
> cgit_print_error("Repository seems to be empty");
> cgit_print_docend();
> return 1;
> @@ -605,11 +601,7 @@ static int prepare_repo_cmd(struct cgit_context *ctx)
> if (get_sha1(ctx->qry.head, sha1)) {
> char *tmp = xstrdup(ctx->qry.head);
> ctx->qry.head = ctx->repo->defbranch;
> - ctx->page.status = 404;
> - ctx->page.statusmsg = "Not found";
> - cgit_print_http_headers(ctx);
> - cgit_print_docstart(ctx);
> - cgit_print_pageheader(ctx);
> + cgit_print_pagestart_error(ctx, 404, "Not found");
> cgit_print_error("Invalid branch: %s", tmp);
> cgit_print_docend();
> return 1;
> @@ -628,11 +620,7 @@ static void process_request(void *cbdata)
> cmd = cgit_get_cmd(ctx);
> if (!cmd) {
> ctx->page.title = "cgit error";
> - ctx->page.status = 404;
> - ctx->page.statusmsg = "Not found";
> - cgit_print_http_headers(ctx);
> - cgit_print_docstart(ctx);
> - cgit_print_pageheader(ctx);
> + cgit_print_pagestart_error(ctx, 404, "Not found");
> cgit_print_error("Invalid request");
> cgit_print_docend();
> return;
> @@ -650,9 +638,7 @@ static void process_request(void *cbdata)
> ctx->qry.vpath = cmd->want_vpath ? ctx->qry.path : NULL;
>
> if (cmd->want_repo && !ctx->repo) {
> - cgit_print_http_headers(ctx);
> - cgit_print_docstart(ctx);
> - cgit_print_pageheader(ctx);
> + cgit_print_pagestart(ctx);
> cgit_print_error("No repository selected");
> cgit_print_docend();
> return;
> @@ -661,16 +647,7 @@ static void process_request(void *cbdata)
> if (ctx->repo && prepare_repo_cmd(ctx))
> return;
>
> - if (cmd->want_layout) {
> - cgit_print_http_headers(ctx);
> - cgit_print_docstart(ctx);
> - cgit_print_pageheader(ctx);
> - }
> -
> cmd->fn(ctx);
> -
> - if (cmd->want_layout)
> - cgit_print_docend();
> }
>
> static int cmp_repos(const void *a, const void *b)
> diff --git a/cmd.c b/cmd.c
> index 0202917..d414450 100644
> --- a/cmd.c
> +++ b/cmd.c
> @@ -39,10 +39,14 @@ static void atom_fn(struct cgit_context *ctx)
>
> static void about_fn(struct cgit_context *ctx)
Re: [PATCH 1/1] enable cgit to show gravatar for author, committer and tagger
On Wed, Nov 27, 2013 at 04:17:17PM +0100, Christian Hesse wrote:
> diff --git a/parsing.c b/parsing.c
> index 658621d..a8005f6 100644
> --- a/parsing.c
> +++ b/parsing.c
> @@ -8,6 +8,9 @@
>
> #include "cgit.h"
>
> +/* we need md5 hashing algorithm to calculate Gravatar URL */
> +#include
We don't currently depend on OpenSSL, except via Git which can use
alternative SHA-1 implementations.
At the very least Debian will not distribute GPL'd packages
linked against OpenSSL [1].
[1] http://lintian.debian.org/tags/possible-gpl-code-linked-with-openssl.html
> +char * cgit_get_gravatar(const char * email) {
> + int n, length;
> + MD5_CTX c;
> + unsigned char digest[16];
> + char hex[33];
> + char * gravatar = malloc(67);
> +
> + /* skip brackets! */
> + email++;
> + length = strlen(email) - 1;
> +
> + MD5_Init(&c);
> +
> + while (length > 0) {
> + if (length > 512)
> + MD5_Update(&c, email, 512);
> + else
> + MD5_Update(&c, email, length);
> + length -= 512;
> + email += 512;
> + }
> +
> + MD5_Final(digest, &c);
Would it be possible to extract everything from MD5_Init to MD5_Final to
a function that just computes the MD5 of a string? That should make it
easier to add alternative implementations.
> + for (n = 0; n < 16; ++n)
> + snprintf(&(hex[n*2]), 16*2, "%02x", (unsigned int)digest[n]);
> +
> + sprintf(gravatar, "http://www.gravatar.com/avatar/%s?s=";, hex);
> +
> + return gravatar;
> +}
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH 1/1] enable cgit to show gravatar for author, committer and tagger
On Wed, Nov 27, 2013 at 09:51:33PM +0100, Christian Hesse wrote:
> John Keeping on Wed, 2013/11/27 16:37:
> > On Wed, Nov 27, 2013 at 04:17:17PM +0100, Christian Hesse wrote:
> > > diff --git a/parsing.c b/parsing.c
> > > index 658621d..a8005f6 100644
> > > --- a/parsing.c
> > > +++ b/parsing.c
> > > @@ -8,6 +8,9 @@
> > >
> > > #include "cgit.h"
> > >
> > > +/* we need md5 hashing algorithm to calculate Gravatar URL */
> > > +#include
> >
> > We don't currently depend on OpenSSL, except via Git which can use
> > alternative SHA-1 implementations.
> >
> > At the very least Debian will not distribute GPL'd packages
> > linked against OpenSSL [1].
> >
> > [1]
> > http://lintian.debian.org/tags/possible-gpl-code-linked-with-openssl.html
>
> Damn licensing stuff... Ok, but cgit is already linked against libcrypt which
> belongs to openssl.
>
> Any ideas what to use instead? Shipping a complete MD5 implementation is a
> bad idea I think.
I think writing against OpenSSL is fine, everyone else tends to have a
compatibility layer for that API anyway. But it would be nice to define
a MD5_HEADER variable in a similar way to git.git's SHA1_HEADER.
> > > +char * cgit_get_gravatar(const char * email) {
> > > + int n, length;
> > > + MD5_CTX c;
> > > + unsigned char digest[16];
> > > + char hex[33];
> > > + char * gravatar = malloc(67);
> > > +
> > > + /* skip brackets! */
> > > + email++;
> > > + length = strlen(email) - 1;
> > > +
> > > + MD5_Init(&c);
> > > +
> > > + while (length > 0) {
> > > + if (length > 512)
> > > + MD5_Update(&c, email, 512);
> > > + else
> > > + MD5_Update(&c, email, length);
> > > + length -= 512;
> > > + email += 512;
> > > + }
> > > +
> > > + MD5_Final(digest, &c);
> >
> > Would it be possible to extract everything from MD5_Init to MD5_Final to
> > a function that just computes the MD5 of a string? That should make it
> > easier to add alternative implementations.
>
> I updated the code to use MD5(...), but that is still openssl.
>
> > > + for (n = 0; n < 16; ++n)
> > > + snprintf(&(hex[n*2]), 16*2, "%02x", (unsigned
> > > int)digest[n]); +
> > > + sprintf(gravatar, "http://www.gravatar.com/avatar/%s?s=";, hex);
> > > +
> > > + return gravatar;
> > > +}
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
Re: Help with installing cgit
On Sun, Dec 29, 2013 at 08:46:37AM +, Shlomit Afgin wrote:
> I download from http://git.zx2c4.com/cgit/refs/ the file cgit-0.9.2.tar.xz
> I follow the instruction in README:
> make get-git
> make
> make install
> Edit Apache conf file and add
>
> AllowOverride None
> Options +ExecCGI
> Order allow,deny
> Allow from all
>
> I also add alias:
> Alias /cgit /var/www/htdocs/cgit/
>
> When I go to http://server.domain/cgit I get the following error:
> You don't have permission to access /cgit/ on this server
> And In the error_log I get:
> Directory index forbidden by Options directive: /var/www/htdocs/cgit/
> I tried to add to 'Options' the +Indexes, So I get the list of the
> content but the cgit did not work.
The "cgit" program is a CGI executable that you need to run. Do you
have "cgit" in /var/www/htdocs/cgit/ ? If so, what happens if you go to
http://your.domain/cgit/cgit ?
I have the following in my Apache config for CGit:
RewriteEngine on
RewriteCond %{REQUEST_FILENAME} !-f
RewriteRule ^/var/www/localhost/htdocs/cgit(.*) /cgi-bin/cgit.cgi$1
[L,PT]
This rewrites all requests under /cgit to go to the "cgit" program in
/cgi-bin/.
There is some more information on Apache's CGI support here [1].
[1] http://httpd.apache.org/docs/current/howto/cgi.html
> Also I did not find instruction, how to set the file cgit.conf in
> order to change the place of cgit files location.
You can either specify CGIT_CONFIG in the environment under which CGit
runs (e.g. using Apache's "SetEnv" directive) or just change the default
when you build CGit by setting CGIT_CONFIG in the "cgit.conf" file
that's included by the makefile.
Hope this helps,
John
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
Re: Help with installing cgit
[administrivia: please don't top post.]
On Sun, Dec 29, 2013 at 10:37:00AM +, Shlomit Afgin wrote:
> On 12/29/13 12:18 PM, "John Keeping" wrote:
>
> >On Sun, Dec 29, 2013 at 08:46:37AM +, Shlomit Afgin wrote:
> >> I download from http://git.zx2c4.com/cgit/refs/ the file
> >>cgit-0.9.2.tar.xz
> >> I follow the instruction in README:
> >> make get-git
> >> make
> >> make install
> >> Edit Apache conf file and add
> >>
> >> AllowOverride None
> >> Options +ExecCGI
> >> Order allow,deny
> >> Allow from all
> >>
> >> I also add alias:
> >> Alias /cgit /var/www/htdocs/cgit/
> >>
> >> When I go to http://server.domain/cgit I get the following error:
> >> You don't have permission to access /cgit/ on this server
> >> And In the error_log I get:
> >> Directory index forbidden by Options directive:
> >>/var/www/htdocs/cgit/
> >> I tried to add to 'Options' the +Indexes, So I get the list of the
> >> content but the cgit did not work.
> >
> >The "cgit" program is a CGI executable that you need to run. Do you
> >have "cgit" in /var/www/htdocs/cgit/ ? If so, what happens if you go to
> >http://your.domain/cgit/cgit ?
> >
> >I have the following in my Apache config for CGit:
> >
> >
> >RewriteEngine on
> >RewriteCond %{REQUEST_FILENAME} !-f
> >RewriteRule ^/var/www/localhost/htdocs/cgit(.*)
> >/cgi-bin/cgit.cgi$1 [L,PT]
> >
> >
> >This rewrites all requests under /cgit to go to the "cgit" program in
> >/cgi-bin/.
>
> I have /var/www/htdocs/cgit/cgit.cgi and when I go to
> http://server.domain/cgit/cgit.cgi,
> It try to open the file (and as where to save it) instead of run it.
Do you have a suitable "AddHandler" directive? The link I gave below
has a section on how to use ExecCGI and says you will need something
like this:
AddHandler cgi-script .cgi
> >There is some more information on Apache's CGI support here [1].
> >
> >[1] http://httpd.apache.org/docs/current/howto/cgi.html
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
Re: Help with installing cgit
On Sun, Dec 29, 2013 at 02:00:56PM +, Shlomit Afgin wrote:
> On 12/29/13 2:02 PM, "John Keeping" wrote:
>
> >[administrivia: please don't top post.]
> >
> >On Sun, Dec 29, 2013 at 10:37:00AM +, Shlomit Afgin wrote:
> >> On 12/29/13 12:18 PM, "John Keeping" wrote:
> >>
> >> >On Sun, Dec 29, 2013 at 08:46:37AM +, Shlomit Afgin wrote:
> >> >> I download from http://git.zx2c4.com/cgit/refs/ the file
> >> >>cgit-0.9.2.tar.xz
> >> >> I follow the instruction in README:
> >> >> make get-git
> >> >> make
> >> >> make install
> >> >> Edit Apache conf file and add
> >> >>
> >> >> AllowOverride None
> >> >> Options +ExecCGI
> >> >> Order allow,deny
> >> >> Allow from all
> >> >>
> >> >> I also add alias:
> >> >> Alias /cgit /var/www/htdocs/cgit/
> >> >>
> >> >> When I go to http://server.domain/cgit I get the following error:
> >> >> You don't have permission to access /cgit/ on this server
> >> >> And In the error_log I get:
> >> >> Directory index forbidden by Options directive:
> >> >>/var/www/htdocs/cgit/
> >> >> I tried to add to 'Options' the +Indexes, So I get the list of the
> >> >> content but the cgit did not work.
> >> >
> >> >The "cgit" program is a CGI executable that you need to run. Do you
> >> >have "cgit" in /var/www/htdocs/cgit/ ? If so, what happens if you go
> >>to
> >> >http://your.domain/cgit/cgit ?
> >> >
> >> >I have the following in my Apache config for CGit:
> >> >
> >> >
> >> >RewriteEngine on
> >> >RewriteCond %{REQUEST_FILENAME} !-f
> >> >RewriteRule ^/var/www/localhost/htdocs/cgit(.*)
> >> >/cgi-bin/cgit.cgi$1 [L,PT]
> >> >
> >> >
> >> >This rewrites all requests under /cgit to go to the "cgit" program in
> >> >/cgi-bin/.
> >>
> >> I have /var/www/htdocs/cgit/cgit.cgi and when I go to
> >> http://server.domain/cgit/cgit.cgi,
> >> It try to open the file (and as where to save it) instead of run it.
> >
> >Do you have a suitable "AddHandler" directive? The link I gave below
> >has a section on how to use ExecCGI and says you will need something
> >like this:
> >
> >AddHandler cgi-script .cgi
> >
> >> >There is some more information on Apache's CGI support here [1].
> >> >
> >> >[1] http://httpd.apache.org/docs/current/howto/cgi.html
>
> I'm sorry, I did not had the 'Addhandler'.
> Now I get a web page, but when I click on the link I get a regular browse
> of the directory in the web.
> I cannot see the files exist in the repository.
Sorry, I'm not an expert on configuring Apache. I recommend you read
the CGI tutorial linked above thoroughly.
Do you have CGit's "virtual-root" configuration turned on? It will
probably be simpler to disable that.
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH 2/2] README: Update dependencies
On Thu, Jan 09, 2014 at 04:13:08PM +0100, Jason A. Donenfeld wrote: > On Thu, Jan 9, 2014 at 8:30 AM, Lukas Fleischer wrote: > > We depend on Git in the test suite. Maybe this should be changed to use > > the binary from the Git submodule instead? > > I think we discussed the possibility of this a while ago with John. > This would make most sense to me, though we are using git's test > harness, and I'd like to avoid making invasive changes in that, if > possible. We do already build the Git tools when building the test target (as opposed to just libgit.a when building the cgit binary), but I can't remember why we do this... It's probably simplest just to further tweak $PATH in tests/setup.sh so it also prepends "$(pwd)/../../git/bin-wrappers". That should cause us to use the Git tools from the submodule without needing to do anything else. > In either case, I wouldn't consider the test case a hard dependency, > and so I'm okay with removing git from the dep list. Or changing it to > "Git 1.8.5 (included)" or similar. ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: RFE: author/committer/tagger links (enable cgit to show gravatar for author, committer and tagger)
On Thu, Jan 09, 2014 at 06:50:29PM +0100, Jason A. Donenfeld wrote: > On Thu, Jan 9, 2014 at 4:21 PM, Konstantin Ryabitsev > wrote: > > That's pretty nifty. > > Cool. Would you consider enabling this on kernel.org? I'll probably > merge it in a few days (still working out some bugs). Seems like the > kind of thing that might give cgit a lot of positive attention... > > > That reminds me -- I'm working on a web-of-trust > > site for kernel.org and something I wouldn't mind having is a way to > > link from cgit to the web of trust for that person. E.g. an email > > address for "torva...@linux-foundation.org" on this page > > (http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=d6e0a2dd12f4067a5bcefb8bbd8ddbeff800afbc) > > would be wrapped in a link such as: > > > > https://blah.kernel.org/?user=torvalds%40linux-foundation.org";> > > torva...@linux-foundation.org > > > > which will bring up a page similar to: > > https://www.kernel.org/doc/wot/torvalds.html > > > > Not sure whether that would clash with any of your existing plans for > > user links, but figured I'd bring this up for a discussion. > > We already support module-link and repo.module-link for making > clickable submodules. I don't see why we wouldn't be able to apply the > same idiom to a email-link and repo.email-link setting. I'll look into > this. I suppose a default setting, or maybe just a suggested setting > in the documentation, would be to issue a search for that email. But > this could then be used to instead link out to your upcoming web of > trust platform. How does this sound? It feels to me like it might be better to allow a filter to be applied here. That way we don't put the Gravatar code in CGit itself but can distribute an example script that does apply Gravatar links to email addresses. I'm not sure what the cost of forking a process here will end up being though; I guess for cases where the email is likely to be repeated we could assume that the filter is pure and memoize the result. But that would give the greatest flexibility for both these use cases, with a simple link value the Gravatar case needs some other forwarding program on the server to do the hashing of the address (unless we provide substitution values for a range of different things, but currently everything just uses printf formats so that would be more work unless we can reuse Git's log formatter somehow). ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: RFE: author/committer/tagger links (enable cgit to show gravatar for author, committer and tagger)
On Thu, Jan 09, 2014 at 07:59:27PM +0100, Jason A. Donenfeld wrote: > On Thu, Jan 9, 2014 at 7:07 PM, John Keeping wrote: > > It feels to me like it might be better to allow a filter to be applied > > here. That way we don't put the Gravatar code in CGit itself but can > > distribute an example script that does apply Gravatar links to email > > addresses. > > > > I'm not sure what the cost of forking a process here will end up being > > though; I guess for cases where the email is likely to be repeated we > > could assume that the filter is pure and memoize the result. > > Gravatar icons are placed next to author names (in the log for > example) as well as the full email address (in the commit). So we'd > have to give such a filter the email address, as well as the original > string to be printed. Such memorization would then need to apply to > that pair. Presumably this would just pass the "A. U. Thor " string to the filter and we could just map that to the output. > > > > But that would give the greatest flexibility for both these use cases, > > with a simple link value the Gravatar case needs some other forwarding > > program on the server to do the hashing of the address (unless we > > provide substitution values for a range of different things, but > > currently everything just uses printf formats so that would be more > > work unless we can reuse Git's log formatter somehow). > > I do like this idea quite a bit. But the cost of forking indeed seems > a bit high -- some listings have many many authors all at once. I'll > investigate a little bit. We could take an incremental approach like git-check-attr and friends do when GIT_FLUSH=1, but I'm not sure how well we could delimit the output in that case - I doubt a single line would be sufficient for all cases. If we're not careful that approach could end up with capability flags and other complex things like the fast-import protocol. My gut feeling is that it /should/ be OK because CGit's caching layer means that we don't actually regenerate pages too often and CGit tends to be blazingly fast anyway, but it would be interesting to see some benchmarks of how much overhead a call to "true" adds. ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH 2/2] README: Update dependencies
On Thu, Jan 09, 2014 at 08:26:24PM +0100, Lukas Fleischer wrote: > On Thu, 09 Jan 2014 at 19:01:42, John Keeping wrote: > > On Thu, Jan 09, 2014 at 04:13:08PM +0100, Jason A. Donenfeld wrote: > > > On Thu, Jan 9, 2014 at 8:30 AM, Lukas Fleischer > > > wrote: > > > > We depend on Git in the test suite. Maybe this should be changed to use > > > > the binary from the Git submodule instead? > > > > > > I think we discussed the possibility of this a while ago with John. > > > This would make most sense to me, though we are using git's test > > > harness, and I'd like to avoid making invasive changes in that, if > > > possible. > > > > We do already build the Git tools when building the test target (as > > opposed to just libgit.a when building the cgit binary), but I can't > > remember why we do this... > > > > It's probably simplest just to further tweak $PATH in tests/setup.sh so > > it also prepends "$(pwd)/../../git/bin-wrappers". That should cause us > > to use the Git tools from the submodule without needing to do anything > > else. > > That sounds like a good idea to me. Do you want to prepare a patch? Now that I've dug into this some more, it turns out that we're already using the Git tools from the submodule! The Git test framework already sets up the path to use the tools from the Git tree, which explains why the Makefile makes sure we build the Git CLI utilities. ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: RFE: .so filters
On Thu, Jan 09, 2014 at 10:34:26PM +0100, Jason A. Donenfeld wrote: > I'm thinking about this filtering situation w.r.t. gravatar and > potentially running multiple filters on one page. Something I've been > considering is implementing a simple dlopen() mechanism for filters, > if the filter filename starts with "soname:" or "lib:" or similar, so > as to avoid the fork()ing and exec()ing we currently have, for high > frequency filters. The idea is that first use of the filter would be > dlopen()'d, but wouldn't be dlclose()'d until the end of the > processing. This way the same function could be used over and over > again without significant penalty. > > In my first thinking of this, the method of action would be the same > as the current system -- "int filter_run(int argc, char *argv[])" is > dlopen()'d, executed, and it reads and writes to the dup2()'d file > descriptor. Unfortunately, the piping in this introduces a cost that > I'd rather avoid. In the case of gravatar (or more generally, email > author filters), we'd be better off with a "char *filter_run(int argc, > char *argv[])", that can just return the string that the html > functions will then print. This, however, breaks the current filtering > paradigm, and might not be ideal for filters that enjoy a stream of > data (such as source code filters). This distinction more or less > points toward coming up with a library API of sorts, but I really > really really don't want to add a full fledged plugin system. So this > has me leaning toward the simpler first idea. > > But I'm undecided at the moment. Comments and suggestions are most > welcome. That interface doesn't really match the way the current filters work. Currently when we open a filter we replace cgit's stdout with a pipe into the filter process, so none of the existing CGit code will work with this interface. We could swap out write with a function pointer into the filter, but I don't think we guarantee that all of the data is written in one go which makes life harder for filter writers (although for simple cases like author info we probably could guarantee to write it all at once). If we allow filters to act incrementally, then we can just leave the filter running and swap it in or out when required. That would require a single dup2 to make it work the same way that the filters currently work. Interestingly, there is an "htmlfd" variable in html.c but it is never changed from STDOUT_FILENO; I wonder if that can be used or are there other places (possibly in libgit.a code) that just use stdout, in which case we should remove that variable. But there is the problem of terminating the response; Lukas' suggestion of using NUL for that may be the best, it's not that hard to printf '\0' in shell. OTOH, the particular case of author details the input is more clearly defined than items for which we currently provide filters, so maybe it could use a different interface. One final point (although I don't think you're suggesting this) is that we shouldn't require shared objects; I think scripts using stdin+stdout are a much simpler interface and provides a much lower barrier to entry, not least because the range of languages that can be used to implement the filters is so much greater. ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: RFE: .so filters
On Fri, Jan 10, 2014 at 03:11:54AM +0100, Jason A. Donenfeld wrote: > On Fri, Jan 10, 2014 at 2:41 AM, Jason A. Donenfeld wrote: > > and does its thing per usual. At the end, however, it does not exit. > > Instead of waitpid()ing on it in close filter, we SIGSTOP it, put the > > fds back in place, etc. Then the next time that filter is called, we > > SIGCONT it, it reads the first N lines as arguments again, and so > > forth. I'm most tempted to go with this approach at the moment. > > Problems abound. This has race condition issues, where the parent > process will SIGSTOP the child before the child can write its output. > This could be fixed with a more complicated signaling protocol, but > that's more complex than I'd like it. Back to the drawing board. This seems drastically over complicated. Why don't we just have something like this: install_filter: if filter_running? dup2(filter_stdin, STDOUT_FILENO) else open_filter uninstall_filter: read until NUL or EOF Then the filter just sits waiting for data on stdin and we don't need to stop it at all. It does complicate things slightly from where we are because we can't just let the filters writes to stdout go straight to our (real) stdout but instead we'll have to read data from it. Annoyingly, although it is probably good enough in this case, we can't just do the read in uninstall_filter in case we get to a deadlock where we want to write to the filter but it's waiting for us to read its output. I suspect that means we'd need a thread to do the reading and set a condition variable when it sees NUL or EOF. I'd rather put that complexity in CGit and make the filter processes really simple though - it's better to do the complex bit once than many times! ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH] Disallow downloading disabled snapshot formats
On Fri, Jan 10, 2014 at 03:38:06PM +0100, Lukas Fleischer wrote:
> We did only display enabled snapshot formats but we did not prevent from
> downloading disabled formats when requested. Fix this by adding an
> appropriate check.
>
> Also, add a test case that checks whether downloading disabled snapshot
> formats is denied, as expected.
>
> Signed-off-by: Lukas Fleischer
> ---
> tests/t0107-snapshot.sh | 5 +
> ui-snapshot.c | 2 +-
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/tests/t0107-snapshot.sh b/tests/t0107-snapshot.sh
> index 6cf7aaa..01e8d22 100755
> --- a/tests/t0107-snapshot.sh
> +++ b/tests/t0107-snapshot.sh
> @@ -79,4 +79,9 @@ test_expect_success UNZIP 'verify unzipped file-5' '
> test_line_count = 1 master/file-5
> '
>
> +test_expect_success 'try to download a disabled snapshot format' '
> + cgit_url "foo/snapshot/master.tar.xz" |
> + grep "Unsupported snapshot format"
I really dislike seeing pipes in the test suite. Can we redirect to
file instead and then grep the file? This helps ensure that the exit
code from CGit is correct (I don't know if we expect it to be zero or
non-zero here, but if the latter then at least test_must_fail checks
that the process didn't segfault - I suspect it should be zero though).
> +'
> +
> test_done
> diff --git a/ui-snapshot.c b/ui-snapshot.c
> index 8f82119..ab20a4a 100644
> --- a/ui-snapshot.c
> +++ b/ui-snapshot.c
> @@ -205,7 +205,7 @@ void cgit_print_snapshot(const char *head, const char
> *hex,
> }
>
> f = get_format(filename);
> - if (!f) {
> + if (!f || (snapshots & f->bit) == 0) {
> show_error("Unsupported snapshot format: %s", filename);
> return;
> }
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
Re: RFE: .so filters
On Fri, Jan 10, 2014 at 06:12:25PM +0100, Florian Pritz wrote:
> On 10.01.2014 16:57, Jason A. Donenfeld wrote:
> > On Fri, Jan 10, 2014 at 10:06 AM, John Keeping wrote:
> >>
> >> This seems drastically over complicated.
> >
> > So here's the situation. There's a lot of "state" that we're taking
> > advantage of in using processes that terminate, that needs to be
> > replicated:
>
> Isn't this (fast scripting with lots of features) when people normally
> start using lua?
I would have no problem including LuaJIT for this sort of thing. There
was even a PoC for using Lua to format Git log messages a year or so
ago.
I was also wondering if supporting "unix:/path/to/socket" would be
useful, then the filter would connect on a Unix socket, run and
disconnect, on the assumption that the administrator has a daemon
running to do the filtering.
If we're introducing this ":" support then it would be good
to do it in a reasonably generic way so that any types that add new
dependencies can be compiled out easily. Maybe a table like this?
struct filter_handler handlers[] = {
{ "unix", open_unix_socket_filter, close_unix_socket_filter },
{ "persistent", "open_persistent_filter, close_persistent_filter },
#ifndef NO_LUA
{ "lua", open_lua_filter, close_lua_filter },
#endif
};
I might have a look at the Lua case over the weekend if no one beats me
to it.
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
Re: RFE: .so filters
On Fri, Jan 10, 2014 at 09:03:24PM +0100, Florian Pritz wrote: > On 10.01.2014 18:57, Jason A. Donenfeld wrote: > > On Fri, Jan 10, 2014 at 6:12 PM, Florian Pritz wrote: > >> > >> Isn't this (fast scripting with lots of features) when people normally > >> start using lua? > >> > > > > This would have the same challenges as using .so files, w.r.t. hooking > > write() (or the html functions), but would be very interesting indeed, > > because Lua... > > How about using the current fork approach but instead of calling execvp > use lua. I believe forks are pretty cheap on linux, it's the exec that's > costly. > > If we do it like that we could reuse stdin/stdout, we could pass > arguments via lua tables (like command line arguments now), but we > should have little overhead for the script loading/executing. Forking and using Lua in the child is an interesting idea. I need to investigate how Lua generally deals with I/O, but it feels like it will be simpler to use a simple function interface than deal with slurping in the input in Lua. So it may be simpler to swap out the write function in CGit while the filter is active and collect the output in a buffer instead, then call a Lua function and pass whatever comes back from that to the real write(2). ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: RFE: .so filters
On Fri, Jan 10, 2014 at 09:25:18PM +0100, Florian Pritz wrote: > On 10.01.2014 21:11, John Keeping wrote: > > Forking and using Lua in the child is an interesting idea. > > > > I need to investigate how Lua generally deals with I/O, but it feels > > like it will be simpler to use a simple function interface than deal > > with slurping in the input in Lua. > > Looks rather easy to slurp stdin (from http://www.lua.org/pil/21.1.html): Interesting. But I think it will be simpler from both side if the interface is just a function call: function filter(value) return value .. " some trailing data" end The change on the CGit side is then quite easy, we just change the switchable value in html.c from htmlfd to html_out_fn which has the same signature as html_raw (which is the default implementation). Then we can collect output in a strbuf until it's time to call the function. The only thing I'm not sure about is how the specification of the filter function works, given that I don't think we can call a complete Lua script as a function. (I'm also assuming that the Lua script will be in an external file and not stored inline in the CGit config). ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH] cache: don't leave cache_slot fields uninitialized
Valgrind says:
==18344== Conditional jump or move depends on uninitialised value(s)
==18344==at 0x406C83: open_slot (cache.c:63)
==18344==by 0x407478: cache_ls (cache.c:403)
==18344==by 0x404C9A: process_request (cgit.c:639)
==18344==by 0x406BD2: fill_slot (cache.c:190)
==18344==by 0x4071A0: cache_process (cache.c:284)
==18344==by 0x404461: main (cgit.c:952)
==18344== Uninitialised value was created by a stack allocation
==18344==at 0x40738B: cache_ls (cache.c:375)
This is caused by the keylen field being used to calculate whether or
not a slot is matched. We never then check the value of this and the
length of data read depends on the key length read from the file so this
isn't dangerous, but it's nice to avoid branching based on uninitialized
data.
Signed-off-by: John Keeping
---
Is there any chance of getting jk/valgrind-tests merged as well? It's
been sat there for a while now and is quite useful.
cache.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/cache.c b/cache.c
index d339435..fa83ddc 100644
--- a/cache.c
+++ b/cache.c
@@ -376,7 +376,7 @@ int cache_ls(const char *path)
DIR *dir;
struct dirent *ent;
int err = 0;
- struct cache_slot slot;
+ struct cache_slot slot = { 0 };
struct strbuf fullname = STRBUF_INIT;
size_t prefixlen;
--
1.8.5.226.g0d60d77
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH 4/6] filter: add fprintf_filter function
This stops the code in cgit.c::print_repo needing to inspect the
cgit_filter structure, meaning that we can abstract out different filter
types that will have different fields that need to be printed.
Signed-off-by: John Keeping
---
cgit.c | 6 +++---
cgit.h | 1 +
filter.c | 5 +
3 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/cgit.c b/cgit.c
index 0be41b8..29b658e 100644
--- a/cgit.c
+++ b/cgit.c
@@ -706,11 +706,11 @@ static void print_repo(FILE *f, struct cgit_repo *repo)
fprintf(f, "repo.enable-log-linecount=%d\n",
repo->enable_log_linecount);
if (repo->about_filter && repo->about_filter != ctx.cfg.about_filter)
- fprintf(f, "repo.about-filter=%s\n", repo->about_filter->cmd);
+ cgit_fprintf_filter(repo->about_filter, f,
"repo.about-filter=");
if (repo->commit_filter && repo->commit_filter != ctx.cfg.commit_filter)
- fprintf(f, "repo.commit-filter=%s\n", repo->commit_filter->cmd);
+ cgit_fprintf_filter(repo->commit_filter, f,
"repo.commit-filter=");
if (repo->source_filter && repo->source_filter != ctx.cfg.source_filter)
- fprintf(f, "repo.source-filter=%s\n", repo->source_filter->cmd);
+ cgit_fprintf_filter(repo->source_filter, f,
"repo.source-filter=");
if (repo->snapshots != ctx.cfg.snapshots) {
char *tmp = build_snapshot_setting(repo->snapshots);
fprintf(f, "repo.snapshots=%s\n", tmp ? tmp : "");
diff --git a/cgit.h b/cgit.h
index e6e7715..9b4be26 100644
--- a/cgit.h
+++ b/cgit.h
@@ -345,6 +345,7 @@ extern int cgit_parse_snapshots_mask(const char *str);
extern int cgit_open_filter(struct cgit_filter *filter, ...);
extern int cgit_close_filter(struct cgit_filter *filter);
+extern void cgit_fprintf_filter(struct cgit_filter *filter, FILE *f, const
char *prefix);
extern struct cgit_filter *cgit_new_filter(const char *cmd, filter_type
filtertype);
extern void cgit_prepare_repo_env(struct cgit_repo * repo);
diff --git a/filter.c b/filter.c
index d8c0116..80cf689 100644
--- a/filter.c
+++ b/filter.c
@@ -63,6 +63,11 @@ done:
}
+void cgit_fprintf_filter(struct cgit_filter *filter, FILE *f, const char
*prefix)
+{
+ fprintf(f, "%s%s\n", prefix, filter->cmd);
+}
+
struct cgit_filter *cgit_new_filter(const char *cmd, filter_type filtertype)
{
struct cgit_filter *f;
--
1.8.5.226.g0d60d77
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH 5/6] filter: add interface layer
Change the existing cgit_{open,close,fprintf}_filter functions to
delegate to filter-specific implementations accessed via function
pointers on the cgit_filter object.
We treat the "exec" filter type slightly specially here by putting its
structure definition in the header file and providing an "init" function
to set up the function pointers. This is required so that the
ui-snapshot.c code that applies a compression filter can continue to use
the filter interface to do so.
Signed-off-by: John Keeping
---
cgit.h| 8
filter.c | 66 ---
ui-snapshot.c | 11 +-
3 files changed, 63 insertions(+), 22 deletions(-)
diff --git a/cgit.h b/cgit.h
index 9b4be26..92e8c55 100644
--- a/cgit.h
+++ b/cgit.h
@@ -57,6 +57,13 @@ typedef enum {
} filter_type;
struct cgit_filter {
+ int (*open)(struct cgit_filter *, va_list ap);
+ int (*close)(struct cgit_filter *);
+ void (*fprintf)(struct cgit_filter *, FILE *, const char *prefix);
+};
+
+struct cgit_exec_filter {
+ struct cgit_filter base;
char *cmd;
char **argv;
int extra_args;
@@ -346,6 +353,7 @@ extern int cgit_parse_snapshots_mask(const char *str);
extern int cgit_open_filter(struct cgit_filter *filter, ...);
extern int cgit_close_filter(struct cgit_filter *filter);
extern void cgit_fprintf_filter(struct cgit_filter *filter, FILE *f, const
char *prefix);
+extern void cgit_exec_filter_init(struct cgit_exec_filter *filter, char *cmd,
char **argv);
extern struct cgit_filter *cgit_new_filter(const char *cmd, filter_type
filtertype);
extern void cgit_prepare_repo_env(struct cgit_repo * repo);
diff --git a/filter.c b/filter.c
index 80cf689..0f3edb0 100644
--- a/filter.c
+++ b/filter.c
@@ -13,15 +13,13 @@
#include
#include
-int cgit_open_filter(struct cgit_filter *filter, ...)
+static int open_exec_filter(struct cgit_filter *base, va_list ap)
{
+ struct cgit_exec_filter *filter = (struct cgit_exec_filter *) base;
int i;
- va_list ap;
- va_start(ap, filter);
for (i = 0; i < filter->extra_args; i++)
filter->argv[i+1] = va_arg(ap, char *);
- va_end(ap);
filter->old_stdout = chk_positive(dup(STDOUT_FILENO),
"Unable to duplicate STDOUT");
@@ -41,9 +39,9 @@ int cgit_open_filter(struct cgit_filter *filter, ...)
return 0;
}
-
-int cgit_close_filter(struct cgit_filter *filter)
+static int close_exec_filter(struct cgit_filter *base)
{
+ struct cgit_exec_filter *filter = (struct cgit_exec_filter *) base;
int i, exit_status;
chk_non_negative(dup2(filter->old_stdout, STDOUT_FILENO),
@@ -63,21 +61,50 @@ done:
}
-void cgit_fprintf_filter(struct cgit_filter *filter, FILE *f, const char
*prefix)
+static void fprintf_exec_filter(struct cgit_filter *base, FILE *f, const char
*prefix)
{
+ struct cgit_exec_filter *filter = (struct cgit_exec_filter *) base;
fprintf(f, "%s%s\n", prefix, filter->cmd);
}
-struct cgit_filter *cgit_new_filter(const char *cmd, filter_type filtertype)
+int cgit_open_filter(struct cgit_filter *filter, ...)
{
- struct cgit_filter *f;
- int args_size = 0;
+ int result;
+ va_list ap;
+ va_start(ap, filter);
+ result = filter->open(filter, ap);
+ va_end(ap);
+ return result;
+}
- if (!cmd || !cmd[0])
- return NULL;
+int cgit_close_filter(struct cgit_filter *filter)
+{
+ return filter->close(filter);
+}
+
+void cgit_fprintf_filter(struct cgit_filter *filter, FILE *f, const char
*prefix)
+{
+ filter->fprintf(filter, f, prefix);
+}
+
+void cgit_exec_filter_init(struct cgit_exec_filter *filter, char *cmd, char
**argv)
+{
+ memset(filter, 0, sizeof(*filter));
+ filter->base.open = open_exec_filter;
+ filter->base.close = close_exec_filter;
+ filter->base.fprintf = fprintf_exec_filter;
+ filter->cmd = cmd;
+ filter->argv = argv;
+}
+
+static struct cgit_filter *new_exec_filter(const char *cmd, filter_type
filtertype)
+{
+ struct cgit_exec_filter *f;
+ int args_size = 0;
- f = xmalloc(sizeof(struct cgit_filter));
- memset(f, 0, sizeof(struct cgit_filter));
+ f = xmalloc(sizeof(*f));
+ /* We leave argv for now and assign it below. */
+ cgit_exec_filter_init(f, xstrdup(cmd), NULL);
switch (filtertype) {
case SOURCE:
@@ -91,10 +118,17 @@ struct cgit_filter *cgit_new_filter(const char *cmd,
filter_type filtertype)
break;
}
- f->cmd = xstrdup(cmd);
args_size = (2 + f->extra_args) * sizeof(char *);
f->argv = xmalloc(args_size);
memset(f->argv, 0, args_size);
f->argv[0] = f->cmd;
- return f;
+ return &f->base;
+}
[RFC/PATCH 0/6] Preparation for more filter types
This is the preliminary refactoring for supporting more types of filter (for example Lua scripts or persistent filters). The final patch adds a table where more implementations can be added. The first three (maybe four) patches are sensible cleanups even if we don't want to take the whole plan any further. John Keeping (6): html: remove redundant htmlfd variable ui-snapshot: set unused cgit_filter fields to zero filter: pass extra arguments via cgit_open_filter filter: add fprintf_filter function filter: add interface layer filter: introduce "filter type" prefix cgit.c| 6 +-- cgit.h| 12 +- cgitrc.5.txt | 9 + filter.c | 119 -- html.c| 4 +- ui-repolist.c | 10 ++--- ui-snapshot.c | 9 ++--- ui-summary.c | 13 +++ ui-tree.c | 7 ++-- 9 files changed, 140 insertions(+), 49 deletions(-) -- 1.8.5.226.g0d60d77 ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH 1/6] html: remove redundant htmlfd variable
This is never changed from STDOUT_FILENO, so just use that value
directly.
Signed-off-by: John Keeping
---
html.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/html.c b/html.c
index 903d4b7..f0ee2d6 100644
--- a/html.c
+++ b/html.c
@@ -41,8 +41,6 @@ static const char* url_escape_table[256] = {
"%fe", "%ff"
};
-static int htmlfd = STDOUT_FILENO;
-
char *fmt(const char *format, ...)
{
static char buf[8][1024];
@@ -77,7 +75,7 @@ char *fmtalloc(const char *format, ...)
void html_raw(const char *data, size_t size)
{
- if (write(htmlfd, data, size) != size)
+ if (write(STDOUT_FILENO, data, size) != size)
die_errno("write error on html output");
}
--
1.8.5.226.g0d60d77
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH 6/6] filter: introduce "filter type" prefix
This allows different filter implementations to be specified in the
configuration file. Currently only "exec" is supported, but it may now
be specified either with or without the "exec:" prefix.
Signed-off-by: John Keeping
---
cgitrc.5.txt | 9 +
filter.c | 33 +++--
2 files changed, 40 insertions(+), 2 deletions(-)
diff --git a/cgitrc.5.txt b/cgitrc.5.txt
index 52caed0..60159f6 100644
--- a/cgitrc.5.txt
+++ b/cgitrc.5.txt
@@ -557,6 +557,15 @@ config files, e.g. "repo.desc" becomes "desc".
FILTER API
--
+By default, filters are separate processes that are executed each time they
+are needed. Alternative technologies may be used by prefixing the filter
+specification with the relevant string; available values are:
+
+'exec:'::
+ The default "one process per filter" mode.
+
+Parameters are provided to filters as follows.
+
about 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
diff --git a/filter.c b/filter.c
index 0f3edb0..ba66e46 100644
--- a/filter.c
+++ b/filter.c
@@ -64,7 +64,7 @@ done:
static void fprintf_exec_filter(struct cgit_filter *base, FILE *f, const char
*prefix)
{
struct cgit_exec_filter *filter = (struct cgit_exec_filter *) base;
- fprintf(f, "%s%s\n", prefix, filter->cmd);
+ fprintf(f, "%sexec:%s\n", prefix, filter->cmd);
}
int cgit_open_filter(struct cgit_filter *filter, ...)
@@ -125,10 +125,39 @@ static struct cgit_filter *new_exec_filter(const char
*cmd, filter_type filterty
return &f->base;
}
+static const struct {
+ const char *prefix;
+ struct cgit_filter *(*ctor)(const char *cmd, filter_type filtertype);
+} filter_specs[] = {
+ { "exec", new_exec_filter },
+};
+
struct cgit_filter *cgit_new_filter(const char *cmd, filter_type filtertype)
{
+ char *colon;
+ int i;
+ size_t len;
if (!cmd || !cmd[0])
return NULL;
- return new_exec_filter(cmd, filtertype);
+ colon = strchr(cmd, ':');
+ len = colon - cmd;
+ /*
+* In case we're running on Windows, don't allow a single letter before
+* the colon.
+*/
+ if (len == 1)
+ colon = NULL;
+
+ /* If no prefix is given, exec filter is the default. */
+ if (!colon)
+ return new_exec_filter(cmd, filtertype);
+
+ for (i = 0; i < ARRAY_SIZE(filter_specs); i++) {
+ if (len == strlen(filter_specs[i].prefix) &&
+ !strncmp(filter_specs[i].prefix, cmd, len))
+ return filter_specs[i].ctor(colon + 1, filtertype);
+ }
+
+ die("Invalid filter type: %.*s", (int) len, cmd);
}
--
1.8.5.226.g0d60d77
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH 2/6] ui-snapshot: set unused cgit_filter fields to zero
By switching the assignment of fields in the cgit_filter structure to
use designated initializers, the compiler will initialize all other
fields to their default value. This will be needed when we add the
extra_args field in the next patch.
Signed-off-by: John Keeping
---
ui-snapshot.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/ui-snapshot.c b/ui-snapshot.c
index 8f82119..5136c49 100644
--- a/ui-snapshot.c
+++ b/ui-snapshot.c
@@ -58,10 +58,10 @@ static int write_compressed_tar_archive(const char *hex,
char *filter_argv[])
{
int rv;
- struct cgit_filter f;
-
- f.cmd = filter_argv[0];
- f.argv = filter_argv;
+ struct cgit_filter f = {
+ .cmd = filter_argv[0],
+ .argv = filter_argv,
+ };
cgit_open_filter(&f);
rv = write_tar_archive(hex, prefix);
cgit_close_filter(&f);
--
1.8.5.226.g0d60d77
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH 3/6] filter: pass extra arguments via cgit_open_filter
This avoids poking into the filter data structure at various points in
the code. We rely on the fact that the number of arguments is fixed
based on the filter type (set in cgit_new_filter) and that the call
sites all know which filter type they're using.
Signed-off-by: John Keeping
---
cgit.h| 3 ++-
filter.c | 35 ---
ui-repolist.c | 10 +++---
ui-summary.c | 13 ++---
ui-tree.c | 7 +++
5 files changed, 38 insertions(+), 30 deletions(-)
diff --git a/cgit.h b/cgit.h
index a72c503..e6e7715 100644
--- a/cgit.h
+++ b/cgit.h
@@ -59,6 +59,7 @@ typedef enum {
struct cgit_filter {
char *cmd;
char **argv;
+ int extra_args;
int old_stdout;
int pipe_fh[2];
int pid;
@@ -342,7 +343,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, ...);
extern int cgit_close_filter(struct cgit_filter *filter);
extern struct cgit_filter *cgit_new_filter(const char *cmd, filter_type
filtertype);
diff --git a/filter.c b/filter.c
index f4ee9ac..d8c0116 100644
--- a/filter.c
+++ b/filter.c
@@ -13,8 +13,16 @@
#include
#include
-int cgit_open_filter(struct cgit_filter *filter)
+int cgit_open_filter(struct cgit_filter *filter, ...)
{
+ int i;
+ va_list ap;
+
+ va_start(ap, filter);
+ for (i = 0; i < filter->extra_args; i++)
+ filter->argv[i+1] = va_arg(ap, char *);
+ va_end(ap);
+
filter->old_stdout = chk_positive(dup(STDOUT_FILENO),
"Unable to duplicate STDOUT");
chk_zero(pipe(filter->pipe_fh), "Unable to create pipe to subprocess");
@@ -36,45 +44,50 @@ int cgit_open_filter(struct cgit_filter *filter)
int cgit_close_filter(struct cgit_filter *filter)
{
- int exit_status;
+ int i, exit_status;
chk_non_negative(dup2(filter->old_stdout, STDOUT_FILENO),
"Unable to restore STDOUT");
close(filter->old_stdout);
if (filter->pid < 0)
- return 0;
+ goto done;
waitpid(filter->pid, &exit_status, 0);
if (WIFEXITED(exit_status) && !WEXITSTATUS(exit_status))
- return 0;
+ goto done;
die("Subprocess %s exited abnormally", filter->cmd);
+
+done:
+ for (i = 0; i < filter->extra_args; i++)
+ filter->argv[i+1] = NULL;
+ return 0;
+
}
struct cgit_filter *cgit_new_filter(const char *cmd, filter_type filtertype)
{
struct cgit_filter *f;
int args_size = 0;
- int extra_args;
if (!cmd || !cmd[0])
return NULL;
+ f = xmalloc(sizeof(struct cgit_filter));
+ memset(f, 0, sizeof(struct cgit_filter));
+
switch (filtertype) {
case SOURCE:
case ABOUT:
- extra_args = 1;
+ f->extra_args = 1;
break;
case COMMIT:
default:
- extra_args = 0;
+ f->extra_args = 0;
break;
}
-
- f = xmalloc(sizeof(struct cgit_filter));
- memset(f, 0, sizeof(struct cgit_filter));
f->cmd = xstrdup(cmd);
- args_size = (2 + extra_args) * sizeof(char *);
+ args_size = (2 + f->extra_args) * sizeof(char *);
f->argv = xmalloc(args_size);
memset(f->argv, 0, args_size);
f->argv[0] = f->cmd;
diff --git a/ui-repolist.c b/ui-repolist.c
index d4ee279..f622a01 100644
--- a/ui-repolist.c
+++ b/ui-repolist.c
@@ -331,13 +331,9 @@ void cgit_print_site_readme()
{
if (!ctx.cfg.root_readme)
return;
- if (ctx.cfg.about_filter) {
- ctx.cfg.about_filter->argv[1] = ctx.cfg.root_readme;
- cgit_open_filter(ctx.cfg.about_filter);
- }
+ if (ctx.cfg.about_filter)
+ cgit_open_filter(ctx.cfg.about_filter, ctx.cfg.root_readme);
html_include(ctx.cfg.root_readme);
- if (ctx.cfg.about_filter) {
+ if (ctx.cfg.about_filter)
cgit_close_filter(ctx.cfg.about_filter);
- ctx.cfg.about_filter->argv[1] = NULL;
- }
}
diff --git a/ui-summary.c b/ui-summary.c
index 63a5a75..725f3ab 100644
--- a/ui-summary.c
+++ b/ui-summary.c
@@ -151,18 +151,17 @@ void cgit_print_repo_readme(char *path)
* filesystem, while applying the about-filter.
*/
html("");
- if (ctx.repo->about_filter) {
- ctx.repo->about_filter->argv[1] = filename;
- cgit_open_filter(ctx.repo->about_filter);
- }
+ if (ctx.repo
Re: [PATCH 4/6] filter: add fprintf_filter function
On Sun, Jan 12, 2014 at 08:23:02PM +0100, Jason A. Donenfeld wrote: > What's the purpose of this? Why not just keep the original string that > was passed to about-filter=... in the cmd variable as we have now? The > thing that's variable from filter to filter is argv, the type (commit, > about, etc), and the mechanism (lua, stdout, etc). But the variable > aspects don't require changing ->cmd do they? I'm looking at splitting up the data so there is a filter object that contains function pointers to implementation functions and then some data that is specific to to given filter type. With that change, cmd moves to the "exec filter" structure and is no longer accessible. I'm expecting some filter types to have a structured value in the config file that will be parsed to multiple fields which will be glued back together in the fprintf function for that filter type. ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH 1/3] ui-refs: escape HTML chars in author and tagger names
Everywhere else we use html_txt to escape any special characters in
these variables. Do so here as well.
Signed-off-by: John Keeping
---
I spotted this while looking at Jason's jd/gravatar series. The
following two patches cover other similar issues I spotted while
auditing all uses of "html()". I think everything else is OK.
ui-refs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/ui-refs.c b/ui-refs.c
index 20c91e3..c97b0c6 100644
--- a/ui-refs.c
+++ b/ui-refs.c
@@ -155,9 +155,9 @@ static int print_tag(struct refinfo *ref)
html("");
if (info) {
if (info->tagger)
- html(info->tagger);
+ html_txt(info->tagger);
} else if (ref->object->type == OBJ_COMMIT) {
- html(ref->commit->author);
+ html_txt(ref->commit->author);
}
html("");
if (info) {
--
1.8.5.226.g0d60d77
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH 3/3] ui-repolist: HTML-escape cgit_rooturl() response
This is for consistency with other callers. The value returned from
cgit_rooturl is not guaranteed to be HTML-safe.
Signed-off-by: John Keeping
---
ui-repolist.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/ui-repolist.c b/ui-repolist.c
index d4ee279..e8d5eb6 100644
--- a/ui-repolist.c
+++ b/ui-repolist.c
@@ -106,7 +106,9 @@ static int is_in_url(struct cgit_repo *repo)
static void print_sort_header(const char *title, const char *sort)
{
- htmlf("
[PATCH 2/3] ui-shared: URL-escape script_name
As far as I know, there is no requirement that $SCRIPT_NAME contain only
URL-safe characters, so we need to make sure that any special characters
are escaped.
Signed-off-by: John Keeping
---
ui-shared.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/ui-shared.c b/ui-shared.c
index 2c12de7..abe15cd 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -139,7 +139,7 @@ static void site_url(const char *page, const char *search,
const char *sort, int
if (ctx.cfg.virtual_root)
html_attr(ctx.cfg.virtual_root);
else
- html(ctx.cfg.script_name);
+ html_url_path(ctx.cfg.script_name);
if (page) {
htmlf("?p=%s", page);
@@ -219,7 +219,7 @@ static char *repolink(const char *title, const char *class,
const char *page,
html_url_path(path);
}
} else {
- html(ctx.cfg.script_name);
+ html_url_path(ctx.cfg.script_name);
html("?url=");
html_url_arg(ctx.repo->url);
if (ctx.repo->url[strlen(ctx.repo->url) - 1] != '/')
--
1.8.5.226.g0d60d77
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH] tests: add CGIT_TEST_OPTS variable to Makefile
This allows running the entire test suite with a set of command-line options. For example: make test CGIT_TEST_OPTS=--valgrind Signed-off-by: John Keeping --- tests/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Makefile b/tests/Makefile index 8c6c236..1556475 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -5,7 +5,7 @@ T = $(wildcard t[0-9][0-9][0-9][0-9]-*.sh) all: $(T) $(T): - @./$@ + @./$@ $(CGIT_TEST_OPTS) clean: $(RM) -rf trash -- 1.8.5.226.g0d60d77 ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH 2/3] ui-shared: URL-escape script_name
On Sun, Jan 12, 2014 at 10:18:30PM +0100, Jason A. Donenfeld wrote: > Are there any circumstances in which this could have prior lead to an XSS? I'm pretty sure this is entirely under the control of the system administrator, so it should be fine. ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH 1/3] ui-refs: escape HTML chars in author and tagger names
On Sun, Jan 12, 2014 at 11:02:01PM +0100, Jason A. Donenfeld wrote: > Same question here -- XSS potential? This is the one that worries me. But actually, Git strips "<", ">" and "\n" from GIT_*_NAME, so the question becomes whether we can manually construct a Git object to exploit this. I think the parsing.c::parse_user() function then saves us by stopping the name as soon as it hits "<". So there cannot be any way to insert HTML elements here. ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH] filter: basic write hooking infrastructure
On Sun, Jan 12, 2014 at 10:58:01PM +0100, Jason A. Donenfeld wrote:
> Filters can now call hook_write and unhook_write if they want to
> redirect writing to stdout to a different function. This saves us from
> potential file descriptor pipes and other less efficient mechanisms.
>
> We do this instead of replacing the call in html_raw because some places
> stdlib's printf functions are used (ui-patch or within git itself),
> which has its own internal buffering, which makes it difficult to
> interlace our function calls. So, we dlsym libc's write and then
> override it in the link stage.
>
> Signed-off-by: Jason A. Donenfeld
> ---
Clever. I've been looking at hooking in at a higher level before html.c
escapes the output. But I think the approach taken with source_filter
may be a better way of getting the raw output to the filter.
I can't help thinking it would be easier to just replace html_raw here
though. All our writes go through there anyway and we don't need to
worry about writing to other file descriptors.
> cgit.c | 2 ++
> cgit.h | 1 +
> cgit.mk | 4 +++-
> filter.c | 30 ++
> 4 files changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/cgit.c b/cgit.c
> index 4f31e58..725fd65 100644
> --- a/cgit.c
> +++ b/cgit.c
> @@ -904,6 +904,8 @@ int main(int argc, const char **argv)
> const char *path;
> int err, ttl;
>
> + cgit_init_filters();
> +
> prepare_context(&ctx);
> cgit_repolist.length = 0;
> cgit_repolist.count = 0;
> diff --git a/cgit.h b/cgit.h
> index 893c38f..db34493 100644
> --- a/cgit.h
> +++ b/cgit.h
> @@ -357,6 +357,7 @@ extern void cgit_fprintf_filter(struct cgit_filter
> *filter, FILE *f, const char
> extern void cgit_exec_filter_init(struct cgit_exec_filter *filter, char
> *cmd, char **argv);
> extern struct cgit_filter *cgit_new_filter(const char *cmd, filter_type
> filtertype);
> extern void cgit_cleanup_filters(void);
> +extern void cgit_init_filters(void);
>
> extern void cgit_prepare_repo_env(struct cgit_repo * repo);
>
> diff --git a/cgit.mk b/cgit.mk
> index 19a76e7..9d6dea8 100644
> --- a/cgit.mk
> +++ b/cgit.mk
> @@ -61,6 +61,8 @@ $(CGIT_VERSION_OBJS): $(CGIT_PREFIX)VERSION
> $(CGIT_VERSION_OBJS): EXTRA_CPPFLAGS = \
> -DCGIT_VERSION='"$(CGIT_VERSION)"'
>
> +CGIT_LIBS += -ldl
> +
>
> # Git handles dependencies using ":=" so dependencies in CGIT_OBJ are not
> # handled by that and we must handle them ourselves.
> @@ -88,4 +90,4 @@ $(CGIT_OBJS): %.o: %.c GIT-CFLAGS $(CGIT_PREFIX)CGIT-CFLAGS
> $(missing_dep_dirs)
> $(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS)
> $(CGIT_CFLAGS) $<
>
> $(CGIT_PREFIX)cgit: $(CGIT_OBJS) GIT-LDFLAGS $(GITLIBS)
> - $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^)
> $(LIBS)
> + $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^)
> $(LIBS) $(CGIT_LIBS)
> diff --git a/filter.c b/filter.c
> index 86c1d5d..d85b1bb 100644
> --- a/filter.c
> +++ b/filter.c
> @@ -12,6 +12,10 @@
> #include
> #include
> #include
> +#include
> +
> +static ssize_t (*libc_write)(int fd, const void *buf, size_t count);
> +static ssize_t (*filter_write)(int fd, const void *buf, size_t count) = NULL;
>
> static int open_exec_filter(struct cgit_filter *base, va_list ap)
> {
> @@ -192,3 +196,29 @@ void cgit_cleanup_filters(void)
> reap_filter(cgit_repolist.repos[i].source_filter);
> }
> }
> +
> +void cgit_init_filters(void)
> +{
> + libc_write = dlsym(RTLD_NEXT, "write");
> + if (!libc_write)
> + die("Could not locate libc's write function");
> +}
> +
> +ssize_t write(int fd, const void *buf, size_t count)
> +{
> + if (fd != STDOUT_FILENO || !filter_write)
> + return libc_write(fd, buf, count);
> + return filter_write(fd, buf, count);
> +}
> +
> +static inline void hook_write(ssize_t (*new_write)(int fd, const void *buf,
> size_t count))
> +{
> + /* We want to avoid buggy nested patterns. */
> + assert(filter_write == NULL);
> + filter_write = new_write;
> +}
Missing blank line here.
> +static inline void unhook_write()
> +{
> + assert(filter_write != NULL);
> + filter_write = NULL;
> +}
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH 06/12] filter: add preliminary lua support
On Mon, Jan 13, 2014 at 09:31:39AM +0100, Lukas Fleischer wrote: > This patch is quite messy and hard to read. I read your cover-letter but > maybe you still want to clean this up when dealing with the other > suggestions during a rebase -- shouldn't be too hard when using an > editor with good Git integration (like fugitive for Vim). > > On Mon, 13 Jan 2014 at 05:11:13, Jason A. Donenfeld wrote: > > [...] > > +ifdef NO_LUA > > We should document this in the installation instructions section in the > README. I also wonder whether this should made an opt-in feature? > > > + CFLAGS += -DNO_LUA > > +else > > + CGIT_LIBS += -llua > > Similar: Add Lua/LuaJIT (Will you squash the LuaJIT Makefile fix into > this one? Or is there any reason to use Lua first and switch to LuaJIT > later?) to the dependencies section of the README file and mention that > it is optional. I think we should support both vanilla Lua and LuaJIT if we can (I believe LuaJIT can be used as a drop-in replacement, so there's no reason this shouldn't be possible). > > +endif > > + > > +CGIT_LIBS += -ldl > > + > > + > > + > > CGIT_OBJ_NAMES += cgit.o > > CGIT_OBJ_NAMES += cache.o > > CGIT_OBJ_NAMES += cmd.o > > [...] ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH 06/12] filter: add preliminary lua support
On Mon, Jan 13, 2014 at 05:11:13AM +0100, Jason A. Donenfeld wrote:
[snip]
> +static int html_lua_filter(lua_State *lua_state)
> +{
> + size_t len;
> + const char *str;
> +
> + str = lua_tostring(lua_state, 1);
> + if (!str)
> + return 0;
> + len = strlen(str);
> + libc_write(STDOUT_FILENO, str, len);
> + return 0;
> +}
> +
> +static void cleanup_lua_filter(struct cgit_filter *base)
> +{
> + struct lua_filter *filter = (struct lua_filter *) base;
> +
> + if (!filter->lua_state)
> + return;
> +
> + lua_close(filter->lua_state);
> + filter->lua_state = NULL;
> + if (filter->script_file) {
> + free(filter->script_file);
> + filter->script_file = NULL;
> + }
> +}
> +
> +static int init_lua_filter(struct lua_filter *filter)
> +{
> + if (filter->lua_state)
> + return 0;
> +
> + if (!(filter->lua_state = luaL_newstate()))
> + return 1;
> +
> + luaL_openlibs(filter->lua_state);
> +
> + lua_pushcfunction(filter->lua_state, html_lua_filter);
> + lua_setglobal(filter->lua_state, "html");
It would be good to provide some of the other html_* functions here,
particularly html_txt so that filters don't need to re-invent the wheel
for escaping output. That's probably slightly harder than the plain
HTML, but I suspect we just need to wrap the call to the underlying
function in an unhook_write/hook_write pair.
> +
> + if (luaL_dofile(filter->lua_state, filter->script_file)) {
> + lua_close(filter->lua_state);
> + filter->lua_state = NULL;
> + return 1;
> + }
> + return 0;
> +}
> +
> +static int open_lua_filter(struct cgit_filter *base, va_list ap)
> +{
> + struct lua_filter *filter = (struct lua_filter *) base;
> + int i;
> +
> + if (init_lua_filter(filter))
> + return 1;
> +
> + hook_write(base, write_lua_filter);
> +
> + lua_getglobal(filter->lua_state, "filter_open");
> + for (i = 0; i < filter->base.argument_count; ++i)
> + lua_pushstring(filter->lua_state, va_arg(ap, char *));
> + if (lua_pcall(filter->lua_state, filter->base.argument_count, 0, 0))
> + return 1;
> + return 0;
> +}
> +
> +static int close_lua_filter(struct cgit_filter *base)
> +{
> + struct lua_filter *filter = (struct lua_filter *) base;
> + int ret = 0;
> +
> + lua_getglobal(filter->lua_state, "filter_close");
> + if (lua_pcall(filter->lua_state, 0, 0, 0))
> + ret = 1;
> + unhook_write();
> + return ret;
> +}
> +
> +static void fprintf_lua_filter(struct cgit_filter *base, FILE *f, const char
> *prefix)
> +{
> + struct lua_filter *filter = (struct lua_filter *) base;
> + fprintf(f, "%slua:%s\n", prefix, filter->script_file);
> +}
> +
> +
> +static struct cgit_filter *new_lua_filter(const char *cmd, int
> argument_count)
> +{
> + struct lua_filter *filter;
> +
> + filter = xmalloc(sizeof(*filter));
> + memset(filter, 0, sizeof(*filter));
> + filter->base.open = open_lua_filter;
> + filter->base.close = close_lua_filter;
> + filter->base.fprintf = fprintf_lua_filter;
> + filter->base.cleanup = cleanup_lua_filter;
> + filter->base.argument_count = argument_count;
> + filter->script_file = xstrdup(cmd);
> +
> + return &filter->base;
> }
>
> +#endif
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
Re: lua vs luajit vs both
On Tue, Jan 14, 2014 at 02:02:40AM +0100, Jason A. Donenfeld wrote: > I've gone ahead and merged the lua work to master, for testing and > subsequent cleanup before release. > > Regarding "to jit or not to jit", I currently have this fancy autodetection > logic: > http://git.zx2c4.com/cgit/commit/?id=3488d124052f5c3ddef303ed5306ad6a458794c1 > > John -- I'm waiting for your input on the parent email, as you seem to be > the originator of the opinion that "both are good". It was more of a "there doesn't seem much overhead to supporting both, since the API is the same". I think the Makefile should take an approach more like this though: ifdef NO_LUA CGIT_CFLAGS += -DNO_LUA else if defined(USE_LUAJIT) # LuaJIT code goes here else # Lua code goes here endif Basically, use vanilla Lua by default by provide an easy way for users to switch to LuaJIT if they want. ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: Idle time in project overview
On Tue, Jan 14, 2014 at 11:50:23AM +0100, Stefan Tatschner wrote: > I don't know if it is a bug or a feature but I think on this nice > mailing list I could ask without being shot or mutilated. :) > > If I have a git repository with multiple branches and I push to another > branch as 'master' the idle time on the project overview is not updated. > I think this behaviour is not correct. When I push to another branch as > master the project is absolutely not in an idle state. I think the idle > time should be updated when I push to any branch. This question comes up periodically. The answer is to use a hook to update an "agefile" on push. The best reference is this message: http://article.gmane.org/gmane.comp.version-control.cgit/1059 It would be nice to have that example hook in the repository somewhere (contrib/hooks/ ?) if someone feels like working up a patch... ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH] filter: refactor cgit_new_filter()
On Tue, Jan 14, 2014 at 02:00:48PM +0100, Jason A. Donenfeld wrote:
> From: Lukas Fleischer
>
> Use prefixcmp() as a preparation for using strip_prefix() later. Also,
> interpret the command as a file name if it contains a colon but none of
> the filter prefixes matches instead of bailing out and adding a special
> check for Windows.
>
> Signed-off-by: Lukas Fleischer
> ---
> John -- this is your code so I'm awaiting your input on this.
> Personally, I like receiving an error message that says "invalid filter
> type", and this patch kills that. But I'll go with whatever you prefer.
Yeah, it would be nice to keep the "unrecognised prefix" warning.
I like the simplification, but I'm not sure the result is better. Even
without the rest we should replace the strncmp with prefixcmp though.
There's actually no reason we couldn't mutate "cmd" here, which would
simplify it a lot, but I'm not sure we want to remove the const
modifiers all the way through. Then we can just do "*colon = '\0'" and
use strcmp.
> filter.c | 30 +++---
> 1 file changed, 7 insertions(+), 23 deletions(-)
>
> diff --git a/filter.c b/filter.c
> index 4d4acaf..1997881 100644
> --- a/filter.c
> +++ b/filter.c
> @@ -379,31 +379,19 @@ static const struct {
> const char *prefix;
> struct cgit_filter *(*ctor)(const char *cmd, int argument_count);
> } filter_specs[] = {
> - { "exec", new_exec_filter },
> + { "exec:", new_exec_filter },
> #ifndef NO_LUA
> - { "lua", new_lua_filter },
> + { "lua:", new_lua_filter },
> #endif
> };
>
> struct cgit_filter *cgit_new_filter(const char *cmd, filter_type filtertype)
> {
> - char *colon;
> - int i;
> - size_t len;
> - int argument_count;
> + int argument_count, i;
>
> if (!cmd || !cmd[0])
> return NULL;
>
> - colon = strchr(cmd, ':');
> - len = colon - cmd;
> - /*
> - * In case we're running on Windows, don't allow a single letter before
> - * the colon.
> - */
> - if (len == 1)
> - colon = NULL;
> -
> switch (filtertype) {
> case EMAIL:
> argument_count = 2;
> @@ -420,15 +408,11 @@ struct cgit_filter *cgit_new_filter(const char *cmd,
> filter_type filtertype)
> break;
> }
>
> - /* If no prefix is given, exec filter is the default. */
> - if (!colon)
> - return new_exec_filter(cmd, argument_count);
> -
> for (i = 0; i < ARRAY_SIZE(filter_specs); i++) {
> - if (len == strlen(filter_specs[i].prefix) &&
> - !strncmp(filter_specs[i].prefix, cmd, len))
> - return filter_specs[i].ctor(colon + 1, argument_count);
> + if (!prefixcmp(cmd, filter_specs[i].prefix))
> + return filter_specs[i].ctor(strchr(cmd, ':') + 1,
> argument_count);
Using strchr here feels wrong, why not:
cmd + strlen(filter_specs[i].prefix)
?
> }
>
> - die("Invalid filter type: %.*s", (int) len, cmd);
> + /* If no valid prefix is given, exec filter is the default. */
> + return new_exec_filter(cmd, argument_count);
> }
> --
> 1.8.5.2
>
> ___
> CGit mailing list
> CGit@lists.zx2c4.com
> http://lists.zx2c4.com/mailman/listinfo/cgit
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH] filter: refactor cgit_new_filter()
On Tue, Jan 14, 2014 at 09:54:21PM +0100, Jason A. Donenfeld wrote: > On Tue, Jan 14, 2014 at 9:39 PM, John Keeping wrote: > > I like the simplification, but I'm not sure the result is better. Even > > without the rest we should replace the strncmp with prefixcmp though. > > Agreed. > > > > There's actually no reason we couldn't mutate "cmd" here, which would > > simplify it a lot, but I'm not sure we want to remove the const > > modifiers all the way through. Then we can just do "*colon = '\0'" and > > use strcmp. > > IMHO, it's better to keep lookup tables like these in the read-only > section. Let's keep the constness. I meant for the input, which is read from the config file. Just terminate the user-specified command string at the colon and treat it as two genuinely separate strings. ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: lua vs luajit vs both
On Tue, Jan 14, 2014 at 07:06:34PM +0100, Jason A. Donenfeld wrote: > > > > On Tue, Jan 14, 2014 at 10:08 AM, John Keeping wrote: > > It was more of a "there doesn't seem much overhead to supporting both, > > since the API is the same". I think the Makefile should take an > > approach more like this though: > > > > ifdef NO_LUA > > CGIT_CFLAGS += -DNO_LUA > > else if defined(USE_LUAJIT) > > # LuaJIT code goes here > > else > > # Lua code goes here > > endif > > Okay we've got this fancy autodetection logic now. From the README: > > > If you'd like to compile without Lua support, you may use: > >$ make NO_LUA=1 > > And if you'd like to specify a Lua implementation, you may use: > >$ make LUA_IMPLEMENTATION=JIT > > for using the LuaJIT project. Or: > > > >$ make LUA_IMPLEMENTATION=VANILLA > > for the mainline Lua project. If you specify neither implementation, it will > > be auto-detected, preferring LuaJIT if both are present. > > From cgit.mk: > > > ifdef NO_LUA > > LUA_MESSAGE := linking without specified Lua support > > CGIT_CFLAGS += -DNO_LUA > > else > > LUAJIT_CFLAGS := $(shell pkg-config --cflags luajit 2>/dev/null) > > LUAJIT_LIBS := $(shell pkg-config --libs luajit 2>/dev/null) > > LUA_LIBS := $(shell pkg-config --libs lua 2>/dev/null) > > LUA_CFLAGS := $(shell pkg-config --cflags lua 2>/dev/null) > > ifeq (JIT,$(LUA_IMPLEMENTATION)) > > ifeq ($(strip $(LUAJIT_LIBS)),) > > $(error LuaJIT specified via LUA_IMPLEMENTATION=JIT, but library > > could not be found.) > > endif > > LUA_MESSAGE := linking with selected LuaJIT > > CGIT_LIBS += $(LUAJIT_LIBS) > > CGIT_CFLAGS += $(LUAJIT_CFLAGS) > > else ifeq (VANILLA,$(LUA_IMPLEMENTATION)) > > ifeq ($(strip $(LUA_LIBS)),) > > $(error Lua specified via LUA_IMPLEMENTATION=VANILLA, but library > > could not be found.) > > endif > > LUA_MESSAGE := linking with selected Lua > > CGIT_LIBS += $(LUA_LIBS) > > CGIT_LIBS += $(LUA_CFLAGS) > > else ifneq ($(strip $(LUAJIT_LIBS)),) > > LUA_MESSAGE := linking with autodetected LuaJIT > > CGIT_LIBS += $(LUAJIT_LIBS) > > CGIT_CFLAGS += $(LUAJIT_CFLAGS) > > else ifneq ($(strip $(LUA_LIBS)),) > > LUA_MESSAGE := linking with autodetected Lua > > CGIT_LIBS += $(LUA_LIBS) > > CGIT_CFLAGS += $(LUA_CFLAGS) > > else > > LUA_MESSAGE := linking without autodetected Lua support > > NO_LUA := YesPlease > > CGIT_CFLAGS += -DNO_LUA > > endif > > > > endif > > > > # Add -ldl to linker flags on non-BSD systems. > > ifeq ($(findstring BSD,$(uname_S)),) > > CGIT_LIBS += -ldl > > endif > > How's this look to you? The correct way to be doing things? I think it does the right thing for all the explicitly specified combinations. Personally I would let the compiler error out if Lua isn't installed, and add some documentation in Makefile to point users at NO_LUA, but I don't feel particularly strongly about that, and since you've done the hard work to make it more intelligent... ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: Policy on global variables
On Thu, Jan 16, 2014 at 12:31:15PM +0100, Jason A. Donenfeld wrote: > On Thu, Jan 16, 2014 at 11:47 AM, Eric Wong wrote: > > Lars Hjemli wrote: > >> Supporting something like FCGI in cgit will require a fork(2) for each > >> request, before invoking libgit.a functions, since these functions are > >> not generally reentrant (they tend to use global state and/or > >> inconveniently die(3)). > > > > Unfortunately true for now, but libgit.a could evolve (or cgit can use > > something like libgit2 instead). > > Cgit is unlikely to move to libgit2 in the near future. (Unless > someone is willing to do the job and argue for why it's preferred over > mainline git, beyond its reentrancy...) > > I guess, though, libgit.a is likely to never evolve to receive > reentrant functions and do away with die() (though the die calls could > easily be circumvented by hooking libc's exit...yuck), because libgit2 > exists for this reason. I had a look at porting to libgit2 about a year ago and it mostly isn't too bad. IIRC the only problematic area is the graph output which we currently get from libgit.a but would have to do ourselves if we switch to libgit2. ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: Policy on global variables
On Thu, Jan 16, 2014 at 07:38:02PM +0100, Jason A. Donenfeld wrote: > On Thu, Jan 16, 2014 at 2:08 PM, John Keeping wrote: > > > > I had a look at porting to libgit2 about a year ago and it mostly isn't > > too bad. IIRC the only problematic area is the graph output which we > > currently get from libgit.a but would have to do ourselves if we switch > > to libgit2. > > Are there any downsides of doing this? I know we've put a lot of work > into cozying up with internal git utility functions and their build > system. Would we have to reimplement a lot of this? Would it be worth > it? Are there general benefits of using libgit2 over what we have now? > Are there general downsides? Given the CGit and Git are both GPLv2, we could always just take strbuf.[ch] and the argv-array bits from git.git and copy them into CGit. Likewise the test suite could switch to using Sharness with very little effort. So I don't think the recent changes towards using more bits of Git actually have too much impact here. > More importantly, is this something you would be willing to do, if we > decided it was the best direction? I won't have time to do this in the near future. The first step in this direction may actually be useful even if we stick with embedding libgit.a. Re-writing the commit graph drawing ourselves could allow prettier output than the ASCII we inherit from Git. ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: Policy on global variables
On Thu, Jan 16, 2014 at 10:26:08PM +0100, Jason A. Donenfeld wrote: > On Thu, Jan 16, 2014 at 10:21 PM, John Keeping wrote: > > The first step in this direction may actually be useful even if we stick > > with embedding libgit.a. > > So what do you think ought to be done with the global-ctx patch? Merge > it, and then refactor afterward (whenever we "step in this > direction"), to get rid of the global? Or use what we have now > (without the patch) as a starting point for gradually getting rid of > the global? I'm not sure it makes much difference either way. Even if we use libgit2, providing we're not processing more than one request at once we can still use a global cgit_context. ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: Policy on global variables
On Thu, Jan 16, 2014 at 10:36:34PM +0100, Jason A. Donenfeld wrote: > On Thu, Jan 16, 2014 at 10:34 PM, John Keeping wrote: > > > > I'm not sure it makes much difference either way. Even if we use > > libgit2, providing we're not processing more than one request at once we > > can still use a global cgit_context. > > Well, the idea of moving to libgit2, in the first place, would be to > benefit from its reentrancy, so that we could process multiple > requests at once (potentially). At once (as in in parallel), or without needing to fork for every request? I think that many requests serially in the same process is a much more likely scenario (that's what FastCGI does); in that case all we need to do is clean up after each request and it doesn't make much difference if that state is global or passed down through the functions that need it. ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: The road to v0.10.1 or v0.11
On Fri, Jan 17, 2014 at 05:12:26PM +0100, Jason A. Donenfeld wrote: > Here's what I'm thinking about for the next release (or releases?): > > + FastCGI support I really can't see this being sensible without moving to libgit2. As long as we stick with libgit.a then we need to fork for each request so I'm not sure there's much benefit to supporting FastCGI without moving to something that lets us free resources when we're done processing a request. > + More malloc()/free() cleanups > + git-blame support > + git-grep support > + HTML5 compliance > + More filters everywhere > + Expanded test suite > + Line number anchors highlighting the current line > + sendfile() support > > This is in no particular order. Any opinions about priorities? Or > anything else to add? I'd like to get "new graph implementation" into this list - it's come up on the list twice in the last 24 hours! That doesn't mean I'm claiming the task though ;-) ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: The road to v0.10.1 or v0.11
On Fri, Jan 17, 2014 at 05:38:29PM +0100, Jason A. Donenfeld wrote: > On Fri, Jan 17, 2014 at 5:28 PM, John Keeping wrote: > > I really can't see this being sensible without moving to libgit2. As > > long as we stick with libgit.a then we need to fork for each request so > > I'm not sure there's much benefit to supporting FastCGI without moving > > to something that lets us free resources when we're done processing a > > request. > > The advantage would be not having to reparse the config and scan for > repos on every.single.solitary.request. But scan for repos is caught by the cache most of the time, and presumably even if we run persistently we still need to do that periodically (or use inotify); or do we just rely on the process being replaced when the set of repositories changes? ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: The road to v0.10.1 or v0.11
On Fri, Jan 17, 2014 at 06:09:15PM +0100, Jason A. Donenfeld wrote: > On Fri, Jan 17, 2014 at 5:38 PM, Jason A. Donenfeld wrote: > > On Fri, Jan 17, 2014 at 5:28 PM, John Keeping wrote: > >> I really can't see this being sensible without moving to libgit2. As > >> long as we stick with libgit.a then we need to fork for each request so > >> I'm not sure there's much benefit to supporting FastCGI without moving > >> to something that lets us free resources when we're done processing a > >> request. > > > > The advantage would be not having to reparse the config and scan for > > repos on every.single.solitary.request. > > Oh, and we could avoid a fork() for cached pages... Good point. I think that probably does make it worthwhile. It may even make sense for a FastCGI process to do: while read_request if not in cache: process and exit return_cache and just rely on the web server restarting us. ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: The road to v0.10.1 or v0.11
On Fri, Jan 17, 2014 at 02:29:19PM -0500, Konstantin Ryabitsev wrote: > On 17/01/14 01:22 PM, Jason A. Donenfeld wrote: > >> But scan for repos is caught by the cache most of the time, and > >> > presumably even if we run persistently we still need to do that > >> > periodically (or use inotify); or do we just rely on the process being > >> > replaced when the set of repositories changes? > > Generally the idea is you restart the fcgi process when things change, > > or at least send it a SIGUSR1. But we could be fancy and support > > inotify/kqueue... > > The process that updates the repositories may not have permissions to > send SIGUSR1 to the fcgid process -- either because they are running as > different users or because there are SELinux policies preventing it. > > It's really best if cgit recognizes when things like projects.list have > changed. Presumably you are OK with this having the same latency as the existing cache mechanism. The simplest implementation will probably be to keep the existing "cache valid?" check and re-scan repositories as we currently do. ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH] gen-version.sh: check if git is available before trying to call it
On Sat, Feb 01, 2014 at 02:23:49PM +0100, Fabien C. wrote: > On 01/02/2014 11:07, Jason A. Donenfeld wrote: > > Maybe you want to direct the output to /dev/null? > > You're right, that was a too quick fix. > > Here you go with the /dev/null redirect. > From 3dc2ce06df3ccbdae9c05325e93cbbcabc1d1b7f Mon Sep 17 00:00:00 2001 > From: Fabien C. > Date: Sat, 1 Feb 2014 14:18:29 +0100 > Subject: [PATCH] gen-version.sh: check if git is available before trying to > call it > > Some people may clone the cgit repository and compile within a sandbox > or on another machine where git is not necessarily installed. When it > happens, cgit is getting compiled with an empty version number. > > This commit fixes this. > --- > gen-version.sh |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/gen-version.sh b/gen-version.sh > index 3a08015..13ff979 100755 > --- a/gen-version.sh > +++ b/gen-version.sh > @@ -4,7 +4,7 @@ > V=$1 > > # Use `git describe` to get current version if we're inside a git repo > -if test -d .git > +if test -d .git && command -v git > /dev/null Style: no space between redirect and file: >/dev/null I'm not sure command is the most portable way to achieve this, how about this instead: git --version >/dev/null 2>&1 > then > V=$(git describe --abbrev=4 HEAD 2>/dev/null) > fi > -- > 1.7.10.4 ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: Can I get detailed Cgit logs?
On Tue, Feb 25, 2014 at 11:53:30AM -0600, Nolan Darilek wrote: > I'm encountering the infamous "no repositories found" issue. I've > exhaustively checked permissions, up to and including assigning a shell > to the www-data user and verifying that it can read my Git repositories. > I've checked every directory permission from /, and it all looks good. > > One complicating factor of my setup is that it is in Docker > (http://docker.io) containers. I currently have a Docker container > hosting Git repositories via Gitolite, another running Jenkins, and > Cgit. Jenkins communicates with Gitolite just fine, though it uses SSH > which I know works. > > It would be very helpful if Cgit gave me an exact error as to why it > can't access my repositories. I had to jump through a hoop with Selinux > on 13.10 in the Gitolite container, so if that's tripping up Cgit for > some reason then knowing would be helpful. How is CGit able to see the repositories? Unlike Jenkins, CGit does not speak any of the Git protocols, it expects to see repositories in the file system. Are you Docker containers sharing an underlying filesystem, or do you have some external job that is cloning the repositories for CGit? > Is there any way to get detailed error logs? I don't see any in my > Apache logs, which could mean a misconfiguration on that end. But having > access to some sort of detailed error log would be very helpful in > debugging this. Since CGit is a CGI program, you can just run it and see what happens. You may want to export PATH_INFO=/ before doing so to make sure it generates the index. You can run under strace to monitor what is going on during repository discovery. ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: Can I get detailed Cgit logs?
On Tue, Feb 25, 2014 at 12:40:40PM -0600, Nolan Darilek wrote: > On 02/25/2014 12:08 PM, John Keeping wrote: > > How is CGit able to see the repositories? Unlike Jenkins, CGit does > > not speak any of the Git protocols, it expects to see repositories in > > the file system. Are you Docker containers sharing an underlying > > filesystem, or do you have some external job that is cloning the > > repositories for CGit? > > Sharing an underlying filesystem, yes. Git repositories are in a > volume shared by Gitolite and Cgit. > > >> Is there any way to get detailed error logs? I don't see any in my > >> Apache logs, which could mean a misconfiguration on that end. But having > >> access to some sort of detailed error log would be very helpful in > >> debugging this. > > > > Since CGit is a CGI program, you can just run it and see what happens. > > You may want to export PATH_INFO=/ before doing so to make sure it > > generates the index. > > OK, new complicating factor. If I spin up a container that I can > interact with (I.e. running a shell) then running cgit.cgi directly > shows my repositories. If I spin up a non-interactive container (I.e. > running in the background) then I see no repositories. The only > difference between these environments should be the lack of an attached > stdin/stdout. Is there any chance that the CGIT_CONFIG environment variable is defined differently for the two invocations? If it is not defined then CGit will default to a built-in location (by default /etc/cgitrc). > So is there any way to redirect CGI errors to the Apache error log? I > have the following in my configuration: > >LogLevel info >CustomLog |cat combined > > My assumption is that this will redirect all relevant logs to stdout so > they are visible in the "docker logs" command. But I only see server > accesses, no CGI errors. I have never tried using a CustomLog like that, but with a default logging setup I do see CGit errors in /var/log/apache2/error_log. ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: Can I get detailed Cgit logs?
On Tue, Feb 25, 2014 at 01:24:02PM -0600, Nolan Darilek wrote: > OK, figured it out. I was proxying to the Cgit docker container via > Nginx, and specified: > > proxy_pass http://cgit.prod.docker; > > instead of: > > proxy_pass http://cgit.prod.docker/; > > Maybe some sort of "debug mode" might have helped diagnose the issue? > Presumably Cgit was looking for repositories in a directory that didn't > exist. Having a way to output "No repositories found in > /srv/git/doesnotexist" might have helped me track that down more > quickly. Or maybe such a thing exists and my Google powers are weak. :) CGit does log "Error opening directory" if either the directory specified to scan does not exist or a directory listed in the project list does not exist. ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: Bug: alternates not followed when enable-http-clone=1
On Mon, Apr 28, 2014 at 12:18:39PM -0400, Konstantin Ryabitsev wrote:
> Someone tried to clone a cgit URL instead of the canonical clone URL for
> one of the projects and I thus discovered that cgit's http-clone support
> doesn't do the right thing with repositories that use alternates.
> Probably should be fixed.
Do you have absolute or relative paths in $GIT_DIR/info/alternates?
I can't see anything that CGit's HTTP implementation does differently
from git-http-backend, but gitrepository-layout(5) indicates that you
may need to create $GIT_DIR/info/http-alternates as well to provide the
correct paths to HTTP clients.
If you don't want CGit's HTTP clone, it might make more sense to just
turn it off in cgitrc ("enable-http-clone = 0").
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
Re: Error when searching for a bogus range
On Tue, Jun 10, 2014 at 02:05:14PM -0400, Konstantin Ryabitsev wrote:
> cgit-0.10.1-1.el6 (EPEL version)
>
> If you search for a bogus range string here:
>
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/log/
>
> Using something like "range" and "qwerty123456", it returns an "Internal
> Server Error" and the following in the logs:
>
> > [Tue Jun 10 17:45:32 2014] [error] [client 172.21.1.6] fatal: ambiguous
> > argument 'qwerty123456': unknown revision or path not in the working tree.,
> > referer: http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/
> > [Tue Jun 10 17:45:32 2014] [error] [client 172.21.1.6] Use '--' to separate
> > paths from revisions, like this:, referer:
> > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/
> > [Tue Jun 10 17:45:32 2014] [error] [client 172.21.1.6] 'git
> > [...] -- [...]', referer:
> > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/
> > [Tue Jun 10 17:45:32 2014] [error] [client 172.21.1.6] Premature end of
> > script headers: cgit, referer:
> > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/
This is because we just pass the arguments straight to Git's revision
parsing machinery which die()s if it cannot parse an argument, printing
the above to stderr and exiting.
The patch below makes it a bit friendlier by just ignoring unhandled
arguments, but I can't see an easy way to report errors when we can't
parse revision arguments without losing the flexibility of supporting
all of the revision specifiers supported by Git.
-- >8 --
diff --git a/ui-log.c b/ui-log.c
index 499534c..4cb26bc 100644
--- a/ui-log.c
+++ b/ui-log.c
@@ -337,16 +337,16 @@ void cgit_print_log(const char *tip, int ofs, int cnt,
char *grep, char *pattern
else if (commit_sort == 2)
argv_array_push(&rev_argv, "--topo-order");
- if (path) {
- argv_array_push(&rev_argv, "--");
+ argv_array_push(&rev_argv, "--");
+ if (path)
argv_array_push(&rev_argv, path);
- }
init_revisions(&rev, NULL);
rev.abbrev = DEFAULT_ABBREV;
rev.commit_format = CMIT_FMT_DEFAULT;
rev.verbose_header = 1;
rev.show_root_diff = 0;
+ rev.ignore_missing = 1;
setup_revisions(rev_argv.argc, rev_argv.argv, &rev, NULL);
load_ref_decorations(DECORATE_FULL_REFS);
rev.show_decorations = 1;
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH] ui-patch: Flush stdout after outputting data
Since the "html" functions use raw write(2) to STDIO_FILENO, we don't
notice problems with most pages, but raw patches write using printf(3).
This is fine if we're outputting straight to stdout since the buffers
are flushed on exit, but we close the cache output before this, so the
cached output ends up being truncated.
Make sure the buffers are flushed when we finish outputting a patch so
that we avoid this.
No other UIs use printf(3) so we do not need to worry about them.
Signed-off-by: John Keeping
---
ui-patch.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/ui-patch.c b/ui-patch.c
index 6878a46..fc6c145 100644
--- a/ui-patch.c
+++ b/ui-patch.c
@@ -82,4 +82,6 @@ void cgit_print_patch(const char *new_rev, const char
*old_rev,
log_tree_commit(&rev, commit);
printf("-- \ncgit %s\n\n", cgit_version);
}
+
+ fflush(stdout);
}
--
2.0.0.rc4.417.gc642ced
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH] ui-patch: Flush stdout after outputting data
On Wed, Jun 11, 2014 at 04:28:26PM -0400, Konstantin Ryabitsev wrote: > On 11/06/14 04:01 PM, John Keeping wrote: > > Since the "html" functions use raw write(2) to STDIO_FILENO, we don't > > notice problems with most pages, but raw patches write using printf(3). > > This is fine if we're outputting straight to stdout since the buffers > > are flushed on exit, but we close the cache output before this, so the > > cached output ends up being truncated. Actually, it's slightly more interesting than this... since we don't set GIT_FLUSH, Git decides whether or not it will flush stdout after writing each commit based on whether or not stdout points to a regular file (in maybe_flush_or_die()). Which means that when writing directly to the webserver, Git flushes stdout for us, but when we redirect stdout to the cache it points to a regular file so Git no longer flushes the output for us. The patch is still correct, but perhaps the full explanation is interesting! ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH 3/8] Skip forbidden characters.
On Tue, Jul 01, 2014 at 09:40:28AM +0200, zwin...@kit.edu wrote:
> From: Sebastian Buchwald
Why do we want to do this? Does it not break anything that uses
whitespace="pre" (explicitly or implicitly)?
> ---
> html.c | 9 ++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/html.c b/html.c
> index 91047ad..6037eec 100644
> --- a/html.c
> +++ b/html.c
> @@ -129,7 +129,8 @@ void html_txt(const char *txt)
> const char *t = txt;
> while (t && *t) {
> int c = *t;
> - if (c == '<' || c == '>' || c == '&') {
> + if ((c < 0x20 && c != '\t' && c != '\n' && c != '\r')
> + || (c == '<' || c == '>' || c == '&')) {
> html_raw(txt, t - txt);
> if (c == '>')
> html(">");
> @@ -150,7 +151,8 @@ void html_ntxt(int len, const char *txt)
> const char *t = txt;
> while (t && *t && len--) {
> int c = *t;
> - if (c == '<' || c == '>' || c == '&') {
> + if ((c < 0x20 && c != '\t' && c != '\n' && c != '\r')
> + || (c == '<' || c == '>' || c == '&')) {
> html_raw(txt, t - txt);
> if (c == '>')
> html(">");
> @@ -186,7 +188,8 @@ void html_attr(const char *txt)
> const char *t = txt;
> while (t && *t) {
> int c = *t;
> - if (c == '<' || c == '>' || c == '\'' || c == '\"' || c == '&')
> {
> + if (c == '<' || c == '>' || c == '\'' || c == '\"' || c == '&'
> + || (c < 0x20 && c != '\t' && c != '\n' && c !=
> '\r')) {
> html_raw(txt, t - txt);
> if (c == '>')
> html(">");
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
Re: certificate problem with libravatar
On Thu, Jul 03, 2014 at 11:16:21AM +0200, Christian Hesse wrote: > looks like we have a certificate problem with libravatar email filter. For > base URL we use "//cdn.libravatar.org/", with is fine if cgit serves > unencrypted html pages. The url evaluates to "http://cdn.libravatar.org/"; > then. However if cgit sends an encrypted site the url is > "https://cdn.libravatar.org/";, with results in a certificate error as CN does > not match. > > We could just change the url to "//seccdn.libravatar.org/" or > "https://seccdn.libravatar.org/";, but that would fetch the avatar via https > all the some. In fact the first one makes two requests as the http server > redirects to https one. > > Does the script know whether or not the site is encrypted? That would allow > us to choose the correct url. Any other ideas? FWIW my vote would be to always use "https://seccdn.libravatar.org/";, since HTTP->HTTPS is OK but HTTPS->HTTP is not and if HTTP is just going to redirect to HTTPS then we might as well go directly to the HTTPS. ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: owner filter?
On Thu, Jul 17, 2014 at 10:10:38AM -0400, Chris Burroughs wrote: > We would like to decorate the owner field (make a link to a wiki for > internal teams, maybe an icon). I've read the FILTER API and other > parts of cgitrc and as far as I can tell it's not an existing filter > option. Is that correct? Yes, it would need to new filter added in order to filter the owner field in cgit_print_repolist(). Presumably the interface would be similar to that of the email filter. ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH 3/3] ui-stats.c: set parent pointer to NULL after freeing it
We do this everywhere else, so we should be doing it here as well. Signed-off-by: John Keeping --- ui-stats.c | 1 + 1 file changed, 1 insertion(+) diff --git a/ui-stats.c b/ui-stats.c index 6f13c32..a264f6a 100644 --- a/ui-stats.c +++ b/ui-stats.c @@ -246,6 +246,7 @@ static struct string_list collect_stats(struct cgit_period *period) add_commit(&authors, commit, period); free_commit_buffer(commit); free_commit_list(commit->parents); + commit->parents = NULL; } return authors; } -- 2.0.1.472.g6f92e5f.dirty ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH 2/3] git: update to v2.0.3
This is slightly more involved than just bumping the version number
because it pulls in a change to convert the commit buffer to a slab,
removing the "buffer" field from "struct commit". All sites that access
"commit->buffer" have been changed to use the new functions provided for
this purpose.
Signed-off-by: John Keeping
---
Makefile | 2 +-
git| 2 +-
parsing.c | 3 ++-
ui-atom.c | 3 +--
ui-log.c | 6 ++
ui-stats.c | 2 +-
6 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/Makefile b/Makefile
index bf8be02..93b525a 100644
--- a/Makefile
+++ b/Makefile
@@ -14,7 +14,7 @@ htmldir = $(docdir)
pdfdir = $(docdir)
mandir = $(prefix)/share/man
SHA1_HEADER =
-GIT_VER = 2.0.1
+GIT_VER = 2.0.3
GIT_URL = https://www.kernel.org/pub/software/scm/git/git-$(GIT_VER).tar.gz
INSTALL = install
COPYTREE = cp -r
diff --git a/git b/git
index 341e7e8..740c281 16
--- a/git
+++ b/git
@@ -1 +1 @@
-Subproject commit 341e7e8eda3dbeb6867f4f8f45b671201b807de5
+Subproject commit 740c281d21ef5b27f6f1b942a4f2fc20f51e8c7e
diff --git a/parsing.c b/parsing.c
index edb3416..3dbd122 100644
--- a/parsing.c
+++ b/parsing.c
@@ -132,7 +132,8 @@ static const char *reencode(char **txt, const char
*src_enc, const char *dst_enc
struct commitinfo *cgit_parse_commit(struct commit *commit)
{
struct commitinfo *ret;
- const char *p = commit->buffer, *t;
+ const char *p = get_cached_commit_buffer(commit, NULL);
+ const char *t;
ret = xmalloc(sizeof(*ret));
ret->commit = commit;
diff --git a/ui-atom.c b/ui-atom.c
index b22d745..e2b39ee 100644
--- a/ui-atom.c
+++ b/ui-atom.c
@@ -133,8 +133,7 @@ void cgit_print_atom(char *tip, char *path, int max_count)
}
while ((commit = get_revision(&rev)) != NULL) {
add_entry(commit, host);
- free(commit->buffer);
- commit->buffer = NULL;
+ free_commit_buffer(commit);
free_commit_list(commit->parents);
commit->parents = NULL;
}
diff --git a/ui-log.c b/ui-log.c
index b5846e4..bcdb666 100644
--- a/ui-log.c
+++ b/ui-log.c
@@ -388,16 +388,14 @@ void cgit_print_log(const char *tip, int ofs, int cnt,
char *grep, char *pattern
ofs = 0;
for (i = 0; i < ofs && (commit = get_revision(&rev)) != NULL; i++) {
- free(commit->buffer);
- commit->buffer = NULL;
+ free_commit_buffer(commit);
free_commit_list(commit->parents);
commit->parents = NULL;
}
for (i = 0; i < cnt && (commit = get_revision(&rev)) != NULL; i++) {
print_commit(commit, &rev);
- free(commit->buffer);
- commit->buffer = NULL;
+ free_commit_buffer(commit);
free_commit_list(commit->parents);
commit->parents = NULL;
}
diff --git a/ui-stats.c b/ui-stats.c
index bc27308..6f13c32 100644
--- a/ui-stats.c
+++ b/ui-stats.c
@@ -244,7 +244,7 @@ static struct string_list collect_stats(struct cgit_period
*period)
memset(&authors, 0, sizeof(authors));
while ((commit = get_revision(&rev)) != NULL) {
add_commit(&authors, commit, period);
- free(commit->buffer);
+ free_commit_buffer(commit);
free_commit_list(commit->parents);
}
return authors;
--
2.0.1.472.g6f92e5f.dirty
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH 0/3] Update to Git 2.0.3
This isn't as simple as just bumping the version number because the "buffer" field has been removed from "struct commit" so we need to change all of the places that use it to use the new wrapper functions. The first commit is a preparatory change because the new access function returns a pointer-to-const where the "buffer" field was previously just a "char *". The middle commit is the real change and the final one is just an unrelated fix I noticed while in the area. John Keeping (3): parsing.c: make commit buffer const git: update to v2.0.3 ui-stats.c: set parent pointer to NULL after freeing it Makefile | 2 +- git| 2 +- parsing.c | 9 + ui-atom.c | 3 +-- ui-log.c | 6 ++ ui-stats.c | 3 ++- 6 files changed, 12 insertions(+), 13 deletions(-) -- 2.0.1.472.g6f92e5f.dirty ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH 1/3] parsing.c: make commit buffer const
This will be required in order to incorporate the changes to commit
buffer handling in Git 2.0.2.
Signed-off-by: John Keeping
---
parsing.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/parsing.c b/parsing.c
index 073f46f..edb3416 100644
--- a/parsing.c
+++ b/parsing.c
@@ -69,9 +69,9 @@ static char *substr(const char *head, const char *tail)
return buf;
}
-static char *parse_user(char *t, char **name, char **email, unsigned long
*date)
+static const char *parse_user(const char *t, char **name, char **email,
unsigned long *date)
{
- char *p = t;
+ const char *p = t;
int mode = 1;
while (p && *p) {
@@ -132,7 +132,7 @@ static const char *reencode(char **txt, const char
*src_enc, const char *dst_enc
struct commitinfo *cgit_parse_commit(struct commit *commit)
{
struct commitinfo *ret;
- char *p = commit->buffer, *t;
+ const char *p = commit->buffer, *t;
ret = xmalloc(sizeof(*ret));
ret->commit = commit;
@@ -223,7 +223,7 @@ struct taginfo *cgit_parse_tag(struct tag *tag)
void *data;
enum object_type type;
unsigned long size;
- char *p;
+ const char *p;
struct taginfo *ret;
data = read_sha1_file(tag->object.sha1, &type, &size);
--
2.0.1.472.g6f92e5f.dirty
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
Re: source-filter isn't getting applied
On Mon, Jul 28, 2014 at 05:14:43PM -0400, Nik Nyby wrote: > I have cgit installed and the source-filter isn't working on any of my > source files. I have Python and Pygments installed. I tried manually > running the script on some files, and it's giving back html > correctly. > > Here's my /etc/cgitrc: > > scan-path=/home/git > > branch-sort=age > repository-sort=age > > source-filter=/usr/local/lib/cgit/filters/syntax-highlighting.py > about-filter=/usr/local/lib/cgit/filters/about-formatting.sh > > > My cgit is installed in /var/www/cgit, and I'm using cgit v0.10.2. > > Let me know if you have any suggestions. What are the permissions on the script files? How do you run them manually? Assuming that CGit is definitely reading that `cgitrc` file, I'd guess that the permissions are not set correctly for the user `cgit` runs as when called from your web server. If all else fails, you can try moving CGit out of the way and replacing it with something like this: -- >8 -- #!/bin/sh strace -o /tmp/cgit.strace /path/to/real/cgit "$@" -- 8< -- ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: Fwd: source-filter isn't getting applied
On Mon, Jul 28, 2014 at 11:59:10PM -0400, Nik Nyby wrote: > The permissions on the script files are set to be executable by everyone: > -rwxr-xr-x > > Thanks for the strace idea. I'm looking through the strace, but I haven't > seen any helpful mention of the filter scripts yet. I've attached an > abridged version of my strace with the middle of the file taken out. I > don't know if this is helpful or not. I'll let you know if I solve the > problem. If CGit was going to open a filter, it would do so after this line: write(1, "", 29) = 29 So it looks like something in the configuration isn't being read correctly, but I think you snipped the strace log before we could see which config file it reads. ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: Fwd: source-filter isn't getting applied
On Wed, Jul 30, 2014 at 12:17:26AM -0400, Nik Nyby wrote: > Great, this helped me debug the error. I looked at the part of the strace > where it was reading the /etc/cgitrc, and it looked like it was only > reading the first setting, even though the following settings are getting > applied. I'm not sure exactly why yet, but putting the source-filter > setting at the top of the /etc/cgitrc fixes the problem, and my source > files are now highlighted. I suspect that was just strace truncating the data, but at least it pointed to the answer! Global settings get frozen for each repository when it is added, so you should generally do all the configuration near the top of the file and then add repositories (either explicity or with "scan-path=...") near the bottom. > Now I'm dealing with the same problem with the about-filter, because > putting that setting at the top isn't giving me markdown highlighting. > > > On Tue, Jul 29, 2014 at 3:37 AM, John Keeping wrote: > > > On Mon, Jul 28, 2014 at 11:59:10PM -0400, Nik Nyby wrote: > > > The permissions on the script files are set to be executable by everyone: > > > -rwxr-xr-x > > > > > > Thanks for the strace idea. I'm looking through the strace, but I haven't > > > seen any helpful mention of the filter scripts yet. I've attached an > > > abridged version of my strace with the middle of the file taken out. I > > > don't know if this is helpful or not. I'll let you know if I solve the > > > problem. > > > > If CGit was going to open a filter, it would do so after this line: > > > > write(1, "", 29) = 29 > > > > So it looks like something in the configuration isn't being read > > correctly, but I think you snipped the strace log before we could see > > which config file it reads. ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH] ui-tree.c: check source filter if set globally
On Wed, Jul 30, 2014 at 02:53:21PM -0400, Jamie Couture wrote:
> When parsing cgitrc users that place 'source-filter' setting after
> 'scan-path' find their source filter is never run.
>
> scan-path=/home/git/repositories
> source-filter=/usr/local/lib/cgit/filters/syntax-highlighting.sh
>
> ui-tree.c will print out our repository objects, but will only consider
> ctx.repo->source_filter struct and not checking ctx.cfg.source_filter
>
> The value would have been set in shared.c, via cgit_add_repo() but this
> isn't the case when using scan-path, because we have not yet processed
> the source-filter line in our cgitrc, and thus the source_filter struct
> will not be initialized.
>
> Checking the global configuration in ui-tree.c is necessary to avoid an
> issue where users declare the source filter after scan-path.
I think this is OK because we only fall back if repo->source_filter is
unset and there is no way to unset the source filter for a repository
AFAICT. But if we do this for source-filter, why not also about-filter,
email-filter and commit-filter?
The documentation already makes it clear that settings should be
configured before "scan-path", and scan-path isn't special here you
could equally well demonstrate the same effect with:
repo.url=my-repository
source-filter=/path/to/source-filter.sh
I'm not convinced that this change makes things less confusing, it just
means that "everything except source-filter must be configured before
adding repositories", which seems more confusing.
The full list of captured settings is:
ret->section = ctx.cfg.section;
ret->snapshots = ctx.cfg.snapshots;
ret->enable_commit_graph = ctx.cfg.enable_commit_graph;
ret->enable_log_filecount = ctx.cfg.enable_log_filecount;
ret->enable_log_linecount = ctx.cfg.enable_log_linecount;
ret->enable_remote_branches = ctx.cfg.enable_remote_branches;
ret->enable_subject_links = ctx.cfg.enable_subject_links;
ret->max_stats = ctx.cfg.max_stats;
ret->branch_sort = ctx.cfg.branch_sort;
ret->commit_sort = ctx.cfg.commit_sort;
ret->module_link = ctx.cfg.module_link;
ret->readme = ctx.cfg.readme;
ret->about_filter = ctx.cfg.about_filter;
ret->commit_filter = ctx.cfg.commit_filter;
ret->source_filter = ctx.cfg.source_filter;
ret->email_filter = ctx.cfg.email_filter;
ret->clone_url = ctx.cfg.clone_url;
And I don't think we want to change all of these to allow the config
file to be out-of-order.
Perhaps we would be better off making the documentation clearer rather
than trying to fix some particular cases?
> Signed-off-by: Jamie Couture
> ---
> ui-tree.c | 11 +--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/ui-tree.c b/ui-tree.c
> index e4c3d22..f52f15c 100644
> --- a/ui-tree.c
> +++ b/ui-tree.c
> @@ -22,6 +22,7 @@ static void print_text_buffer(const char *name, char *buf,
> unsigned long size)
> {
> unsigned long lineno, idx;
> const char *numberfmt = "%1$d\n";
> + struct cgit_filter *source_filter = NULL;
>
> html("\n");
>
> @@ -45,11 +46,17 @@ static void print_text_buffer(const char *name, char
> *buf, unsigned long size)
> }
>
> if (ctx.repo->source_filter) {
> + source_filter = ctx.repo->source_filter;
> + } else if (ctx.cfg.source_filter) {
> + source_filter = ctx.cfg.source_filter;
> + }
> +
> + if (source_filter) {
> char *filter_arg = xstrdup(name);
> html("");
> - cgit_open_filter(ctx.repo->source_filter, filter_arg);
> + cgit_open_filter(source_filter, filter_arg);
> html_raw(buf, size);
> - cgit_close_filter(ctx.repo->source_filter);
> + cgit_close_filter(source_filter);
> free(filter_arg);
> html("\n");
> return;
> --
> 1.9.1.377.g96e67c8
>
> ___
> CGit mailing list
> CGit@lists.zx2c4.com
> http://lists.zx2c4.com/mailman/listinfo/cgit
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH] ui-tree.c: check source filter if set globally
On Wed, Jul 30, 2014 at 04:47:17PM -0400, Jamie Couture wrote: > On Wed, Jul 30, 2014 at 08:23:21PM +0100, John Keeping wrote: > > On Wed, Jul 30, 2014 at 02:53:21PM -0400, Jamie Couture wrote: > > I'm not convinced that this change makes things less confusing, it just > > means that "everything except source-filter must be configured before > > adding repositories", which seems more confusing. > > > > Well, 'everything except source-filter' isn't really the case. It is the case that all repo-specific settings except source-filter would be ignored if they come after the repository is loaded. > To be clear, the behaviour is that we setup the repo as you pointed > out below, but because we didn't process the appropriate filter > lines yet we did not instantiate the stuct. A full pass of the > configuration has not happened. We would have to make two passes to > know about all filter options, and then copy those settings during > scan-path if that is also defined. > > I think better wording would be 'everything should to be declared > before scan-path and scan-tree', which we know well enough, but > users still run into that issue. > > Users should not have to worry about where they define things. Why not? They can define different settings for different repositories by interleaving the settings and repositories. > > The full list of captured settings is: > > > > ret->section = ctx.cfg.section; > > ret->snapshots = ctx.cfg.snapshots; > > ret->enable_commit_graph = ctx.cfg.enable_commit_graph; > > ret->enable_log_filecount = ctx.cfg.enable_log_filecount; > > ret->enable_log_linecount = ctx.cfg.enable_log_linecount; > > ret->enable_remote_branches = ctx.cfg.enable_remote_branches; > > ret->enable_subject_links = ctx.cfg.enable_subject_links; > > ret->max_stats = ctx.cfg.max_stats; > > ret->branch_sort = ctx.cfg.branch_sort; > > ret->commit_sort = ctx.cfg.commit_sort; > > ret->module_link = ctx.cfg.module_link; > > ret->readme = ctx.cfg.readme; > > ret->about_filter = ctx.cfg.about_filter; > > ret->commit_filter = ctx.cfg.commit_filter; > > ret->source_filter = ctx.cfg.source_filter; > > ret->email_filter = ctx.cfg.email_filter; > > ret->clone_url = ctx.cfg.clone_url; > > > > And I don't think we want to change all of these to allow the config > > file to be out-of-order. > > > > Maybe we do if users are having issues? Currently we support something like this: scan-path=/path/to/world-a source-filter=/path/to/source-filter.sh scan-path=/path/to/world-b Where the source filter isn't applied to "world-a". I don't know if anyone makes use of this, but the current scheme does give quite a lot of flexibility. > > Perhaps we would be better off making the documentation clearer rather > > than trying to fix some particular cases? > > > > I'm okay with this change, as long as it avoids problems for the > user. I don't think it introdces any sort of unwanted or unexpected > behaviour. However, it should try to be more complete with the > remaining filter options. > > I'm fine with this being turfed, but we do get a lot of questions > about scan-path problems. I tend to think that shows that the documentation is lacking rather than the implementation, but I dunno. Maybe I just don't want to deal with making sure that all repo variables fall back to the global one if it's set after scan-path or repo.url. ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: Displaying latest commit message instead of description on repository list
On Thu, Jul 31, 2014 at 10:35:12AM -0400, Nik Nyby wrote: > I have cgit set up with my list of a bunch of different projects, and > none of those repos have a very descriptive "description" file. I > thought it would be nice to display each repository's latest commit line > instead of "Unnamed repository; edit this file 'description' to name the > repository." in the Description column of this table. > > Does cgit have a mechanism to easily configure what I'm talking about, > or would this require new functionality? There is no functionality to do this in CGit, but there is nothing to stop you adding a Git post-update hook to write the latest commit into the description file whenever anyone pushes to the repository. ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: support the rel-vcs microformat?
On Fri, Aug 01, 2014 at 04:01:28PM +0800, Paul Wise wrote: > Would it be possible to support the rel-vcs microformat? > > https://joeyh.name/rfc/rel-vcs/ Can you provide some more details about where you expect CGit to apply this? I'm guessing that when on a repo page of any sort the element should be used and the rel="vcs-git" attribute should be added to the clone links at the bottom of the repo summary page. Does that sound right? Have I missed anything? ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: commit-filter not being applied on log page
On Thu, Jul 31, 2014 at 10:27:19AM -0400, Chris Burroughs wrote: > I'm trying to write a commit-filter to hyperlink references to our bug > tracker. It seems to work fine for commit detailed pages, but not at > all for the log view. Since our developers often make brief messages > like "fixes #123' so getting the url in the log view is arguably more > useful than in the detail page. I think this is an issue of the cost of forking a filter process for each line in the log view. Now that we have Lua filters that may not be so much of an issue, but I don't think we can just start using the source filter on the log view due to the impact that will have on people with an "exec" source-filter already configured. Perhaps we need to add a "log-filter" which you could configure to be the same as "source-filter" but which can be left blank for people whose links are normally in the body of the commit message. ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH] filter: add support for owner-filter
On Fri, Aug 01, 2014 at 04:01:53PM -0400, Chris Burroughs wrote:
> revised patch
This type of comment should go below the "---" line below, since it's
not intended to be part of the commit message in the permanent history.
Also "filter" in the subject doesn't really identify a code area. How
about something like this:
repolist: add owner-filter
This allows custom links to be used for repository owners by
configuring a filter to be applied in the "Owner" column in the
repository list.
More below...
> ---
> cgit.c|6 ++
> cgit.h|4 +++-
> cgitrc.5.txt | 18 ++
> filter.c |6 ++
> filters/owner-example.lua | 19 +++
> shared.c |1 +
> ui-repolist.c | 20 +---
> 7 files changed, 66 insertions(+), 8 deletions(-)
> create mode 100644 filters/owner-example.lua
>
> diff --git a/cgit.c b/cgit.c
> index 20f6e27..d29e7f5 100644
> --- a/cgit.c
> +++ b/cgit.c
> @@ -91,6 +91,8 @@ static void repo_config(struct cgit_repo *repo, const char
> *name, const char *va
> repo->source_filter = cgit_new_filter(value, SOURCE);
> else if (!strcmp(name, "email-filter"))
> repo->email_filter = cgit_new_filter(value, EMAIL);
> + else if (!strcmp(name, "owner-filter"))
> + repo->owner_filter = cgit_new_filter(value, OWNER);
> }
> }
>
> @@ -194,6 +196,8 @@ static void config_cb(const char *name, const char *value)
> ctx.cfg.commit_filter = cgit_new_filter(value, COMMIT);
> else if (!strcmp(name, "email-filter"))
> ctx.cfg.email_filter = cgit_new_filter(value, EMAIL);
> + else if (!strcmp(name, "owner-filter"))
> + ctx.cfg.owner_filter = cgit_new_filter(value, OWNER);
> else if (!strcmp(name, "auth-filter"))
> ctx.cfg.auth_filter = cgit_new_filter(value, AUTH);
> else if (!strcmp(name, "embedded"))
> @@ -802,6 +806,8 @@ static void print_repo(FILE *f, struct cgit_repo *repo)
> cgit_fprintf_filter(repo->source_filter, f,
> "repo.source-filter=");
> if (repo->email_filter && repo->email_filter != ctx.cfg.email_filter)
> cgit_fprintf_filter(repo->email_filter, f,
> "repo.email-filter=");
> + if (repo->owner_filter && repo->owner_filter != ctx.cfg.owner_filter)
> + cgit_fprintf_filter(repo->owner_filter, f,
> "repo.owner-filter=");
> if (repo->snapshots != ctx.cfg.snapshots) {
> char *tmp = build_snapshot_setting(repo->snapshots);
> fprintf(f, "repo.snapshots=%s\n", tmp ? tmp : "");
> diff --git a/cgit.h b/cgit.h
> index 0badc64..0078ac1 100644
> --- a/cgit.h
> +++ b/cgit.h
> @@ -53,7 +53,7 @@ typedef void (*filepair_fn)(struct diff_filepair *pair);
> typedef void (*linediff_fn)(char *line, int len);
>
> typedef enum {
> - ABOUT, COMMIT, SOURCE, EMAIL, AUTH
> + ABOUT, COMMIT, SOURCE, EMAIL, AUTH, OWNER
> } filter_type;
>
> struct cgit_filter {
> @@ -100,6 +100,7 @@ struct cgit_repo {
> struct cgit_filter *commit_filter;
> struct cgit_filter *source_filter;
> struct cgit_filter *email_filter;
> + struct cgit_filter *owner_filter;
> struct string_list submodules;
> };
>
> @@ -253,6 +254,7 @@ struct cgit_config {
> struct cgit_filter *commit_filter;
> struct cgit_filter *source_filter;
> struct cgit_filter *email_filter;
> + struct cgit_filter *owner_filter;
> struct cgit_filter *auth_filter;
> };
>
> diff --git a/cgitrc.5.txt b/cgitrc.5.txt
> index b7570db..d774ed3 100644
> --- a/cgitrc.5.txt
> +++ b/cgitrc.5.txt
> @@ -247,6 +247,15 @@ logo-link::
> calculated url of the repository index page will be used. Default
> value: none.
>
> +owner-filter::
> + Specifies a command which will be invoked to format the Owner
> + column of the main page. The command will get the owner on STDIN,
> + and the STDOUT from the command will be included verbatim in the
> + table. This can be used to link to additional context such as an
> + owners home page. When active this filter is used instead of the
> + default owner query url. Default value: none.
> + See also: "FILTER API".
> +
> max-atom-items::
> Specifies the number of items to display in atom feeds view. Default
> value: "10".
> @@ -509,6 +518,10 @@ repo.logo-link::
> calculated url of the repository index page will be used. Default
> value: global logo-link.
>
> +repo.owner-filter::
> + Override the default owner-filter. Default value: none. See also:
> + "enable-filter-overrides". See also: "FILTER API".
> +
> repo.module-link::
> Text which will be used as the formatstring for a hyperlink when a
> submodule is printed in a directory listing. The arguments for
[PATCH 2/3] ui-summary: add "rel='vcs-git'" to clone URL links
This is described in the rel-vcs microformat[1].
[1] https://joeyh.name/rfc/rel-vcs/
Signed-off-by: John Keeping
---
ui-summary.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/ui-summary.c b/ui-summary.c
index 70ea908..46ca713 100644
--- a/ui-summary.c
+++ b/ui-summary.c
@@ -31,9 +31,11 @@ static void print_url(const char *url)
htmlf("Clone\n",
columns);
}
- htmlf("");
+ html("' title='");
+ html_attr(ctx.repo->name);
+ html(" Git repository'>");
html_txt(url);
html("\n");
}
--
2.0.3.890.g700db9e
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH 1/3] Extract clone URL printing to ui-shared.c
This will allow us to reuse the same logic to add clone URL
elements to the header of all repo-specific pages in order to support
the rel-vcs microformat.
Signed-off-by: John Keeping
---
ui-shared.c | 37 +
ui-shared.h | 2 ++
ui-summary.c | 58 --
3 files changed, 51 insertions(+), 46 deletions(-)
diff --git a/ui-shared.c b/ui-shared.c
index 9dde0a3..5bae02d 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -728,6 +728,43 @@ void cgit_print_docend()
html("\n\n");
}
+static void add_clone_urls(void (*fn)(const char *), char *txt, char *suffix)
+{
+ struct strbuf buf = STRBUF_INIT;
+ char *h = txt, *t, c;
+
+ while (h && *h) {
+ while (h && *h == ' ')
+ h++;
+ if (!*h)
+ break;
+ t = h;
+ while (t && *t && *t != ' ')
+ t++;
+ c = *t;
+ *t = 0;
+
+ if (suffix && *suffix) {
+ strbuf_reset(&buf);
+ strbuf_addf(&buf, "%s/%s", h, suffix);
+ h = buf.buf;
+ }
+ fn(h);
+ *t = c;
+ h = t;
+ }
+
+ strbuf_release(&buf);
+}
+
+void cgit_add_clone_urls(void (*fn)(const char *))
+{
+ if (ctx.repo->clone_url)
+ add_clone_urls(fn, expand_macros(ctx.repo->clone_url), NULL);
+ else if (ctx.cfg.clone_prefix)
+ add_clone_urls(fn, ctx.cfg.clone_prefix, ctx.repo->url);
+}
+
static int print_branch_option(const char *refname, const unsigned char *sha1,
int flags, void *cb_data)
{
diff --git a/ui-shared.h b/ui-shared.h
index 3e7a91b..79662f7 100644
--- a/ui-shared.h
+++ b/ui-shared.h
@@ -11,6 +11,8 @@ extern char *cgit_fileurl(const char *reponame, const char
*pagename,
extern char *cgit_pageurl(const char *reponame, const char *pagename,
const char *query);
+extern void cgit_add_clone_urls(void (*fn)(const char *));
+
extern void cgit_index_link(const char *name, const char *title,
const char *class, const char *pattern, const char
*sort, int ofs);
extern void cgit_summary_link(const char *name, const char *title,
diff --git a/ui-summary.c b/ui-summary.c
index 3728c3e..70ea908 100644
--- a/ui-summary.c
+++ b/ui-summary.c
@@ -12,62 +12,30 @@
#include "ui-log.h"
#include "ui-refs.h"
#include "ui-blob.h"
+#include "ui-shared.h"
#include
-static void print_url(char *base, char *suffix)
+static int urls;
+
+static void print_url(const char *url)
{
int columns = 3;
- struct strbuf basebuf = STRBUF_INIT;
if (ctx.repo->enable_log_filecount)
columns++;
if (ctx.repo->enable_log_linecount)
columns++;
- if (!base || !*base)
- return;
- if (suffix && *suffix) {
- strbuf_addf(&basebuf, "%s/%s", base, suffix);
- base = basebuf.buf;
+ if (urls++ == 0) {
+ htmlf(" ",
columns);
+ htmlf("Clone\n",
columns);
}
+
htmlf("");
- html_txt(base);
+ html_txt(url);
html("\n");
- strbuf_release(&basebuf);
-}
-
-static void print_urls(char *txt, char *suffix)
-{
- char *h = txt, *t, c;
- int urls = 0;
- int columns = 3;
-
- if (ctx.repo->enable_log_filecount)
- columns++;
- if (ctx.repo->enable_log_linecount)
- columns++;
-
-
- while (h && *h) {
- while (h && *h == ' ')
- h++;
- if (!*h)
- break;
- t = h;
- while (t && *t && *t != ' ')
- t++;
- c = *t;
- *t = 0;
- if (urls++ == 0) {
- htmlf(" ", columns);
- htmlf("Clone\n", columns);
- }
- print_url(h, suffix);
- *t = c;
- h = t;
- }
}
void cgit_print_summary()
@@ -88,10 +56,8 @@ void cgit_print_summary()
cgit_print_log(ctx.qry.head, 0, ctx.cfg.summary_log, NULL,
NULL, NULL, 0, 0, 0);
}
- if (ctx.repo->clone_url)
- print_urls(expand_macros(ctx.repo->clone_url), NULL);
- else if (ctx.cfg.clone_prefix)
- print_urls(ctx.cfg.clone_prefix, ctx.repo->url);
+ urls = 0;
+ cgit_add_clone_urls(print_url);
html("");
}
--
2.0.3.890.g700db9e
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH 3/3] ui-shared: add rel-vcs microformat links to HTML header
As described at https://joeyh.name/rfc/rel-vcs/.
Signed-off-by: John Keeping
---
ui-shared.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/ui-shared.c b/ui-shared.c
index 5bae02d..9ac65ab 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -661,6 +661,15 @@ void cgit_print_http_headers(void)
exit(0);
}
+static void print_rel_vcs_link(const char *url)
+{
+ html("\n");
+}
+
void cgit_print_docstart(void)
{
if (ctx.cfg.embedded) {
@@ -699,6 +708,8 @@ void cgit_print_docstart(void)
html("' type='application/atom+xml'/>\n");
strbuf_release(&sb);
}
+ if (ctx.repo)
+ cgit_add_clone_urls(print_rel_vcs_link);
if (ctx.cfg.head_include)
html_include(ctx.cfg.head_include);
html("\n");
--
2.0.3.890.g700db9e
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH] git: update to v2.0.4
No CGit changes required. Signed-off-by: John Keeping --- Makefile | 2 +- git | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 93b525a..6a8a125 100644 --- a/Makefile +++ b/Makefile @@ -14,7 +14,7 @@ htmldir = $(docdir) pdfdir = $(docdir) mandir = $(prefix)/share/man SHA1_HEADER = -GIT_VER = 2.0.3 +GIT_VER = 2.0.4 GIT_URL = https://www.kernel.org/pub/software/scm/git/git-$(GIT_VER).tar.gz INSTALL = install COPYTREE = cp -r diff --git a/git b/git index 740c281..32f5660 16 --- a/git +++ b/git @@ -1 +1 @@ -Subproject commit 740c281d21ef5b27f6f1b942a4f2fc20f51e8c7e +Subproject commit 32f56600bb6ac6fc57183e79d2c1515dfa56672f -- 2.0.3.890.g700db9e ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: Compile cGit 2.0.4
On Mon, Aug 11, 2014 at 08:40:53PM +0100, Jorge Bastos wrote: > Trying to compile cgit last git head cgit (2.0.4) to see if one charset > problem is solved, but got this: > > Peixe:/usr/local/src/cgit/cgit# make > SUBDIR git > ./gen-version.sh: line 2: $'\r': command not found > ./gen-version.sh: line 5: $'\r': command not found > ./gen-version.sh: line 21: syntax error: unexpected end of file > make[1]: *** No rule to make target `../VERSION', needed by `../cgit.o'. > Stop. > make: *** [cgit] Error 2 Do you have a weird CGIT_VERSION defined in your environment? The string $'\r' doesn't appear in gen-version.sh so I don't understand where that error can be coming from. Alternatively do you have a weird (non-POSIX) /bin/sh? What operating system are you using? ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: Compile cGit 2.0.4
On Mon, Aug 11, 2014 at 08:56:40PM +0100, Jorge Bastos wrote: > > Do you have a weird CGIT_VERSION defined in your environment? The > > string $'\r' doesn't appear in gen-version.sh so I don't understand > > where that error can be coming from. > > > > Alternatively do you have a weird (non-POSIX) /bin/sh? What operating > > system are you using? > > I've always compiled cgit manually here. > Debian, sh is the normal "shtool" package. "shtool" isn't a shell. /bin/sh is probably either dash or bash. What does "dpkg -S /bin/sh" say? It looks like the problem is actually that the file has DOS line endings. I get the exact same problem you do if I run: unix2dos gen-version.sh Have you got Git's "core.eol" config set to "crlf"? ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH] Handle If-None-Match HTTP header in plain view
On Mon, Aug 11, 2014 at 05:53:23PM -0300, Damián Nohales wrote:
> We are sending Etag to clients but this header is basically unusefulness
> if the server doesn't tell the client if the content has been changed or
> not for a given Path/Etag pair.
>
> Signed-off-by: Damián Nohales
> ---
How does this interact with CGit's cache? What happens if the original
page expires from the cache and then a request comes in with a matching
Etag?
> cgit.c | 1 +
> cgit.h | 1 +
> ui-plain.c | 41 +
> ui-shared.c | 20
> ui-shared.h | 1 +
> 5 files changed, 44 insertions(+), 20 deletions(-)
>
> diff --git a/cgit.c b/cgit.c
> index 8c4517d..7af7712 100644
> --- a/cgit.c
> +++ b/cgit.c
> @@ -385,6 +385,7 @@ static void prepare_context(void)
> ctx.env.server_port = getenv("SERVER_PORT");
> ctx.env.http_cookie = getenv("HTTP_COOKIE");
> ctx.env.http_referer = getenv("HTTP_REFERER");
> + ctx.env.if_none_match = getenv("HTTP_IF_NONE_MATCH");
> ctx.env.content_length = getenv("CONTENT_LENGTH") ?
> strtoul(getenv("CONTENT_LENGTH"), NULL, 10) : 0;
> ctx.env.authenticated = 0;
> ctx.page.mimetype = "text/html";
> diff --git a/cgit.h b/cgit.h
> index 0badc64..eddd4c7 100644
> --- a/cgit.h
> +++ b/cgit.h
> @@ -282,6 +282,7 @@ struct cgit_environment {
> const char *server_port;
> const char *http_cookie;
> const char *http_referer;
> + const char *if_none_match;
> unsigned int content_length;
> int authenticated;
> };
> diff --git a/ui-plain.c b/ui-plain.c
> index 30fff89..a08dc5b 100644
> --- a/ui-plain.c
> +++ b/ui-plain.c
> @@ -103,8 +103,8 @@ static int print_object(const unsigned char *sha1, const
> char *path)
> ctx.page.filename = path;
> ctx.page.size = size;
> ctx.page.etag = sha1_to_hex(sha1);
> - cgit_print_http_headers();
> - html_raw(buf, size);
> + if (!cgit_print_http_headers_matching_etag())
> + html_raw(buf, size);
> /* If we allocated this, then casting away const is safe. */
> if (freemime)
> free((char*) ctx.page.mimetype);
> @@ -128,24 +128,25 @@ static void print_dir(const unsigned char *sha1, const
> char *base,
> fullpath = buildpath(base, baselen, path);
> slash = (fullpath[0] == '/' ? "" : "/");
> ctx.page.etag = sha1_to_hex(sha1);
> - cgit_print_http_headers();
> - htmlf("%s", slash);
> - html_txt(fullpath);
> - htmlf("\n\n%s", slash);
> - html_txt(fullpath);
> - html("\n\n");
> - len = strlen(fullpath);
> - if (len > 1) {
> - fullpath[len - 1] = 0;
> - slash = strrchr(fullpath, '/');
> - if (slash)
> - *(slash + 1) = 0;
> - else
> - fullpath = NULL;
> - html("");
> - cgit_plain_link("../", NULL, NULL, ctx.qry.head, ctx.qry.sha1,
> - fullpath);
> - html("\n");
> + if (!cgit_print_http_headers_matching_etag()) {
> + htmlf("%s", slash);
> + html_txt(fullpath);
> + htmlf("\n\n%s", slash);
> + html_txt(fullpath);
> + html("\n\n");
> + len = strlen(fullpath);
> + if (len > 1) {
> + fullpath[len - 1] = 0;
> + slash = strrchr(fullpath, '/');
> + if (slash)
> + *(slash + 1) = 0;
> + else
> + fullpath = NULL;
> + html("");
> + cgit_plain_link("../", NULL, NULL, ctx.qry.head,
> ctx.qry.sha1,
> + fullpath);
> + html("\n");
> + }
> }
> free(fullpath);
> }
> diff --git a/ui-shared.c b/ui-shared.c
> index 9dde0a3..84c7efd 100644
> --- a/ui-shared.c
> +++ b/ui-shared.c
> @@ -661,6 +661,26 @@ void cgit_print_http_headers(void)
> exit(0);
> }
>
> +int cgit_print_http_headers_matching_etag(void)
> +{
> + int match = 0;
> + char *etag;
> + if (ctx.page.etag && ctx.env.if_none_match) {
> + etag = fmtalloc("\"%s\"", ctx.page.etag);
> + if (!strcmp(etag, ctx.env.if_none_match)) {
> + ctx.page.status = 304;
> + ctx.page.statusmsg = "Not Modified";
> + ctx.page.mimetype = NULL;
> + ctx.page.size = 0;
> + ctx.page.filename = NULL;
> + match = 1;
> + }
> + free(etag);
> + }
> + cgit_print_http_headers();
> + return match;
> +}
> +
> void cgit_print_docstart(void)
> {
> if (ctx.cfg.embedded) {
> diff --git a/ui-shared.h b/ui-shared.h
> index 3e7a91b..e279f42 100644
> --- a/ui-shared.h
> +++ b/ui-shared.h
> @@ -60,6 +60,7 @@ extern void cgit_vprint_error(cons
