Re: [PATCH v6 1/1] ui-shared: allow to split the repository link
On Tue, Dec 06, 2016 at 06:58:04PM +0100, Petr Vorel wrote:
> Teach cgit split the repository link in the top of repository "summary"
> view. This emulates the same behaviour as it's in gitweb.
>
> This behaviour is not implemented for repositories which have
> "repo.name" set different than "repo.url".
>
> Signed-off-by: Petr Vorel
> Cc: "Jason A. Donenfeld"
> Cc: John Keeping
With one minor nit below,
Reviewed-by: John Keeping
> ---
> Explanation im man page might need to be improved.
>
> Changes v5->v6:
> * Remove config variable, as requested by Jason.
>
> Changes v4->v5:
> * Rename config variable to split-summary-repo-link
>
> Changes v3->v4:
> Implement suggestions from John Keeping (thanks for them :-)):
> * use strchr() instead of strtok_r() and variadic arrays,
> * use cgit_repourl() and html_*() instead of cgit_summary_link(),
> * add comment why we compare page.url and page.name.
>
> Changes v2->v3:
> * New config variable "summary-enable-split-repo-link", minor cleanup.
>
> Changes v1->v2:
> * Minor cleanup.
> ---
> cgitrc.5.txt | 4 +++-
> ui-shared.c | 27 ++-
> 2 files changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/cgitrc.5.txt b/cgitrc.5.txt
> index 9fcf445..500b7ee 100644
> --- a/cgitrc.5.txt
> +++ b/cgitrc.5.txt
> @@ -604,7 +604,9 @@ repo.source-filter::
>
> repo.url::
> The relative url used to access the repository. This must be the first
> - setting specified for each repo. Default value: none.
> + setting specified for each repo. Unless repo.url is different from
> + repo.name, cgit splits the repository link in the top of repository
> + "summary" view. Default value: none.
>
>
> REPOSITORY-SPECIFIC CGITRC FILE
> diff --git a/ui-shared.c b/ui-shared.c
> index 2e4fcd9..b4cad55 100644
> --- a/ui-shared.c
> +++ b/ui-shared.c
> @@ -936,7 +936,32 @@ static void print_header(void)
> if (ctx.repo) {
> cgit_index_link("index", NULL, NULL, NULL, NULL, 0, 1);
> html(" : ");
> - cgit_summary_link(ctx.repo->name, ctx.repo->name, NULL, NULL);
> +
> + /*
> + * NOTE: If repo.name and repo.url are different, we don't
> split link as
> + * it wouldn't make sense to split the path.
> + * */
Extra "* " before the closing */ here.
> + if (!strcmp(ctx.repo->name, ctx.repo->url)) {
> + char *name = ctx.repo->name;
> + char *start = name;
> + for (;;) {
> + char *delim = strchr(start, '/');
> + if (delim)
> + *delim = '\0';
> +
> + html_link_open(cgit_repourl(name), NULL, NULL);
> + html_ntxt(strlen(start), start);
> + html_link_close();
> +
> + if (!delim)
> + break;
> + *delim = '/';
> + html("/");
> + start = delim + 1;
> + }
> + } else
> + cgit_summary_link(ctx.repo->name, ctx.repo->name, NULL,
> NULL);
> +
> if (ctx.env.authenticated) {
> html("");
> html("\n");
> --
> 2.11.0
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
[PULL] Fix escaping in atom output
Hi Jason, [Apologies for sending this twice, I forgot to add the list on the first attempt.] The following changes since commit be39d22328f841536b8e44e8aaeed80a74ebb353: ui-patch: fix crash when using path limit (2017-01-23 18:36:04 +0100) are available in the git repository at: git://git.zx2c4.com/cgit jk/for-jason for you to fetch changes up to 8d03b82036d45cd6e378b3696c22d8e356f8bb88: ui-atom: properly escape delimiter in page link (2017-02-18 16:34:52 +) This patch was originally posted at [0] and hasn't had any comment for over a month, but our output is clearly wrong here. [0] https://lists.zx2c4.com/pipermail/cgit/2017-January/003454.html John Keeping (1): ui-atom: properly escape delimiter in page link ui-atom.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH 0/3] Fix segfault found by AFL
I set AFL [0] loose on CGit's URL input yesterday and it managed to find one issue that leads to a segfault via a null dereference. Either of the first or third patches fixes the segfault, but I much prefer the first as a solid fix, the third is a bit too subtle as a way to ensure that the necessary invariant holds. The second patch also fixes the route that AFL found, but it's possible to get the same effect using broken out query parameters like "?p=log&path=foo" but I'm including it because it seems to make sense to use the value of the final "url" parameter we receive fully rather than some combination of that and a previous URL. [0] http://lcamtuf.coredump.cx/afl/ John Keeping (3): ui-shared: don't print path crumbs without a repo parsing: clear query path before starting cgit: don't set vpath unless repo is set cgit.c | 12 ++-- parsing.c | 2 +- ui-shared.c | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) -- 2.12.0.rc2.230.ga28edc07cd ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH 1/3] ui-shared: don't print path crumbs without a repo
cgit_print_path_crumbs() can call repolink() which assumes that ctx.repo
is non-null. Currently we don't have any commands that set want_vpath
without also setting want_repo so it shouldn't be possible to fail this
test, but the check in cgit.c is in the wrong order so it is possible to
specify a query string like "?p=log&path=foo/bar" to end up here without
a valid repository.
This was found by American fuzzy lop [0].
[0] http://lcamtuf.coredump.cx/afl/
Signed-off-by: John Keeping
---
ui-shared.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ui-shared.c b/ui-shared.c
index 2e4fcd9..e5c9a02 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -1039,7 +1039,7 @@ void cgit_print_pageheader(void)
free(currenturl);
}
html("\n");
- if (ctx.env.authenticated && ctx.qry.vpath) {
+ if (ctx.env.authenticated && ctx.repo && ctx.qry.vpath) {
html("");
html("path: ");
cgit_print_path_crumbs(ctx.qry.vpath);
--
2.12.0.rc2.230.ga28edc07cd
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH 2/3] parsing: clear query path before starting
By specifying the "url" query parameter multiple times it is possible to end up with ctx.qry.vpath set while ctx.repo is null, which triggers an invalid code path from cgit_print_pageheader() while printing path crumbs, resulting in a null dereference. The previous patch fixed this segfault, but it makes no sense for us to clear ctx.repo while leaving ctx.qry.path set to the previous value, so let's just clear it here so that the last "url" parameter given takes full effect rather than partially overriding the effect of the previous value. Signed-off-by: John Keeping --- parsing.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/parsing.c b/parsing.c index 9dacb16..b8d7f10 100644 --- a/parsing.c +++ b/parsing.c @@ -21,6 +21,7 @@ void cgit_parse_url(const char *url) struct cgit_repo *repo; ctx.repo = NULL; + ctx.qry.page = NULL; if (!url || url[0] == '\0') return; @@ -53,7 +54,6 @@ void cgit_parse_url(const char *url) } if (cmd[1]) ctx.qry.page = xstrdup(cmd + 1); - return; } } -- 2.12.0.rc2.230.ga28edc07cd ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH 3/3] cgit: don't set vpath unless repo is set
After the previous two patches, this can be classified as a tidy up
rather than a bug fix, but I think it makes sense to group all of the
tests together before setting up the environment for the command to
execute.
Signed-off-by: John Keeping
---
cgit.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/cgit.c b/cgit.c
index 1075753..1dae4b8 100644
--- a/cgit.c
+++ b/cgit.c
@@ -726,18 +726,18 @@ static void process_request(void)
return;
}
- /* If cmd->want_vpath is set, assume ctx.qry.path contains a "virtual"
-* in-project path limit to be made available at ctx.qry.vpath.
-* Otherwise, no path limit is in effect (ctx.qry.vpath = NULL).
-*/
- ctx.qry.vpath = cmd->want_vpath ? ctx.qry.path : NULL;
-
if (cmd->want_repo && !ctx.repo) {
cgit_print_error_page(400, "Bad request",
"No repository selected");
return;
}
+ /* If cmd->want_vpath is set, assume ctx.qry.path contains a "virtual"
+* in-project path limit to be made available at ctx.qry.vpath.
+* Otherwise, no path limit is in effect (ctx.qry.vpath = NULL).
+*/
+ ctx.qry.vpath = cmd->want_vpath ? ctx.qry.path : NULL;
+
if (ctx.repo && prepare_repo_cmd())
return;
--
2.12.0.rc2.230.ga28edc07cd
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: possible bug in config readme
On Sat, Feb 18, 2017 at 11:12:08PM +0100, MonkZ wrote: > As i have my cgit + uwsgi + nginx + gitolite setup in a LXD container, i > can publish it. > Would this be helpful? Probably not necessary - I had another look at your config file and I realised that almost every "readme" line has a trailing space. The CGit configfile parser strips whitespace around the "=" character but does not strip trailing whitespace. Strangely the rest of the lines in the file do not have any trailing spaces and the line "readme=:install" doesn't but all the rest do (try "cat -E cgitrc" to show this). > Am 18.02.2017 um 17:57 schrieb John Keeping: > > On Tue, Jan 24, 2017 at 11:19:09PM +0100, MonkZ wrote: > >> i'm using cgit 1.1 and trying to configure a global list of possible > >> readme files (see attachment), but all i can get to work are > >> "cgit.readme" entries in git-config files. > >> > >> I would expect, that this list would be active until a repo config or > >> git-config comes around to overwrite it. > > > > I can't see anything wrong with your config, and I have just tried > > something similar here and it seems to work. > > > > Having looked at the code, one small subtlety is that if there is a > > repository-specific readme configuration it overrides the default list > > rather than appending to it, but if you don't have any repository > > configuration then the list in cgitrc should be used. ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH 1/3] filter: introduce cgit_open_email_filter() wrapper
We provide email addresses to the email filter surrounded by angle
brackets, but we will soon remove these in our internal representation.
Introduce a wrapper so that we only have to add them in one place.
Signed-off-by: John Keeping
Signed-off-by: Jason A. Smith
---
cgit.h | 2 ++
filter.c| 5 +
ui-commit.c | 4 ++--
ui-log.c| 2 +-
ui-refs.c | 6 +++---
ui-tag.c| 2 +-
6 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/cgit.h b/cgit.h
index fbc6c6a..204e59e 100644
--- a/cgit.h
+++ b/cgit.h
@@ -382,6 +382,8 @@ extern struct cgit_filter *cgit_new_filter(const char *cmd, filter_type filterty
extern void cgit_cleanup_filters(void);
extern void cgit_init_filters(void);
+extern void cgit_open_email_filter(const char *email, const char *origin);
+
extern void cgit_prepare_repo_env(struct cgit_repo * repo);
extern int readfile(const char *path, char **buf, size_t *size);
diff --git a/filter.c b/filter.c
index 949c931..88098ba 100644
--- a/filter.c
+++ b/filter.c
@@ -454,3 +454,8 @@ struct cgit_filter *cgit_new_filter(const char *cmd, filter_type filtertype)
die("Invalid filter type: %.*s", (int) len, cmd);
}
+
+void cgit_open_email_filter(const char *email, const char *origin)
+{
+ cgit_open_filter(ctx.repo->email_filter, email, origin);
+}
diff --git a/ui-commit.c b/ui-commit.c
index db69d54..04f0411 100644
--- a/ui-commit.c
+++ b/ui-commit.c
@@ -47,7 +47,7 @@ void cgit_print_commit(char *hex, const char *prefix)
cgit_print_diff_ctrls();
html("\n");
html("author");
- cgit_open_filter(ctx.repo->email_filter, info->author_email, "commit");
+ cgit_open_email_filter(info->author_email, "commit");
html_txt(info->author);
if (!ctx.cfg.noplainemail) {
html(" ");
@@ -59,7 +59,7 @@ void cgit_print_commit(char *hex, const char *prefix)
cgit_date_mode(DATE_ISO8601)));
html("\n");
html("committer");
- cgit_open_filter(ctx.repo->email_filter, info->committer_email, "commit");
+ cgit_open_email_filter(info->committer_email, "commit");
html_txt(info->committer);
if (!ctx.cfg.noplainemail) {
html(" ");
diff --git a/ui-log.c b/ui-log.c
index 3220fd9..b78f56f 100644
--- a/ui-log.c
+++ b/ui-log.c
@@ -238,7 +238,7 @@ static void print_commit(struct commit *commit, struct rev_info *revs)
oid_to_hex(&commit->object.oid), ctx.qry.vpath);
show_commit_decorations(commit);
html("");
- cgit_open_filter(ctx.repo->email_filter, info->author_email, "log");
+ cgit_open_email_filter(info->author_email, "log");
html_txt(info->author);
cgit_close_filter(ctx.repo->email_filter);
diff --git a/ui-refs.c b/ui-refs.c
index 75f2789..0c4eef9 100644
--- a/ui-refs.c
+++ b/ui-refs.c
@@ -69,7 +69,7 @@ static int print_branch(struct refinfo *ref)
if (ref->object->type == OBJ_COMMIT) {
cgit_commit_link(info->subject, NULL, NULL, name, NULL, NULL);
html("");
- cgit_open_filter(ctx.repo->email_filter, info->author_email, "refs");
+ cgit_open_email_filter(info->author_email, "refs");
html_txt(info->author);
cgit_close_filter(ctx.repo->email_filter);
html("");
@@ -143,12 +143,12 @@ static int print_tag(struct refinfo *ref)
html("");
if (info) {
if (info->tagger) {
- cgit_open_filter(ctx.repo->email_filter, info->tagger_email, "refs");
+ cgit_open_email_filter(info->tagger_email, "refs");
html_txt(info->tagger);
cgit_close_filter(ctx.repo->email_filter);
}
} else if (ref->object->type == OBJ_COMMIT) {
- cgit_open_filter(ctx.repo->email_filter, ref->commit->author_email, "refs");
+ cgit_open_email_filter(ref->commit->author_email, "refs");
html_txt(ref->commit->author);
cgit_close_filter(ctx.repo->email_filter);
}
diff --git a/ui-tag.c b/ui-tag.c
index afd7d61..d1e5db9 100644
--- a/ui-tag.c
+++ b/ui-tag.c
@@ -83,7 +83,7 @@ void cgit_print_tag(char *revname)
}
if (info->tagger) {
html("tagged by");
- cgit_open_filter(ctx.repo->email_filter, info->tagger_email, "tag");
+ cgit_open_email_filter(info->tagger_email, "tag");
html_txt(info->tagger);
if (info->tagger_email && !ctx.cfg.noplainemail) {
html(" ");
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH 2/3] Remove angle brackets from {author,committer}_email
This matches the internal representation in libgit.a, so it will make it
much easier to use Git's mailmap code.
The change in ui-atom.c isn't strictly necessary since the code copes
with email addresses both with and without angle brackets, but it's a
nice simplification since we know that the email address will always be
provided in the correct form.
Signed-off-by: John Keeping
Signed-off-by: Jason A. Smith
---
filter.c| 10 +-
parsing.c | 6 +-
ui-atom.c | 13 +
ui-commit.c | 6 --
ui-tag.c| 3 ++-
5 files changed, 17 insertions(+), 21 deletions(-)
diff --git a/filter.c b/filter.c
index 88098ba..ba9000a 100644
--- a/filter.c
+++ b/filter.c
@@ -457,5 +457,13 @@ struct cgit_filter *cgit_new_filter(const char *cmd, filter_type filtertype)
void cgit_open_email_filter(const char *email, const char *origin)
{
- cgit_open_filter(ctx.repo->email_filter, email, origin);
+ struct strbuf sb = STRBUF_INIT;
+
+ /* Don't bother allocating any memory if we don't have a filter. */
+ if (!ctx.repo->email_filter)
+ return;
+
+ strbuf_addf(&sb, "<%s>", email);
+ cgit_open_filter(ctx.repo->email_filter, sb.buf, origin);
+ strbuf_release(&sb);
}
diff --git a/parsing.c b/parsing.c
index 9dacb16..352338d 100644
--- a/parsing.c
+++ b/parsing.c
@@ -72,14 +72,10 @@ static char *substr(const char *head, const char *tail)
static void parse_user(const char *t, char **name, char **email, unsigned long *date, int *tz)
{
struct ident_split ident;
- unsigned email_len;
if (!split_ident_line(&ident, t, strchrnul(t, '\n') - t)) {
*name = substr(ident.name_begin, ident.name_end);
-
- email_len = ident.mail_end - ident.mail_begin;
- *email = xmalloc(strlen("<") + email_len + strlen(">") + 1);
- sprintf(*email, "<%.*s>", email_len, ident.mail_begin);
+ *email = substr(ident.mail_begin, ident.mail_end);
if (ident.date_begin)
*date = strtoul(ident.date_begin, NULL, 10);
diff --git a/ui-atom.c b/ui-atom.c
index 41838d3..7c17d6a 100644
--- a/ui-atom.c
+++ b/ui-atom.c
@@ -15,7 +15,6 @@ static void add_entry(struct commit *commit, const char *host)
{
char delim = '&';
char *hex;
- char *mail, *t, *t2;
struct commitinfo *info;
info = cgit_parse_commit(commit);
@@ -35,19 +34,9 @@ static void add_entry(struct commit *commit, const char *host)
html("\n");
}
if (info->author_email && !ctx.cfg.noplainemail) {
- mail = xstrdup(info->author_email);
- t = strchr(mail, '<');
- if (t)
- t++;
- else
- t = mail;
- t2 = strchr(t, '>');
- if (t2)
- *t2 = '\0';
html("");
- html_txt(t);
+ html_txt(info->author_email);
html("\n");
- free(mail);
}
html("\n");
html("");
diff --git a/ui-commit.c b/ui-commit.c
index 04f0411..dccb5f3 100644
--- a/ui-commit.c
+++ b/ui-commit.c
@@ -50,8 +50,9 @@ void cgit_print_commit(char *hex, const char *prefix)
cgit_open_email_filter(info->author_email, "commit");
html_txt(info->author);
if (!ctx.cfg.noplainemail) {
- html(" ");
+ html(" <");
html_txt(info->author_email);
+ html(">");
}
cgit_close_filter(ctx.repo->email_filter);
html("");
@@ -62,8 +63,9 @@ void cgit_print_commit(char *hex, const char *prefix)
cgit_open_email_filter(info->committer_email, "commit");
html_txt(info->committer);
if (!ctx.cfg.noplainemail) {
- html(" ");
+ html(" <");
html_txt(info->committer_email);
+ html(">");
}
cgit_close_filter(ctx.repo->email_filter);
html("");
diff --git a/ui-tag.c b/ui-tag.c
index d1e5db9..ad7854e 100644
--- a/ui-tag.c
+++ b/ui-tag.c
@@ -86,8 +86,9 @@ void cgit_print_tag(char *revname)
cgit_open_email_filter(info->tagger_email, "tag");
html_txt(info->tagger);
if (info->tagger_email && !ctx.cfg.noplainemail) {
-html(" ");
+html(" <");
html_txt(info->tagger_email);
+html(">");
}
cgit_close_filter(ctx.repo->email_filter);
html("\n");
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH 1/1] git: update to v2.12.0
On Wed, Mar 01, 2017 at 11:57:55AM +0100, Christian Hesse wrote:
> From: Christian Hesse
>
> Update to git version v2.12.0: With commit 8aee769f (pathspec: copy and free
> owned memory) member 'match' of 'struct pathspec_item' is no longer
> 'const char*' but just 'char*'.
>
> Signed-off-by: Christian Hesse
> ---
> Makefile| 2 +-
> cgit.h | 4 ++--
> git | 2 +-
> shared.c| 4 ++--
> ui-blob.c | 2 +-
> ui-blob.h | 2 +-
> ui-commit.c | 2 +-
> ui-commit.h | 2 +-
> ui-diff.c | 4 ++--
> ui-diff.h | 2 +-
> 10 files changed, 13 insertions(+), 13 deletions(-)
This feels a bit invasive for a change that just affects pathspec_item.
Can't we keep most of CGit's API the same (and keep prefixes "const")
and just adjust at the lowest level?
Maybe something like this instead?
-- >8 --
The definition of struct pathspec_item has changed with the expectation
that pathspecs will be managed dynamically. We work around this a bit
by setting up a static structure, but let's allocate the match string to
avoid needing to cast away const.
Signed-off-by: John Keeping
---
shared.c | 4 +++-
ui-blob.c | 9 ++---
2 files changed, 9 insertions(+), 4 deletions(-)
--- a/shared.c
+++ b/shared.c
@@ -352,7 +352,7 @@ void cgit_diff_tree(const struct object_id *old_oid,
opt.format_callback = cgit_diff_tree_cb;
opt.format_callback_data = fn;
if (prefix) {
- item.match = prefix;
+ item.match = xstrdup(prefix);
item.len = strlen(prefix);
opt.pathspec.nr = 1;
opt.pathspec.items = &item;
@@ -365,6 +365,8 @@ void cgit_diff_tree(const struct object_id *old_oid,
diff_root_tree_sha1(new_oid->hash, "", &opt);
diffcore_std(&opt);
diff_flush(&opt);
+
+ free(item.match);
}
void cgit_diff_commit(struct commit *commit, filepair_fn fn, const char
*prefix)
diff --git a/ui-blob.c b/ui-blob.c
index 5f30de7..793817f 100644
--- a/ui-blob.c
+++ b/ui-blob.c
@@ -38,7 +38,7 @@ int cgit_ref_path_exists(const char *path, const char *ref,
int file_only)
struct object_id oid;
unsigned long size;
struct pathspec_item path_items = {
- .match = path,
+ .match = xstrdup(path),
.len = strlen(path)
};
struct pathspec paths = {
@@ -53,10 +53,13 @@ int cgit_ref_path_exists(const char *path, const char *ref,
int file_only)
};
if (get_oid(ref, &oid))
- return 0;
+ goto done;
if (sha1_object_info(oid.hash, &size) != OBJ_COMMIT)
- return 0;
+ goto done;
read_tree_recursive(lookup_commit_reference(oid.hash)->tree, "", 0, 0,
&paths, walk_tree, &walk_tree_ctx);
+
+done:
+ free(path_items.match);
return walk_tree_ctx.found_path;
}
--
2.12.0.rc2.230.ga28edc07cd
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH] filter: set environment variable PYTHONIOENCODING to utf-8
On Thu, Feb 23, 2017 at 03:48:23PM +, Roy Marples wrote:
> This allows different versions of Python to be used rather than
> forcing version specific encoding in each script.
>
> Signed-off-by: Roy Marples
> ---
> filter.c | 8
> filters/email-gravatar.py | 3 ---
> filters/syntax-highlighting.py | 5 +
> 3 files changed, 9 insertions(+), 7 deletions(-)
Neat! This definitely makes writing it easier to get it right with
Python filters, but having filter_env_set seems unnecessary.
Is there a reason not to either:
1) set PYTHONIOENCODING unconditionally early in startup
or
2) set the environment in the child after forking
?
> diff --git a/filter.c b/filter.c
> index 949c931..3c0f978 100644
> --- a/filter.c
> +++ b/filter.c
> @@ -15,6 +15,8 @@
> #include
> #endif
>
> +static bool filter_env_set;
> +
> static inline void reap_filter(struct cgit_filter *filter)
> {
> if (filter && filter->cleanup)
> @@ -44,6 +46,12 @@ static int open_exec_filter(struct cgit_filter *base,
> va_list ap)
> struct cgit_exec_filter *filter = (struct cgit_exec_filter *)base;
> int i;
>
> + if (!filter_env_set) {
> + /* Always input/output utf-8 for a Python filter. */
> + setenv("PYTHONIOENCODING", "utf-8", 1);
> + filter_env_set = true;
> + }
> +
> for (i = 0; i < filter->base.argument_count; i++)
> filter->argv[i + 1] = va_arg(ap, char *);
>
> diff --git a/filters/email-gravatar.py b/filters/email-gravatar.py
> index d70440e..8b98471 100755
> --- a/filters/email-gravatar.py
> +++ b/filters/email-gravatar.py
> @@ -30,9 +30,6 @@ if email[-1] == '>':
>
> page = sys.argv[2]
>
> -sys.stdin = codecs.getreader("utf-8")(sys.stdin.detach())
> -sys.stdout = codecs.getwriter("utf-8")(sys.stdout.detach())
> -
> md5 = hashlib.md5(email.encode()).hexdigest()
> text = sys.stdin.read().strip()
>
> diff --git a/filters/syntax-highlighting.py b/filters/syntax-highlighting.py
> index 5888b50..936fdb7 100755
> --- a/filters/syntax-highlighting.py
> +++ b/filters/syntax-highlighting.py
> @@ -29,9 +29,6 @@ from pygments.lexers import guess_lexer
> from pygments.lexers import guess_lexer_for_filename
> from pygments.formatters import HtmlFormatter
>
> -
> -sys.stdin = io.TextIOWrapper(sys.stdin.buffer, encoding='utf-8',
> errors='replace')
> -sys.stdout = io.TextIOWrapper(sys.stdout.buffer, encoding='utf-8',
> errors='replace')
> data = sys.stdin.read()
> filename = sys.argv[1]
> formatter = HtmlFormatter(style='pastie')
> @@ -52,4 +49,4 @@ except TypeError:
> sys.stdout.write('')
> sys.stdout.write(formatter.get_style_defs('.highlight'))
> sys.stdout.write('')
> -sys.stdout.write(highlight(data, lexer, formatter, outfile=None))
> +highlight(data, lexer, formatter, outfile=sys.stdout)
> --
> 2.11.1
>
> ___
> CGit mailing list
> CGit@lists.zx2c4.com
> https://lists.zx2c4.com/mailman/listinfo/cgit
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: cgit and symlinks
On Mon, Mar 06, 2017 at 11:42:14AM +0100, Olliver Schinagl wrote:
> Having said that, I was playing around with a simple script and noticed
> wget gave 404's on symlinks.
>
> As an example:
>
> http://git.schinagl.nl/debootstrap.git/plain/scripts/stretch
>
> at the very least I would expect the content of the link as for example
> github shows:
> https://raw.githubusercontent.com/oliv3r/debootstrap/master/scripts/stretch
>
> having said that, even github handles it wrong imo, as I would expect
> the webserver to 'get' a symlink and thus just follow the symlink and
> thus get the content of the file (in raw/plain mode).
>
> I have tried googeling and checked cgitrc for my 0.12 but have not found
> anything yet.
>
> Did I simply miss something configuration wise?
Nope, it looks like we simply ignore symlinks in the plain UI. Below is
a patch to fix this by printing the content in the same we as in the
tree UI.
We can't reliably follow the link because there is no guarantee that the
target lies within the repository and I don't know what we would output
for the case where we can't display the target.
-- >8 --
Subject: [PATCH] ui-plain: print symlink content
We currently ignore symlinks in ui-plain, leading to a 404. In ui-tree
we print the content of the blob (that is, the path to the target of the
link), so it makes sense to do the same here.
Signed-off-by: John Keeping
---
ui-plain.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ui-plain.c b/ui-plain.c
index 8d541e3..01e8b36 100644
--- a/ui-plain.c
+++ b/ui-plain.c
@@ -135,7 +135,7 @@ static int walk_tree(const unsigned char *sha1, struct
strbuf *base,
struct walk_tree_context *walk_tree_ctx = cbdata;
if (base->len == walk_tree_ctx->match_baselen) {
- if (S_ISREG(mode)) {
+ if (S_ISREG(mode) || S_ISLNK(mode)) {
if (print_object(sha1, pathname))
walk_tree_ctx->match = 1;
} else if (S_ISDIR(mode)) {
--
2.12.0.377.gf910686b23.dirty
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: cgit and symlinks
On Wed, Mar 08, 2017 at 12:38:38PM +0100, MonkZ wrote: > Am 07.03.2017 um 00:35 schrieb John Keeping: > > We can't reliably follow the link because there is no guarantee that the > > target lies within the repository and I don't know what we would output > > for the case where we can't display the target. > > INADH (I'm not a dev here) > > I would recommend to continue ignoring it or returning the blob, because > following symlinks (internally) might result - if not done carefully - > in directory traversal security issues. Maybe resolving a symlink to a > HTTP301 could work. > > For the UI there might be a html-link (in a notification box "This is a > symlink that points to ...") to the symlink-destination below or above > the blob, to get a user via click to a file/directory. We're talking about the "plain" UI here (for example [0]), so we don't have anywhere to put additional content and it has to be something basic. I'm not actually too worried about directory traversal if we were to try following links because we're looking things up in a Git tree at a particular commit and not on the filesystem. A bigger concern would be whether the internals of Git do anything bad (like invalid memory access) if we give the tree traversal machinery a path that goes up out of the repository; I doubt it but I have not checked. [0] https://git.zx2c4.com/cgit/plain/README ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH] filter: set environment variable PYTHONIOENCODING to utf-8
On Wed, Mar 08, 2017 at 07:01:59PM +, Roy Marples wrote: > On 06/03/2017 09:14, Roy Marples wrote: > > On 04/03/2017 12:35, John Keeping wrote: > >> On Thu, Feb 23, 2017 at 03:48:23PM +, Roy Marples wrote: > >>> This allows different versions of Python to be used rather than > >>> forcing version specific encoding in each script. > >>> > >>> Signed-off-by: Roy Marples > >>> --- > >>> filter.c | 8 > >>> filters/email-gravatar.py | 3 --- > >>> filters/syntax-highlighting.py | 5 + > >>> 3 files changed, 9 insertions(+), 7 deletions(-) > >> > >> Neat! This definitely makes writing it easier to get it right with > >> Python filters, but having filter_env_set seems unnecessary. > >> > >> Is there a reason not to either: > >> > >> 1) set PYTHONIOENCODING unconditionally early in startup > >> > >> or > >> > >> 2) set the environment in the child after forking > > > > No reason. > > I went this approach so that it's only set once when needed, happy to > > re-base my work with either of the two above options. > > Is there anything more you want me to do here? I was hoping someone else would jump in with an opinion, but this is quite a quiet mailing list so it might take a while. Having thought about it a bit more, my vote is for moving the setenv into the child. It keeps it where it is being used and I expect most requests to hit zero or one exec filter(s) so we won't be calling setenv() multiple times (and if we do, the cost will be dwarfed by the cost of executing a new process anyway). ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: cgit and symlinks
On Wed, Mar 08, 2017 at 02:28:11PM +0100, MonkZ wrote: > > > Am 08.03.2017 um 13:30 schrieb John Keeping: > > On Wed, Mar 08, 2017 at 12:38:38PM +0100, MonkZ wrote: > >> Am 07.03.2017 um 00:35 schrieb John Keeping: > >>> We can't reliably follow the link because there is no guarantee that the > >>> target lies within the repository and I don't know what we would output > >>> for the case where we can't display the target. > >> > >> INADH (I'm not a dev here) > >> > >> I would recommend to continue ignoring it or returning the blob, because > >> following symlinks (internally) might result - if not done carefully - > >> in directory traversal security issues. Maybe resolving a symlink to a > >> HTTP301 could work. > >> > >> For the UI there might be a html-link (in a notification box "This is a > >> symlink that points to ...") to the symlink-destination below or above > >> the blob, to get a user via click to a file/directory. > > > > We're talking about the "plain" UI here (for example [0]), so we don't > > have anywhere to put additional content and it has to be something > > basic. > Of course. It would be handled like a content-rewrite to return a http301. > > Pseudocode: > handle_symlinks = True # new config item > if this_file_is_a_symlink and symlink_is_relative and handle_symlinks: > if plain_ui: > # rewrite blob to http301 > # by attaching the path to the end of current basedir > # cgit is already able to handle ../ in a path > if !plain_ui: > # show blob > # show notification that this is a symlink > # show a link to a url > # like the one that would be used in plain_ui > > > > > I'm not actually too worried about directory traversal if we were to try > > following links because we're looking things up in a Git tree at a > > particular commit and not on the filesystem. A bigger concern would be > > whether the internals of Git do anything bad (like invalid memory > > access) if we give the tree traversal machinery a path that goes up out > > of the repository; I doubt it but I have not checked. > If we use url-rewrites (and let the http-client care about getting the > correct file or directory), this would be a non-issue. It could also mean that cross-repository symlinks work if the server layout matches that that is expected for checkouts of the repositories. But it's not exactly helpful if a repository contains an absolute symlink and I don't think we want to start figuring out whether a redirect makes sense - what do we do if we decide it doesn't? ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH] filter: set environment variable PYTHONIOENCODING to utf-8
On Thu, Mar 09, 2017 at 12:18:10AM +, Roy Marples wrote:
> This allows different versions of Python to be used rather than
> forcing version specific encoding in each script.
>
> Signed-off-by: Roy Marples
Reviewed-by: John Keeping
> ---
> filter.c | 2 ++
> filters/email-gravatar.py | 3 ---
> filters/syntax-highlighting.py | 5 +
> 3 files changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/filter.c b/filter.c
> index 949c931..84dd0ed 100644
> --- a/filter.c
> +++ b/filter.c
> @@ -55,6 +55,8 @@ static int open_exec_filter(struct cgit_filter *base,
> va_list ap)
> close(filter->pipe_fh[1]);
> chk_non_negative(dup2(filter->pipe_fh[0], STDIN_FILENO),
> "Unable to use pipe as STDIN");
> + /* Always input/output utf-8 for a Python filter. */
> + setenv("PYTHONIOENCODING", "utf-8", 1);
> execvp(filter->cmd, filter->argv);
> die_errno("Unable to exec subprocess %s", filter->cmd);
> }
> diff --git a/filters/email-gravatar.py b/filters/email-gravatar.py
> index d70440e..8b98471 100755
> --- a/filters/email-gravatar.py
> +++ b/filters/email-gravatar.py
> @@ -30,9 +30,6 @@ if email[-1] == '>':
>
> page = sys.argv[2]
>
> -sys.stdin = codecs.getreader("utf-8")(sys.stdin.detach())
> -sys.stdout = codecs.getwriter("utf-8")(sys.stdout.detach())
> -
> md5 = hashlib.md5(email.encode()).hexdigest()
> text = sys.stdin.read().strip()
>
> diff --git a/filters/syntax-highlighting.py b/filters/syntax-highlighting.py
> index 5888b50..936fdb7 100755
> --- a/filters/syntax-highlighting.py
> +++ b/filters/syntax-highlighting.py
> @@ -29,9 +29,6 @@ from pygments.lexers import guess_lexer
> from pygments.lexers import guess_lexer_for_filename
> from pygments.formatters import HtmlFormatter
>
> -
> -sys.stdin = io.TextIOWrapper(sys.stdin.buffer, encoding='utf-8',
> errors='replace')
> -sys.stdout = io.TextIOWrapper(sys.stdout.buffer, encoding='utf-8',
> errors='replace')
> data = sys.stdin.read()
> filename = sys.argv[1]
> formatter = HtmlFormatter(style='pastie')
> @@ -52,4 +49,4 @@ except TypeError:
> sys.stdout.write('')
> sys.stdout.write(formatter.get_style_defs('.highlight'))
> sys.stdout.write('')
> -sys.stdout.write(highlight(data, lexer, formatter, outfile=None))
> +highlight(data, lexer, formatter, outfile=sys.stdout)
> --
> 2.11.1
>
> ___
> CGit mailing list
> CGit@lists.zx2c4.com
> https://lists.zx2c4.com/mailman/listinfo/cgit
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: cgit and symlinks
On Thu, Mar 09, 2017 at 08:58:43AM +0100, MonkZ wrote: > Am 09.03.2017 um 01:15 schrieb John Keeping: > > On Wed, Mar 08, 2017 at 02:28:11PM +0100, MonkZ wrote: > >> > >> > >> Am 08.03.2017 um 13:30 schrieb John Keeping: > >>> On Wed, Mar 08, 2017 at 12:38:38PM +0100, MonkZ wrote: > >>>> Am 07.03.2017 um 00:35 schrieb John Keeping: > >>>>> We can't reliably follow the link because there is no guarantee that the > >>>>> target lies within the repository and I don't know what we would output > >>>>> for the case where we can't display the target. > >>>> > >>>> INADH (I'm not a dev here) > >>>> > >>>> I would recommend to continue ignoring it or returning the blob, because > >>>> following symlinks (internally) might result - if not done carefully - > >>>> in directory traversal security issues. Maybe resolving a symlink to a > >>>> HTTP301 could work. > >>>> > >>>> For the UI there might be a html-link (in a notification box "This is a > >>>> symlink that points to ...") to the symlink-destination below or above > >>>> the blob, to get a user via click to a file/directory. > >>> > >>> We're talking about the "plain" UI here (for example [0]), so we don't > >>> have anywhere to put additional content and it has to be something > >>> basic. > >> Of course. It would be handled like a content-rewrite to return a http301. > >> > >> Pseudocode: > >> handle_symlinks = True # new config item > >> if this_file_is_a_symlink and symlink_is_relative and handle_symlinks: > >>if plain_ui: > >># rewrite blob to http301 > >># by attaching the path to the end of current basedir > >># cgit is already able to handle ../ in a path > >>if !plain_ui: > >># show blob > >># show notification that this is a symlink > >># show a link to a url > >># like the one that would be used in plain_ui > >> > >>> > >>> I'm not actually too worried about directory traversal if we were to try > >>> following links because we're looking things up in a Git tree at a > >>> particular commit and not on the filesystem. A bigger concern would be > >>> whether the internals of Git do anything bad (like invalid memory > >>> access) if we give the tree traversal machinery a path that goes up out > >>> of the repository; I doubt it but I have not checked. > >> If we use url-rewrites (and let the http-client care about getting the > >> correct file or directory), this would be a non-issue. > > > > It could also mean that cross-repository symlinks work if the server > > layout matches that that is expected for checkouts of the repositories. > > > > But it's not exactly helpful if a repository contains an absolute > > symlink and I don't think we want to start figuring out whether a > > redirect makes sense - what do we do if we decide it doesn't? > > > > Absolute symlinks must be ignored. There is no deterministic way to > resolve them - every clone can be at a different location, and there > isn't really a deterministic mapping from url to filesystem. Absolute > symlinks would only work if resolved internally - with additional > security risks. > > Relative inter-repository links may allowed/handled/redirected if > explicitly configured, otherwise it might be confusing if the server > layout doesn't match. On the other hand a notification "This is a > symlink outside this repository" might suffice (but i don't have a plan > for plain-ui). We can consider improvements to the tree UI separately, but I really don't think we should be getting into anything clever with symlinks in the plain UI because it ends up with complicated rules like the above. It's difficult to explain and will end up surprising users, so my preference is for my original patch that just displays the content of the blob when a symlink is found. This is consistent with both "git show" and "git cat-file". ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH v7 1/1] ui-shared: allow to split the repository link
On Tue, Feb 21, 2017 at 12:36:04AM +0100, Petr Vorel wrote:
> Teach cgit split the repository link in the top of repository "summary"
> view. This emulates the same behaviour as it's in gitweb.
>
> This behaviour is not implemented for repositories which have
> "repo.name" set different than "repo.url".
>
> Signed-off-by: Petr Vorel
> Cc: "Jason A. Donenfeld"
> Cc: John Keeping
> Reviewed-by: John Keeping
> ---
> Thanks for your review John. Are we there yet? :-)
I thought we were, but... I just applied this and it breaks t0103-log.sh
'no links with space in path'.
Having looked at it, our general printing of repo URLs falls foul of
that test - if you copy+paste it into t0101-index.sh then that fails as
well, and the tests are right to fail here.
Given that, I don't see the need to block this patch for introducing a
failure here (exactly the same failure will already exist on other
pages), but we should do something about the failing test case.
I propose squashing this in:
-- >8 --
diff --git a/tests/t0103-log.sh b/tests/t0103-log.sh
index bdf1435..2818016 100755
--- a/tests/t0103-log.sh
+++ b/tests/t0103-log.sh
@@ -17,7 +17,7 @@ test_expect_success 'generate
"with%20space/log?qt=grep&q=commit+1"' '
test_expect_success 'find commit 1' 'grep "commit 1" tmp'
test_expect_success 'find link with %20 in path' 'grep
"/with%20space/log/?qt=grep" tmp'
test_expect_success 'find link with + in arg' 'grep
"/log/?qt=grep&q=commit+1" tmp'
-test_expect_success 'no links with space in path' '! grep "href=./with space/"
tmp'
+test_expect_failure 'no links with space in path' '! grep "href=./with space/"
tmp'
test_expect_success 'no links with space in arg' '! grep "q=commit 1" tmp'
test_expect_success 'commit 2 is not visible' '! grep "commit 2" tmp'
-- 8< --
> Changes v6->v7:
> * Remove redundant '*'
> * Add Reviewed-by
> ---
> cgitrc.5.txt | 4 +++-
> ui-shared.c | 27 ++-
> 2 files changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/cgitrc.5.txt b/cgitrc.5.txt
> index 9fcf445..500b7ee 100644
> --- a/cgitrc.5.txt
> +++ b/cgitrc.5.txt
> @@ -604,7 +604,9 @@ repo.source-filter::
>
> repo.url::
> The relative url used to access the repository. This must be the first
> - setting specified for each repo. Default value: none.
> + setting specified for each repo. Unless repo.url is different from
> + repo.name, cgit splits the repository link in the top of repository
> + "summary" view. Default value: none.
>
>
> REPOSITORY-SPECIFIC CGITRC FILE
> diff --git a/ui-shared.c b/ui-shared.c
> index 2e4fcd9..e4f956d 100644
> --- a/ui-shared.c
> +++ b/ui-shared.c
> @@ -936,7 +936,32 @@ static void print_header(void)
> if (ctx.repo) {
> cgit_index_link("index", NULL, NULL, NULL, NULL, 0, 1);
> html(" : ");
> - cgit_summary_link(ctx.repo->name, ctx.repo->name, NULL, NULL);
> +
> + /*
> + * NOTE: If repo.name and repo.url are different, we don't
> split link as
> + * it wouldn't make sense to split the path.
> + */
> + if (!strcmp(ctx.repo->name, ctx.repo->url)) {
> + char *name = ctx.repo->name;
> + char *start = name;
> + for (;;) {
> + char *delim = strchr(start, '/');
> + if (delim)
> + *delim = '\0';
> +
> + html_link_open(cgit_repourl(name), NULL, NULL);
> + html_ntxt(strlen(start), start);
> + html_link_close();
> +
> + if (!delim)
> + break;
> + *delim = '/';
> + html("/");
> + start = delim + 1;
> + }
> + } else
> + cgit_summary_link(ctx.repo->name, ctx.repo->name, NULL,
> NULL);
> +
> if (ctx.env.authenticated) {
> html("");
> html("\n");
> --
> 2.11.0
>
> ___
> CGit mailing list
> CGit@lists.zx2c4.com
> https://lists.zx2c4.com/mailman/listinfo/cgit
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
[PULL] Collected patches
Hi Jason, My changes are a collection of bugfixes, including a series fixing an URL-parsing issue I found by running AFL. I've picked up Roy's patch as well because I think it's in decent shape and it makes it easier to write Python filters that do the right thing with non-ASCII content. -- >8 -- The following changes since commit be39d22328f841536b8e44e8aaeed80a74ebb353: ui-patch: fix crash when using path limit (2017-01-23 18:36:04 +0100) are available in the git repository at: git://git.zx2c4.com/cgit jk/for-jason for you to fetch changes up to da76e84d09c6d153a072fc73aed8347af59d32db: ui-plain: print symlink content (2017-03-12 14:14:57 +) -------- John Keeping (5): ui-atom: properly escape delimiter in page link ui-shared: don't print path crumbs without a repo parsing: clear query path before starting cgit: don't set vpath unless repo is set ui-plain: print symlink content Roy Marples (1): filter: set environment variable PYTHONIOENCODING to utf-8 cgit.c | 12 ++-- filter.c | 2 ++ filters/email-gravatar.py | 3 --- filters/syntax-highlighting.py | 5 + parsing.c | 2 +- ui-atom.c | 2 +- ui-plain.c | 2 +- ui-shared.c| 2 +- 8 files changed, 13 insertions(+), 17 deletions(-) ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH v7 1/1] ui-shared: allow to split the repository link
On Tue, Feb 21, 2017 at 12:36:04AM +0100, Petr Vorel wrote:
> Teach cgit split the repository link in the top of repository "summary"
> view. This emulates the same behaviour as it's in gitweb.
>
> This behaviour is not implemented for repositories which have
> "repo.name" set different than "repo.url".
>
> Signed-off-by: Petr Vorel
> Cc: "Jason A. Donenfeld"
> Cc: John Keeping
> Reviewed-by: John Keeping
> ---
> Thanks for your review John. Are we there yet? :-)
I just noticed one more thing...
> Changes v6->v7:
> * Remove redundant '*'
> * Add Reviewed-by
> ---
> cgitrc.5.txt | 4 +++-
> ui-shared.c | 27 ++-
> 2 files changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/cgitrc.5.txt b/cgitrc.5.txt
> index 9fcf445..500b7ee 100644
> --- a/cgitrc.5.txt
> +++ b/cgitrc.5.txt
> @@ -604,7 +604,9 @@ repo.source-filter::
>
> repo.url::
> The relative url used to access the repository. This must be the first
> - setting specified for each repo. Default value: none.
> + setting specified for each repo. Unless repo.url is different from
> + repo.name, cgit splits the repository link in the top of repository
> + "summary" view. Default value: none.
>
>
> REPOSITORY-SPECIFIC CGITRC FILE
> diff --git a/ui-shared.c b/ui-shared.c
> index 2e4fcd9..e4f956d 100644
> --- a/ui-shared.c
> +++ b/ui-shared.c
> @@ -936,7 +936,32 @@ static void print_header(void)
> if (ctx.repo) {
> cgit_index_link("index", NULL, NULL, NULL, NULL, 0, 1);
> html(" : ");
> - cgit_summary_link(ctx.repo->name, ctx.repo->name, NULL, NULL);
> +
> + /*
> + * NOTE: If repo.name and repo.url are different, we don't
> split link as
> + * it wouldn't make sense to split the path.
> + */
> + if (!strcmp(ctx.repo->name, ctx.repo->url)) {
> + char *name = ctx.repo->name;
> + char *start = name;
> + for (;;) {
> + char *delim = strchr(start, '/');
> + if (delim)
> + *delim = '\0';
> +
> + html_link_open(cgit_repourl(name), NULL, NULL);
The value returned by cgit_repourl() should be freed.
> + html_ntxt(strlen(start), start);
> + html_link_close();
> +
> + if (!delim)
> + break;
> + *delim = '/';
> + html("/");
> + start = delim + 1;
> + }
> + } else
> + cgit_summary_link(ctx.repo->name, ctx.repo->name, NULL,
> NULL);
> +
> if (ctx.env.authenticated) {
> html("");
> html("\n");
> --
> 2.11.0
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH 2/3] ui-{repolist,atom}: fix repository URL escaping
Remove cgit_repourl() and replace it with a new function that prints a
suitable escaped version of the repository URL designed for use in an
attribute value. There are only two places that use this at the moment
and both want the value to be used in attribute context.
We could introduce new escaping functions to let us keep cgit_repourl()
and return a suitable escaped string, but there's no need for us to be
doing memory management here so this ends up simplifying the call sites.
Signed-off-by: John Keeping
---
tests/t0101-index.sh | 2 +-
ui-atom.c| 4 +---
ui-repolist.c| 11 ---
ui-shared.c | 13 -
ui-shared.h | 3 ++-
5 files changed, 16 insertions(+), 17 deletions(-)
diff --git a/tests/t0101-index.sh b/tests/t0101-index.sh
index 6450b9b..91c1355 100755
--- a/tests/t0101-index.sh
+++ b/tests/t0101-index.sh
@@ -11,7 +11,7 @@ test_expect_success 'find bar description' 'grep "the bar
repo" tmp'
test_expect_success 'find foo+bar repo' 'grep ">foo+bar<" tmp'
test_expect_success 'verify foo+bar link' 'grep "/foo+bar/" tmp'
test_expect_success 'verify "with%20space" link' 'grep "/with%20space/" tmp'
-test_expect_failure 'no links with space in path' '! grep "href=./with space/"
tmp'
+test_expect_success 'no links with space in path' '! grep "href=./with space/"
tmp'
test_expect_success 'no tree-link' '! grep "foo/tree" tmp'
test_expect_success 'no log-link' '! grep "foo/log" tmp'
diff --git a/ui-atom.c b/ui-atom.c
index 41838d3..671fc15 100644
--- a/ui-atom.c
+++ b/ui-atom.c
@@ -130,13 +130,11 @@ void cgit_print_atom(char *tip, char *path, int max_count)
html_txt(ctx.repo->desc);
html("\n");
if (host) {
- char *repourl = cgit_repourl(ctx.repo->url);
html("\n");
- free(repourl);
}
while ((commit = get_revision(&rev)) != NULL) {
add_entry(commit, host);
diff --git a/ui-repolist.c b/ui-repolist.c
index b57ea60..e1302b4 100644
--- a/ui-repolist.c
+++ b/ui-repolist.c
@@ -275,7 +275,6 @@ void cgit_print_repolist(void)
int i, columns = 3, hits = 0, header = 0;
char *last_section = NULL;
char *section;
- char *repourl;
int sorted = 0;
if (!any_repos_visible()) {
@@ -330,13 +329,11 @@ void cgit_print_repolist(void)
htmlf("",
!sorted && section ? "sublevel-repo" : "toplevel-repo");
cgit_summary_link(ctx.repo->name, ctx.repo->name, NULL, NULL);
- html("");
- repourl = cgit_repourl(ctx.repo->url);
- html_link_open(repourl, NULL, NULL);
- free(repourl);
+ html("");
html_ntxt(ctx.cfg.max_repodesc_len, ctx.repo->desc);
- html_link_close();
- html("");
+ html("");
if (ctx.cfg.enable_index_owner) {
if (ctx.repo->owner_filter) {
cgit_open_filter(ctx.repo->owner_filter);
diff --git a/ui-shared.c b/ui-shared.c
index 2e4fcd9..4aca983 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -92,12 +92,15 @@ const char *cgit_loginurl(void)
return login_url;
}
-char *cgit_repourl(const char *reponame)
+void cgit_repo_url_path(const char *reponame)
{
- if (ctx.cfg.virtual_root)
- return fmtalloc("%s%s/", ctx.cfg.virtual_root, reponame);
- else
- return fmtalloc("?r=%s", reponame);
+ if (ctx.cfg.virtual_root) {
+ html_url_path(ctx.cfg.virtual_root);
+ html_url_path(reponame);
+ } else {
+ html("?r=");
+ html_url_arg(reponame);
+ }
}
char *cgit_fileurl(const char *reponame, const char *pagename,
diff --git a/ui-shared.h b/ui-shared.h
index 87799f1..90dfcbc 100644
--- a/ui-shared.h
+++ b/ui-shared.h
@@ -6,7 +6,6 @@ extern char *cgit_hosturl(void);
extern const char *cgit_rooturl(void);
extern char *cgit_currenturl(void);
extern const char *cgit_loginurl(void);
-extern char *cgit_repourl(const char *reponame);
extern char *cgit_fileurl(const char *reponame, const char *pagename,
const char *filename, const char *query);
extern char *cgit_pageurl(const char *reponame, const char *pagename,
@@ -14,6 +13,8 @@ extern char *cgit_pageurl(const char *reponame, const char
*pagename,
extern void cgit_add_clone_urls(void (*fn)(const char *));
+extern void cgit_repo_url_path(const char *reponame);
+
extern void cgit_index_link(const char *name, const char *title,
const char *class, const char *pattern, const char
*sort, int ofs, int always_root);
extern void cgit_summary_link(const char *name, const char *title,
--
2.12.0.415.g6c14041932.dirty
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH 1/3] tests: document handling of repository names with spaces
t0103-log.sh has a test to check that we don't properly escape spaces in repository URLs, but no other tests do. It turns out that the handling of this case in the index page is broken, so mark this as test_expect_failure while adding successful tests for all other pages. Signed-off-by: John Keeping --- tests/t0101-index.sh | 1 + tests/t0102-summary.sh | 7 +++ tests/t0104-tree.sh| 7 +++ tests/t0105-commit.sh | 7 +++ tests/t0106-diff.sh| 7 +++ 5 files changed, 29 insertions(+) diff --git a/tests/t0101-index.sh b/tests/t0101-index.sh index 82ef9b0..6450b9b 100755 --- a/tests/t0101-index.sh +++ b/tests/t0101-index.sh @@ -11,6 +11,7 @@ test_expect_success 'find bar description' 'grep "the bar repo" tmp' test_expect_success 'find foo+bar repo' 'grep ">foo+bar<" tmp' test_expect_success 'verify foo+bar link' 'grep "/foo+bar/" tmp' test_expect_success 'verify "with%20space" link' 'grep "/with%20space/" tmp' +test_expect_failure 'no links with space in path' '! grep "href=./with space/" tmp' test_expect_success 'no tree-link' '! grep "foo/tree" tmp' test_expect_success 'no log-link' '! grep "foo/log" tmp' diff --git a/tests/t0102-summary.sh b/tests/t0102-summary.sh index b8864cb..1d91dca 100755 --- a/tests/t0102-summary.sh +++ b/tests/t0102-summary.sh @@ -22,4 +22,11 @@ test_expect_success 'clone-url expanded correctly' ' grep "git://example.org/bar.git" tmp ' +test_expect_success 'generate with-space summary' ' + cgit_url "with20space" >tmp +' +test_expect_success 'no links with space in path' ' + ! grep "href=./with space/" tmp +' + test_done diff --git a/tests/t0104-tree.sh b/tests/t0104-tree.sh index 2e140f5..de8c4f8 100755 --- a/tests/t0104-tree.sh +++ b/tests/t0104-tree.sh @@ -29,4 +29,11 @@ test_expect_success 'verify a+b?h=1+2 link' ' grep "/foo+bar/tree/a+b?h=1%2b2" tmp ' +test_expect_success 'generate with%20space/tree' ' + cgit_url "with%20space/tree" >tmp +' +test_expect_success 'no links with space in path' ' + ! grep "href=./with space/" tmp +' + test_done diff --git a/tests/t0105-commit.sh b/tests/t0105-commit.sh index 9cdf55c..1932d5f 100755 --- a/tests/t0105-commit.sh +++ b/tests/t0105-commit.sh @@ -33,4 +33,11 @@ test_expect_success 'root commit contains diff' ' grep "+1" tmp ' +test_expect_success 'generate with%20space/commit' ' + cgit_url "with%20space/commit" >tmp +' +test_expect_success 'no links with space in path' ' + ! grep "href=./with space/" tmp +' + test_done diff --git a/tests/t0106-diff.sh b/tests/t0106-diff.sh index 82b645e..89532f5 100755 --- a/tests/t0106-diff.sh +++ b/tests/t0106-diff.sh @@ -16,4 +16,11 @@ test_expect_success 'find added line' ' grep "+5" tmp ' +test_expect_success 'generate with%20space/diff' ' + cgit_url "with%20space/diff" >tmp +' +test_expect_success 'no links with space in path' ' + ! grep "href=./with space/" tmp +' + test_done -- 2.12.0.415.g6c14041932.dirty ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH 3/3] html: remove unused functions
The previous commit removed the only use of these functions, so let's
remove them.
Signed-off-by: John Keeping
---
html.c | 20
html.h | 2 --
2 files changed, 22 deletions(-)
diff --git a/html.c b/html.c
index e7e6e07..e4dbb05 100644
--- a/html.c
+++ b/html.c
@@ -295,26 +295,6 @@ void html_intoption(int value, const char *text, int
selected_value)
html("");
}
-void html_link_open(const char *url, const char *title, const char *class)
-{
- html("");
-}
-
-void html_link_close(void)
-{
- html("");
-}
-
void html_fileperm(unsigned short mode)
{
htmlf("%c%c%c", (mode & 4 ? 'r' : '-'),
diff --git a/html.h b/html.h
index 1b24e55..e0f5195 100644
--- a/html.h
+++ b/html.h
@@ -27,8 +27,6 @@ extern void html_header_arg_in_quotes(const char *txt);
extern void html_hidden(const char *name, const char *value);
extern void html_option(const char *value, const char *text, const char
*selected_value);
extern void html_intoption(int value, const char *text, int selected_value);
-extern void html_link_open(const char *url, const char *title, const char
*class);
-extern void html_link_close(void);
extern void html_fileperm(unsigned short mode);
extern int html_include(const char *filename);
--
2.12.0.415.g6c14041932.dirty
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH] filter: set environment variable PYTHONIOENCODING to utf-8
On Sun, Mar 12, 2017 at 10:01:10AM -0700, Jason A. Donenfeld wrote:
> Sorry for the delay. I'm currently on the road traveling and won't be
> properly back at my desk until the end of next week.
>
> However, my initial reaction is that hard coding various
> interpreter-specific environment variables in cgit itself is not
> correct, and that this is something better left to the CGI environment
> as it sees fit. However, we may benefit from explicit script level
> configuration of unicode stuff.
While I'm inclined to agree with this, in this particular case we
explicitly encode pages as UTF-8 so there is an argument that we should
be telling child processes that UTF-8 is the correct encoding.
Maybe we should be looking to change LANG instead, but I'm not sure how
reliably we can do that. Is it safe to do something like:
const char *lang = getenv("LANG");
struct strbuf sb = STRBUF_INIT;
if (!lang)
lang = "C";
strbuf_addf(&sb, "%.*s.UTF-8",
(int) (strchrnul(lang, '.') - lang), lang);
setenv("LANG", sb.buf);
?
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH v8 1/1] ui-shared: allow to split the repository link
On Sun, Mar 12, 2017 at 06:56:03PM +0100, Petr Vorel wrote: > > Teach cgit split the repository link in the top of repository "summary" > > view. This emulates the same behaviour as it's in gitweb. > > > This behaviour is not implemented for repositories which have > > "repo.name" set different than "repo.url". > > > Signed-off-by: Petr Vorel > > Cc: "Jason A. Donenfeld" > > Cc: John Keeping > > Reviewed-by: John Keeping > > --- > > cgitrc.5.txt | 4 +++- > > ui-shared.c | 28 +++- > > 2 files changed, 30 insertions(+), 2 deletions(-) > > > --- > > v7->v8: add missing free(). > actually, this breaks even more tests, please use v7. > Patch v7 breaks t0103-log.sh, but v8 breaks even some tests before: > t0104-tree.sh. > Failure is in "summary, refs, log, tree, commit, diff" links > > John, I suppose free() shouldn't be used. Or am I missing something and I > should free > something else? It's the return value of cgit_repourl() that needs to be freed, not the name. You'll need a new local variable for the result of calling cgit_repourl() which then needs to be freed in the loop. ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH] filter: set environment variable PYTHONIOENCODING to utf-8
On Fri, Mar 17, 2017 at 07:07:02PM +0100, Jason A. Donenfeld wrote:
> On Sun, Mar 12, 2017 at 6:51 PM, John Keeping wrote:
> > While I'm inclined to agree with this, in this particular case we
> > explicitly encode pages as UTF-8 so there is an argument that we should
> > be telling child processes that UTF-8 is the correct encoding.
>
> That's a compelling argument, actually.
>
> >
> > Maybe we should be looking to change LANG instead, but I'm not sure how
> > reliably we can do that.
>
> I'm more onboard with that. Does changing LANG influence the PYTHON
> variable implicitly?
Yes, if there is no explicit encoding requested then Python derives it
from the locale.
However, it only works if the locale actually exists on the system; for
example on my system I get:
$ LANG=en_GB.UTF-8 python2 -c 'import sys; print(sys.stdin.encoding)'
UTF-8
$ LANG=en_GB.ISO-8859-1 python2 -c 'import sys; print(sys.stdin.encoding)'
ISO-8859-1
but I don't have C.UTF-8, so:
$ LC_ALL=C.UTF-8 python2 -c 'import sys; print(sys.stdin.encoding)'
ANSI_X3.4-1968
There's an open glibc bug [1] to support C.UTF-8 but for now it looks
like it's only available on Debian and derivatives.
> > Is it safe to do something like:
> >
> > const char *lang = getenv("LANG");
> > struct strbuf sb = STRBUF_INIT;
> >
> > if (!lang)
> > lang = "C";
> > strbuf_addf(&sb, "%.*s.UTF-8",
> > (int) (strchrnul(lang, '.') - lang), lang);
> > setenv("LANG", sb.buf);
>
> That's probably not too bad, though I wonder if we could get away with
> just explicitly setting a more generic UTF-8 instead of trying to read
> the user's language preferences.
Other people have already found that it's not quite that simple [2] if
we want it to work on all systems.
[1] https://sourceware.org/bugzilla/show_bug.cgi?id=17318
[2] https://github.com/commercialhaskell/stack/issues/856
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH] cache: flush stdio before restoring FDs
As described in commit 2efb59e (ui-patch: Flush stdout after outputting
data, 2014-06-11), we need to ensure that stdout is flushed before
restoring the file descriptor when writing to the cache. It turns out
that it's not just ui-patch that is affected by this but also raw diff
which writes to stdout internally.
Let's avoid risking more places doing this by ensuring that stdout is
flushed after writing in fill_slot().
Signed-off-by: John Keeping
---
On Mon, Apr 24, 2017 at 11:54:40AM -0400, Konstantin Ryabitsev wrote:
> Very reminiscent of a fix for ui-patch.c that fixed patch truncation [1]
> there's a similar problem with cached rawdiff output. E.g.:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/rawdiff/?id=v4.11-rc7&id2=v4.11-rc8
>
> You will notice that the output is truncated (if it isn't just reload to
> get the cached version).
>
> I would love a speedy fix, as we're trying to use this in place of
> publishing pre-generated patches.
I guess it's the same problem we fixed before for ui-patch, so here's a
patch to move the fix into the common path for all pages.
cache.c| 6 ++
ui-patch.c | 2 --
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/cache.c b/cache.c
index 6736a01..2ccdc4e 100644
--- a/cache.c
+++ b/cache.c
@@ -224,6 +224,12 @@ static int fill_slot(struct cache_slot *slot)
/* Generate cache content */
slot->fn();
+ /* Make sure any buffered data is flushed to the file */
+ if (fflush(stdout)) {
+ close(tmp);
+ return errno;
+ }
+
/* update stat info */
if (fstat(slot->lock_fd, &slot->cache_st)) {
close(tmp);
diff --git a/ui-patch.c b/ui-patch.c
index 047e2f9..6745b69 100644
--- a/ui-patch.c
+++ b/ui-patch.c
@@ -92,6 +92,4 @@ 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.12.2.648.g6730d8bc62.dirty
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: Customization of log view
Hi Simon, On Sun, Apr 30, 2017 at 02:20:05PM +, Simon Steinbeiss wrote: > I couldn't find any information on this, so excuse me if you've heard this > question (or: request?) a gazillion times before. > We've been using cgit for Xfce for many years and we're really happy about > it. Recently we set up hooks to add references to related commits to > bugreports in our bugzilla instance. We also managed to make the bug > references in cgit's commit view hyperlinks to bugzilla, so those two parts > of our toolchain are more connected. > > What would be a nice extra is having clickable bug links in the log view, > but I couldn't find any reference that that is currently possible. So first > of all: is it possible to customize the log view so that we can either make > the bug references clickable hyperlinks (appearance-wise similar to the > branch and tag references) directly or e.g. add a new column with the > buglinks? > And if it currently isn't possible, is this a feature/option you would also > be interested in supporting? > > Finally I'm grateful about any pointers as to how to get going in that > direction! There's no functionality like this at the moment. It looks like you're putting bug references in the subject, so presumably adding a filter around the subject in ui-log.c is what you're looking for. We currently pass the subject through the commit filter for the commit UI, so I guess that could be reued for the log view. The problem is that in the log UI the subject is a hyperlink but all our current filters act on plain text. I guess you could pull apart cgit_commit_link() to allow opening the tag as a separate step and have the caller output the body of that tag, which could be passed through a filter. But I haven't thought through all of the edge cases here - what if the filter wants its own hyperlink, like your bug link? Would it be better to call the filter with the href as an argument, so it's up to the filter to output the anchor tag around the subject? John ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: RFE: have a way to request compressed patches and rawdiffs
On Fri, Apr 28, 2017 at 04:46:34PM +, Konstantin Ryabitsev wrote:
> Since I'm on an RFE binge, here's another one. :)
>
> Some of the rawdiffs we have been generating end up pretty large (e.g.
> linux-next to mainline rawdiff is around 80MB). We compress them using http
> protocol compression, but the reality is that most people would be getting
> these using wget or curl, which do not add the "accept-encoding: gzip"
> header and therefore get the uncompressed version.
>
> It would be great to be able to tell cgit to generate and send compressed
> versions of raw output like patches or rawdiffs.
Here's an initial attempt at this, it needs splitting up into a series
of a few patches (I think there's about four hiding in here), but I'm
sending it now to see if this is roughly what's needed.
The idea is to support extensions on page names (for pages that opt in)
and it's implemented for patch and rawdiff at the moment with support
for GZIP compressing the response body.
For example, given the URL:
https://git.zx2c4.com/cgit/patch/?id=8e9ddd21a50beb9fd660cf6cd6a583234924b932
with this change you can add ".gz" to the end of "patch":
https://git.zx2c4.com/cgit/patch.gz/?id=8e9ddd21a50beb9fd660cf6cd6a583234924b932
and the content will be returned compressed with gzip.
I've only added gzip for now, but the setup is table driven so adding
more compression filters should be straightforward.
Signed-off-by: John Keeping
---
cgit.c | 72 ++
cgit.h | 2 ++
cmd.c | 44 +++---
cmd.h | 3 ++-
filter.c | 2 ++
ui-diff.c | 6 +-
ui-patch.c | 10 +++--
7 files changed, 113 insertions(+), 26 deletions(-)
diff --git a/cgit.c b/cgit.c
index 1075753..8d25680 100644
--- a/cgit.c
+++ b/cgit.c
@@ -695,6 +695,46 @@ static inline void authenticate_cookie(void)
ctx.env.authenticated = cgit_close_filter(ctx.cfg.auth_filter);
}
+static struct cgit_filter *create_gzip_filter(void)
+{
+ char **argv = xcalloc(3, sizeof(char *));
+ struct cgit_exec_filter *f = xmalloc(sizeof(*f));
+
+ argv[0] = xstrdup("gzip");
+ argv[1] = xstrdup("-n");
+ argv[2] = NULL;
+
+ cgit_exec_filter_init(f, argv[0], argv);
+ return &f->base;
+}
+
+struct cgit_page_compression {
+ const char *const ext;
+ const char *const mimetype;
+ struct cgit_filter *(*create)(void);
+};
+
+static const struct cgit_page_compression cgit_page_compressions[] = {
+ { ".gz", "application/x-gzip", create_gzip_filter, },
+};
+
+static struct cgit_filter *get_page_compression(void)
+{
+ size_t i;
+
+ for (i = 0; i < ARRAY_SIZE(cgit_page_compressions); i++) {
+ const struct cgit_page_compression *c;
+
+ c = &cgit_page_compressions[i];
+ if (!strcmp(c->ext, ctx.qry.pageext)) {
+ ctx.page.mimetype = xstrdup(c->mimetype);
+ return c->create();
+ }
+ }
+
+ return NULL;
+}
+
static void process_request(void)
{
struct cgit_cmd *cmd;
@@ -738,6 +778,21 @@ static void process_request(void)
return;
}
+ if (ctx.qry.pageext) {
+ if (!cmd->want_compression) {
+ cgit_print_error_page(404, "Not found",
+ "Invalid request");
+ return;
+ }
+
+ ctx.page.body_filter = get_page_compression();
+ if (!ctx.page.body_filter) {
+ cgit_print_error_page(404, "Not found",
+ "Invalid request");
+ return;
+ }
+ }
+
if (ctx.repo && prepare_repo_cmd())
return;
@@ -1008,6 +1063,21 @@ static void cgit_parse_args(int argc, const char **argv)
}
}
+static void split_page_ext(void)
+{
+ char *dot;
+
+ if (!ctx.qry.page)
+ return;
+
+ dot = strchr(ctx.qry.page, '.');
+ if (!dot)
+ return;
+
+ ctx.qry.pageext = xstrdup(dot);
+ *dot = '\0';
+}
+
static int calc_ttl(void)
{
if (!ctx.repo)
@@ -1075,6 +1145,8 @@ int cmd_main(int argc, const char **argv)
cgit_parse_url(ctx.qry.url);
}
+ split_page_ext();
+
/* Before we go any further, we set ctx.env.authenticated by checking
to see
* if the supplied cookie is valid. All cookies are valid if there is no
* auth_filter. If there is an auth_filter, the filter decides. */
diff --git a/cgit.h b/cgit.h
index fbc6c6a..a04b03b 100644
--- a/cgit.h
Re: CGit in subdomain
On Fri, May 12, 2017 at 05:48:29PM +0200, Santi Moreno wrote: > I trying to config CGit in a server and all it's fine in the link > domain.org/cgit but I try config with Apache2 in subdomain > git.domain.org but the cgit.css not it's apply. I don't understand why > the default path in cgitrc is css=/css-git/css.git when my css file is > in /usr/share/cgit/css.git > Could you help me? The css value in cgitrc is the URL to be included in the element in the HTML documents served by CGit. You need to copy cgit.css so that your webserver will make it available and then set a suitable path in cgitrc. ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: CGit in subdomain
On Fri, May 12, 2017 at 06:54:31PM +0200, Santi Moreno wrote: > On 12 may, John Keeping wrote: > > On Fri, May 12, 2017 at 05:48:29PM +0200, Santi Moreno wrote: > > > I trying to config CGit in a server and all it's fine in the link > > > domain.org/cgit but I try config with Apache2 in subdomain > > > git.domain.org but the cgit.css not it's apply. I don't understand why > > > the default path in cgitrc is css=/css-git/css.git when my css file is > > > in /usr/share/cgit/css.git > > > Could you help me? > > > > The css value in cgitrc is the URL to be included in the > > element in the HTML documents served by > > CGit. You need to copy cgit.css so that your webserver will make it > > available and then set a suitable path in cgitrc. > > Thanks, but the css config is default but with this config in Apache2: > > > ServerName git.domain.org > DocumentRoot "/usr/share/cgit/" > ScriptAlias / "/usr/lib/cgit/cgit.cgi/" > Alias /cgit-css "/usr/share/cgit/" > > AllowOverride None > Options ExecCGI FollowSymlinks > Require all granted > > > > the final url is git.domain.org/css-cgit/css.git and not found it. The > url correct is domain.org/css-cgit/css.git. I need remove "git." from > the url for that found the file. Why not just copy the CSS file into /usr/lib/cgit/ ? It looks like that directory is already served, so what's the benefit of using /usr/share/cgit as the document root? ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: Bug: SIGSEGV in OPENSSL_cleanse
On Mon, May 22, 2017 at 07:54:53PM +0200, Jan Jancar wrote: > I am, or rather was, running an instance of cgit on an ARM box: > > > uname -srm > Linux 4.9.28-2-ARCH armv6l > > I run ArchLinuxARM and they recently had an update to openssl: > > openssl 1.1.0.e-1 > openssl-1.0 1.0.2.k-3 > > So I currently have 2 versions of openssl on that box. > > After running cgit for awhile I noticed it now SIGSEGVs on certain requests: > >PID: 12517 (cgit.cgi) >UID: 33 (http) >GID: 33 (http) > Signal: 11 (SEGV) > Timestamp: Sun 2017-05-21 13:26:35 CEST (1 day 6h ago) > Command Line: /usr/lib/cgit/cgit.cgi > Executable: /usr/lib/cgit/cgit.cgi > Control Group: /system.slice/system-uwsgi.slice/uwsgi@cgit.service > Unit: uwsgi@cgit.service > Slice: system-uwsgi.slice >Boot ID: 93dadbde0e144f3ab346f1e21ac7ee5d > Machine ID: 4bd17fc498ad478094fa58c3a7782769 > Hostname: Neuromancer >Storage: > /var/lib/systemd/coredump/core.cgit\x2ecgi.33.93dadbde0e144f3ab346f1e21ac7ee5d.12517.1495365995.lz4 >Message: Process 12517 (cgit.cgi) of user 33 dumped core. > > Stack trace of thread 12517: > #0 0x7678e7d8 OPENSSL_cleanse > (/usr/lib/libcrypto.so.1.0.0) > > > A bit more of the stack is shown when running the dump through gdb: > > #0 0x765c37d8 in OPENSSL_cleanse () from /usr/lib/libcrypto.so.1.0.0 > #1 0x7664243c in EVP_MD_CTX_cleanup () from /usr/lib/libcrypto.so.1.0.0 > #2 0x76642774 in EVP_MD_CTX_destroy () from /usr/lib/libcrypto.so.1.0.0 > Backtrace stopped: previous frame identical to this frame (corrupt stack?) > > > Investigating more, some weird behavior is shown, while ldd says > cgit.cgi will run with /usr/lib/libcrypto.so.1.0.0 and even the coredump > confirms that, when actually debugging the coredump, `info sharedlib` says: > > warning: Corrupted shared library list: 0x76ffa8b0 != 0x0 > FromTo Syms Read Shared Object Library > 0x76fcf8f0 0x76feaaf0 Yes (*) /lib/ld-linux-armhf.so.3 > 0x76e47000 0x76f6f794 Yes (*) /usr/lib/libcrypto.so.1.1 > 0x76dcdb88 0x76dda8a0 Yes (*) /usr/lib/libz.so.1 > 0x76da5190 0x76db6ce4 Yes (*) /usr/lib/libpthread.so.0 > 0x76d8b7d0 0x76d8f418 Yes (*) /usr/lib/librt.so.1 > 0x76d20580 0x76d74288 Yes (*) /usr/lib/libluajit-5.1.so.2 > 0x76d09a30 0x76d0a944 Yes (*) /usr/lib/libdl.so.2 > 0x76bd41f0 0x76cd4920 Yes (*) /usr/lib/libc.so.6 > 0x76b3c680 0x76b776f0 Yes (*) /usr/lib/libm.so.6 > 0x76b18180 0x76b26930 Yes /usr/lib/libgcc_s.so.1 > > After that, setting LD_PRELOAD=/usr/lib/libcrypto.so.1.0 > actually fixes the problem. So is cgit incompatible with openssl 1.1 > or is there some other bug hiding? Did you compile CGit yourself or are you using a pre-built package? What version of libssl-dev is installed? I wouldn't be surprised if compiling against openssl-1.0 headers but linking with openssl-1.1 produces the behaviour you describe above. ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: Sorting directories before files in tree view
On Fri, Jul 21, 2017 at 09:13:33PM +0530, John wrote: > Is this possible?. Not without significant changes to ui-tree.c. Currently we output items in the order they appear in the tree object being displayed (by definition, this is lexicographically sorted). Displaying them in a different order would require saving all of the items in memory and then sorting them before display. ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH 1/1] git: update to v2.13.2
On Wed, Jul 05, 2017 at 10:45:15AM +0200, Christian Hesse wrote: > Christian Hesse on Wed, 2017/07/05 10:43: > > From: Christian Hesse > > > > Update to git version v2.13.2: With commit 8aee769f (pathspec: copy and free > > owned memory) the definition of struct pathspec_item has changed with the > > expectation that pathspecs will be managed dynamically. We work around this > > a bit by setting up a static structure, but let's allocate the match string > > to avoid needing to cast away const. > > > > Updated a patch from John Keeping for git v2.12.1. > > John, please complain if you want to send this in. ;) No, I haven't had much time for CGit recently, so I'm happy for someone else to take over! ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH 04/07] Inject repo authorization filter. Provide sample for gitolite integration.
On Fri, Jun 23, 2017 at 06:57:12PM +0200, Georg Faerber wrote:
> Any chance of getting this merged?
The code changes look reasonable from a cursory reading, but all of the
patches are missing commit messages, and far more importantly are not
signed off (see https://git-scm.com/docs/git-commit.html#git-commit--s
and https://developercertificate.org/ for what this means).
> On 15-11-27 22:46:57, The Ranger wrote:
> > ---
> > filters/gitolite-authorization.lua | 74
> > ++
> > scan-tree.c| 18 ++
> > 2 files changed, 92 insertions(+)
> > create mode 100644 filters/gitolite-authorization.lua
> >
> > diff --git a/filters/gitolite-authorization.lua
> > b/filters/gitolite-authorization.lua
> > new file mode 100644
> > index 000..2f0e4f5
> > --- /dev/null
> > +++ b/filters/gitolite-authorization.lua
> > @@ -0,0 +1,74 @@
> > +-- This script can be used with project-filter option
> > +-- It uses REMOTE_USER environment variable to obtain the user who needs
> > to be authorized
> > +-- This variable is normally set by HTTP Basic Authentication.
> > +-- In Apache something like this can be used:
> > +--
> > +--AuthType Basic
> > +--AuthName Protected area
> > +--AuthUserFile users.htpasswd
> > +--Require valid-user
> > +--
> > +-- For anonymous access a public username can be set in environment config.
> > +-- In Apache, using mod_env:
> > +--
> > +--SetEnv REMOTE_USER gitweb
> > +--
> > +-- Gitolite requires HOME environment variable to work properly and point
> > to valid Gitolite
> > +-- environment. Since the user, under which web server process runs,
> > usually does not have
> > +-- this set, HOME should be explicitly configured and pointed to valid
> > gitolite setup.
> > +-- In Apache, using mod_env:
> > +--
> > +--SetEnv HOME /path/to/gitolite/home
> > +
> > +
> > +local git = {}
> > +local http = {}
> > +local repos = {}
> > +local action = nil
> > +
> > +function action_init()
> > + -- Anonymous access, cancel repo list building
> > + if git.user == nil or git.user == "" then return end
> > +
> > + local handle = io.popen("gitolite list-phy-repos | gitolite access % "
> > .. git.user .. " R any")
> > +
> > + while true do
> > + local repo = handle:read()
> > + if repo == nil then break end
> > +
> > + -- Skip DENIED repos
> > + if not string.find(repo, "DENIED") then
> > + -- Gitolite returns string: \t\t
> > + -- We are interested only in the first field for now
> > + -- Append .git extension since Gitolite does not and
> > cgit repo name has it
> > + local name = string.sub(repo, 0, string.find(repo,
> > "\t") - 1) .. ".git"
> > + repos[name] = 1 -- Authorize flag is > 0
> > + end
> > + end
> > +
> > + handle:close()
> > +end
> > +
> > +function action_filter()
> > + -- Return > 0 if access is authorized
> > + return repos[git.repo]
> > +end
> > +
> > +local actions = {}
> > +actions["init"] = action_init;
> > +actions["filter"] = action_filter;
> > +
> > +function filter_open(...)
> > + action = actions[select(1, ...)]
> > +
> > + git["repo"] = select(2, ...)
> > + git["user"] = select(3, ...)
> > +
> > + http["server"] = select(4, ...)
> > + http["path"] = select(5, ...)
> > +end
> > +
> > +function filter_close()
> > + return action()
> > +end
> > +
> > diff --git a/scan-tree.c b/scan-tree.c
> > index e17bca9..7490e74 100644
> > --- a/scan-tree.c
> > +++ b/scan-tree.c
> > @@ -74,6 +74,14 @@ static char *xstrrchr(char *s, char *from, int c)
> > return from < s ? NULL : from;
> > }
> >
> > +static int open_project_filter(const char *action, const char *repo) {
> > + return cgit_open_filter(ctx.cfg.project_filter, action, repo,
> > + ctx.env.remote_user ? ctx.env.remote_user : "",
> > + ctx.env.server_name ? ctx.env.server_name : "",
> > + ctx.env.path_info ? ctx.env.path_info : ""
> > + );
> > +}
> > +
> > static void add_repo(const char *base, struct strbuf *path, repo_config_fn
> > fn)
> > {
> > struct stat st;
> > @@ -115,6 +123,11 @@ static void add_repo(const char *base, struct strbuf
> > *path, repo_config_fn fn)
> > else if (rel.len && rel.buf[rel.len - 1] == '/')
> > strbuf_setlen(&rel, rel.len - 1);
> >
> > + if(ctx.cfg.project_filter) {
> > + if(open_project_filter("filter", rel.buf)) return;
> > + if(cgit_close_filter(ctx.cfg.project_filter) < 1) return;
> > + }
> > +
> > repo = cgit_add_repo(rel.buf);
> > config_fn = fn;
> > if (ctx.cfg.enable_git_config) {
> > @@ -261,6 +274,11 @@ void scan_projects(const char *path, const char
> > *projectsfile, repo_config_fn fn
> >
> > void scan_tree(const char *path, repo_config_fn fn)
> > {
> > + if (ctx.cfg.project_filter)
Re: [PATCH 1/1] ui-repolist: remove unused variable
On Tue, Jun 06, 2017 at 04:14:47PM +0200, Christian Hesse wrote:
> From: Christian Hesse
>
> Signed-off-by: Christian Hesse
Reviewed-by: John Keeping
Fixes: 87c4748 (ui-repolist: properly sort by age, 2017-03-30)
> ---
> ui-repolist.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/ui-repolist.c b/ui-repolist.c
> index 20a4f56..7272e87 100644
> --- a/ui-repolist.c
> +++ b/ui-repolist.c
> @@ -225,7 +225,6 @@ static int sort_section(const void *a, const void *b)
> const struct cgit_repo *r1 = a;
> const struct cgit_repo *r2 = b;
> int result;
> - time_t t;
>
> result = cmp(r1->section, r2->section);
> if (!result) {
> --
> 2.13.1
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [RFC PATCH 1/4] git: update to v2.14
On Wed, Jun 07, 2017 at 09:18:07PM -0500, Jeff Smith wrote:
> Update to git version v2.14: commit 6b526ce (Merge branch bc/object-id)
> merged changes for several functions from using sha1 hashes to using
> struct object_id pointers. The functions that affect cgit are:
> parse_object, lookup_commit_reference, lookup_tag, lookup_tree, and
> parse_tree_indirect.
>
> Signed-off-by: Jeff Smith
Of course, our tests fail with this patch because the submodule has been
updated without the Makefile... other than that, the changes all look
reasonable, so when 2.14 final is out and we can update the Makefile to
a tag:
Reviewed-by: John Keeping
> ---
> git | 2 +-
> shared.c | 2 +-
> ui-blob.c | 6 +++---
> ui-clone.c| 2 +-
> ui-commit.c | 4 ++--
> ui-diff.c | 4 ++--
> ui-patch.c| 4 ++--
> ui-plain.c| 2 +-
> ui-snapshot.c | 2 +-
> ui-tag.c | 4 ++--
> ui-tree.c | 18 +-
> 11 files changed, 25 insertions(+), 25 deletions(-)
>
> diff --git a/git b/git
> index 2c04f63..8d1b103 16
> --- a/git
> +++ b/git
> @@ -1 +1 @@
> -Subproject commit 2c04f6340579518c55a554fcac9fe21c01b3d3ea
> +Subproject commit 8d1b10321b20bd2a73a5b561cfc3cf2e8051b70b
> diff --git a/shared.c b/shared.c
> index 13a65a9..c93b193 100644
> --- a/shared.c
> +++ b/shared.c
> @@ -160,7 +160,7 @@ static struct refinfo *cgit_mk_refinfo(const char
> *refname, const struct object_
>
> ref = xmalloc(sizeof (struct refinfo));
> ref->refname = xstrdup(refname);
> - ref->object = parse_object(oid->hash);
> + ref->object = parse_object(oid);
> switch (ref->object->type) {
> case OBJ_TAG:
> ref->tag = cgit_parse_tag((struct tag *)ref->object);
> diff --git a/ui-blob.c b/ui-blob.c
> index 793817f..761e886 100644
> --- a/ui-blob.c
> +++ b/ui-blob.c
> @@ -56,7 +56,7 @@ int cgit_ref_path_exists(const char *path, const char *ref,
> int file_only)
> goto done;
> if (sha1_object_info(oid.hash, &size) != OBJ_COMMIT)
> goto done;
> - read_tree_recursive(lookup_commit_reference(oid.hash)->tree, "", 0, 0,
> &paths, walk_tree, &walk_tree_ctx);
> + read_tree_recursive(lookup_commit_reference(&oid)->tree, "", 0, 0,
> &paths, walk_tree, &walk_tree_ctx);
>
> done:
> free(path_items.match);
> @@ -89,7 +89,7 @@ int cgit_print_file(char *path, const char *head, int
> file_only)
> return -1;
> type = sha1_object_info(oid.hash, &size);
> if (type == OBJ_COMMIT) {
> - commit = lookup_commit_reference(oid.hash);
> + commit = lookup_commit_reference(&oid);
> read_tree_recursive(commit->tree, "", 0, 0, &paths, walk_tree,
> &walk_tree_ctx);
> if (!walk_tree_ctx.found_path)
> return -1;
> @@ -145,7 +145,7 @@ void cgit_print_blob(const char *hex, char *path, const
> char *head, int file_onl
> type = sha1_object_info(oid.hash, &size);
>
> if ((!hex) && type == OBJ_COMMIT && path) {
> - commit = lookup_commit_reference(oid.hash);
> + commit = lookup_commit_reference(&oid);
> read_tree_recursive(commit->tree, "", 0, 0, &paths, walk_tree,
> &walk_tree_ctx);
> type = sha1_object_info(oid.hash, &size);
> }
> diff --git a/ui-clone.c b/ui-clone.c
> index 5f6606a..0d11672 100644
> --- a/ui-clone.c
> +++ b/ui-clone.c
> @@ -17,7 +17,7 @@ static int print_ref_info(const char *refname, const struct
> object_id *oid,
> {
> struct object *obj;
>
> - if (!(obj = parse_object(oid->hash)))
> + if (!(obj = parse_object(oid)))
> return 0;
>
> htmlf("%s\t%s\n", oid_to_hex(oid), refname);
> diff --git a/ui-commit.c b/ui-commit.c
> index db69d54..e1d4a9b 100644
> --- a/ui-commit.c
> +++ b/ui-commit.c
> @@ -31,7 +31,7 @@ void cgit_print_commit(char *hex, const char *prefix)
> "Bad object id: %s", hex);
> return;
> }
> - commit = lookup_commit_reference(oid.hash);
> + commit = lookup_commit_reference(&oid);
> if (!commit) {
> cgit_print_error_page(404, "Not found",
> "Bad commit reference: %s", hex);
> @@ -87,7 +87,7 @@ void cgit_print_commit(char *hex, const char *prefix)
> free(tmp);
> html("\n");
> for (p = commit->parents; p; p =
Re: [RFC PATCH 2/4] ui-blame: create placeholder and links
On Wed, Jun 07, 2017 at 09:18:08PM -0500, Jeff Smith wrote:
> Create a placeholder for and links to a page that will contain the
> 'blame' for a file in the repository.
>
> Signed-off-by: Jeff Smith
> ---
> cgit.mk | 1 +
> cmd.c | 8 ++-
> ui-blame.c | 160
>
> ui-blame.h | 7 +++
> ui-shared.c | 20 ++--
> ui-shared.h | 3 ++
> ui-tree.c | 8 ++-
> 7 files changed, 202 insertions(+), 5 deletions(-)
> create mode 100644 ui-blame.c
> create mode 100644 ui-blame.h
>
> diff --git a/cgit.mk b/cgit.mk
> index 90a2346..3fcc1ca 100644
> --- a/cgit.mk
> +++ b/cgit.mk
> @@ -77,6 +77,7 @@ CGIT_OBJ_NAMES += parsing.o
> CGIT_OBJ_NAMES += scan-tree.o
> CGIT_OBJ_NAMES += shared.o
> CGIT_OBJ_NAMES += ui-atom.o
> +CGIT_OBJ_NAMES += ui-blame.o
> CGIT_OBJ_NAMES += ui-blob.o
> CGIT_OBJ_NAMES += ui-clone.o
> CGIT_OBJ_NAMES += ui-commit.o
> diff --git a/cmd.c b/cmd.c
> index d280e95..262e31f 100644
> --- a/cmd.c
> +++ b/cmd.c
> @@ -1,6 +1,6 @@
> /* cmd.c: the cgit command dispatcher
> *
> - * Copyright (C) 2006-2014 cgit Development Team
> + * Copyright (C) 2006-2017 cgit Development Team
> *
> * Licensed under GNU General Public License v2
> * (see COPYING for full license text)
> @@ -11,6 +11,7 @@
> #include "cache.h"
> #include "ui-shared.h"
> #include "ui-atom.h"
> +#include "ui-blame.h"
> #include "ui-blob.h"
> #include "ui-clone.h"
> #include "ui-commit.h"
> @@ -63,6 +64,10 @@ static void about_fn(void)
> cgit_print_site_readme();
> }
>
> +static void blame_fn(void)
> +{
> + cgit_print_blame();
> +}
Missing blank line here.
> static void blob_fn(void)
> {
> cgit_print_blob(ctx.qry.sha1, ctx.qry.path, ctx.qry.head, 0);
I don't have any comments on the rest of this patch, although I'm not
sure about the order of this series. I think it would be cleaner to
have:
- a patch adding ui-blame.[ch] in their final form along with the
Makefile change to enable building but no other changes
- follow-up patch(es) to link the blame UI in to the rest of the system
(I don't know if this should be one patch, or a series adding the
command handling, then ui-shared changes followed by links in various
other ui-*.c files)
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [RFC PATCH 3/4] ui-blame: create needed html_ntxt_noellipsis function
On Wed, Jun 07, 2017 at 09:18:09PM -0500, Jeff Smith wrote:
> For implementing a ui-blame page, there is need for a function that
> outputs a selection from a block of text, transformed for HTML output,
> but with no further modifications or additions.
>
> Signed-off-by: Jeff Smith
> ---
> html.c | 37 -
> html.h | 1 +
> 2 files changed, 17 insertions(+), 21 deletions(-)
We only use the current html_ntxt() function in one place so I don't
think there's any real need to keep the name as is.
The end result would be nicer if html_ntxt() is your new "no ellipsis"
variant which can return an int to indicate whether the text was
truncated, then ui-repolist can switch to a new html_txt_ellipsis()
which has the same effect as the current html_ntxt().
> diff --git a/html.c b/html.c
> index e7e6e07..c7d1c64 100644
> --- a/html.c
> +++ b/html.c
> @@ -122,10 +122,10 @@ void html_vtxtf(const char *format, va_list ap)
> strbuf_release(&buf);
> }
>
> -void html_txt(const char *txt)
> +static int _html_txt(int len, const char *txt)
> {
> const char *t = txt;
> - while (t && *t) {
> + while (t && *t && len--) {
> int c = *t;
> if (c == '<' || c == '>' || c == '&') {
> html_raw(txt, t - txt);
> @@ -140,32 +140,27 @@ void html_txt(const char *txt)
> t++;
> }
> if (t != txt)
> - html(txt);
> + html_raw(txt, t - txt);
> + return len;
> +}
> +
> +void html_txt(const char *txt)
> +{
> + if (txt)
> + _html_txt(strlen(txt), txt);
> }
>
> void html_ntxt(int len, const char *txt)
> {
> - const char *t = txt;
> - while (t && *t && len--) {
> - int c = *t;
> - if (c == '<' || c == '>' || c == '&') {
> - html_raw(txt, t - txt);
> - if (c == '>')
> - html(">");
> - else if (c == '<')
> - html("<");
> - else if (c == '&')
> - html("&");
> - txt = t + 1;
> - }
> - t++;
> - }
> - if (t != txt)
> - html_raw(txt, t - txt);
> - if (len < 0)
> + if (_html_txt(len, txt) < 0)
> html("...");
> }
>
> +void html_ntxt_noellipsis(int len, const char *txt)
> +{
> + _html_txt(len, txt);
> +}
> +
> void html_attrf(const char *fmt, ...)
> {
> va_list ap;
> diff --git a/html.h b/html.h
> index 1b24e55..e1b85b3 100644
> --- a/html.h
> +++ b/html.h
> @@ -20,6 +20,7 @@ extern void html_attrf(const char *format,...);
>
> extern void html_txt(const char *txt);
> extern void html_ntxt(int len, const char *txt);
> +extern void html_ntxt_noellipsis(int len, const char *txt);
> extern void html_attr(const char *txt);
> extern void html_url_path(const char *txt);
> extern void html_url_arg(const char *txt);
> --
> 2.9.3
>
> ___
> CGit mailing list
> CGit@lists.zx2c4.com
> https://lists.zx2c4.com/mailman/listinfo/cgit
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [RFC PATCH 4/4] ui-blame: fill in the contents
On Wed, Jun 07, 2017 at 09:18:10PM -0500, Jeff Smith wrote:
> Use the blame interface added in libgit to output the blame information
> of a file in the repository.
>
> Signed-off-by: Jeff Smith
> ---
> cgit.css | 8 +++
> ui-blame.c | 214
> -
> 2 files changed, 220 insertions(+), 2 deletions(-)
>
> diff --git a/cgit.css b/cgit.css
> index 1dc2c11..258991c 100644
> --- a/cgit.css
> +++ b/cgit.css
> @@ -329,6 +329,14 @@ div#cgit table.ssdiff td.lineno a:hover {
> color: black;
> }
>
> +div#cgit table.blame tr:nth-child(even) {
> + background: #f7f0e7;
> +}
> +
> +div#cgit table.blame tr:nth-child(odd) {
> + background: white;
> +}
> +
> div#cgit table.bin-blob {
> margin-top: 0.5em;
> border: solid 1px black;
> diff --git a/ui-blame.c b/ui-blame.c
> index 901ca89..6ad0009 100644
> --- a/ui-blame.c
> +++ b/ui-blame.c
> @@ -10,6 +10,187 @@
> #include "ui-blame.h"
> #include "html.h"
> #include "ui-shared.h"
> +#include "argv-array.h"
> +#include "blame.h"
> +
> +
> +/* Remember to update object flag allocation in object.h */
This comment doesn't really apply in CGit, but it looks like the define
is entirely unused, so we can just delete it.
> +#define MORE_THAN_ONE_PATH (1u<<13)
> +
> +/*
> + * Information on commits, used for output.
> + */
Can we reuse our existing struct commitinfo here? I can't see anything
that this function does that our existing cgit_parse_commit() doesn't do
and cgit_parse_commit() deals with encoding properly as well.
> +struct commit_info {
> + struct strbuf author;
> + struct strbuf author_mail;
> + struct ident_split author_ident;
> +
> + /* filled only when asked for details */
> + struct strbuf committer;
> + struct strbuf committer_mail;
> + struct ident_split committer_ident;
> +
> + struct strbuf summary;
> +};
> +
[snip parsing details]
> +static void get_commit_info(struct commit *commit,
> + struct commit_info *ret,
> + int detailed)
This is only ever called with detailed == 1.
> +{
> + int len;
> + const char *subject, *encoding;
> + const char *message;
> +
> + commit_info_init(ret);
> +
> + encoding = get_log_output_encoding();
> + message = logmsg_reencode(commit, NULL, encoding);
> + get_ac_line(message, "\nauthor ",
> + &ret->author, &ret->author_mail,
> + &ret->author_ident);
> +
> + if (!detailed) {
> + unuse_commit_buffer(commit, message);
> + return;
> + }
> +
> + get_ac_line(message, "\ncommitter ",
> + &ret->committer, &ret->committer_mail,
> + &ret->committer_ident);
> +
> + len = find_commit_subject(message, &subject);
> + if (len)
> + strbuf_add(&ret->summary, subject, len);
> + else
> + strbuf_addf(&ret->summary, "(%s)",
> oid_to_hex(&commit->object.oid));
> +
> + unuse_commit_buffer(commit, message);
> +}
[snip most of the code, which all looks entirely reasonable.]
> @@ -76,7 +278,15 @@ static void print_object(const unsigned char *sha1, const
> char *path, const char
> return;
> }
>
> - /* Output data here */
> + html("");
> + for (ent = sb.ent; ent; ) {
> + struct blame_entry *e = ent->next;
> + emit_blame_entry(&sb, ent);
> + free(ent);
> + ent = e;
> + }
> + html("\n");
> + free((void *)sb.final_buf);
There's no need to cast to void* in C, so drop the cast here.
>
> cgit_print_layout_end();
> return;
> --
> 2.9.3
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [RFC PATCH 0/4] Add ui-blame
On Wed, Jun 07, 2017 at 09:18:06PM -0500, Jeff Smith wrote: > I split git blame functionality into libgit, and the changes were > accepted upstream a few days ago. Now that the git infrastructure is in > place, it is time to get back to the cgit part. > > The first patch advances git to current master (which should be future > v2.14), and makes changes made necessary by doing so. The remaining > patches deal with adding the new blame functionality. > > Based on branch ch/git-2-13-1. > > Jeff Smith (4): > git: update to v2.14 > ui-blame: create placeholder and links > ui-blame: create needed html_ntxt_noellipsis function > ui-blame: fill in the contents This looks like a good start. I've commented on the individual patches, but apart from the patch order and struct commit_info question they're all minor. I have a general question, possible more for any administrators of CGit sites who happen to see this: Should we offer a configuration switch to disable the blame function? Blame is a comparatively expensive operation compared to everything else we do to display a page, so is there a desire to disable this feature for sites worried about resource usage? Finally, some general comments on the output formatting in the browser: - Should the commit SHA be fixed-width? I'm comparing the output with http://repo.or.cz/git.git/blame_incremental/HEAD:/branch.c which I think is a bit easier to read. - The tooltip looks very confusing since the labels and values aren't aligned. I'd simplify it to show either the author's name and date (like GitWeb in the link above) or just the commit message. ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: your mail
On Tue, May 09, 2017 at 03:35:57AM -0400, Ghost Squad 57 wrote: > Lately I've gotten into the habit of signing commits and tags with my GPG > key (https://git-scm.com/book/en/v2/Git-Tools-Signing-Your-Work) > > But it appears cgit doesn't support showing commits that have been signed. > > Is there a way to enable this? No, we don't have any support for this at the moment. What would you expect to see for a signed commit? Do you want the server to validate the signature? In which case, how should the trusted signers be configured? ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: your mail
[Please keep the mailing list cc'd.] On Sat, Jul 22, 2017 at 12:32:40PM -0400, Ghost Squad 57 wrote: > Personally, I just want cgit to show the key used to sign the commit, not > necessarily validate it. Validation could always be done on the user's side. I would be very concerned about giving a false sense of security by doing this. It sounds like you want something like "good signature by untrusted key ...", but then doing validation on the user's side requires cloning the repository, doesn't it? Or do you mean that the user should trust the server and just say "yes, that's the key I expect to have signed this"? That's not behaviour that we should be encouraging. > On Jul 22, 2017 8:04 AM, "John Keeping" wrote: > > > On Tue, May 09, 2017 at 03:35:57AM -0400, Ghost Squad 57 wrote: > > > Lately I've gotten into the habit of signing commits and tags with my GPG > > > key (https://git-scm.com/book/en/v2/Git-Tools-Signing-Your-Work) > > > > > > But it appears cgit doesn't support showing commits that have been > > signed. > > > > > > Is there a way to enable this? > > > > No, we don't have any support for this at the moment. What would you > > expect to see for a signed commit? Do you want the server to validate > > the signature? In which case, how should the trusted signers be > > configured? > > ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH 0/4] Advance to git 2.14
On Tue, Aug 08, 2017 at 07:03:01AM -0500, Jeff Smith wrote: > This set of patches advances git to v2.14, and makes the neccessary > changes in cgit to build with it. My ui-blame changes depend on > v2.14 and will thus depend on these changes. I haven't pulled this to test, but it looks like the following patches are all build fixes without which the initial commit doesn't build. If that is the case, these should be squashed to a single patch. "git rebase -x 'make test' master" should succeed when run on a branch containing these patches. > Jeff Smith (4): > git: update to v2.14 > git v2.14: use object_id structure for hashes > git v2.14: include config.h when needed > git v2.14: change how ignore-case is specified > > Makefile | 2 +- > git | 2 +- > scan-tree.c | 5 +++-- > shared.c | 6 +++--- > ui-blob.c | 6 +++--- > ui-clone.c| 2 +- > ui-commit.c | 6 +++--- > ui-diff.c | 18 +- > ui-log.c | 10 +- > ui-patch.c| 4 ++-- > ui-plain.c| 2 +- > ui-snapshot.c | 2 +- > ui-tag.c | 4 ++-- > ui-tree.c | 18 +- > 14 files changed, 44 insertions(+), 43 deletions(-) > > -- > 2.9.4 ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH v2] git: update to v2.14
On Wed, Aug 09, 2017 at 07:02:56PM -0500, Jeff Smith wrote: > Numerous changes were made to git functions to use an object_id > structure rather than sending sha1 hashes as raw unsigned character > arrays. The functions that affect cgit are: parse_object, > lookup_commit_reference, lookup_tag, lookup_tree, parse_tree_indirect, > diff_root_tree_sha1, diff_tree_sha1, and format_display_notes. > > Commit b2141fc (config: don't include config.h by default) made it > necessary to that config.h be explicitly included when needed. > > Commit 07a3d41 (grep: remove regflags from the public grep_opt API) > removed one way of specifying the ignore-case grep option. > > Signed-off-by: Jeff Smith > --- > Makefile | 2 +- > git | 2 +- > scan-tree.c | 5 +++-- > shared.c | 6 +++--- > ui-blob.c | 6 +++--- > ui-clone.c| 2 +- > ui-commit.c | 6 +++--- > ui-diff.c | 18 +- > ui-log.c | 10 +- > ui-patch.c| 4 ++-- > ui-plain.c| 2 +- > ui-snapshot.c | 2 +- > ui-tag.c | 4 ++-- > ui-tree.c | 18 +- > 14 files changed, 44 insertions(+), 43 deletions(-) > > diff --git a/Makefile b/Makefile > index 3d792ce..f3ee84c 100644 > --- a/Makefile > +++ b/Makefile > @@ -14,7 +14,7 @@ htmldir = $(docdir) > pdfdir = $(docdir) > mandir = $(prefix)/share/man > SHA1_HEADER = > -GIT_VER = 2.13.4 > +GIT_VER = 2.14.0 > 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 cf8899d..4384e3c 16 > --- a/git > +++ b/git > @@ -1 +1 @@ > -Subproject commit cf8899d285d2648013040ec7196ffd3de0606664 > +Subproject commit 4384e3cde2ce8ecd194202e171ae16333d241326 > diff --git a/scan-tree.c b/scan-tree.c > index 08f3f1d..c8d175e 100644 > --- a/scan-tree.c > +++ b/scan-tree.c > @@ -10,6 +10,7 @@ > #include "scan-tree.h" > #include "configfile.h" > #include "html.h" > +#include "config.h" We normally include Git headers with <>, and in fact they're all in cgit.h. I'm not sure whether it's better to add config.h to the list in cgit.h or keep it here, but I do think it should be distinguished from CGit headers by using <> and probably separating it a bit. The rest of the patch looks good to me. ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: cgit segfaults
On Wed, Aug 16, 2017 at 01:26:52AM -0500, Robby Workman wrote: > We're running cgit-1.1 with git-2.10.4 at https://git.slackbuilds.org and are > seeing > some reproducible segfaults. > > root@git:/var/log# dmesg -T > [Wed Aug 16 01:14:23 2017] traps: cgit.cgi[2210] general protection ip:4515bd > sp:7ffd787a9470 error:0 in cgit.cgi[40+103000] > > This can be reliably triggered (i.e. every time) with at least one particular > link (I'll share it > privately with cgit devs, but since I don't know if there's any security > impact, I'm not going > to put it out on the list as yet). > > I've applied 1b4ef6783a71962f8b5da3a23f283 and c699866699411346c5dba4064575 > from git master since they appeared to address some segfaults, but apparently > they were > unrelated to whatever it is that we're seeing. > > Aside from (obviously) sharing the reproducer, any tips on debugging this? We > of course > have a strong preference for debugging tips that don't impact services on the > machine, > but if needed, we'll do what we have to do... You can run cgit from the command line with your config and the URL using something like: CGIT_CONFIG=/path/to/cgitrc QUERY_STRING=url=cgit/repo/... cgit This is what the tests do in tests/setup.sh::cgit_url(). That should allow you to build a debug binary and reproduce under that without a webserver involved, which means you can run under gdb or valgrind. ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: cgit segfaults
On Thu, Aug 24, 2017 at 01:18:20AM -0500, Robby Workman wrote: > On Wed, 16 Aug 2017 09:36:28 +0100 > John Keeping wrote: > > > On Wed, Aug 16, 2017 at 01:26:52AM -0500, Robby Workman wrote: > > > We're running cgit-1.1 with git-2.10.4 at > > > https://git.slackbuilds.org and are seeing some reproducible > > > segfaults. > > > > > > root@git:/var/log# dmesg -T > > > [Wed Aug 16 01:14:23 2017] traps: cgit.cgi[2210] general protection > > > ip:4515bd sp:7ffd787a9470 error:0 in cgit.cgi[40+103000] > > > > > > This can be reliably triggered (i.e. every time) with at least one > > > particular link (I'll share it privately with cgit devs, but since > > > I don't know if there's any security impact, I'm not going to put > > > it out on the list as yet). > > > > > > I've applied 1b4ef6783a71962f8b5da3a23f283 and > > > c699866699411346c5dba4064575 from git master since they appeared to > > > address some segfaults, but apparently they were unrelated to > > > whatever it is that we're seeing. > > > > > > Aside from (obviously) sharing the reproducer, any tips on > > > debugging this? We of course have a strong preference for debugging > > > tips that don't impact services on the machine, but if needed, > > > we'll do what we have to do... > > > > You can run cgit from the command line with your config and the URL > > using something like: > > > > CGIT_CONFIG=/path/to/cgitrc QUERY_STRING=url=cgit/repo/... > > cgit > > > > This is what the tests do in tests/setup.sh::cgit_url(). > > > > That should allow you to build a debug binary and reproduce under that > > without a webserver involved, which means you can run under gdb or > > valgrind. > > > Okay, that's helpful - thanks! I've got something that seems to point > at git's pathspec.c (we're building with (and using on the machine) > git-2.10.4 currently), but I have no idea where to go from here. > This is the gdb output: > > (gdb) run > Starting program: /var/www/cgi-bin/cgit.cgi > [Thread debugging using libthread_db enabled] > Using host libthread_db library "/lib64/libthread_db.so.1". > Content-Type: text/plain; charset=UTF-8 > Content-Disposition: inline; > filename="82746b4b48cec68acdbb5b7a5ad841b1a21872af..65131f01e212203fbde61d3074640651a02cb6e0.patch" > Last-Modified: Thu, 24 Aug 2017 06:08:13 GMT > Expires: Thu, 24 Aug 2017 06:13:13 GMT > > > Program received signal SIGSEGV, Segmentation fault. > 0x004515bd in prefix_pathspec (elt=0x6234623634373238 access memory at address 0x6234623634373238>, prefixlen=0, prefix=0x0, > flags=0, > raw=0x77a138, p_short_magic=, item=0x77a808) at > pathspec.c:149 > 149 if (elt[0] != ':' || literal_global || > (gdb) What version of CGit are you using? It looks like you could be missing commit be39d22 (ui-patch: fix crash when using path limit, 2016-11-24) and using a version affected by the problem that patch fixes. ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH] Use https for submodule
Please make use of the commit message to explain why this is a good change. On Mon, Sep 18, 2017 at 04:09:53PM -0400, Daniel M. Weeks wrote: > Signed-off-by: Daniel M. Weeks With a more comprehensive commit message, Reviewed-by: John Keeping > --- > .gitmodules | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/.gitmodules b/.gitmodules > index 1daea94..5c6ecb4 100644 > --- a/.gitmodules > +++ b/.gitmodules > @@ -1,3 +1,3 @@ > [submodule "git"] > - url = git://git.kernel.org/pub/scm/git/git.git > + url = https://git.kernel.org/pub/scm/git/git.git > path = git ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [RFCv2 PATCH 2/7] ui-blame: create framework
On Fri, Sep 22, 2017 at 10:38:43PM -0500, Jeff Smith wrote:
> Create framework for a page that will contain the 'blame' for a file
> in the repository.
>
> Signed-off-by: Jeff Smith
> ---
> ui-blame.c | 160
> +
> ui-blame.h | 7 +++
> 2 files changed, 167 insertions(+)
> create mode 100644 ui-blame.c
> create mode 100644 ui-blame.h
>
> diff --git a/ui-blame.c b/ui-blame.c
> new file mode 100644
> index 000..901ca89
> --- /dev/null
> +++ b/ui-blame.c
> @@ -0,0 +1,160 @@
> +/* ui-blame.c: functions for blame output
> + *
> + * Copyright (C) 2006-2017 cgit Development Team
> + *
> + * Licensed under GNU General Public License v2
> + * (see COPYING for full license text)
> + */
> +
> +#include "cgit.h"
> +#include "ui-blame.h"
> +#include "html.h"
> +#include "ui-shared.h"
> +
> +struct walk_tree_context {
> + char *curr_rev;
> + int match_baselen;
> + int state;
> +};
> +
> +static void set_title_from_path(const char *path)
This looks exactly the same as the function in ui-tree.c, so can't we
extract it to ui-shared.c?
> +{
> + size_t path_len, path_index, path_last_end;
> + char *new_title;
> +
> + if (!path)
> + return;
> +
> + path_len = strlen(path);
> + new_title = xmalloc(path_len + 3 + strlen(ctx.page.title) + 1);
> + new_title[0] = '\0';
> +
> + for (path_index = path_len, path_last_end = path_len; path_index-- >
> 0;) {
> + if (path[path_index] == '/') {
> + if (path_index == path_len - 1) {
> + path_last_end = path_index - 1;
> + continue;
> + }
> + strncat(new_title, &path[path_index + 1], path_last_end
> - path_index - 1);
> + strcat(new_title, "\\");
> + path_last_end = path_index;
> + }
> + }
> + if (path_last_end)
> + strncat(new_title, path, path_last_end);
> +
> + strcat(new_title, " - ");
> + strcat(new_title, ctx.page.title);
> + ctx.page.title = new_title;
> +}
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [RFCv2 PATCH 1/7] ui-blame: create enable-blame config item
On Fri, Sep 22, 2017 at 10:38:42PM -0500, Jeff Smith wrote:
> Signed-off-by: Jeff Smith
> ---
> cgit.c | 3 +++
> cgit.h | 1 +
> cgitrc.5.txt | 5 +
> 3 files changed, 9 insertions(+)
>
> diff --git a/cgit.c b/cgit.c
> index 1dae4b8..c03f69c 100644
> --- a/cgit.c
> +++ b/cgit.c
> @@ -183,6 +183,8 @@ static void config_cb(const char *name, const char *value)
> ctx.cfg.enable_tree_linenumbers = atoi(value);
> else if (!strcmp(name, "enable-git-config"))
> ctx.cfg.enable_git_config = atoi(value);
> + else if (!strcmp(name, "enable-blame"))
> + ctx.cfg.enable_blame = atoi(value);
> else if (!strcmp(name, "max-stats"))
> ctx.cfg.max_stats = cgit_find_stats_period(value, NULL);
> else if (!strcmp(name, "cache-size"))
> @@ -373,6 +375,7 @@ static void prepare_context(void)
> ctx.cfg.enable_index_owner = 1;
> ctx.cfg.enable_tree_linenumbers = 1;
> ctx.cfg.enable_git_config = 0;
> + ctx.cfg.enable_blame = 1;
Should this default to false? Either way, I think the commit message
should have some discussion of what the default is any why it has been
chosen.
> ctx.cfg.max_repo_count = 50;
> ctx.cfg.max_commit_count = 50;
> ctx.cfg.max_lock_attempts = 5;
> diff --git a/cgit.h b/cgit.h
> index fbc6c6a..a48e622 100644
> --- a/cgit.h
> +++ b/cgit.h
> @@ -236,6 +236,7 @@ struct cgit_config {
> int enable_html_serving;
> int enable_tree_linenumbers;
> int enable_git_config;
> + int enable_blame;
> int local_time;
> int max_atom_items;
> int max_repo_count;
> diff --git a/cgitrc.5.txt b/cgitrc.5.txt
> index 9fcf445..b236bf2 100644
> --- a/cgitrc.5.txt
> +++ b/cgitrc.5.txt
> @@ -211,6 +211,11 @@ enable-git-config::
> with "cgit." will be mapped to the corresponding "repo." key in cgit.
> Default value: "0". See also: scan-path, section-from-path.
>
> +enable-blame::
> + Flag which, when set to "1", will allow cgit to provide a "blame" page
> + for files, and will make it generate links to that page in appropriate
> + places. Default value: "1".
> +
Although the handful of enable-* options above this are not well sorted,
most of this file is, so this should move to above enable-commit-graph.
> favicon::
> Url used as link to a shortcut icon for cgit. It is suggested to use
> the value "/favicon.ico" since certain browsers will ignore other
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [RFCv2 PATCH 4/7] ui-blame: html_ntxt with no ellipsis
On Fri, Sep 22, 2017 at 10:38:45PM -0500, Jeff Smith wrote:
> For implementing a ui-blame page, there is need for a function that
> outputs a selection from a block of text, transformed for HTML output,
> but with no further modifications or additions.
>
> Signed-off-by: Jeff Smith
> ---
> html.c| 37 -
> html.h| 1 +
> ui-repolist.c | 2 +-
> 3 files changed, 18 insertions(+), 22 deletions(-)
>
> diff --git a/html.c b/html.c
> index e7e6e07..345300e 100644
> --- a/html.c
> +++ b/html.c
> @@ -122,10 +122,10 @@ void html_vtxtf(const char *format, va_list ap)
> strbuf_release(&buf);
> }
>
> -void html_txt(const char *txt)
> +static int _html_txt(int len, const char *txt)
This should use ssize_t not int.
> {
> const char *t = txt;
> - while (t && *t) {
> + while (t && *t && len--) {
> int c = *t;
> if (c == '<' || c == '>' || c == '&') {
> html_raw(txt, t - txt);
> @@ -140,29 +140,24 @@ void html_txt(const char *txt)
> t++;
> }
> if (t != txt)
> - html(txt);
> + html_raw(txt, t - txt);
> + return len;
> +}
> +
> +void html_txt(const char *txt)
> +{
> + if (txt)
> + _html_txt(strlen(txt), txt);
> }
>
> void html_ntxt(int len, const char *txt)
> {
> - const char *t = txt;
> - while (t && *t && len--) {
> - int c = *t;
> - if (c == '<' || c == '>' || c == '&') {
> - html_raw(txt, t - txt);
> - if (c == '>')
> - html(">");
> - else if (c == '<')
> - html("<");
> - else if (c == '&')
> - html("&");
> - txt = t + 1;
> - }
> - t++;
> - }
> - if (t != txt)
> - html_raw(txt, t - txt);
> - if (len < 0)
> + _html_txt(len, txt);
Why bother with this wrapper function? Just make html_ntxt be _html_txt
and ignore the result everywhere except html_ntxt_ellipsis.
> +}
> +
> +void html_ntxt_ellipsis(int len, const char *txt)
> +{
> + if (_html_txt(len, txt) < 0)
> html("...");
> }
>
> diff --git a/html.h b/html.h
> index 1b24e55..de53430 100644
> --- a/html.h
> +++ b/html.h
> @@ -20,6 +20,7 @@ extern void html_attrf(const char *format,...);
>
> extern void html_txt(const char *txt);
> extern void html_ntxt(int len, const char *txt);
> +extern void html_ntxt_ellipsis(int len, const char *txt);
> extern void html_attr(const char *txt);
> extern void html_url_path(const char *txt);
> extern void html_url_arg(const char *txt);
> diff --git a/ui-repolist.c b/ui-repolist.c
> index 7272e87..77c33fd 100644
> --- a/ui-repolist.c
> +++ b/ui-repolist.c
> @@ -329,7 +329,7 @@ void cgit_print_repolist(void)
> repourl = cgit_repourl(ctx.repo->url);
> html_link_open(repourl, NULL, NULL);
> free(repourl);
> - html_ntxt(ctx.cfg.max_repodesc_len, ctx.repo->desc);
> + html_ntxt_ellipsis(ctx.cfg.max_repodesc_len, ctx.repo->desc);
> html_link_close();
> html("");
> if (ctx.cfg.enable_index_owner) {
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [RFCv2 PATCH 3/7] ui-blame: create links
On Fri, Sep 22, 2017 at 10:38:44PM -0500, Jeff Smith wrote:
> Create links to the blame page.
>
> Signed-off-by: Jeff Smith
> ---
> ui-shared.c | 20 +---
> ui-shared.h | 3 +++
> ui-tree.c | 10 +-
> 3 files changed, 29 insertions(+), 4 deletions(-)
>
> diff --git a/ui-shared.c b/ui-shared.c
> index e5c9a02..faa0d6a 100644
> --- a/ui-shared.c
> +++ b/ui-shared.c
> @@ -986,8 +996,12 @@ void cgit_print_pageheader(void)
> cgit_log_link("log", NULL, hc("log"), ctx.qry.head,
> NULL, ctx.qry.vpath, 0, NULL, NULL,
> ctx.qry.showmsg, ctx.qry.follow);
> - cgit_tree_link("tree", NULL, hc("tree"), ctx.qry.head,
> -ctx.qry.sha1, ctx.qry.vpath);
> + if (ctx.qry.page && !strcmp(ctx.qry.page, "blame"))
> + cgit_blame_link("blame", NULL, hc("blame"),
> ctx.qry.head,
> + ctx.qry.sha1, ctx.qry.vpath);
> + else
> + cgit_tree_link("tree", NULL, hc("tree"), ctx.qry.head,
> +ctx.qry.sha1, ctx.qry.vpath);
I'm curious what other people think of this link, I'm not sure it's
right but I can't think of a better behaviour. In some sense it feels
like blame is a subtype of tree so we should still show the tree link
here, but that makes the "is it active?" check more complicated, and we
can't add blame unconditionally because it has no meaning if the path is
a tree rather than a blob.
> cgit_commit_link("commit", NULL, hc("commit"),
>ctx.qry.head, ctx.qry.sha1, ctx.qry.vpath);
> cgit_diff_link("diff", NULL, hc("diff"), ctx.qry.head,
> diff --git a/ui-shared.h b/ui-shared.h
> index 87799f1..e5992bc 100644
> --- a/ui-shared.h
> +++ b/ui-shared.h
> @@ -26,6 +26,9 @@ extern void cgit_tree_link(const char *name, const char
> *title,
> extern void cgit_plain_link(const char *name, const char *title,
> const char *class, const char *head,
> const char *rev, const char *path);
> +extern void cgit_blame_link(const char *name, const char *title,
> + const char *class, const char *head,
> + const char *rev, const char *path);
> extern void cgit_log_link(const char *name, const char *title,
> const char *class, const char *head, const char *rev,
> const char *path, int ofs, const char *grep,
> diff --git a/ui-tree.c b/ui-tree.c
> index ca24a03..3513d5e 100644
> --- a/ui-tree.c
> +++ b/ui-tree.c
> @@ -1,6 +1,6 @@
> /* ui-tree.c: functions for tree output
> *
> - * Copyright (C) 2006-2014 cgit Development Team
> + * Copyright (C) 2006-2017 cgit Development Team
> *
> * Licensed under GNU General Public License v2
> * (see COPYING for full license text)
> @@ -141,6 +141,11 @@ static void print_object(const unsigned char *sha1, char
> *path, const char *base
> htmlf("blob: %s (", sha1_to_hex(sha1));
> cgit_plain_link("plain", NULL, NULL, ctx.qry.head,
> rev, path);
> + if (ctx.cfg.enable_blame) {
> + html(") (");
> + cgit_blame_link("blame", NULL, NULL, ctx.qry.head,
> + rev, path);
> + }
> html(")\n");
>
> if (ctx.cfg.max_blob_size && size / 1024 > ctx.cfg.max_blob_size) {
> @@ -275,6 +280,9 @@ static int ls_item(const unsigned char *sha1, struct
> strbuf *base,
> if (!S_ISGITLINK(mode))
> cgit_plain_link("plain", NULL, "button", ctx.qry.head,
> walk_tree_ctx->curr_rev, fullpath.buf);
> + if (!S_ISDIR(mode) && ctx.cfg.enable_blame)
> + cgit_blame_link("blame", NULL, "button", ctx.qry.head,
> + walk_tree_ctx->curr_rev, fullpath.buf);
> html("\n");
> free(name);
> strbuf_release(&fullpath);
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [RFCv2 PATCH 5/7] ui-blame: pull blame info from libgit
On Fri, Sep 22, 2017 at 10:38:46PM -0500, Jeff Smith wrote:
> Use the blame interface added in libgit to output the blame information
> of a file in the repository.
>
> Signed-off-by: Jeff Smith
> ---
> diff --git a/ui-blame.c b/ui-blame.c
> index 901ca89..cc4457a 100644
> --- a/ui-blame.c
> +++ b/ui-blame.c
> @@ -10,6 +10,183 @@
> #include "ui-blame.h"
> #include "html.h"
> #include "ui-shared.h"
> +#include "argv-array.h"
> +#include "blame.h"
> +
> +
> +/*
> + * Information on commits, used for output.
> + */
> +struct commit_info {
> + struct strbuf author;
> + struct strbuf author_mail;
> + struct ident_split author_ident;
> +
> + /* filled only when asked for details */
> + struct strbuf committer;
> + struct strbuf committer_mail;
> + struct ident_split committer_ident;
> +
> + struct strbuf summary;
> +};
I think I commented on this last time, but I still don't understand why
this adds a new commit_info structure rather than reusing our existing
commitinfo and cgit_parse_commit(), especially since...
> +static void get_commit_info(struct commit *commit,
> + struct commit_info *ret,
> + int detailed)
> +{
> + int len;
> + const char *subject, *encoding;
> + const char *message;
> +
> + commit_info_init(ret);
> +
> + encoding = get_log_output_encoding();
... this is wrong as our output encoding has nothing to do with the
environment but is specified by PAGE_ENCODING.
> + message = logmsg_reencode(commit, NULL, encoding);
> + get_ac_line(message, "\nauthor ",
> + &ret->author, &ret->author_mail,
> + &ret->author_ident);
> +
> + if (!detailed) {
> + unuse_commit_buffer(commit, message);
> + return;
> + }
> +
> + get_ac_line(message, "\ncommitter ",
> + &ret->committer, &ret->committer_mail,
> + &ret->committer_ident);
> +
> + len = find_commit_subject(message, &subject);
> + if (len)
> + strbuf_add(&ret->summary, subject, len);
> + else
> + strbuf_addf(&ret->summary, "(%s)",
> oid_to_hex(&commit->object.oid));
> +
> + unuse_commit_buffer(commit, message);
> +}
> +
> +static char *emit_one_suspect_detail(struct blame_origin *suspect, const
> char *hex)
> +{
> + struct commit_info ci;
> + struct strbuf detail = STRBUF_INIT;
> +
> + get_commit_info(suspect->commit, &ci, 1);
> +
> + strbuf_addf(&detail, "author %s", ci.author.buf);
> + if (!ctx.cfg.noplainemail)
> + strbuf_addf(&detail, " %s", ci.author_mail.buf);
> + strbuf_addf(&detail, " %s\n",
> + show_ident_date(&ci.author_ident,
> + cgit_date_mode(DATE_ISO8601)));
> +
> + strbuf_addf(&detail, "committer %s", ci.committer.buf);
> + if (!ctx.cfg.noplainemail)
> + strbuf_addf(&detail, " %s", ci.committer_mail.buf);
> + strbuf_addf(&detail, " %s\n\n",
> + show_ident_date(&ci.committer_ident,
> + cgit_date_mode(DATE_ISO8601)));
> +
> + strbuf_addbuf(&detail, &ci.summary);
> +
> + commit_info_destroy(&ci);
> + return strbuf_detach(&detail, NULL);
> +}
> +
> +static void emit_blame_entry(struct blame_scoreboard *sb, struct blame_entry
> *ent)
> +{
> + struct blame_origin *suspect = ent->suspect;
> + char hex[GIT_SHA1_HEXSZ + 1];
> + char *detail, *abbrev;
> + unsigned long lineno;
> + const char *numberfmt = "%1$d\n";
> + const char *cp, *cpend;
> +
> + oid_to_hex_r(hex, &suspect->commit->object.oid);
> + detail = emit_one_suspect_detail(suspect, hex);
> + abbrev = xstrdup(find_unique_abbrev(suspect->commit->object.oid.hash,
> + DEFAULT_ABBREV));
nit: I don't think there's any need to strdup for abbrev, we use the
result immediately so the static buffer won't get overwritten.
> + html("");
> + cgit_commit_link(abbrev, detail, NULL, ctx.qry.head, hex,
> suspect->path);
> + html("\n");
> +
> + free(detail);
> + free(abbrev);
> +
> + if (ctx.cfg.enable_tree_linenumbers) {
> + html("");
> + lineno = ent->lno;
> + while (lineno < ent->lno + ent->num_lines)
> + htmlf(numberfmt, ++lineno);
> + html("\n");
> + }
> +
> + cp = blame_nth_line(sb, ent->lno);
> + cpend = blame_nth_line(sb, ent->lno + ent->num_lines);
> +
> + html("");
> + html_ntxt(cpend - cp, cp);
> + html("\n");
> +}
>
> struct walk_tree_context {
> char *curr_rev;
> @@ -47,10 +224,16 @@ static void set_title_from_path(const char *path)
> strcat(new_title, ctx.page.title);
> ctx.page.title = new_title;
> }
> +
> static void print_object(const unsigned char *sha1, const char *path, const
> char *basename, const char *rev)
> {
> enum object_type type;
>
Re: [RFCv2 PATCH 0/7] Add ui-blame
On Fri, Sep 22, 2017 at 10:38:41PM -0500, Jeff Smith wrote: > I split git blame functionality into libgit, and the changes were > accepted upstream and are a part of git 2.14. Now that the git > infrastructure is in place, here is what is needed for cgit to make use > of it. > > Jeff Smith (7): > ui-blame: create enable-blame config item > ui-blame: create framework > ui-blame: create links > ui-blame: html_ntxt with no ellipsis > ui-blame: pull blame info from libgit > ui-blame: begin building > ui-blame: generate blame page when requested I still find the arrangement of this patch series a bit strange, I think it should be more like: - html: html_ntxt with no ellipsis - ui-tree: move set_title_from_path to ui-shared - ui-blame: add blame UI A squashed version of your "begin building" and "pull blame info from libgit"; splitting them up just makes it harder to review I think it also makes sense to squash the config, cmd and Makefile changes into this patch so that the feature springs into life fully formed, and then the following patches just link it into place - ui-tree: link to blame UI if enabled ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [RFCv2 PATCH 5/7] ui-blame: pull blame info from libgit
On Sun, Sep 24, 2017 at 03:09:53PM -0500, Jeffrey Smith wrote:
> On Sun, Sep 24, 2017 at 2:06 PM, Jeffrey Smith wrote:
> > A few of the original suggestions apparently got lost in the shuffle.
> > Anyway, it appears that making your suggested structure change
> > should clean some things up a lot.
> >
> > On Sat, Sep 23, 2017 at 10:47 AM, John Keeping wrote:
> >> On Fri, Sep 22, 2017 at 10:38:46PM -0500, Jeff Smith wrote:
> >>> Use the blame interface added in libgit to output the blame information
> >>> of a file in the repository.
> >>>
> >>> Signed-off-by: Jeff Smith
> >>> ---
> >>> diff --git a/ui-blame.c b/ui-blame.c
> >>> index 901ca89..cc4457a 100644
> >>> --- a/ui-blame.c
> >>> +++ b/ui-blame.c
> >>> +static void emit_blame_entry(struct blame_scoreboard *sb, struct
> >>> blame_entry *ent)
> >>> +{
> >>> + struct blame_origin *suspect = ent->suspect;
> >>> + char hex[GIT_SHA1_HEXSZ + 1];
> >>> + char *detail, *abbrev;
> >>> + unsigned long lineno;
> >>> + const char *numberfmt = "%1$d\n";
> >>> + const char *cp, *cpend;
> >>> +
> >>> + oid_to_hex_r(hex, &suspect->commit->object.oid);
> >>> + detail = emit_one_suspect_detail(suspect, hex);
> >>> + abbrev =
> >>> xstrdup(find_unique_abbrev(suspect->commit->object.oid.hash,
> >>> + DEFAULT_ABBREV));
> >>
> >> nit: I don't think there's any need to strdup for abbrev, we use the
> >> result immediately so the static buffer won't get overwritten.
>
> But find_unique_abbrev() returns a const char*, and abbrev is passed
> as the first parameter
> to cgit_commit_link(), which is char* (and the function can in fact
> change the contents).
>
> I will add a patch to the series that avoids altering the first
> parameter of cgit_commit_link
> so that it will be const char*.
The strdup approach is probably better than requiring cgit_commit_link()
to make a copy of its argument. I hadn't spotted that
find_unique_abbrev() returned a const char*.
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH 1/5] html: html_ntxt with no ellipsis
On Wed, Sep 27, 2017 at 05:43:27PM -0500, Jeff Smith wrote:
> For implementing a ui-blame page, there is need for a function that
> outputs a selection from a block of text, transformed for HTML output,
> but with no further modifications or additions.
>
> Signed-off-by: Jeff Smith
> ---
> html.c| 24
> html.h| 2 +-
> ui-repolist.c | 3 ++-
> 3 files changed, 7 insertions(+), 22 deletions(-)
>
> diff --git a/html.c b/html.c
> index e7e6e07..87ef5aa 100644
> --- a/html.c
> +++ b/html.c
> @@ -124,26 +124,11 @@ void html_vtxtf(const char *format, va_list ap)
[snip]
> -void html_ntxt(int len, const char *txt)
> +int html_ntxt(int len, const char *txt)
Should this be:
ssize_t html_ntxt(size_t len, const char *txt)
? Maybe with a check to die if len > SSIZE_MAX.
> {
> const char *t = txt;
> while (t && *t && len--) {
> @@ -162,8 +147,7 @@ void html_ntxt(int len, const char *txt)
> }
> if (t != txt)
> html_raw(txt, t - txt);
> - if (len < 0)
> - html("...");
> + return len;
> }
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH 2/5] ui-tree: move set_title_from_path to ui-shared
On Wed, Sep 27, 2017 at 05:43:28PM -0500, Jeff Smith wrote: > The ui-blame code will also need to call set_title_from_path, so go > ahead and move it to ui-shared. > > Signed-off-by: Jeff Smith > --- > ui-shared.c | 31 +++ > ui-shared.h | 2 ++ > ui-tree.c | 31 --- > 3 files changed, 33 insertions(+), 31 deletions(-) > > diff --git a/ui-shared.c b/ui-shared.c > index e5c9a02..4ace20c 100644 > --- a/ui-shared.c > +++ b/ui-shared.c > @@ -,3 +,34 @@ void cgit_print_snapshot_links(const char *repo, const > char *head, > } > strbuf_release(&filename); > } > + > +void set_title_from_path(const char *path) Should this be renamed to cgit_set_title_from_path now that it's exported? ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH 3/5] ui-shared: make a char* parameter const
On Wed, Sep 27, 2017 at 05:43:29PM -0500, Jeff Smith wrote: > All cgit_xxx_link functions take const char* for the 'name' parameter, > except for cgit_commit_link, which takes a char* and subsequently > modifies the contents. Avoiding the content changes, and making it > const char* will avoid the need to make copies of const char* strings > being passed to cgit_commit_link. > > Signed-off-by: Jeff Smith Reviewed-by: John Keeping > --- > ui-shared.c | 19 --- > ui-shared.h | 2 +- > 2 files changed, 9 insertions(+), 12 deletions(-) ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH 4/5] ui-blame: add blame UI
On Wed, Sep 27, 2017 at 05:43:30PM -0500, Jeff Smith wrote:
> Implement a page which provides the blame view of a specified file.
>
> This feature is controlled by a new config variable, "enable-blame",
> which is disabled by default.
>
> Signed-off-by: Jeff Smith
Reviewed-by: John Keeping
Linux's checkpatch.pl has a few minor complaints if you're re-rolling,
but I don't think these need to block merging:
WARNING: line over 80 characters
#174: FILE: ui-blame.c:17:
+static char *emit_one_suspect_detail(struct blame_origin *suspect, const char
*hex)
WARNING: line over 80 characters
#201: FILE: ui-blame.c:44:
+static void emit_blame_entry(struct blame_scoreboard *sb, struct blame_entry
*ent)
WARNING: line over 80 characters
#213: FILE: ui-blame.c:56:
+ abbrev = find_unique_abbrev(suspect->commit->object.oid.hash,
DEFAULT_ABBREV);
WARNING: line over 80 characters
#216: FILE: ui-blame.c:59:
+ cgit_commit_link(abbrev, detail, NULL, ctx.qry.head, hex,
suspect->path);
WARNING: line over 80 characters
#243: FILE: ui-blame.c:86:
+static void print_object(const unsigned char *sha1, const char *path, const
char *basename, const char *rev)
WARNING: line over 80 characters
#287: FILE: ui-blame.c:130:
+ htmlf("blob size (%ldKB) exceeds display
size limit (%dKB).",
WARNING: void function return statements are not generally useful
#304: FILE: ui-blame.c:147:
+ return;
+}
WARNING: line over 80 characters
#307: FILE: ui-blame.c:150:
+const char *pathname, unsigned mode, int stage, void
*cbdata)
WARNING: line over 80 characters
#316: FILE: ui-blame.c:159:
+ print_object(sha1, buffer.buf, pathname,
walk_tree_ctx->curr_rev);
WARNING: line over 80 characters
#322: FILE: ui-blame.c:165:
+ } else if (base->len < INT_MAX && (int)base->len >
walk_tree_ctx->match_baselen) {
WARNING: line over 80 characters
#374: FILE: ui-blame.c:217:
+ read_tree_recursive(commit->tree, "", 0, 0, &paths, walk_tree,
&walk_tree_ctx);
WARNING: line over 80 characters
#378: FILE: ui-blame.c:221:
+ cgit_print_error_page(404, "No blame for folders", "Blame is
not available for folders.");
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH 5/5] ui-tree: link to blame UI if enabled
On Wed, Sep 27, 2017 at 05:43:31PM -0500, Jeff Smith wrote:
> Create links to the blame page.
>
> Signed-off-by: Jeff Smith
Reviewed-by: John Keeping
> ---
> ui-shared.c | 20 +---
> ui-shared.h | 3 +++
> ui-tree.c | 10 +-
> 3 files changed, 29 insertions(+), 4 deletions(-)
>
> diff --git a/ui-shared.c b/ui-shared.c
> index ee96755..f75338a 100644
> --- a/ui-shared.c
> +++ b/ui-shared.c
> @@ -1,6 +1,6 @@
> /* ui-shared.c: common web output functions
> *
> - * Copyright (C) 2006-2014 cgit Development Team
> + * Copyright (C) 2006-2017 cgit Development Team
> *
> * Licensed under GNU General Public License v2
> * (see COPYING for full license text)
> @@ -304,6 +304,12 @@ void cgit_plain_link(const char *name, const char
> *title, const char *class,
> reporevlink("plain", name, title, class, head, rev, path);
> }
>
> +void cgit_blame_link(const char *name, const char *title, const char *class,
> + const char *head, const char *rev, const char *path)
> +{
> + reporevlink("blame", name, title, class, head, rev, path);
> +}
> +
> void cgit_log_link(const char *name, const char *title, const char *class,
> const char *head, const char *rev, const char *path,
> int ofs, const char *grep, const char *pattern, int showmsg,
> @@ -478,6 +484,10 @@ static void cgit_self_link(char *name, const char
> *title, const char *class)
> cgit_plain_link(name, title, class, ctx.qry.head,
> ctx.qry.has_sha1 ? ctx.qry.sha1 : NULL,
> ctx.qry.path);
> + else if (!strcmp(ctx.qry.page, "blame"))
> + cgit_blame_link(name, title, class, ctx.qry.head,
> + ctx.qry.has_sha1 ? ctx.qry.sha1 : NULL,
> + ctx.qry.path);
> else if (!strcmp(ctx.qry.page, "log"))
> cgit_log_link(name, title, class, ctx.qry.head,
> ctx.qry.has_sha1 ? ctx.qry.sha1 : NULL,
> @@ -983,8 +993,12 @@ void cgit_print_pageheader(void)
> cgit_log_link("log", NULL, hc("log"), ctx.qry.head,
> NULL, ctx.qry.vpath, 0, NULL, NULL,
> ctx.qry.showmsg, ctx.qry.follow);
> - cgit_tree_link("tree", NULL, hc("tree"), ctx.qry.head,
> -ctx.qry.sha1, ctx.qry.vpath);
> + if (ctx.qry.page && !strcmp(ctx.qry.page, "blame"))
> + cgit_blame_link("blame", NULL, hc("blame"),
> ctx.qry.head,
> + ctx.qry.sha1, ctx.qry.vpath);
> + else
> + cgit_tree_link("tree", NULL, hc("tree"), ctx.qry.head,
> +ctx.qry.sha1, ctx.qry.vpath);
> cgit_commit_link("commit", NULL, hc("commit"),
>ctx.qry.head, ctx.qry.sha1, ctx.qry.vpath);
> cgit_diff_link("diff", NULL, hc("diff"), ctx.qry.head,
> diff --git a/ui-shared.h b/ui-shared.h
> index 18e3994..cc9b4c6 100644
> --- a/ui-shared.h
> +++ b/ui-shared.h
> @@ -26,6 +26,9 @@ extern void cgit_tree_link(const char *name, const char
> *title,
> extern void cgit_plain_link(const char *name, const char *title,
> const char *class, const char *head,
> const char *rev, const char *path);
> +extern void cgit_blame_link(const char *name, const char *title,
> + const char *class, const char *head,
> + const char *rev, const char *path);
> extern void cgit_log_link(const char *name, const char *title,
> const char *class, const char *head, const char *rev,
> const char *path, int ofs, const char *grep,
> diff --git a/ui-tree.c b/ui-tree.c
> index 12eaaf0..27c9003 100644
> --- a/ui-tree.c
> +++ b/ui-tree.c
> @@ -1,6 +1,6 @@
> /* ui-tree.c: functions for tree output
> *
> - * Copyright (C) 2006-2014 cgit Development Team
> + * Copyright (C) 2006-2017 cgit Development Team
> *
> * Licensed under GNU General Public License v2
> * (see COPYING for full license text)
> @@ -110,6 +110,11 @@ static void print_object(const unsigned char *sha1, char
> *path, const char *base
> htmlf("blob: %s (", sha1_to_hex(sha1));
> cgit_plain_link("plain", NULL, NULL, ctx.qry.head,
> rev, path
Re: [PATCH 0/5] Add ui-blame
On Wed, Sep 27, 2017 at 05:43:26PM -0500, Jeff Smith wrote: > I split git blame functionality into libgit, and the changes were > accepted upstream and are a part of git 2.14. Now that the git > infrastructure is in place, here is what is needed for cgit to make use > of it. > > Since RFCv2: > . Re-ordered and squashed some of the patches > . Used cgit's commitinfo structure > . Factored set_title_from_path out to ui-shared > . Made cgit_commit_link's first parameter const char* > . Disabled the feature by default > . Simplified changes to html_ntxt / html_txt > > Jeff Smith (5): > html: html_ntxt with no ellipsis > ui-tree: move set_title_from_path to ui-shared > ui-shared: make a char* parameter const > ui-blame: add blame UI > ui-tree: link to blame UI if enabled This looks pretty good now. I've sent some comments on a couple of patches, but the only one I think is important is using (s)size_t for html_nxt in patch 1. ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH 0/5] Add ui-blame
On Tue, Oct 03, 2017 at 01:29:59AM +0200, Christian Hesse wrote: > Jeffrey Smith on Mon, 2017/10/02 17:35: > > From what I can see, it appears that it is in ch/for-jason. > > > > Also, I noticed a typo in my html_ntxt patch: > > ssize_t slen = (size_t) len; > >should be > > ssize_t slen = (ssize_t) len; > > > > Do I need to resend, or can someone fix that one up for me? > > Updated. Also added proper Reviewed-by lines. That also added decl-after-statement, so I've pulled the patches over to my jk/for-jason branch and fixed that, as well as a trailing blank line in ui-blame.h that git-am warned about. ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH] parsing: don't clear existing state with empty input
Since commit c699866 (parsing: clear query path before starting,
2017-02-19), we clear the "page" variable simply by calling
cgit_parse_url() even if the URL is empty. This breaks a URL like:
.../cgit?p=about
which is generated when using the "root-readme" configuration option.
This happens because "page" is set to "about" when parsing the query
string before we handle the path (which is empty, but non-null).
It turns out that this is not the only case which is broken, but
specifying repository and page via query options has been broken since
before the commit mentioned above, for example:
.../cgit?r=git&p=log
Fix both of these by allowing the previous state to persist if PATH_INFO
is empty, falling back to the query parameters if no path has been
requested.
Reported-by: Tom Ryder
Signed-off-by: John Keeping
---
This is still a bit of a mess, but I'm not really sure what we should be
doing when both PATH_INFO and query parameters are provided. At least
this change fixes the particular case Tom reported and I think it also
makes sure that any links we generate will be handled correctly.
parsing.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/parsing.c b/parsing.c
index b8d7f10..fd1ea99 100644
--- a/parsing.c
+++ b/parsing.c
@@ -20,11 +20,10 @@ void cgit_parse_url(const char *url)
char *c, *cmd, *p;
struct cgit_repo *repo;
- ctx.repo = NULL;
- ctx.qry.page = NULL;
if (!url || url[0] == '\0')
return;
+ ctx.qry.page = NULL;
ctx.repo = cgit_get_repoinfo(url);
if (ctx.repo) {
ctx.qry.repo = ctx.repo->url;
--
2.14.1.555.g1b9dbff880.dirty
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH 0/4] adding color to ui-blame
On Sat, Oct 14, 2017 at 04:17:46PM +0200, Jason A. Donenfeld wrote: > This patch set is currently broken, because in the exec filter, > processes like to buffer their output. The result is that the text winds > up at the bottom: > > https://git.zx2c4.com/cgit/blame/cache.c > > If anybody has some ideas on how we might enact a flush operation, please > pipe up (hah). The only thing I can think of is stdbuf, but that uses LD_PRELOAD internally and only works if the filter only uses the default stdio buffering and processing the input incrementally; our syntax-highlighting.py slurps the entire input stream before outputting anything. I wonder if it would be more reliable to split the output of the filter on the assumption that there is a one-to-one mapping from input lines to output lines. Both of the filters we ship do this, although the Python version dumps out a stylesheet before starting the document. Having thought about this a bit, I can't see any way to make this work reliably with our current filter protocol. Maybe we should consider introducing a more capable protocol for source filters that is designed to be usable in this scenario. ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: Gerrit features and Docker based testing
On Thu, Oct 19, 2017 at 05:07:30PM -0600, bbuh...@codeaurora.org wrote: > Hi, Cgit is frequently paired with Gerrit installations, so I have a > some Gerrit integration features I'd like to contribute. One is a Lua > authentication filter for Gerrit. Background is that Gerrit allows > different > users to have access to different repos, so in essence this auth filter > leverages the user's Gerrit login HTTP cookie to access the Gerrit REST > API, > which it uses to figure out whether the logged in user should have > access to > the particular repo. > > Anyway, to help ensure it keeps working along with the Lua code itself, > I'd > ideally like to contribute some tests for it. But the environment those > tests would need to run in would be a bit complicated, in particular > needing: > >(1) the auth filter's Lua package dependencies installed- > - specifically: crypto, ssl.https and ltn12 >and, >(2) a test Gerrit instance configured that the filter could talk to, > - or a simple python/node.js based webserver that looks > sufficiently >like the part of the Gerrit REST API that the filter cares > about > > An obvious way to define this environment would be a Dockerfile that the > "make test" command invokes Docker against. To enable people who don't > care > about the Gerrit auth-filter, the makefile could be setup to skip the > test > if Docker isn't installed on the build machine. But unless the cgit > maintainers were to trigger the tests in a build environment with Docker > available, the auth filter code would eventually get stale and break. > > Assuming the above sounds reasonable and I provided said feature and > tests, > would it be possible for the cgit maintainers to generally have Docker > available when regularly running "make test"? I wouldn't want the top-level "make test" to add additional dependencies, but I'm not sure that's necessary here. Assuming this lives under contrib/, it can have its own build and test infrastructure that is separate from the main CGit source. ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH 1/4] ui-blame: Distinguish hashes column from lines column
On Tue, Oct 17, 2017 at 11:17:32PM -0500, Jeff Smith wrote:
> Signed-off-by: Jeff Smith
Reviewed-by: John Keeping
> ---
> cgit.css | 1 +
> ui-blame.c | 2 +-
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/cgit.css b/cgit.css
> index 836f8ae..893ebeb 100644
> --- a/cgit.css
> +++ b/cgit.css
> @@ -300,6 +300,7 @@ div#cgit table.blob {
> border-top: solid 1px black;
> }
>
> +div#cgit table.blob td.hashes,
> div#cgit table.blob td.lines {
> margin: 0; padding: 0 0 0 0.5em;
> vertical-align: top;
> diff --git a/ui-blame.c b/ui-blame.c
> index 62cf431..a5ac590 100644
> --- a/ui-blame.c
> +++ b/ui-blame.c
> @@ -51,7 +51,7 @@ static void emit_blame_entry(struct blame_scoreboard *sb,
>
> char *detail = emit_suspect_detail(suspect);
>
> - html("");
> + html("");
> cgit_commit_link(find_unique_abbrev(oid->hash, DEFAULT_ABBREV), detail,
>NULL, ctx.qry.head, oid_to_hex(oid), suspect->path);
> html("\n");
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH 2/4] ui-blame: Break out emit_blame_entry into component methods
On Tue, Oct 17, 2017 at 11:17:33PM -0500, Jeff Smith wrote:
> Signed-off-by: Jeff Smith
Reviewed-by: John Keeping
> ---
> ui-blame.c | 44 ++--
> 1 file changed, 30 insertions(+), 14 deletions(-)
>
> diff --git a/ui-blame.c b/ui-blame.c
> index a5ac590..9b84147 100644
> --- a/ui-blame.c
> +++ b/ui-blame.c
> @@ -41,36 +41,52 @@ static char *emit_suspect_detail(struct blame_origin
> *suspect)
> return strbuf_detach(&detail, NULL);
> }
>
> -static void emit_blame_entry(struct blame_scoreboard *sb,
> - struct blame_entry *ent)
> +static void emit_blame_entry_hash(struct blame_entry *ent)
> {
> struct blame_origin *suspect = ent->suspect;
> struct object_id *oid = &suspect->commit->object.oid;
> +
> + char *detail = emit_suspect_detail(suspect);
> + cgit_commit_link(find_unique_abbrev(oid->hash, DEFAULT_ABBREV), detail,
> + NULL, ctx.qry.head, oid_to_hex(oid), suspect->path);
> + free(detail);
> +}
> +
> +static void emit_blame_entry_linenumber(struct blame_entry *ent)
> +{
> const char *numberfmt = "%1$d\n";
> +
> + unsigned long lineno = ent->lno;
> + while (lineno < ent->lno + ent->num_lines)
> + htmlf(numberfmt, ++lineno);
> +}
> +
> +static void emit_blame_entry_line(struct blame_scoreboard *sb,
> + struct blame_entry *ent)
> +{
> const char *cp, *cpend;
>
> - char *detail = emit_suspect_detail(suspect);
> + cp = blame_nth_line(sb, ent->lno);
> + cpend = blame_nth_line(sb, ent->lno + ent->num_lines);
> +
> + html_ntxt(cp, cpend - cp);
> +}
>
> +static void emit_blame_entry(struct blame_scoreboard *sb,
> + struct blame_entry *ent)
> +{
> html("");
> - cgit_commit_link(find_unique_abbrev(oid->hash, DEFAULT_ABBREV), detail,
> - NULL, ctx.qry.head, oid_to_hex(oid), suspect->path);
> + emit_blame_entry_hash(ent);
> html("\n");
>
> - free(detail);
> -
> if (ctx.cfg.enable_tree_linenumbers) {
> - unsigned long lineno = ent->lno;
> html("");
> - while (lineno < ent->lno + ent->num_lines)
> - htmlf(numberfmt, ++lineno);
> + emit_blame_entry_linenumber(ent);
> html("\n");
> }
>
> - cp = blame_nth_line(sb, ent->lno);
> - cpend = blame_nth_line(sb, ent->lno + ent->num_lines);
> -
> html("");
> - html_ntxt(cp, cpend - cp);
> + emit_blame_entry_line(sb, ent);
> html("\n");
> }
>
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH 3/4] ui-blame: Make each column into a single table cell
On Tue, Oct 17, 2017 at 11:17:34PM -0500, Jeff Smith wrote:
> Signed-off-by: Jeff Smith
Reviewed-by: John Keeping
> ---
> cgit.css | 19 +--
> ui-blame.c | 58 +-
> 2 files changed, 54 insertions(+), 23 deletions(-)
>
> diff --git a/cgit.css b/cgit.css
> index 893ebeb..20b7e86 100644
> --- a/cgit.css
> +++ b/cgit.css
> @@ -330,11 +330,26 @@ div#cgit table.ssdiff td.lineno a:hover {
> color: black;
> }
>
> -div#cgit table.blame tr:nth-child(even) {
> +div#cgit table.blame td.hashes,
> +div#cgit table.blame td.lines,
> +div#cgit table.blame td.linenumbers {
> + padding: 0;
> +}
> +
> +div#cgit table.blame td.hashes div.alt,
> +div#cgit table.blame td.lines div.alt {
> + padding: 0 0.5em 0 0.5em;
> +}
> +
> +div#cgit table.blame td.linenumbers div.alt {
> + padding: 0 0.5em 0 0;
> +}
> +
> +div#cgit table.blame div.alt:nth-child(even) {
> background: #eee;
> }
>
> -div#cgit table.blame tr:nth-child(odd) {
> +div#cgit table.blame div.alt:nth-child(odd) {
> background: white;
> }
>
> diff --git a/ui-blame.c b/ui-blame.c
> index 9b84147..f506616 100644
> --- a/ui-blame.c
> +++ b/ui-blame.c
> @@ -45,11 +45,17 @@ static void emit_blame_entry_hash(struct blame_entry *ent)
> {
> struct blame_origin *suspect = ent->suspect;
> struct object_id *oid = &suspect->commit->object.oid;
> + unsigned long line = 0;
>
> char *detail = emit_suspect_detail(suspect);
> + html("");
> cgit_commit_link(find_unique_abbrev(oid->hash, DEFAULT_ABBREV), detail,
>NULL, ctx.qry.head, oid_to_hex(oid), suspect->path);
> + html("");
> free(detail);
> +
> + while (line++ < ent->num_lines)
> + html("\n");
> }
>
> static void emit_blame_entry_linenumber(struct blame_entry *ent)
> @@ -72,24 +78,6 @@ static void emit_blame_entry_line(struct blame_scoreboard
> *sb,
> html_ntxt(cp, cpend - cp);
> }
>
> -static void emit_blame_entry(struct blame_scoreboard *sb,
> - struct blame_entry *ent)
> -{
> - html("");
> - emit_blame_entry_hash(ent);
> - html("\n");
> -
> - if (ctx.cfg.enable_tree_linenumbers) {
> - html("");
> - emit_blame_entry_linenumber(ent);
> - html("\n");
> - }
> -
> - html("");
> - emit_blame_entry_line(sb, ent);
> - html("\n");
> -}
> -
> struct walk_tree_context {
> char *curr_rev;
> int match_baselen;
> @@ -147,16 +135,44 @@ static void print_object(const unsigned char *sha1,
> const char *path,
> return;
> }
>
> - html("");
> + html("\n\n");
> +
> + /* Commit hashes */
> + html("");
> + for (ent = sb.ent; ent; ent = ent->next) {
> + html("");
> + emit_blame_entry_hash(ent);
> + html("");
> + }
> + html("\n");
> +
> + /* Line numbers */
> + if (ctx.cfg.enable_tree_linenumbers) {
> + html("");
> + for (ent = sb.ent; ent; ent = ent->next) {
> + html("");
> + emit_blame_entry_linenumber(ent);
> + html("");
> + }
> + html("\n");
> + }
> +
> + /* Lines */
> + html("");
> for (ent = sb.ent; ent; ) {
> struct blame_entry *e = ent->next;
> - emit_blame_entry(&sb, ent);
> + html("");
> + emit_blame_entry_line(&sb, ent);
> + html("");
> free(ent);
> ent = e;
> }
> - html("\n");
> + html("\n");
> +
> free((void *)sb.final_buf);
>
> + html("\n\n");
> +
> cgit_print_layout_end();
> }
>
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH 4/4] ui-blame: Allow syntax highlighting
On Tue, Oct 17, 2017 at 11:17:35PM -0500, Jeff Smith wrote:
> Place file contents into a single block so that syntax highlighting can
> be applied in the usual fashion. Place the alternating color bars
> behind the file contents. Force the default syntax highlighting
> background to transparent.
>
> Signed-off-by: Jeff Smith
Other than a couple of minor comments below, this looks reasonable to
me.
It does suffer the same drawback as the normal tree view with source
highlighting in that a little care is needed to avoid the line numbers
and content getting out of step (try adding "font-size: larger" to one
of the syntax highlighting CSS rules!), but since we already accept that
there I think we can accept it here as well.
> ---
> cgit.css | 10
> filters/syntax-highlighting.py | 2 +-
> ui-blame.c | 55
> +-
> 3 files changed, 54 insertions(+), 13 deletions(-)
>
> diff --git a/cgit.css b/cgit.css
> index 20b7e86..217a05a 100644
> --- a/cgit.css
> +++ b/cgit.css
> @@ -353,6 +353,16 @@ div#cgit table.blame div.alt:nth-child(odd) {
> background: white;
> }
>
> +div#cgit table.blame td.lines > div {
> + position: relative;
> +}
> +
> +div#cgit table.blame td.lines > div > pre {
> + padding: 0 0 0 0.5em;
> + position: absolute;
> + top: 0;
> +}
> +
> div#cgit table.bin-blob {
> margin-top: 0.5em;
> border: solid 1px black;
> diff --git a/filters/syntax-highlighting.py b/filters/syntax-highlighting.py
> index 5888b50..e912594 100755
> --- a/filters/syntax-highlighting.py
> +++ b/filters/syntax-highlighting.py
> @@ -34,7 +34,7 @@ sys.stdin = io.TextIOWrapper(sys.stdin.buffer,
> encoding='utf-8', errors='replace
> sys.stdout = io.TextIOWrapper(sys.stdout.buffer, encoding='utf-8',
> errors='replace')
> data = sys.stdin.read()
> filename = sys.argv[1]
> -formatter = HtmlFormatter(style='pastie')
> +formatter = HtmlFormatter(style='pastie', nobackground=True)
>
> try:
> lexer = guess_lexer_for_filename(filename, data)
> diff --git a/ui-blame.c b/ui-blame.c
> index f506616..574c3ee 100644
> --- a/ui-blame.c
> +++ b/ui-blame.c
> @@ -67,15 +67,21 @@ static void emit_blame_entry_linenumber(struct
> blame_entry *ent)
> htmlf(numberfmt, ++lineno);
> }
>
> -static void emit_blame_entry_line(struct blame_scoreboard *sb,
> - struct blame_entry *ent)
> +static void emit_blame_entry_line_background(struct blame_scoreboard *sb,
> + struct blame_entry *ent)
> {
> - const char *cp, *cpend;
> + unsigned long line;
> + size_t len, maxlen = 2;
>
> - cp = blame_nth_line(sb, ent->lno);
> - cpend = blame_nth_line(sb, ent->lno + ent->num_lines);
> + for (line = ent->lno; line < ent->lno + ent->num_lines; line++) {
> + html("\n");
> + len = blame_nth_line(sb, line + 1) - blame_nth_line(sb, line);
This doesn't account for tabs, which is noticable in long lines (I
happened to test with cgit.c where line 615 is quite a bit longer than
average).
It is fixed by using:
const char *start = blame_nth_line(sb, line);
const char *end = blame_nth_line(sb, line + 1);
html("\n");
len = end - start;
while (start < end)
if (*(start++) == '\t')
len += 7;
> + if (len > maxlen)
> + maxlen = len;
> + }
>
> - html_ntxt(cp, cpend - cp);
> + for (len = 0; len < maxlen - 1; len++)
> + html(" ");
Should this use or is a plain space guaranteed to be okay here?
> }
>
> struct walk_tree_context {
> @@ -88,6 +94,7 @@ static void print_object(const unsigned char *sha1, const
> char *path,
>const char *basename, const char *rev)
> {
> enum object_type type;
> + char *buf;
> unsigned long size;
> struct argv_array rev_argv = ARGV_ARRAY_INIT;
> struct rev_info revs;
> @@ -102,6 +109,13 @@ static void print_object(const unsigned char *sha1,
> const char *path,
> return;
> }
>
> + buf = read_sha1_file(sha1, &type, &size);
> + if (!buf) {
> + cgit_print_error_page(500, "Internal server error",
> + "Error reading object %s", sha1_to_hex(sha1));
> + return;
> + }
> +
> argv_array_push(&rev_argv, "blame");
> argv_array_push(&rev_argv, rev);
> init_revisions(&revs, NULL);
> @@ -157,20 +171,37 @@ static void print_object(const unsigned char *sha1,
> const char *path,
> html("\n");
> }
>
> - /* Lines */
> - html("");
> + html("");
> +
> + /* Colored bars behind lines */
> + html("");
> for (ent = sb.ent; ent; ) {
> struct blame_entry *e = ent->next;
> - html("");
> - emit_blame_entry_line(&sb, ent);
> -
Re: [PATCH 0/3] Add support for git's mailmap.
On Tue, Oct 24, 2017 at 10:55:21AM -0400, Jason A. Smith wrote: > Seeing a lot of recent activity I checked the upstream and see that the > patch I first submitted over a year ago has still not been merged. I > already had to rebase it once several months ago to fix conflicts, I > don't want to have to do that again. Is there something wrong with this > patch? Please let me know so I can fix it. I've taken another look at this patch and one thing that stands out is that we end up inserting code to map users in a lot of different places. I haven't audited the code, but after the final patch, is there anywhere that calls cgit_parse_commit() and doesn't go on to map the users? (Even if not all callers do map the user, can we push cgit_map_user() into cgit_parse_commit() and use a flag parameter to enable it?) But I think that points to a deeper philosophical question of whether we should be mapping the user in all cases (and I suspect this is why the patch has sat on the list for so long - there are arguments in both directions). As I understand it, mailmap was originally introduced so that git-shortlog would coalesce commits from the same author but with different email addresses. When using the git command, mailmap is enabled by default for git-shortlog and git-blame but not for git-log, git-show, and other commands that print commits. Extending this to CGit, it seems that there's a clear argument for enabling mailmap in ui-stats and possibly the new ui-blame, but when we print a commit in ui-commit or ui-log is it always correct to apply the mailmap or should we show the commit headers as-is? It would be interesting to know what others on the list think about this. If there isn't a consensus then we may want a new config option to allow this to be enabled according to the preferences of individual sites. ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH 0/3] Add support for git's mailmap.
On Thu, Nov 02, 2017 at 04:48:08PM -0400, Jason A. Smith wrote: > I always thought that by going through the trouble of collecting the > duplicate email addresses and creating the .mailmap file that you wanted > this coalescing behavior. > > Personally, I don't really want to have to remember everyone's old email > address and variation of their name that they might have used several > years ago to push a commit. I don't really care what email & name they > used a long time ago, I am more interested in just knowing who made the > commit, which is why the .mailmap coalesces all of their old identities > into their current one for display. > > If people don't need or want this coalescing behavior and want to see > each individual identity then they don't need to make the .mailmap file > to begin with. Consider someone working on an open source project who changes employer; in this case .mailmap allows email to be directed to an address at the new employer rather than at the previous employer, especially for auto-generated CC lists from something like Linux's get_maintainer.pl script. However, when looking at historic commits, it may be very interesting that a particular change was committed by someone with an email address at a cerain company. Blindly applying .mailmap rules loses that information (admittedly, in the case of the kernel, DCO rules mean the relevant information also lives in the S-o-b but that doesn't apply everywhere). On the other hand, it might be that some early commits by a developer in a project use the default "user@(none)" fallback address and .mailmap is used to change that to a genuine email address. In that case applying .mailmap in all cases is desirable. I don't think we can take a global decision on which behaviour is correct for all projects, so IMHO there must be a configuration switch to allow projects to decide what is right for them. > On 10/28/2017 07:36 AM, John Keeping wrote: > > On Tue, Oct 24, 2017 at 10:55:21AM -0400, Jason A. Smith wrote: > >> Seeing a lot of recent activity I checked the upstream and see that the > >> patch I first submitted over a year ago has still not been merged. I > >> already had to rebase it once several months ago to fix conflicts, I > >> don't want to have to do that again. Is there something wrong with this > >> patch? Please let me know so I can fix it. > > > > I've taken another look at this patch and one thing that stands out is > > that we end up inserting code to map users in a lot of different places. > > I haven't audited the code, but after the final patch, is there anywhere > > that calls cgit_parse_commit() and doesn't go on to map the users? > > (Even if not all callers do map the user, can we push cgit_map_user() > > into cgit_parse_commit() and use a flag parameter to enable it?) > > > > But I think that points to a deeper philosophical question of whether we > > should be mapping the user in all cases (and I suspect this is why the > > patch has sat on the list for so long - there are arguments in both > > directions). > > > > As I understand it, mailmap was originally introduced so that > > git-shortlog would coalesce commits from the same author but with > > different email addresses. When using the git command, mailmap is > > enabled by default for git-shortlog and git-blame but not for git-log, > > git-show, and other commands that print commits. > > > > Extending this to CGit, it seems that there's a clear argument for > > enabling mailmap in ui-stats and possibly the new ui-blame, but when we > > print a commit in ui-commit or ui-log is it always correct to apply the > > mailmap or should we show the commit headers as-is? > > > > It would be interesting to know what others on the list think about > > this. If there isn't a consensus then we may want a new config option > > to allow this to be enabled according to the preferences of individual > > sites. ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH 4/4 v3] ui-blame: Allow syntax highlighting
On Sun, Oct 29, 2017 at 11:23:38PM +0100, Jason A. Donenfeld wrote: > Works remarkably well. Excellent work. Example, for others on the list: > > https://git.zx2c4.com/WireGuard/blame/src/noise.c > > Pending objections from others, I'll merge this from jd/color-blame to > master in a few days. This version looks great to me, so I say merge away! ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH 1/1] git: update to v2.15.1
On Wed, Nov 29, 2017 at 10:26:44PM +0100, Christian Hesse wrote: > From: Christian Hesse > > Update to git version v2.15.1: With commit 0abe14f6 prepare_packed_git() > moved to packfile.[ch]. > > Signed-off-by: Christian Hesse Reviewed-by: John Keeping > --- > Makefile | 2 +- > git| 2 +- > ui-clone.c | 1 + > 3 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/Makefile b/Makefile > index f3ee84c..274f37e 100644 > --- a/Makefile > +++ b/Makefile > @@ -14,7 +14,7 @@ htmldir = $(docdir) > pdfdir = $(docdir) > mandir = $(prefix)/share/man > SHA1_HEADER = > -GIT_VER = 2.14.0 > +GIT_VER = 2.15.1 > 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 4384e3c..9b185be 16 > --- a/git > +++ b/git > @@ -1 +1 @@ > -Subproject commit 4384e3cde2ce8ecd194202e171ae16333d241326 > +Subproject commit 9b185bef0c15cec1ea8753ce091e42ea041f2c31 > diff --git a/ui-clone.c b/ui-clone.c > index 0d11672..bc98980 100644 > --- a/ui-clone.c > +++ b/ui-clone.c > @@ -11,6 +11,7 @@ > #include "ui-clone.h" > #include "html.h" > #include "ui-shared.h" > +#include "packfile.h" > > static int print_ref_info(const char *refname, const struct object_id *oid, >int flags, void *cb_data) ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: Commit _subject_ filter, everywhere
On Wed, Nov 29, 2017 at 10:10:00PM +0100, Bjjjrn Forsman wrote:
> I got this idea to generate a link to the CI server job in front of
> each commit message subject. Think small green/red boxes that are
> clickable. I figure that getting the build status directly in cgit is
> much more efficient than navigating the CI server itself, which
> typically requires many clicks to find the build that comes out of a
> given commit.
>
> I had high hopes for the the commit-filter= setting, but found that
>
> * commit-filter= is only run when showing a full commit, not other
> places. I.e. it is applied when showing the "commit" tab, but not on
> "summary", "logs" or "refs".
> * commit-filter= is run twice on a commit message. Once for the
> subject and once for the body. I expected only once. (Is this a bug?)
>
> Any thoughts on if/how this use case can be supported? A separate
> filter? Or simply run commit-filter= everywhere (breaking change)?
I was going to suggest introducing a new subject-filter which is used to
filter the commit subject wherever it is displayed, but there is a
subtlety which makes this more complicated in the summary, log and refs
pages than it is in the commit page.
When we are showing a single-line summary of a commit, the subject is a
hyperlink so adding new content outside the existing subject is not
possible with the obvious filter implementation.
One option would be to push the tag generation down into the
filter, but I'd rather avoid that if possible.
What do you think about adding a "filter" which is called before the
commit subject is printed and allows outputting additional HTML content?
("filter" is in quotes since there will not be any content copied
through the program.)
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: Commit _subject_ filter, everywhere
On Sat, Dec 02, 2017 at 01:33:28PM +0100, Bjjjrn Forsman wrote:
> On 2 December 2017 at 12:31, John Keeping wrote:
> > I was going to suggest introducing a new subject-filter which is used to
> > filter the commit subject wherever it is displayed, but there is a
> > subtlety which makes this more complicated in the summary, log and refs
> > pages than it is in the commit page.
> >
> > When we are showing a single-line summary of a commit, the subject is a
> > hyperlink so adding new content outside the existing subject is not
> > possible with the obvious filter implementation.
> >
> > One option would be to push the tag generation down into the
> > filter, but I'd rather avoid that if possible.
>
> Aha, I see.
After I wrote that I realised that for the existing commit-filter we
escape the content before writing to the filter. So it might be
reasonable to pass the whole tag through the filter.
That would make it easy for the filter to put extra content either
before or after the commit subject. It also makes the filter
integration much simpler, for example in ui-log.c:
-- >8 --
diff --git a/ui-log.c b/ui-log.c
index 2d2bb31..f2a7852 100644
--- a/ui-log.c
+++ b/ui-log.c
@@ -234,8 +234,11 @@ static void print_commit(struct commit *commit, struct
rev_info *revs)
strcpy(info->subject + i, wrap_symbol);
}
}
+ cgit_open_filter(ctx.repo->subject_filter,
+oid_to_hex(&commit->object.oid));
cgit_commit_link(info->subject, NULL, NULL, ctx.qry.head,
oid_to_hex(&commit->object.oid), ctx.qry.vpath);
+ cgit_close_filter(ctx.repo->subject_filter);
show_commit_decorations(commit);
html("");
cgit_open_filter(ctx.repo->email_filter, info->author_email, "log");
-- 8< --
> > What do you think about adding a "filter" which is called before the
> > commit subject is printed and allows outputting additional HTML content?
> > ("filter" is in quotes since there will not be any content copied
> > through the program.)
>
> Sounds good to me. It covers my use case.
>
> Perhaps this new "filter" should not have filter in its name.
> Suggestion: commit-prefix=.
>
> Oh, and this new command must somehow receive the commit id from cgit.
> New environment variable?
We normally use arguments for this, which is all handled via the
cgit_open_filter() function.
I expect Lua filters may be preferred for this use case to avoid forking
50 child processes when displaying the log view.
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: Commit _subject_ filter, everywhere
On Sat, Dec 02, 2017 at 03:44:15PM +0100, Bjjjrn Forsman wrote:
> On 2 December 2017 at 13:49, John Keeping wrote:
> > After I wrote that I realised that for the existing commit-filter we
> > escape the content before writing to the filter. So it might be
> > reasonable to pass the whole tag through the filter.
> >
> > That would make it easy for the filter to put extra content either
> > before or after the commit subject. It also makes the filter
> > integration much simpler, for example in ui-log.c:
> >
> > -- >8 --
> > diff --git a/ui-log.c b/ui-log.c
> > index 2d2bb31..f2a7852 100644
> > --- a/ui-log.c
> > +++ b/ui-log.c
> > @@ -234,8 +234,11 @@ static void print_commit(struct commit *commit, struct
> > rev_info *revs)
> > strcpy(info->subject + i, wrap_symbol);
> > }
> > }
> > + cgit_open_filter(ctx.repo->subject_filter,
> > +oid_to_hex(&commit->object.oid));
> > cgit_commit_link(info->subject, NULL, NULL, ctx.qry.head,
> > oid_to_hex(&commit->object.oid), ctx.qry.vpath);
> > + cgit_close_filter(ctx.repo->subject_filter);
> > show_commit_decorations(commit);
> > html("");
> > cgit_open_filter(ctx.repo->email_filter, info->author_email, "log");
> > -- 8< --
>
> I applied that patch, with s/subject_filter/commit_filter/, and
> updated my commit-filter to output a small SVG (borrowed from
> https://shields.io/) in front of the commit message.
>
> Here is the result:
>
> https://bforsman.name/cgit/nixos-config/log/
>
> I think it's looking good!
>
> On small annoyance though is that the CI status element, when loaded,
> causes the commit messages to move sideways. Ideally, the CI status
> (or whatever the user chose to attach to the commit) would not cause
> the layout to move (unless it's very big). I guess that's a
> conflicting goal between having a "commit prefix" and a dedicated
> column for this extra info. Another alternative is _appending_ the
> element, which a filter program has the ability to do. Then the layout
> would be stable, but the CI status elements would not align nicely in
> a column anymore. Hmm... :-)
I wonder if it would look better with a smaller symbol, like the
Libravatar icons for the author names here:
https://git.zx2c4.com/cgit/log/
Maybe something like the small tick/cross icons GitHub and GitLab use
for build status, for example:
https://github.com/rust-lang/rust/branches/all
https://gitlab.com/gitlab-org/gitlab-ce/commits/master
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH] extra-head-content: introduce another option for meta tags
On Mon, Feb 12, 2018 at 11:34:53PM +0100, Jason A. Donenfeld wrote:
> This is to support things like go-import meta tags, which are on a
> per-repo basis.
>
> Signed-off-by: Jason A. Donenfeld
> ---
> This is kind of really ugly, and I'm not keen on its approach, but I
> thought I'd post it to get some feedback on the general "requirement".
> It turns out, the ugly Go ecosystem requires meta tags like these to
> function:
>
> https://git.zx2c4.com/terrible-go-package"; />
>
> The question is: is cgit a good place for doing this? Is this here,
> below, actually a good way of going about it, if so?
It looks like it's generally expected that imports can use the path to
the Git repo, so I think it is reasonable for CGit to add these tags.
However, I wonder if extra-head-content is too generic. Should we just
accept that "support meta go-import" is a feature and have a cgitrc
option for "go-import-url" a bit like the existing clone-url option?
> cgit.c | 4
> cgit.h | 1 +
> cgitrc.5.txt | 4
> shared.c | 1 +
> ui-shared.c | 2 ++
> 5 files changed, 12 insertions(+)
>
> diff --git a/cgit.c b/cgit.c
> index bd9cb3f..cf65bbd 100644
> --- a/cgit.c
> +++ b/cgit.c
> @@ -46,6 +46,8 @@ static void repo_config(struct cgit_repo *repo, const char
> *name, const char *va
> repo->homepage = xstrdup(value);
> else if (!strcmp(name, "defbranch"))
> repo->defbranch = xstrdup(value);
> + else if (!strcmp(name, "extra-head-content"))
> + repo->extra_head_content = xstrdup(value);
> else if (!strcmp(name, "snapshots"))
> repo->snapshots = ctx.cfg.snapshots &
> cgit_parse_snapshots_mask(value);
> else if (!strcmp(name, "enable-commit-graph"))
> @@ -802,6 +804,8 @@ static void print_repo(FILE *f, struct cgit_repo *repo)
> }
> if (repo->defbranch)
> fprintf(f, "repo.defbranch=%s\n", repo->defbranch);
> + if (repo->extra_head_content)
> + fprintf(f, "repo.extra-head-content=%s\n",
> repo->extra_head_content);
> if (repo->module_link)
> fprintf(f, "repo.module-link=%s\n", repo->module_link);
> if (repo->section)
> diff --git a/cgit.h b/cgit.h
> index 005ae63..edec271 100644
> --- a/cgit.h
> +++ b/cgit.h
> @@ -79,6 +79,7 @@ struct cgit_repo {
> char *name;
> char *path;
> char *desc;
> + char *extra_head_content;
> char *owner;
> char *homepage;
> char *defbranch;
> diff --git a/cgitrc.5.txt b/cgitrc.5.txt
> index 4da166c..fa2fbfc 100644
> --- a/cgitrc.5.txt
> +++ b/cgitrc.5.txt
> @@ -501,6 +501,10 @@ repo.defbranch::
> repo.desc::
> The value to show as repository description. Default value: none.
>
> +repo.extra-head-content::
> + This value will be added verbatim to the head section of each page
> + displayed for this repo. Default value: none.
> +
> repo.homepage::
> The value to show as repository homepage. Default value: none.
>
> diff --git a/shared.c b/shared.c
> index 21ac8f4..d0405cf 100644
> --- a/shared.c
> +++ b/shared.c
> @@ -53,6 +53,7 @@ struct cgit_repo *cgit_add_repo(const char *url)
> ret->name = ret->url;
> ret->path = NULL;
> ret->desc = cgit_default_repo_desc;
> + ret->extra_head_content = NULL;
> ret->owner = NULL;
> ret->homepage = NULL;
> ret->section = ctx.cfg.section;
> diff --git a/ui-shared.c b/ui-shared.c
> index 9d8f66b..5197cd6 100644
> --- a/ui-shared.c
> +++ b/ui-shared.c
> @@ -766,6 +766,8 @@ void cgit_print_docstart(void)
> cgit_add_clone_urls(print_rel_vcs_link);
> if (ctx.cfg.head_include)
> html_include(ctx.cfg.head_include);
> + if (ctx.repo && ctx.repo->extra_head_content)
> + html(ctx.repo->extra_head_content);
> html("\n");
> html("\n");
> if (ctx.cfg.header)
> --
> 2.16.1
>
> ___
> CGit mailing list
> CGit@lists.zx2c4.com
> https://lists.zx2c4.com/mailman/listinfo/cgit
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: NGINX and linux
On Wed, Feb 28, 2018 at 08:37:17PM -0500, Rolan Pichler wrote: > The README is for APACHE and unix systems. Would there be a way to get this > working with nginx and on linux Nginx doesn't support CGI directly, so you need to use fcgiwrap or an equivalent as a wrapper to run CGit. I found this post about setting up CGit and nginx which looks like it covers the necessary configuration: https://levlaz.org/installing-cgit-nginx-on-debian-jessie/ Patches to README are welcome if you get it working. ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: Default view
On Mon, Mar 19, 2018 at 10:01:58PM +0300, Alex Ivanov wrote: > How to set a default view? When I open repo link a summary view is > opened by default, but I want to have tree view as default. You'll have to patch CGit if you want to do that, out of the box you can set enable-index-links=1 in your cgitrc to add a tree link in the repo list. I don't think you can easily make the plain repo url display the tree because there wouldn't be any way to get to the summary view without further changes, but it should be possible to swap the cgit_summary_link() call in ui-repolist.c to cgit_tree_link() so that the primary link in the repo list points at the tree view. ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: RFC: snapshot tarball information in refs/notes/snapshots
On Wed, Mar 21, 2018 at 10:03:11AM -0400, Konstantin Ryabitsev wrote:
> It would be easy to load and parse refs/notes/cgitrc, and the security
> implications shouldn't be much different than loading the same from
> gitconfig.
Based on this comment, I had a go at just wiring up config parsing to
the existing parsing for gitconfig files; I don't think it's ideal
because that's in gitconfig format not cgitrc format but it's an easy
proof of concept.
Unfortunately there is one big gotcha I encountered doing this, which is
that we don't have the repository set up when we are scanning this
configuration, because this is done when building the repository list
not just for loading repository-specific pages. Since one of the
configuration options is the repository description, I think this is
unavoidable.
The ongoing work in git.git to support submodules in-process may help us
load multiple repositories into the same process while we're building
the repo list, but it looks like the changes we need haven't hit master
yet (although there are some changes in pu that look like they might be
exactly what we need, so maybe Git 2.18 will let us make this work).
Here's the patch I have for wiring up "read from ref", the idea is to
point to a blob containing the config, so it could be any of the
following:
refs/heads/master:cgitrc# stored in master branch
refs/heads/config:cgitrc# stored in a config branch
refs/blobs/cgitrc # ref points directly at a blob
If you try it you'll find it promptly BUGs because the repo isn't set
up, but I'm interested in opinions on the configuration aspect of this.
-- >8 --
cgit.c | 2 ++
cgit.h | 1 +
scan-tree.c | 8
3 files changed, 11 insertions(+)
diff --git a/cgit.c b/cgit.c
index bd9cb3f..4170533 100644
--- a/cgit.c
+++ b/cgit.c
@@ -207,6 +207,8 @@ static void config_cb(const char *name, const char *value)
ctx.cfg.cache_snapshot_ttl = atoi(value);
else if (!strcmp(name, "case-sensitive-sort"))
ctx.cfg.case_sensitive_sort = atoi(value);
+ else if (!strcmp(name, "config-ref"))
+ ctx.cfg.config_ref = xstrdup(value);
else if (!strcmp(name, "about-filter"))
ctx.cfg.about_filter = cgit_new_filter(value, ABOUT);
else if (!strcmp(name, "commit-filter"))
diff --git a/cgit.h b/cgit.h
index 005ae63..fa2c35e 100644
--- a/cgit.h
+++ b/cgit.h
@@ -189,6 +189,7 @@ struct cgit_config {
char *cache_root;
char *clone_prefix;
char *clone_url;
+ char *config_ref;
char *css;
char *favicon;
char *footer;
diff --git a/scan-tree.c b/scan-tree.c
index 6a2f65a..b616005 100644
--- a/scan-tree.c
+++ b/scan-tree.c
@@ -127,6 +127,14 @@ static void add_repo(const char *base, struct strbuf
*path, repo_config_fn fn)
git_config_from_file(gitconfig_config, path->buf, NULL);
strbuf_setlen(path, pathlen);
}
+ if (ctx.cfg.config_ref) {
+ struct object_id oid;
+
+ if (!get_oid(ctx.cfg.config_ref, &oid))
+ git_config_from_blob_oid(gitconfig_config,
+ctx.cfg.config_ref, &oid,
+NULL);
+ }
if (ctx.cfg.remove_suffix) {
size_t urllen;
--
2.14.1
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: About or readme formatting for subfolders
On Tue, Mar 20, 2018 at 07:40:33PM +0300, Alex Ivanov wrote: > Currently about tab is only for readme file in root folder. Is it > possible to show it for subfolders with readme files also? Or other > way show say readme.mkd (markdown) preformatted in file preview > (unless one clicks plain) You can load pretty much anything via the about view if it's configured to point at the correct branch, just append the relevant path to the /about/ URL. I also had some patches a while ago [1] to add a render mode to CGit's tree view which is probably closer to what you want, but I don't think there was ever any feedback and I didn't pursue it. There's a version rebased onto an up-to-date master on my GitHub [2]. [1] https://lists.zx2c4.com/pipermail/cgit/2016-September/003289.html [2] https://github.com/johnkeeping/cgit/tree/render-mode ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: RFC: snapshot tarball information in refs/notes/snapshots
On Fri, Mar 30, 2018 at 11:38:38AM -0400, Konstantin Ryabitsev wrote: > On Fri, Mar 30, 2018 at 12:53:57PM +0100, John Keeping wrote: > > Unfortunately there is one big gotcha I encountered doing this, which is > > that we don't have the repository set up when we are scanning this > > configuration, because this is done when building the repository list > > not just for loading repository-specific pages. Since one of the > > configuration options is the repository description, I think this is > > unavoidable. > > One other way I'd thought about doing this is via a post-update hook > that would read the configuration from an object in the repo into a file > cgit could read, but I didn't want to write out into .git/config. > > That might be one way of achieving compromise -- support per-repository > configuration that users themselves can push, but make it up to the > server administrator to set up hooks so that the config gets written out > into .git/cgitrc (or repo.git/cgitrc) for cgit to consume. In my mind, > it was a note attached to the initial commit of the master branch, but > it can be any object that post-update can access and write out. > > This way cgit doesn't need to rely on git to read this data, as it's a > regular config file inside the git dir. The post-update hook can also do > any sanitization of config parameters it deems necessary. > > Does that make sense? Yes, repo.git/cgitrc is already read unconditionally when using scan-path to find repositories (under the same conditions as .git/config). In a few months, I think it will be possible to pull configuration directly from objects in the Git repository, but if you want a solution today then a post-update hook seems like the best way to do it. As you say, this also has the advantage that the adminstrator can apply additional policy to the variables set in the repository. I don't think notes are the right solution because they tie information to a particular commit and it's difficult to consistently pick a commit from which to pull the configuration: anything other than the tip of a branch will incur the cost of walking to find an annotated commit but keeping the note at the tip of the branch is difficult and likely to result in the configuration being lost. A special ref for configuration is reasonably easy to maintain and if it's a branch then you can get history attached config file changes for free. ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH 2/2] Add "snapshot-prefix" repo configuration
Allow using a user-specified value for the prefix in snapshot files
instead of the repository basename. For example, files downloaded from
the linux-stable.git repository should be named linux-$VERSION and not
linux-stable-$VERSION, which can be achieved by setting:
repo.snapshot-prefix=linux
Signed-off-by: John Keeping
---
cgit.c | 2 ++
cgit.h | 1 +
cgitrc.5.txt | 7 +++
ui-refs.c| 2 +-
ui-shared.c | 10 +-
ui-shared.h | 1 +
6 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/cgit.c b/cgit.c
index bd9cb3f..d2f7b9c 100644
--- a/cgit.c
+++ b/cgit.c
@@ -79,6 +79,8 @@ static void repo_config(struct cgit_repo *repo, const char
*name, const char *va
item->util = xstrdup(value);
} else if (!strcmp(name, "section"))
repo->section = xstrdup(value);
+ else if (!strcmp(name, "snapshot-prefix"))
+ repo->snapshot_prefix = xstrdup(value);
else if (!strcmp(name, "readme") && value != NULL) {
if (repo->readme.items == ctx.cfg.readme.items)
memset(&repo->readme, 0, sizeof(repo->readme));
diff --git a/cgit.h b/cgit.h
index 005ae63..847cd2e 100644
--- a/cgit.h
+++ b/cgit.h
@@ -88,6 +88,7 @@ struct cgit_repo {
char *clone_url;
char *logo;
char *logo_link;
+ char *snapshot_prefix;
int snapshots;
int enable_commit_graph;
int enable_log_filecount;
diff --git a/cgitrc.5.txt b/cgitrc.5.txt
index 4da166c..a9d3d0a 100644
--- a/cgitrc.5.txt
+++ b/cgitrc.5.txt
@@ -599,6 +599,13 @@ repo.snapshots::
restricted by the global "snapshots" setting. Default value:
.
+repo.snapshot-prefix::
+ Prefix to use for snapshot links instead of the repository basename.
+ For example, the "linux-stable" repository may wish to set this to
+ "linux" so that snapshots are in the format "linux-3.15.4" instead
+ of "linux-stable-3.15.4". Default value: meaning to use
+ the repository basename.
+
repo.section::
Override the current section name for this repository. Default value:
none.
diff --git a/ui-refs.c b/ui-refs.c
index 75f2789..50d9d30 100644
--- a/ui-refs.c
+++ b/ui-refs.c
@@ -100,7 +100,7 @@ static void print_tag_downloads(const struct cgit_repo
*repo, const char *ref)
if (!ref || strlen(ref) < 1)
return;
- basename = cgit_repobasename(repo->url);
+ basename = cgit_snapshot_prefix(repo);
if (starts_with(ref, basename))
strbuf_addstr(&filename, ref);
else
diff --git a/ui-shared.c b/ui-shared.c
index da92594..df9bbbf 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -151,6 +151,14 @@ const char *cgit_repobasename(const char *reponame)
return rvbuf;
}
+const char *cgit_snapshot_prefix(const struct cgit_repo *repo)
+{
+ if (repo->snapshot_prefix)
+ return repo->snapshot_prefix;
+
+ return cgit_repobasename(repo->url);
+}
+
static void site_url(const char *page, const char *search, const char *sort,
int ofs, int always_root)
{
char *delim = "?";
@@ -1109,7 +1117,7 @@ void cgit_print_snapshot_links(const struct cgit_repo
*repo, const char *head,
struct strbuf filename = STRBUF_INIT;
size_t prefixlen;
- cgit_compose_snapshot_prefix(&filename, cgit_repobasename(repo->url),
hex);
+ cgit_compose_snapshot_prefix(&filename, cgit_snapshot_prefix(repo),
hex);
prefixlen = filename.len;
for (f = cgit_snapshot_formats; f->suffix; f++) {
if (!(repo->snapshots & f->bit))
diff --git a/ui-shared.h b/ui-shared.h
index b3eb8c5..92a1755 100644
--- a/ui-shared.h
+++ b/ui-shared.h
@@ -78,6 +78,7 @@ extern void cgit_compose_snapshot_prefix(struct strbuf
*filename,
const char *base, const char *ref);
extern void cgit_print_snapshot_links(const struct cgit_repo *repo,
const char *head, const char *hex);
+extern const char *cgit_snapshot_prefix(const struct cgit_repo *repo);
extern void cgit_add_hidden_formfields(int incl_head, int incl_search,
const char *page);
--
2.14.1.555.g1b9dbff880.dirty
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH 0/2] Custom snapshot prefix
This isn't the mechanism Konstantin asked for, but for this particular feature I think this is a better way of achieving the same effect. I don't see much need for a per-tag configuration option for the snapshot prefix, so this enables it only at the repository level. Please let me know if I've missed something here! John Keeping (2): ui-shared: pass repo object to print_snapshot_links() Add "snapshot-prefix" repo configuration cgit.c | 2 ++ cgit.h | 1 + cgitrc.5.txt | 7 +++ ui-commit.c | 3 +-- ui-refs.c| 2 +- ui-shared.c | 16 ui-shared.h | 5 +++-- ui-tag.c | 3 +-- 8 files changed, 28 insertions(+), 11 deletions(-) -- 2.14.1.555.g1b9dbff880.dirty ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH 1/2] ui-shared: pass repo object to print_snapshot_links()
Both call sites of cgit_print_snapshot_links() use the same values for
the snapshot mask and repository name, which are derived from the
cgit_repo structure so let's pass in the structure and access the fields
directly.
Signed-off-by: John Keeping
---
ui-commit.c | 3 +--
ui-shared.c | 8
ui-shared.h | 4 ++--
ui-tag.c| 3 +--
4 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/ui-commit.c b/ui-commit.c
index abf58f6..ea17461 100644
--- a/ui-commit.c
+++ b/ui-commit.c
@@ -110,8 +110,7 @@ void cgit_print_commit(char *hex, const char *prefix)
}
if (ctx.repo->snapshots) {
html("download");
- cgit_print_snapshot_links(ctx.qry.repo, ctx.qry.head,
- hex, ctx.repo->snapshots);
+ cgit_print_snapshot_links(ctx.repo, ctx.qry.head, hex);
html("");
}
html("\n");
diff --git a/ui-shared.c b/ui-shared.c
index 9d8f66b..da92594 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -1102,17 +1102,17 @@ void cgit_compose_snapshot_prefix(struct strbuf
*filename, const char *base,
strbuf_addf(filename, "%s-%s", base, ref);
}
-void cgit_print_snapshot_links(const char *repo, const char *head,
- const char *hex, int snapshots)
+void cgit_print_snapshot_links(const struct cgit_repo *repo, const char *head,
+ const char *hex)
{
const struct cgit_snapshot_format* f;
struct strbuf filename = STRBUF_INIT;
size_t prefixlen;
- cgit_compose_snapshot_prefix(&filename, cgit_repobasename(repo), hex);
+ cgit_compose_snapshot_prefix(&filename, cgit_repobasename(repo->url),
hex);
prefixlen = filename.len;
for (f = cgit_snapshot_formats; f->suffix; f++) {
- if (!(snapshots & f->bit))
+ if (!(repo->snapshots & f->bit))
continue;
strbuf_setlen(&filename, prefixlen);
strbuf_addstr(&filename, f->suffix);
diff --git a/ui-shared.h b/ui-shared.h
index b760a17..b3eb8c5 100644
--- a/ui-shared.h
+++ b/ui-shared.h
@@ -76,8 +76,8 @@ extern void cgit_print_pageheader(void);
extern void cgit_print_filemode(unsigned short mode);
extern void cgit_compose_snapshot_prefix(struct strbuf *filename,
const char *base, const char *ref);
-extern void cgit_print_snapshot_links(const char *repo, const char *head,
- const char *hex, int snapshots);
+extern void cgit_print_snapshot_links(const struct cgit_repo *repo,
+ const char *head, const char *hex);
extern void cgit_add_hidden_formfields(int incl_head, int incl_search,
const char *page);
diff --git a/ui-tag.c b/ui-tag.c
index 909cde0..a7c44c0 100644
--- a/ui-tag.c
+++ b/ui-tag.c
@@ -34,8 +34,7 @@ static void print_tag_content(char *buf)
static void print_download_links(char *revname)
{
html("download");
- cgit_print_snapshot_links(ctx.qry.repo, ctx.qry.head,
- revname, ctx.repo->snapshots);
+ cgit_print_snapshot_links(ctx.repo, ctx.qry.head, revname);
html("");
}
--
2.14.1.555.g1b9dbff880.dirty
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH v2 3/3] Add "snapshot-prefix" repo configuration
Allow using a user-specified value for the prefix in snapshot files
instead of the repository basename. For example, files downloaded from
the linux-stable.git repository should be named linux-$VERSION and not
linux-stable-$VERSION, which can be achieved by setting:
repo.snapshot-prefix=linux
Signed-off-by: John Keeping
---
v2:
- also update request handling in ui-snapshot.c
cgit.c| 2 ++
cgit.h| 1 +
cgitrc.5.txt | 7 +++
ui-refs.c | 2 +-
ui-shared.c | 10 +-
ui-shared.h | 1 +
ui-snapshot.c | 4 ++--
7 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/cgit.c b/cgit.c
index bd9cb3f..d2f7b9c 100644
--- a/cgit.c
+++ b/cgit.c
@@ -79,6 +79,8 @@ static void repo_config(struct cgit_repo *repo, const char
*name, const char *va
item->util = xstrdup(value);
} else if (!strcmp(name, "section"))
repo->section = xstrdup(value);
+ else if (!strcmp(name, "snapshot-prefix"))
+ repo->snapshot_prefix = xstrdup(value);
else if (!strcmp(name, "readme") && value != NULL) {
if (repo->readme.items == ctx.cfg.readme.items)
memset(&repo->readme, 0, sizeof(repo->readme));
diff --git a/cgit.h b/cgit.h
index 005ae63..847cd2e 100644
--- a/cgit.h
+++ b/cgit.h
@@ -88,6 +88,7 @@ struct cgit_repo {
char *clone_url;
char *logo;
char *logo_link;
+ char *snapshot_prefix;
int snapshots;
int enable_commit_graph;
int enable_log_filecount;
diff --git a/cgitrc.5.txt b/cgitrc.5.txt
index 4da166c..a9d3d0a 100644
--- a/cgitrc.5.txt
+++ b/cgitrc.5.txt
@@ -599,6 +599,13 @@ repo.snapshots::
restricted by the global "snapshots" setting. Default value:
.
+repo.snapshot-prefix::
+ Prefix to use for snapshot links instead of the repository basename.
+ For example, the "linux-stable" repository may wish to set this to
+ "linux" so that snapshots are in the format "linux-3.15.4" instead
+ of "linux-stable-3.15.4". Default value: meaning to use
+ the repository basename.
+
repo.section::
Override the current section name for this repository. Default value:
none.
diff --git a/ui-refs.c b/ui-refs.c
index 75f2789..50d9d30 100644
--- a/ui-refs.c
+++ b/ui-refs.c
@@ -100,7 +100,7 @@ static void print_tag_downloads(const struct cgit_repo
*repo, const char *ref)
if (!ref || strlen(ref) < 1)
return;
- basename = cgit_repobasename(repo->url);
+ basename = cgit_snapshot_prefix(repo);
if (starts_with(ref, basename))
strbuf_addstr(&filename, ref);
else
diff --git a/ui-shared.c b/ui-shared.c
index da92594..df9bbbf 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -151,6 +151,14 @@ const char *cgit_repobasename(const char *reponame)
return rvbuf;
}
+const char *cgit_snapshot_prefix(const struct cgit_repo *repo)
+{
+ if (repo->snapshot_prefix)
+ return repo->snapshot_prefix;
+
+ return cgit_repobasename(repo->url);
+}
+
static void site_url(const char *page, const char *search, const char *sort,
int ofs, int always_root)
{
char *delim = "?";
@@ -1109,7 +1117,7 @@ void cgit_print_snapshot_links(const struct cgit_repo
*repo, const char *head,
struct strbuf filename = STRBUF_INIT;
size_t prefixlen;
- cgit_compose_snapshot_prefix(&filename, cgit_repobasename(repo->url),
hex);
+ cgit_compose_snapshot_prefix(&filename, cgit_snapshot_prefix(repo),
hex);
prefixlen = filename.len;
for (f = cgit_snapshot_formats; f->suffix; f++) {
if (!(repo->snapshots & f->bit))
diff --git a/ui-shared.h b/ui-shared.h
index b3eb8c5..92a1755 100644
--- a/ui-shared.h
+++ b/ui-shared.h
@@ -78,6 +78,7 @@ extern void cgit_compose_snapshot_prefix(struct strbuf
*filename,
const char *base, const char *ref);
extern void cgit_print_snapshot_links(const struct cgit_repo *repo,
const char *head, const char *hex);
+extern const char *cgit_snapshot_prefix(const struct cgit_repo *repo);
extern void cgit_add_hidden_formfields(int incl_head, int incl_search,
const char *page);
diff --git a/ui-snapshot.c b/ui-snapshot.c
index 237a75f..b9e2a36 100644
--- a/ui-snapshot.c
+++ b/ui-snapshot.c
@@ -154,7 +154,7 @@ static const char *get_ref_from_filename(const struct
cgit_repo *repo,
if (get_oid(snapshot.buf, &oid) == 0)
goto out;
- reponame = cgit_repobasename(repo->url);
+ reponame = cgit_snapshot_prefix(repo);
if (starts_with(snapshot.buf, reponame)) {
const char *new_start = snapshot.buf;
[PATCH v2 1/3] ui-shared: pass repo object to print_snapshot_links()
Both call sites of cgit_print_snapshot_links() use the same values for
the snapshot mask and repository name, which are derived from the
cgit_repo structure so let's pass in the structure and access the fields
directly.
Signed-off-by: John Keeping
---
v2: unchanged
ui-commit.c | 3 +--
ui-shared.c | 8
ui-shared.h | 4 ++--
ui-tag.c| 3 +--
4 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/ui-commit.c b/ui-commit.c
index abf58f6..ea17461 100644
--- a/ui-commit.c
+++ b/ui-commit.c
@@ -110,8 +110,7 @@ void cgit_print_commit(char *hex, const char *prefix)
}
if (ctx.repo->snapshots) {
html("download");
- cgit_print_snapshot_links(ctx.qry.repo, ctx.qry.head,
- hex, ctx.repo->snapshots);
+ cgit_print_snapshot_links(ctx.repo, ctx.qry.head, hex);
html("");
}
html("\n");
diff --git a/ui-shared.c b/ui-shared.c
index 9d8f66b..da92594 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -1102,17 +1102,17 @@ void cgit_compose_snapshot_prefix(struct strbuf
*filename, const char *base,
strbuf_addf(filename, "%s-%s", base, ref);
}
-void cgit_print_snapshot_links(const char *repo, const char *head,
- const char *hex, int snapshots)
+void cgit_print_snapshot_links(const struct cgit_repo *repo, const char *head,
+ const char *hex)
{
const struct cgit_snapshot_format* f;
struct strbuf filename = STRBUF_INIT;
size_t prefixlen;
- cgit_compose_snapshot_prefix(&filename, cgit_repobasename(repo), hex);
+ cgit_compose_snapshot_prefix(&filename, cgit_repobasename(repo->url),
hex);
prefixlen = filename.len;
for (f = cgit_snapshot_formats; f->suffix; f++) {
- if (!(snapshots & f->bit))
+ if (!(repo->snapshots & f->bit))
continue;
strbuf_setlen(&filename, prefixlen);
strbuf_addstr(&filename, f->suffix);
diff --git a/ui-shared.h b/ui-shared.h
index b760a17..b3eb8c5 100644
--- a/ui-shared.h
+++ b/ui-shared.h
@@ -76,8 +76,8 @@ extern void cgit_print_pageheader(void);
extern void cgit_print_filemode(unsigned short mode);
extern void cgit_compose_snapshot_prefix(struct strbuf *filename,
const char *base, const char *ref);
-extern void cgit_print_snapshot_links(const char *repo, const char *head,
- const char *hex, int snapshots);
+extern void cgit_print_snapshot_links(const struct cgit_repo *repo,
+ const char *head, const char *hex);
extern void cgit_add_hidden_formfields(int incl_head, int incl_search,
const char *page);
diff --git a/ui-tag.c b/ui-tag.c
index 909cde0..a7c44c0 100644
--- a/ui-tag.c
+++ b/ui-tag.c
@@ -34,8 +34,7 @@ static void print_tag_content(char *buf)
static void print_download_links(char *revname)
{
html("download");
- cgit_print_snapshot_links(ctx.qry.repo, ctx.qry.head,
- revname, ctx.repo->snapshots);
+ cgit_print_snapshot_links(ctx.repo, ctx.qry.head, revname);
html("");
}
--
2.16.3
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH v2 0/3] Custom snapshot prefix
It turns out I should have tested whether downloads with a custom prefix actually work... they didn't! v2 adds a new refactoring patch in the middle and ensures that we update the request handling logic in the final patch. John Keeping (3): ui-shared: pass repo object to print_snapshot_links() ui-snapshot: pass repo into get_ref_from_filename() Add "snapshot-prefix" repo configuration cgit.c| 2 ++ cgit.h| 1 + cgitrc.5.txt | 7 +++ ui-commit.c | 3 +-- ui-refs.c | 2 +- ui-shared.c | 16 ui-shared.h | 5 +++-- ui-snapshot.c | 9 + ui-tag.c | 3 +-- 9 files changed, 33 insertions(+), 15 deletions(-) -- 2.16.3 ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH v2 2/3] ui-snapshot: pass repo into get_ref_from_filename()
Prepare to allow a custom snapshot prefix.
Signed-off-by: John Keeping
---
v2: new patch
ui-snapshot.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/ui-snapshot.c b/ui-snapshot.c
index b2d95f7..237a75f 100644
--- a/ui-snapshot.c
+++ b/ui-snapshot.c
@@ -139,7 +139,8 @@ static int make_snapshot(const struct cgit_snapshot_format
*format,
* pending a 'v' or a 'V' to the remaining snapshot name ("0.7.2" ->
* "v0.7.2") gives us something valid.
*/
-static const char *get_ref_from_filename(const char *url, const char *filename,
+static const char *get_ref_from_filename(const struct cgit_repo *repo,
+const char *filename,
const struct cgit_snapshot_format
*format)
{
const char *reponame;
@@ -153,7 +154,7 @@ static const char *get_ref_from_filename(const char *url,
const char *filename,
if (get_oid(snapshot.buf, &oid) == 0)
goto out;
- reponame = cgit_repobasename(url);
+ reponame = cgit_repobasename(repo->url);
if (starts_with(snapshot.buf, reponame)) {
const char *new_start = snapshot.buf;
new_start += strlen(reponame);
@@ -200,7 +201,7 @@ void cgit_print_snapshot(const char *head, const char *hex,
}
if (!hex && dwim) {
- hex = get_ref_from_filename(ctx.repo->url, filename, f);
+ hex = get_ref_from_filename(ctx.repo, filename, f);
if (hex == NULL) {
cgit_print_error_page(404, "Not found", "Not found");
return;
--
2.16.3
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH] ui-snapshot: filter permitted snapshot requests
Currently the snapshots configuration option only filters which links
are displayed, not which snapshots may be generated and downloaded.
Apply the filter to requests as well to ensure that the system policy is
enforced.
Signed-off-by: John Keeping
---
ui-snapshot.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ui-snapshot.c b/ui-snapshot.c
index b2d95f7..01a 100644
--- a/ui-snapshot.c
+++ b/ui-snapshot.c
@@ -193,7 +193,7 @@ void cgit_print_snapshot(const char *head, const char *hex,
}
f = get_format(filename);
- if (!f) {
+ if (!f || !(ctx.repo->snapshots & f->bit)) {
cgit_print_error_page(400, "Bad request",
"Unsupported snapshot format: %s", filename);
return;
--
2.16.3
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
[RFC/PATCH 0/7] Snapshot signatures
Again, this isn't exactly what Konstantin asked for, but this approach is easier to implement and I think achieves the main goal of exposing signatures for snapshots. The difference is that this approach has a notes ref for each archive format and the note simply contains the signature content; there is no additional metadata you just stick the signature in (for example) refs/notes/signatures/tar.gz against the relevant tag object. This disadvantage of this is that we only support a single signature format, but I don't think that's a huge drawback as I expect any future format to be obviously different so that we can easily inspect the signature content to tell the difference. The first 6 patches are refactoring which I think makes sense even without the end goal of this series (they also fix an inconsistency between snapshot links in the summary and commit pages). The series builds on my "Custom snapshot prefix" series [1]. The final patch is the one that I expect feedback on; there's definitely a lack of documentation but there's no point in writing that if this approach is vetoed. [1] https://lists.zx2c4.com/pipermail/cgit/2018-March/003767.html John Keeping (7): ui-refs: remove unnecessary sanity check ui-shared: remove unused parameter ui-shared: rename parameter to cgit_print_snapshot_links() ui-shared: use the same snapshot logic as ui-refs ui-shared: pass separator in to cgit_print_snapshot_links() ui-refs: use shared function to print tag downloads snapshot: support archive signatures cgit.h| 2 ++ ui-commit.c | 2 +- ui-refs.c | 30 +-- ui-shared.c | 21 + ui-shared.h | 2 +- ui-snapshot.c | 76 ++- ui-tag.c | 2 +- 7 files changed, 98 insertions(+), 37 deletions(-) -- 2.16.3 ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
[RFC/PATCH 1/7] ui-refs: remove unnecessary sanity check
There is no way for refinfo::refname to be null, and Git will prevent zero-length refs so this check is unnecessary. Signed-off-by: John Keeping --- ui-refs.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/ui-refs.c b/ui-refs.c index 50d9d30..7b95e8b 100644 --- a/ui-refs.c +++ b/ui-refs.c @@ -97,9 +97,6 @@ static void print_tag_downloads(const struct cgit_repo *repo, const char *ref) struct strbuf filename = STRBUF_INIT; size_t prefixlen; - if (!ref || strlen(ref) < 1) - return; - basename = cgit_snapshot_prefix(repo); if (starts_with(ref, basename)) strbuf_addstr(&filename, ref); -- 2.16.3 ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
[RFC/PATCH 3/7] ui-shared: rename parameter to cgit_print_snapshot_links()
This is expected to be a ref not a hex object ID, so name it more
appropriately.
Signed-off-by: John Keeping
---
ui-shared.c | 4 ++--
ui-shared.h | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/ui-shared.c b/ui-shared.c
index 47b8dc9..0375006 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -1110,13 +1110,13 @@ void cgit_compose_snapshot_prefix(struct strbuf
*filename, const char *base,
strbuf_addf(filename, "%s-%s", base, ref);
}
-void cgit_print_snapshot_links(const struct cgit_repo *repo, const char *hex)
+void cgit_print_snapshot_links(const struct cgit_repo *repo, const char *ref)
{
const struct cgit_snapshot_format* f;
struct strbuf filename = STRBUF_INIT;
size_t prefixlen;
- cgit_compose_snapshot_prefix(&filename, cgit_snapshot_prefix(repo),
hex);
+ cgit_compose_snapshot_prefix(&filename, cgit_snapshot_prefix(repo),
ref);
prefixlen = filename.len;
for (f = cgit_snapshot_formats; f->suffix; f++) {
if (!(repo->snapshots & f->bit))
diff --git a/ui-shared.h b/ui-shared.h
index 5c5dc33..d44d7c6 100644
--- a/ui-shared.h
+++ b/ui-shared.h
@@ -77,7 +77,7 @@ extern void cgit_print_filemode(unsigned short mode);
extern void cgit_compose_snapshot_prefix(struct strbuf *filename,
const char *base, const char *ref);
extern void cgit_print_snapshot_links(const struct cgit_repo *repo,
- const char *hex);
+ const char *ref);
extern const char *cgit_snapshot_prefix(const struct cgit_repo *repo);
extern void cgit_add_hidden_formfields(int incl_head, int incl_search,
const char *page);
--
2.16.3
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
[RFC/PATCH 2/7] ui-shared: remove unused parameter
The "head" parameter to cgit_print_snapshot_links() is never used, so
remove it.
Signed-off-by: John Keeping
---
ui-commit.c | 2 +-
ui-shared.c | 3 +--
ui-shared.h | 2 +-
ui-tag.c| 2 +-
4 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/ui-commit.c b/ui-commit.c
index ea17461..37c7d8d 100644
--- a/ui-commit.c
+++ b/ui-commit.c
@@ -110,7 +110,7 @@ void cgit_print_commit(char *hex, const char *prefix)
}
if (ctx.repo->snapshots) {
html("download");
- cgit_print_snapshot_links(ctx.repo, ctx.qry.head, hex);
+ cgit_print_snapshot_links(ctx.repo, hex);
html("");
}
html("\n");
diff --git a/ui-shared.c b/ui-shared.c
index df9bbbf..47b8dc9 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -1110,8 +1110,7 @@ void cgit_compose_snapshot_prefix(struct strbuf
*filename, const char *base,
strbuf_addf(filename, "%s-%s", base, ref);
}
-void cgit_print_snapshot_links(const struct cgit_repo *repo, const char *head,
- const char *hex)
+void cgit_print_snapshot_links(const struct cgit_repo *repo, const char *hex)
{
const struct cgit_snapshot_format* f;
struct strbuf filename = STRBUF_INIT;
diff --git a/ui-shared.h b/ui-shared.h
index 92a1755..5c5dc33 100644
--- a/ui-shared.h
+++ b/ui-shared.h
@@ -77,7 +77,7 @@ extern void cgit_print_filemode(unsigned short mode);
extern void cgit_compose_snapshot_prefix(struct strbuf *filename,
const char *base, const char *ref);
extern void cgit_print_snapshot_links(const struct cgit_repo *repo,
- const char *head, const char *hex);
+ const char *hex);
extern const char *cgit_snapshot_prefix(const struct cgit_repo *repo);
extern void cgit_add_hidden_formfields(int incl_head, int incl_search,
const char *page);
diff --git a/ui-tag.c b/ui-tag.c
index a7c44c0..3a6dcee 100644
--- a/ui-tag.c
+++ b/ui-tag.c
@@ -34,7 +34,7 @@ static void print_tag_content(char *buf)
static void print_download_links(char *revname)
{
html("download");
- cgit_print_snapshot_links(ctx.repo, ctx.qry.head, revname);
+ cgit_print_snapshot_links(ctx.repo, revname);
html("");
}
--
2.16.3
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
[RFC/PATCH 4/7] ui-shared: use the same snapshot logic as ui-refs
Make snapshot links in the commit UI use the same prefix algorithm as
those in the summary UI, so that refs starting with the snapshot prefix
are used as-is rather than composed with the prefix repeated.
Signed-off-by: John Keeping
---
ui-shared.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/ui-shared.c b/ui-shared.c
index 0375006..588f0bf 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -1114,9 +1114,15 @@ void cgit_print_snapshot_links(const struct cgit_repo
*repo, const char *ref)
{
const struct cgit_snapshot_format* f;
struct strbuf filename = STRBUF_INIT;
+ const char *basename;
size_t prefixlen;
- cgit_compose_snapshot_prefix(&filename, cgit_snapshot_prefix(repo),
ref);
+ basename = cgit_snapshot_prefix(repo);
+ if (starts_with(ref, basename))
+ strbuf_addstr(&filename, ref);
+ else
+ cgit_compose_snapshot_prefix(&filename, basename, ref);
+
prefixlen = filename.len;
for (f = cgit_snapshot_formats; f->suffix; f++) {
if (!(repo->snapshots & f->bit))
--
2.16.3
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
[RFC/PATCH 6/7] ui-refs: use shared function to print tag downloads
cgit_compose_snapshot_prefix() is identical to print_tag_downloads(), so
remove the latter and use the function from ui-shared.c instead.
Signed-off-by: John Keeping
---
ui-refs.c | 27 +--
1 file changed, 1 insertion(+), 26 deletions(-)
diff --git a/ui-refs.c b/ui-refs.c
index 7b95e8b..2ec3858 100644
--- a/ui-refs.c
+++ b/ui-refs.c
@@ -90,31 +90,6 @@ static void print_tag_header(void)
"Age\n");
}
-static void print_tag_downloads(const struct cgit_repo *repo, const char *ref)
-{
- const struct cgit_snapshot_format* f;
- const char *basename;
- struct strbuf filename = STRBUF_INIT;
- size_t prefixlen;
-
- basename = cgit_snapshot_prefix(repo);
- if (starts_with(ref, basename))
- strbuf_addstr(&filename, ref);
- else
- cgit_compose_snapshot_prefix(&filename, basename, ref);
- prefixlen = filename.len;
- for (f = cgit_snapshot_formats; f->suffix; f++) {
- if (!(repo->snapshots & f->bit))
- continue;
- strbuf_setlen(&filename, prefixlen);
- strbuf_addstr(&filename, f->suffix);
- cgit_snapshot_link(filename.buf, NULL, NULL, NULL, NULL,
filename.buf);
- html(" ");
- }
-
- strbuf_release(&filename);
-}
-
static int print_tag(struct refinfo *ref)
{
struct tag *tag = NULL;
@@ -134,7 +109,7 @@ static int print_tag(struct refinfo *ref)
cgit_tag_link(name, NULL, NULL, name);
html("");
if (ctx.repo->snapshots && (obj->type == OBJ_COMMIT))
- print_tag_downloads(ctx.repo, name);
+ cgit_print_snapshot_links(ctx.repo, name, " ");
else
cgit_object_link(obj);
html("");
--
2.16.3
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
[RFC/PATCH 5/7] ui-shared: pass separator in to cgit_print_snapshot_links()
cgit_print_snapshot_links() is almost identical to
print_tag_downloads(), so let's extract the difference to a parameter in
preparation for removing print_tag_downloads() in the next commit.
Signed-off-by: John Keeping
---
ui-commit.c | 2 +-
ui-shared.c | 5 +++--
ui-shared.h | 2 +-
ui-tag.c| 2 +-
4 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/ui-commit.c b/ui-commit.c
index 37c7d8d..65b4603 100644
--- a/ui-commit.c
+++ b/ui-commit.c
@@ -110,7 +110,7 @@ void cgit_print_commit(char *hex, const char *prefix)
}
if (ctx.repo->snapshots) {
html("download");
- cgit_print_snapshot_links(ctx.repo, hex);
+ cgit_print_snapshot_links(ctx.repo, hex, "");
html("");
}
html("\n");
diff --git a/ui-shared.c b/ui-shared.c
index 588f0bf..6415a33 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -1110,7 +1110,8 @@ void cgit_compose_snapshot_prefix(struct strbuf
*filename, const char *base,
strbuf_addf(filename, "%s-%s", base, ref);
}
-void cgit_print_snapshot_links(const struct cgit_repo *repo, const char *ref)
+void cgit_print_snapshot_links(const struct cgit_repo *repo, const char *ref,
+ const char *separator)
{
const struct cgit_snapshot_format* f;
struct strbuf filename = STRBUF_INIT;
@@ -1131,7 +1132,7 @@ void cgit_print_snapshot_links(const struct cgit_repo
*repo, const char *ref)
strbuf_addstr(&filename, f->suffix);
cgit_snapshot_link(filename.buf, NULL, NULL, NULL, NULL,
filename.buf);
- html("");
+ html(separator);
}
strbuf_release(&filename);
}
diff --git a/ui-shared.h b/ui-shared.h
index d44d7c6..4d5978b 100644
--- a/ui-shared.h
+++ b/ui-shared.h
@@ -77,7 +77,7 @@ extern void cgit_print_filemode(unsigned short mode);
extern void cgit_compose_snapshot_prefix(struct strbuf *filename,
const char *base, const char *ref);
extern void cgit_print_snapshot_links(const struct cgit_repo *repo,
- const char *ref);
+ const char *ref, const char *separator);
extern const char *cgit_snapshot_prefix(const struct cgit_repo *repo);
extern void cgit_add_hidden_formfields(int incl_head, int incl_search,
const char *page);
diff --git a/ui-tag.c b/ui-tag.c
index 3a6dcee..0bac93a 100644
--- a/ui-tag.c
+++ b/ui-tag.c
@@ -34,7 +34,7 @@ static void print_tag_content(char *buf)
static void print_download_links(char *revname)
{
html("download");
- cgit_print_snapshot_links(ctx.repo, revname);
+ cgit_print_snapshot_links(ctx.repo, revname, "");
html("");
}
--
2.16.3
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
[RFC/PATCH 7/7] snapshot: support archive signatures
Read signatures from the notes refs refs/notes/signatures/$FORMAT where
FORMAT is one of our archive formats ("tar", "tar.gz", ...). The note
is expected to simply contain the signature content to be returned when
the snapshot "${filename}.asc" is requested, so the signature for
cgit-1.1.tar.xz can be stored against the v1.1 tag with:
git notes --ref=refs/notes/signatures/tar.xz add -C "$(
gpg --output - --armor --detach-sign cgit-1.1.tar.xz |
git hash-object -w --stdin
)" v1.1
and then downloaded by simply appending ".asc" to the archive URL.
Signed-off-by: John Keeping
---
cgit.h| 2 ++
ui-shared.c | 7 ++
ui-snapshot.c | 76 ++-
3 files changed, 84 insertions(+), 1 deletion(-)
diff --git a/cgit.h b/cgit.h
index 847cd2e..a686390 100644
--- a/cgit.h
+++ b/cgit.h
@@ -374,6 +374,8 @@ extern void cgit_parse_url(const char *url);
extern const char *cgit_repobasename(const char *reponame);
extern int cgit_parse_snapshots_mask(const char *str);
+extern const struct object_id *cgit_snapshot_get_sig(const char *ref,
+const struct
cgit_snapshot_format *f);
extern int cgit_open_filter(struct cgit_filter *filter, ...);
extern int cgit_close_filter(struct cgit_filter *filter);
diff --git a/ui-shared.c b/ui-shared.c
index 6415a33..b5e57e0 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -1132,6 +1132,13 @@ void cgit_print_snapshot_links(const struct cgit_repo
*repo, const char *ref,
strbuf_addstr(&filename, f->suffix);
cgit_snapshot_link(filename.buf, NULL, NULL, NULL, NULL,
filename.buf);
+ if (cgit_snapshot_get_sig(ref, f)) {
+ strbuf_addstr(&filename, ".asc");
+ html(" (");
+ cgit_snapshot_link("sig", NULL, NULL, NULL, NULL,
+ filename.buf);
+ html(")");
+ }
html(separator);
}
strbuf_release(&filename);
diff --git a/ui-snapshot.c b/ui-snapshot.c
index abf8399..c7611e8 100644
--- a/ui-snapshot.c
+++ b/ui-snapshot.c
@@ -94,6 +94,31 @@ const struct cgit_snapshot_format cgit_snapshot_formats[] = {
{ NULL }
};
+static struct notes_tree snapshot_sig_notes[ARRAY_SIZE(cgit_snapshot_formats)];
+
+const struct object_id *cgit_snapshot_get_sig(const char *ref,
+ const struct cgit_snapshot_format
*f)
+{
+ struct notes_tree *tree;
+ struct object_id oid;
+
+ if (get_oid(ref, &oid))
+ return NULL;
+
+ tree = &snapshot_sig_notes[f - &cgit_snapshot_formats[0]];
+ if (!tree->initialized) {
+ struct strbuf notes_ref = STRBUF_INIT;
+
+ strbuf_addf(¬es_ref, "refs/notes/signatures/%s",
+ f->suffix + 1);
+
+ init_notes(tree, notes_ref.buf, combine_notes_ignore, 0);
+ strbuf_release(¬es_ref);
+ }
+
+ return get_note(tree, &oid);
+}
+
static const struct cgit_snapshot_format *get_format(const char *filename)
{
const struct cgit_snapshot_format *fmt;
@@ -129,6 +154,39 @@ static int make_snapshot(const struct cgit_snapshot_format
*format,
return 0;
}
+static int write_sig(const struct cgit_snapshot_format *format,
+const char *hex, const char *archive,
+const char *filename)
+{
+ const struct object_id *note = cgit_snapshot_get_sig(hex, format);
+ enum object_type type;
+ unsigned long size;
+ char *buf;
+
+ if (!note) {
+ cgit_print_error_page(404, "Not found",
+ "No signature for %s", archive);
+ return 0;
+ }
+
+ buf = read_sha1_file(note->hash, &type, &size);
+ if (!buf) {
+ cgit_print_error_page(404, "Not found", "Not found");
+ return 0;
+ }
+
+ html("X-Content-Type-Options: nosniff\n");
+ html("Content-Security-Policy: default-src 'none'\n");
+ ctx.page.etag = oid_to_hex(note);
+ ctx.page.mimetype = xstrdup("application/pgp-signature");
+ ctx.page.filename = xstrdup(filename);
+ cgit_print_http_headers();
+
+ html_raw(buf, size);
+ free(buf);
+ return 0;
+}
+
/* Try to guess the requested revision from the requested snapshot name.
* First the format extension is stripped, e.g. "cgit-0.7.2.tar.gz" become
* "cgit-0.7.2". If this is a valid commit object name we've got a winner.
@@ -185,6 +243,8 @@ void cgit
Re: [PATCH 1/1] ui-log: highlight annotated tags in different color
On Tue, Jun 05, 2018 at 12:47:52PM +0200, Christian Hesse wrote:
> Signed-off-by: Christian Hesse
Can you say a little about why this is desirable as a feature?
> ---
> cgit.css | 8
> ui-log.c | 7 ++-
> 2 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/cgit.css b/cgit.css
> index 217a05a..05c4530 100644
> --- a/cgit.css
> +++ b/cgit.css
> @@ -685,6 +685,14 @@ div#cgit a.tag-deco {
> border: solid 1px #00;
> }
>
> +div#cgit a.tag-annotated-deco {
> + color: #000;
> + margin: 0px 0.5em;
> + padding: 0px 0.25em;
> + background-color: #ffcc88;
> + border: solid 1px #00;
> +}
> +
> div#cgit a.remote-deco {
> color: #000;
> margin: 0px 0.5em;
> diff --git a/ui-log.c b/ui-log.c
> index 8e36fba..b8344a8 100644
> --- a/ui-log.c
> +++ b/ui-log.c
> @@ -56,8 +56,10 @@ static void inspect_files(struct diff_filepair *pair)
>
> void show_commit_decorations(struct commit *commit)
> {
> + struct object_id peeled;
> const struct name_decoration *deco;
> static char buf[1024];
> + int is_annotated;
The two new declarations could be moved ...
> buf[sizeof(buf) - 1] = 0;
> deco = get_name_decoration(&commit->object);
> @@ -65,6 +67,7 @@ void show_commit_decorations(struct commit *commit)
> return;
> html("");
> while (deco) {
> + is_annotated = 0;
... here, which will make it more obvious that is_annotated is being
initialized correctly (it took me a minute to spot this and I initially
worried that is_annotated was used without initialization if peel_ref
fails).
> strncpy(buf, prettify_refname(deco->name), sizeof(buf) - 1);
> switch(deco->type) {
> case DECORATION_NONE:
> @@ -77,7 +80,9 @@ void show_commit_decorations(struct commit *commit)
> ctx.qry.showmsg, 0);
> break;
> case DECORATION_REF_TAG:
> - cgit_tag_link(buf, NULL, "tag-deco", buf);
> + if (!peel_ref(deco->name, &peeled))
> + is_annotated = !oidcmp(&commit->object.oid,
> &peeled);
> + cgit_tag_link(buf, NULL, is_annotated ?
> "tag-annotated-deco" : "tag-deco", buf);
> break;
> case DECORATION_REF_REMOTE:
> if (!ctx.repo->enable_remote_branches)
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH v2 1/1] ui-log: highlight annotated tags in different color
On Tue, Jun 05, 2018 at 01:40:13PM +0200, Christian Hesse wrote:
> From: Christian Hesse
It would be nice to include the rationale from your previous message in
the commit to it's easily accessible via git blame in the future.
But I don't think that's absolutely necessary, so:
Reviewed-by: John Keeping
> Signed-off-by: Christian Hesse
> ---
> cgit.css | 8
> ui-log.c | 6 +-
> 2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/cgit.css b/cgit.css
> index 217a05a..05c4530 100644
> --- a/cgit.css
> +++ b/cgit.css
> @@ -685,6 +685,14 @@ div#cgit a.tag-deco {
> border: solid 1px #00;
> }
>
> +div#cgit a.tag-annotated-deco {
> + color: #000;
> + margin: 0px 0.5em;
> + padding: 0px 0.25em;
> + background-color: #ffcc88;
> + border: solid 1px #00;
> +}
> +
> div#cgit a.remote-deco {
> color: #000;
> margin: 0px 0.5em;
> diff --git a/ui-log.c b/ui-log.c
> index 8e36fba..b5cd2f6 100644
> --- a/ui-log.c
> +++ b/ui-log.c
> @@ -65,6 +65,8 @@ void show_commit_decorations(struct commit *commit)
> return;
> html("");
> while (deco) {
> + struct object_id peeled;
> + int is_annotated = 0;
> strncpy(buf, prettify_refname(deco->name), sizeof(buf) - 1);
> switch(deco->type) {
> case DECORATION_NONE:
> @@ -77,7 +79,9 @@ void show_commit_decorations(struct commit *commit)
> ctx.qry.showmsg, 0);
> break;
> case DECORATION_REF_TAG:
> - cgit_tag_link(buf, NULL, "tag-deco", buf);
> + if (!peel_ref(deco->name, &peeled))
> + is_annotated = !oidcmp(&commit->object.oid,
> &peeled);
> + cgit_tag_link(buf, NULL, is_annotated ?
> "tag-annotated-deco" : "tag-deco", buf);
> break;
> case DECORATION_REF_REMOTE:
> if (!ctx.repo->enable_remote_branches)
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
