From bc2f5a6d53fa8ec1dee3335dd6de4650f834231b Mon Sep 17 00:00:00 2001 From: Lukas Fleischer Date: Mon, 4 Mar 2013 13:25:37 +0100 Subject: [PATCH 01/16] cgit_print_commit(): Free tmp variable Fixes following memory leak seen with "PATH_INFO=/cgit/commit/": ==16894== 7 bytes in 1 blocks are definitely lost in loss record 4 of 92 ==16894== at 0x4C2C04B: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==16894== by 0x56F2DF1: strdup (in /usr/lib/libc-2.17.so) ==16894== by 0x46CAC8: xstrdup (wrapper.c:35) ==16894== by 0x40CD6F: cgit_print_commit (ui-commit.c:70) ==16894== by 0x407B06: commit_fn (cmd.c:54) ==16894== by 0x405E16: process_request (cgit.c:574) ==16894== by 0x4074C8: cache_process (cache.c:322) ==16894== by 0x406C4F: main (cgit.c:872) Signed-off-by: Lukas Fleischer --- ui-commit.c | 1 + 1 file changed, 1 insertion(+) diff --git a/ui-commit.c b/ui-commit.c index 74f37c8..0783285 100644 --- a/ui-commit.c +++ b/ui-commit.c @@ -74,6 +74,7 @@ void cgit_print_commit(char *hex, const char *prefix) html(" /"); cgit_tree_link(prefix, NULL, NULL, ctx.qry.head, tmp, prefix); } + free(tmp); html("\n"); for (p = commit->parents; p; p = p->next) { parent = lookup_commit_reference(p->item->object.sha1); -- 2.50.1 From 59fe348deaa270434f05afc56ca8d13618af9ca9 Mon Sep 17 00:00:00 2001 From: Lukas Fleischer Date: Mon, 4 Mar 2013 13:25:38 +0100 Subject: [PATCH 02/16] cgit_print_snapshot_links(): Free prefix variable Fixes following memory leak seen with "PATH_INFO=/cgit/commit/": ==16894== 12 bytes in 1 blocks are definitely lost in loss record 9 of 92 ==16894== at 0x4C2C04B: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==16894== by 0x56F2DF1: strdup (in /usr/lib/libc-2.17.so) ==16894== by 0x46CAC8: xstrdup (wrapper.c:35) ==16894== by 0x414E34: cgit_print_snapshot_links (ui-shared.c:926) ==16894== by 0x40CFA1: cgit_print_commit (ui-commit.c:102) ==16894== by 0x407B06: commit_fn (cmd.c:54) ==16894== by 0x405E16: process_request (cgit.c:574) ==16894== by 0x4074C8: cache_process (cache.c:322) ==16894== by 0x406C4F: main (cgit.c:872) Signed-off-by: Lukas Fleischer --- ui-shared.c | 1 + 1 file changed, 1 insertion(+) diff --git a/ui-shared.c b/ui-shared.c index 77a302d..d3e6488 100644 --- a/ui-shared.c +++ b/ui-shared.c @@ -931,4 +931,5 @@ void cgit_print_snapshot_links(const char *repo, const char *head, cgit_snapshot_link(filename, NULL, NULL, NULL, NULL, filename); html("
"); } + free(prefix); } -- 2.50.1 From 6d8a789d61f3a682bc040f1f7f44050b1f723546 Mon Sep 17 00:00:00 2001 From: John Keeping Date: Wed, 6 Mar 2013 20:51:54 +0000 Subject: [PATCH 03/16] ui-shared: fix return type of cgit_self_link cgit_self_link() is a void function but implements each case it handles by doing "return " which is not valid C; section 6.8.6.4 of C11 says: A return statement with an expression shall not appear in a function whose return type is void. Fix this by removing the return keywords and converting the final code block into an "else". Signed-off-by: John Keeping --- ui-shared.c | 83 +++++++++++++++++++++++++++-------------------------- 1 file changed, 42 insertions(+), 41 deletions(-) diff --git a/ui-shared.c b/ui-shared.c index af5310b..80f4aee 100644 --- a/ui-shared.c +++ b/ui-shared.c @@ -437,58 +437,59 @@ void cgit_self_link(char *name, const char *title, const char *class, struct cgit_context *ctx) { if (!strcmp(ctx->qry.page, "repolist")) - return cgit_index_link(name, title, class, ctx->qry.search, ctx->qry.sort, - ctx->qry.ofs); + cgit_index_link(name, title, class, ctx->qry.search, ctx->qry.sort, + ctx->qry.ofs); else if (!strcmp(ctx->qry.page, "summary")) - return cgit_summary_link(name, title, class, ctx->qry.head); + cgit_summary_link(name, title, class, ctx->qry.head); else if (!strcmp(ctx->qry.page, "tag")) - return 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.head, + ctx->qry.has_sha1 ? ctx->qry.sha1 : NULL); else if (!strcmp(ctx->qry.page, "tree")) - return cgit_tree_link(name, title, class, ctx->qry.head, - ctx->qry.has_sha1 ? ctx->qry.sha1 : NULL, - ctx->qry.path); + cgit_tree_link(name, title, class, ctx->qry.head, + ctx->qry.has_sha1 ? ctx->qry.sha1 : NULL, + ctx->qry.path); else if (!strcmp(ctx->qry.page, "plain")) - return cgit_plain_link(name, title, class, ctx->qry.head, - ctx->qry.has_sha1 ? ctx->qry.sha1 : NULL, - ctx->qry.path); + cgit_plain_link(name, title, class, ctx->qry.head, + ctx->qry.has_sha1 ? ctx->qry.sha1 : NULL, + ctx->qry.path); else if (!strcmp(ctx->qry.page, "log")) - return cgit_log_link(name, title, class, ctx->qry.head, - ctx->qry.has_sha1 ? ctx->qry.sha1 : NULL, - ctx->qry.path, ctx->qry.ofs, - ctx->qry.grep, ctx->qry.search, - ctx->qry.showmsg); + cgit_log_link(name, title, class, ctx->qry.head, + ctx->qry.has_sha1 ? ctx->qry.sha1 : NULL, + ctx->qry.path, ctx->qry.ofs, + ctx->qry.grep, ctx->qry.search, + ctx->qry.showmsg); else if (!strcmp(ctx->qry.page, "commit")) - return cgit_commit_link(name, title, class, ctx->qry.head, - ctx->qry.has_sha1 ? ctx->qry.sha1 : NULL, - ctx->qry.path, 0); + cgit_commit_link(name, title, class, ctx->qry.head, + ctx->qry.has_sha1 ? ctx->qry.sha1 : NULL, + ctx->qry.path, 0); else if (!strcmp(ctx->qry.page, "patch")) - return cgit_patch_link(name, title, class, ctx->qry.head, - ctx->qry.has_sha1 ? ctx->qry.sha1 : NULL, - ctx->qry.path); + cgit_patch_link(name, title, class, ctx->qry.head, + ctx->qry.has_sha1 ? ctx->qry.sha1 : NULL, + ctx->qry.path); else if (!strcmp(ctx->qry.page, "refs")) - return cgit_refs_link(name, title, class, ctx->qry.head, - ctx->qry.has_sha1 ? ctx->qry.sha1 : NULL, - ctx->qry.path); + cgit_refs_link(name, title, class, ctx->qry.head, + ctx->qry.has_sha1 ? ctx->qry.sha1 : NULL, + ctx->qry.path); else if (!strcmp(ctx->qry.page, "snapshot")) - return cgit_snapshot_link(name, title, class, ctx->qry.head, - ctx->qry.has_sha1 ? ctx->qry.sha1 : NULL, - ctx->qry.path); + cgit_snapshot_link(name, title, class, ctx->qry.head, + ctx->qry.has_sha1 ? ctx->qry.sha1 : NULL, + ctx->qry.path); else if (!strcmp(ctx->qry.page, "diff")) - return cgit_diff_link(name, title, class, ctx->qry.head, - ctx->qry.sha1, ctx->qry.sha2, - ctx->qry.path, 0); + cgit_diff_link(name, title, class, ctx->qry.head, + ctx->qry.sha1, ctx->qry.sha2, + ctx->qry.path, 0); else if (!strcmp(ctx->qry.page, "stats")) - return cgit_stats_link(name, title, class, ctx->qry.head, - ctx->qry.path); - - /* Don't known how to make link for this page */ - repolink(title, class, ctx->qry.page, ctx->qry.head, ctx->qry.path); - html(">"); - html_txt(name); - html(""); + cgit_stats_link(name, title, class, ctx->qry.head, + ctx->qry.path); + else { + /* Don't known how to make link for this page */ + repolink(title, class, ctx->qry.page, ctx->qry.head, ctx->qry.path); + html(">"); + html_txt(name); + html(""); + } } void cgit_object_link(struct object *obj) -- 2.50.1 From e1e0e038fd0fee3fe10524d7466deab03e78deb5 Mon Sep 17 00:00:00 2001 From: John Keeping Date: Tue, 19 Mar 2013 20:00:29 +0000 Subject: [PATCH 04/16] tests: check that Git version are in sync This ensures that the Git version pointed at by the submodule is the same as the one that will be fetched using "make get-git". Suggested-by: Ferry Huberts Signed-off-by: John Keeping --- tests/t0001-validate-git-versions.sh | 36 ++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100755 tests/t0001-validate-git-versions.sh diff --git a/tests/t0001-validate-git-versions.sh b/tests/t0001-validate-git-versions.sh new file mode 100755 index 0000000..3378358 --- /dev/null +++ b/tests/t0001-validate-git-versions.sh @@ -0,0 +1,36 @@ +#!/bin/sh + +. ./setup.sh + +prepare_tests 'Check Git version is correct' + +run_test 'extract Git version from Makefile' ' + sed -n -e "/^GIT_VER[ ]*=/ { + s/^GIT_VER[ ]*=[ ]*// + p + }" ../Makefile >trash/makefile_version +' + +run_test 'test Git version matches Makefile' ' + ( cat ../git/GIT-VERSION-FILE || echo "No GIT-VERSION-FILE" ) | + sed -e "s/GIT_VERSION[ ]*=[ ]*//" >trash/git_version && + diff -u trash/git_version trash/makefile_version +' + +run_test 'test submodule version matches Makefile' ' + if ! test -e ../git/.git + then + echo "git/ is not a Git repository" >&2 + else + ( + cd .. && + sm_sha1=$(git ls-files --stage -- git | + sed -e "s/^[0-9]* \\([0-9a-f]*\\) [0-9] .*$/\\1/") && + cd git && + git describe --match "v[0-9]*" $sm_sha1 + ) | sed -e "s/^v//" >trash/sm_version && + diff -u trash/sm_version trash/makefile_version + fi +' + +tests_done -- 2.50.1 From 5f323c1ff45c10d8f8b0a673d2fe7e98272f5d78 Mon Sep 17 00:00:00 2001 From: John Keeping Date: Wed, 6 Mar 2013 21:22:06 +0000 Subject: [PATCH 05/16] Makefile: re-use Git's Makefile where possible Git does quite a lot of platform-specific detection in its Makefile, which can result in it defining preprocessor variables that are used in its header files. If CGit does not define the same variables it can result in different sizes of some structures in different places in the same application. For example, on Solaris Git uses it's "compat" regex library which has a different sized regex_t structure than that available in the platform regex.h. This has a knock-on effect on the size of "struct rev_info" and leads to hard to diagnose runtime issues. In order to avoid all of this, introduce a "cgit.mk" file that includes Git's Makefile and make all of the existing logic apply to CGit's objects as well. This is slightly complicated because Git's Makefile must run in Git's directory, so all references to CGit files need to be prefixed with "../". In addition, OBJECTS is a simply expanded variable in Git's Makefile so we cannot just add our objects to it. Instead we must copy the two applicable rules into "cgit.mk". This has the advantage that we can split CGit-specific CFLAGS from Git's CFLAGS and hence avoid rebuilding all of Git whenever a CGit-specific value changes. Signed-off-by: John Keeping Acked-by: Jamie Couture --- .gitignore | 1 + Makefile | 124 +++-------------------------------------------------- cgit.mk | 74 ++++++++++++++++++++++++++++++++ 3 files changed, 80 insertions(+), 119 deletions(-) create mode 100644 cgit.mk diff --git a/.gitignore b/.gitignore index 487728b..8011b39 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,7 @@ # Files I don't care to see in git-status/commit cgit cgit.conf +CGIT-CFLAGS VERSION cgitrc.5 cgitrc.5.fo diff --git a/Makefile b/Makefile index 1127961..8c00190 100644 --- a/Makefile +++ b/Makefile @@ -23,13 +23,6 @@ DOC_MAN5 = $(patsubst %.txt,%,$(MAN5_TXT)) DOC_HTML = $(patsubst %.txt,%.html,$(MAN_TXT)) DOC_PDF = $(patsubst %.txt,%.pdf,$(MAN_TXT)) -# Define NO_STRCASESTR if you don't have strcasestr. -# -# Define NO_OPENSSL to disable linking with OpenSSL and use bundled SHA1 -# implementation (slower). -# -# Define NEEDS_LIBICONV if linking with libc is not enough (eg. Darwin). -# # Define NO_C99_FORMAT if your formatted IO functions (printf/scanf et.al.) # do not support the 'size specifiers' introduced by C99, namely ll, hh, # j, z, t. (representing long long int, char, intmax_t, size_t, ptrdiff_t). @@ -38,23 +31,13 @@ DOC_PDF = $(patsubst %.txt,%.pdf,$(MAN_TXT)) #-include config.mak -# -# Platform specific tweaks -# - -VERSION: force-version - @./gen-version.sh "$(CGIT_VERSION)" --include VERSION - -uname_S := $(shell sh -c 'uname -s 2>/dev/null || echo not') -uname_O := $(shell sh -c 'uname -o 2>/dev/null || echo not') -uname_R := $(shell sh -c 'uname -r 2>/dev/null || echo not') - # # Let the user override the above settings. # -include cgit.conf +export CGIT_SCRIPT_NAME CGIT_SCRIPT_PATH CGIT_DATA_PATH CGIT_CONFIG CACHE_ROOT + # # Define a way to invoke make in subdirs quietly, shamelessly ripped # from git.git @@ -69,8 +52,6 @@ NO_SUBDIR = : endif ifndef V - QUIET_CC = @echo ' ' CC $@; - QUIET_LINK = @echo ' ' LINK $@; QUIET_SUBDIR0 = +@subdir= QUIET_SUBDIR1 = ;$(NO_SUBDIR) echo ' ' SUBDIR $$subdir; \ $(MAKE) $(PRINT_DIR) -C $$subdir @@ -78,107 +59,12 @@ ifndef V export V endif -LDFLAGS ?= -CFLAGS ?= -g -Wall -CFLAGS += -Igit -CFLAGS += -DSHA1_HEADER='$(SHA1_HEADER)' -CFLAGS += -DCGIT_VERSION='"$(CGIT_VERSION)"' -CFLAGS += -DCGIT_CONFIG='"$(CGIT_CONFIG)"' -CFLAGS += -DCGIT_SCRIPT_NAME='"$(CGIT_SCRIPT_NAME)"' -CFLAGS += -DCGIT_CACHE_ROOT='"$(CACHE_ROOT)"' - -ifeq ($(uname_O),Cygwin) - NO_STRCASESTR = YesPlease - NEEDS_LIBICONV = YesPlease -endif - -ifeq ($(uname_S),$(filter $(uname_S),FreeBSD OpenBSD)) - # Apparantly libiconv is installed in /usr/local on BSD - LDFLAGS += -L/usr/local/lib - CFLAGS += -I/usr/local/include - NEEDS_LIBICONV = yes -endif - -GIT_OPTIONS = prefix=/usr NO_GETTEXT=1 -OBJECTS = - -ifdef NO_ICONV - CFLAGS += -DNO_ICONV -endif -ifdef NO_STRCASESTR - CFLAGS += -DNO_STRCASESTR -endif -ifdef NO_C99_FORMAT - CFLAGS += -DNO_C99_FORMAT -endif -ifdef NO_OPENSSL - CFLAGS += -DNO_OPENSSL - GIT_OPTIONS += NO_OPENSSL=1 -else - LDLIBS += -lcrypto -endif - -ifdef NEEDS_LIBICONV - LDLIBS += -liconv -endif - -LDLIBS += git/libgit.a git/xdiff/lib.a -lz -lpthread - -OBJECTS += cgit.o -OBJECTS += cache.o -OBJECTS += cmd.o -OBJECTS += configfile.o -OBJECTS += html.o -OBJECTS += parsing.o -OBJECTS += scan-tree.o -OBJECTS += shared.o -OBJECTS += ui-atom.o -OBJECTS += ui-blob.o -OBJECTS += ui-clone.o -OBJECTS += ui-commit.o -OBJECTS += ui-diff.o -OBJECTS += ui-log.o -OBJECTS += ui-patch.o -OBJECTS += ui-plain.o -OBJECTS += ui-refs.o -OBJECTS += ui-repolist.o -OBJECTS += ui-shared.o -OBJECTS += ui-snapshot.o -OBJECTS += ui-ssdiff.o -OBJECTS += ui-stats.o -OBJECTS += ui-summary.o -OBJECTS += ui-tag.o -OBJECTS += ui-tree.o -OBJECTS += vector.o - -dep_files := $(foreach f,$(OBJECTS),$(dir $f).deps/$(notdir $f).d) -dep_dirs := $(addsuffix .deps,$(sort $(dir $OBJECTS))) - -$(dep_dirs): - @mkdir -p $@ - -missing_dep_dirs := $(filter-out $(wildcard $(dep_dirs)),$(dep_dirs)) -dep_file = $(dir $@).deps/$(notdir $@).d -dep_args = -MF $(dep_file) -MMD -MP - .SUFFIXES: -$(OBJECTS): %.o: %.c $(missing_dep_dirs) - $(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(CFLAGS) $< - -dep_files_present := $(wildcard $(dep_files)) -ifneq ($(dep_files_present),) -include $(dep_files_present) -endif - all:: cgit -cgit: VERSION $(OBJECTS) libgit - $(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $(OBJECTS) $(LDLIBS) - -libgit: - $(QUIET_SUBDIR0)git $(QUIET_SUBDIR1) NO_CURL=1 $(GIT_OPTIONS) libgit.a - $(QUIET_SUBDIR0)git $(QUIET_SUBDIR1) NO_CURL=1 $(GIT_OPTIONS) xdiff/lib.a +cgit: + $(QUIET_SUBDIR0)git $(QUIET_SUBDIR1) -f ../cgit.mk ../cgit NO_CURL=1 test: all $(QUIET_SUBDIR0)tests $(QUIET_SUBDIR1) all @@ -259,7 +145,7 @@ get-git: tags: $(QUIET_TAGS)find . -name '*.[ch]' | xargs ctags -.PHONY: all cgit get-git libgit force-version +.PHONY: all cgit get-git .PHONY: clean clean-doc cleanall .PHONY: doc doc-html doc-man doc-pdf .PHONY: install install-doc install-html install-man install-pdf diff --git a/cgit.mk b/cgit.mk new file mode 100644 index 0000000..4869c55 --- /dev/null +++ b/cgit.mk @@ -0,0 +1,74 @@ +# This Makefile is run in the "git" directory in order to re-use Git's +# build variables and operating system detection. Hence all files in +# CGit's directory must be prefixed with "../". +include Makefile + +CGIT_PREFIX = ../ + +# The CGIT_* variables are inherited when this file is called from the +# main Makefile - they are defined there. + +$(CGIT_PREFIX)VERSION: force-version + @cd $(CGIT_PREFIX) && ./gen-version.sh "$(CGIT_VERSION)" +-include $(CGIT_PREFIX)VERSION +.PHONY: force-version + +# CGIT_CFLAGS is a separate variable so that we can track it separately +# and avoid rebuilding all of Git when these variables change. +CGIT_CFLAGS += -DCGIT_VERSION='"$(CGIT_VERSION)"' +CGIT_CFLAGS += -DCGIT_CONFIG='"$(CGIT_CONFIG)"' +CGIT_CFLAGS += -DCGIT_SCRIPT_NAME='"$(CGIT_SCRIPT_NAME)"' +CGIT_CFLAGS += -DCGIT_CACHE_ROOT='"$(CACHE_ROOT)"' + +ifdef NO_C99_FORMAT + CFLAGS += -DNO_C99_FORMAT +endif + +CGIT_OBJ_NAMES += cgit.o +CGIT_OBJ_NAMES += cache.o +CGIT_OBJ_NAMES += cmd.o +CGIT_OBJ_NAMES += configfile.o +CGIT_OBJ_NAMES += html.o +CGIT_OBJ_NAMES += parsing.o +CGIT_OBJ_NAMES += scan-tree.o +CGIT_OBJ_NAMES += shared.o +CGIT_OBJ_NAMES += ui-atom.o +CGIT_OBJ_NAMES += ui-blob.o +CGIT_OBJ_NAMES += ui-clone.o +CGIT_OBJ_NAMES += ui-commit.o +CGIT_OBJ_NAMES += ui-diff.o +CGIT_OBJ_NAMES += ui-log.o +CGIT_OBJ_NAMES += ui-patch.o +CGIT_OBJ_NAMES += ui-plain.o +CGIT_OBJ_NAMES += ui-refs.o +CGIT_OBJ_NAMES += ui-repolist.o +CGIT_OBJ_NAMES += ui-shared.o +CGIT_OBJ_NAMES += ui-snapshot.o +CGIT_OBJ_NAMES += ui-ssdiff.o +CGIT_OBJ_NAMES += ui-stats.o +CGIT_OBJ_NAMES += ui-summary.o +CGIT_OBJ_NAMES += ui-tag.o +CGIT_OBJ_NAMES += ui-tree.o +CGIT_OBJ_NAMES += vector.o + +CGIT_OBJS := $(addprefix $(CGIT_PREFIX),$(CGIT_OBJ_NAMES)) + +ifeq ($(wildcard $(CGIT_PREFIX).depend),) +missing_dep_dirs += $(CGIT_PREFIX).depend +endif + +$(CGIT_PREFIX).depend: + @mkdir -p $@ + +$(CGIT_PREFIX)CGIT-CFLAGS: FORCE + @FLAGS='$(subst ','\'',$(CGIT_CFLAGS))'; \ + if test x"$$FLAGS" != x"`cat ../CGIT-CFLAGS 2>/dev/null`" ; then \ + echo 1>&2 " * new CGit build flags"; \ + echo "$$FLAGS" >$(CGIT_PREFIX)CGIT-CFLAGS; \ + fi + +$(CGIT_OBJS): %.o: %.c GIT-CFLAGS $(CGIT_PREFIX)CGIT-CFLAGS $(missing_dep_dirs) + $(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $(CGIT_CFLAGS) $< + +$(CGIT_PREFIX)cgit: $(CGIT_OBJS) GIT-LDFLAGS $(GITLIBS) + $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS) -- 2.50.1 From 1a6feaf5fafdaec4a1adb1c0d54b95106122ede5 Mon Sep 17 00:00:00 2001 From: John Keeping Date: Wed, 6 Mar 2013 21:22:07 +0000 Subject: [PATCH 06/16] ui-patch: use cgit_version not CGIT_VERSION We already have a global cgit_version which is set from the #define'd CGIT_VERSION in cgit.c. Change ui-patch.c to use this so that we only need to rebuild cgit.o when the version changes. Signed-off-by: John Keeping --- ui-patch.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui-patch.c b/ui-patch.c index 79bc509..12abe10 100644 --- a/ui-patch.c +++ b/ui-patch.c @@ -131,6 +131,6 @@ void cgit_print_patch(char *hex, const char *prefix) htmlf("(limited to '%s')\n\n", prefix); cgit_diff_tree(old_sha1, sha1, filepair_cb, prefix, 0); html("--\n"); - htmlf("cgit %s\n", CGIT_VERSION); + htmlf("cgit %s\n", cgit_version); cgit_free_commitinfo(info); } -- 2.50.1 From d6768a67093166810621d2521f10fd016bd75721 Mon Sep 17 00:00:00 2001 From: John Keeping Date: Wed, 6 Mar 2013 21:22:08 +0000 Subject: [PATCH 07/16] cgit.mk: don't rebuild everything if CGIT_VERSION changes If CGIT_VERSION is in CGIT_CFLAGS then a change in version (for example because you have committed your changes) causes all of the CGit objects to be rebuilt. Avoid this by using EXTRA_CPPFLAGS to add the version for only those files that are affected and make them depend on VERSION. Signed-off-by: John Keeping --- cgit.mk | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/cgit.mk b/cgit.mk index 4869c55..c8ecd3a 100644 --- a/cgit.mk +++ b/cgit.mk @@ -15,7 +15,6 @@ $(CGIT_PREFIX)VERSION: force-version # CGIT_CFLAGS is a separate variable so that we can track it separately # and avoid rebuilding all of Git when these variables change. -CGIT_CFLAGS += -DCGIT_VERSION='"$(CGIT_VERSION)"' CGIT_CFLAGS += -DCGIT_CONFIG='"$(CGIT_CONFIG)"' CGIT_CFLAGS += -DCGIT_SCRIPT_NAME='"$(CGIT_SCRIPT_NAME)"' CGIT_CFLAGS += -DCGIT_CACHE_ROOT='"$(CACHE_ROOT)"' @@ -53,6 +52,14 @@ CGIT_OBJ_NAMES += vector.o 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): $(CGIT_PREFIX)VERSION +$(CGIT_VERSION_OBJS): EXTRA_CPPFLAGS = \ + -DCGIT_VERSION='"$(CGIT_VERSION)"' + + ifeq ($(wildcard $(CGIT_PREFIX).depend),) missing_dep_dirs += $(CGIT_PREFIX).depend endif -- 2.50.1 From 7669f7f73082ce9eb1aef28495773492cc5bec90 Mon Sep 17 00:00:00 2001 From: John Keeping Date: Wed, 6 Mar 2013 21:22:09 +0000 Subject: [PATCH 08/16] cgit.mk: Use SHELL_PATH_SQ to run gen-version.sh On some platforms (notably Solaris) /bin/sh doesn't support enough of POSIX for gen-version.sh to run. Git's Makefile provides SHELL_PATH_SQ to address this issue so we just have to use it. Signed-off-by: John Keeping --- cgit.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cgit.mk b/cgit.mk index c8ecd3a..e1aed63 100644 --- a/cgit.mk +++ b/cgit.mk @@ -9,7 +9,7 @@ CGIT_PREFIX = ../ # main Makefile - they are defined there. $(CGIT_PREFIX)VERSION: force-version - @cd $(CGIT_PREFIX) && ./gen-version.sh "$(CGIT_VERSION)" + @cd $(CGIT_PREFIX) && '$(SHELL_PATH_SQ)' ./gen-version.sh "$(CGIT_VERSION)" -include $(CGIT_PREFIX)VERSION .PHONY: force-version -- 2.50.1 From 40e1d9b6177558bf4069c09ca6d8e3a682baa988 Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Wed, 20 Mar 2013 20:43:13 +0100 Subject: [PATCH 09/16] ui-shared: squelch compiler warning. Since tail is initialized to 0, we will never get a warning on the last if statement, but recent gcc complains anyway. So, we initialize len as well. Future gcc versions should be able to optimize this out anyway. --- ui-shared.c | 1 + 1 file changed, 1 insertion(+) diff --git a/ui-shared.c b/ui-shared.c index 968933f..d4fb3d9 100644 --- a/ui-shared.c +++ b/ui-shared.c @@ -523,6 +523,7 @@ void cgit_submodule_link(const char *class, char *path, const char *rev) char tail, *dir; size_t len; + len = 0; tail = 0; list = &ctx.repo->submodules; item = lookup_path(list, path); -- 2.50.1 From 6d7e3596ebb387265d8cfdc5b312e0ea76da8c8a Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Wed, 20 Mar 2013 20:44:20 +0100 Subject: [PATCH 10/16] html: check return value of write This squelches a gcc warning. It's also correct that we check to see if there are any partial or failed writes. For now, we just print a warning to stderr. In the future, perhaps it will prove wise to exit(1) on partial writes. --- html.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/html.c b/html.c index b5c6903..d60a41f 100644 --- a/html.c +++ b/html.c @@ -63,12 +63,13 @@ char *fmt(const char *format, ...) void html_raw(const char *data, size_t size) { - write(htmlfd, data, size); + if (write(htmlfd, data, size) != size) + fprintf(stderr, "[html.c] html output truncated.\n"); } void html(const char *txt) { - write(htmlfd, txt, strlen(txt)); + html_raw(txt, strlen(txt)); } void htmlf(const char *format, ...) -- 2.50.1 From ef8a97d9c6983e4fc3710bdbe771edd4e3550dba Mon Sep 17 00:00:00 2001 From: Lukas Fleischer Date: Tue, 5 Mar 2013 15:42:14 +0100 Subject: [PATCH 11/16] Fix colspan values This fixes a couple of minor oversights in previous commits and adjusts all cells using colspan to use the correct width. Signed-off-by: Lukas Fleischer --- ui-log.c | 14 +++++++------- ui-refs.c | 4 ++-- ui-summary.c | 24 +++++++++++++++++++----- 3 files changed, 28 insertions(+), 14 deletions(-) diff --git a/ui-log.c b/ui-log.c index 857c05c..954d3e1 100644 --- a/ui-log.c +++ b/ui-log.c @@ -98,14 +98,14 @@ next: static void print_commit(struct commit *commit, struct rev_info *revs) { struct commitinfo *info; - int cols = revs->graph ? 3 : 2; + int columns = revs->graph ? 4 : 3; struct strbuf graphbuf = STRBUF_INIT; struct strbuf msgbuf = STRBUF_INIT; if (ctx.repo->enable_log_filecount) - cols++; + columns++; if (ctx.repo->enable_log_linecount) - cols++; + columns++; if (revs->graph) { /* Advance graph until current commit */ @@ -113,7 +113,7 @@ static void print_commit(struct commit *commit, struct rev_info *revs) /* Print graph segment in otherwise empty table row */ html(""); html(graphbuf.buf); - htmlf("\n", cols); + htmlf("\n", columns); strbuf_setlen(&graphbuf, 0); } /* Current commit's graph segment is now ready in graphbuf */ @@ -232,7 +232,7 @@ static void print_commit(struct commit *commit, struct rev_info *revs) html(""); /* Empty 'Age' column */ /* Print msgbuf into remainder of table row */ - htmlf("\n", cols, + htmlf("\n", columns - (revs->graph ? 1 : 0), ctx.qry.showmsg ? " class='logmsg'" : ""); html_txt(msgbuf.buf); html("\n"); @@ -283,7 +283,7 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern struct rev_info rev; struct commit *commit; struct vector vec = VECTOR_INIT(char *); - int i, columns = 3; + int i, columns = commit_graph ? 4 : 3; char *arg; /* First argv is NULL */ @@ -421,7 +421,7 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern } html(""); } else if ((commit = get_revision(&rev)) != NULL) { - html(""); + htmlf("", columns); cgit_log_link("[...]", NULL, NULL, ctx.qry.head, NULL, ctx.qry.vpath, 0, NULL, NULL, ctx.qry.showmsg); html("\n"); diff --git a/ui-refs.c b/ui-refs.c index e89f836..45db2ac 100644 --- a/ui-refs.c +++ b/ui-refs.c @@ -177,7 +177,7 @@ static int print_tag(struct refinfo *ref) static void print_refs_link(char *path) { - html(""); + html(""); cgit_refs_link("[...]", NULL, NULL, ctx.qry.head, NULL, path); html(""); } @@ -252,7 +252,7 @@ void cgit_print_refs() cgit_print_tags(0); else { cgit_print_branches(0); - html(" "); + html(" "); cgit_print_tags(0); } html(""); diff --git a/ui-summary.c b/ui-summary.c index b4fdd57..38639ce 100644 --- a/ui-summary.c +++ b/ui-summary.c @@ -17,15 +17,22 @@ int urls = 0; static void print_url(char *base, char *suffix) { + int columns = 3; + + if (ctx.repo->enable_log_filecount) + columns++; + if (ctx.repo->enable_log_linecount) + columns++; + if (!base || !*base) return; if (urls++ == 0) { - html(" "); - html("Clone\n"); + htmlf(" ", columns); + htmlf("Clone\n", columns); } if (suffix && *suffix) base = fmt("%s/%s", base, suffix); - html(""); html_txt(base); @@ -52,12 +59,19 @@ static void print_urls(char *txt, char *suffix) void cgit_print_summary() { + int columns = 3; + + if (ctx.repo->enable_log_filecount) + columns++; + if (ctx.repo->enable_log_linecount) + columns++; + html(""); cgit_print_branches(ctx.cfg.summary_branches); - html(""); + htmlf("", columns); cgit_print_tags(ctx.cfg.summary_tags); if (ctx.cfg.summary_log > 0) { - html(""); + htmlf("", columns); cgit_print_log(ctx.qry.head, 0, ctx.cfg.summary_log, NULL, NULL, NULL, 0, 0, 0); } -- 2.50.1 From 977a3ad7bf212e6ec7f43c16763321061ee64a69 Mon Sep 17 00:00:00 2001 From: Lukas Fleischer Date: Tue, 5 Mar 2013 16:48:27 +0100 Subject: [PATCH 12/16] ui-summary.c: Move urls variable into print_urls() There's no need for this variable to be global. Printing the header in print_urls() instead of print_url() allows for moving this variable into print_urls() without having to pass any status to print_url(). Note that this only works as long as we don't call print_urls() more than once. Signed-off-by: Lukas Fleischer --- ui-summary.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/ui-summary.c b/ui-summary.c index 38639ce..0754bb7 100644 --- a/ui-summary.c +++ b/ui-summary.c @@ -13,8 +13,6 @@ #include "ui-refs.h" #include "ui-blob.h" -int urls = 0; - static void print_url(char *base, char *suffix) { int columns = 3; @@ -26,10 +24,6 @@ static void print_url(char *base, char *suffix) if (!base || !*base) return; - if (urls++ == 0) { - htmlf("", columns); - htmlf("\n", columns); - } if (suffix && *suffix) base = fmt("%s/%s", base, suffix); htmlf("", columns); + htmlf("\n", columns); + } print_url(h, suffix); *t = c; h = t; -- 2.50.1 From 121089ced5e1d3f3103cbc2b37f5fb579d800915 Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Wed, 20 Mar 2013 21:14:22 +0100 Subject: [PATCH 13/16] Makefile: remove CGIT-CFLAGS files in clean stage --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 8c00190..83d4716 100644 --- a/Makefile +++ b/Makefile @@ -130,7 +130,7 @@ $(DOC_PDF): %.pdf : %.txt a2x -f pdf cgitrc.5.txt clean: clean-doc - $(RM) cgit VERSION *.o tags + $(RM) cgit VERSION CGIT-CFLAGS *.o tags $(RM) -r .deps cleanall: clean -- 2.50.1 From b60e6bff75719a5fb0df970bac3be6b2726cf73a Mon Sep 17 00:00:00 2001 From: Lukas Fleischer Date: Thu, 7 Mar 2013 08:56:22 +0100 Subject: [PATCH 14/16] Convert pager navigation into a unordered list It is common practice and semantically appropriate to use unordered lists for long navigation lists. This also fixes the layout of very long pager navigations in Webkit-based browsers. Signed-off-by: Lukas Fleischer --- cgit.css | 14 ++++++++++---- ui-log.c | 9 ++++++--- ui-repolist.c | 6 ++++-- 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/cgit.css b/cgit.css index 54da076..a50d62b 100644 --- a/cgit.css +++ b/cgit.css @@ -538,17 +538,23 @@ div#cgit table.list td.sublevel-repo { padding-left: 1.5em; } -div#cgit div.pager { +div#cgit ul.pager { + list-style-type: none; text-align: center; margin: 1em 0em 0em 0em; + padding: 0; } -div#cgit div.pager a { +div#cgit ul.pager li { + display: inline-block; + margin: 0.25em 0.5em; +} + +div#cgit ul.pager a { color: #777; - margin: 0em 0.5em; } -div#cgit div.pager .current { +div#cgit ul.pager .current { font-weight: bold; } diff --git a/ui-log.c b/ui-log.c index 954d3e1..aaffb4e 100644 --- a/ui-log.c +++ b/ui-log.c @@ -405,21 +405,24 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern commit->parents = NULL; } if (pager) { - html("
 
 
 
 
 
Clone
 
Clone
"); + html("
    "); if (ofs > 0) { + html("
  • "); cgit_log_link("[prev]", NULL, NULL, ctx.qry.head, ctx.qry.sha1, ctx.qry.vpath, ofs - cnt, ctx.qry.grep, ctx.qry.search, ctx.qry.showmsg); - html(" "); + html("
    • "); } if ((commit = get_revision(&rev)) != NULL) { + html("
  • "); cgit_log_link("[next]", NULL, NULL, ctx.qry.head, ctx.qry.sha1, ctx.qry.vpath, ofs + cnt, ctx.qry.grep, ctx.qry.search, ctx.qry.showmsg); + html("
    • "); } - html("
"); + html(""); } else if ((commit = get_revision(&rev)) != NULL) { htmlf("", columns); cgit_log_link("[...]", NULL, NULL, ctx.qry.head, NULL, diff --git a/ui-repolist.c b/ui-repolist.c index 66c88c4..a9751f6 100644 --- a/ui-repolist.c +++ b/ui-repolist.c @@ -128,13 +128,15 @@ static void print_pager(int items, int pagelen, char *search, char *sort) { int i, ofs; char *class = NULL; - html("
"); + html("
    "); for (i = 0, ofs = 0; ofs < items; i++, ofs = i * pagelen) { class = (ctx.qry.ofs == ofs) ? "current" : NULL; + html("
  • "); cgit_index_link(fmt("[%d]", i + 1), fmt("Page %d", i + 1), class, search, sort, ofs); + html("
    • "); } - html("
"); + html(""); } static int cmp(const char *s1, const char *s2) -- 2.50.1 From 1c32e008c8fda46f812c38f46ae7515bcf8002ee Mon Sep 17 00:00:00 2001 From: John Keeping Date: Sun, 7 Apr 2013 15:06:23 +0100 Subject: [PATCH 15/16] ui-blob: don't segfault when no path is given It it possible to inspect blobs by specifying only the SHA-1, and CGit provides links to do so, for example if a tag points directly at a blob. In this case the path_items structure is never used, but creating it still causes strlen to be run on a null pointer. Fix this. This error was introduced by commit c1633c6 (Update git to v1.7.6.5 - 2013-03-02). Signed-off-by: John Keeping --- ui-blob.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui-blob.c b/ui-blob.c index c59fbcb..7aec0b1 100644 --- a/ui-blob.c +++ b/ui-blob.c @@ -80,7 +80,7 @@ void cgit_print_blob(const char *hex, char *path, const char *head) struct commit *commit; struct pathspec_item path_items = { .match = path, - .len = strlen(path) + .len = path ? strlen(path) : 0 }; struct pathspec paths = { .nr = 1, -- 2.50.1 From 849ecd961df9454d6f849eac34e6f501395c4f01 Mon Sep 17 00:00:00 2001 From: John Keeping Date: Mon, 8 Apr 2013 09:00:22 +0100 Subject: [PATCH 16/16] Update git to v1.8.2.1 This requires a small change to how we handle notes, but otherwise just works. Note that we can't use anything from v1.8.0 until v1.8.2.1 because some of the symbols that we need for graph drawing were made private in v1.8.0 and this was not reverted until v1.8.2.1. Signed-off-by: John Keeping --- Makefile | 2 +- git | 2 +- ui-commit.c | 3 ++- ui-log.c | 6 +++--- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index 83d4716..59edab0 100644 --- a/Makefile +++ b/Makefile @@ -14,7 +14,7 @@ htmldir = $(docdir) pdfdir = $(docdir) mandir = $(prefix)/share/man SHA1_HEADER = -GIT_VER = 1.7.12.4 +GIT_VER = 1.8.2.1 GIT_URL = https://git-core.googlecode.com/files/git-$(GIT_VER).tar.gz INSTALL = install MAN5_TXT = $(wildcard *.5.txt) diff --git a/git b/git index 7e20105..5bda18c 160000 --- a/git +++ b/git @@ -1 +1 @@ -Subproject commit 7e2010537e96d0a1144520222f20ba1dc3d61441 +Subproject commit 5bda18c186e455f8e65f976d3bf333ab1f4b5b53 diff --git a/ui-commit.c b/ui-commit.c index 0783285..5a552a1 100644 --- a/ui-commit.c +++ b/ui-commit.c @@ -36,7 +36,8 @@ void cgit_print_commit(char *hex, const char *prefix) } info = cgit_parse_commit(commit); - format_note(NULL, sha1, ¬es, PAGE_ENCODING, 0); + init_display_notes(NULL); + format_display_notes(sha1, ¬es, PAGE_ENCODING, 0); load_ref_decorations(DECORATE_FULL_REFS); diff --git a/ui-log.c b/ui-log.c index aaffb4e..8d8b235 100644 --- a/ui-log.c +++ b/ui-log.c @@ -195,9 +195,8 @@ static void print_commit(struct commit *commit, struct rev_info *revs) strbuf_addstr(&msgbuf, info->msg); strbuf_addch(&msgbuf, '\n'); } - format_note(NULL, commit->object.sha1, &msgbuf, - PAGE_ENCODING, - NOTES_SHOW_HEADER | NOTES_INDENT); + format_display_notes(commit->object.sha1, + &msgbuf, PAGE_ENCODING, 0); strbuf_addch(&msgbuf, '\n'); strbuf_ltrim(&msgbuf); } @@ -397,6 +396,7 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern commit->parents = NULL; } + init_display_notes(NULL); for (i = 0; i < cnt && (commit = get_revision(&rev)) != NULL; i++) { print_commit(commit, &rev); free(commit->buffer); -- 2.50.1