[PATCH 2/2] shared: use strbuf for expanding macros
Avoid a fixed size buffer by using struct strbuf to build the expanded
value.
Signed-off-by: John Keeping
---
shared.c | 73 +---
1 file changed, 27 insertions(+), 46 deletions(-)
diff --git a/shared.c b/shared.c
index 32d0f46..6ce3990 100644
--- a/shared.c
+++ b/shared.c
@@ -467,69 +467,50 @@ static int is_token_char(char c)
return isalnum(c) || c == '_';
}
-/* Replace name with getenv(name), return pointer to zero-terminating char
+/* Replace name with getenv(name).
*/
-static char *expand_macro(char *name, int maxlength)
+static void expand_macro(struct strbuf *buf, ssize_t start)
{
- char *value;
- int len;
+ const char *name = buf->buf + start;
+ const char *value = getenv(name);
- len = 0;
- value = getenv(name);
- if (value) {
- len = strlen(value);
- if (len > maxlength)
- len = maxlength;
- strncpy(name, value, len);
- }
- return name + len;
+ /* Truncate output to remove variable name. */
+ strbuf_setlen(buf, start);
+
+ if (value)
+ strbuf_addstr(buf, value);
}
-#define EXPBUFSIZE (1024 * 8)
-
/* Replace all tokens prefixed by '$' in the specified text with the
* value of the named environment variable.
* NB: the return value is allocated, it must be freed by the caller.
*/
char *expand_macros(const char *txt)
{
- static char result[EXPBUFSIZE];
- char *p, *start;
- int len;
+ struct strbuf result = STRBUF_INIT;
+ ssize_t start = -1;
- p = result;
- start = NULL;
- while (p < result + EXPBUFSIZE - 1 && txt && *txt) {
- *p = *txt;
- if (start) {
+ while (txt && *txt) {
+ if (start >= 0) {
if (!is_token_char(*txt)) {
- if (p - start > 0) {
- *p = '\0';
- len = result + EXPBUFSIZE - start - 1;
- p = expand_macro(start, len) - 1;
- }
- start = NULL;
- txt--;
+ if (result.len - start >= 0)
+ expand_macro(, start);
+ start = -1;
}
- p++;
- txt++;
- continue;
}
- if (*txt == '$') {
- start = p;
- txt++;
- continue;
- }
- p++;
+
+ if (*txt == '$')
+ start = result.len;
+ else
+ strbuf_addch(, *txt);
+
txt++;
}
- *p = '\0';
- if (start && p - start > 0) {
- len = result + EXPBUFSIZE - start - 1;
- p = expand_macro(start, len);
- *p = '\0';
- }
- return xstrdup(result);
+
+ if (start >= 0 && result.len - start > 0)
+ expand_macro(, start);
+
+ return strbuf_detach(, NULL);
}
char *get_mimetype_for_filename(const char *filename)
--
2.17.1
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH 1/2] shared: allocate return value from expand_macros()
In preparation for switching the implementation of expand_macros away
from a static buffer, return an allocated buffer and update all callers
to release it.
Signed-off-by: John Keeping
---
cgit.c | 33 +++--
shared.c| 5 ++---
ui-shared.c | 9 ++---
3 files changed, 31 insertions(+), 16 deletions(-)
diff --git a/cgit.c b/cgit.c
index bd9cb3f..ca1bc15 100644
--- a/cgit.c
+++ b/cgit.c
@@ -190,7 +190,7 @@ static void config_cb(const char *name, const char *value)
else if (!strcmp(name, "cache-size"))
ctx.cfg.cache_size = atoi(value);
else if (!strcmp(name, "cache-root"))
- ctx.cfg.cache_root = xstrdup(expand_macros(value));
+ ctx.cfg.cache_root = expand_macros(value);
else if (!strcmp(name, "cache-root-ttl"))
ctx.cfg.cache_root_ttl = atoi(value);
else if (!strcmp(name, "cache-repo-ttl"))
@@ -232,16 +232,20 @@ static void config_cb(const char *name, const char *value)
else if (!strcmp(name, "max-commit-count"))
ctx.cfg.max_commit_count = atoi(value);
else if (!strcmp(name, "project-list"))
- ctx.cfg.project_list = xstrdup(expand_macros(value));
- else if (!strcmp(name, "scan-path"))
+ ctx.cfg.project_list = expand_macros(value);
+ else if (!strcmp(name, "scan-path")) {
+ char *expanded = expand_macros(value);
+
if (!ctx.cfg.nocache && ctx.cfg.cache_size)
- process_cached_repolist(expand_macros(value));
+ process_cached_repolist(expanded);
else if (ctx.cfg.project_list)
- scan_projects(expand_macros(value),
+ scan_projects(expanded,
ctx.cfg.project_list, repo_config);
else
- scan_tree(expand_macros(value), repo_config);
- else if (!strcmp(name, "scan-hidden-path"))
+ scan_tree(expanded, repo_config);
+
+ free(expanded);
+ } else if (!strcmp(name, "scan-hidden-path"))
ctx.cfg.scan_hidden_path = atoi(value);
else if (!strcmp(name, "section-from-path"))
ctx.cfg.section_from_path = atoi(value);
@@ -287,8 +291,12 @@ static void config_cb(const char *name, const char *value)
ctx.cfg.branch_sort = 0;
} else if (skip_prefix(name, "mimetype.", ))
add_mimetype(arg, value);
- else if (!strcmp(name, "include"))
- parse_configfile(expand_macros(value), config_cb);
+ else if (!strcmp(name, "include")) {
+ char *expanded = expand_macros(value);
+
+ parse_configfile(expanded, config_cb);
+ free(expanded);
+ }
}
static void querystring_cb(const char *name, const char *value)
@@ -1041,6 +1049,7 @@ static int calc_ttl(void)
int cmd_main(int argc, const char **argv)
{
const char *path;
+ char *expanded;
int err, ttl;
cgit_init_filters();
@@ -1052,7 +1061,11 @@ int cmd_main(int argc, const char **argv)
cgit_repolist.repos = NULL;
cgit_parse_args(argc, argv);
- parse_configfile(expand_macros(ctx.env.cgit_config), config_cb);
+
+ expanded = expand_macros(ctx.env.cgit_config);
+ parse_configfile(expanded, config_cb);
+ free(expanded);
+
ctx.repo = NULL;
http_parse_querystring(ctx.qry.raw, querystring_cb);
diff --git a/shared.c b/shared.c
index 21ac8f4..32d0f46 100644
--- a/shared.c
+++ b/shared.c
@@ -489,8 +489,7 @@ static char *expand_macro(char *name, int maxlength)
/* Replace all tokens prefixed by '$' in the specified text with the
* value of the named environment variable.
- * NB: the return value is a static buffer, i.e. it must be strdup'd
- * by the caller.
+ * NB: the return value is allocated, it must be freed by the caller.
*/
char *expand_macros(const char *txt)
{
@@ -530,7 +529,7 @@ char *expand_macros(const char *txt)
p = expand_macro(start, len);
*p = '\0';
}
- return result;
+ return xstrdup(result);
}
char *get_mimetype_for_filename(const char *filename)
diff --git a/ui-shared.c b/ui-shared.c
index 9d8f66b..c48f422 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -838,9 +838,12 @@ static void add_clone_urls(void (*fn)(const char *), char
*txt, char *suffix)
void cgit_add_clone_urls(void (*fn)(const char *))
{
- if (ctx.repo->clone_url)
- add_clone_urls(fn, expand_macros(ctx.repo->clone_url), NULL);
- else if (ctx.cfg.clone_prefix)
+ if (ctx.repo->clone_url) {
+ char *expanded = expand_macros(ctx.repo->clone_url);
+
+ add_clone_urls(fn, expanded, NULL);
+ free(expanded);
+ } else if (ctx.cfg.clone_prefix)
add_clone_urls(fn,
Re: [PATCH 07/11] ui-blame: free read_sha1_file() buffer after use
On Wed, Jun 13, 2018 at 10:02:05AM +0800, Andy Green wrote:
> Signed-off-by: Andy Green
> ---
> ui-blame.c |3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/ui-blame.c b/ui-blame.c
> index 17e2d60..a5c7d69 100644
> --- a/ui-blame.c
> +++ b/ui-blame.c
> @@ -206,6 +206,9 @@ static void print_object(const unsigned char *sha1, const
> char *path,
There's an early return above here after allocating buf, I'm not sure if
we care about freeing buf on that path as well.
If we do, I guess we push free(buf) to the end and add a free_buf label
for the error path to jump to.
> } else {
> html_txt(buf);
> }
> +
> + free(buf);
> +
> html("");
>
> html("\n");
>
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH 2/2] gcc8.1: fix strcat warning
On Wed, Jun 13, 2018 at 07:34:07AM +0800, Andy Green wrote:
> ../ui-ssdiff.c: In function ‘replace_tabs’:
> ../ui-ssdiff.c:142:4: warning: ‘strncat’ output truncated copying between 1
> and 8 bytes from a string of length 8 [-Wstringop-truncation]
> strncat(result, spaces, 8 - (strlen(result) % 8));
> ^
>
> Actually the strncat that was there before intends that its
> stock of spaces gets truncated, and it's not a problem.
>
> However gcc8.1 is also right, normally truncation is undesirable.
>
> Make the code do the padding explicitly.
>
> Signed-off-by: Andy Green
Reviewed-by: John Keeping
> ---
> ui-ssdiff.c | 12 ++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/ui-ssdiff.c b/ui-ssdiff.c
> index 7f261ed..e520b95 100644
> --- a/ui-ssdiff.c
> +++ b/ui-ssdiff.c
> @@ -118,7 +118,6 @@ static char *replace_tabs(char *line)
> int n_tabs = 0;
> int i;
> char *result;
> - char *spaces = "";
>
> if (linelen == 0) {
> result = xmalloc(1);
> @@ -138,8 +137,17 @@ static char *replace_tabs(char *line)
> strcat(result, prev_buf);
> break;
> } else {
> + char *p;
> + int n;
> +
> strncat(result, prev_buf, cur_buf - prev_buf);
> - strncat(result, spaces, 8 - (strlen(result) % 8));
> +
> + n = strlen(result);
> + p = result + n;
> + n = 8 - (n % 8);
> + while (n--)
> + *p++ = ' ';
> + *p = '\0';
> }
> prev_buf = cur_buf + 1;
> }
>
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [RFC PATCH] Makefile: work around parallel make issues in docs
On Sat, Jun 16, 2018 at 01:38:31AM -0400, Todd Zullinger wrote:
> When make is run with multiple jobs, doc-man and doc-html fail. The a2x
> command tries to write %.5.xml for each invocation, overwriting each
> other.
>
> Work around this by copying %.5 to %.5+ in doc-man. This is a rather
> gross hack, but nothing better came to mind. Using --asciidoc-opts to
> pass the '--out-file' did not affect the temporary .xml file at issue.
>
> Signed-off-by: Todd Zullinger
> ---
> I ran into this in the Fedora/EPEL package builds long ago and set -j1 for the
> doc build at the time. I finally got around to looking a little deeper.
>
> I'm not happy with this patch, but as I said above, nothing better came to
> mind. I'm hoping that submitting this will spur some discussion about a
> better
> way to handle this. Though maybe the best way to handle it will be "don't
> build the docs in parallel, you're not saving any time for the single file
> that
> is built." :)
>
> That's effectively what I've done for ages, a separate 'make -j1 doc-man
> doc-html' invocation.
>
> Reproduction should be simple: make -j2 doc-man doc-html
>
> Each a2x process tries to create cgitrc.5.xml in the process of generating the
> man and html files, clobbering each other and causing xmllint to fail.
>
> This is still broken when calling 'make -j2 doc' as I only added the rename
> work-around to doc-man, so if doc-man, doc-html, and doc-pdf are all run, the
> two without the work-around will cause the same issue.
>
> I consider this more of a demonstration of the issue than a fix. I hope that
> by sending it, someone will respond with the obvious, elegant, and correct
> solution. Or maybe moments after I send it, that solution will come to me.
> Things often work out that way. :)
How about the patch below instead? It's a bigger change to the output
format for HTML, but as a side effect it fixes the parallel build.
I considered removing a2x for the manpage output as well, but Asciidoc
can't generate that directly so we'd have to build the documentation
pipeline ourselves. Git does that, but I don't think it's worth the
effort here because we don't need to customise the process.
-- >8 --
Subject: [PATCH] Makefile: drive asciidoc directly for HTML output
This is mostly taken from Git's doc/Makefile, although simplified for
our use. The output now uses Asciidoc's default CSS which I think looks
a bit nicer than the Docbook formatting; as a result of this we no
longer need our custom .css file.
A side effect of this change is that temporary files generated from the
HTML output no longer conflict with the manpage output format (because
any temporary HTML output files use names derived from the output
filename which includes .html).
---
Makefile | 9 -
cgit-doc.css | 3 ---
2 files changed, 8 insertions(+), 4 deletions(-)
delete mode 100644 cgit-doc.css
diff --git a/Makefile b/Makefile
index 687069f..70f32a4 100644
--- a/Makefile
+++ b/Makefile
@@ -24,6 +24,12 @@ DOC_MAN5 = $(patsubst %.txt,%,$(MAN5_TXT))
DOC_HTML = $(patsubst %.txt,%.html,$(MAN_TXT))
DOC_PDF = $(patsubst %.txt,%.pdf,$(MAN_TXT))
+ASCIIDOC = asciidoc
+ASCIIDOC_EXTRA =
+ASCIIDOC_HTML = xhtml11
+ASCIIDOC_COMMON = $(ASCIIDOC) $(ASCIIDOC_EXTRA)
+TXT_TO_HTML = $(ASCIIDOC_COMMON) -b $(ASCIIDOC_HTML)
+
# Define NO_C99_FORMAT if your formatted IO functions (printf/scanf et.al.)
# do not support the 'size specifiers' introduced by C99, namely ll, hh,
# j, z, t. (representing long long int, char, intmax_t, size_t, ptrdiff_t).
@@ -134,7 +140,8 @@ doc-pdf: $(DOC_PDF)
a2x -f manpage $<
$(DOC_HTML): %.html : %.txt
- a2x -f xhtml --stylesheet=cgit-doc.css --xsltproc-opts="--param
generate.consistent.ids 1" $<
+ $(TXT_TO_HTML) -o $@+ $< && \
+ mv $@+ $@
$(DOC_PDF): %.pdf : %.txt
a2x -f pdf cgitrc.5.txt
diff --git a/cgit-doc.css b/cgit-doc.css
deleted file mode 100644
index 5a399b6..000
--- a/cgit-doc.css
+++ /dev/null
@@ -1,3 +0,0 @@
-div.variablelist dt {
- margin-top: 1em;
-}
--
2.17.1
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [RFC PATCH] Makefile: work around parallel make issues in docs
Hey John, That seems like a reasonable approach. Feel free to queue it up in jk/for-jason. Jason ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH 1/2] gcc8.1: fix strncpy bounds warnings
On Sat, Jun 16, 2018 at 09:12:08PM +0800, Andy Green wrote: > > > On June 16, 2018 9:04:48 PM GMT+08:00, John Keeping > wrote: > >On Wed, Jun 13, 2018 at 07:33:59AM +0800, Andy Green wrote: > >> These warnings are coming on default Fedora 28 build and probably > >others using gcc 8.1 > >> > >> ../shared.c: In function ‘expand_macro’: > >> ../shared.c:483:3: warning: ‘strncpy’ specified bound depends on the > >length of the source argument [-Wstringop-overflow=] > >>strncpy(name, value, len); > >>^ > >> ../shared.c:480:9: note: length computed here > >>len = strlen(value); > >> ^ > >> > >> strncpy with a computed length via strlen is usually > >> not the right thing. > >> > >> ../ui-shared.c: In function ‘cgit_repobasename’: > >> ../ui-shared.c:135:2: warning: ‘strncpy’ specified bound 1024 equals > >destination size [-Wstringop-truncation] > >> strncpy(rvbuf, reponame, sizeof(rvbuf)); > >> ^~~ > >> > >> add one char of padding and adjust so the code does the same. > >> > >> Signed-off-by: Andy Green > >> --- > >> shared.c|2 +- > >> ui-shared.c |7 --- > >> 2 files changed, 5 insertions(+), 4 deletions(-) > >> > >> diff --git a/shared.c b/shared.c > >> index 21ac8f4..477db0a 100644 > >> --- a/shared.c > >> +++ b/shared.c > >> @@ -480,7 +480,7 @@ static char *expand_macro(char *name, int > >maxlength) > >>len = strlen(value); > >>if (len > maxlength) > >>len = maxlength; > >> - strncpy(name, value, len); > >> + memcpy(name, value, len); > > > >This is a change in behaviour because strncpy is guaranteed to null > >terminate the output (even writing one beyond len if necessary) whereas > >memcpy does not. > > Eh... are you sure about that? It's not my understanding, and ---> > > https://linux.die.net/man/3/strncpy > > The strncpy() function is similar, except that at most n bytes of src > are copied. Warning: If there is no null byte among the first n bytes > of src, the string placed in dest will not be null-terminated. Yes, I'm getting it confused with strncat. And in this case we do ensure that the output is null terminated separately. ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [RFC PATCH] Makefile: work around parallel make issues in docs
Todd Zullinger : > When make is run with multiple jobs, doc-man and doc-html fail. The > a2x command tries to write %.5.xml for each invocation, overwriting > each other. > > Work around this by copying %.5 to %.5+ in doc-man. This is a rather > gross hack, but nothing better came to mind. Using --asciidoc-opts to > pass the '--out-file' did not affect the temporary .xml file at issue. What about using a --destination-dir (-D) to a2x? It puts the temporary .xml also into that dir. Something like this: diff --git a/Makefile b/Makefile index 687069f..2ccee34 100644 --- a/Makefile +++ b/Makefile @@ -131,7 +131,10 @@ doc-html: $(DOC_HTML) doc-pdf: $(DOC_PDF) %.5 : %.5.txt - a2x -f manpage $< + mkdir $@.dir + a2x -f manpage -D $@.dir $< + mv $@.dir/$@ . + rmdir $@.dir $(DOC_HTML): %.html : %.txt a2x -f xhtml --stylesheet=cgit-doc.css --xsltproc-opts="--param generate.consistent.ids 1" $< One might want to additionally clean up the directory somewhere (as it is not removed in case something goes wrong with the a2x call). - Thomas ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH 1/2] gcc8.1: fix strncpy bounds warnings
On Wed, Jun 13, 2018 at 07:33:59AM +0800, Andy Green wrote:
> These warnings are coming on default Fedora 28 build and probably others
> using gcc 8.1
>
> ../shared.c: In function ‘expand_macro’:
> ../shared.c:483:3: warning: ‘strncpy’ specified bound depends on the length
> of the source argument [-Wstringop-overflow=]
>strncpy(name, value, len);
>^
> ../shared.c:480:9: note: length computed here
>len = strlen(value);
> ^
>
> strncpy with a computed length via strlen is usually
> not the right thing.
>
> ../ui-shared.c: In function ‘cgit_repobasename’:
> ../ui-shared.c:135:2: warning: ‘strncpy’ specified bound 1024 equals
> destination size [-Wstringop-truncation]
> strncpy(rvbuf, reponame, sizeof(rvbuf));
> ^~~
>
> add one char of padding and adjust so the code does the same.
>
> Signed-off-by: Andy Green
> ---
> shared.c|2 +-
> ui-shared.c |7 ---
> 2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/shared.c b/shared.c
> index 21ac8f4..477db0a 100644
> --- a/shared.c
> +++ b/shared.c
> @@ -480,7 +480,7 @@ static char *expand_macro(char *name, int maxlength)
> len = strlen(value);
> if (len > maxlength)
> len = maxlength;
> - strncpy(name, value, len);
> + memcpy(name, value, len);
This is a change in behaviour because strncpy is guaranteed to null
terminate the output (even writing one beyond len if necessary) whereas
memcpy does not.
But I think we can improve this by removing the fixed buffer completely
and using struct strbuf to build the value (then return the allocated
buffer and rely on the caller to free it). I'll follow up with a couple
of patches that make this change.
> }
> return name + len;
> }
> diff --git a/ui-shared.c b/ui-shared.c
> index 9d8f66b..6656bd5 100644
> --- a/ui-shared.c
> +++ b/ui-shared.c
> @@ -129,11 +129,12 @@ char *cgit_pageurl(const char *reponame, const char
> *pagename,
> const char *cgit_repobasename(const char *reponame)
> {
> /* I assume we don't need to store more than one repo basename */
> - static char rvbuf[1024];
> + static char rvbuf[1025];
This is just an arbitrary size, so I think it can stay at 1024.
However, again, I think there's a better way to do this! We don't need
to copy the full reponame and modify it, why not figure out the start
and end of the basename in reponame and then strncpy the relevant
substring into rvbuf?
> int p;
> const char *rv;
> - strncpy(rvbuf, reponame, sizeof(rvbuf));
> - if (rvbuf[sizeof(rvbuf)-1])
> +
> + strncpy(rvbuf, reponame, sizeof(rvbuf) - 1);
> + if (rvbuf[sizeof(rvbuf) - 2])
> die("cgit_repobasename: truncated repository name '%s'",
> reponame);
> p = strlen(rvbuf)-1;
> /* strip trailing slashes */
>
> ___
> 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: [PATCH 06/11] ui-tree: free read_sha1_file() buffer after use
On Wed, Jun 13, 2018 at 10:02:00AM +0800, Andy Green wrote:
> Free up the buffer allocated in read_sha1_file()
>
> Signed-off-by: Andy Green
Reviewed-by: John Keeping
I've extracted this from the series and pushed it on my for-jason
branch: https://git.zx2c4.com/cgit/log/?h=jk/for-jason
> ---
> ui-tree.c |1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/ui-tree.c b/ui-tree.c
> index 723e16e..fe5dc75 100644
> --- a/ui-tree.c
> +++ b/ui-tree.c
> @@ -222,6 +222,7 @@ static void print_object(const unsigned char *sha1, char
> *path, const char *base
> }
>
> free(mimetype);
> + free(buf);
> }
>
> struct single_tree_ctx {
>
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: Rendering of README.md inline with inner tree view dirs
On Wed, Jun 13, 2018 at 09:47:38AM +0800, Andy Green wrote: > > > On 06/12/2018 05:31 PM, John Keeping wrote: > > On Tue, Jun 12, 2018 at 01:53:27PM +0800, Andy Green wrote: > >> On 06/11/2018 11:38 PM, John Keeping wrote: > >>> On Mon, Jun 11, 2018 at 04:05:38PM +0800, Andy Green wrote: > > >> 1) I did not attempt to have it learn about suitable filenames from the > >> config yet, I just have ls_item() looking for "README.md". I saw > >> there's already a way (readme=) for the config to list them for the > >> about page... basically now tree view becomes a superset of the > >> operation of the about page; the about page content appears in tree view. > >> > >> So do you have any thoughts about just re-using that list? > > > > I think that list is refs to blobs, so you can specify something like: > > > > refs/heads/wiki:README.md > > > > but here we need base filenames to look for in the current directory, so > > it will have to be a new config variable. > > OK. I added a new cgit-scope variable that appends filenames to be > considered for showing in tree view, eg, > > tree-readme=README.md This should be overridable at the repo level to match our other configuration variables which affect this sort of behaviour. Also, if this was my bikeshed I'd paint it as "inline-readme", but I don't feel strongly about that. > >> 2) In the current patches, I allowed for ls_item to discover a > >> linked-list of files and render them later one after the other. Eg, a > >> dir of READMEs would render them like that. It's welcome or preferable > >> to just restrict it to one? > > > > My choice would be to take the first matching file, but I don't have a > > strong opinion on what is the right behaviour. > > I'll leave this as a list for the first try. If it looks excessive I'll > reduce it to just the one. The code is definitely simpler if it's just one, and I'm not sure what benefit we get from including multiple files. But if you have a use case for multiple readme files, then a list is okay. > >> 3) You can see on the top level of the tree, the README.md references > >> > >> > >> > >> This url format works in github. In the cgit About view, this resolves to > >> > >> /git/libwebsockets/about/doc-assets/lws-overview.png > >> > >> which also serves the right mimetype and content. So that kind of URL > >> format is useful. But when we render the same markup and relative path > >> via /tree/, it tries to show an html page containing the content. > >> That's why the picture is missing in the /tree/ view... other pictures > >> in that markup are coming with absolute URLs outside of cgit and are > >> working. > >> > >> I can have the direct content from cgit generally, but either the markup > >> needs fixing up to > >> > >> /git/libwebsockets/plain/doc-assets/lws-overview.png > >> > >> or /tree/ needs to learn to do what /about/ does. > >> > >> I'm wondering whether mmd2html might grow an environment var to know the > >> base part for URLS that want to direct-render from cgit. Or if better > >> to follow what /about/ did in /tree/. > > > > Making tree do this will break the normal use of tree unless we add some > > extra query parameter or path element. Given that, I think teaching the > > renderer to use a path to /about/ is the right thing to do. > > OK. Unfortunately I don't know python very well. It looks like the > markdown python library is able to be told to use extensions that are > capable to do this > > https://python-markdown.github.io/extensions/api/ > > from the md2html wrapper. But I don't know enough python to do it. > > It's a shame, because in-tree assets correctly follow the ref context > being viewed, eg, if you look at a v2 branch you see v2 pngs, master you > see master pngs etc. > > I'll "solve" this part for now by changing the README to use external URLs. Yeah, I think we have to solve it by having the filter apply a mapping. We have ui-plain which provides the right content for images, but what should we do for link targets? For the purpose of discussion, consider the following HTML fragment that could be generated by rendering a README file: For more details see the dedicated data flow document. If dataflow.html is generated from a source file in a similar way, then it doesn't exist in the repository and we can't link to it, so the ideal output ends up being something like: For more details see the dedicated data flow document. The render filter API isn't finalised yet, so we can change the parameters that are passed in order to add more information for the renderer to use. At the very least I think we should add a parameter for the asset prefix which is essentially the tree path with /tree/ replaces with /plain/. However, I'm not sure how to handle relative links: do we need to pass additional parameters for this? Or can we rely on a render filter doing the right thing? Regards, John
Re: [PATCH 05/11] ui-tree: use render fileters to display content
If you're including these patches in your series, please fix my typo in
the subject ("fileters" has a stray 'e') :-)
On Wed, Jun 13, 2018 at 10:01:55AM +0800, Andy Green wrote:
> From: John Keeping
>
> This allows applying filters to files in the repository, for example to
> render Markdown or AsciiDoc as HTML.
>
> Signed-off-by: John Keeping
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH 1/2] gcc8.1: fix strncpy bounds warnings
On June 16, 2018 9:04:48 PM GMT+08:00, John Keeping wrote:
>On Wed, Jun 13, 2018 at 07:33:59AM +0800, Andy Green wrote:
>> These warnings are coming on default Fedora 28 build and probably
>others using gcc 8.1
>>
>> ../shared.c: In function ‘expand_macro’:
>> ../shared.c:483:3: warning: ‘strncpy’ specified bound depends on the
>length of the source argument [-Wstringop-overflow=]
>>strncpy(name, value, len);
>>^
>> ../shared.c:480:9: note: length computed here
>>len = strlen(value);
>> ^
>>
>> strncpy with a computed length via strlen is usually
>> not the right thing.
>>
>> ../ui-shared.c: In function ‘cgit_repobasename’:
>> ../ui-shared.c:135:2: warning: ‘strncpy’ specified bound 1024 equals
>destination size [-Wstringop-truncation]
>> strncpy(rvbuf, reponame, sizeof(rvbuf));
>> ^~~
>>
>> add one char of padding and adjust so the code does the same.
>>
>> Signed-off-by: Andy Green
>> ---
>> shared.c|2 +-
>> ui-shared.c |7 ---
>> 2 files changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/shared.c b/shared.c
>> index 21ac8f4..477db0a 100644
>> --- a/shared.c
>> +++ b/shared.c
>> @@ -480,7 +480,7 @@ static char *expand_macro(char *name, int
>maxlength)
>> len = strlen(value);
>> if (len > maxlength)
>> len = maxlength;
>> -strncpy(name, value, len);
>> +memcpy(name, value, len);
>
>This is a change in behaviour because strncpy is guaranteed to null
>terminate the output (even writing one beyond len if necessary) whereas
>memcpy does not.
Eh... are you sure about that? It's not my understanding, and --->
https://linux.die.net/man/3/strncpy
The strncpy() function is similar, except that at most n bytes of src are
copied. Warning: If there is no null byte among the first n bytes of src, the
string placed in dest will not be null-terminated.
>But I think we can improve this by removing the fixed buffer completely
>and using struct strbuf to build the value (then return the allocated
>buffer and rely on the caller to free it). I'll follow up with a
>couple
>of patches that make this change.
I just did the minimal change to resolve the warning. If that's solution's
better for larger reasons by all means.
>> }
>> return name + len;
>> }
>> diff --git a/ui-shared.c b/ui-shared.c
>> index 9d8f66b..6656bd5 100644
>> --- a/ui-shared.c
>> +++ b/ui-shared.c
>> @@ -129,11 +129,12 @@ char *cgit_pageurl(const char *reponame, const
>char *pagename,
>> const char *cgit_repobasename(const char *reponame)
>> {
>> /* I assume we don't need to store more than one repo basename */
>> -static char rvbuf[1024];
>> +static char rvbuf[1025];
>
>This is just an arbitrary size, so I think it can stay at 1024.
>
>However, again, I think there's a better way to do this! We don't need
>to copy the full reponame and modify it, why not figure out the start
>and end of the basename in reponame and then strncpy the relevant
>substring into rvbuf?
Same as above, if you prefer a larger refactor rather than just fix the
warning, no worries.
-Andy
>> int p;
>> const char *rv;
>> -strncpy(rvbuf, reponame, sizeof(rvbuf));
>> -if (rvbuf[sizeof(rvbuf)-1])
>> +
>> +strncpy(rvbuf, reponame, sizeof(rvbuf) - 1);
>> +if (rvbuf[sizeof(rvbuf) - 2])
>> die("cgit_repobasename: truncated repository name '%s'",
>reponame);
>> p = strlen(rvbuf)-1;
>> /* strip trailing slashes */
>>
>> ___
>> 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: [PATCH 00/11] Render READMEs inline in tree view
On Thu, Jun 14, 2018 at 11:47:41AM +0800, Andy Green wrote: > On 06/13/2018 10:01 AM, Andy Green wrote: > > The following series adds config to allow rendering of > > selected READMEs inline after the tree view, where > > present in the directory being viewed. > > > > It builds on John Keeping's RENDER mode series from 2016. > > > > Typical config to enable it, if you have a README.md > > looks like > > > > tree-readme=README.md > > render.md=/usr/libexec/cgit/filters/html-converters/md2html > > I have updated this against Christian's git 2.18.0-rc2 patch from > earlier, rather than post it to the list again before any comment you > can find it here if you're interested. > > https://warmcat.com/git/cgit/ > > Presumably this is what should be targeted for the next push rather than > current master. I think that makes sense, yes. Git 2.18.0 final is due out tomorrow [1] so we should get the version bump committed pretty soon. [1] https://tinyurl.com/gitcal ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH] Encode value and field before calculating cookie digest, the same way secure_value() does
On Thu, Apr 12, 2018 at 08:54:31PM +0300, thev...@gmail.com wrote:
> From: Vlad Safronov
>
> Bugfix: Encode value and field before calculating cookie digest, the same way
> as secure_value() does
> so validating will work correctly on encoded values.
Missing sign-off (see [1] for what this means).
[1] https://elinux.org/Developer_Certificate_Of_Origin
However, I don't think this change is correct. secure_value() places
the encoded strings into the authstr, which is what validate_value() is
reading, so field and value are already URL encoded here.
This is why field is url_decode()'d later in this function before
comparing with the expected value.
> ---
> filters/simple-authentication.lua | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/filters/simple-authentication.lua
> b/filters/simple-authentication.lua
> index de34d09..b40a819 100644
> --- a/filters/simple-authentication.lua
> +++ b/filters/simple-authentication.lua
> @@ -230,6 +230,8 @@ function validate_value(expected_field, cookie)
> return nil
> end
>
> + value = url_encode(value)
> + field = url_encode(field)
> -- Lua hashes strings, so these comparisons are time invariant.
> if hmac ~= crypto.hmac.digest("sha1", field .. "|" .. value .. "|" ..
> tostring(expiration) .. "|" .. salt, secret) then
> return nil
> --
> 2.17.0
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: Rendering of README.md inline with inner tree view dirs
On Sat, Jun 16, 2018 at 03:12:09PM +0100, John Keeping wrote:
> > >> 3) You can see on the top level of the tree, the README.md references
> > >>
> > >>
> > >>
> > >> This url format works in github. In the cgit About view, this resolves
> > >> to
> > >>
> > >> /git/libwebsockets/about/doc-assets/lws-overview.png
> > >>
> > >> which also serves the right mimetype and content. So that kind of URL
> > >> format is useful. But when we render the same markup and relative path
> > >> via /tree/, it tries to show an html page containing the content.
> > >> That's why the picture is missing in the /tree/ view... other pictures
> > >> in that markup are coming with absolute URLs outside of cgit and are
> > >> working.
> > >>
> > >> I can have the direct content from cgit generally, but either the markup
> > >> needs fixing up to
> > >>
> > >> /git/libwebsockets/plain/doc-assets/lws-overview.png
> > >>
> > >> or /tree/ needs to learn to do what /about/ does.
> > >>
> > >> I'm wondering whether mmd2html might grow an environment var to know the
> > >> base part for URLS that want to direct-render from cgit. Or if better
> > >> to follow what /about/ did in /tree/.
> > >
> > > Making tree do this will break the normal use of tree unless we add some
> > > extra query parameter or path element. Given that, I think teaching the
> > > renderer to use a path to /about/ is the right thing to do.
> >
> > OK. Unfortunately I don't know python very well. It looks like the
> > markdown python library is able to be told to use extensions that are
> > capable to do this
> >
> > https://python-markdown.github.io/extensions/api/
> >
> > from the md2html wrapper. But I don't know enough python to do it.
> >
> > It's a shame, because in-tree assets correctly follow the ref context
> > being viewed, eg, if you look at a v2 branch you see v2 pngs, master you
> > see master pngs etc.
> >
> > I'll "solve" this part for now by changing the README to use external URLs.
>
> Yeah, I think we have to solve it by having the filter apply a mapping.
> We have ui-plain which provides the right content for images, but what
> should we do for link targets?
>
> For the purpose of discussion, consider the following HTML fragment that
> could be generated by rendering a README file:
>
>
> For more details see the dedicated
> data flow document.
>
> If dataflow.html is generated from a source file in a similar way, then
> it doesn't exist in the repository and we can't link to it, so the ideal
> output ends up being something like:
>
>
> For more details see the dedicated
> data flow document.
>
> The render filter API isn't finalised yet, so we can change the
> parameters that are passed in order to add more information for the
> renderer to use. At the very least I think we should add a parameter
> for the asset prefix which is essentially the tree path with /tree/
> replaces with /plain/.
>
> However, I'm not sure how to handle relative links: do we need to pass
> additional parameters for this? Or can we rely on a render filter doing
> the right thing?
Modifying md2html to use the extension API is reasonably
straightforward. Below is a modified version which remaps the "src"
attribute on elements according to a second command line argument,
you can try it out with:
md2html 8 --
#!/usr/bin/env python3
import markdown
import sys
import io
from markdown.util import etree
from pygments.formatters import HtmlFormatter
from urllib.parse import urljoin
class AssetMappingProcessor(markdown.treeprocessors.Treeprocessor):
def __init__(self, asset_prefix):
self.asset_prefix = asset_prefix
def run(self, root):
asset_prefix = self.asset_prefix
for img in root.iter('img'):
src = img.get('src')
if src is None:
continue
img.set('src', urljoin(asset_prefix, src))
class AssetMappingExtension(markdown.extensions.Extension):
def __init__(self, **kwargs):
self.config = {'asset_prefix': ['', 'prefix for relative asset URLs']}
super(AssetMappingExtension, self).__init__(**kwargs)
def extendMarkdown(self, md, md_globals):
asset_prefix = self.getConfig('asset_prefix')
if not asset_prefix:
return
md.treeprocessors.add('asset_mapping',
AssetMappingProcessor(asset_prefix),
'_end')
sys.stdin = io.TextIOWrapper(sys.stdin.buffer, encoding='utf-8')
sys.stdout = io.TextIOWrapper(sys.stdout.buffer, encoding='utf-8')
sys.stdout.write('''
.markdown-body {
font-size: 14px;
line-height: 1.6;
overflow: hidden;
}
.markdown-body>*:first-child {
margin-top: 0 !important;
}
.markdown-body>*:last-child {
margin-bottom: 0 !important;
}
.markdown-body a.absent {
color: #c00;
}
.markdown-body a.anchor {
display: block;
padding-left: 30px;
margin-left: -30px;
Re: [PATCH 05/11] ui-tree: use render fileters to display content
On 06/16/2018 10:26 PM, John Keeping wrote:
If you're including these patches in your series, please fix my typo in
the subject ("fileters" has a stray 'e') :-)
OK.
On Wed, Jun 13, 2018 at 10:01:55AM +0800, Andy Green wrote:
From: John Keeping
This allows applying filters to files in the repository, for example to
render Markdown or AsciiDoc as HTML.
Signed-off-by: John Keeping
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH 07/11] ui-blame: free read_sha1_file() buffer after use
On 06/16/2018 10:23 PM, John Keeping wrote:
On Wed, Jun 13, 2018 at 10:02:05AM +0800, Andy Green wrote:
Signed-off-by: Andy Green
---
ui-blame.c |3 +++
1 file changed, 3 insertions(+)
diff --git a/ui-blame.c b/ui-blame.c
index 17e2d60..a5c7d69 100644
--- a/ui-blame.c
+++ b/ui-blame.c
@@ -206,6 +206,9 @@ static void print_object(const unsigned char *sha1, const
char *path,
There's an early return above here after allocating buf, I'm not sure if
we care about freeing buf on that path as well.
If we do, I guess we push free(buf) to the end and add a free_buf label
for the error path to jump to.
Yes, I missed it thanks. I adapted it with the label.
} else {
html_txt(buf);
}
+
+ free(buf);
+
html("");
html("\n");
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH 1/1] RFC: git: update to v2.18.0-rc2
On Sat, Jun 16, 2018 at 6:14 PM John Keeping wrote: > But we still have a few mentions of sha1 in our code. Most of this is > sha1 and sha2 in struct cgit_query, but these aren't actually hashes > most of the time, they can be any object reference that Git understands, > so I think we should rename them to ref1 and ref2 (along with has_sha1 > -> has_ref). What do you think? Yes, please. This is some excellent progress. ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [RFC PATCH] Makefile: work around parallel make issues in docs
Thomas Moschny wrote: > Todd Zullinger : > >> When make is run with multiple jobs, doc-man and doc-html fail. The >> a2x command tries to write %.5.xml for each invocation, overwriting >> each other. >> >> Work around this by copying %.5 to %.5+ in doc-man. This is a rather >> gross hack, but nothing better came to mind. Using --asciidoc-opts to >> pass the '--out-file' did not affect the temporary .xml file at issue. > > What about using a --destination-dir (-D) to a2x? It puts the > temporary .xml also into that dir. Something like this: > > diff --git a/Makefile b/Makefile > index 687069f..2ccee34 100644 > --- a/Makefile > +++ b/Makefile > @@ -131,7 +131,10 @@ doc-html: $(DOC_HTML) > doc-pdf: $(DOC_PDF) > > %.5 : %.5.txt > - a2x -f manpage $< > + mkdir $@.dir > + a2x -f manpage -D $@.dir $< > + mv $@.dir/$@ . > + rmdir $@.dir > > $(DOC_HTML): %.html : %.txt > a2x -f xhtml --stylesheet=cgit-doc.css --xsltproc-opts="--param > generate.consistent.ids 1" $< > > > One might want to additionally clean up the directory somewhere (as it > is not removed in case something goes wrong with the a2x call). Yeah, that's a nice solution too. Thanks! It's really a2x's fault for stripping the extension and replacing it with .xml rather than just appending .xml to the target name. (But perhaps someone on Windows doesn't want to see a temp file named .html.xml or something.) -- Todd ~~ A cynic is a man who, when he smells flowers, looks around for a coffin. -- H. L. Mencken ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH 1/1] snapshot: support special value 'all' to enable all formats
On Thu, Jun 07, 2018 at 10:05:50PM +0200, Christian Hesse wrote: > From: Christian Hesse > > Signed-off-by: Christian Hesse Reviewed-by: John Keeping > --- > cgitrc.5.txt | 1 + > shared.c | 3 +++ > 2 files changed, 4 insertions(+) > > diff --git a/cgitrc.5.txt b/cgitrc.5.txt > index a9d3d0a..3bfacfa 100644 > --- a/cgitrc.5.txt > +++ b/cgitrc.5.txt > @@ -429,6 +429,7 @@ snapshots:: > Text which specifies the default set of snapshot formats that cgit > generates links for. The value is a space-separated list of zero or > more of the values "tar", "tar.gz", "tar.bz2", "tar.xz" and "zip". > + The special value "all" enables all snapshot formats. > Default value: none. > > source-filter:: > diff --git a/shared.c b/shared.c > index 21ac8f4..0a11e68 100644 > --- a/shared.c > +++ b/shared.c > @@ -390,6 +390,9 @@ int cgit_parse_snapshots_mask(const char *str) > if (atoi(str)) > return 1; > > + if (strcmp(str, "all") == 0) > + return INT_MAX; > + > string_list_split(, str, ' ', -1); > string_list_remove_empty_items(, 0); > ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH] ui-tag: Fix inconsistent capitalization
On Sun, Jun 10, 2018 at 06:28:49PM -0400, Jon DeVree wrote:
> Way back in 2009 all of these were lower cased except this one
> occurrence.
>
> Signed-off-by: Jon DeVree
Thanks! I've picked this up in jk/for-jason.
> ---
> ui-tag.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/ui-tag.c b/ui-tag.c
> index 909cde0..2c216d0 100644
> --- a/ui-tag.c
> +++ b/ui-tag.c
> @@ -107,7 +107,7 @@ void cgit_print_tag(char *revname)
> htmlf("tag name");
> html_txt(revname);
> html("\n");
> - html("Tagged object");
> + html("tagged object");
> cgit_object_link(obj);
> html("\n");
> if (ctx.repo->snapshots)
> --
> 2.17.1
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: Bug: cgit shows empty pages, when viewing Tag-Only-Repo
On Fri, May 04, 2018 at 08:20:10AM +0200, Björn Lässig wrote:
> I use repositories for holding some tags.
> Unfortunately cgit shows only
>
> Repository seems to be empty
>
> I looked into the code and saw the problem, but i have not the insight
> to fix this.
>
> one (small) example for the Problem is:
>
> https://git.pengutronix.de/cgit/ukl/distrokit/refs/
>
> git ls-remote https://git.pengutronix.de/git/ukl/distrokit/
> shows a handful of refs.
I had a related patch hanging around which improves the behaviour when
the default branch doesn't exist, and it is easy to expand this to pick
a default ref when there are no branches.
I'm not sure if I sent this to the list in the past; certainly there are
arguments as to why this behaviour isn't desirable.
The requirement for a default ref is baked in throughout the CGit code
base and I don't think changing that is worthwhile, but providing we can
pick some ref as the default, it doesn't necessarily have to be a
branch. So this patch allows any ref to be the default (albeit with a
strong preference for branches if any exist).
-- >8 --
Subject: [PATCH] cgit: improve default branch selection
If the named default branch does not exist (for example because none is
specified in the config and "master" is not used in the repositry), then
we currently select the first branch in whatever order Git returns them
to us.
Instead, let's use the newest branch (by commit date), since this is
likely to be the most interesting branch in the repository. This does
make default branch selection more expensive, but people who care should
specify a default branch in their repository configuration.
Further, if a repository has no branches but does have tags, let's pick
the newest tag as the default head [1]. This is a separate pass because
we want to prefer branches if there are any.
[1] In fact, we allow any ref at this point because we want to avoid
falsely claiming that the repository is empty when it isn't.
Signed-off-by: John Keeping
---
cgit.c | 46 +++---
1 file changed, 35 insertions(+), 11 deletions(-)
diff --git a/cgit.c b/cgit.c
index bd9cb3f..43b8c96 100644
--- a/cgit.c
+++ b/cgit.c
@@ -430,7 +430,6 @@ static void prepare_context(void)
struct refmatch {
char *req_ref;
- char *first_ref;
int match;
};
@@ -442,34 +441,59 @@ static int find_current_ref(const char *refname, const
struct object_id *oid,
info = (struct refmatch *)cb_data;
if (!strcmp(refname, info->req_ref))
info->match = 1;
- if (!info->first_ref)
- info->first_ref = xstrdup(refname);
return info->match;
}
-static void free_refmatch_inner(struct refmatch *info)
+struct newest_ref {
+ char *ref;
+ unsigned long date;
+};
+
+static int find_newest_ref(const char *refname, const struct object_id *oid,
+ int flags, void *cb_data)
{
- if (info->first_ref)
- free(info->first_ref);
+ struct newest_ref *newest = cb_data;
+ struct object *object;
+ unsigned long date;
+
+ object = parse_object(oid);
+ if (object->type == OBJ_COMMIT) {
+ date = ((struct commit *)object)->date;
+ if (date > newest->date) {
+ free(newest->ref);
+ newest->ref = xstrdup(refname);
+ newest->date = date;
+ }
+ }
+ return 0;
}
static char *find_default_branch(struct cgit_repo *repo)
{
struct refmatch info;
+ struct newest_ref newest = { NULL };
char *ref;
info.req_ref = repo->defbranch;
- info.first_ref = NULL;
info.match = 0;
for_each_branch_ref(find_current_ref, );
- if (info.match)
+ if (info.match) {
ref = info.req_ref;
- else
- ref = info.first_ref;
+ } else {
+ /*
+* If the default branch isn't found, pick the most recently
+* updated branch, and if there are no branches, check if
+* this is a tag-only repo and pick any ref we can.
+*/
+ for_each_branch_ref(find_newest_ref, );
+ if (!newest.ref)
+ for_each_ref(find_newest_ref, );
+ ref = newest.ref;
+ }
if (ref)
ref = xstrdup(ref);
- free_refmatch_inner();
+ free(newest.ref);
return ref;
}
--
2.17.1
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: Adding short commit id to repo Log tab
Sorry for the delay responding. On Wed, Apr 11, 2018 at 12:00:28PM +0300, Vlad wrote: > I think adding short (7 chars) commit id to repository Log tab output > might be more useful than file or line count.. > > I have examined the code, it it not easy to add an extra column > to the output. > > Would you suggest ways how to implement this? I'm not sure I agree that this is useful, but personally I wouldn't be opposed to adding this if the code is simple. I don't think it's hard to do, you just need to add a new element in cgit_print_log() and then write the element in print_commit(), making sure that columns is also adjusted. All of the code should be in ui-log.c unless you add a config switch to enable this feature.. ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: Translation
On Mon, Jun 04, 2018 at 10:24:24AM +0200, Malte Kiefer wrote: > first of all I want to say thank you for this amazing software. > I start to using it, and I really like it. > > Then I wanted to inquire with you if you think about offering cgit in > multiple languages? > I would like to translate cgit into german. There's some preparatory work to be done to make CGit translatable. Patches to do this would be welcome, but I don't think anyone currently has plans to work on this. At a minimum, we need patches to: - Mark user-visible strings in the source as translated and generate translation catalogs - Integrate translations into the build and install process so that they can be found and loaded when handling a request - Extract preferred languages from the request and initialise gettext with the most preferred language that is available ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH 10/11] config: add tree-readme list
On Wed, Jun 13, 2018 at 10:02:20AM +0800, Andy Green wrote:
> Allows the user to specify a list of filenames that should be
> rendered inline with tree view, if present in the directory.
>
> Signed-off-by: Andy Green
As mentioned in reply to the cover leter, I think this needs to be a
repo config rather than just a global configuration. It seems likely
that different repositories may have different preferences or
conventions for directory level readme files.
A few more comments below...
> ---
> cgit.c | 11 ++-
> cgit.h |1 +
> cgitrc.5.txt |7 +++
> 3 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/cgit.c b/cgit.c
> index 975e573..017ce78 100644
> --- a/cgit.c
> +++ b/cgit.c
> @@ -1,6 +1,6 @@
> /* cgit.c: cgi for the git scm
> *
> - * Copyright (C) 2006-2014 cgit Development Team
> + * Copyright (C) 2006-2018 cgit Development Team
> *
> * Licensed under GNU General Public License v2
> * (see COPYING for full license text)
> @@ -39,6 +39,12 @@ static void add_render_filter(const char *name, const char
> *cmd)
> item->util = filter;
> }
>
> +static void add_tree_readme(const char *name)
> +{
> + string_list_insert(_readme, name);
> +}
> +
> +
Extra blank line?
> static void process_cached_repolist(const char *path);
>
> static void repo_config(struct cgit_repo *repo, const char *name, const char
> *value)
> @@ -301,6 +307,8 @@ static void config_cb(const char *name, const char *value)
> add_mimetype(name + 9, value);
> else if (starts_with(name, "render."))
> add_render_filter(name + 7, value);
> + else if (!strcmp(name, "tree-readme"))
> + add_tree_readme(value);
> else if (!strcmp(name, "include"))
> parse_configfile(expand_macros(value), config_cb);
> }
> @@ -435,6 +443,7 @@ static void prepare_context(void)
> ctx.page.etag = NULL;
> string_list_init(, 1);
> string_list_init(_filters, 1);
> + string_list_init(_readme, 1);
> if (ctx.env.script_name)
> ctx.cfg.script_name = xstrdup(ctx.env.script_name);
> if (ctx.env.query_string)
> diff --git a/cgit.h b/cgit.h
> index a19742f..1076568 100644
> --- a/cgit.h
> +++ b/cgit.h
> @@ -261,6 +261,7 @@ struct cgit_config {
> int branch_sort;
> int commit_sort;
> struct string_list mimetypes;
> + struct string_list tree_readme;
> struct string_list render_filters;
> struct cgit_filter *about_filter;
> struct cgit_filter *commit_filter;
> diff --git a/cgitrc.5.txt b/cgitrc.5.txt
> index 793a0c1..597 100644
> --- a/cgitrc.5.txt
> +++ b/cgitrc.5.txt
> @@ -469,6 +469,13 @@ strict-export::
> repositories to match those exported by git-daemon. This option must
> be defined prior to scan-path.
>
> +tree-readme::
> + Append given filename to the list of filenames to be rendered after the
> + tree navigation in tree view, if present in the directory being viewed.
> Eg,
> + 'tree-readme=README.md'. There must also be a corresponding render.
> + entry for the readme suffix, eg,
> + 'render.md=/usr/libexec/cgit/filters/html-converters/md2html'
I don't think there does need to be a render entry, there could be a
mimetype entry which will allow the content to be displayed in an
iframe, or in the absense of that the file will simply be included
verbatim which may be absolutely fine.
> will also cause cgit to generate 'virtual urls', i.e. urls like
>
> ___
> 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: cache-size implementation downsides
On Wed, Jun 13, 2018 at 03:02:42PM -0400, Konstantin Ryabitsev wrote:
> 2. I have witnessed cache corruption due to collisions (which is
> a bug in itself). One of our frontends was hit by a lot of agressive
> crawling of snapshots that raised the load to 60+ (many, many gzip
> processes). After we blackholed the bot, some of the cache objects for
> non-snapshot URLs had trailing gzip junk in them, meaning that either
> two instances were writing to the same file, or something else resulted
> in cache corruption. This is probably a race condition somewhere in the
> locking code.
I've had a look at this, and I think we might end up dropping our lock
too early thanks to this code (in fill_slot()):
/* Restore stdout */
if (dup2(tmp, STDOUT_FILENO) == -1) {
Before this line, STDOUT_FILENO refers to lock_fd which has a POSIX
advisory record lock on the entire file. However, the documentation for
that says:
* If a process closes any file descriptor referring to a file, then all
of the process's locks on that file are released, regardless of the
file descriptor(s) on which the locks were obtained. This is
bad: it means that a process can lose its locks on a file such as
/etc/passwd or /etc/mtab when for some reason a library function
decides to open, read, and close the same file.
I haven't verified this, but I suspect that dup'ing the original stdout
over STDOUT_FILENO is equivalent to closing a file descriptor referring
to our lock file. And thus the lock is released at this point, which is
before we rename the lock file over the cache file.
If that is correct, then there is a window during which a new process
can open the lock file to write new content and successfully acquire the
lock on that file even though it is still being used by another process.
-- >8 --
Subject: [PATCH] cache: close race window when unlocking slots
We use POSIX advisory record locks to control access to cache slots, but
these have an unhelpful behaviour in that they are released when any
file descriptor referencing the file is closed by this process.
Mostly this is okay, since we know we won't be opening the lock file
anywhere else, but there is one place that it does matter: when we
restore stdout we dup2() over a file descriptor referring to the file,
thus closing that descriptor.
Since we restore stdout before unlocking the slot, this creates a window
during which the slot content can be overwritten. The fix is reasonably
straightforward: simply restore stdout after unlocking the slot, but the
diff is a bit bigger because this requires us to move the temporary
stdout FD into struct cache_slot.
Signed-off-by: John Keeping
---
cache.c | 37 ++---
1 file changed, 14 insertions(+), 23 deletions(-)
diff --git a/cache.c b/cache.c
index 0901e6e..2c70be7 100644
--- a/cache.c
+++ b/cache.c
@@ -29,6 +29,7 @@ struct cache_slot {
cache_fill_fn fn;
int cache_fd;
int lock_fd;
+ int stdout_fd;
const char *cache_name;
const char *lock_name;
int match;
@@ -197,6 +198,13 @@ static int unlock_slot(struct cache_slot *slot, int
replace_old_slot)
else
err = unlink(slot->lock_name);
+ /* Restore stdout and close the temporary FD. */
+ if (slot->stdout_fd >= 0) {
+ dup2(slot->stdout_fd, STDOUT_FILENO);
+ close(slot->stdout_fd);
+ slot->stdout_fd = -1;
+ }
+
if (err)
return errno;
@@ -208,42 +216,24 @@ static int unlock_slot(struct cache_slot *slot, int
replace_old_slot)
*/
static int fill_slot(struct cache_slot *slot)
{
- int tmp;
-
/* Preserve stdout */
- tmp = dup(STDOUT_FILENO);
- if (tmp == -1)
+ slot->stdout_fd = dup(STDOUT_FILENO);
+ if (slot->stdout_fd == -1)
return errno;
/* Redirect stdout to lockfile */
- if (dup2(slot->lock_fd, STDOUT_FILENO) == -1) {
- close(tmp);
+ if (dup2(slot->lock_fd, STDOUT_FILENO) == -1)
return errno;
- }
/* Generate cache content */
slot->fn();
/* Make sure any buffered data is flushed to the file */
- if (fflush(stdout)) {
- close(tmp);
+ if (fflush(stdout))
return errno;
- }
/* update stat info */
- if (fstat(slot->lock_fd, >cache_st)) {
- close(tmp);
- return errno;
- }
-
- /* Restore stdout */
- if (dup2(tmp, STDOUT_FILENO) == -1) {
- close(tmp);
- return errno;
- }
-
- /* Close the temporary filedescriptor */
- if (close(tmp))
+ if (fstat(slot->lock_fd, >cache_st))
return errno;
return 0;
@@ -393,6 +383,7 @@ int cache_process(int size, const char *path, const char
*key, int ttl,
Re: [PATCH 1/1] RFC: git: update to v2.18.0-rc2
John Keeping on Sat, 2018/06/16 17:14:
> With Andy's fix,
>
> Reviewed-by: John Keeping
That is in git 2.17.1 commit already. ;)
--
main(a){char*c=/*Schoene Gruesse */"B?IJj;MEH"
"CX:;",b;for(a/*Best regards my address:*/=0;b=c[a++];)
putchar(b-1/(/*Chriscc -ox -xc - && ./x*/b/42*2-3)*42);}
pgptF4XlvTbeG.pgp
Description: OpenPGP digital signature
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH] expand environmentvariables in root-title and root-descr
On Thu, Apr 12, 2018 at 11:19:12AM +0200, b.laes...@pengutronix.de wrote: > From: Björn Lässig > > For having personanlized cgit configuration i need to use > > root-desc=$REMOTE_USER@$HTTP_HOST Missing sign-off (see [1] for what this means). Also, this needs a corresponding change in cgitrc.5.txt to indicate that these variables are now subject to macro expansion. [1] https://elinux.org/Developer_Certificate_Of_Origin > --- > cgit.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/cgit.c b/cgit.c > index bd9cb3f..1d7d67e 100644 > --- a/cgit.c > +++ b/cgit.c > @@ -120,9 +120,9 @@ static void config_cb(const char *name, const char *value) > else if (!strcmp(name, "readme")) > string_list_append(, xstrdup(value)); > else if (!strcmp(name, "root-title")) > - ctx.cfg.root_title = xstrdup(value); > + ctx.cfg.root_title = xstrdup(expand_macros(value)); > else if (!strcmp(name, "root-desc")) > - ctx.cfg.root_desc = xstrdup(value); > + ctx.cfg.root_desc = xstrdup(expand_macros(value)); > else if (!strcmp(name, "root-readme")) > ctx.cfg.root_readme = xstrdup(value); > else if (!strcmp(name, "css")) > -- > 2.11.0 ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [RFC PATCH] Makefile: work around parallel make issues in docs
Hi Todd, On Sat, Jun 16, 2018 at 12:32:06PM -0400, Todd Zullinger wrote: > John Keeping wrote: > > How about the patch below instead? It's a bigger change to the output > > format for HTML, but as a side effect it fixes the parallel build. > > It does, but only if the targets are 'doc-man doc-html'. > For the default 'doc' target which includes 'doc-pdf', the > issue is still present. > > As a bonus, the output is much nicer, I think. > > We could perhaps work around the 'doc-pdf' failure by adding a > dep on '($DOC_MAN5)': > > -- >8 -- > diff --git i/Makefile w/Makefile > index 70f32a4..4879a5a 100644 > --- i/Makefile > +++ w/Makefile > @@ -143,7 +143,7 @@ $(DOC_HTML): %.html : %.txt > $(TXT_TO_HTML) -o $@+ $< && \ > mv $@+ $@ > > -$(DOC_PDF): %.pdf : %.txt > +$(DOC_PDF): %.pdf : $(DOC_MAN5) %.txt > a2x -f pdf cgitrc.5.txt > > clean: clean-doc > -- >8 -- > > That's hackish, no doubt. > > We might also want to drop 'doc-pdf' from the default 'doc' > target. The alternative is driving the asciidoc pipeline > for the pdf generation too. That looks a little more > involved than doing it for html, but perhaps it's not as bad > as I think. I think we can definitely drop doc-pdf from the default output. I'm half tempted to say we should just delete the PDF output completely and see if anyone complains, unless you know of anyone using this? Otherwise, the dependency on $(DOC_MAN5) seems reasonable to me, probably accompanied with a comment explaining the clash in a2x's intermediate files. Regards, John ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH 2/2] gcc8.1: fix strcat warning
John Keeping on Sat, 2018/06/16 14:11:
> On Wed, Jun 13, 2018 at 07:34:07AM +0800, Andy Green wrote:
> > ../ui-ssdiff.c: In function ‘replace_tabs’:
> > ../ui-ssdiff.c:142:4: warning: ‘strncat’ output truncated copying between
> > 1 and 8 bytes from a string of length 8 [-Wstringop-truncation]
> > strncat(result, spaces, 8 - (strlen(result) % 8));
> > ^
> >
> > Actually the strncat that was there before intends that its
> > stock of spaces gets truncated, and it's not a problem.
> >
> > However gcc8.1 is also right, normally truncation is undesirable.
> >
> > Make the code do the padding explicitly.
> >
> > Signed-off-by: Andy Green
>
> Reviewed-by: John Keeping
Agreed, except the typo in commit message. This is about strncat, not strcat.
--
main(a){char*c=/*Schoene Gruesse */"B?IJj;MEH"
"CX:;",b;for(a/*Best regards my address:*/=0;b=c[a++];)
putchar(b-1/(/*Chriscc -ox -xc - && ./x*/b/42*2-3)*42);}
pgpEm3JZgPnMd.pgp
Description: OpenPGP digital signature
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH 2/2] gcc8.1: fix strcat warning
On 06/17/2018 05:52 AM, Christian Hesse wrote: John Keeping on Sat, 2018/06/16 14:11: On Wed, Jun 13, 2018 at 07:34:07AM +0800, Andy Green wrote: ../ui-ssdiff.c: In function ‘replace_tabs’: ../ui-ssdiff.c:142:4: warning: ‘strncat’ output truncated copying between 1 and 8 bytes from a string of length 8 [-Wstringop-truncation] strncat(result, spaces, 8 - (strlen(result) % 8)); ^ Actually the strncat that was there before intends that its stock of spaces gets truncated, and it's not a problem. However gcc8.1 is also right, normally truncation is undesirable. Make the code do the padding explicitly. Signed-off-by: Andy Green Reviewed-by: John Keeping Agreed, except the typo in commit message. This is about strncat, not strcat. OK... I fixed that and dropped the first patch. ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH] Update COPYING
The address of the Free Software Foundation has changed since the
license was added in 7640d90 ("Add license file and copyright notices",
2006-12-10). Update the license file from gnu.org¹.
The only non-whitespace changes are the updated FSF address and two
references to the L in LGPL changed from Library to Lesser.
¹ https://www.gnu.org/licenses/old-licenses/gpl-2.0.txt
---
This will help users & distributors who download & package cgit.
COPYING | 39 +++
1 file changed, 19 insertions(+), 20 deletions(-)
diff --git a/COPYING b/COPYING
index 5b6e7c6..d159169 100644
--- a/COPYING
+++ b/COPYING
@@ -1,12 +1,12 @@
- GNU GENERAL PUBLIC LICENSE
- Version 2, June 1991
+GNU GENERAL PUBLIC LICENSE
+ Version 2, June 1991
- Copyright (C) 1989, 1991 Free Software Foundation, Inc.
- 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ Copyright (C) 1989, 1991 Free Software Foundation, Inc.,
+ 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
Everyone is permitted to copy and distribute verbatim copies
of this license document, but changing it is not allowed.
- Preamble
+Preamble
The licenses for most software are designed to take away your
freedom to share and change it. By contrast, the GNU General Public
@@ -15,7 +15,7 @@ software--to make sure the software is free for all its
users. This
General Public License applies to most of the Free Software
Foundation's software and to any other program whose authors commit to
using it. (Some other Free Software Foundation software is covered by
-the GNU Library General Public License instead.) You can apply it to
+the GNU Lesser General Public License instead.) You can apply it to
your programs, too.
When we speak of free software, we are referring to freedom, not
@@ -55,8 +55,8 @@ patent must be licensed for everyone's free use or not
licensed at all.
The precise terms and conditions for copying, distribution and
modification follow.
-
- GNU GENERAL PUBLIC LICENSE
+
+GNU GENERAL PUBLIC LICENSE
TERMS AND CONDITIONS FOR COPYING, DISTRIBUTION AND MODIFICATION
0. This License applies to any program or other work which contains
@@ -110,7 +110,7 @@ above, provided that you also meet all of these conditions:
License. (Exception: if the Program itself is interactive but
does not normally print such an announcement, your work based on
the Program is not required to print an announcement.)
-
+
These requirements apply to the modified work as a whole. If
identifiable sections of that work are not derived from the Program,
and can be reasonably considered independent and separate works in
@@ -168,7 +168,7 @@ access to copy from a designated place, then offering
equivalent
access to copy the source code from the same place counts as
distribution of the source code, even though third parties are not
compelled to copy the source along with the object code.
-
+
4. You may not copy, modify, sublicense, or distribute the Program
except as expressly provided under this License. Any attempt
otherwise to copy, modify, sublicense or distribute the Program is
@@ -225,7 +225,7 @@ impose that choice.
This section is intended to make thoroughly clear what is believed to
be a consequence of the rest of this License.
-
+
8. If the distribution and/or use of the Program is restricted in
certain countries either by patents or by copyrighted interfaces, the
original copyright holder who places the Program under this License
@@ -255,7 +255,7 @@ make exceptions for this. Our decision will be guided by
the two goals
of preserving the free status of all derivatives of our free software and
of promoting the sharing and reuse of software generally.
- NO WARRANTY
+NO WARRANTY
11. BECAUSE THE PROGRAM IS LICENSED FREE OF CHARGE, THERE IS NO WARRANTY
FOR THE PROGRAM, TO THE EXTENT PERMITTED BY APPLICABLE LAW. EXCEPT WHEN
@@ -277,9 +277,9 @@ YOU OR THIRD PARTIES OR A FAILURE OF THE PROGRAM TO OPERATE
WITH ANY OTHER
PROGRAMS), EVEN IF SUCH HOLDER OR OTHER PARTY HAS BEEN ADVISED OF THE
POSSIBILITY OF SUCH DAMAGES.
-END OF TERMS AND CONDITIONS
-
- How to Apply These Terms to Your New Programs
+ END OF TERMS AND CONDITIONS
+
+How to Apply These Terms to Your New Programs
If you develop a new program, and you want it to be of the greatest
possible use to the public, the best way to achieve this is to make it
@@ -303,10 +303,9 @@ the "copyright" line and a pointer to where the full
notice is found.
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
-You should have received a
Re: [PATCH 09/11] ui-tree: ls_tail: add walk table param
On Wed, Jun 13, 2018 at 10:02:15AM +0800, Andy Green wrote:
> Arrange that walk_tree_ctx is available in ls_tail, we
> will make use of it shortly.
>
> Signed-off-by: Andy Green
Reviewed-by: John Keeping
> ---
> ui-tree.c |6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/ui-tree.c b/ui-tree.c
> index bb10b17..53b5594 100644
> --- a/ui-tree.c
> +++ b/ui-tree.c
> @@ -368,7 +368,7 @@ static void ls_head(void)
> html("\n");
> }
>
> -static void ls_tail(void)
> +static void ls_tail(struct walk_tree_context *walk_tree_ctx)
> {
> html("\n");
> cgit_print_layout_end();
> @@ -390,7 +390,7 @@ static void ls_tree(const struct object_id *oid, char
> *path, struct walk_tree_co
>
> ls_head();
> read_tree_recursive(tree, "", 0, 1, , ls_item, walk_tree_ctx);
> - ls_tail();
> + ls_tail(walk_tree_ctx);
> }
>
>
> @@ -473,7 +473,7 @@ void cgit_print_tree(const char *rev, char *path, bool
> use_render)
>
> read_tree_recursive(commit->tree, "", 0, 0, , walk_tree,
> _tree_ctx);
> if (walk_tree_ctx.state == 1)
> - ls_tail();
> + ls_tail(_tree_ctx);
> else if (walk_tree_ctx.state == 2)
> cgit_print_layout_end();
> else
>
> ___
> 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: [PATCH 08/11] ui-tree: print_object: add is_inline param
On Wed, Jun 13, 2018 at 10:02:10AM +0800, Andy Green wrote:
> We will reuse print_object to render things inline shortly.
>
> Add a parameter that lets us adapt its behaviour slightly
> for that case.
>
> Signed-off-by: Andy Green
> ---
> ui-tree.c |7 ---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/ui-tree.c b/ui-tree.c
> index fe5dc75..bb10b17 100644
> --- a/ui-tree.c
> +++ b/ui-tree.c
> @@ -158,7 +158,7 @@ static void include_file(const unsigned char *sha1, const
> char *path,
> }
>
> static void print_object(const unsigned char *sha1, char *path, const char
> *basename,
> - const char *rev, bool use_render)
> + const char *rev, bool use_render, bool is_inline)
I don't think this is the right way to handle this. There's other
behaviour here that needs to be suppressed if this is inline content,
such as the call to cgit_set_title_from_path(), and I don't think we
want the object header above the readme content.
Additionally, if read_sha1_file() fails we'll output another page header
here.
Since it's not much code overall, I think we probably don't want to
reuse print_object() for the inline rendering, but can instead just do
the render & mimetype lookup and then do:
if (render)
render_buffer(render, basename, buf, size);
else if (mimetype)
include_file(sha1, path, mimetype);
else
print_buffer(basename, buf, size);
> {
> enum object_type type;
> struct cgit_filter *render;
> @@ -191,7 +191,8 @@ static void print_object(const unsigned char *sha1, char
> *path, const char *base
> if (!render && !mimetype)
> use_render = false;
>
> - cgit_print_layout_start();
> + if (!is_inline)
> + cgit_print_layout_start();
> htmlf("blob: %s (", sha1_to_hex(sha1));
> cgit_plain_link("plain", NULL, NULL, ctx.qry.head,
> rev, path);
> @@ -415,7 +416,7 @@ static int walk_tree(const unsigned char *sha1, struct
> strbuf *base,
> } else {
> walk_tree_ctx->state = 2;
> print_object(sha1, buffer.buf, pathname,
> walk_tree_ctx->curr_rev,
> - walk_tree_ctx->use_render);
> + walk_tree_ctx->use_render, 0);
> strbuf_release();
>
> return 0;
>
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH 1/1] RFC: git: update to v2.18.0-rc2
On Thu, Jun 14, 2018 at 12:52:25AM +0200, Christian Hesse wrote: > From: Christian Hesse > > Update to git version v2.18.0-rc2. Required changes follow upstream commits: > > * Convert find_unique_abbrev* to struct object_id > (aab9583f7b5ea5463eb3f653a0b4ecac7539dc94) > * sha1_file: convert read_sha1_file to struct object_id > (b4f5aca40e6f77cbabcbf4ff003c3cf30a1830c8) > * sha1_file: convert sha1_object_info* to object_id > (abef9020e3df87c441c9a3a95f592fce5fa49bb9) > * object-store: move packed_git and packed_git_mru to object store > (a80d72db2a73174b3f22142eb2014b33696fd795) > * treewide: rename tree to maybe_tree > (891435d55da80ca3654b19834481205be6bdfe33) > > The changed data types required some of our own functions to be converted > to struct object_id: > > ls_item > print_dir > print_dir_entry > print_object > single_tree_cb > walk_tree > write_tree_link > > And finally we use new upstream functions that were added for > struct object_id: > > hashcpy -> oidcpy > sha1_to_hex -> oid_to_hex > > Signed-off-by: Christian Hesse With Andy's fix, Reviewed-by: John Keeping It looks like our interface to libgit.a is completely converted to struct object_id after this update! But we still have a few mentions of sha1 in our code. Most of this is sha1 and sha2 in struct cgit_query, but these aren't actually hashes most of the time, they can be any object reference that Git understands, so I think we should rename them to ref1 and ref2 (along with has_sha1 -> has_ref). What do you think? (ui-blob does require sha1 to be hex, but it's a bit of a special case anyway and I think it's fine to impose an extra requirement there.) After that, I think all that's left is parsing.c and there's no reason to do anything there until we now how newhash will alter the commit and tag object encodings. ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
[PATCH] cgitrc.5: fix auth filter return value documentation
We don't treat all return values as invalid! Signed-off-by: John Keeping --- cgitrc.5.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cgitrc.5.txt b/cgitrc.5.txt index 4da166c..3e53bbd 100644 --- a/cgitrc.5.txt +++ b/cgitrc.5.txt @@ -715,7 +715,7 @@ auth filter:: When the filter action is "body", this filter must write to output the HTML for displaying the login form, which POSTs to the login url. When the filter action is "authenticate-cookie", this filter must validate - the http cookie and return a 0 if it is invalid or 1 if it is invalid, + the http cookie and return a 0 if it is invalid or 1 if it is valid, in the exit code / close function. If the filter action is "authenticate-post", this filter receives POST'd parameters on standard input, and should write a complete CGI response, preferably -- 2.17.1 ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [RFC PATCH] Makefile: work around parallel make issues in docs
Hi John, John Keeping wrote: > How about the patch below instead? It's a bigger change to the output > format for HTML, but as a side effect it fixes the parallel build. It does, but only if the targets are 'doc-man doc-html'. For the default 'doc' target which includes 'doc-pdf', the issue is still present. As a bonus, the output is much nicer, I think. We could perhaps work around the 'doc-pdf' failure by adding a dep on '($DOC_MAN5)': -- >8 -- diff --git i/Makefile w/Makefile index 70f32a4..4879a5a 100644 --- i/Makefile +++ w/Makefile @@ -143,7 +143,7 @@ $(DOC_HTML): %.html : %.txt $(TXT_TO_HTML) -o $@+ $< && \ mv $@+ $@ -$(DOC_PDF): %.pdf : %.txt +$(DOC_PDF): %.pdf : $(DOC_MAN5) %.txt a2x -f pdf cgitrc.5.txt clean: clean-doc -- >8 -- That's hackish, no doubt. We might also want to drop 'doc-pdf' from the default 'doc' target. The alternative is driving the asciidoc pipeline for the pdf generation too. That looks a little more involved than doing it for html, but perhaps it's not as bad as I think. At first I was concerned that this loses the xsltproc option to generate consistent id's in the html, but the html generated by asciidoc this way doesn't appear to suffer from the problem solved by that xsltproc option. :) Thanks, -- Todd ~~ Common sense is the collection of prejudices acquired by age eighteen. -- Albert Einstein ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
Re: [RFC PATCH] Makefile: work around parallel make issues in docs
Hi, John Keeping wrote: > On Sat, Jun 16, 2018 at 12:32:06PM -0400, Todd Zullinger wrote: >> We might also want to drop 'doc-pdf' from the default 'doc' >> target. The alternative is driving the asciidoc pipeline >> for the pdf generation too. That looks a little more >> involved than doing it for html, but perhaps it's not as bad >> as I think. > > I think we can definitely drop doc-pdf from the default output. > > I'm half tempted to say we should just delete the PDF output completely > and see if anyone complains, unless you know of anyone using this? We don't generate the pdf docs for the Fedora/EPEL cgit builds and no one has yet filed a bug asking for it. Poking around, none of Arch, Debian, Gentoo, and Ubuntu build the pdf docs (in fact, they don't include the html target either). Maybe that's a good sign that the pdf target wouldn't be missed by many people. > Otherwise, the dependency on $(DOC_MAN5) seems reasonable > to me, probably accompanied with a comment explaining the > clash in a2x's intermediate files. Cool. I'll wait a bit and see if anyone chimes in with support for keeping the pdf target. If not, dropping it is the easier option. Thanks, -- Todd ~~ I dropped acid on a Saturday night Just to see what the fuss was about. Now there goes the neighborhood. ___ CGit mailing list CGit@lists.zx2c4.com https://lists.zx2c4.com/mailman/listinfo/cgit
