[PATCH] git: update to 1.8.4

2013-08-23 Thread John Keeping
No code changes required, just bump the submodule and makefile versions.

Signed-off-by: John Keeping j...@keeping.me.uk
---
 Makefile | 2 +-
 git  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 49b6b0c..82d5b58 100644
--- a/Makefile
+++ b/Makefile
@@ -14,7 +14,7 @@ htmldir = $(docdir)
 pdfdir = $(docdir)
 mandir = $(prefix)/share/man
 SHA1_HEADER = openssl/sha.h
-GIT_VER = 1.8.3
+GIT_VER = 1.8.4
 GIT_URL = https://git-core.googlecode.com/files/git-$(GIT_VER).tar.gz
 INSTALL = install
 COPYTREE = cp -r
diff --git a/git b/git
index edca415..e230c56 16
--- a/git
+++ b/git
@@ -1 +1 @@
-Subproject commit edca4152560522a431a51fc0a06147fc680b5b18
+Subproject commit e230c568c4b9a991e3175e5f65171a566fd8e39c
-- 
1.8.4.rc3.504.gcab7572

___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: HEAD tag when branch viewed

2013-09-16 Thread John Keeping
On Mon, Sep 16, 2013 at 06:35:43PM +, Smith, Eric wrote:
 Is this the right place to ask a cgit question?

Yes :-)

 Shouldn't the red HEAD indicator reflect the selected branch?

 Using cgit v0.9.2 the Summary display of the 'master' branch correctly
 displays the red HEAD indicator next to the green master
 indicator.

 But when I use the 'switch' dropdown to display the 'develop' branch
 the red HEAD indicator remains where it was, even though the green
 develop indicator is now (correctly) displayed on a different
 commit.

 The 'git show-ref' does have different commit shas for
 refs/head/develop and refs/head/master.

The HEAD indicator is showing where the HEAD symref on the server
points.  By default this is refs/heads/master.

You don't normally see a remote's HEAD ref although it does affect the
default branch checked out by git clone, but you will see it if you
use git ls-remote.
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: Encoding problem

2013-10-06 Thread 'John Keeping'
On Sat, Oct 05, 2013 at 11:32:54AM +0100, Jorge Bastos wrote:
  On Sat, Sep 28, 2013 at 12:19:38AM +0100, Jorge Bastos wrote:
   Is it possible to define charset in cgitrc?
  
   I'm having encoding problems in the frontend, in the latest version
   1.8.4 from version 0.9.2, and now non-ascii chars are shown with ??
  or
   some other char instead of the correct one.
  
  
  
   Is there a charset option for cgit ? I can't find it.
  
  The charset is hardcoded to UTF-8, which should be the default
  encoding for Git commit messages and CGit does attempt to transcode Git
  messages to the correct encoding.
  
  Are you seeing '??' in the commit message or in blob/tree content?
  
  Do you have a public repository that is exhibiting these symptoms?
 
 I was checking and the file in question was indeed in ANSI, changed the file
 encoding to utf8 and it's OK.
 Anyway, I have gitweb install side-by-side, and in gitweb it was shown
 correctly.
 
 I have other places where chars are not shown OK but didn't get any
 conclution about the file encoding, I'll tell you later,

I've had another look at this, and Gitweb is doing this for all data it
outputs:

# decode sequences of octets in utf8 into Perl's internal form,
# which is utf-8 with utf8 flag set if needed.  gitweb writes out
# in utf-8 thanks to binmode STDOUT, ':utf8' at beginning
sub to_utf8 {
my $str = shift;
return undef unless defined $str;

if (utf8::is_utf8($str) || utf8::decode($str)) {
return $str;
} else {
return decode($fallback_encoding, $str, Encode::FB_DEFAULT);
}
}

Do you know what the fallback encoding on your Gitweb installation is?
(The default is 'latin1').

If you're not using any other source filter with CGit, you should get
the same result by configuring the following script as source-filter
in your cgitrc file.

We'll still get it wrong in plain view though, since we
unconditionally set the charset to UTF-8 there and dump the content out
raw; that can be tweaked in the config file but it looks like we get
that wrong and unconditionally append a charset= to the MIME type even
for binary types.

-- 8 --
#!/usr/bin/perl
use strict;
use warnings;
use Encode;

binmode STDOUT, ':utf8';

my $str = do { local $/; STDIN };

if (utf8::decode($str)) {
print $str;
} else {
print decode('latin1', $str, Encode::FB_DEFAULT);
}
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: Help with installing cgit

2013-12-29 Thread John Keeping
On Sun, Dec 29, 2013 at 08:46:37AM +, Shlomit Afgin wrote:
 I download from http://git.zx2c4.com/cgit/refs/  the file cgit-0.9.2.tar.xz
 I follow the instruction in README:
  make get-git
  make
  make install
  Edit Apache conf file and add
 Directory /var/www/htdocs/cgit/
   AllowOverride None
   Options +ExecCGI
   Order allow,deny
   Allow from all
 /Directory
  I also add alias:
 Alias /cgit  /var/www/htdocs/cgit/
 
 When I go to http://server.domain/cgit I get the following error:
 You don't have permission to access /cgit/ on this server
 And In the error_log I get:
 Directory index forbidden by Options directive: /var/www/htdocs/cgit/
 I tried to add to 'Options' the +Indexes, So I get the list of the
 content but the cgit did not work.

The cgit program is a CGI executable that you need to run.  Do you
have cgit in /var/www/htdocs/cgit/ ?  If so, what happens if you go to
http://your.domain/cgit/cgit ?

I have the following in my Apache config for CGit:

Location /cgit
RewriteEngine on
RewriteCond %{REQUEST_FILENAME} !-f
RewriteRule ^/var/www/localhost/htdocs/cgit(.*) /cgi-bin/cgit.cgi$1 
[L,PT]
/Location

This rewrites all requests under /cgit to go to the cgit program in
/cgi-bin/.

There is some more information on Apache's CGI support here [1].

[1] http://httpd.apache.org/docs/current/howto/cgi.html

 Also I did not find instruction, how to set the file cgit.conf in
 order to change the place of cgit files location.

You can either specify CGIT_CONFIG in the environment under which CGit
runs (e.g. using Apache's SetEnv directive) or just change the default
when you build CGit by setting CGIT_CONFIG in the cgit.conf file
that's included by the makefile.


Hope this helps,
John
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: [PATCH 2/2] README: Update dependencies

2014-01-09 Thread John Keeping
On Thu, Jan 09, 2014 at 04:13:08PM +0100, Jason A. Donenfeld wrote:
 On Thu, Jan 9, 2014 at 8:30 AM, Lukas Fleischer c...@cryptocrack.de wrote:
  We depend on Git in the test suite. Maybe this should be changed to use
  the binary from the Git submodule instead?
 
 I think we discussed the possibility of this a while ago with John.
 This would make most sense to me, though we are using git's test
 harness, and I'd like to avoid making invasive changes in that, if
 possible.

We do already build the Git tools when building the test target (as
opposed to just libgit.a when building the cgit binary), but I can't
remember why we do this...

It's probably simplest just to further tweak $PATH in tests/setup.sh so
it also prepends $(pwd)/../../git/bin-wrappers.  That should cause us
to use the Git tools from the submodule without needing to do anything
else.

 In either case, I wouldn't consider the test case a hard dependency,
 and so I'm okay with removing git from the dep list. Or changing it to
 Git 1.8.5 (included) or similar.
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: RFE: author/committer/tagger links (enable cgit to show gravatar for author, committer and tagger)

2014-01-09 Thread John Keeping
On Thu, Jan 09, 2014 at 06:50:29PM +0100, Jason A. Donenfeld wrote:
 On Thu, Jan 9, 2014 at 4:21 PM, Konstantin Ryabitsev mri...@kernel.org 
 wrote:
  That's pretty nifty.
 
 Cool. Would you consider enabling this on kernel.org? I'll probably
 merge it in a few days (still working out some bugs). Seems like the
 kind of thing that might give cgit a lot of positive attention...
 
  That reminds me -- I'm working on a web-of-trust
  site for kernel.org and something I wouldn't mind having is a way to
  link from cgit to the web of trust for that person. E.g. an email
  address for torva...@linux-foundation.org on this page
  (http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=d6e0a2dd12f4067a5bcefb8bbd8ddbeff800afbc)
  would be wrapped in a link such as:
 
  a href=https://blah.kernel.org/?user=torvalds%40linux-foundation.org;
  torva...@linux-foundation.org/a
 
  which will bring up a page similar to:
  https://www.kernel.org/doc/wot/torvalds.html
 
  Not sure whether that would clash with any of your existing plans for
  user links, but figured I'd bring this up for a discussion.
 
 We already support module-link and repo.module-link for making
 clickable submodules. I don't see why we wouldn't be able to apply the
 same idiom to a email-link and repo.email-link setting. I'll look into
 this. I suppose a default setting, or maybe just a suggested setting
 in the documentation, would be to issue a search for that email. But
 this could then be used to instead link out to your upcoming web of
 trust platform. How does this sound?

It feels to me like it might be better to allow a filter to be applied
here.  That way we don't put the Gravatar code in CGit itself but can
distribute an example script that does apply Gravatar links to email
addresses.

I'm not sure what the cost of forking a process here will end up being
though; I guess for cases where the email is likely to be repeated we
could assume that the filter is pure and memoize the result.

But that would give the greatest flexibility for both these use cases,
with a simple link value the Gravatar case needs some other forwarding
program on the server to do the hashing of the address (unless we
provide substitution values for a range of different things, but
currently everything just uses printf formats so that would be more
work unless we can reuse Git's log formatter somehow).
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: RFE: .so filters

2014-01-09 Thread John Keeping
On Thu, Jan 09, 2014 at 10:34:26PM +0100, Jason A. Donenfeld wrote:
 I'm thinking about this filtering situation w.r.t. gravatar and
 potentially running multiple filters on one page. Something I've been
 considering is implementing a simple dlopen() mechanism for filters,
 if the filter filename starts with soname: or lib: or similar, so
 as to avoid the fork()ing and exec()ing we currently have, for high
 frequency filters. The idea is that first use of the filter would be
 dlopen()'d, but wouldn't be dlclose()'d until the end of the
 processing. This way the same function could be used over and over
 again without significant penalty.
 
 In my first thinking of this, the method of action would be the same
 as the current system -- int filter_run(int argc, char *argv[]) is
 dlopen()'d, executed, and it reads and writes to the dup2()'d file
 descriptor. Unfortunately, the piping in this introduces a cost that
 I'd rather avoid. In the case of gravatar (or more generally, email
 author filters), we'd be better off with a char *filter_run(int argc,
 char *argv[]), that can just return the string that the html
 functions will then print. This, however, breaks the current filtering
 paradigm, and might not be ideal for filters that enjoy a stream of
 data (such as source code filters). This distinction more or less
 points toward coming up with a library API of sorts, but I really
 really really don't want to add a full fledged plugin system. So this
 has me leaning toward the simpler first idea.
 
 But I'm undecided at the moment. Comments and suggestions are most
 welcome.

That interface doesn't really match the way the current filters work.
Currently when we open a filter we replace cgit's stdout with a pipe
into the filter process, so none of the existing CGit code will work
with this interface.  We could swap out write with a function pointer
into the filter, but I don't think we guarantee that all of the data is
written in one go which makes life harder for filter writers (although
for simple cases like author info we probably could guarantee to write
it all at once).

If we allow filters to act incrementally, then we can just leave the
filter running and swap it in or out when required.  That would require
a single dup2 to make it work the same way that the filters currently
work.  Interestingly, there is an htmlfd variable in html.c but it is
never changed from STDOUT_FILENO; I wonder if that can be used or are
there other places (possibly in libgit.a code) that just use stdout, in
which case we should remove that variable.  But there is the problem of
terminating the response; Lukas' suggestion of using NUL for that may be
the best, it's not that hard to printf '\0' in shell.

OTOH, the particular case of author details the input is more clearly
defined than items for which we currently provide filters, so maybe it
could use a different interface.

One final point (although I don't think you're suggesting this) is that
we shouldn't require shared objects; I think scripts using stdin+stdout
are a much simpler interface and provides a much lower barrier to entry,
not least because the range of languages that can be used to implement
the filters is so much greater.
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: [PATCH] Disallow downloading disabled snapshot formats

2014-01-10 Thread John Keeping
On Fri, Jan 10, 2014 at 03:38:06PM +0100, Lukas Fleischer wrote:
 We did only display enabled snapshot formats but we did not prevent from
 downloading disabled formats when requested. Fix this by adding an
 appropriate check.
 
 Also, add a test case that checks whether downloading disabled snapshot
 formats is denied, as expected.
 
 Signed-off-by: Lukas Fleischer c...@cryptocrack.de
 ---
  tests/t0107-snapshot.sh | 5 +
  ui-snapshot.c   | 2 +-
  2 files changed, 6 insertions(+), 1 deletion(-)
 
 diff --git a/tests/t0107-snapshot.sh b/tests/t0107-snapshot.sh
 index 6cf7aaa..01e8d22 100755
 --- a/tests/t0107-snapshot.sh
 +++ b/tests/t0107-snapshot.sh
 @@ -79,4 +79,9 @@ test_expect_success UNZIP 'verify unzipped file-5' '
   test_line_count = 1 master/file-5
  '
  
 +test_expect_success 'try to download a disabled snapshot format' '
 + cgit_url foo/snapshot/master.tar.xz |
 + grep Unsupported snapshot format

