[PATCH 0/8] Portability fixes

[PATCH 0/8] Portability fixes

[john]

This is a collection of vaguely-related changes that make us take fuller
advantage of git.git's support for a variety of platform so that we
build more reliably out of the box.

The header ordering issue is definitely painful on Solaris where the
system headers define _FILE_OFFSET_BITS=32 if it's not already defined
and git-compat-util.h defines it to 64 unconditionally.


John Keeping (8):
  tests: allow shell to be overridden
  Makefile: include Git's config.mak.uname
  Remove redundant includes
  configfile.c: don't include system headers directly
  cache.c: fix header order
  cgit.h: move stdbool.h from ui-shared.h
  ui-tree: use "sane" isgraph()
  filter: don't use dlsym unnecessarily

 Makefile       |  1 +
 cache.c        |  6 ++---
 cgit.h         |  5 ++++
 configfile.c   |  3 +--
 filter.c       | 84 +++++++++++++++++++++++++++++-----------------------------
 html.c         |  6 -----
 shared.c       |  1 -
 tests/Makefile |  6 ++++-
 ui-plain.c     |  1 -
 ui-repolist.c  |  1 -
 ui-shared.h    |  2 --
 ui-summary.c   |  1 -
 ui-tree.c      |  1 -
 13 files changed, 57 insertions(+), 61 deletions(-)

-- 
2.5.0.466.g9af26fa

[PATCH 1/8] tests: allow shell to be overridden

[john]

On some systems (e.g. Solaris), /bin/sh is not a POSIX shell.  Git
already provides suitable overrides in its config.mak.uname file and we
provide cgit.conf to allow the user to further change this.

The code for this is taken from Git's t/Makefile, meaning that we now
invoke the tests in the same way that Git does.

Signed-off-by: John Keeping <john at keeping.me.uk>
---
 tests/Makefile | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tests/Makefile b/tests/Makefile
index 1556475..65e1117 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -1,11 +1,15 @@
+include ../git/config.mak.uname
+-include ../cgit.conf
 
+SHELL_PATH ?= $(SHELL)
+SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
 
 T = $(wildcard t[0-9][0-9][0-9][0-9]-*.sh)
 
 all: $(T)
 
 $(T):
-	@./$@ $(CGIT_TEST_OPTS)
+	@'$(SHELL_PATH_SQ)' $@ $(CGIT_TEST_OPTS)
 
 clean:
 	$(RM) -rf trash
-- 
2.5.0.466.g9af26fa

[PATCH 2/8] Makefile: include Git's config.mak.uname

[john]

This pulls in the correct value of $(INSTALL) on a wide variety of
systems.

Signed-off-by: John Keeping <john at keeping.me.uk>
---
 Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Makefile b/Makefile
index 65b4318..74061a3 100644
--- a/Makefile
+++ b/Makefile
@@ -33,6 +33,7 @@ DOC_PDF  = $(patsubst %.txt,%.pdf,$(MAN_TXT))
 
 #-include config.mak
 
+include git/config.mak.uname
 #
 # Let the user override the above settings.
 #
-- 
2.5.0.466.g9af26fa

[PATCH 3/8] Remove redundant includes

[john]

These are all included in git-compat-util.h (when necessary), which we
include in cgit.h.

Signed-off-by: John Keeping <john at keeping.me.uk>
---
 filter.c      | 6 ------
 html.c        | 6 ------
 shared.c      | 1 -
 ui-plain.c    | 1 -
 ui-repolist.c | 1 -
 ui-summary.c  | 1 -
 6 files changed, 16 deletions(-)

diff --git a/filter.c b/filter.c
index ebf4937..c7037a3 100644
--- a/filter.c
+++ b/filter.c
@@ -8,13 +8,7 @@
 
 #include "cgit.h"
 #include "html.h"
-#include <sys/types.h>
-#include <sys/wait.h>
-#include <unistd.h>
-#include <string.h>
-#include <stdlib.h>
 #include <dlfcn.h>