I really dislike seeing pipes in the test suite.  Can we redirect to
file instead and then grep the file?  This helps ensure that the exit
code from CGit is correct (I don't know if we expect it to be zero or
non-zero here, but if the latter then at least test_must_fail checks
that the process didn't segfault - I suspect it should be zero though).

 +'
 +
  test_done
 diff --git a/ui-snapshot.c b/ui-snapshot.c
 index 8f82119..ab20a4a 100644
 --- a/ui-snapshot.c
 +++ b/ui-snapshot.c
 @@ -205,7 +205,7 @@ void cgit_print_snapshot(const char *head, const char 
 *hex,
   }
  
   f = get_format(filename);
 - if (!f) {
 + if (!f || (snapshots  f-bit) == 0) {
   show_error(Unsupported snapshot format: %s, filename);
   return;
   }
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: RFE: .so filters

2014-01-10 Thread John Keeping
On Fri, Jan 10, 2014 at 06:12:25PM +0100, Florian Pritz wrote:
 On 10.01.2014 16:57, Jason A. Donenfeld wrote:
  On Fri, Jan 10, 2014 at 10:06 AM, John Keeping j...@keeping.me.uk wrote:
 
  This seems drastically over complicated.
  
  So here's the situation. There's a lot of state that we're taking
  advantage of in using processes that terminate, that needs to be
  replicated:
 
 Isn't this (fast scripting with lots of features) when people normally
 start using lua?

I would have no problem including LuaJIT for this sort of thing.  There
was even a PoC for using Lua to format Git log messages a year or so
ago.

I was also wondering if supporting unix:/path/to/socket would be
useful, then the filter would connect on a Unix socket, run and
disconnect, on the assumption that the administrator has a daemon
running to do the filtering.

If we're introducing this type:spec support then it would be good
to do it in a reasonably generic way so that any types that add new
dependencies can be compiled out easily.  Maybe a table like this?

struct filter_handler handlers[] = {
{ unix, open_unix_socket_filter, close_unix_socket_filter },
{ persistent, open_persistent_filter, close_persistent_filter },
#ifndef NO_LUA
{ lua, open_lua_filter, close_lua_filter },
#endif
};

I might have a look at the Lua case over the weekend if no one beats me
to it.
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: RFE: .so filters

2014-01-10 Thread John Keeping
On Fri, Jan 10, 2014 at 09:03:24PM +0100, Florian Pritz wrote:
 On 10.01.2014 18:57, Jason A. Donenfeld wrote:
  On Fri, Jan 10, 2014 at 6:12 PM, Florian Pritz bluew...@xinu.at wrote:
 
  Isn't this (fast scripting with lots of features) when people normally
  start using lua?
 
  
  This would have the same challenges as using .so files, w.r.t. hooking
  write() (or the html functions), but would be very interesting indeed,
  because Lua...
 
 How about using the current fork approach but instead of calling execvp
 use lua. I believe forks are pretty cheap on linux, it's the exec that's
 costly.
 
 If we do it like that we could reuse stdin/stdout, we could pass
 arguments via lua tables (like command line arguments now), but we
 should have little overhead for the script loading/executing.

Forking and using Lua in the child is an interesting idea.

I need to investigate how Lua generally deals with I/O, but it feels
like it will be simpler to use a simple function interface than deal
with slurping in the input in Lua.  So it may be simpler to swap out the
write function in CGit while the filter is active and collect the output
in a buffer instead, then call a Lua function and pass whatever comes
back from that to the real write(2).
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


[PATCH 5/6] filter: add interface layer

2014-01-12 Thread John Keeping
Change the existing cgit_{open,close,fprintf}_filter functions to
delegate to filter-specific implementations accessed via function
pointers on the cgit_filter object.

We treat the exec filter type slightly specially here by putting its
structure definition in the header file and providing an init function
to set up the function pointers.  This is required so that the
ui-snapshot.c code that applies a compression filter can continue to use
the filter interface to do so.

Signed-off-by: John Keeping j...@keeping.me.uk
---
 cgit.h|  8 
 filter.c  | 66 ---
 ui-snapshot.c | 11 +-
 3 files changed, 63 insertions(+), 22 deletions(-)

diff --git a/cgit.h b/cgit.h
index 9b4be26..92e8c55 100644
--- a/cgit.h
+++ b/cgit.h
@@ -57,6 +57,13 @@ typedef enum {
 } filter_type;
 
 struct cgit_filter {
+   int (*open)(struct cgit_filter *, va_list ap);
+   int (*close)(struct cgit_filter *);
+   void (*fprintf)(struct cgit_filter *, FILE *, const char *prefix);
+};
+
+struct cgit_exec_filter {
+   struct cgit_filter base;
char *cmd;
char **argv;
int extra_args;
@@ -346,6 +353,7 @@ extern int cgit_parse_snapshots_mask(const char *str);
 extern int cgit_open_filter(struct cgit_filter *filter, ...);
 extern int cgit_close_filter(struct cgit_filter *filter);
 extern void cgit_fprintf_filter(struct cgit_filter *filter, FILE *f, const 
char *prefix);
+extern void cgit_exec_filter_init(struct cgit_exec_filter *filter, char *cmd, 
char **argv);
 extern struct cgit_filter *cgit_new_filter(const char *cmd, filter_type 
filtertype);
 
 extern void cgit_prepare_repo_env(struct cgit_repo * repo);
diff --git a/filter.c b/filter.c
index 80cf689..0f3edb0 100644
--- a/filter.c
+++ b/filter.c
@@ -13,15 +13,13 @@
 #include string.h
 #include stdlib.h
 
-int cgit_open_filter(struct cgit_filter *filter, ...)
+static int open_exec_filter(struct cgit_filter *base, va_list ap)
 {
+   struct cgit_exec_filter *filter = (struct cgit_exec_filter *) base;
int i;
-   va_list ap;
 
-   va_start(ap, filter);
for (i = 0; i  filter-extra_args; i++)
filter-argv[i+1] = va_arg(ap, char *);
-   va_end(ap);
 
filter-old_stdout = chk_positive(dup(STDOUT_FILENO),
Unable to duplicate STDOUT);
@@ -41,9 +39,9 @@ int cgit_open_filter(struct cgit_filter *filter, ...)
return 0;
 }
 
-
-int cgit_close_filter(struct cgit_filter *filter)
+static int close_exec_filter(struct cgit_filter *base)
 {
+   struct cgit_exec_filter *filter = (struct cgit_exec_filter *) base;
int i, exit_status;
 
chk_non_negative(dup2(filter-old_stdout, STDOUT_FILENO),
@@ -63,21 +61,50 @@ done:
 
 }
 
-void cgit_fprintf_filter(struct cgit_filter *filter, FILE *f, const char 
*prefix)
+static void fprintf_exec_filter(struct cgit_filter *base, FILE *f, const char 
*prefix)
 {
+   struct cgit_exec_filter *filter = (struct cgit_exec_filter *) base;
fprintf(f, %s%s\n, prefix, filter-cmd);
 }
 
-struct cgit_filter *cgit_new_filter(const char *cmd, filter_type filtertype)
+int cgit_open_filter(struct cgit_filter *filter, ...)
 {
-   struct cgit_filter *f;
-   int args_size = 0;
+   int result;
+   va_list ap;
+   va_start(ap, filter);
+   result = filter-open(filter, ap);
+   va_end(ap);
+   return result;
+}
 
-   if (!cmd || !cmd[0])
-   return NULL;
+int cgit_close_filter(struct cgit_filter *filter)
+{
+   return filter-close(filter);
+}
+
+void cgit_fprintf_filter(struct cgit_filter *filter, FILE *f, const char 
*prefix)
+{
+   filter-fprintf(filter, f, prefix);
+}
+
+void cgit_exec_filter_init(struct cgit_exec_filter *filter, char *cmd, char 
**argv)
+{
+   memset(filter, 0, sizeof(*filter));
+   filter-base.open = open_exec_filter;
+   filter-base.close = close_exec_filter;
+   filter-base.fprintf = fprintf_exec_filter;
+   filter-cmd = cmd;
+   filter-argv = argv;
+}
+
+static struct cgit_filter *new_exec_filter(const char *cmd, filter_type 
filtertype)
+{
+   struct cgit_exec_filter *f;
+   int args_size = 0;
 
-   f = xmalloc(sizeof(struct cgit_filter));
-   memset(f, 0, sizeof(struct cgit_filter));
+   f = xmalloc(sizeof(*f));
+   /* We leave argv for now and assign it below. */
+   cgit_exec_filter_init(f, xstrdup(cmd), NULL);
 
switch (filtertype) {
case SOURCE:
@@ -91,10 +118,17 @@ struct cgit_filter *cgit_new_filter(const char *cmd, 
filter_type filtertype)
break;
}
 
-   f-cmd = xstrdup(cmd);
args_size = (2 + f-extra_args) * sizeof(char *);
f-argv = xmalloc(args_size);
memset(f-argv, 0, args_size);
f-argv[0] = f-cmd;
-   return f;
+   return f-base;
+}
+
+struct cgit_filter *cgit_new_filter(const char *cmd, filter_type filtertype)
+{
+   if (!cmd

[RFC/PATCH 0/6] Preparation for more filter types

2014-01-12 Thread John Keeping
This is the preliminary refactoring for supporting more types of filter
(for example Lua scripts or persistent filters).  The final patch adds a
table where more implementations can be added.

The first three (maybe four) patches are sensible cleanups even if we
don't want to take the whole plan any further.

John Keeping (6):
  html: remove redundant htmlfd variable
  ui-snapshot: set unused cgit_filter fields to zero
  filter: pass extra arguments via cgit_open_filter
  filter: add fprintf_filter function
  filter: add interface layer
  filter: introduce filter type prefix

 cgit.c|   6 +--
 cgit.h|  12 +-
 cgitrc.5.txt  |   9 +
 filter.c  | 119 --
 html.c|   4 +-
 ui-repolist.c |  10 ++---
 ui-snapshot.c |   9 ++---
 ui-summary.c  |  13 +++
 ui-tree.c |   7 ++--
 9 files changed, 140 insertions(+), 49 deletions(-)

-- 
1.8.5.226.g0d60d77

___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


[PATCH 2/6] ui-snapshot: set unused cgit_filter fields to zero

2014-01-12 Thread John Keeping
By switching the assignment of fields in the cgit_filter structure to
use designated initializers, the compiler will initialize all other
fields to their default value.  This will be needed when we add the
extra_args field in the next patch.

Signed-off-by: John Keeping j...@keeping.me.uk
---
 ui-snapshot.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/ui-snapshot.c b/ui-snapshot.c
index 8f82119..5136c49 100644
--- a/ui-snapshot.c
+++ b/ui-snapshot.c
@@ -58,10 +58,10 @@ static int write_compressed_tar_archive(const char *hex,
char *filter_argv[])
 {
int rv;
-   struct cgit_filter f;
-
-   f.cmd = filter_argv[0];
-   f.argv = filter_argv;
+   struct cgit_filter f = {
+   .cmd = filter_argv[0],
+   .argv = filter_argv,
+   };
cgit_open_filter(f);
rv = write_tar_archive(hex, prefix);
cgit_close_filter(f);
-- 
1.8.5.226.g0d60d77

___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: [PATCH 4/6] filter: add fprintf_filter function

2014-01-12 Thread John Keeping
On Sun, Jan 12, 2014 at 08:23:02PM +0100, Jason A. Donenfeld wrote:
 What's the purpose of this? Why not just keep the original string that
 was passed to about-filter=... in the cmd variable as we have now? The
 thing that's variable from filter to filter is argv, the type (commit,
 about, etc), and the mechanism (lua, stdout, etc). But the variable
 aspects don't require changing -cmd do they?

I'm looking at splitting up the data so there is a filter object that
contains function pointers to implementation functions and then some
data that is specific to to given filter type.  With that change, cmd
moves to the exec filter structure and is no longer accessible.  I'm
expecting some filter types to have a structured value in the config
file that will be parsed to multiple fields which will be glued back
together in the fprintf function for that filter type.
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


[PATCH 1/3] ui-refs: escape HTML chars in author and tagger names

2014-01-12 Thread John Keeping
Everywhere else we use html_txt to escape any special characters in
these variables.  Do so here as well.

Signed-off-by: John Keeping j...@keeping.me.uk
---
I spotted this while looking at Jason's jd/gravatar series.  The
following two patches cover other similar issues I spotted while
auditing all uses of html().  I think everything else is OK.

 ui-refs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ui-refs.c b/ui-refs.c
index 20c91e3..c97b0c6 100644
--- a/ui-refs.c
+++ b/ui-refs.c
@@ -155,9 +155,9 @@ static int print_tag(struct refinfo *ref)
html(/tdtd);
if (info) {
if (info-tagger)
-   html(info-tagger);
+   html_txt(info-tagger);
} else if (ref-object-type == OBJ_COMMIT) {
-   html(ref-commit-author);
+   html_txt(ref-commit-author);
}
html(/tdtd colspan='2');
if (info) {
-- 
1.8.5.226.g0d60d77

___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


[PATCH 3/3] ui-repolist: HTML-escape cgit_rooturl() response

2014-01-12 Thread John Keeping
This is for consistency with other callers.  The value returned from
cgit_rooturl is not guaranteed to be HTML-safe.

Signed-off-by: John Keeping j...@keeping.me.uk
---
 ui-repolist.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/ui-repolist.c b/ui-repolist.c
index d4ee279..e8d5eb6 100644
--- a/ui-repolist.c
+++ b/ui-repolist.c
@@ -106,7 +106,9 @@ static int is_in_url(struct cgit_repo *repo)
 
 static void print_sort_header(const char *title, const char *sort)
 {
-   htmlf(th class='left'a href='%s?s=%s, cgit_rooturl(), sort);
+   html(th class='left'a href=');
+   html_attr(cgit_rooturl());
+   htmlf(?s=%s, sort);
if (ctx.qry.search) {
html(amp;q=);
html_url_arg(ctx.qry.search);
-- 
1.8.5.226.g0d60d77

___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


[PATCH 2/3] ui-shared: URL-escape script_name

2014-01-12 Thread John Keeping
As far as I know, there is no requirement that $SCRIPT_NAME contain only
URL-safe characters, so we need to make sure that any special characters
are escaped.

Signed-off-by: John Keeping j...@keeping.me.uk
---
 ui-shared.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ui-shared.c b/ui-shared.c
index 2c12de7..abe15cd 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -139,7 +139,7 @@ static void site_url(const char *page, const char *search, 
const char *sort, int
if (ctx.cfg.virtual_root)
html_attr(ctx.cfg.virtual_root);
else
-   html(ctx.cfg.script_name);
+   html_url_path(ctx.cfg.script_name);
 
if (page) {
htmlf(?p=%s, page);
@@ -219,7 +219,7 @@ static char *repolink(const char *title, const char *class, 
const char *page,
html_url_path(path);
}
} else {
-   html(ctx.cfg.script_name);
+   html_url_path(ctx.cfg.script_name);
html(?url=);
html_url_arg(ctx.repo-url);
if (ctx.repo-url[strlen(ctx.repo-url) - 1] != '/')
-- 
1.8.5.226.g0d60d77

___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


[PATCH] tests: add CGIT_TEST_OPTS variable to Makefile

2014-01-12 Thread John Keeping
This allows running the entire test suite with a set of command-line
options.  For example:

make test CGIT_TEST_OPTS=--valgrind

Signed-off-by: John Keeping j...@keeping.me.uk
---
 tests/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/Makefile b/tests/Makefile
index 8c6c236..1556475 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -5,7 +5,7 @@ T = $(wildcard t[0-9][0-9][0-9][0-9]-*.sh)
 all: $(T)
 
 $(T):
-   @./$@
+   @./$@ $(CGIT_TEST_OPTS)
 
 clean:
$(RM) -rf trash
-- 
1.8.5.226.g0d60d77

___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: [PATCH 06/12] filter: add preliminary lua support

2014-01-13 Thread John Keeping
On Mon, Jan 13, 2014 at 09:31:39AM +0100, Lukas Fleischer wrote:
 This patch is quite messy and hard to read. I read your cover-letter but
 maybe you still want to clean this up when dealing with the other
 suggestions during a rebase -- shouldn't be too hard when using an
 editor with good Git integration (like fugitive for Vim).
 
 On Mon, 13 Jan 2014 at 05:11:13, Jason A. Donenfeld wrote:
  [...]
  +ifdef NO_LUA
 
 We should document this in the installation instructions section in the
 README. I also wonder whether this should made an opt-in feature?
 
  +   CFLAGS += -DNO_LUA
  +else
  +   CGIT_LIBS += -llua
 
 Similar: Add Lua/LuaJIT (Will you squash the LuaJIT Makefile fix into
 this one? Or is there any reason to use Lua first and switch to LuaJIT
 later?) to the dependencies section of the README file and mention that
 it is optional.

I think we should support both vanilla Lua and LuaJIT if we can (I
believe LuaJIT can be used as a drop-in replacement, so there's no
reason this shouldn't be possible).

  +endif
  +
  +CGIT_LIBS += -ldl
  +
  +
  +
   CGIT_OBJ_NAMES += cgit.o
   CGIT_OBJ_NAMES += cache.o
   CGIT_OBJ_NAMES += cmd.o
  [...]
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: [PATCH 06/12] filter: add preliminary lua support

2014-01-13 Thread John Keeping
On Mon, Jan 13, 2014 at 05:11:13AM +0100, Jason A. Donenfeld wrote:
[snip]
 +static int html_lua_filter(lua_State *lua_state)
 +{
 + size_t len;
 + const char *str;
 +
 + str = lua_tostring(lua_state, 1);
 + if (!str)
 + return 0;
 + len = strlen(str);
 + libc_write(STDOUT_FILENO, str, len);
 + return 0;
 +}
 +
 +static void cleanup_lua_filter(struct cgit_filter *base)
 +{
 + struct lua_filter *filter = (struct lua_filter *) base;
 +
 + if (!filter-lua_state)
 + return;
 +
 + lua_close(filter-lua_state);
 + filter-lua_state = NULL;
 + if (filter-script_file) {
 + free(filter-script_file);
 + filter-script_file = NULL;
 + }
 +}
 +
 +static int init_lua_filter(struct lua_filter *filter)
 +{
 + if (filter-lua_state)
 + return 0;
 +
 + if (!(filter-lua_state = luaL_newstate()))
 + return 1;
 +
 + luaL_openlibs(filter-lua_state);
 +
 + lua_pushcfunction(filter-lua_state, html_lua_filter);
 + lua_setglobal(filter-lua_state, html);

It would be good to provide some of the other html_* functions here,
particularly html_txt so that filters don't need to re-invent the wheel
for escaping output.  That's probably slightly harder than the plain
HTML, but I suspect we just need to wrap the call to the underlying
function in an unhook_write/hook_write pair.

 +
 + if (luaL_dofile(filter-lua_state, filter-script_file)) {
 + lua_close(filter-lua_state);
 + filter-lua_state = NULL;
 + return 1;
 + }
 + return 0;
 +}
 +
 +static int open_lua_filter(struct cgit_filter *base, va_list ap)
 +{
 + struct lua_filter *filter = (struct lua_filter *) base;
 + int i;
 +
 + if (init_lua_filter(filter))
 + return 1;
 +
 + hook_write(base, write_lua_filter);
 +
 + lua_getglobal(filter-lua_state, filter_open);
 + for (i = 0; i  filter-base.argument_count; ++i)
 + lua_pushstring(filter-lua_state, va_arg(ap, char *));
 + if (lua_pcall(filter-lua_state, filter-base.argument_count, 0, 0))
 + return 1;
 + return 0;
 +}
 +
 +static int close_lua_filter(struct cgit_filter *base)
 +{
 + struct lua_filter *filter = (struct lua_filter *) base;
 + int ret = 0;
 +
 + lua_getglobal(filter-lua_state, filter_close);
 + if (lua_pcall(filter-lua_state, 0, 0, 0))
 + ret = 1;
 + unhook_write();
 + return ret;
 +}
 +
 +static void fprintf_lua_filter(struct cgit_filter *base, FILE *f, const char 
 *prefix)
 +{
 + struct lua_filter *filter = (struct lua_filter *) base;
 + fprintf(f, %slua:%s\n, prefix, filter-script_file);
 +}
 +
 +
 +static struct cgit_filter *new_lua_filter(const char *cmd, int 
 argument_count)
 +{
 + struct lua_filter *filter;
 +
 + filter = xmalloc(sizeof(*filter));
 + memset(filter, 0, sizeof(*filter));
 + filter-base.open = open_lua_filter;
 + filter-base.close = close_lua_filter;
 + filter-base.fprintf = fprintf_lua_filter;
 + filter-base.cleanup = cleanup_lua_filter;
 + filter-base.argument_count = argument_count;
 + filter-script_file = xstrdup(cmd);
 +
 + return filter-base;
  }
  
 +#endif
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: lua vs luajit vs both

2014-01-14 Thread John Keeping
On Tue, Jan 14, 2014 at 02:02:40AM +0100, Jason A. Donenfeld wrote:
 I've gone ahead and merged the lua work to master, for testing and
 subsequent cleanup before release.
 
 Regarding to jit or not to jit, I currently have this fancy autodetection
 logic:
 http://git.zx2c4.com/cgit/commit/?id=3488d124052f5c3ddef303ed5306ad6a458794c1
 
 John -- I'm waiting for your input on the parent email, as you seem to be
 the originator of the opinion that both are good.

It was more of a there doesn't seem much overhead to supporting both,
since the API is the same.  I think the Makefile should take an
approach more like this though:

ifdef NO_LUA
CGIT_CFLAGS += -DNO_LUA
else if defined(USE_LUAJIT)
# LuaJIT code goes here
else
# Lua code goes here
endif

Basically, use vanilla Lua by default by provide an easy way for users
to switch to LuaJIT if they want.
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: Idle time in project overview

2014-01-14 Thread John Keeping
On Tue, Jan 14, 2014 at 11:50:23AM +0100, Stefan Tatschner wrote:
 I don't know if it is a bug or a feature but I think on this nice
 mailing list I could ask without being shot or mutilated. :)
 
 If I have a git repository with multiple branches and I push to another
 branch as 'master' the idle time on the project overview is not updated.
 I think this behaviour is not correct. When I push to another branch as
 master the project is absolutely not in an idle state. I think the idle
 time should be updated when I push to any branch.

This question comes up periodically.  The answer is to use a hook to
update an agefile on push.  The best reference is this message:

http://article.gmane.org/gmane.comp.version-control.cgit/1059

It would be nice to have that example hook in the repository somewhere
(contrib/hooks/ ?) if someone feels like working up a patch...
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: [PATCH] filter: refactor cgit_new_filter()

2014-01-14 Thread John Keeping
On Tue, Jan 14, 2014 at 02:00:48PM +0100, Jason A. Donenfeld wrote:
 From: Lukas Fleischer c...@cryptocrack.de
 
 Use prefixcmp() as a preparation for using strip_prefix() later. Also,
 interpret the command as a file name if it contains a colon but none of
 the filter prefixes matches instead of bailing out and adding a special
 check for Windows.
 
 Signed-off-by: Lukas Fleischer c...@cryptocrack.de
 ---
 John -- this is your code so I'm awaiting your input on this.
 Personally, I like receiving an error message that says invalid filter
 type, and this patch kills that. But I'll go with whatever you prefer.

Yeah, it would be nice to keep the unrecognised prefix warning.

I like the simplification, but I'm not sure the result is better.  Even
without the rest we should replace the strncmp with prefixcmp though.

There's actually no reason we couldn't mutate cmd here, which would
simplify it a lot, but I'm not sure we want to remove the const
modifiers all the way through.  Then we can just do *colon = '\0' and
use strcmp.

  filter.c | 30 +++---
  1 file changed, 7 insertions(+), 23 deletions(-)
 
 diff --git a/filter.c b/filter.c
 index 4d4acaf..1997881 100644
 --- a/filter.c
 +++ b/filter.c
 @@ -379,31 +379,19 @@ static const struct {
   const char *prefix;
   struct cgit_filter *(*ctor)(const char *cmd, int argument_count);
  } filter_specs[] = {
 - { exec, new_exec_filter },
 + { exec:, new_exec_filter },
  #ifndef NO_LUA
 - { lua, new_lua_filter },
 + { lua:, new_lua_filter },
  #endif
  };
  
  struct cgit_filter *cgit_new_filter(const char *cmd, filter_type filtertype)
  {
 - char *colon;
 - int i;
 - size_t len;
 - int argument_count;
 + int argument_count, i;
  
   if (!cmd || !cmd[0])
   return NULL;
  
 - colon = strchr(cmd, ':');
 - len = colon - cmd;
 - /*
 -  * In case we're running on Windows, don't allow a single letter before
 -  * the colon.
 -  */
 - if (len == 1)
 - colon = NULL;
 -
   switch (filtertype) {
   case EMAIL:
   argument_count = 2;
 @@ -420,15 +408,11 @@ struct cgit_filter *cgit_new_filter(const char *cmd, 
 filter_type filtertype)
   break;
   }
  
 - /* If no prefix is given, exec filter is the default. */
 - if (!colon)
 - return new_exec_filter(cmd, argument_count);
 -
   for (i = 0; i  ARRAY_SIZE(filter_specs); i++) {
 - if (len == strlen(filter_specs[i].prefix) 
 - !strncmp(filter_specs[i].prefix, cmd, len))
 - return filter_specs[i].ctor(colon + 1, argument_count);
 + if (!prefixcmp(cmd, filter_specs[i].prefix))
 + return filter_specs[i].ctor(strchr(cmd, ':') + 1, 
 argument_count);

Using strchr here feels wrong, why not:

cmd + strlen(filter_specs[i].prefix)

?

   }
  
 - die(Invalid filter type: %.*s, (int) len, cmd);
 + /* If no valid prefix is given, exec filter is the default. */
 + return new_exec_filter(cmd, argument_count);
  }
 -- 
 1.8.5.2
 
 ___
 CGit mailing list
 CGit@lists.zx2c4.com
 http://lists.zx2c4.com/mailman/listinfo/cgit
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: [PATCH] filter: refactor cgit_new_filter()

2014-01-14 Thread John Keeping
On Tue, Jan 14, 2014 at 09:54:21PM +0100, Jason A. Donenfeld wrote:
 On Tue, Jan 14, 2014 at 9:39 PM, John Keeping j...@keeping.me.uk wrote:
  I like the simplification, but I'm not sure the result is better.  Even
  without the rest we should replace the strncmp with prefixcmp though.
 
 Agreed.
 
  There's actually no reason we couldn't mutate cmd here, which would
  simplify it a lot, but I'm not sure we want to remove the const
  modifiers all the way through.  Then we can just do *colon = '\0' and
  use strcmp.
 
 IMHO, it's better to keep lookup tables like these in the read-only
 section. Let's keep the constness.

I meant for the input, which is read from the config file.  Just
terminate the user-specified command string at the colon and treat it as
two genuinely separate strings.
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: lua vs luajit vs both

2014-01-14 Thread John Keeping
On Tue, Jan 14, 2014 at 07:06:34PM +0100, Jason A. Donenfeld wrote:
 
 
 On Tue, Jan 14, 2014 at 10:08 AM, John Keeping j...@keeping.me.uk wrote:
  It was more of a there doesn't seem much overhead to supporting both,
  since the API is the same.  I think the Makefile should take an
  approach more like this though:
 
  ifdef NO_LUA
  CGIT_CFLAGS += -DNO_LUA
  else if defined(USE_LUAJIT)
  # LuaJIT code goes here
  else
  # Lua code goes here
  endif
 
 Okay we've got this fancy autodetection logic now. From the README:
 
  If you'd like to compile without Lua support, you may use:
 $ make NO_LUA=1
  And if you'd like to specify a Lua implementation, you may use:
 $ make LUA_IMPLEMENTATION=JIT
  for using the LuaJIT project. Or:
 
 $ make LUA_IMPLEMENTATION=VANILLA
  for the mainline Lua project. If you specify neither implementation, it will
  be auto-detected, preferring LuaJIT if both are present.
 
 From cgit.mk:
 
  ifdef NO_LUA
  LUA_MESSAGE := linking without specified Lua support
  CGIT_CFLAGS += -DNO_LUA
  else
  LUAJIT_CFLAGS := $(shell pkg-config --cflags luajit 2/dev/null)
  LUAJIT_LIBS := $(shell pkg-config --libs luajit 2/dev/null)
  LUA_LIBS := $(shell pkg-config --libs lua 2/dev/null)
  LUA_CFLAGS := $(shell pkg-config --cflags lua 2/dev/null)
  ifeq (JIT,$(LUA_IMPLEMENTATION))
  ifeq ($(strip $(LUAJIT_LIBS)),)
   $(error LuaJIT specified via LUA_IMPLEMENTATION=JIT, but library 
  could not be found.)
  endif
  LUA_MESSAGE := linking with selected LuaJIT
  CGIT_LIBS += $(LUAJIT_LIBS)
  CGIT_CFLAGS += $(LUAJIT_CFLAGS)
  else ifeq (VANILLA,$(LUA_IMPLEMENTATION))
  ifeq ($(strip $(LUA_LIBS)),)
   $(error Lua specified via LUA_IMPLEMENTATION=VANILLA, but library 
  could not be found.)
  endif
  LUA_MESSAGE := linking with selected Lua
  CGIT_LIBS += $(LUA_LIBS)
  CGIT_LIBS += $(LUA_CFLAGS)
  else ifneq ($(strip $(LUAJIT_LIBS)),)
  LUA_MESSAGE := linking with autodetected LuaJIT
  CGIT_LIBS += $(LUAJIT_LIBS)
  CGIT_CFLAGS += $(LUAJIT_CFLAGS)
  else ifneq ($(strip $(LUA_LIBS)),)
  LUA_MESSAGE := linking with autodetected Lua
  CGIT_LIBS += $(LUA_LIBS)
  CGIT_CFLAGS += $(LUA_CFLAGS)
  else
  LUA_MESSAGE := linking without autodetected Lua support
  NO_LUA := YesPlease
  CGIT_CFLAGS += -DNO_LUA
  endif
 
  endif
 
  # Add -ldl to linker flags on non-BSD systems.
  ifeq ($(findstring BSD,$(uname_S)),)
  CGIT_LIBS += -ldl
  endif
 
 How's this look to you? The correct way to be doing things?

I think it does the right thing for all the explicitly specified
combinations.

Personally I would let the compiler error out if Lua isn't installed,
and add some documentation in Makefile to point users at NO_LUA, but I
don't feel particularly strongly about that, and since you've done the
hard work to make it more intelligent...
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: Policy on global variables

2014-01-16 Thread John Keeping
On Thu, Jan 16, 2014 at 12:31:15PM +0100, Jason A. Donenfeld wrote:
 On Thu, Jan 16, 2014 at 11:47 AM, Eric Wong normalper...@yhbt.net wrote:
  Lars Hjemli hje...@gmail.com wrote:
  Supporting something like FCGI in cgit will require a fork(2) for each
  request, before invoking libgit.a functions, since these functions are
  not generally reentrant (they tend to use global state and/or
  inconveniently die(3)).
 
  Unfortunately true for now, but libgit.a could evolve (or cgit can use
  something like libgit2 instead).
 
 Cgit is unlikely to move to libgit2 in the near future. (Unless
 someone is willing to do the job and argue for why it's preferred over
 mainline git, beyond its reentrancy...)
 
 I guess, though, libgit.a is likely to never evolve to receive
 reentrant functions and do away with die() (though the die calls could
 easily be circumvented by hooking libc's exit...yuck), because libgit2
 exists for this reason.

I had a look at porting to libgit2 about a year ago and it mostly isn't
too bad.  IIRC the only problematic area is the graph output which we
currently get from libgit.a but would have to do ourselves if we switch
to libgit2.
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: Policy on global variables

2014-01-16 Thread John Keeping
On Thu, Jan 16, 2014 at 07:38:02PM +0100, Jason A. Donenfeld wrote:
 On Thu, Jan 16, 2014 at 2:08 PM, John Keeping j...@keeping.me.uk wrote:
 
  I had a look at porting to libgit2 about a year ago and it mostly isn't
  too bad.  IIRC the only problematic area is the graph output which we
  currently get from libgit.a but would have to do ourselves if we switch
  to libgit2.
 
 Are there any downsides of doing this? I know we've put a lot of work
 into cozying up with internal git utility functions and their build
 system. Would we have to reimplement a lot of this? Would it be worth
 it? Are there general benefits of using libgit2 over what we have now?
 Are there general downsides?

Given the CGit and Git are both GPLv2, we could always just take
strbuf.[ch] and the argv-array bits from git.git and copy them into
CGit.  Likewise the test suite could switch to using Sharness with very
little effort.

So I don't think the recent changes towards using more bits of Git
actually have too much impact here.

 More importantly, is this something you would be willing to do, if we
 decided it was the best direction?

I won't have time to do this in the near future.

The first step in this direction may actually be useful even if we stick
with embedding libgit.a.  Re-writing the commit graph drawing ourselves
could allow prettier output than the ASCII we inherit from Git.
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: Policy on global variables

2014-01-16 Thread John Keeping
On Thu, Jan 16, 2014 at 10:26:08PM +0100, Jason A. Donenfeld wrote:
 On Thu, Jan 16, 2014 at 10:21 PM, John Keeping j...@keeping.me.uk wrote:
  The first step in this direction may actually be useful even if we stick
  with embedding libgit.a.
 
 So what do you think ought to be done with the global-ctx patch? Merge
 it, and then refactor afterward (whenever we step in this
 direction), to get rid of the global? Or use what we have now
 (without the patch) as a starting point for gradually getting rid of
 the global?

I'm not sure it makes much difference either way.  Even if we use
libgit2, providing we're not processing more than one request at once we
can still use a global cgit_context.
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: Policy on global variables

2014-01-16 Thread John Keeping
On Thu, Jan 16, 2014 at 10:36:34PM +0100, Jason A. Donenfeld wrote:
 On Thu, Jan 16, 2014 at 10:34 PM, John Keeping j...@keeping.me.uk wrote:
 
  I'm not sure it makes much difference either way.  Even if we use
  libgit2, providing we're not processing more than one request at once we
  can still use a global cgit_context.
 
 Well, the idea of moving to libgit2, in the first place, would be to
 benefit from its reentrancy, so that we could process multiple
 requests at once (potentially).

At once (as in in parallel), or without needing to fork for every
request?  I think that many requests serially in the same process is a
much more likely scenario (that's what FastCGI does); in that case all
we need to do is clean up after each request and it doesn't make much
difference if that state is global or passed down through the functions
that need it.
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: The road to v0.10.1 or v0.11

2014-01-17 Thread John Keeping
On Fri, Jan 17, 2014 at 05:12:26PM +0100, Jason A. Donenfeld wrote:
 Here's what I'm thinking about for the next release (or releases?):
 
 + FastCGI support

I really can't see this being sensible without moving to libgit2.  As
long as we stick with libgit.a then we need to fork for each request so
I'm not sure there's much benefit to supporting FastCGI without moving
to something that lets us free resources when we're done processing a
request.

 + More malloc()/free() cleanups
 + git-blame support
 + git-grep support
 + HTML5 compliance
 + More filters everywhere
 + Expanded test suite
 + Line number anchors highlighting the current line
 + sendfile() support
 
 This is in no particular order. Any opinions about priorities? Or
 anything else to add?

I'd like to get new graph implementation into this list - it's come up
on the list twice in the last 24 hours!  That doesn't mean I'm claiming
the task though ;-)
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: The road to v0.10.1 or v0.11

2014-01-17 Thread John Keeping
On Fri, Jan 17, 2014 at 06:09:15PM +0100, Jason A. Donenfeld wrote:
 On Fri, Jan 17, 2014 at 5:38 PM, Jason A. Donenfeld ja...@zx2c4.com wrote:
  On Fri, Jan 17, 2014 at 5:28 PM, John Keeping j...@keeping.me.uk wrote:
  I really can't see this being sensible without moving to libgit2.  As
  long as we stick with libgit.a then we need to fork for each request so
  I'm not sure there's much benefit to supporting FastCGI without moving
  to something that lets us free resources when we're done processing a
  request.
 
  The advantage would be not having to reparse the config and scan for
  repos on every.single.solitary.request.
 
 Oh, and we could avoid a fork() for cached pages...

Good point.  I think that probably does make it worthwhile.

It may even make sense for a FastCGI process to do:

while read_request
if not in cache:
process and exit
return_cache

and just rely on the web server restarting us.
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: The road to v0.10.1 or v0.11

2014-01-17 Thread John Keeping
On Fri, Jan 17, 2014 at 02:29:19PM -0500, Konstantin Ryabitsev wrote:
 On 17/01/14 01:22 PM, Jason A. Donenfeld wrote:
  But scan for repos is caught by the cache most of the time, and
   presumably even if we run persistently we still need to do that
   periodically (or use inotify); or do we just rely on the process being
   replaced when the set of repositories changes?
  Generally the idea is you restart the fcgi process when things change,
  or at least send it a SIGUSR1. But we could be fancy and support
  inotify/kqueue...
 
 The process that updates the repositories may not have permissions to
 send SIGUSR1 to the fcgid process -- either because they are running as
 different users or because there are SELinux policies preventing it.
 
 It's really best if cgit recognizes when things like projects.list have
 changed.

Presumably you are OK with this having the same latency as the existing
cache mechanism.  The simplest implementation will probably be to keep
the existing cache valid? check and re-scan repositories as we
currently do.
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: Error when searching for a bogus range

2014-06-10 Thread John Keeping
On Tue, Jun 10, 2014 at 02:05:14PM -0400, Konstantin Ryabitsev wrote:
 cgit-0.10.1-1.el6 (EPEL version)
 
 If you search for a bogus range string here:
 
 http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/log/
 
 Using something like range and qwerty123456, it returns an Internal
 Server Error and the following in the logs:
 
  [Tue Jun 10 17:45:32 2014] [error] [client 172.21.1.6] fatal: ambiguous 
  argument 'qwerty123456': unknown revision or path not in the working tree., 
  referer: http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/
  [Tue Jun 10 17:45:32 2014] [error] [client 172.21.1.6] Use '--' to separate 
  paths from revisions, like this:, referer: 
  http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/
  [Tue Jun 10 17:45:32 2014] [error] [client 172.21.1.6] 'git command 
  [revision...] -- [file...]', referer: 
  http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/
  [Tue Jun 10 17:45:32 2014] [error] [client 172.21.1.6] Premature end of 
  script headers: cgit, referer: 
  http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/

This is because we just pass the arguments straight to Git's revision
parsing machinery which die()s if it cannot parse an argument, printing
the above to stderr and exiting.

The patch below makes it a bit friendlier by just ignoring unhandled
arguments, but I can't see an easy way to report errors when we can't
parse revision arguments without losing the flexibility of supporting
all of the revision specifiers supported by Git.

-- 8 --
diff --git a/ui-log.c b/ui-log.c
index 499534c..4cb26bc 100644
--- a/ui-log.c
+++ b/ui-log.c
@@ -337,16 +337,16 @@ void cgit_print_log(const char *tip, int ofs, int cnt, 
char *grep, char *pattern
else if (commit_sort == 2)
argv_array_push(rev_argv, --topo-order);
 
-   if (path) {
-   argv_array_push(rev_argv, --);
+   argv_array_push(rev_argv, --);
+   if (path)
argv_array_push(rev_argv, path);
-   }
 
init_revisions(rev, NULL);
rev.abbrev = DEFAULT_ABBREV;
rev.commit_format = CMIT_FMT_DEFAULT;
rev.verbose_header = 1;
rev.show_root_diff = 0;
+   rev.ignore_missing = 1;
setup_revisions(rev_argv.argc, rev_argv.argv, rev, NULL);
load_ref_decorations(DECORATE_FULL_REFS);
rev.show_decorations = 1;
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


[PATCH] ui-patch: Flush stdout after outputting data

2014-06-11 Thread John Keeping
Since the html functions use raw write(2) to STDIO_FILENO, we don't
notice problems with most pages, but raw patches write using printf(3).
This is fine if we're outputting straight to stdout since the buffers
are flushed on exit, but we close the cache output before this, so the
cached output ends up being truncated.

Make sure the buffers are flushed when we finish outputting a patch so
that we avoid this.

No other UIs use printf(3) so we do not need to worry about them.

Signed-off-by: John Keeping j...@keeping.me.uk
---
 ui-patch.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ui-patch.c b/ui-patch.c
index 6878a46..fc6c145 100644
--- a/ui-patch.c
+++ b/ui-patch.c
@@ -82,4 +82,6 @@ void cgit_print_patch(const char *new_rev, const char 
*old_rev,
log_tree_commit(rev, commit);
printf(-- \ncgit %s\n\n, cgit_version);
}
+
+   fflush(stdout);
 }
-- 
2.0.0.rc4.417.gc642ced

___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: [PATCH] ui-patch: Flush stdout after outputting data

2014-06-11 Thread John Keeping
On Wed, Jun 11, 2014 at 04:28:26PM -0400, Konstantin Ryabitsev wrote:
 On 11/06/14 04:01 PM, John Keeping wrote:
  Since the html functions use raw write(2) to STDIO_FILENO, we don't
  notice problems with most pages, but raw patches write using printf(3).
  This is fine if we're outputting straight to stdout since the buffers
  are flushed on exit, but we close the cache output before this, so the
  cached output ends up being truncated.

Actually, it's slightly more interesting than this... since we don't set
GIT_FLUSH, Git decides whether or not it will flush stdout after writing
each commit based on whether or not stdout points to a regular file (in
maybe_flush_or_die()).

Which means that when writing directly to the webserver, Git flushes
stdout for us, but when we redirect stdout to the cache it points to a
regular file so Git no longer flushes the output for us.

The patch is still correct, but perhaps the full explanation is
interesting!
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: [PATCH 3/8] Skip forbidden characters.

2014-07-01 Thread John Keeping
On Tue, Jul 01, 2014 at 09:40:28AM +0200, zwin...@kit.edu wrote:
 From: Sebastian Buchwald sebastian.buchw...@kit.edu

Why do we want to do this?  Does it not break anything that uses
whitespace=pre (explicitly or implicitly)?

 ---
  html.c | 9 ++---
  1 file changed, 6 insertions(+), 3 deletions(-)
 
 diff --git a/html.c b/html.c
 index 91047ad..6037eec 100644
 --- a/html.c
 +++ b/html.c
 @@ -129,7 +129,8 @@ void html_txt(const char *txt)
   const char *t = txt;
   while (t  *t) {
   int c = *t;
 - if (c == '' || c == '' || c == '') {
 + if ((c  0x20  c != '\t'  c != '\n'  c != '\r')
 + || (c == '' || c == '' || c == '')) {
   html_raw(txt, t - txt);
   if (c == '')
   html(gt;);
 @@ -150,7 +151,8 @@ void html_ntxt(int len, const char *txt)
   const char *t = txt;
   while (t  *t  len--) {
   int c = *t;
 - if (c == '' || c == '' || c == '') {
 + if ((c  0x20  c != '\t'  c != '\n'  c != '\r')
 + || (c == '' || c == '' || c == '')) {
   html_raw(txt, t - txt);
   if (c == '')
   html(gt;);
 @@ -186,7 +188,8 @@ void html_attr(const char *txt)
   const char *t = txt;
   while (t  *t) {
   int c = *t;
 - if (c == '' || c == '' || c == '\'' || c == '\' || c == '') 
 {
 + if (c == '' || c == '' || c == '\'' || c == '\' || c == ''
 + || (c  0x20  c != '\t'  c != '\n'  c != 
 '\r')) {
   html_raw(txt, t - txt);
   if (c == '')
   html(gt;);
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: certificate problem with libravatar

2014-07-03 Thread John Keeping
On Thu, Jul 03, 2014 at 11:16:21AM +0200, Christian Hesse wrote:
 looks like we have a certificate problem with libravatar email filter. For
 base URL we use //cdn.libravatar.org/, with is fine if cgit serves
 unencrypted html pages. The url evaluates to http://cdn.libravatar.org/;
 then. However if cgit sends an encrypted site the url is
 https://cdn.libravatar.org/;, with results in a certificate error as CN does
 not match.
 
 We could just change the url to //seccdn.libravatar.org/ or
 https://seccdn.libravatar.org/;, but that would fetch the avatar via https
 all the some. In fact the first one makes two requests as the http server
 redirects to https one.
 
 Does the script know whether or not the site is encrypted? That would allow
 us to choose the correct url. Any other ideas?

FWIW my vote would be to always use https://seccdn.libravatar.org/;,
since HTTP-HTTPS is OK but HTTPS-HTTP is not and if HTTP is just going
to redirect to HTTPS then we might as well go directly to the HTTPS.
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: owner filter?

2014-07-17 Thread John Keeping
On Thu, Jul 17, 2014 at 10:10:38AM -0400, Chris Burroughs wrote:
 We would like to decorate the owner field (make a link to a wiki for 
 internal teams, maybe an icon).  I've read the FILTER API and other 
 parts of cgitrc and as far as I can tell it's not an existing filter 
 option.  Is that correct?

Yes, it would need to new filter added in order to filter the owner
field in cgit_print_repolist().  Presumably the interface would be
similar to that of the email filter.
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


[PATCH 3/3] ui-stats.c: set parent pointer to NULL after freeing it

2014-07-27 Thread John Keeping
We do this everywhere else, so we should be doing it here as well.

Signed-off-by: John Keeping j...@keeping.me.uk
---
 ui-stats.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ui-stats.c b/ui-stats.c
index 6f13c32..a264f6a 100644
--- a/ui-stats.c
+++ b/ui-stats.c
@@ -246,6 +246,7 @@ static struct string_list collect_stats(struct cgit_period 
*period)
add_commit(authors, commit, period);
free_commit_buffer(commit);
free_commit_list(commit-parents);
+   commit-parents = NULL;
}
return authors;
 }
-- 
2.0.1.472.g6f92e5f.dirty

___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


[PATCH 2/3] git: update to v2.0.3

2014-07-27 Thread John Keeping
This is slightly more involved than just bumping the version number
because it pulls in a change to convert the commit buffer to a slab,
removing the buffer field from struct commit.  All sites that access
commit-buffer have been changed to use the new functions provided for
this purpose.

Signed-off-by: John Keeping j...@keeping.me.uk
---
 Makefile   | 2 +-
 git| 2 +-
 parsing.c  | 3 ++-
 ui-atom.c  | 3 +--
 ui-log.c   | 6 ++
 ui-stats.c | 2 +-
 6 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/Makefile b/Makefile
index bf8be02..93b525a 100644
--- a/Makefile
+++ b/Makefile
@@ -14,7 +14,7 @@ htmldir = $(docdir)
 pdfdir = $(docdir)
 mandir = $(prefix)/share/man
 SHA1_HEADER = openssl/sha.h
-GIT_VER = 2.0.1
+GIT_VER = 2.0.3
 GIT_URL = https://www.kernel.org/pub/software/scm/git/git-$(GIT_VER).tar.gz
 INSTALL = install
 COPYTREE = cp -r
diff --git a/git b/git
index 341e7e8..740c281 16
--- a/git
+++ b/git
@@ -1 +1 @@
-Subproject commit 341e7e8eda3dbeb6867f4f8f45b671201b807de5
+Subproject commit 740c281d21ef5b27f6f1b942a4f2fc20f51e8c7e
diff --git a/parsing.c b/parsing.c
index edb3416..3dbd122 100644
--- a/parsing.c
+++ b/parsing.c
@@ -132,7 +132,8 @@ static const char *reencode(char **txt, const char 
*src_enc, const char *dst_enc
 struct commitinfo *cgit_parse_commit(struct commit *commit)
 {
struct commitinfo *ret;
-   const char *p = commit-buffer, *t;
+   const char *p = get_cached_commit_buffer(commit, NULL);
+   const char *t;
 
ret = xmalloc(sizeof(*ret));
ret-commit = commit;
diff --git a/ui-atom.c b/ui-atom.c
index b22d745..e2b39ee 100644
--- a/ui-atom.c
+++ b/ui-atom.c
@@ -133,8 +133,7 @@ void cgit_print_atom(char *tip, char *path, int max_count)
}
while ((commit = get_revision(rev)) != NULL) {
add_entry(commit, host);
-   free(commit-buffer);
-   commit-buffer = NULL;
+   free_commit_buffer(commit);
free_commit_list(commit-parents);
commit-parents = NULL;
}
diff --git a/ui-log.c b/ui-log.c
index b5846e4..bcdb666 100644
--- a/ui-log.c
+++ b/ui-log.c
@@ -388,16 +388,14 @@ void cgit_print_log(const char *tip, int ofs, int cnt, 
char *grep, char *pattern
ofs = 0;
 
for (i = 0; i  ofs  (commit = get_revision(rev)) != NULL; i++) {
-   free(commit-buffer);
-   commit-buffer = NULL;
+   free_commit_buffer(commit);
free_commit_list(commit-parents);
commit-parents = NULL;
}
 
for (i = 0; i  cnt  (commit = get_revision(rev)) != NULL; i++) {
print_commit(commit, rev);
-   free(commit-buffer);
-   commit-buffer = NULL;
+   free_commit_buffer(commit);
free_commit_list(commit-parents);
commit-parents = NULL;
}
diff --git a/ui-stats.c b/ui-stats.c
index bc27308..6f13c32 100644
--- a/ui-stats.c
+++ b/ui-stats.c
@@ -244,7 +244,7 @@ static struct string_list collect_stats(struct cgit_period 
*period)
memset(authors, 0, sizeof(authors));
while ((commit = get_revision(rev)) != NULL) {
add_commit(authors, commit, period);
-   free(commit-buffer);
+   free_commit_buffer(commit);
free_commit_list(commit-parents);
}
return authors;
-- 
2.0.1.472.g6f92e5f.dirty

___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


[PATCH 0/3] Update to Git 2.0.3

2014-07-27 Thread John Keeping
This isn't as simple as just bumping the version number because the
buffer field has been removed from struct commit so we need to
change all of the places that use it to use the new wrapper functions.

The first commit is a preparatory change because the new access function
returns a pointer-to-const where the buffer field was previously just
a char *.  The middle commit is the real change and the final one is
just an unrelated fix I noticed while in the area.

John Keeping (3):
  parsing.c: make commit buffer const
  git: update to v2.0.3
  ui-stats.c: set parent pointer to NULL after freeing it

 Makefile   | 2 +-
 git| 2 +-
 parsing.c  | 9 +
 ui-atom.c  | 3 +--
 ui-log.c   | 6 ++
 ui-stats.c | 3 ++-
 6 files changed, 12 insertions(+), 13 deletions(-)

-- 
2.0.1.472.g6f92e5f.dirty

___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: source-filter isn't getting applied

2014-07-28 Thread John Keeping
On Mon, Jul 28, 2014 at 05:14:43PM -0400, Nik Nyby wrote:
 I have cgit installed and the source-filter isn't working on any of my
 source files. I have Python and Pygments installed. I tried manually
 running the script on some files, and it's giving back html
 correctly.
 
 Here's my /etc/cgitrc:
 
 scan-path=/home/git
 
 branch-sort=age
 repository-sort=age
 
 source-filter=/usr/local/lib/cgit/filters/syntax-highlighting.py
 about-filter=/usr/local/lib/cgit/filters/about-formatting.sh
 
 
 My cgit is installed in /var/www/cgit, and I'm using cgit v0.10.2.
 
 Let me know if you have any suggestions.

What are the permissions on the script files?  How do you run them
manually?

Assuming that CGit is definitely reading that `cgitrc` file, I'd guess
that the permissions are not set correctly for the user `cgit` runs as
when called from your web server.

If all else fails, you can try moving CGit out of the way and replacing
it with something like this:

-- 8 --
#!/bin/sh
strace -o /tmp/cgit.strace /path/to/real/cgit $@
-- 8 --
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: Fwd: source-filter isn't getting applied

2014-07-29 Thread John Keeping
On Mon, Jul 28, 2014 at 11:59:10PM -0400, Nik Nyby wrote:
 The permissions on the script files are set to be executable by everyone:
 -rwxr-xr-x
 
 Thanks for the strace idea. I'm looking through the strace, but I haven't
 seen any helpful mention of the filter scripts yet. I've attached an
 abridged version of my strace with the middle of the file taken out. I
 don't know if this is helpful or not. I'll let you know if I solve the
 problem.

If CGit was going to open a filter, it would do so after this line:

write(1, td class='lines'precode, 29) = 29

So it looks like something in the configuration isn't being read
correctly, but I think you snipped the strace log before we could see
which config file it reads.
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: commit-filter not being applied on log page

2014-08-01 Thread John Keeping
On Thu, Jul 31, 2014 at 10:27:19AM -0400, Chris Burroughs wrote:
 I'm trying to write a commit-filter to hyperlink references to our bug 
 tracker.  It seems to work fine for commit detailed pages, but not at 
 all for the log view.  Since our developers often make brief messages 
 like fixes #123' so getting the url in the log view is arguably more 
 useful than in the detail page.

I think this is an issue of the cost of forking a filter process for
each line in the log view.  Now that we have Lua filters that may not be
so much of an issue, but I don't think we can just start using the
source filter on the log view due to the impact that will have on people
with an exec source-filter already configured.

Perhaps we need to add a log-filter which you could configure to be
the same as source-filter but which can be left blank for people whose
links are normally in the body of the commit message.
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: [PATCH] filter: add support for owner-filter

2014-08-01 Thread John Keeping
On Fri, Aug 01, 2014 at 04:01:53PM -0400, Chris Burroughs wrote:
 revised patch

This type of comment should go below the --- line below, since it's
not intended to be part of the commit message in the permanent history.
Also filter in the subject doesn't really identify a code area.  How
about something like this:

repolist: add owner-filter

This allows custom links to be used for repository owners by
configuring a filter to be applied in the Owner column in the
repository list.

More below...

 ---
  cgit.c|6 ++
  cgit.h|4 +++-
  cgitrc.5.txt  |   18 ++
  filter.c  |6 ++
  filters/owner-example.lua |   19 +++
  shared.c  |1 +
  ui-repolist.c |   20 +---
  7 files changed, 66 insertions(+), 8 deletions(-)
  create mode 100644 filters/owner-example.lua
 
 diff --git a/cgit.c b/cgit.c
 index 20f6e27..d29e7f5 100644
 --- a/cgit.c
 +++ b/cgit.c
 @@ -91,6 +91,8 @@ static void repo_config(struct cgit_repo *repo, const char 
 *name, const char *va
   repo-source_filter = cgit_new_filter(value, SOURCE);
   else if (!strcmp(name, email-filter))
   repo-email_filter = cgit_new_filter(value, EMAIL);
 + else if (!strcmp(name, owner-filter))
 + repo-owner_filter = cgit_new_filter(value, OWNER);
   }
  }
  
 @@ -194,6 +196,8 @@ static void config_cb(const char *name, const char *value)
   ctx.cfg.commit_filter = cgit_new_filter(value, COMMIT);
   else if (!strcmp(name, email-filter))
   ctx.cfg.email_filter = cgit_new_filter(value, EMAIL);
 + else if (!strcmp(name, owner-filter))
 + ctx.cfg.owner_filter = cgit_new_filter(value, OWNER);
   else if (!strcmp(name, auth-filter))
   ctx.cfg.auth_filter = cgit_new_filter(value, AUTH);
   else if (!strcmp(name, embedded))
 @@ -802,6 +806,8 @@ static void print_repo(FILE *f, struct cgit_repo *repo)
   cgit_fprintf_filter(repo-source_filter, f, 
 repo.source-filter=);
   if (repo-email_filter  repo-email_filter != ctx.cfg.email_filter)
   cgit_fprintf_filter(repo-email_filter, f, 
 repo.email-filter=);
 + if (repo-owner_filter  repo-owner_filter != ctx.cfg.owner_filter)
 + cgit_fprintf_filter(repo-owner_filter, f, 
 repo.owner-filter=);
   if (repo-snapshots != ctx.cfg.snapshots) {
   char *tmp = build_snapshot_setting(repo-snapshots);
   fprintf(f, repo.snapshots=%s\n, tmp ? tmp : );
 diff --git a/cgit.h b/cgit.h
 index 0badc64..0078ac1 100644
 --- a/cgit.h
 +++ b/cgit.h
 @@ -53,7 +53,7 @@ typedef void (*filepair_fn)(struct diff_filepair *pair);
  typedef void (*linediff_fn)(char *line, int len);
  
  typedef enum {
 - ABOUT, COMMIT, SOURCE, EMAIL, AUTH
 + ABOUT, COMMIT, SOURCE, EMAIL, AUTH, OWNER
  } filter_type;
  
  struct cgit_filter {
 @@ -100,6 +100,7 @@ struct cgit_repo {
   struct cgit_filter *commit_filter;
   struct cgit_filter *source_filter;
   struct cgit_filter *email_filter;
 + struct cgit_filter *owner_filter;
   struct string_list submodules;
  };
  
 @@ -253,6 +254,7 @@ struct cgit_config {
   struct cgit_filter *commit_filter;
   struct cgit_filter *source_filter;
   struct cgit_filter *email_filter;
 + struct cgit_filter *owner_filter;
   struct cgit_filter *auth_filter;
  };
  
 diff --git a/cgitrc.5.txt b/cgitrc.5.txt
 index b7570db..d774ed3 100644
 --- a/cgitrc.5.txt
 +++ b/cgitrc.5.txt
 @@ -247,6 +247,15 @@ logo-link::
   calculated url of the repository index page will be used. Default
   value: none.
  
 +owner-filter::
 + Specifies a command which will be invoked to format the Owner
 + column of the main page.  The command will get the owner on STDIN,
 + and the STDOUT from the command will be included verbatim in the
 + table.  This can be used to link to additional context such as an
 + owners home page.  When active this filter is used instead of the
 + default owner query url.  Default value: none.
 + See also: FILTER API.
 +
  max-atom-items::
   Specifies the number of items to display in atom feeds view. Default
   value: 10.
 @@ -509,6 +518,10 @@ repo.logo-link::
   calculated url of the repository index page will be used. Default
   value: global logo-link.
  
 +repo.owner-filter::
 + Override the default owner-filter. Default value: none. See also:
 + enable-filter-overrides. See also: FILTER API.
 +
  repo.module-link::
   Text which will be used as the formatstring for a hyperlink when a
   submodule is printed in a directory listing. The arguments for the
 @@ -641,6 +654,11 @@ email filter::
   expected to write to standard output the formatted text to be included
   in the page.
  
 +owner 

[PATCH 2/3] ui-summary: add rel='vcs-git' to clone URL links

2014-08-01 Thread John Keeping
This is described in the rel-vcs microformat[1].

[1] https://joeyh.name/rfc/rel-vcs/

Signed-off-by: John Keeping j...@keeping.me.uk
---
 ui-summary.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/ui-summary.c b/ui-summary.c
index 70ea908..46ca713 100644
--- a/ui-summary.c
+++ b/ui-summary.c
@@ -31,9 +31,11 @@ static void print_url(const char *url)
htmlf(trth class='left' colspan='%d'Clone/th/tr\n, 
columns);
}
 
-   htmlf(trtd colspan='%d'a href=', columns);
+   htmlf(trtd colspan='%d'a rel='vcs-git' href=', columns);
html_url_path(url);
-   html(');
+   html(' title=');
+   html_attr(ctx.repo-name);
+   html( Git repository');
html_txt(url);
html(/a/td/tr\n);
 }
-- 
2.0.3.890.g700db9e

___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


[PATCH 1/3] Extract clone URL printing to ui-shared.c

2014-08-01 Thread John Keeping
This will allow us to reuse the same logic to add clone URL link/
elements to the header of all repo-specific pages in order to support
the rel-vcs microformat.

Signed-off-by: John Keeping j...@keeping.me.uk
---
 ui-shared.c  | 37 +
 ui-shared.h  |  2 ++
 ui-summary.c | 58 --
 3 files changed, 51 insertions(+), 46 deletions(-)

diff --git a/ui-shared.c b/ui-shared.c
index 9dde0a3..5bae02d 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -728,6 +728,43 @@ void cgit_print_docend()
html(/body\n/html\n);
 }
 
+static void add_clone_urls(void (*fn)(const char *), char *txt, char *suffix)
+{
+   struct strbuf buf = STRBUF_INIT;
+   char *h = txt, *t, c;
+
+   while (h  *h) {
+   while (h  *h == ' ')
+   h++;
+   if (!*h)
+   break;
+   t = h;
+   while (t  *t  *t != ' ')
+   t++;
+   c = *t;
+   *t = 0;
+
+   if (suffix  *suffix) {
+   strbuf_reset(buf);
+   strbuf_addf(buf, %s/%s, h, suffix);
+   h = buf.buf;
+   }
+   fn(h);
+   *t = c;
+   h = t;
+   }
+
+   strbuf_release(buf);
+}
+
+void cgit_add_clone_urls(void (*fn)(const char *))
+{
+   if (ctx.repo-clone_url)
+   add_clone_urls(fn, expand_macros(ctx.repo-clone_url), NULL);
+   else if (ctx.cfg.clone_prefix)
+   add_clone_urls(fn, ctx.cfg.clone_prefix, ctx.repo-url);
+}
+
 static int print_branch_option(const char *refname, const unsigned char *sha1,
   int flags, void *cb_data)
 {
diff --git a/ui-shared.h b/ui-shared.h
index 3e7a91b..79662f7 100644
--- a/ui-shared.h
+++ b/ui-shared.h
@@ -11,6 +11,8 @@ extern char *cgit_fileurl(const char *reponame, const char 
*pagename,
 extern char *cgit_pageurl(const char *reponame, const char *pagename,
  const char *query);
 
+extern void cgit_add_clone_urls(void (*fn)(const char *));
+
 extern void cgit_index_link(const char *name, const char *title,
const char *class, const char *pattern, const char 
*sort, int ofs);
 extern void cgit_summary_link(const char *name, const char *title,
diff --git a/ui-summary.c b/ui-summary.c
index 3728c3e..70ea908 100644
--- a/ui-summary.c
+++ b/ui-summary.c
@@ -12,62 +12,30 @@
 #include ui-log.h
 #include ui-refs.h
 #include ui-blob.h
+#include ui-shared.h
 #include libgen.h
 
-static void print_url(char *base, char *suffix)
+static int urls;
+
+static void print_url(const char *url)
 {
int columns = 3;
-   struct strbuf basebuf = STRBUF_INIT;
 
if (ctx.repo-enable_log_filecount)
columns++;
if (ctx.repo-enable_log_linecount)
columns++;
 
-   if (!base || !*base)
-   return;
-   if (suffix  *suffix) {
-   strbuf_addf(basebuf, %s/%s, base, suffix);
-   base = basebuf.buf;
+   if (urls++ == 0) {
+   htmlf(tr class='nohover'td colspan='%d'nbsp;/td/tr, 
columns);
+   htmlf(trth class='left' colspan='%d'Clone/th/tr\n, 
columns);
}
+
htmlf(trtd colspan='%d'a href=', columns);
-   html_url_path(base);
+   html_url_path(url);
html(');
-   html_txt(base);
+   html_txt(url);
html(/a/td/tr\n);
-   strbuf_release(basebuf);
-}
-
-static void print_urls(char *txt, char *suffix)
-{
-   char *h = txt, *t, c;
-   int urls = 0;
-   int columns = 3;
-
-   if (ctx.repo-enable_log_filecount)
-   columns++;
-   if (ctx.repo-enable_log_linecount)
-   columns++;
-
-
-   while (h  *h) {
-   while (h  *h == ' ')
-   h++;
-   if (!*h)
-   break;
-   t = h;
-   while (t  *t  *t != ' ')
-   t++;
-   c = *t;
-   *t = 0;
-   if (urls++ == 0) {
-   htmlf(tr class='nohover'td 
colspan='%d'nbsp;/td/tr, columns);
-   htmlf(trth class='left' 
colspan='%d'Clone/th/tr\n, columns);
-   }
-   print_url(h, suffix);
-   *t = c;
-   h = t;
-   }
 }
 
 void cgit_print_summary()
@@ -88,10 +56,8 @@ void cgit_print_summary()
cgit_print_log(ctx.qry.head, 0, ctx.cfg.summary_log, NULL,
   NULL, NULL, 0, 0, 0);
}
-   if (ctx.repo-clone_url)
-   print_urls(expand_macros(ctx.repo-clone_url), NULL);
-   else if (ctx.cfg.clone_prefix)
-   print_urls(ctx.cfg.clone_prefix, ctx.repo-url);
+   urls = 0;
+   cgit_add_clone_urls(print_url);
html(/table);
 }
 
-- 
2.0.3.890.g700db9e

[PATCH 3/3] ui-shared: add rel-vcs microformat links to HTML header

2014-08-01 Thread John Keeping
As described at https://joeyh.name/rfc/rel-vcs/.

Signed-off-by: John Keeping j...@keeping.me.uk
---
 ui-shared.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/ui-shared.c b/ui-shared.c
index 5bae02d..9ac65ab 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -661,6 +661,15 @@ void cgit_print_http_headers(void)
exit(0);
 }
 
+static void print_rel_vcs_link(const char *url)
+{
+   html(link rel='vcs-git' href=');
+   html_attr(url);
+   html(' title=');
+   html_attr(ctx.repo-name);
+   html( Git repository'/\n);
+}
+
 void cgit_print_docstart(void)
 {
if (ctx.cfg.embedded) {
@@ -699,6 +708,8 @@ void cgit_print_docstart(void)
html(' type='application/atom+xml'/\n);
strbuf_release(sb);
}
+   if (ctx.repo)
+   cgit_add_clone_urls(print_rel_vcs_link);
if (ctx.cfg.head_include)
html_include(ctx.cfg.head_include);
html(/head\n);
-- 
2.0.3.890.g700db9e

___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


[PATCH] git: update to v2.0.4

2014-08-03 Thread John Keeping
No CGit changes required.

Signed-off-by: John Keeping j...@keeping.me.uk
---
 Makefile | 2 +-
 git  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 93b525a..6a8a125 100644
--- a/Makefile
+++ b/Makefile
@@ -14,7 +14,7 @@ htmldir = $(docdir)
 pdfdir = $(docdir)
 mandir = $(prefix)/share/man
 SHA1_HEADER = openssl/sha.h
-GIT_VER = 2.0.3
+GIT_VER = 2.0.4
 GIT_URL = https://www.kernel.org/pub/software/scm/git/git-$(GIT_VER).tar.gz
 INSTALL = install
 COPYTREE = cp -r
diff --git a/git b/git
index 740c281..32f5660 16
--- a/git
+++ b/git
@@ -1 +1 @@
-Subproject commit 740c281d21ef5b27f6f1b942a4f2fc20f51e8c7e
+Subproject commit 32f56600bb6ac6fc57183e79d2c1515dfa56672f
-- 
2.0.3.890.g700db9e

___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: Compile cGit 2.0.4

2014-08-11 Thread John Keeping
On Mon, Aug 11, 2014 at 08:40:53PM +0100, Jorge Bastos wrote:
 Trying to compile cgit last git head cgit (2.0.4) to see if one charset
 problem is solved, but got this:
 
 Peixe:/usr/local/src/cgit/cgit# make
 SUBDIR git
 ./gen-version.sh: line 2: $'\r': command not found
 ./gen-version.sh: line 5: $'\r': command not found
 ./gen-version.sh: line 21: syntax error: unexpected end of file
 make[1]: *** No rule to make target `../VERSION', needed by `../cgit.o'.
 Stop.
 make: *** [cgit] Error 2

Do you have a weird CGIT_VERSION defined in your environment?  The
string $'\r' doesn't appear in gen-version.sh so I don't understand
where that error can be coming from.

Alternatively do you have a weird (non-POSIX) /bin/sh?  What operating
system are you using?
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: [PATCH] Handle If-None-Match HTTP header in plain view

2014-08-11 Thread John Keeping
On Mon, Aug 11, 2014 at 05:53:23PM -0300, Damián Nohales wrote:
 We are sending Etag to clients but this header is basically unusefulness
 if the server doesn't tell the client if the content has been changed or
 not for a given Path/Etag pair.
 
 Signed-off-by: Damián Nohales damiannoha...@gmail.com
 ---

How does this interact with CGit's cache?  What happens if the original
page expires from the cache and then a request comes in with a matching
Etag?

  cgit.c  |  1 +
  cgit.h  |  1 +
  ui-plain.c  | 41 +
  ui-shared.c | 20 
  ui-shared.h |  1 +
  5 files changed, 44 insertions(+), 20 deletions(-)
 
 diff --git a/cgit.c b/cgit.c
 index 8c4517d..7af7712 100644
 --- a/cgit.c
 +++ b/cgit.c
 @@ -385,6 +385,7 @@ static void prepare_context(void)
   ctx.env.server_port = getenv(SERVER_PORT);
   ctx.env.http_cookie = getenv(HTTP_COOKIE);
   ctx.env.http_referer = getenv(HTTP_REFERER);
 + ctx.env.if_none_match = getenv(HTTP_IF_NONE_MATCH);
   ctx.env.content_length = getenv(CONTENT_LENGTH) ? 
 strtoul(getenv(CONTENT_LENGTH), NULL, 10) : 0;
   ctx.env.authenticated = 0;
   ctx.page.mimetype = text/html;
 diff --git a/cgit.h b/cgit.h
 index 0badc64..eddd4c7 100644
 --- a/cgit.h
 +++ b/cgit.h
 @@ -282,6 +282,7 @@ struct cgit_environment {
   const char *server_port;
   const char *http_cookie;
   const char *http_referer;
 + const char *if_none_match;
   unsigned int content_length;
   int authenticated;
  };
 diff --git a/ui-plain.c b/ui-plain.c
 index 30fff89..a08dc5b 100644
 --- a/ui-plain.c
 +++ b/ui-plain.c
 @@ -103,8 +103,8 @@ static int print_object(const unsigned char *sha1, const 
 char *path)
   ctx.page.filename = path;
   ctx.page.size = size;
   ctx.page.etag = sha1_to_hex(sha1);
 - cgit_print_http_headers();
 - html_raw(buf, size);
 + if (!cgit_print_http_headers_matching_etag())
 + html_raw(buf, size);
   /* If we allocated this, then casting away const is safe. */
   if (freemime)
   free((char*) ctx.page.mimetype);
 @@ -128,24 +128,25 @@ static void print_dir(const unsigned char *sha1, const 
 char *base,
   fullpath = buildpath(base, baselen, path);
   slash = (fullpath[0] == '/' ?  : /);
   ctx.page.etag = sha1_to_hex(sha1);
 - cgit_print_http_headers();
 - htmlf(htmlheadtitle%s, slash);
 - html_txt(fullpath);
 - htmlf(/title/head\nbody\nh2%s, slash);
 - html_txt(fullpath);
 - html(/h2\nul\n);
 - len = strlen(fullpath);
 - if (len  1) {
 - fullpath[len - 1] = 0;
 - slash = strrchr(fullpath, '/');
 - if (slash)
 - *(slash + 1) = 0;
 - else
 - fullpath = NULL;
 - html(li);
 - cgit_plain_link(../, NULL, NULL, ctx.qry.head, ctx.qry.sha1,
 - fullpath);
 - html(/li\n);
 + if (!cgit_print_http_headers_matching_etag()) {
 + htmlf(htmlheadtitle%s, slash);
 + html_txt(fullpath);
 + htmlf(/title/head\nbody\nh2%s, slash);
 + html_txt(fullpath);
 + html(/h2\nul\n);
 + len = strlen(fullpath);
 + if (len  1) {
 + fullpath[len - 1] = 0;
 + slash = strrchr(fullpath, '/');
 + if (slash)
 + *(slash + 1) = 0;
 + else
 + fullpath = NULL;
 + html(li);
 + cgit_plain_link(../, NULL, NULL, ctx.qry.head, 
 ctx.qry.sha1,
 + fullpath);
 + html(/li\n);
 + }
   }
   free(fullpath);
  }
 diff --git a/ui-shared.c b/ui-shared.c
 index 9dde0a3..84c7efd 100644
 --- a/ui-shared.c
 +++ b/ui-shared.c
 @@ -661,6 +661,26 @@ void cgit_print_http_headers(void)
   exit(0);
  }
  
 +int cgit_print_http_headers_matching_etag(void)
 +{
 + int match = 0;
 + char *etag;
 + if (ctx.page.etag  ctx.env.if_none_match) {
 + etag = fmtalloc(\%s\, ctx.page.etag);
 + if (!strcmp(etag, ctx.env.if_none_match)) {
 + ctx.page.status = 304;
 + ctx.page.statusmsg = Not Modified;
 + ctx.page.mimetype = NULL;
 + ctx.page.size = 0;
 + ctx.page.filename = NULL;
 + match = 1;
 + }
 + free(etag);
 + }
 + cgit_print_http_headers();
 + return match;
 +}
 +
  void cgit_print_docstart(void)
  {
   if (ctx.cfg.embedded) {
 diff --git a/ui-shared.h b/ui-shared.h
 index 3e7a91b..e279f42 100644
 --- a/ui-shared.h
 +++ b/ui-shared.h
 @@ -60,6 +60,7 @@ extern void cgit_vprint_error(const char *fmt, va_list ap);
  extern void cgit_print_date(time_t secs, const char 

Re: [PATCH] Handle If-None-Match HTTP header in plain view

2014-08-13 Thread John Keeping
On Tue, Aug 12, 2014 at 06:53:01PM -0300, Damián Nohales wrote:
 2014-08-12 16:15 GMT-03:00 John Keeping j...@keeping.me.uk:
  If we have sufficient infrastructure to handle HEAD requests then it
  should be trivial to add proper Etag handling on top, but I don't think
  it's trivial to add that.
 
 With this patch, the Etag verification works on both GET and HEAD
 requests, CGit only disable cache and force the exit after
 cgit_print_http_headers when the request is HEAD, that doesn't
 interfere with the Etag calculation.
 
 What interfere with this is the server's cache. To do it right with
 interoperability with server cache, we need to save to the cache the
 200 OK response independently on Etag matches so we can retrieve
 Etag by parsing the cache instead of calculating it in future request.
 Then we need an extra step after getting the content from the cache
 that decides between actually sends the content or sends a 304 Not
 Modified.

I think the only sensible approach would be to put a handler between
cache_process() and stdout when If-None-Match is present in the request
so that we can return 304 if the Etag matches, then it doesn't matter
whether we're streaming from the cache or not and we just have to leave
the original request running after returning the response so that it
streams the body to the cache.

I was assuming that if we handled HEAD requests we would want to do
something similar and not just bypass the cache, but I had not actually
looked at the code.

  2014-08-12 6:00 GMT-03:00 John Keeping j...@keeping.me.uk:
   On Mon, Aug 11, 2014 at 07:45:53PM -0300, Damián Nohales wrote:
   (Sorry, I forgot to include the list address in my response.)
  
   This does not interact with CGit's cache at all, this interacts with
   client side cache.
  
   If the client sends an Etag it means Hey, I have the content cached
   for this URL in the client and its Etag is abc123, so, for a given
   URL, if server calculates the Etag and results to the same as the Etag
   sent by client, it means Ok, client has the same Etag than me, so
   client's cached content is up-to-date.
  
   The Etag is calculated from the requested object hash. I'm not a Git
   expert, but I know that the hash will be modified when the file is
   modified by a commit.
  
   So, if we request a file, then, one year later, we request the same
   file and we still have it cached with its Etag, CGit will respond 304
   Not Modified if the file was not modified by newer commits (CGit
   doesn't check its internal cache, it checks the client's cache).
  
   Actually, we should avoid send Expires and Last-Modified when
   Etag is available, so the caching information is consistent (and
   also, using Etag we have a more precise and smart caching).
  
   Yes, but I think the code you've changed comes after CGit has duplicated
   stdout into the cache file.  So if a request comes in with an
   If-None-Match header, then CGit will cache the 304 response and send it
   in response to all future request for the same page (until the cached
   copy times out).
  
   2014-08-11 18:32 GMT-03:00 John Keeping j...@keeping.me.uk:
On Mon, Aug 11, 2014 at 05:53:23PM -0300, Damián Nohales wrote:
We are sending Etag to clients but this header is basically 
unusefulness
if the server doesn't tell the client if the content has been 
changed or
not for a given Path/Etag pair.
   
Signed-off-by: Damián Nohales damiannoha...@gmail.com
---
   
How does this interact with CGit's cache?  What happens if the 
original
page expires from the cache and then a request comes in with a 
matching
Etag?
   
 cgit.c  |  1 +
 cgit.h  |  1 +
 ui-plain.c  | 41 +
 ui-shared.c | 20 
 ui-shared.h |  1 +
 5 files changed, 44 insertions(+), 20 deletions(-)
   
diff --git a/cgit.c b/cgit.c
index 8c4517d..7af7712 100644
--- a/cgit.c
+++ b/cgit.c
@@ -385,6 +385,7 @@ static void prepare_context(void)
  ctx.env.server_port = getenv(SERVER_PORT);
  ctx.env.http_cookie = getenv(HTTP_COOKIE);
  ctx.env.http_referer = getenv(HTTP_REFERER);
+ ctx.env.if_none_match = getenv(HTTP_IF_NONE_MATCH);
  ctx.env.content_length = getenv(CONTENT_LENGTH) ? 
strtoul(getenv(CONTENT_LENGTH), NULL, 10) : 0;
  ctx.env.authenticated = 0;
  ctx.page.mimetype = text/html;
diff --git a/cgit.h b/cgit.h
index 0badc64..eddd4c7 100644
--- a/cgit.h
+++ b/cgit.h
@@ -282,6 +282,7 @@ struct cgit_environment {
  const char *server_port;
  const char *http_cookie;
  const char *http_referer;
+ const char *if_none_match;
  unsigned int content_length;
  int authenticated;
 };
diff --git a/ui-plain.c b/ui-plain.c
index 30fff89..a08dc5b 100644
--- a/ui-plain.c
+++ b/ui-plain.c
@@ -103,8 +103,8 @@ static

Re: regular expression in search

2014-08-25 Thread John Keeping
On Sat, Aug 23, 2014 at 03:49:52PM +0200, William Bell wrote:
 I would like to use a regular expression in the search field as in
 gitweb.

What do you mean by this?  The log msg, author and committer
searches already take regular expressions.

Do you mean a pickaxe search option?
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Bash vulnerability (CVE-2014-6271)

2014-09-24 Thread John Keeping
In case anyone hasn't seen it yet, today's Bash vulnerability
(CVE-2014-6271) [0] may affect CGit servers.

I don't believe CGit in its default configuration will cause a shell to
be executed, but if you configure a filter then you may well be causing
a shell to be executed with the environment of the cgit process, which
will include user-specified variables such as the HTTP User-Agent
header.

The example syntax-highlighting.sh, about-formatting.sh and
commit-links.sh are vulnerable if /bin/sh on your system is a vulnerable
version of Bash.

I confirmed that I can echo arbitrary content from the User-Agent header
into the response on a CGit server I run which has custom filters
installed.

[0] http://seclists.org/oss-sec/2014/q3/650
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


[PATCH 0/4] Add stat only diff mode

2014-10-05 Thread John Keeping
This series extends the diff type combobox to include a new stat only
mode, which outputs only the stat section of the diff.

The first two patches are preparatory cleanups to remove some references
to the existing ssdiff flag, then we change that flag to an enum in
patch 3 and add the stat only value in the final patch.

John Keeping (4):
  ui-shared: remove toggle_ssdiff arg to cgit_commit_link()
  ui-shared: remove toggle_ssdiff arg to cgit_diff_link()
  Change ss diff flag to an enum
  ui-diff: add stat only diff type

 cgit.c  | 12 
 cgit.h  | 10 +++---
 ui-commit.c |  6 +++---
 ui-diff.c   | 16 +++-
 ui-log.c|  4 ++--
 ui-refs.c   |  2 +-
 ui-shared.c | 23 +++
 ui-shared.h |  5 ++---
 8 files changed, 45 insertions(+), 33 deletions(-)

-- 
2.1.0.374.g390713e

___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


[PATCH 1/4] ui-shared: remove toggle_ssdiff arg to cgit_commit_link()

2014-10-05 Thread John Keeping
This argument is never used with a value other than zero, so remove it
and simplify the code.

Signed-off-by: John Keeping j...@keeping.me.uk
---
 ui-commit.c |  4 ++--
 ui-log.c|  4 ++--
 ui-refs.c   |  2 +-
 ui-shared.c | 11 +--
 ui-shared.h |  3 +--
 5 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/ui-commit.c b/ui-commit.c
index c48bfe8..ec99264 100644
--- a/ui-commit.c
+++ b/ui-commit.c
@@ -67,7 +67,7 @@ void cgit_print_commit(char *hex, const char *prefix)
html(/td/tr\n);
html(trthcommit/thtd colspan='2' class='sha1');
tmp = sha1_to_hex(commit-object.sha1);
-   cgit_commit_link(tmp, NULL, NULL, ctx.qry.head, tmp, prefix, 0);
+   cgit_commit_link(tmp, NULL, NULL, ctx.qry.head, tmp, prefix);
html( ();
cgit_patch_link(patch, NULL, NULL, NULL, tmp, prefix);
html()/td/tr\n);
@@ -96,7 +96,7 @@ void cgit_print_commit(char *hex, const char *prefix)
parent_info = cgit_parse_commit(parent);
tmp2 = parent_info-subject;
}
-   cgit_commit_link(tmp2, NULL, NULL, ctx.qry.head, tmp, prefix, 
0);
+   cgit_commit_link(tmp2, NULL, NULL, ctx.qry.head, tmp, prefix);
html( ();
cgit_diff_link(diff, NULL, NULL, ctx.qry.head, hex,
   sha1_to_hex(p-item-object.sha1), prefix, 0);
diff --git a/ui-log.c b/ui-log.c
index bcdb666..ad2f5fc 100644
--- a/ui-log.c
+++ b/ui-log.c
@@ -90,7 +90,7 @@ void show_commit_decorations(struct commit *commit)
strncpy(buf, deco-name, sizeof(buf) - 1);
cgit_commit_link(buf, NULL, deco, ctx.qry.head,
 sha1_to_hex(commit-object.sha1),
-ctx.qry.vpath, 0);
+ctx.qry.vpath);
}
 next:
deco = deco-next;
@@ -165,7 +165,7 @@ static void print_commit(struct commit *commit, struct 
rev_info *revs)
}
}
cgit_commit_link(info-subject, NULL, NULL, ctx.qry.head,
-sha1_to_hex(commit-object.sha1), ctx.qry.vpath, 0);
+sha1_to_hex(commit-object.sha1), ctx.qry.vpath);
show_commit_decorations(commit);
html(/tdtd);
cgit_open_filter(ctx.repo-email_filter, info-author_email, log);
diff --git a/ui-refs.c b/ui-refs.c
index 7e58737..d2ba48d 100644
--- a/ui-refs.c
+++ b/ui-refs.c
@@ -67,7 +67,7 @@ static int print_branch(struct refinfo *ref)
html(/tdtd);
 
if (ref-object-type == OBJ_COMMIT) {
-   cgit_commit_link(info-subject, NULL, NULL, name, NULL, NULL, 
0);
+   cgit_commit_link(info-subject, NULL, NULL, name, NULL, NULL);
html(/tdtd);
cgit_open_filter(ctx.repo-email_filter, info-author_email, 
refs);
html_txt(info-author);
diff --git a/ui-shared.c b/ui-shared.c
index 9dde0a3..497dfd0 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -328,8 +328,7 @@ void cgit_log_link(const char *name, const char *title, 
const char *class,
 }
 
 void cgit_commit_link(char *name, const char *title, const char *class,
- const char *head, const char *rev, const char *path,
- int toggle_ssdiff)
+ const char *head, const char *rev, const char *path)
 {
if (strlen(name)  ctx.cfg.max_msg_len  ctx.cfg.max_msg_len = 15) {
name[ctx.cfg.max_msg_len] = '\0';
@@ -347,7 +346,7 @@ void cgit_commit_link(char *name, const char *title, const 
char *class,
html_url_arg(rev);
delim = amp;;
}
-   if ((ctx.qry.ssdiff  !toggle_ssdiff) || (!ctx.qry.ssdiff  
toggle_ssdiff)) {
+   if (ctx.qry.ssdiff) {
html(delim);
html(ss=1);
delim = amp;;
@@ -463,7 +462,7 @@ static void cgit_self_link(char *name, const char *title, 
const char *class)
else if (!strcmp(ctx.qry.page, commit))
cgit_commit_link(name, title, class, ctx.qry.head,
 ctx.qry.has_sha1 ? ctx.qry.sha1 : NULL,
-ctx.qry.path, 0);
+ctx.qry.path);
else if (!strcmp(ctx.qry.page, patch))
cgit_patch_link(name, title, class, ctx.qry.head,
ctx.qry.has_sha1 ? ctx.qry.sha1 : NULL,
@@ -503,7 +502,7 @@ void cgit_object_link(struct object *obj)
shortrev[10] = '\0';
if (obj-type == OBJ_COMMIT) {
cgit_commit_link(fmt(commit %s..., shortrev), NULL, NULL,
-ctx.qry.head, fullrev, NULL, 0);
+ctx.qry.head, fullrev, NULL);
return;
} else if (obj-type == OBJ_TREE)
page = tree;
@@ -875,7 +874,7 @@ void cgit_print_pageheader(void

[PATCH 2/4] ui-shared: remove toggle_ssdiff arg to cgit_diff_link()

2014-10-05 Thread John Keeping
This argument is never used with a value other than zero, so remove it
and simplify the code.

Signed-off-by: John Keeping j...@keeping.me.uk
---
 ui-commit.c | 2 +-
 ui-diff.c   | 4 ++--
 ui-shared.c | 8 
 ui-shared.h | 2 +-
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/ui-commit.c b/ui-commit.c
index ec99264..d5a888d 100644
--- a/ui-commit.c
+++ b/ui-commit.c
@@ -99,7 +99,7 @@ void cgit_print_commit(char *hex, const char *prefix)
cgit_commit_link(tmp2, NULL, NULL, ctx.qry.head, tmp, prefix);
html( ();
cgit_diff_link(diff, NULL, NULL, ctx.qry.head, hex,
-  sha1_to_hex(p-item-object.sha1), prefix, 0);
+  sha1_to_hex(p-item-object.sha1), prefix);
html()/td/tr);
parents++;
}
diff --git a/ui-diff.c b/ui-diff.c
index 71273aa..49bd748 100644
--- a/ui-diff.c
+++ b/ui-diff.c
@@ -97,7 +97,7 @@ static void print_fileinfo(struct fileinfo *info)
}
htmlf(/tdtd class='%s', class);
cgit_diff_link(info-new_path, NULL, NULL, ctx.qry.head, ctx.qry.sha1,
-  ctx.qry.sha2, info-new_path, 0);
+  ctx.qry.sha2, info-new_path);
if (info-status == DIFF_STATUS_COPIED || info-status == 
DIFF_STATUS_RENAMED) {
htmlf( (%s from ,
  info-status == DIFF_STATUS_COPIED ? copied : 
renamed);
@@ -175,7 +175,7 @@ static void cgit_print_diffstat(const unsigned char 
*old_sha1,
 
html(div class='diffstat-header');
cgit_diff_link(Diffstat, NULL, NULL, ctx.qry.head, ctx.qry.sha1,
-  ctx.qry.sha2, NULL, 0);
+  ctx.qry.sha2, NULL);
if (prefix) {
html( (limited to ');
html_txt(prefix);
diff --git a/ui-shared.c b/ui-shared.c
index 497dfd0..68e0d7c 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -385,7 +385,7 @@ void cgit_snapshot_link(const char *name, const char 
*title, const char *class,
 
 void cgit_diff_link(const char *name, const char *title, const char *class,
const char *head, const char *new_rev, const char *old_rev,
-   const char *path, int toggle_ssdiff)
+   const char *path)
 {
char *delim;
 
@@ -402,7 +402,7 @@ void cgit_diff_link(const char *name, const char *title, 
const char *class,
html_url_arg(old_rev);
delim = amp;;
}
-   if ((ctx.qry.ssdiff  !toggle_ssdiff) || (!ctx.qry.ssdiff  
toggle_ssdiff)) {
+   if (ctx.qry.ssdiff) {
html(delim);
html(ss=1);
delim = amp;;
@@ -478,7 +478,7 @@ static void cgit_self_link(char *name, const char *title, 
const char *class)
else if (!strcmp(ctx.qry.page, diff))
cgit_diff_link(name, title, class, ctx.qry.head,
   ctx.qry.sha1, ctx.qry.sha2,
-  ctx.qry.path, 0);
+  ctx.qry.path);
else if (!strcmp(ctx.qry.page, stats))
cgit_stats_link(name, title, class, ctx.qry.head,
ctx.qry.path);
@@ -876,7 +876,7 @@ void cgit_print_pageheader(void)
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,
-  ctx.qry.sha1, ctx.qry.sha2, ctx.qry.vpath, 0);
+  ctx.qry.sha1, ctx.qry.sha2, ctx.qry.vpath);
if (ctx.repo-max_stats)
cgit_stats_link(stats, NULL, hc(stats),
ctx.qry.head, ctx.qry.vpath);
diff --git a/ui-shared.h b/ui-shared.h
index bb193ec..fc9be3b 100644
--- a/ui-shared.h
+++ b/ui-shared.h
@@ -43,7 +43,7 @@ extern void cgit_snapshot_link(const char *name, const char 
*title,
 extern void cgit_diff_link(const char *name, const char *title,
   const char *class, const char *head,
   const char *new_rev, const char *old_rev,
-  const char *path, int toggle_ssdiff);
+  const char *path);
 extern void cgit_stats_link(const char *name, const char *title,
const char *class, const char *head,
const char *path);
-- 
2.1.0.374.g390713e

___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


[PATCH 3/4] Change ss diff flag to an enum

2014-10-05 Thread John Keeping
This will allow us to introduce a new stat only diff mode without
needing an explosion of mutually incompatible flags.

The old ss query parameter is still accepted in order to avoid
breaking saved links, but we no longer generate any URIs using it;
instead the new dt (diff type) parameter is used.

Signed-off-by: John Keeping j...@keeping.me.uk
---
 cgit.c  | 12 
 cgit.h  | 10 +++---
 ui-diff.c   |  8 +---
 ui-shared.c |  8 
 4 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/cgit.c b/cgit.c
index 8c4517d..db60107 100644
--- a/cgit.c
+++ b/cgit.c
@@ -237,7 +237,7 @@ static void config_cb(const char *name, const char *value)
else if (!strcmp(name, summary-tags))
ctx.cfg.summary_tags = atoi(value);
else if (!strcmp(name, side-by-side-diffs))
-   ctx.cfg.ssdiff = atoi(value);
+   ctx.cfg.difftype = atoi(value) ? DIFF_SSDIFF : DIFF_UNIFIED;
else if (!strcmp(name, agefile))
ctx.cfg.agefile = xstrdup(value);
else if (!strcmp(name, mimetype-file))
@@ -312,9 +312,13 @@ static void querystring_cb(const char *name, const char 
*value)
ctx.qry.showmsg = atoi(value);
} else if (!strcmp(name, period)) {
ctx.qry.period = xstrdup(value);
+   } else if (!strcmp(name, dt)) {
+   ctx.qry.difftype = atoi(value);
+   ctx.qry.has_difftype = 1;
} else if (!strcmp(name, ss)) {
-   ctx.qry.ssdiff = atoi(value);
-   ctx.qry.has_ssdiff = 1;
+   /* No longer generated, but there may be links out there. */
+   ctx.qry.difftype = atoi(value) ? DIFF_SSDIFF : DIFF_UNIFIED;
+   ctx.qry.has_difftype = 1;
} else if (!strcmp(name, all)) {
ctx.qry.show_all = atoi(value);
} else if (!strcmp(name, context)) {
@@ -372,7 +376,7 @@ static void prepare_context(void)
ctx.cfg.summary_log = 10;
ctx.cfg.summary_tags = 10;
ctx.cfg.max_atom_items = 10;
-   ctx.cfg.ssdiff = 0;
+   ctx.cfg.difftype = DIFF_UNIFIED;
ctx.env.cgit_config = getenv(CGIT_CONFIG);
ctx.env.http_host = getenv(HTTP_HOST);
ctx.env.https = getenv(HTTPS);
diff --git a/cgit.h b/cgit.h
index 0badc64..0eb5ed5 100644
--- a/cgit.h
+++ b/cgit.h
@@ -53,6 +53,10 @@ typedef void (*filepair_fn)(struct diff_filepair *pair);
 typedef void (*linediff_fn)(char *line, int len);
 
 typedef enum {
+   DIFF_UNIFIED, DIFF_SSDIFF
+} diff_type;
+
+typedef enum {
ABOUT, COMMIT, SOURCE, EMAIL, AUTH
 } filter_type;
 
@@ -150,7 +154,7 @@ struct reflist {
 struct cgit_query {
int has_symref;
int has_sha1;
-   int has_ssdiff;
+   int has_difftype;
char *raw;
char *repo;
char *page;
@@ -168,7 +172,7 @@ struct cgit_query {
int nohead;
char *sort;
int showmsg;
-   int ssdiff;
+   diff_type difftype;
int show_all;
int context;
int ignorews;
@@ -245,7 +249,7 @@ struct cgit_config {
int summary_branches;
int summary_log;
int summary_tags;
-   int ssdiff;
+   diff_type difftype;
int branch_sort;
int commit_sort;
struct string_list mimetypes;
diff --git a/ui-diff.c b/ui-diff.c
index 49bd748..a4ade4d 100644
--- a/ui-diff.c
+++ b/ui-diff.c
@@ -345,8 +345,8 @@ void cgit_print_diff_ctrls()
html(/trtr);
html(td class='label'mode:/td);
html(td class='ctrl');
-   html(select name='ss' onchange='this.form.submit();');
-   curr = ctx.qry.has_ssdiff ? ctx.qry.ssdiff : ctx.cfg.ssdiff;
+   html(select name='dt' onchange='this.form.submit();');
+   curr = ctx.qry.has_difftype ? ctx.qry.difftype : ctx.cfg.difftype;
html_intoption(0, unified, curr);
html_intoption(1, ssdiff, curr);
html(/select/td/tr);
@@ -362,6 +362,7 @@ void cgit_print_diff(const char *new_rev, const char 
*old_rev,
 {
struct commit *commit, *commit2;
const unsigned char *old_tree_sha1, *new_tree_sha1;
+   diff_type difftype;
 
if (!new_rev)
new_rev = ctx.qry.head;
@@ -420,7 +421,8 @@ void cgit_print_diff(const char *new_rev, const char 
*old_rev,
return;
}
 
-   use_ssdiff = ctx.qry.has_ssdiff ? ctx.qry.ssdiff : ctx.cfg.ssdiff;
+   difftype = ctx.qry.has_difftype ? ctx.qry.difftype : ctx.cfg.difftype;
+   use_ssdiff = difftype == DIFF_SSDIFF;
 
if (show_ctrls)
cgit_print_diff_ctrls();
diff --git a/ui-shared.c b/ui-shared.c
index 68e0d7c..6243d1b 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -346,9 +346,9 @@ void cgit_commit_link(char *name, const char *title, const 
char *class,
html_url_arg(rev);
delim = amp;;
}
-   if (ctx.qry.ssdiff) {
+   if (ctx.qry.difftype) {
html(delim);
-   html(ss=1

[PATCH 4/4] ui-diff: add stat only diff type

2014-10-05 Thread John Keeping
This prints the diffstat but stops before printing (or generating) any
of the body of the diff.

No cgitrc option is added here so that we can wait to see how useful
this is before letting people set it as the default.

Suggested-by: Konstantin Ryabitsev mri...@kernel.org
Signed-off-by: John Keeping j...@keeping.me.uk
---
 cgit.h| 2 +-
 ui-diff.c | 4 
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/cgit.h b/cgit.h
index 0eb5ed5..0c1585d 100644
--- a/cgit.h
+++ b/cgit.h
@@ -53,7 +53,7 @@ typedef void (*filepair_fn)(struct diff_filepair *pair);
 typedef void (*linediff_fn)(char *line, int len);
 
 typedef enum {
-   DIFF_UNIFIED, DIFF_SSDIFF
+   DIFF_UNIFIED, DIFF_SSDIFF, DIFF_STATONLY
 } diff_type;
 
 typedef enum {
diff --git a/ui-diff.c b/ui-diff.c
index a4ade4d..bf2ec57 100644
--- a/ui-diff.c
+++ b/ui-diff.c
@@ -349,6 +349,7 @@ void cgit_print_diff_ctrls()
curr = ctx.qry.has_difftype ? ctx.qry.difftype : ctx.cfg.difftype;
html_intoption(0, unified, curr);
html_intoption(1, ssdiff, curr);
+   html_intoption(2, stat only, curr);
html(/select/td/tr);
html(trtd/td class='ctrl');
html(noscriptinput type='submit' value='reload'//noscript);
@@ -429,6 +430,9 @@ void cgit_print_diff(const char *new_rev, const char 
*old_rev,
 
cgit_print_diffstat(old_rev_sha1, new_rev_sha1, prefix);
 
+   if (difftype == DIFF_STATONLY)
+   return;
+
if (use_ssdiff) {
html(table summary='ssdiff' class='ssdiff');
} else {
-- 
2.1.0.374.g390713e

___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: Fwd: JSON interface

2014-10-05 Thread John Keeping
On Sun, Oct 05, 2014 at 02:13:35PM +0300, Wilhelm Matilainen wrote:
 Instead of acting as a server providing html and css files, could there be
 a only-json option?

You can already configure the URL for CGit's CSS and logo files to point
at a separate machine if you want to do that.

 I could then provide all the static files minified and cached from a proper
 http server already in use and request only the data from cgit.
 
 Using jquery:
 ---
 $.getJSON('/git/repository/repo', function(data) {
 showRepo(data);
 });
 ---
 
 By loading https://
 server/git/what-I-need/parameter-1/parameter-2/etc.
 CGit would output something like:
 ---
 {
 repository: test,
 property1: value1,
 property2: value2
 }
 ---

I don't think it would be particularly difficult to add a new json URL
in cmd.c and handle all of the necessary sub-options, but you would then
need to write an entire UI in JavaScript.

At that point, I'm not sure it would be CGit any more since you would be
using essentially none of the code that exists now.
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: [PATCH 1/1] git: update to v2.2.0

2014-11-27 Thread John Keeping
On Thu, Nov 27, 2014 at 01:30:56AM +0100, Christian Hesse wrote:
 Update to git version v2.2.0, including API changes.
 ---
[...]
 diff --git a/ui-repolist.c b/ui-repolist.c
 index c2bcce1..0e57c53 100644
 --- a/ui-repolist.c
 +++ b/ui-repolist.c
 @@ -17,16 +17,18 @@ static time_t read_agefile(char *path)
   time_t result;
   size_t size;
   char *buf;
 - static char buf2[64];
 + struct strbuf date_buf = STRBUF_INIT;
  
   if (readfile(path, buf, size))
   return -1;
  
 - if (parse_date(buf, buf2, sizeof(buf2))  0)
 - result = strtoul(buf2, NULL, 10);
 + strbuf_reset(date_buf);

Why is this necessary?  date_buf hasn't been used since it was
initialized.

 + if (parse_date(buf, date_buf) == 0)
 + result = strtoul(date_buf.buf, NULL, 10);
   else
   result = 0;
   free(buf);
 + strbuf_release(date_buf);
   return result;
  }
  
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: [PATCH] contrib/hooks: add sample post-receive hook using agefile

2014-12-23 Thread John Keeping
On Tue, Dec 23, 2014 at 04:15:15PM +0100, Ferry Huberts wrote:
 
 
 On 23/12/14 15:40, John Keeping wrote:
  +agefile=$(git rev-parse --git-dir)/info/web/last-modified
 
 use $GIT_DIR here instead of rev-parse

githooks(5) doesn't guarantee that GIT_DIR will be set and the fact that
it currently is seems to be an artifact of how old versions of Git work
([0]).

I'd prefer to stick with rev-parse, which is what the example hooks in
git.git use.

[0] http://permalink.gmane.org/gmane.comp.version-control.git/136276

  +
  +mkdir -p $(dirname $agefile) 
  +git for-each-ref \
  +   --sort=-authordate --count=1 \
  +   --format='%(authordate:iso8601)' \
  +   $agefile
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


[PATCH 1/2] t0108: modernize style

2014-12-28 Thread John Keeping
* -chaining
* use test_cmp instead of cmp
* use strip_headers instead of knowing how many lines there will be

Signed-off-by: John Keeping j...@keeping.me.uk
---
 tests/t0108-patch.sh | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/tests/t0108-patch.sh b/tests/t0108-patch.sh
index fcc749d..4e8aba3 100755
--- a/tests/t0108-patch.sh
+++ b/tests/t0108-patch.sh
@@ -24,10 +24,10 @@ test_expect_success 'find `cgit` signature' '
 '
 
 test_expect_success 'compare with output of git-format-patch(1)' '
-   CGIT_VERSION=$(sed -n s/CGIT_VERSION = //p ../../VERSION)
-   git --git-dir=$PWD/repos/foo/.git format-patch -p --subject-prefix= 
--signature=cgit $CGIT_VERSION --stdout HEAD^ tmp2
-   sed 1,5d tmp tmp_
-   cmp tmp_ tmp2
+   CGIT_VERSION=$(sed -n s/CGIT_VERSION = //p ../../VERSION) 
+   git --git-dir=$PWD/repos/foo/.git format-patch -p --subject-prefix= 
--signature=cgit $CGIT_VERSION --stdout HEAD^ tmp2 
+   strip_headers tmp tmp_ 
+   test_cmp tmp_ tmp2
 '
 
 test_expect_success 'find initial commit' '
@@ -43,8 +43,8 @@ test_expect_success 'find `cgit` signature' '
 '
 
 test_expect_success 'generate patches for multiple commits' '
-   id=$(git --git-dir=$PWD/repos/foo/.git rev-parse HEAD)
-   id2=$(git --git-dir=$PWD/repos/foo/.git rev-parse HEAD~3)
+   id=$(git --git-dir=$PWD/repos/foo/.git rev-parse HEAD) 
+   id2=$(git --git-dir=$PWD/repos/foo/.git rev-parse HEAD~3) 
cgit_query url=foo/patchid=$idid2=$id2 tmp
 '
 
@@ -53,10 +53,10 @@ test_expect_success 'find `cgit` signature' '
 '
 
 test_expect_success 'compare with output of git-format-patch(1)' '
-   CGIT_VERSION=$(sed -n s/CGIT_VERSION = //p ../../VERSION)
-   git --git-dir=$PWD/repos/foo/.git format-patch -p -N 
--subject-prefix= --signature=cgit $CGIT_VERSION --stdout HEAD~3..HEAD tmp2
-   sed 1,5d tmp tmp_
-   cmp tmp_ tmp2
+   CGIT_VERSION=$(sed -n s/CGIT_VERSION = //p ../../VERSION) 
+   git --git-dir=$PWD/repos/foo/.git format-patch -p -N 
--subject-prefix= --signature=cgit $CGIT_VERSION --stdout HEAD~3..HEAD 
tmp2 
+   strip_headers tmp tmp_ 
+   test_cmp tmp_ tmp2
 '
 
 test_done
-- 
2.2.1.286.gdf3164c

___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


[PATCH 2/2] ui-patch: match git-format-patch(1) output

2014-12-28 Thread John Keeping
Using (DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_PATCH) causes Git to emit a
--- line between the commit message and the body of the patch, which
fixes a regression introduced in commit 455b598 (ui-patch.c: Use
log_tree_commit() to generate diffs, 2013-08-20), prior to which we
inserted the --- line ourselves.

DIFF_FORMAT_SUMMARY is added so that we match the output of
git-format-patch(1) without the -p option.

Signed-off-by: John Keeping j...@keeping.me.uk
---
 tests/t0108-patch.sh | 4 ++--
 ui-patch.c   | 3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/tests/t0108-patch.sh b/tests/t0108-patch.sh
index 4e8aba3..013d680 100755
--- a/tests/t0108-patch.sh
+++ b/tests/t0108-patch.sh
@@ -25,7 +25,7 @@ test_expect_success 'find `cgit` signature' '
 
 test_expect_success 'compare with output of git-format-patch(1)' '
CGIT_VERSION=$(sed -n s/CGIT_VERSION = //p ../../VERSION) 
-   git --git-dir=$PWD/repos/foo/.git format-patch -p --subject-prefix= 
--signature=cgit $CGIT_VERSION --stdout HEAD^ tmp2 
+   git --git-dir=$PWD/repos/foo/.git format-patch --subject-prefix= 
--signature=cgit $CGIT_VERSION --stdout HEAD^ tmp2 
strip_headers tmp tmp_ 
test_cmp tmp_ tmp2
 '
@@ -54,7 +54,7 @@ test_expect_success 'find `cgit` signature' '
 
 test_expect_success 'compare with output of git-format-patch(1)' '
CGIT_VERSION=$(sed -n s/CGIT_VERSION = //p ../../VERSION) 
-   git --git-dir=$PWD/repos/foo/.git format-patch -p -N 
--subject-prefix= --signature=cgit $CGIT_VERSION --stdout HEAD~3..HEAD 
tmp2 
+   git --git-dir=$PWD/repos/foo/.git format-patch -N --subject-prefix= 
--signature=cgit $CGIT_VERSION --stdout HEAD~3..HEAD tmp2 
strip_headers tmp tmp_ 
test_cmp tmp_ tmp2
 '
diff --git a/ui-patch.c b/ui-patch.c
index fc6c145..6ec89b4 100644
--- a/ui-patch.c
+++ b/ui-patch.c
@@ -73,7 +73,8 @@ void cgit_print_patch(const char *new_rev, const char 
*old_rev,
rev.diff = 1;
rev.show_root_diff = 1;
rev.max_parents = 1;
-   rev.diffopt.output_format |= DIFF_FORMAT_PATCH;
+   rev.diffopt.output_format |= DIFF_FORMAT_DIFFSTAT |
+   DIFF_FORMAT_PATCH | DIFF_FORMAT_SUMMARY;
setup_revisions(ARRAY_SIZE(rev_argv), (const char **)rev_argv, rev,
NULL);
prepare_revision_walk(rev);
-- 
2.2.1.286.gdf3164c

___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


[PATCH] ui-diff: don't link to single file diff stat

2014-12-29 Thread John Keeping
Seeing the diff stat for a single file is pretty useless, so reset the
diff type before generating the links to individual files in the diff
stat so that the links will show a useful diff.

Reported-by: Konstantin Ryabitsev mri...@kernel.org
Signed-off-by: John Keeping j...@keeping.me.uk
---
On Mon, Dec 29, 2014 at 03:09:55PM -0500, Konstantin Ryabitsev wrote:
 On 23/12/14 09:30 PM, Jason A. Donenfeld wrote:
  Ahh yes, of course -- John has done it, and it's been merged
  here: ddfaef6bb28e697491b25bff5a7b260d44ce6ccf
 
 I actually had a chance to try it out now. It looks pretty nice, but has
 an unintended downside in the sense that dt=2 is sticky. E.g. for a
 link like:
 
 https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/diff/?id=v3.19-rc2id2=v3.19-rc1dt=2
 
 This will show the diffstat only, but then if I click on any of the
 files for more details, the diffstat only mode persists. The user
 would then need to know to change to unified in the dropdown on the
 right to see actual diff contents.
 
 It's not a bug per se, just wondering if we can improve the user experience.

Something like this, perhaps?

 ui-diff.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/ui-diff.c b/ui-diff.c
index bf2ec57..5b6df1f 100644
--- a/ui-diff.c
+++ b/ui-diff.c
@@ -428,6 +428,16 @@ void cgit_print_diff(const char *new_rev, const char 
*old_rev,
if (show_ctrls)
cgit_print_diff_ctrls();
 
+   /*
+* Clicking on a link to a file in the diff stat should show a diff
+* of the file, showing the diff stat limited to a single file is
+* pretty useless.  All links from this point on will be to
+* individual files, so we simply reset the difftype in the query
+* here to avoid propagating DIFF_STATONLY to the individual files.
+*/
+   if (difftype == DIFF_STATONLY)
+   ctx.qry.difftype = ctx.cfg.difftype;
+
cgit_print_diffstat(old_rev_sha1, new_rev_sha1, prefix);
 
if (difftype == DIFF_STATONLY)
-- 
2.2.1.286.gdf3164c

___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: [PATCH v2 1/1] git: preview for v2.3.0-rc0

2015-01-15 Thread John Keeping
On Wed, Jan 14, 2015 at 04:34:20PM +0100, l...@eworm.de wrote:
 From: Christian Hesse m...@eworm.de
 
 * sort_string_list(): rename to string_list_sort() (upstream commit
 * 3383e199)
 * update read_tree_recursive callback to pass strbuf as base (upstream
   commit 6a0b0b6d)
 
 Signed-off-by: Christian Hesse m...@eworm.de
 ---
  Makefile   |  4 ++--
  cgit.c |  2 +-
  git|  2 +-
  ui-blob.c  | 15 ---
  ui-plain.c | 16 
  ui-tree.c  | 23 +++
  6 files changed, 31 insertions(+), 31 deletions(-)
 
 diff --git a/Makefile b/Makefile
 index 38bf595..36c30a8 100644
 --- a/Makefile
 +++ b/Makefile
 @@ -14,8 +14,8 @@ htmldir = $(docdir)
  pdfdir = $(docdir)
  mandir = $(prefix)/share/man
  SHA1_HEADER = openssl/sha.h
 -GIT_VER = 2.2.1
 -GIT_URL = https://www.kernel.org/pub/software/scm/git/git-$(GIT_VER).tar.gz
 +GIT_VER = 2.3.0.rc0
 +GIT_URL = 
 https://www.kernel.org/pub/software/scm/git/testing/git-$(GIT_VER).tar.gz
  INSTALL = install
  COPYTREE = cp -r
  MAN5_TXT = $(wildcard *.5.txt)
 diff --git a/cgit.c b/cgit.c
 index 79019c2..df301ea 100644
 --- a/cgit.c
 +++ b/cgit.c
 @@ -599,7 +599,7 @@ static int prepare_repo_cmd(void)
   free(tmp);
   return 1;
   }
 - sort_string_list(ctx.repo-submodules);
 + string_list_sort(ctx.repo-submodules);
   cgit_prepare_repo_env(ctx.repo);
   choose_readme(ctx.repo);
   return 0;
 diff --git a/git b/git
 index 9b7cbb3..addfb21 16
 --- a/git
 +++ b/git
 @@ -1 +1 @@
 -Subproject commit 9b7cbb315923e61bb0c4297c701089f30e116750
 +Subproject commit addfb21a94fb4e6b9d07b270f7bb3748767a8f38
 diff --git a/ui-blob.c b/ui-blob.c
 index c2de8d6..4e29223 100644
 --- a/ui-blob.c
 +++ b/ui-blob.c
 @@ -18,15 +18,16 @@ struct walk_tree_context {
   int file_only:1;
  };
  
 -static int walk_tree(const unsigned char *sha1, const char *base, int 
 baselen,
 - const char *pathname, unsigned mode, int stage, void *cbdata)
 +static int walk_tree_buf(const unsigned char *sha1, struct strbuf *base,
 + const char *pathname, unsigned mode, int stage,
 + void *cbdata)

This seems unnecessarily noisy.  The patch would be easier to read if
the function name stays the same and you avoid re-wrapping the
parameters.  It would also remove several of the hunks below because the
call sites won't need to change.

  {
   struct walk_tree_context *walk_tree_ctx = cbdata;
  
   if (walk_tree_ctx-file_only  !S_ISREG(mode))
   return READ_TREE_RECURSIVE;
 - if (strncmp(base, walk_tree_ctx-match_path, baselen)
 - || strcmp(walk_tree_ctx-match_path + baselen, pathname))
 + if (strncmp(base-buf, walk_tree_ctx-match_path, base-len)
 + || strcmp(walk_tree_ctx-match_path + base-len, pathname))
   return READ_TREE_RECURSIVE;
   memmove(walk_tree_ctx-matched_sha1, sha1, 20);
   walk_tree_ctx-found_path = 1;
 @@ -56,7 +57,7 @@ int cgit_ref_path_exists(const char *path, const char *ref, 
 int file_only)
   return 0;
   if (sha1_object_info(sha1, size) != OBJ_COMMIT)
   return 0;
 - read_tree_recursive(lookup_commit_reference(sha1)-tree, , 0, 0, 
 paths, walk_tree, walk_tree_ctx);
 + read_tree_recursive(lookup_commit_reference(sha1)-tree, , 0, 0, 
 paths, walk_tree_buf, walk_tree_ctx);
   return walk_tree_ctx.found_path;
  }
  
 @@ -87,7 +88,7 @@ int cgit_print_file(char *path, const char *head, int 
 file_only)
   type = sha1_object_info(sha1, size);
   if (type == OBJ_COMMIT  path) {
   commit = lookup_commit_reference(sha1);
 - read_tree_recursive(commit-tree, , 0, 0, paths, walk_tree, 
 walk_tree_ctx);
 + read_tree_recursive(commit-tree, , 0, 0, paths, 
 walk_tree_buf, walk_tree_ctx);
   if (!walk_tree_ctx.found_path)
   return -1;
   type = sha1_object_info(sha1, size);
 @@ -140,7 +141,7 @@ void cgit_print_blob(const char *hex, char *path, const 
 char *head, int file_onl
  
   if ((!hex)  type == OBJ_COMMIT  path) {
   commit = lookup_commit_reference(sha1);
 - read_tree_recursive(commit-tree, , 0, 0, paths, walk_tree, 
 walk_tree_ctx);
 + read_tree_recursive(commit-tree, , 0, 0, paths, 
 walk_tree_buf, walk_tree_ctx);
   type = sha1_object_info(sha1,size);
   }
  
 diff --git a/ui-plain.c b/ui-plain.c
 index 30fff89..061180d 100644
 --- a/ui-plain.c
 +++ b/ui-plain.c
 @@ -173,23 +173,23 @@ static void print_dir_tail(void)
   html( /ul\n/body/html\n);
  }
  
 -static int walk_tree(const unsigned char *sha1, const char *base, int 
 baselen,
 -  const char *pathname, unsigned mode, int stage,
 -  void *cbdata)
 +static int walk_tree_buf(const unsigned char *sha1, struct strbuf *base,
 + const char *pathname, unsigned mode, int stage,
 + void *cbdata)
  {
   struct walk_tree_context *walk_tree_ctx 

[PATCH] tag: reference with h instead of id

2015-01-15 Thread John Keeping
When clicking on log from a tag we end up showing the log of whatever
branch we used to reach the tag.  If the tag doesn't point onto a branch
then the tagged commit won't appear in this output.

By linking to tags with the head parameter instead of the id parameter
the log link will show the log of the tag.  This is clearly desirable
when the tag has been reached from the refs UI and changing the
behaviour for tag decorations makes them match branch decorations where
log - decoration - log shows the log of the decoration.

Reported-by: Ferry Huberts maili...@hupie.com
Signed-off-by: John Keeping j...@keeping.me.uk
---
 I think we actually want something like this:
 
 -- 8 --
 diff --git a/ui-refs.c b/ui-refs.c
 index bdd3b2c..d3017ec 100644
 --- a/ui-refs.c
 +++ b/ui-refs.c
 @@ -140,7 +140,7 @@ static int print_tag(struct refinfo *ref)
   }
  
   html(trtd);
 - cgit_tag_link(name, NULL, NULL, ctx.qry.head, name);
 + cgit_tag_link(name, NULL, NULL, name, NULL);
   html(/tdtd);
   if (ctx.repo-snapshots  (obj-type == OBJ_COMMIT))
   print_tag_downloads(ctx.repo, name);
 -- 8 --
 
 The tag UI already does the right thing if we use h instead of id.
 
 The only thing I'm not sure about is whether we should change all tag
 links to behave like this.  That would make tag decorations consistent
 with branch decorations which already change the head, so I think that
 is the way to go.

This is what that looks like.

 ui-log.c| 4 ++--
 ui-refs.c   | 2 +-
 ui-shared.c | 8 
 ui-shared.h | 3 +--
 4 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/ui-log.c b/ui-log.c
index 657ff3c..1b60591 100644
--- a/ui-log.c
+++ b/ui-log.c
@@ -71,11 +71,11 @@ void show_commit_decorations(struct commit *commit)
}
else if (starts_with(deco-name, tag: refs/tags/)) {
strncpy(buf, deco-name + 15, sizeof(buf) - 1);
-   cgit_tag_link(buf, NULL, tag-deco, ctx.qry.head, buf);
+   cgit_tag_link(buf, NULL, tag-deco, buf);
}
else if (starts_with(deco-name, refs/tags/)) {
strncpy(buf, deco-name + 10, sizeof(buf) - 1);
-   cgit_tag_link(buf, NULL, tag-deco, ctx.qry.head, buf);
+   cgit_tag_link(buf, NULL, tag-deco, buf);
}
else if (starts_with(deco-name, refs/remotes/)) {
if (!ctx.repo-enable_remote_branches)
diff --git a/ui-refs.c b/ui-refs.c
index d2ba48d..ac8a6d4 100644
--- a/ui-refs.c
+++ b/ui-refs.c
@@ -140,7 +140,7 @@ static int print_tag(struct refinfo *ref)
}
 
html(trtd);
-   cgit_tag_link(name, NULL, NULL, ctx.qry.head, name);
+   cgit_tag_link(name, NULL, NULL, name);
html(/tdtd);
if (ctx.repo-snapshots  (obj-type == OBJ_COMMIT))
print_tag_downloads(ctx.repo, name);
diff --git a/ui-shared.c b/ui-shared.c
index 32f23f9..d8cc4d7 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -273,9 +273,9 @@ void cgit_summary_link(const char *name, const char *title, 
const char *class,
 }
 
 void cgit_tag_link(const char *name, const char *title, const char *class,
-  const char *head, const char *rev)
+  const char *tag)
 {
-   reporevlink(tag, name, title, class, head, rev, NULL);
+   reporevlink(tag, name, title, class, tag, NULL, NULL);
 }
 
 void cgit_tree_link(const char *name, const char *title, const char *class,
@@ -443,8 +443,8 @@ static void cgit_self_link(char *name, const char *title, 
const char *class)
else if (!strcmp(ctx.qry.page, summary))
cgit_summary_link(name, title, class, ctx.qry.head);
else if (!strcmp(ctx.qry.page, tag))
-   cgit_tag_link(name, title, class, ctx.qry.head,
- ctx.qry.has_sha1 ? ctx.qry.sha1 : NULL);
+   cgit_tag_link(name, title, class, ctx.qry.has_sha1 ?
+  ctx.qry.sha1 : ctx.qry.head);
else if (!strcmp(ctx.qry.page, tree))
cgit_tree_link(name, title, class, ctx.qry.head,
   ctx.qry.has_sha1 ? ctx.qry.sha1 : NULL,
diff --git a/ui-shared.h b/ui-shared.h
index f8cf220..021fe4e 100644
--- a/ui-shared.h
+++ b/ui-shared.h
@@ -18,8 +18,7 @@ extern void cgit_index_link(const char *name, const char 
*title,
 extern void cgit_summary_link(const char *name, const char *title,
  const char *class, const char *head);
 extern void cgit_tag_link(const char *name, const char *title,
- const char *class, const char *head,
- const char *rev);
+ const char *class, const char *tag);
 extern void cgit_tree_link(const char *name, const char *title,
   const char *class, const char *head,
   const char *rev, const char *path

Re: Git blame support

2015-01-21 Thread John Keeping
On Wed, Jan 21, 2015 at 11:02:32AM -0500, Elijah Lynn wrote:
 I am not sure if there is an issue tracker being used for the project. I
 asked the list a while back but haven't heard back. At any rate, is there a
 way I can see if there is git blame support being built? I am part of the
 Drupal community and there may be some of us who could help it along if it
 is planned.

I'm not aware of anyone working on blame support.  Having taken a quick
look, Git does most of the work in builtin/blame.c so it won't be
possible to easily re-use any of that code, which means it's not a small
task to add blame support.
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: [PATCH 1/1] git: preview for v2.3.0-rc0

2015-01-13 Thread John Keeping
On Tue, Jan 13, 2015 at 09:56:47AM +0100, l...@eworm.de wrote:
 From: Christian Hesse m...@eworm.de
 
 * sort_string_list(): rename to string_list_sort() (upstream commit 3383e199)
 * update read_tree_recursive callback to pass strbuf as base (upstream
   commit 6a0b0b6d)
 
 Signed-off-by: Christian Hesse m...@eworm.de
 ---
  Makefile   |  4 ++--
  cgit.c |  2 +-
  git|  2 +-
  ui-blob.c  | 14 +++---
  ui-plain.c | 10 +-
  ui-tree.c  | 21 +++--
  6 files changed, 43 insertions(+), 10 deletions(-)
 
 diff --git a/Makefile b/Makefile
 index 38bf595..36c30a8 100644
 --- a/Makefile
 +++ b/Makefile
 @@ -14,8 +14,8 @@ htmldir = $(docdir)
  pdfdir = $(docdir)
  mandir = $(prefix)/share/man
  SHA1_HEADER = openssl/sha.h
 -GIT_VER = 2.2.1
 -GIT_URL = https://www.kernel.org/pub/software/scm/git/git-$(GIT_VER).tar.gz
 +GIT_VER = 2.3.0.rc0
 +GIT_URL = 
 https://www.kernel.org/pub/software/scm/git/testing/git-$(GIT_VER).tar.gz
  INSTALL = install
  COPYTREE = cp -r
  MAN5_TXT = $(wildcard *.5.txt)
 diff --git a/cgit.c b/cgit.c
 index 79019c2..df301ea 100644
 --- a/cgit.c
 +++ b/cgit.c
 @@ -599,7 +599,7 @@ static int prepare_repo_cmd(void)
   free(tmp);
   return 1;
   }
 - sort_string_list(ctx.repo-submodules);
 + string_list_sort(ctx.repo-submodules);
   cgit_prepare_repo_env(ctx.repo);
   choose_readme(ctx.repo);
   return 0;
 diff --git a/git b/git
 index 9b7cbb3..addfb21 16
 --- a/git
 +++ b/git
 @@ -1 +1 @@
 -Subproject commit 9b7cbb315923e61bb0c4297c701089f30e116750
 +Subproject commit addfb21a94fb4e6b9d07b270f7bb3748767a8f38
 diff --git a/ui-blob.c b/ui-blob.c
 index c2de8d6..90fcf81 100644
 --- a/ui-blob.c
 +++ b/ui-blob.c
 @@ -33,6 +33,14 @@ static int walk_tree(const unsigned char *sha1, const char 
 *base, int baselen,
   return 0;
  }
  
 +static int walk_tree_buf(const unsigned char *sha1, struct strbuf *base,
 + const char *filename, unsigned mode, int stage,
 + void *context)
 +{
 + return walk_tree(sha1, base-buf, base-len,
 + filename, mode, stage, context);
 +}
 +

Is there any benefit to introducing a new function here?  In other
words, is walk_tree() used anywhere else?  If not, wouldn't it be
simpler to update the signature of the existing function?

(The same comment applies to several other places below.)

  int cgit_ref_path_exists(const char *path, const char *ref, int file_only)
  {
   unsigned char sha1[20];
 @@ -56,7 +64,7 @@ int cgit_ref_path_exists(const char *path, const char *ref, 
 int file_only)
   return 0;
   if (sha1_object_info(sha1, size) != OBJ_COMMIT)
   return 0;
 - read_tree_recursive(lookup_commit_reference(sha1)-tree, , 0, 0, 
 paths, walk_tree, walk_tree_ctx);
 + read_tree_recursive(lookup_commit_reference(sha1)-tree, , 0, 0, 
 paths, walk_tree_buf, walk_tree_ctx);
   return walk_tree_ctx.found_path;
  }
  
 @@ -87,7 +95,7 @@ int cgit_print_file(char *path, const char *head, int 
 file_only)
   type = sha1_object_info(sha1, size);
   if (type == OBJ_COMMIT  path) {
   commit = lookup_commit_reference(sha1);
 - read_tree_recursive(commit-tree, , 0, 0, paths, walk_tree, 
 walk_tree_ctx);
 + read_tree_recursive(commit-tree, , 0, 0, paths, 
 walk_tree_buf, walk_tree_ctx);
   if (!walk_tree_ctx.found_path)
   return -1;
   type = sha1_object_info(sha1, size);
 @@ -140,7 +148,7 @@ void cgit_print_blob(const char *hex, char *path, const 
 char *head, int file_onl
  
   if ((!hex)  type == OBJ_COMMIT  path) {
   commit = lookup_commit_reference(sha1);
 - read_tree_recursive(commit-tree, , 0, 0, paths, walk_tree, 
 walk_tree_ctx);
 + read_tree_recursive(commit-tree, , 0, 0, paths, 
 walk_tree_buf, walk_tree_ctx);
   type = sha1_object_info(sha1,size);
   }
  
 diff --git a/ui-plain.c b/ui-plain.c
 index 30fff89..891e9f7 100644
 --- a/ui-plain.c
 +++ b/ui-plain.c
 @@ -198,6 +198,14 @@ static int walk_tree(const unsigned char *sha1, const 
 char *base, int baselen,
   return 0;
  }
  
 +static int walk_tree_buf(const unsigned char *sha1, struct strbuf *base,
 + const char *filename, unsigned mode, int stage,
 + void *context)
 +{
 + return walk_tree(sha1, base-buf, base-len,
 + filename, mode, stage, context);
 +}
 +
  static int basedir_len(const char *path)
  {
   char *p = strrchr(path, '/');
 @@ -243,7 +251,7 @@ void cgit_print_plain(void)
   }
   else
   walk_tree_ctx.match_baselen = basedir_len(path_items.match);
 - read_tree_recursive(commit-tree, , 0, 0, paths, walk_tree, 
 walk_tree_ctx);
 + read_tree_recursive(commit-tree, , 0, 0, paths, walk_tree_buf, 
 walk_tree_ctx);
   if (!walk_tree_ctx.match)
   html_status(404, Not found, 0);
   else if (walk_tree_ctx.match == 2)
 diff --git 

Re: [PATCH 1/1] git: preview for v2.3.0-rc0

2015-01-13 Thread John Keeping
On Tue, Jan 13, 2015 at 10:57:39AM +0100, Christian Hesse wrote:
 John Keeping j...@keeping.me.uk on Tue, 2015/01/13 09:43:
  On Tue, Jan 13, 2015 at 09:56:47AM +0100, l...@eworm.de wrote:
   From: Christian Hesse m...@eworm.de
   
   * sort_string_list(): rename to string_list_sort() (upstream commit
   3383e199)
   * update read_tree_recursive callback to pass strbuf as base (upstream
 commit 6a0b0b6d)
   
   Signed-off-by: Christian Hesse m...@eworm.de
   ---
   [...]
  
  Is there any benefit to introducing a new function here?  In other
  words, is walk_tree() used anywhere else?  If not, wouldn't it be
  simpler to update the signature of the existing function?
  
  (The same comment applies to several other places below.)
 
 I just adopted the changes from git upstream to make things work. This is not
 intended for merge... And I will take a closer look for final patch when
 git v2.3.0 arrives. ;)
 
 Our code include three functions called 'walk_tree()' in ui-plain.c,
 ui-blob.c and ui-tree.c. Can this bring any trouble?

They're all static so they're limited to the files in which they're
declared.  The names won't appear in the final binary at all (except in
debug annotations).
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: [PATCH] ui-commit: display commit notes as 'raw'

2015-03-21 Thread John Keeping
On Sat, Mar 21, 2015 at 02:11:51PM +0100, Stefan Naewe wrote:
 On Fri, Mar 20, 2015 at 10:29 PM, John Keeping j...@keeping.me.uk wrote:
  On Fri, Mar 20, 2015 at 05:39:53PM +0100, Stefan Naewe wrote:
  When the git function format_display_notes() is called
  with a value != 0 as the last argument ('raw') the notes text
  gets displayed w/o an additional 'Notes:' header. This seems
  to be better suited for our needs since we're already displaying
  a similar header.
 
  What happens when there are multiple display notes refs?  It seems that
  without raw, format_note() puts the name of the ref in the header it
  prints (if it's not the default notes ref).
 That's true for 'git show --notes=...' for example.
 
 But how would you specify the name of the notes ref in cgit ?

git config --add notes.displayRef ...

  It's possible that the correct answer is that we don't care about that
  case, but it is potentially a regression for people who want to display
  multiple notes.
 
 Attached are two screenshots of commit notes added with 'git notes
 add..' for the first line and
 'git notes append...' for the second line. That particular commit also
 has another note
 attached in the 'refs/notes/todo' namespace.
 
 My opinion about this topic: I really like the way cgit displays the
 commit notes but not the
 double Notes header line.
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: [PATCH] ui-commit: display commit notes as 'raw'

2015-03-20 Thread John Keeping
On Fri, Mar 20, 2015 at 05:39:53PM +0100, Stefan Naewe wrote:
 When the git function format_display_notes() is called
 with a value != 0 as the last argument ('raw') the notes text
 gets displayed w/o an additional 'Notes:' header. This seems
 to be better suited for our needs since we're already displaying
 a similar header.

What happens when there are multiple display notes refs?  It seems that
without raw, format_note() puts the name of the ref in the header it
prints (if it's not the default notes ref).

It's possible that the correct answer is that we don't care about that
case, but it is potentially a regression for people who want to display
multiple notes.

 Signed-off-by: Stefan Naewe stefan.na...@gmail.com
 ---
  ui-commit.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/ui-commit.c b/ui-commit.c
 index d5a888d..4b40133 100644
 --- a/ui-commit.c
 +++ b/ui-commit.c
 @@ -37,7 +37,7 @@ void cgit_print_commit(char *hex, const char *prefix)
   }
   info = cgit_parse_commit(commit);
  
 - format_display_notes(sha1, notes, PAGE_ENCODING, 0);
 + format_display_notes(sha1, notes, PAGE_ENCODING, 1);
  
   load_ref_decorations(DECORATE_FULL_REFS);
  
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: [PATCH] Check SHA256 sum of git-$VER.tar.gz after downloading

2015-03-09 Thread John Keeping
On Mon, Mar 09, 2015 at 03:39:29PM -0400, Todd Zullinger wrote:
 Those on the list can check the PGP signature on the announcement mail 
 and then use the included SHA1 to check the tarball, but doing that as 
 a non-list member isn't as easy due to many list archives stripping or 
 mangling PGP signatures.  I tried doing this with the 0.11 
 announcement from the Mailman and Gmane archives now and wasn't 
 successful.

It turns out that GMane mangles the list address in the message, so it
is possible to validate it but it's not straightforward:

curl http://article.gmane.org/gmane.comp.version-control.cgit/2387/raw |
sed -e 's/cgit[^ ]*@public.gmane.org/cgit@lists.zx2c4.com/' |
gpg --verify

 Posting a detached PGP signature for the tarball would improve the 
 ability for users to trust and verify the cgit tarball.  It's not a 
 blocker for your patch, but it would make it significantly more 
 useful, so I thought I would broach the subject. ;)

It seems that Jason currently relies on CGit to generate the tarballs by
pointing to http://git.zx2c4.com/cgit/refs/tags, which means that a
signature isn't guaranteed to remain correct (Git has subtly changed the
tar encoding in the past and could do so again).

There's a recent thread on the Git mailing list about a way to handle
this better[0], but there isn't any code yet AFAIK.

[0] http://thread.gmane.org/gmane.comp.version-control.git/264533
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: [PATCH] Check SHA256 sum of git-$VER.tar.gz after downloading

2015-03-07 Thread John Keeping
On Sat, Mar 07, 2015 at 04:59:26PM +0100, Lukas Fleischer wrote:
 On Sat, 07 Mar 2015 at 15:46:41, John Keeping wrote:
  This requires that we save the downloaded file explicitly rather than
  piping it straight to tar, but that is advisable anyway since it allows
  us to check the exit status of curl and make sure that we have
  downloaded the file successfully.
  
  Also add a test to make sure we don't forget to update the file when
  updating our Git version in the future.
  
  Signed-off-by: John Keeping j...@keeping.me.uk
  ---
   Makefile |  8 ++--
   git.sha256sum|  1 +
   tests/t0001-validate-git-versions.sh | 11 +++
   3 files changed, 18 insertions(+), 2 deletions(-)
   create mode 100644 git.sha256sum
  [...]
 
 I like the idea, however, sha256sum is not available on all platforms.
 This breaks `make get-git` under OpenBSD, for example (OpenBSD has a
 utility called sha256 with a different command line interface). Maybe we
 can make the check optional, though?

I'm not sure what benefit it has if it's optional.  Will anyone check?

Maybe we could do something like:

if type sha256sum /dev/null 21
then
sha256sum --check git.sha256sum $(GIT_FILE)
else
echo 2 'WARNING: sha256sum not found so we cannot verify'
echo 2 'WARNING: the integrity of the Git archive!'
fi

 On a related note, can we download a signature and use `gpg --verify`
 instead (should probably be optional as well, to avoid a dependency on
 GnuPG)?

I thought about that, but we'd have to embed a key with CGit for it to
work reliably and how do we choose what key to use (given that
individual Git archives are not signed - the list of SHA256 checksums
is)?
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: [PATCH] Check SHA256 sum of git-$VER.tar.gz after downloading

2015-03-07 Thread John Keeping
On Sat, Mar 07, 2015 at 06:49:32PM +0100, Lukas Fleischer wrote:
 On Sat, 07 Mar 2015 at 18:02:59, John Keeping wrote:
  [...]
  I'm not sure what benefit it has if it's optional.  Will anyone check?
  
  Maybe we could do something like:
  
  if type sha256sum /dev/null 21
  then
  sha256sum --check git.sha256sum $(GIT_FILE)
  else
  echo 2 'WARNING: sha256sum not found so we cannot verify'
  echo 2 'WARNING: the integrity of the Git archive!'
  fi
  
 
 That is exactly what I meant by suggesting to make it optional. Sorry
 for being vague...
 
   On a related note, can we download a signature and use `gpg --verify`
   instead (should probably be optional as well, to avoid a dependency on
   GnuPG)?
  
  I thought about that, but we'd have to embed a key with CGit for it to
  work reliably and how do we choose what key to use (given that
  individual Git archives are not signed - the list of SHA256 checksums
  is)?
  
 
 Huh? This works fine for me:
 
 $ gpg --recv-keys 96AFE6CB 
 gpg: key 713660A7: public key Junio C Hamano gits...@pobox.com 
 imported
 gpg: 3 marginal(s) needed, 1 complete(s) needed, PGP trust model
 gpg: depth: 0  valid:   1  signed:   0  trust: 0-, 0q, 0n, 0m, 0f, 1u
 gpg: Total number processed: 1
 gpg:   imported: 1
 $ curl -OO 
 https://www.kernel.org/pub/software/scm/git/git-2.3.2.tar.{xz,sign} 
   % Total% Received % Xferd  Average Speed   TimeTime Time  
 Current
  Dload  Upload   Total   SpentLeft  
 Speed
 100 3529k  100 3529k0 0  1133k  0  0:00:03  0:00:03 --:--:-- 
 1133k
 100   543  100   5430 0   3404  0 --:--:-- --:--:-- --:--:--  
 3404
 $ unxz git-2.3.2.tar.xz

Oh, I tried this earlier but completely missed the fact that the
signature was on the bare tar file not one of the compressed versions.

I still think we can't rely on `gpg --recv-keys` though, we would have
to distribute the key with CGit and possible also do something to avoid
importing it into the user's keyring by default.

I think a hash is more appropriate for the situation we're in - we are
assuming that the user is happy that the CGit distribution they have is
trustworthy but we must verify that the Git distribution we download is
also correct.

 $ gpg --verify git-2.3.2.tar.sign 
 gpg: assuming signed data in 'git-2.3.2.tar'
 gpg: Signature made Sat 07 Mar 2015 12:10:41 AM CET using RSA key ID 
 96AFE6CB
 gpg: Good signature from Junio C Hamano gits...@pobox.com [unknown]
 gpg: aka Junio C Hamano j...@google.com [unknown]
 gpg: aka Junio C Hamano ju...@pobox.com [unknown]
 gpg: WARNING: This key is not certified with a trusted signature!
 gpg:  There is no indication that the signature belongs to the 
 owner.
 Primary key fingerprint: 96E0 7AF2 5771 9559 80DA  D100 20D0 4E5A 7136 
 60A7
  Subkey fingerprint: E1F0 36B1 FEE7 221F C778  ECEF B0B5 E886 96AF 
 E6CB
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


[PATCH] Check SHA256 sum of git-$VER.tar.gz after downloading

2015-03-07 Thread John Keeping
This requires that we save the downloaded file explicitly rather than
piping it straight to tar, but that is advisable anyway since it allows
us to check the exit status of curl and make sure that we have
downloaded the file successfully.

Also add a test to make sure we don't forget to update the file when
updating our Git version in the future.

Signed-off-by: John Keeping j...@keeping.me.uk
---
 Makefile |  8 ++--
 git.sha256sum|  1 +
 tests/t0001-validate-git-versions.sh | 11 +++
 3 files changed, 18 insertions(+), 2 deletions(-)
 create mode 100644 git.sha256sum

diff --git a/Makefile b/Makefile
index ed329e8..807879f 100644
--- a/Makefile
+++ b/Makefile
@@ -15,7 +15,8 @@ pdfdir = $(docdir)
 mandir = $(prefix)/share/man
 SHA1_HEADER = openssl/sha.h
 GIT_VER = 2.3.2
-GIT_URL = https://www.kernel.org/pub/software/scm/git/git-$(GIT_VER).tar.gz
+GIT_FILE = git-$(GIT_VER).tar.gz
+GIT_URL = https://www.kernel.org/pub/software/scm/git/$(GIT_FILE)
 INSTALL = install
 COPYTREE = cp -r
 MAN5_TXT = $(wildcard *.5.txt)
@@ -146,7 +147,10 @@ clean-doc:
$(RM) cgitrc.5 cgitrc.5.html cgitrc.5.pdf cgitrc.5.xml cgitrc.5.fo
 
 get-git:
-   curl -L $(GIT_URL) | tar -xzf -  rm -rf git  mv git-$(GIT_VER) git
+   curl -L $(GIT_URL) --output $(GIT_FILE)  \
+   sha256sum --check git.sha256sum  \
+   tar -xzf $(GIT_FILE)  \
+   rm -rf git  mv git-$(GIT_VER) git
 
 tags:
$(QUIET_TAGS)find . -name '*.[ch]' | xargs ctags
diff --git a/git.sha256sum b/git.sha256sum
new file mode 100644
index 000..1214d3d
--- /dev/null
+++ b/git.sha256sum
@@ -0,0 +1 @@
+a35aea3a0f63f4cc3dd38fa32127e97273f335a14ea2586b649eb759ecf675a3  
git-2.3.2.tar.gz
diff --git a/tests/t0001-validate-git-versions.sh 
b/tests/t0001-validate-git-versions.sh
index a65b35e..3325c77 100755
--- a/tests/t0001-validate-git-versions.sh
+++ b/tests/t0001-validate-git-versions.sh
@@ -9,6 +9,12 @@ test_expect_success 'extract Git version from Makefile' '
s/^GIT_VER[ ]*=[]*//
p
} ../../Makefile makefile_version
+   GIT_VER=$(cat makefile_version)
+   sed -n -e /^GIT_FILE[  ]*=/ {
+   s/^GIT_FILE[]*=[]*//
+   s/\$(GIT_VER)/$GIT_VER/
+   p
+   } ../../Makefile makefile_file
 '
 
 # Note that Git's GIT-VERSION-GEN script applies s/-/./g to the version
@@ -38,4 +44,9 @@ test_expect_success 'test submodule version matches Makefile' 
'
fi
 '
 
+test_expect_success 'git.sha256sum version matches Makefile' '
+   sed -e s/[0-9a-z]* *// ../../git.sha256sum sha256sum_file
+   test_cmp sha256sum_file makefile_file
+'
+
 test_done
-- 
2.3.1.308.g754cd77

___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: [PATCH] Check SHA256 sum of git-$VER.tar.gz after downloading

2015-03-08 Thread John Keeping
On Sat, Mar 07, 2015 at 06:35:10PM -0500, Todd Zullinger wrote:
 John Keeping wrote:
  I still think we can't rely on `gpg --recv-keys` though, we would 
  have to distribute the key with CGit and possible also do something 
  to avoid importing it into the user's keyring by default.
 
 If the check was to be run from a cgit clone, the key Junio uses to 
 sign git tarballs could be included as a blob, similarly to how it's 
 done in git.git.

My assumption is that if people have cloned CGit then they will probably
clone Git as well, at which point they check out an explicit SHA-1.

 (See the junio-gpg-pub tag in git.git for anyone unfamiliar with this 
 already.  The key can be extracted via:
 
 git cat-file blob junio-gpg-pub
 
 I've always thought that was a neat use of git, but certainly not a 
 common one.  I can't manage to make github display this tagged blob, 
 which is also amusing.
 
 The cgit-hosted kernel.org repo displays it easily though:
 
 http://git.kernel.org/cgit/git/git.git/tag/?id=junio-gpg-pub)
 
 This method does nothing for users who have downloaded a cgit tarball, 
 of course, which I expect is more likely to be the use case you're 
 targeting.

Precisely.

  I think a hash is more appropriate for the situation we're in - we 
  are assuming that the user is happy that the CGit distribution they 
  have is trustworthy but we must verify that the Git distribution we 
  download is also correct.
 
 I don't think this is unreasonable at all.  Trust has to start 
 somewhere.  For users that want to go to the source, they can always 
 download git directly (or just the detached PGP signature) and verify 
 the tarball.  When I updated cgit packages in Fedora and EPEL, this is 
 what I always did.  I don't know if the current maintainers follow 
 that process still, but hopefully they do. ;)
 
 But while we're on the subject, are there PGP signatures available for 
 the cgit tarballs themselves?  I know the git tags are signed, but I 
 don't think I've seen detached signatures for the tarballs.  In this 
 case, how does a user become happy that the CGit distribution they 
 have is trustworthy?  The cgit tarball download isn't available via 
 https either, which might be a reasonable answer in the absence of a 
 detached git signature.
 
 Without a signature on the tarball or some other method to verify the 
 cgit tarball, the sha256 of the git tarball included in the cgit 
 Makefile is more or less only useful as a basic download integrity 
 check (in which case sha256 is mild overkill).
 
 None of this is to say that this patch isn't a step in the right 
 direction.  It certainly helps to display a nicer error message if a 
 user receives a corrupted git tarball.  It's just important that users 
 don't confuse this with providing any real authentication of the git 
 tarball.

I'm not sure this is true.  Providing that the CGit tarball is trusted,
then I think this does provide sufficient authentication of the Git
tarball.  If the CGit tarball isn't trusted, then all bets are off
anyway.
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: unexpected cache issue when http errors

2015-03-31 Thread John Keeping
On Mon, Mar 30, 2015 at 07:15:27PM +0200, Nicolas Dely wrote:
 I would like to share an unexpected cache behaviour with this list and 
 discuss about a solution.
 
 Indeed we are using cgit to provide a web interface to our internal user 
 and also to provide file to our reviewboard server.
 
 We are experiencing some cache issue on static pages when data are not 
 pushed yet to server but reviewboard tries to access these 
 files/repositories. Cache is adding even 404, 401 or 500 error for 
 static page, we can workaround this issue after reducing/setting static 
 cache issue but it looks not very useful to cache http errors.
 
 Do you think it is possible to only cache when HTTP is 200 OK? Other ideas?

I don't think it will be easy to change the behaviour to cache only on
200 responses, and I'm not sure it's desirable to do so since the
process of determining that a result is an error may involve significant
work (e.g. loading packed refs or pack indexes).

I can see an argument for changing the default for cache-static-ttl to
a positive value, so that if we do cache an error result it will
eventually time out without needing another page to be written into that
cache slot, but I'm not sure I understand how you reviewboard server can
access files that have not yet been pushed to the Git server.  If you're
reviewing committed changes, shouldn't the review be posted by a hook on
the server, which would ensure that the commits are available before
reviewboard knows about them?
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: cache issue

2015-02-28 Thread John Keeping
On Sat, Feb 28, 2015 at 12:06:41PM +, Bertrand Jacquin wrote:
 We are still experiencing the issue. Is there any fixes with newer 
 releases ?

I have just tried to reproduce this with the latest version and have not
been able to do so, but I'm not aware of any changes that should have an
effect on this (there is one cache change, 6ceba45 Skip cache slot when
time-to-live is zero, but that only applies if you set one of the *-ttl
values to zero).

The cache timeout logic relies on the mtime of the cache file, so this
could be affected by your filesystem, but it sounds like the problem is
that the .lock files are not being cleaned up.  When CGit finds a .lock
file for a cache slot it is trying to use it will just serve the stale
content, on the assumption that is has only just been replaced.

I can't see many ways that you can end up with stale lock files; the
only options are:

1) CGit crashes, in which case there should be some evidence in the
   system log.
2) rename(2) fails, presumably because the destination file exists.


 On 23/03/2014 14:06, Bertrand Jacquin wrote:
  Hi,
  
  I'm getting some trouble with cgit on enlightenment platforms and cache
  since some time, but at it seems to be reproductable with cgit 0.10 
  here
  is a report.
  
  The cache configuration look like this :
  
  cache-root=../cache
  cache-size=1
  cache-static-ttl=1
  cache-dynamic-ttl=1
  cache-repo-ttl=1
  cache-root-ttl=1
  cache-scanrc-ttl=5
  
  * Main page
  
  $ curl -sD - -o /dev/null https://git.enlightenment.org/ \
| grep -E '^(Date|Expires|Last-Modified): '
  Date: Sun, 23 Mar 2014 14:02:08 GMT
  Expires: Sun, 23 Mar 2014 14:02:52 GMT
  Last-Modified: Sun, 23 Mar 2014 14:01:52 GMT
  
  In this page, core/elementary.git is shown as last modified '58 min.'
  ago.
  
  $ curl -s https://git.enlightenment.org/ \
| sed -e 's;html xmlns=.*;html;' \
  | xmlstarlet fo -o -D -R --html 2 /dev/null \
  | xmlstarlet sel -T -t \
-m 
  html/body/div/div/table/tr/td/a[@title='core/elementary.git'] \
  -v ../..//span[@class='age-mins'] -n
  58 min.
  
  * Repo page
  
  $ curl -sD - -o /dev/null 
  https://git.enlightenment.org/core/elementary.git/ \
| grep -E '^(Date|Expires|Last-Modified): '
  Date: Sun, 23 Mar 2014 14:02:14 GMT
  Expires: Mon, 10 Mar 2014 20:49:55 GMT
  Last-Modified: Mon, 10 Mar 2014 20:48:55 GMT
  
  As you see, the Expires header is wrong as the configuration state it
  should not be older than 1 minute (cache-repo-ttl).
  
  Here, master is the last modified branch and is shown as last modified
  '3 hours' ago.
  
  $ curl -s https://git.enlightenment.org/core/elementary.git/ \
| sed -e 's;html xmlns=.*;html;' \
  | xmlstarlet fo -o -D -R --html 2 /dev/null \
  | xmlstarlet sel -T -t \
-m 
  html/body/div/div/table/tr/td/a[@href='skins/larry/core/elementary.git/log/']
   
  \
-v ../..//span[@class='age-hours'] -n
  3 hours
  
  * All branch page
  
  $ curl -sD - -o /dev/null
  https://git.enlightenment.org/core/elementary.git/refs/heads \
| grep -E '^(Date|Expires|Last-Modified): '
  Date: Sun, 23 Mar 2014 14:02:22 GMT
  Expires: Sun, 23 Mar 2014 14:03:22 GMT
  Last-Modified: Sun, 23 Mar 2014 14:02:22 GMT
  
  In this page, the master is showned as last modified '61 min.' ago.
  
  $ curl -s https://git.enlightenment.org/core/elementary.git/refs/heads 
  \
| sed -e 's;html xmlns=.*;html;' \
  | xmlstarlet fo -o -D -R --html 2 /dev/null \
  | xmlstarlet sel -t \
-m 
  html/body/div/div/table/tr/td/a[@href='skins/larry/core/elementary.git/log/']
   
  \
-v ../..//span[@class='age-mins'] -n
  61 min.
  
  How can we fix this ? I have some .lock files in the cache directory, 
  is
  there any way to flush the lock files after some period ? Also, to help
  debugging this, it should be nice to have a X-Cgit-Cache: header
  containing the cache file used given to user.
  
  Once I remove '*.lock' in cache directory, 'Repo page' and 'All branch
  page' are equal, but 'Main page' is not OK (ordered in the same as
  before) :
  
  62 min.
  65 min.
  65 min.
 
 -- 
 Bertrand
 ___
 CGit mailing list
 CGit@lists.zx2c4.com
 http://lists.zx2c4.com/mailman/listinfo/cgit
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: cache issue

2015-03-03 Thread John Keeping
From eb741581ec16d3249e7d207c3dbc4a433a8f329b Mon Sep 17 00:00:00 2001
Message-Id: 
eb741581ec16d3249e7d207c3dbc4a433a8f329b.1425410489.git.j...@keeping.me.uk
From: John Keeping j...@keeping.me.uk
Date: Tue, 3 Mar 2015 19:01:24 +
Subject: [PATCH] cache: use F_SETLK to avoid stale lock files

If CGit is killed while it holds a lock on a cache slot (for example
because it is taking too long to generate a page), the lock file will be
left in place.  This prevents any future attempt to use the same slot
since it will fail to exclusively create the lock file.

Since CGit is the only program that should be manipulating lock files,
we can use advisory locking to detect whether another process is
actually using the lock file or if it is now stale.

I have confirmed that this works on Linux by setting a short TTL in a
custom cgitrc and running the following with CGit patched to print a
message to stderr if the fcntl(2) fails:

$ export CGIT_CONFIG=$PWD/cgitrc
$ export QUERY_STRING=url=cgit/tree/ui-shared.c
$ ./cgit |
grep -v -e '^div class=.footer.' \
-e '^Last-Modified: ' \
-e ^'Expires: ' expect
$ seq 5 | dd bs=8192 |
parallel -j200 diff -u expect (./cgit |
grep -v -e '^div class=.footer.' \
-e '^Last-Modified: ' \
-e ^'Expires: ') || echo BAD

This printed the fail message several times without ever printing BAD.

Signed-off-by: John Keeping j...@keeping.me.uk
---
On Tue, Mar 03, 2015 at 04:40:01PM +0100, Jason A. Donenfeld wrote:
 Feel free to send a patch, John.

Here it is!

 cache.c | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/cache.c b/cache.c
index 801e63f..900b161 100644
--- a/cache.c
+++ b/cache.c
@@ -161,10 +161,23 @@ static int close_lock(struct cache_slot *slot)
  */
 static int lock_slot(struct cache_slot *slot)
 {
-   slot-lock_fd = open(slot-lock_name, O_RDWR | O_CREAT | O_EXCL,
+   struct flock lock = {
+   .l_type = F_WRLCK,
+   .l_whence = SEEK_SET,
+   .l_start = 0,
+   .l_len = 0,
+   };
+
+   slot-lock_fd = open(slot-lock_name, O_RDWR | O_CREAT,
 S_IRUSR | S_IWUSR);
if (slot-lock_fd == -1)
return errno;
+   if (fcntl(slot-lock_fd, F_SETLK, lock)  0) {
+   int saved_errno = errno;
+   close(slot-lock_fd);
+   slot-lock_fd = -1;
+   return saved_errno;
+   }
if (xwrite(slot-lock_fd, slot-key, slot-keylen + 1)  0)
return errno;
return 0;
-- 
2.3.1.308.g754cd77

___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: [PATCH] Support .git/category files

2015-03-05 Thread John Keeping
On Thu, Mar 05, 2015 at 06:15:04PM +0100, Jan-Marek Glogowski wrote:
 Gitweb reads .git/category to set a repository section for
 grouping. This handles the file in the same way a .git/description
 file is handled.
 
 The file section takes precedence over the ctx.cfg.section_from_path
 setting.

What's the advantage of this over using enable-git-config=1 and
cgit.section (we even support gitweb.section as an alias)?

 ---
  cgit.c  |  2 +-
  cgit.h  |  1 +
  scan-tree.c | 20 +++-
  shared.c|  1 +
  4 files changed, 22 insertions(+), 2 deletions(-)
 
 diff --git a/cgit.c b/cgit.c
 index 0ad8171..239f709 100644
 --- a/cgit.c
 +++ b/cgit.c
 @@ -377,7 +377,7 @@ static void prepare_context(void)
   ctx.cfg.root_desc = a fast webinterface for the git dscm;
   ctx.cfg.scan_hidden_path = 0;
   ctx.cfg.script_name = CGIT_SCRIPT_NAME;
 - ctx.cfg.section = ;
 + ctx.cfg.section = cgit_default_repo_section;
   ctx.cfg.repository_sort = name;
   ctx.cfg.section_sort = 1;
   ctx.cfg.summary_branches = 10;
 diff --git a/cgit.h b/cgit.h
 index 16f8092..ea9aef9 100644
 --- a/cgit.h
 +++ b/cgit.h
 @@ -318,6 +318,7 @@ extern struct cgit_context ctx;
  extern const struct cgit_snapshot_format cgit_snapshot_formats[];
  
  extern char *cgit_default_repo_desc;
 +extern char *cgit_default_repo_section;
  extern struct cgit_repo *cgit_add_repo(const char *url);
  extern struct cgit_repo *cgit_get_repoinfo(const char *url);
  extern void cgit_repo_config_cb(const char *name, const char *value);
 diff --git a/scan-tree.c b/scan-tree.c
 index e900ad9..c7ba509 100644
 --- a/scan-tree.c
 +++ b/scan-tree.c
 @@ -149,7 +149,25 @@ static void add_repo(const char *base, struct strbuf 
 *path, repo_config_fn fn)
   strbuf_setlen(path, pathlen);
   }
  
 - if (ctx.cfg.section_from_path) {
 + if (repo-section == cgit_default_repo_section || !repo-section) {
 + strbuf_addstr(path, category);
 + if (!stat(path-buf, st) 
 + !readfile(path-buf, repo-section, size)) {
 + // trim category, as gitweb does
 + while (size  0  isspace((unsigned 
 char)repo-section[size - 1]))
 + size--;
 + if (size == 0) {
 + free(repo-section);
 + repo-section = cgit_default_repo_section;
 + }
 + else
 + repo-section[size] = '\0';
 + }
 + strbuf_setlen(path, pathlen);
 + }
 +
 + if (ctx.cfg.section_from_path
 + (repo-section == cgit_default_repo_section || 
 !repo-section)) {
   n = ctx.cfg.section_from_path;
   if (n  0) {
   slash = rel.buf - 1;
 diff --git a/shared.c b/shared.c
 index ae17d78..4b21da2 100644
 --- a/shared.c
 +++ b/shared.c
 @@ -34,6 +34,7 @@ int chk_non_negative(int result, char *msg)
  }
  
  char *cgit_default_repo_desc = [no description];
 +char *cgit_default_repo_section = ;
  struct cgit_repo *cgit_add_repo(const char *url)
  {
   struct cgit_repo *ret;
 -- 
 1.9.1
 
 ___
 CGit mailing list
 CGit@lists.zx2c4.com
 http://lists.zx2c4.com/mailman/listinfo/cgit
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: [PATCH] Support .git/category files

2015-03-05 Thread John Keeping
On Thu, Mar 05, 2015 at 06:50:47PM +0100, Jan-Marek Glogowski wrote:
 Am 05.03.2015 um 18:38 schrieb John Keeping:
  What's the advantage of this over using enable-git-config=1 and
  cgit.section (we even support gitweb.section as an alias)?
 
 Well - I have 100+ repositories, which already have the category file.
 There is no advantage over any other method.
 
 Am 05.03.2015 um 18:41 schrieb Lukas Fleischer:
  There already are at least two ways to achieve the same thing (using
  the Git configuration variables gitweb.category or cgit.section and
  repository-specific cgitrc files). Do we really need another
  alternative? Is this just for GitWeb compatibility or does this have
  any advantages over the existing options?
 
 Yes - it's just GitWeb compatibility, like - I guess - the
 description file support just the few lines before my path.

The description file is a bit different though - it's in the default
repository template that ships with Git and is used by the sample hooks
that ship there.

I'm not sure category is similar enough, particularly when you can do:

for repo in $repositories
do
git -C $repo config gitweb.category $(cat $repo/category) 

rm -f $repo/category
done

and still have a configuration that works with both Gitweb and CGit.
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: Support for submodules in tree view?

2015-03-05 Thread John Keeping
On Thu, Mar 05, 2015 at 06:19:31PM +, Dunnigan, Terrence J wrote:
 We are using cgit 0.10.1. Some of our repos have submodules, and when
 I look at a tree view I see the name of the submodule with its current
 hash, e.g.
 
 m-  Utilities @ 350bc94
 
 The submodule names are all hyperlinks, but the actual link is just a
 #. So clicking on it doesn't do anything.
 
 Is this the correct behavior? Or something on my system improperly
 configured to support submodules?

You probably need to set the module-link configuration variable in
your cgitrc file.

Since it's possible for submodules to link to a different server, there
isn't really much CGit can do in the general case.  Note that there's
also repo.module-link.path in case your submodule paths don't match
up to their URLs, although I'm quite surprised we don't support a filter
to map submodule URLs to links - something to go on the TODO list
perhaps...
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: Support for submodules in tree view?

2015-03-05 Thread John Keeping
On Thu, Mar 05, 2015 at 07:31:01PM +0100, Lukas Fleischer wrote:
 On Thu, 05 Mar 2015 at 19:25:53, John Keeping wrote:
  On Thu, Mar 05, 2015 at 06:19:31PM +, Dunnigan, Terrence J wrote:
   We are using cgit 0.10.1. Some of our repos have submodules, and when
   I look at a tree view I see the name of the submodule with its current
   hash, e.g.
   
   m-  Utilities @ 350bc94
   
   The submodule names are all hyperlinks, but the actual link is just a
   #. So clicking on it doesn't do anything.
   
   Is this the correct behavior? Or something on my system improperly
   configured to support submodules?
  
  You probably need to set the module-link configuration variable in
  your cgitrc file.
  
  Since it's possible for submodules to link to a different server, there
  isn't really much CGit can do in the general case.  Note that there's
  also repo.module-link.path in case your submodule paths don't match
  up to their URLs, although I'm quite surprised we don't support a filter
  to map submodule URLs to links - something to go on the TODO list
  perhaps...
 
 What do you think about hiding the # link, though? I don't think it is
 a good idea to pretend there is a link when there isn't...

Good idea.  My one concern would be that it hinders discovery of the
module-link feature, but I think that is outweighed by the fact that it
is perfectly reasonable for someone to only want to link some of the
submodules in their system to a web interface.

 -- 8 --
 diff --git a/ui-shared.c b/ui-shared.c
 index ff03cb2..0eeab6f 100644
 --- a/ui-shared.c
 +++ b/ui-shared.c
 @@ -555,25 +555,27 @@ void cgit_submodule_link(const char *class, char *path, 
 const char *rev)
 item = lookup_path(list, path);
 }
 }
 -   html(a );
 -   if (class)
 -   htmlf(class='%s' , class);
 -   html(href=');
 -   if (item) {
 -   html_attrf(item-util, rev);
 -   } else if (ctx.repo-module_link) {
 -   dir = strrchr(path, '/');
 -   if (dir)
 -   dir++;
 -   else
 -   dir = path;
 -   html_attrf(ctx.repo-module_link, dir, rev);
 +   if (item || ctx.repo-module_link) {
 +   html(a );
 +   if (class)
 +   htmlf(class='%s' , class);
 +   html(href=');
 +   if (item) {
 +   html_attrf(item-util, rev);
 +   } else {
 +   dir = strrchr(path, '/');
 +   if (dir)
 +   dir++;
 +   else
 +   dir = path;
 +   html_attrf(ctx.repo-module_link, dir, rev);
 +   }
 +   html(');
 +   html_txt(path);
 +   html(/a);
 } else {
 -   html(#);
 +   html_txt(path);
 }
 -   html(');
 -   html_txt(path);
 -   html(/a);
 html_txtf( @ %.7s, rev);
 if (item  tail)
 path[len - 1] = tail;
 ___
 CGit mailing list
 CGit@lists.zx2c4.com
 http://lists.zx2c4.com/mailman/listinfo/cgit
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: cache issue

2015-03-03 Thread John Keeping
On Tue, Mar 03, 2015 at 11:56:11PM +0100, Jason A. Donenfeld wrote:
 Thanks! Merged. It would be nice to see a test case built out of the
 example you gave in the commit message.

It has to run for a couple of minutes to get sensible results.  I guess
we could introduce an EXPENSIVE prerequisite in the same way Git does,
but I'm not sure how often anyone would run the test suite with that
turned on.
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: cache issue

2015-03-01 Thread John Keeping
On Sun, Mar 01, 2015 at 06:43:17PM +, Bertrand Jacquin wrote:
 On 28/02/2015 12:37, John Keeping wrote:
  On Sat, Feb 28, 2015 at 12:06:41PM +, Bertrand Jacquin wrote:
  We are still experiencing the issue. Is there any fixes with newer
  releases ?
  
  I have just tried to reproduce this with the latest version and have 
  not
  been able to do so, but I'm not aware of any changes that should have 
  an
  effect on this (there is one cache change, 6ceba45 Skip cache slot when
  time-to-live is zero, but that only applies if you set one of the *-ttl
  values to zero).
  
  The cache timeout logic relies on the mtime of the cache file, so this
  could be affected by your filesystem, but it sounds like the problem is
  that the .lock files are not being cleaned up.
 
 The filesystem here is a ext4 with no specific option except noatime 
 which quiet common.
 
  When CGit finds a .lock
  file for a cache slot it is trying to use it will just serve the stale
  content, on the assumption that is has only just been replaced.
 
 So there is so assumption the .lock can be obsolete ?
 
  I can't see many ways that you can end up with stale lock files; the
  only options are:
  
  1) CGit crashes, in which case there should be some evidence in the
 system log.
 
 That might happend, the cgi can in this case be killed after 60 seconds.

I wonder if we should be doing something like this (which is probably 3
patches if cleaned up, but shows the idea):

-- 8 --
diff --git a/cache.c b/cache.c
index e0bb47a..e7649ad 100644
--- a/cache.c
+++ b/cache.c
@@ -195,6 +195,7 @@ static int unlock_slot(struct cache_slot *slot, int 
replace_old_slot)
else
err = unlink(slot-lock_name);
 
+   slot-lock_name = NULL;
if (err)
return errno;
 
@@ -343,6 +344,14 @@ static int process_slot(struct cache_slot *slot)
return err;
 }
 
+static struct cache_slot the_slot;
+
+void cache_cleanup_locks(void)
+{
+   if (the_slot.lock_name)
+   unlock_slot(the_slot, 0);
+}
+
 /* Print cached content to stdout, generate the content if necessary. */
 int cache_process(int size, const char *path, const char *key, int ttl,
  cache_fill_fn fn)
@@ -351,7 +360,6 @@ int cache_process(int size, const char *path, const char 
*key, int ttl,
int i;
struct strbuf filename = STRBUF_INIT;
struct strbuf lockname = STRBUF_INIT;
-   struct cache_slot slot;
int result;
 
/* If the cache is disabled, just generate the content */
@@ -377,13 +385,13 @@ int cache_process(int size, const char *path, const char 
*key, int ttl,
}
strbuf_addbuf(lockname, filename);
strbuf_addstr(lockname, .lock);
-   slot.fn = fn;
-   slot.ttl = ttl;
-   slot.cache_name = filename.buf;
-   slot.lock_name = lockname.buf;
-   slot.key = key;
-   slot.keylen = strlen(key);
-   result = process_slot(slot);
+   the_slot.fn = fn;
+   the_slot.ttl = ttl;
+   the_slot.cache_name = filename.buf;
+   the_slot.lock_name = lockname.buf;
+   the_slot.key = key;
+   the_slot.keylen = strlen(key);
+   result = process_slot(the_slot);
 
strbuf_release(filename);
strbuf_release(lockname);
diff --git a/cache.h b/cache.h
index 9392836..f0d1c75 100644
--- a/cache.h
+++ b/cache.h
@@ -28,6 +28,9 @@ extern int cache_process(int size, const char *path, const 
char *key, int ttl,
 /* List info about all cache entries on stdout */
 extern int cache_ls(const char *path);
 
+/* Cleanup open cache lock files on abnormal exit */
+extern void cache_cleanup_locks(void);
+
 /* Print a message to stdout */
 __attribute__((format (printf,1,2)))
 extern void cache_log(const char *format, ...);
diff --git a/cgit.c b/cgit.c
index d9a8b1f..0912a3d 100644
--- a/cgit.c
+++ b/cgit.c
@@ -1031,6 +1031,26 @@ static int calc_ttl()
return ctx.cfg.cache_repo_ttl;
 }
 
+static void cleanup_handler(int signum)
+{
+   cache_cleanup_locks();
+}
+
+static void register_signal_handlers(void)
+{
+   struct sigaction sa = {
+   .sa_handler = cleanup_handler,
+   .sa_flags = SA_RESETHAND,
+   };
+   sigemptyset(sa.sa_mask);
+
+   sigaction(SIGHUP, sa, NULL);
+   sigaction(SIGINT, sa, NULL);
+   sigaction(SIGQUIT, sa, NULL);
+   sigaction(SIGPIPE, sa, NULL);
+   sigaction(SIGTERM, sa, NULL);
+}
+
 int main(int argc, const char **argv)
 {
const char *path;
@@ -1039,6 +1059,8 @@ int main(int argc, const char **argv)
cgit_init_filters();
atexit(cgit_cleanup_filters);
 
+   register_signal_handlers();
+
prepare_context();
cgit_repolist.length = 0;
cgit_repolist.count = 0;
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: Description for section and link for repos

2015-03-05 Thread John Keeping
On Thu, Mar 05, 2015 at 10:51:09AM +0100, Pablo Rauzy wrote:
 Hello again, and thanks for your quick response.
 
 On 2015-03-05, John Keeping wrote:
  On Thu, Mar 05, 2015 at 09:22:55AM +0100, Pablo Rauzy wrote:
  My first question is about sections.
  I would like to be able to add a title and a little description for each
  section, just like I can with the cgit.desc configuration for each the
  git repos.
  
  Example : http://clandest.in/sensi
  Here I would like the title to be clandest.in / sensi rather than just
  clandest.in, and I would like the description to be SEN Security
  Inspector rather than the root description.
 
  Are you currently using section-from-path?  If you use the
  repo.section configuration option then you can name the sections
  however you want, but this is not possible if you are generating the
  sections automatically.
 
 Indeed I am using section-from-path, but the name of the sections are
 just fine.
 
 What I was looking for is the possibility to for instance make cgit
 using a file called description in the directory that it makes a
 section as description of that section.

I don't think this has ever been supported.

I hadn't realised that http://clandest.in/sensi wasn't the root of the
repository.  CGit doesn't generate any links to the section pages so I'm
not sure much thought has gone into the formatting of those pages.

  If you're combining CGit with Gitolite you may be able to avoid listing
  repositories twice by using CGit's enable-git-config option to
  configure the repo.section variable (I'm not sure if you can set a
  config variable on a group of repositories in Gitolite, but I think you
  should be able to).
 
 I'm not using Gitolite.
 
  My second question is about repository.
  I would like to add title and a link to a project home page somewhere
  else than on the logo.
  
  Example : http://clandest.in/sensi/finja
  Here I would like the title to be clandest.in / sensi / finja rather
  than index : finja, and I would like to add a tab homepage after
  diff which would link to http://pablo.rauzy.name/sensi/finja.html in
  this example.
  
  
  Is what I'm looking for already possible? If yes, how? If not, I think
  it would be a nice addition.
 
  I don't think this is currently possible.  Note that the word index is
  actually a link to the list of repositories.
 
 Yes I noticed that, but I find it less nice than having the root-title
 for the link.
 
  For the homepage link, have you considered simply adding a link to the
  repository's readme (on the about tab)?  I thought there was a way to
  make the about tab the default instead of the summary tab, but I
  can't find it when I go looking so I must be wrong about that.
 
 That is exactly what I did for now. Maybe it is enough. I'll see with
 the time how much I like it this way :).
 
 FWIW, here is the filter I wrote to make link clickable in plaintext
 READMEs: http://paste.fulltxt.net/LKdVjbHaY (it is quick and dirty, it
 simply considers that anything between  and  and starting with http
 is a link).

Note that there is a collection of scripts in the filters directory
that will handle Markdown, ReStructuredText and a variety of other
formats.
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: [PATCH] Simplify commit and tag parsing

2015-03-05 Thread John Keeping
On Thu, Mar 05, 2015 at 11:46:55AM +0100, Jason A. Donenfeld wrote:
 This commit breaks ui-tag. The first few lines of tag messages are cut off.

It looks like parse_user() consumes the trailing LF on the user line, so
next_header_line() ends up skipping the blank line at the end of the
header.

I suspect the right answer is to stop parse_user() eating the LF since
all of the call sites go through next_header_line() after this patch.
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: Description for section and link for repos

2015-03-05 Thread John Keeping
On Thu, Mar 05, 2015 at 09:22:55AM +0100, Pablo Rauzy wrote:
 My first question is about sections.
 I would like to be able to add a title and a little description for each
 section, just like I can with the cgit.desc configuration for each the
 git repos.
 
 Example : http://clandest.in/sensi
 Here I would like the title to be clandest.in / sensi rather than just
 clandest.in, and I would like the description to be SEN Security
 Inspector rather than the root description.

Are you currently using section-from-path?  If you use the
repo.section configuration option then you can name the sections
however you want, but this is not possible if you are generating the
sections automatically.

If you're combining CGit with Gitolite you may be able to avoid listing
repositories twice by using CGit's enable-git-config option to
configure the repo.section variable (I'm not sure if you can set a
config variable on a group of repositories in Gitolite, but I think you
should be able to).

 My second question is about repository.
 I would like to add title and a link to a project home page somewhere
 else than on the logo.
 
 Example : http://clandest.in/sensi/finja
 Here I would like the title to be clandest.in / sensi / finja rather
 than index : finja, and I would like to add a tab homepage after
 diff which would link to http://pablo.rauzy.name/sensi/finja.html in
 this example.
 
 
 Is what I'm looking for already possible? If yes, how? If not, I think
 it would be a nice addition.

I don't think this is currently possible.  Note that the word index is
actually a link to the list of repositories.

For the homepage link, have you considered simply adding a link to the
repository's readme (on the about tab)?  I thought there was a way to
make the about tab the default instead of the summary tab, but I
can't find it when I go looking so I must be wrong about that.
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


Re: cache issue

2015-03-03 Thread John Keeping
On Sun, Mar 01, 2015 at 07:36:00PM +, John Keeping wrote:
 On Sun, Mar 01, 2015 at 06:43:17PM +, Bertrand Jacquin wrote:
  On 28/02/2015 12:37, John Keeping wrote:
   On Sat, Feb 28, 2015 at 12:06:41PM +, Bertrand Jacquin wrote:
   We are still experiencing the issue. Is there any fixes with newer
   releases ?
   
   I have just tried to reproduce this with the latest version and have 
   not
   been able to do so, but I'm not aware of any changes that should have 
   an
   effect on this (there is one cache change, 6ceba45 Skip cache slot when
   time-to-live is zero, but that only applies if you set one of the *-ttl
   values to zero).
   
   The cache timeout logic relies on the mtime of the cache file, so this
   could be affected by your filesystem, but it sounds like the problem is
   that the .lock files are not being cleaned up.
  
  The filesystem here is a ext4 with no specific option except noatime 
  which quiet common.
  
   When CGit finds a .lock
   file for a cache slot it is trying to use it will just serve the stale
   content, on the assumption that is has only just been replaced.
  
  So there is so assumption the .lock can be obsolete ?
  
   I can't see many ways that you can end up with stale lock files; the
   only options are:
   
   1) CGit crashes, in which case there should be some evidence in the
  system log.
  
  That might happend, the cgi can in this case be killed after 60 seconds.
 
 I wonder if we should be doing something like this (which is probably 3
 patches if cleaned up, but shows the idea):

Having thought about this a bit more, maybe this is the safer way to do
it, since this will detect stale .lock files no matter how they're
caused.

-- 8 --
diff --git a/cache.c b/cache.c
index 801e63f..dbfa6a9 100644
--- a/cache.c
+++ b/cache.c
@@ -161,10 +161,24 @@ static int close_lock(struct cache_slot *slot)
  */
 static int lock_slot(struct cache_slot *slot)
 {
-   slot-lock_fd = open(slot-lock_name, O_RDWR | O_CREAT | O_EXCL,
+   struct flock lock = {
+   .l_type = F_WRLCK,
+   .l_whence = SEEK_SET,
+   .l_start = 0,
+   .l_len = 0,
+   };
+
+   slot-lock_fd = open(slot-lock_name, O_RDWR | O_CREAT,
 S_IRUSR | S_IWUSR);
if (slot-lock_fd == -1)
return errno;
+   if (fcntl(slot-lock_fd, F_SETLK, lock)  0) {
+   int saved_errno = errno;
+   fprintf(stderr, failed to lock slot!);
+   close(slot-lock_fd);
+   slot-lock_fd = -1;
+   return saved_errno;
+   }
if (xwrite(slot-lock_fd, slot-key, slot-keylen + 1)  0)
return errno;
return 0;
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


[PATCH 02/13] Avoid non-ANSI function declarations

2015-03-08 Thread John Keeping
Sparse says things like:

warning: non-ANSI function declaration of function 'calc_ttl'

Signed-off-by: John Keeping j...@keeping.me.uk
---
 cgit.c|  2 +-
 filter.c  |  2 +-
 ui-diff.c |  2 +-
 ui-refs.c |  4 ++--
 ui-repolist.c |  6 +++---
 ui-shared.c   | 12 ++--
 ui-ssdiff.c   | 16 
 ui-summary.c  |  2 +-
 ui-tree.c |  4 ++--
 9 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/cgit.c b/cgit.c
index 0ad8171..c16ed8c 100644
--- a/cgit.c
+++ b/cgit.c
@@ -1013,7 +1013,7 @@ static void cgit_parse_args(int argc, const char **argv)
}
 }
 
-static int calc_ttl()
+static int calc_ttl(void)
 {
if (!ctx.repo)
return ctx.cfg.cache_root_ttl;
diff --git a/filter.c b/filter.c
index 9f433db..ebf4937 100644
--- a/filter.c
+++ b/filter.c
@@ -72,7 +72,7 @@ static inline void hook_write(struct cgit_filter *filter, 
ssize_t (*new_write)(s
filter_write = new_write;
 }
 
-static inline void unhook_write()
+static inline void unhook_write(void)
 {
assert(filter_write != NULL);
assert(current_write_filter != NULL);
diff --git a/ui-diff.c b/ui-diff.c
index 5b6df1f..8eff178 100644
--- a/ui-diff.c
+++ b/ui-diff.c
@@ -311,7 +311,7 @@ static void filepair_cb(struct diff_filepair *pair)
cgit_ssdiff_footer();
 }
 
-void cgit_print_diff_ctrls()
+void cgit_print_diff_ctrls(void)
 {
int i, curr;
 
diff --git a/ui-refs.c b/ui-refs.c
index ac8a6d4..d3d71dd 100644
--- a/ui-refs.c
+++ b/ui-refs.c
@@ -82,7 +82,7 @@ static int print_branch(struct refinfo *ref)
return 0;
 }
 
-static void print_tag_header()
+static void print_tag_header(void)
 {
html(tr class='nohover'th class='left'Tag/th
 th class='left'Download/th
@@ -234,7 +234,7 @@ void cgit_print_tags(int maxcount)
cgit_free_reflist_inner(list);
 }
 
-void cgit_print_refs()
+void cgit_print_refs(void)
 {
 
html(table class='list nowrap');
diff --git a/ui-repolist.c b/ui-repolist.c
index e945f67..a6d0321 100644
--- a/ui-repolist.c
+++ b/ui-repolist.c
@@ -117,7 +117,7 @@ static void print_sort_header(const char *title, const char 
*sort)
htmlf('%s/a/th, title);
 }
 
-static void print_header()
+static void print_header(void)
 {
html(tr class='nohover');
print_sort_header(Name, name);
@@ -247,7 +247,7 @@ static int sort_repolist(char *field)
 }
 
 
-void cgit_print_repolist()
+void cgit_print_repolist(void)
 {
int i, columns = 3, hits = 0, header = 0;
char *last_section = NULL;
@@ -344,7 +344,7 @@ void cgit_print_repolist()
cgit_print_docend();
 }
 
-void cgit_print_site_readme()
+void cgit_print_site_readme(void)
 {
if (!ctx.cfg.root_readme)
return;
diff --git a/ui-shared.c b/ui-shared.c
index ff03cb2..6d3cfa9 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -46,7 +46,7 @@ void cgit_vprint_error(const char *fmt, va_list ap)
html(/div\n);
 }
 
-const char *cgit_httpscheme()
+const char *cgit_httpscheme(void)
 {
if (ctx.env.https  !strcmp(ctx.env.https, on))
return https://;;
@@ -54,7 +54,7 @@ const char *cgit_httpscheme()
return http://;;
 }
 
-const char *cgit_hosturl()
+const char *cgit_hosturl(void)
 {
if (ctx.env.http_host)
return ctx.env.http_host;
@@ -65,14 +65,14 @@ const char *cgit_hosturl()
return fmtalloc(%s:%s, ctx.env.server_name, ctx.env.server_port);
 }
 
-const char *cgit_currenturl()
+const char *cgit_currenturl(void)
 {
if (!ctx.qry.url)
return cgit_rooturl();
return ctx.qry.url;
 }
 
-const char *cgit_rooturl()
+const char *cgit_rooturl(void)
 {
if (ctx.cfg.virtual_root)
return ctx.cfg.virtual_root;
@@ -80,7 +80,7 @@ const char *cgit_rooturl()
return ctx.cfg.script_name;
 }
 
-const char *cgit_loginurl()
+const char *cgit_loginurl(void)
 {
static const char *login_url = 0;
if (!login_url)
@@ -735,7 +735,7 @@ void cgit_print_docstart(void)
html_include(ctx.cfg.header);
 }
 
-void cgit_print_docend()
+void cgit_print_docend(void)
 {
html(/div !-- class=content --\n);
if (ctx.cfg.embedded) {
diff --git a/ui-ssdiff.c b/ui-ssdiff.c
index 08cf513..2146c71 100644
--- a/ui-ssdiff.c
+++ b/ui-ssdiff.c
@@ -18,7 +18,7 @@ struct deferred_lines {
 static struct deferred_lines *deferred_old, *deferred_old_last;
 static struct deferred_lines *deferred_new, *deferred_new_last;
 
-static void create_or_reset_lcs_table()
+static void create_or_reset_lcs_table(void)
 {
int i;
 
@@ -276,7 +276,7 @@ static void print_ssdiff_line(char *class,
free(old_line);
 }
 
-static void print_deferred_old_lines()
+static void print_deferred_old_lines(void)
 {
struct deferred_lines *iter_old, *tmp;
iter_old = deferred_old;
@@ -289,7 +289,7 @@ static void print_deferred_old_lines()
}
 }
 
-static void

[PATCH 00/13] Fixes for problems detected by Sparse

2015-03-08 Thread John Keeping
Sparse[0] detects several potential problems in CGit, which are all
fixed by this set of patches.  Most of these are style issues that are
correct either way (using integer zero as a NULL pointer), but I think
there is value in keeping the build clean of Sparse warnings.


[0] https://sparse.wiki.kernel.org/index.php/Main_Page

John Keeping (13):
  Makefile: add a target to run CGit through sparse
  Avoid non-ANSI function declarations
  Avoid signed bitfields
  scan-tree: make some variables 'static'
  shared: make some variables 'static'
  ui-log: make some variables 'static'
  ui-repolist: make sortcolumn definitions 'static const'
  ui-shared: make cgit_doctype 'static'
  ui-stats: make cgit_period definitions 'static const'
  ui-shared: avoid initializing static variable to zero
  ui-shared: don't use an integer as a NULL pointer
  cache: don't use an integer as a NULL pointer
  html: avoid using a plain integer as a NULL pointer

 Makefile  |  3 +++
 cache.c   |  2 +-
 cgit.c|  2 +-
 cgit.mk   |  9 -
 filter.c  |  2 +-
 html.c| 54 --
 scan-tree.c   |  4 ++--
 shared.c  |  4 ++--
 ui-blob.c |  4 ++--
 ui-diff.c |  4 ++--
 ui-log.c  |  2 +-
 ui-refs.c |  4 ++--
 ui-repolist.c | 10 +-
 ui-shared.c   | 18 +-
 ui-ssdiff.c   | 16 
 ui-stats.c| 14 +++---
 ui-stats.h|  2 +-
 ui-summary.c  |  2 +-
 ui-tree.c |  4 ++--
 19 files changed, 90 insertions(+), 70 deletions(-)

-- 
2.3.1.308.g754cd77

___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


[PATCH 01/13] Makefile: add a target to run CGit through sparse

2015-03-08 Thread John Keeping
Signed-off-by: John Keeping j...@keeping.me.uk
---
 Makefile | 3 +++
 cgit.mk  | 9 -
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index ed329e8..42ed230 100644
--- a/Makefile
+++ b/Makefile
@@ -68,6 +68,9 @@ all:: cgit
 cgit:
$(QUIET_SUBDIR0)git $(QUIET_SUBDIR1) -f ../cgit.mk ../cgit 
$(EXTRA_GIT_TARGETS) NO_CURL=1
 
+sparse:
+   $(QUIET_SUBDIR0)git $(QUIET_SUBDIR1) -f ../cgit.mk NO_CURL=1 cgit-sparse
+
 test:
@$(MAKE) --no-print-directory cgit EXTRA_GIT_TARGETS=all
$(QUIET_SUBDIR0)tests $(QUIET_SUBDIR1) all
diff --git a/cgit.mk b/cgit.mk
index 5048b09..1b50307 100644
--- a/cgit.mk
+++ b/cgit.mk
@@ -96,7 +96,7 @@ CGIT_OBJS := $(addprefix $(CGIT_PREFIX),$(CGIT_OBJ_NAMES))
 
 # Only cgit.c reference CGIT_VERSION so we only rebuild its objects when the
 # version changes.
-CGIT_VERSION_OBJS := $(addprefix $(CGIT_PREFIX),cgit.o)
+CGIT_VERSION_OBJS := $(addprefix $(CGIT_PREFIX),cgit.o cgit.sp)
 $(CGIT_VERSION_OBJS): $(CGIT_PREFIX)VERSION
 $(CGIT_VERSION_OBJS): EXTRA_CPPFLAGS = \
-DCGIT_VERSION='$(CGIT_VERSION)'
@@ -129,3 +129,10 @@ $(CGIT_OBJS): %.o: %.c GIT-CFLAGS 
$(CGIT_PREFIX)CGIT-CFLAGS $(missing_dep_dirs)
 $(CGIT_PREFIX)cgit: $(CGIT_OBJS) GIT-LDFLAGS $(GITLIBS)
@echo 11 * $(LUA_MESSAGE)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) 
$(LIBS) $(CGIT_LIBS)
+
+CGIT_SP_OBJS := $(patsubst %.o,%.sp,$(CGIT_OBJS))
+
+$(CGIT_SP_OBJS): %.sp: %.c GIT-CFLAGS $(CGIT_PREFIX)CGIT-CFLAGS FORCE
+   $(QUIET_SP)cgcc -no-compile $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) 
$(CGIT_CFLAGS) $(SPARSE_FLAGS) $
+
+cgit-sparse: $(CGIT_SP_OBJS)
-- 
2.3.1.308.g754cd77

___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


[PATCH 13/13] html: avoid using a plain integer as a NULL pointer

2015-03-08 Thread John Keeping
Sparse complains about this table because we use the integer zero as the
NULL pointer.  Use this as an opportunity to reformat the table so that
it always contains 8 elements per row, making it easier to see which
values are being set and which are not.

Signed-off-by: John Keeping j...@keeping.me.uk
---
 html.c | 54 --
 1 file changed, 32 insertions(+), 22 deletions(-)

diff --git a/html.c b/html.c
index f0ee2d6..155cde5 100644
--- a/html.c
+++ b/html.c
@@ -17,28 +17,38 @@
 
 /* Percent-encoding of each character, except: a-zA-Z0-9!$()*,./:;@- */
 static const char* url_escape_table[256] = {
-   %00, %01, %02, %03, %04, %05, %06, %07, %08, %09,
-   %0a, %0b, %0c, %0d, %0e, %0f, %10, %11, %12, %13,
-   %14, %15, %16, %17, %18, %19, %1a, %1b, %1c, %1d,
-   %1e, %1f, %20, 0, %22, %23, 0, %25, %26, %27, 0, 0, 0,
-   %2b, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, %3c, %3d,
-   %3e, %3f, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-   0, 0, 0, 0, 0, 0, 0, 0, 0, %5c, 0, %5e, 0, %60, 0, 0, 0, 0, 0,
-   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, %7b,
-   %7c, %7d, 0, %7f, %80, %81, %82, %83, %84, %85,
-   %86, %87, %88, %89, %8a, %8b, %8c, %8d, %8e, %8f,
-   %90, %91, %92, %93, %94, %95, %96, %97, %98, %99,
-   %9a, %9b, %9c, %9d, %9e, %9f, %a0, %a1, %a2, %a3,
-   %a4, %a5, %a6, %a7, %a8, %a9, %aa, %ab, %ac, %ad,
-   %ae, %af, %b0, %b1, %b2, %b3, %b4, %b5, %b6, %b7,
-   %b8, %b9, %ba, %bb, %bc, %bd, %be, %bf, %c0, %c1,
-   %c2, %c3, %c4, %c5, %c6, %c7, %c8, %c9, %ca, %cb,
-   %cc, %cd, %ce, %cf, %d0, %d1, %d2, %d3, %d4, %d5,
-   %d6, %d7, %d8, %d9, %da, %db, %dc, %dd, %de, %df,
-   %e0, %e1, %e2, %e3, %e4, %e5, %e6, %e7, %e8, %e9,
-   %ea, %eb, %ec, %ed, %ee, %ef, %f0, %f1, %f2, %f3,
-   %f4, %f5, %f6, %f7, %f8, %f9, %fa, %fb, %fc, %fd,
-   %fe, %ff
+   %00, %01, %02, %03, %04, %05, %06, %07,
+   %08, %09, %0a, %0b, %0c, %0d, %0e, %0f,
+   %10, %11, %12, %13, %14, %15, %16, %17,
+   %18, %19, %1a, %1b, %1c, %1d, %1e, %1f,
+   %20, NULL,  %22, %23, NULL,  %25, %26, %27,
+   NULL,  NULL,  NULL,  %2b, NULL,  NULL,  NULL,  NULL,
+   NULL,  NULL,  NULL,  NULL,  NULL,  NULL,  NULL,  NULL,
+   NULL,  NULL,  NULL,  NULL,  %3c, %3d, %3e, %3f,
+   NULL,  NULL,  NULL,  NULL,  NULL,  NULL,  NULL,  NULL,
+   NULL,  NULL,  NULL,  NULL,  NULL,  NULL,  NULL,  NULL,
+   NULL,  NULL,  NULL,  NULL,  NULL,  NULL,  NULL,  NULL,
+   NULL,  NULL,  NULL,  NULL,  %5c, NULL,  %5e, NULL,
+   %60, NULL,  NULL,  NULL,  NULL,  NULL,  NULL,  NULL,
+   NULL,  NULL,  NULL,  NULL,  NULL,  NULL,  NULL,  NULL,
+   NULL,  NULL,  NULL,  NULL,  NULL,  NULL,  NULL,  NULL,
+   NULL,  NULL,  NULL,  %7b, %7c, %7d, NULL,  %7f,
+   %80, %81, %82, %83, %84, %85, %86, %87,
+   %88, %89, %8a, %8b, %8c, %8d, %8e, %8f,
+   %90, %91, %92, %93, %94, %95, %96, %97,
+   %98, %99, %9a, %9b, %9c, %9d, %9e, %9f,
+   %a0, %a1, %a2, %a3, %a4, %a5, %a6, %a7,
+   %a8, %a9, %aa, %ab, %ac, %ad, %ae, %af,
+   %b0, %b1, %b2, %b3, %b4, %b5, %b6, %b7,
+   %b8, %b9, %ba, %bb, %bc, %bd, %be, %bf,
+   %c0, %c1, %c2, %c3, %c4, %c5, %c6, %c7,
+   %c8, %c9, %ca, %cb, %cc, %cd, %ce, %cf,
+   %d0, %d1, %d2, %d3, %d4, %d5, %d6, %d7,
+   %d8, %d9, %da, %db, %dc, %dd, %de, %df,
+   %e0, %e1, %e2, %e3, %e4, %e5, %e6, %e7,
+   %e8, %e9, %ea, %eb, %ec, %ed, %ee, %ef,
+   %f0, %f1, %f2, %f3, %f4, %f5, %f6, %f7,
+   %f8, %f9, %fa, %fb, %fc, %fd, %fe, %ff
 };
 
 char *fmt(const char *format, ...)
-- 
2.3.1.308.g754cd77

___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


[PATCH 08/13] ui-shared: make cgit_doctype 'static'

2015-03-08 Thread John Keeping
This is not used outside this file and is not declared.

Signed-off-by: John Keeping j...@keeping.me.uk
---
 ui-shared.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui-shared.c b/ui-shared.c
index 6d3cfa9..d4c4bb9 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -11,7 +11,7 @@
 #include cmd.h
 #include html.h
 
-const char cgit_doctype[] =
+static const char cgit_doctype[] =
 !DOCTYPE html PUBLIC \-//W3C//DTD XHTML 1.0 Transitional//EN\\n
   \http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd\;\n;
 
-- 
2.3.1.308.g754cd77

___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


[PATCH 09/13] ui-stats: make cgit_period definitions 'static const'

2015-03-08 Thread John Keeping
These definitions should not be modified (and never are) so we can move
them to .rodata.

Signed-off-by: John Keeping j...@keeping.me.uk
---
 ui-stats.c | 14 +++---
 ui-stats.h |  2 +-
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/ui-stats.c b/ui-stats.c
index a264f6a..9cd8247 100644
--- a/ui-stats.c
+++ b/ui-stats.c
@@ -125,7 +125,7 @@ static char *pretty_year(struct tm *tm)
return fmt(%d, tm-tm_year + 1900);
 }
 
-struct cgit_period periods[] = {
+static const struct cgit_period periods[] = {
{'w', week, 12, 4, trunc_week, dec_week, inc_week, pretty_week},
{'m', month, 12, 4, trunc_month, dec_month, inc_month, pretty_month},
{'q', quarter, 12, 4, trunc_quarter, dec_quarter, inc_quarter, 
pretty_quarter},
@@ -136,7 +136,7 @@ struct cgit_period periods[] = {
  * and update the period pointer to the correcsponding struct.
  * If no matching code is found, return 0.
  */
-int cgit_find_stats_period(const char *expr, struct cgit_period **period)
+int cgit_find_stats_period(const char *expr, const struct cgit_period **period)
 {
int i;
char code = '\0';
@@ -165,7 +165,7 @@ const char *cgit_find_stats_periodname(int idx)
 }
 
 static void add_commit(struct string_list *authors, struct commit *commit,
-   struct cgit_period *period)
+   const struct cgit_period *period)
 {
struct commitinfo *info;
struct string_list_item *author, *item;
@@ -209,7 +209,7 @@ static int cmp_total_commits(const void *a1, const void *a2)
 /* Walk the commit DAG and collect number of commits per author per
  * timeperiod into a nested string_list collection.
  */
-static struct string_list collect_stats(struct cgit_period *period)
+static struct string_list collect_stats(const struct cgit_period *period)
 {
struct string_list authors;
struct rev_info rev;
@@ -256,7 +256,7 @@ static void print_combined_authorrow(struct string_list 
*authors, int from,
 const char *leftclass,
 const char *centerclass,
 const char *rightclass,
-struct cgit_period *period)
+const struct cgit_period *period)
 {
struct string_list_item *author;
struct authorstat *authorstat;
@@ -295,7 +295,7 @@ static void print_combined_authorrow(struct string_list 
*authors, int from,
 }
 
 static void print_authors(struct string_list *authors, int top,
- struct cgit_period *period)
+ const struct cgit_period *period)
 {
struct string_list_item *author;
struct authorstat *authorstat;
@@ -363,7 +363,7 @@ static void print_authors(struct string_list *authors, int 
top,
 void cgit_show_stats(void)
 {
struct string_list authors;
-   struct cgit_period *period;
+   const struct cgit_period *period;
int top, i;
const char *code = w;
 
diff --git a/ui-stats.h b/ui-stats.h
index 341ab13..0e61b03 100644
--- a/ui-stats.h
+++ b/ui-stats.h
@@ -20,7 +20,7 @@ struct cgit_period {
char *(*pretty)(struct tm *tm);
 };
 
-extern int cgit_find_stats_period(const char *expr, struct cgit_period 
**period);
+extern int cgit_find_stats_period(const char *expr, const struct cgit_period 
**period);
 extern const char *cgit_find_stats_periodname(int idx);
 
 extern void cgit_show_stats(void);
-- 
2.3.1.308.g754cd77

___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


[PATCH 10/13] ui-shared: avoid initializing static variable to zero

2015-03-08 Thread John Keeping
Sparse complains that we are using a plain integer as a NULL pointer
here, but in fact we do not have to specify a value for this variable at
all since it has static storage duration and thus will be initialized to
NULL by the compiler.

Signed-off-by: John Keeping j...@keeping.me.uk
---
 ui-shared.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui-shared.c b/ui-shared.c
index d4c4bb9..1e3c131 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -82,7 +82,7 @@ const char *cgit_rooturl(void)
 
 const char *cgit_loginurl(void)
 {
-   static const char *login_url = 0;
+   static const char *login_url;
if (!login_url)
login_url = fmtalloc(%s?p=login, cgit_rooturl());
return login_url;
-- 
2.3.1.308.g754cd77

___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


[PATCH 12/13] cache: don't use an integer as a NULL pointer

2015-03-08 Thread John Keeping
Signed-off-by: John Keeping j...@keeping.me.uk
---
 cache.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cache.c b/cache.c
index 900b161..cd99812 100644
--- a/cache.c
+++ b/cache.c
@@ -411,7 +411,7 @@ int cache_ls(const char *path)
DIR *dir;
struct dirent *ent;
int err = 0;
-   struct cache_slot slot = { 0 };
+   struct cache_slot slot = { NULL };
struct strbuf fullname = STRBUF_INIT;
size_t prefixlen;
 
-- 
2.3.1.308.g754cd77

___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


[PATCH 07/13] ui-repolist: make sortcolumn definitions 'static const'

2015-03-08 Thread John Keeping
These are not used outside this file and are not declared; they are also
never modified.

Signed-off-by: John Keeping j...@keeping.me.uk
---
 ui-repolist.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ui-repolist.c b/ui-repolist.c
index a6d0321..2453a7f 100644
--- a/ui-repolist.c
+++ b/ui-repolist.c
@@ -223,7 +223,7 @@ struct sortcolumn {
int (*fn)(const void *a, const void *b);
 };
 
-struct sortcolumn sortcolumn[] = {
+static const struct sortcolumn sortcolumn[] = {
{section, sort_section},
{name, sort_name},
{desc, sort_desc},
@@ -234,7 +234,7 @@ struct sortcolumn sortcolumn[] = {
 
 static int sort_repolist(char *field)
 {
-   struct sortcolumn *column;
+   const struct sortcolumn *column;
 
for (column = sortcolumn[0]; column-name; column++) {
if (strcmp(field, column-name))
-- 
2.3.1.308.g754cd77

___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


[PATCH 03/13] Avoid signed bitfields

2015-03-08 Thread John Keeping
Bitfields are only defined for unsigned types.

Detected by sparse.

Signed-off-by: John Keeping j...@keeping.me.uk
---
 ui-blob.c | 4 ++--
 ui-diff.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/ui-blob.c b/ui-blob.c
index a025bca..388a017 100644
--- a/ui-blob.c
+++ b/ui-blob.c
@@ -14,8 +14,8 @@
 struct walk_tree_context {
const char *match_path;
unsigned char *matched_sha1;
-   int found_path:1;
-   int file_only:1;
+   unsigned int found_path:1;
+   unsigned int file_only:1;
 };
 
 static int walk_tree(const unsigned char *sha1, struct strbuf *base,
diff --git a/ui-diff.c b/ui-diff.c
index 8eff178..1cf2ce0 100644
--- a/ui-diff.c
+++ b/ui-diff.c
@@ -31,7 +31,7 @@ static struct fileinfo {
unsigned int removed;
unsigned long old_size;
unsigned long new_size;
-   int binary:1;
+   unsigned int binary:1;
 } *items;
 
 static int use_ssdiff = 0;
-- 
2.3.1.308.g754cd77

___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit


  1   2   3   4   5   >