-#include <errno.h>
 #ifndef NO_LUA
 #include <lua.h>
 #include <lualib.h>
diff --git a/html.c b/html.c
index 155cde5..a47cff0 100644
--- a/html.c
+++ b/html.c
@@ -8,12 +8,6 @@
 
 #include "cgit.h"
 #include "html.h"
-#include <unistd.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <stdarg.h>
-#include <string.h>
-#include <errno.h>
 
 /* Percent-encoding of each character, except: a-zA-Z0-9!$()*,./:;@- */
 static const char* url_escape_table[256] = {
diff --git a/shared.c b/shared.c
index 0431b59..3bd2a9e 100644
--- a/shared.c
+++ b/shared.c
@@ -7,7 +7,6 @@
  */
 
 #include "cgit.h"
-#include <stdio.h>
 
 struct cgit_repolist cgit_repolist;
 struct cgit_context ctx;
diff --git a/ui-plain.c b/ui-plain.c
index b787bc3..3a2cb47 100644
--- a/ui-plain.c
+++ b/ui-plain.c
@@ -6,7 +6,6 @@
  *   (see COPYING for full license text)
  */
 
-#include <stdio.h>
 #include "cgit.h"
 #include "ui-plain.h"
 #include "html.h"
diff --git a/ui-repolist.c b/ui-repolist.c
index edefc4c..43253ed 100644
--- a/ui-repolist.c
+++ b/ui-repolist.c
@@ -10,7 +10,6 @@
 #include "ui-repolist.h"
 #include "html.h"
 #include "ui-shared.h"
-#include <strings.h>
 
 static time_t read_agefile(char *path)
 {
diff --git a/ui-summary.c b/ui-summary.c
index b0af073..a5c7078 100644
--- a/ui-summary.c
+++ b/ui-summary.c
@@ -13,7 +13,6 @@
 #include "ui-refs.h"
 #include "ui-blob.h"
 #include "ui-shared.h"
-#include <libgen.h>
 
 static int urls;
 
-- 
2.5.0.466.g9af26fa

[PATCH 4/8] configfile.c: don't include system headers directly

[john]

git-compat-util.h may define various values that affect the
interpretation of system headers.  In most places we include cgit.h
first, which pulls in git-compat-util.h, but this file does not depend
on anything else in CGit, so use git-compat-util.h directly.

Signed-off-by: John Keeping <john at keeping.me.uk>
---
 configfile.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/configfile.c b/configfile.c
index 833f158..5b0d880 100644
--- a/configfile.c
+++ b/configfile.c
@@ -6,8 +6,7 @@
  *   (see COPYING for full license text)
  */
 
-#include <ctype.h>
-#include <stdio.h>
+#include <git-compat-util.h>
 #include "configfile.h"
 
 static int next_char(FILE *f)
-- 
2.5.0.466.g9af26fa

[PATCH 5/8] cache.c: fix header order

[john]

git-compat-util.h may define values that affect how system headers are
interpreted, so move sys/sendfile.h after cgit.h (which includes
git-compat-util.h).

Signed-off-by: John Keeping <john at keeping.me.uk>
---
 cache.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/cache.c b/cache.c
index cd99812..57c8918 100644
--- a/cache.c
+++ b/cache.c
@@ -13,12 +13,12 @@
  *
  */
 
-#ifdef HAVE_LINUX_SENDFILE
-#include <sys/sendfile.h>
-#endif
 #include "cgit.h"
 #include "cache.h"
 #include "html.h"
+#ifdef HAVE_LINUX_SENDFILE
+#include <sys/sendfile.h>
+#endif
 
 #define CACHE_BUFSIZE (1024 * 4)
 
-- 
2.5.0.466.g9af26fa

[PATCH 6/8] cgit.h: move stdbool.h from ui-shared.h

[john]

Follow the Git policy of including system headers in only one place.

Signed-off-by: John Keeping <john at keeping.me.uk>
---
 cgit.h      | 2 ++
 ui-shared.h | 2 --
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/cgit.h b/cgit.h
index 3120562..508179a 100644
--- a/cgit.h
+++ b/cgit.h
@@ -3,6 +3,8 @@
 
 
 #include <git-compat-util.h>
+#include <stdbool.h>
+
 #include <cache.h>
 #include <grep.h>
 #include <object.h>
diff --git a/ui-shared.h b/ui-shared.h
index 788b1bc..d8a3551 100644
--- a/ui-shared.h
+++ b/ui-shared.h
@@ -1,8 +1,6 @@
 #ifndef UI_SHARED_H
 #define UI_SHARED_H
 
-#include <stdbool.h>
-
 extern const char *cgit_httpscheme();
 extern const char *cgit_hosturl();
 extern const char *cgit_rooturl();
-- 
2.5.0.466.g9af26fa

[PATCH 7/8] ui-tree: use "sane" isgraph()

[john]

Git's git-compat-util.h defines a "sane ctype" that does not use locale
information and works with signed chars, but it does not include
isgraph() so we have included ctype.h ourselves.

However, this means we have to include a system header before
git-compat-util.h which may lead to the system defining some macros
(e.g. _FILE_OFFSET_BITS on Solaris) before git-compat-util.h redefines
them with a different value.  We cannot include ctype.h after
git-compat-util.h because we have defined many of its functions as
macros which causes a stream of compilation errors.

Defining our own "sane" isgraph() using Git's sane isprint() and
isspace() avoids all of these problems.

Signed-off-by: John Keeping <john at keeping.me.uk>
---
 cgit.h    | 3 +++
 ui-tree.c | 1 -
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/cgit.h b/cgit.h
index 508179a..f327627 100644
--- a/cgit.h
+++ b/cgit.h
@@ -25,6 +25,9 @@
 #include <notes.h>
 #include <graph.h>
 
+/* Add isgraph(x) to Git's sane ctype support (see git-compat-util.h) */
+#undef isgraph
+#define isgraph(x) (isprint((x)) && !isspace((x)))
 
 /*
  * Dateformats used on misc. pages
diff --git a/ui-tree.c b/ui-tree.c
index c8d24f6..2dbe89e 100644
--- a/ui-tree.c
+++ b/ui-tree.c
@@ -6,7 +6,6 @@
  *   (see COPYING for full license text)
  */
 
-#include <ctype.h>
 #include "cgit.h"
 #include "ui-tree.h"
 #include "html.h"
-- 
2.5.0.466.g9af26fa

[PATCH 8/8] filter: don't use dlsym unnecessarily

[john]

We only need to hook write() if Lua filter's are in use.  If support has
been disabled, remove the dependency on dlsym().

Signed-off-by: John Keeping <john at keeping.me.uk>
---
 filter.c | 78 ++++++++++++++++++++++++++++++++++------------------------------
 1 file changed, 42 insertions(+), 36 deletions(-)

diff --git a/filter.c b/filter.c
index c7037a3..949c931 100644
--- a/filter.c
+++ b/filter.c
@@ -8,17 +8,13 @@
 
 #include "cgit.h"
 #include "html.h"
-#include <dlfcn.h>
 #ifndef NO_LUA
+#include <dlfcn.h>
 #include <lua.h>
 #include <lualib.h>
 #include <lauxlib.h>
 #endif
 
-static ssize_t (*libc_write)(int fd, const void *buf, size_t count);
-static ssize_t (*filter_write)(struct cgit_filter *base, const void *buf, size_t count) = NULL;
-static struct cgit_filter *current_write_filter = NULL;
-
 static inline void reap_filter(struct cgit_filter *filter)
 {
 	if (filter && filter->cleanup)
@@ -43,37 +39,6 @@ void cgit_cleanup_filters(void)
 	}
 }
 
-void cgit_init_filters(void)
-{
-	libc_write = dlsym(RTLD_NEXT, "write");
-	if (!libc_write)
-		die("Could not locate libc's write function");
-}
-
-ssize_t write(int fd, const void *buf, size_t count)
-{
-	if (fd != STDOUT_FILENO || !filter_write)
-		return libc_write(fd, buf, count);
-	return filter_write(current_write_filter, buf, count);
-}
-
-static inline void hook_write(struct cgit_filter *filter, ssize_t (*new_write)(struct cgit_filter *base, const void *buf, size_t count))
-{
-	/* We want to avoid buggy nested patterns. */
-	assert(filter_write == NULL);
-	assert(current_write_filter == NULL);
-	current_write_filter = filter;
-	filter_write = new_write;
-}
-
-static inline void unhook_write(void)
-{
-	assert(filter_write != NULL);
-	assert(current_write_filter != NULL);
-	filter_write = NULL;
-	current_write_filter = NULL;
-}
-
 static int open_exec_filter(struct cgit_filter *base, va_list ap)
 {
 	struct cgit_exec_filter *filter = (struct cgit_exec_filter *)base;
@@ -170,7 +135,48 @@ void cgit_exec_filter_init(struct cgit_exec_filter *filter, char *cmd, char **ar
 	filter->base.argument_count = 0;
 }
 
+#ifdef NO_LUA
+void cgit_init_filters(void)
+{
+}
+#endif
+
 #ifndef NO_LUA
+static ssize_t (*libc_write)(int fd, const void *buf, size_t count);
+static ssize_t (*filter_write)(struct cgit_filter *base, const void *buf, size_t count) = NULL;
+static struct cgit_filter *current_write_filter = NULL;
+
+void cgit_init_filters(void)
+{
+	libc_write = dlsym(RTLD_NEXT, "write");
+	if (!libc_write)
+		die("Could not locate libc's write function");
+}
+
+ssize_t write(int fd, const void *buf, size_t count)
+{
+	if (fd != STDOUT_FILENO || !filter_write)
+		return libc_write(fd, buf, count);
+	return filter_write(current_write_filter, buf, count);
+}
+
+static inline void hook_write(struct cgit_filter *filter, ssize_t (*new_write)(struct cgit_filter *base, const void *buf, size_t count))
+{
+	/* We want to avoid buggy nested patterns. */
+	assert(filter_write == NULL);
+	assert(current_write_filter == NULL);
+	current_write_filter = filter;
+	filter_write = new_write;
+}
+
+static inline void unhook_write(void)
+{
+	assert(filter_write != NULL);
+	assert(current_write_filter != NULL);
+	filter_write = NULL;
+	current_write_filter = NULL;
+}
+
 struct lua_filter {
 	struct cgit_filter base;
 	char *script_file;
-- 
2.5.0.466.g9af26fa

[PATCH 3/8] Remove redundant includes

[Jason]

Do you have a clever tool of computing these automatically, or is this work
by hand?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20150813/df79ebc8/attachment.html>

[PATCH 0/8] Portability fixes

[Jason]

Merged, thanks.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20150813/3c41f43a/attachment.html>

[PATCH 3/8] Remove redundant includes

[john]

On Thu, Aug 13, 2015 at 03:36:48PM +0200, Jason A. Donenfeld wrote:
> Do you have a clever tool of computing these automatically, or is this work
> by hand?

By hand.  It boils down to checking against:

	git grep '#include <'

and making sure that the relevant headers are indeed in
git-compat-util.h.

[PATCH 3/8] Remove redundant includes

[mathstuf]

On Thu, 13 Aug, 2015 at 13:36:48 GMT, Jason A. Donenfeld wrote:
> Do you have a clever tool of computing these automatically, or is this work
> by hand?

While it appears it was "by hand", there is a tool for this :) :

    https://github.com/include-what-you-use/include-what-you-use

--Ben