[PATCH] configfile: Use git's internal config system instead.

[PATCH] configfile: Use git's internal config system instead.

[Jason]

This commit is completely broken and does not work but here's the
general idea if someone would like to clean it up and work out the
details.
---
 cgit.c       | 12 +++++----
 cgit.mk      |  1 -
 configfile.c | 87 ------------------------------------------------------------
 configfile.h |  8 ------
 scan-tree.c  |  7 ++---
 5 files changed, 11 insertions(+), 104 deletions(-)
 delete mode 100644 configfile.c
 delete mode 100644 configfile.h

diff --git a/cgit.c b/cgit.c
index 5ffd9e0..be080e1 100644
--- a/cgit.c
+++ b/cgit.c
@@ -10,13 +10,13 @@
 #include "cgit.h"
 #include "cache.h"
 #include "cmd.h"
-#include "configfile.h"
 #include "html.h"
 #include "ui-shared.h"
 #include "ui-stats.h"
 #include "ui-blob.h"
 #include "ui-summary.h"
 #include "scan-tree.h"
+#include <cache.h>
 
 const char *cgit_version = CGIT_VERSION;
 
@@ -123,7 +123,7 @@ static void repo_config(struct cgit_repo *repo, const char *name, const char *va
 	}
 }
 
-static void config_cb(const char *name, const char *value)
+static int config_cb(const char *name, const char *value, void *data)
 {
 	if (!strcmp(name, "section") || !strcmp(name, "repo.group"))
 		ctx.cfg.section = xstrdup(value);
@@ -290,7 +290,9 @@ static void config_cb(const char *name, const char *value)
 	} else if (!prefixcmp(name, "mimetype."))
 		add_mimetype(name + 9, value);
 	else if (!strcmp(name, "include"))
-		parse_configfile(expand_macros(value), config_cb);
+		git_config_from_file(config_cb, expand_macros(value), NULL);
+	
+	return 0;
 }
 
 static void querystring_cb(const char *name, const char *value)
@@ -838,7 +840,7 @@ static void process_cached_repolist(const char *path)
 		goto out;
 	}
 
-	parse_configfile(cached_rc.buf, config_cb);
+	git_config_from_file(config_cb, cached_rc.buf, NULL);
 
 	/* If the cached configfile hasn't expired, lets exit now */
 	age = time(NULL) - st.st_mtime;
@@ -947,7 +949,7 @@ int main(int argc, const char **argv)
 	cgit_repolist.repos = NULL;
 
 	cgit_parse_args(argc, argv);
-	parse_configfile(expand_macros(ctx.env.cgit_config), config_cb);
+	git_config_from_file(config_cb, expand_macros(ctx.env.cgit_config), NULL);
 	ctx.repo = NULL;
 	http_parse_querystring(ctx.qry.raw, querystring_cb);
 
diff --git a/cgit.mk b/cgit.mk
index 8af0041..ff24cf3 100644
--- a/cgit.mk
+++ b/cgit.mk
@@ -28,7 +28,6 @@ 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
diff --git a/configfile.c b/configfile.c
deleted file mode 100644
index e7e43a4..0000000
--- a/configfile.c
+++ /dev/null
@@ -1,87 +0,0 @@
-/* configfile.c: parsing of config files
- *
- * Copyright (C) 2008 Lars Hjemli
- * Copyright (C) 2013 Jason A. Donenfeld <Jason at zx2c4.com>. All Rights Reserved.
- *
- * Licensed under GNU General Public License v2
- *   (see COPYING for full license text)
- */
-
-#include <ctype.h>
-#include <stdio.h>
-#include "configfile.h"
-
-static int next_char(FILE *f)
-{
-	int c = fgetc(f);
-	if (c == '\r') {
-		c = fgetc(f);
-		if (c != '\n') {
-			ungetc(c, f);
-			c = '\r';
-		}
-	}
-	return c;
-}
-
-static void skip_line(FILE *f)
-{
-	int c;
-
-	while ((c = next_char(f)) && c != '\n' && c != EOF)
-		;
-}
-
-static int read_config_line(FILE *f, char *line, const char **value, int bufsize)
-{
-	int i = 0, isname = 0;
-
-	*value = NULL;
-	while (i < bufsize - 1) {
-		int c = next_char(f);
-		if (!isname && (c == '#' || c == ';')) {
-			skip_line(f);
-			continue;
-		}
-		if (!isname && isspace(c))
-			continue;
-
-		if (c == '=' && !*value) {
-			line[i] = 0;
-			*value = &line[i + 1];
-		} else if (c == '\n' && !isname) {
-			i = 0;
-			continue;
-		} else if (c == '\n' || c == EOF) {
-			break;
-		} else {
-			line[i] = c;
-		}
-		isname = 1;
-		i++;
-	}
-	line[i] = 0;
-	return i;
-}
-
-int parse_configfile(const char *filename, configfile_value_fn fn)
-{
-	static int nesting;
-	int len;
-	char line[256];
-	const char *value;
-	FILE *f;
-
-	/* cancel deeply nested include-commands */
-	if (nesting > 8)
-		return -1;
-	if (!(f = fopen(filename, "r")))
-		return -1;
-	nesting++;
-	while ((len = read_config_line(f, line, &value, sizeof(line))) > 0)
-		fn(line, value);
-	nesting--;
-	fclose(f);
-	return 0;
-}
-
diff --git a/configfile.h b/configfile.h
deleted file mode 100644
index 04235e5..0000000
--- a/configfile.h
+++ /dev/null
@@ -1,8 +0,0 @@
-#ifndef CONFIGFILE_H
-#define CONFIGFILE_H
-
-typedef void (*configfile_value_fn)(const char *name, const char *value);
-
-extern int parse_configfile(const char *filename, configfile_value_fn fn);
-
-#endif /* CONFIGFILE_H */
diff --git a/scan-tree.c b/scan-tree.c
index 2684b44..60791f1 100644
--- a/scan-tree.c
+++ b/scan-tree.c
@@ -9,8 +9,8 @@
 
 #include "cgit.h"
 #include "scan-tree.h"
-#include "configfile.h"
 #include "html.h"
+#include <cache.h>
 
 /* return 1 if path contains a objects/ directory and a HEAD file */
 static int is_git_dir(const char *path)
@@ -49,9 +49,10 @@ out:
 struct cgit_repo *repo;
 repo_config_fn config_fn;
 
-static void repo_config(const char *name, const char *value)
+static int repo_config(const char *name, const char *value, void *data)
 {
 	config_fn(repo, name, value);
+	return 0;
 }
 
 static int gitconfig_config(const char *key, const char *value, void *cb)
@@ -172,7 +173,7 @@ static void add_repo(const char *base, struct strbuf *path, repo_config_fn fn)
 
 	strbuf_addstr(path, "cgitrc");
 	if (!stat(path->buf, &st))
-		parse_configfile(xstrdup(path->buf), &repo_config);
+		git_config_from_file(&repo_config, xstrdup(path->buf), NULL);
 
 	strbuf_release(&rel);
 }
-- 
1.8.2.1

[PATCH] configfile: Use git's internal config system instead.

[cgit]

On Tue, Jun 04, 2013 at 07:58:48PM +0200, Jason A. Donenfeld wrote:
> This commit is completely broken and does not work but here's the
> general idea if someone would like to clean it up and work out the
> details.
> ---
>  cgit.c       | 12 +++++----
>  cgit.mk      |  1 -
>  configfile.c | 87 ------------------------------------------------------------
>  configfile.h |  8 ------
>  scan-tree.c  |  7 ++---
>  5 files changed, 11 insertions(+), 104 deletions(-)
>  delete mode 100644 configfile.c
>  delete mode 100644 configfile.h
> 
> [...]
> +		git_config_from_file(config_cb, expand_macros(value), NULL);

git_config_from_file() does not allow dots (".") inside configuration
variable names (since they are used to delimit sections from keys). If
we want to use the Git configuration system like this, we have to change
our configuration file format to use sections ("[repo]") instead of
prefixing each variable ("repo.").

[PATCH] configfile: Use git's internal config system instead.

[Jason]

On Tue, Jun 4, 2013 at 8:52 PM, Lukas Fleischer <cgit at cryptocrack.de> wrote:
> git_config_from_file() does not allow dots (".") inside configuration
> variable names (since they are used to delimit sections from keys). If
> we want to use the Git configuration system like this, we have to change
> our configuration file format to use sections ("[repo]") instead of
> prefixing each variable ("repo.").

I had feared this would be the case. I still think it might be
worthwhile to pursue revamping our configuration format to use
git_config_from_file along with all its niceties. This would also put
us somewhat more in line with upstream git and would allow all sorts
of neat things -- for example instead of a per-repo cgitrc, a repo's
bare gitconfig file could be used instead. We already support reading
a few keys out of it, and moving to using git's config format for
everything would allow us to unify all of this.

This would, of course, break compatibility to a certain degree.
Migration strategies might include one of these -- 1) we just break
things and leave a note about that in the release notes of a major
version bump; 2) we support both parsers for an amount of time along
with a depreciated warning message; 3) we write a simple format
converter that either a) runs automatically or b) is included in
contrib and announced in the release notes. I'd probably lean toward
3b, but I'm open to all opinions here.

[PATCH] configfile: Use git's internal config system instead.

[mackyle]

On Aug 12, 2013, at 11:35, Jason A. Donenfeld wrote:
> On Tue, Jun 4, 2013 at 8:52 PM, Lukas Fleischer  
> <cgit at cryptocrack.de> wrote:
>> git_config_from_file() does not allow dots (".") inside configuration
>> variable names (since they are used to delimit sections from keys).  
>> If
>> we want to use the Git configuration system like this, we have to  
>> change
>> our configuration file format to use sections ("[repo]") instead of
>> prefixing each variable ("repo.").
>
> I had feared this would be the case. I still think it might be
> worthwhile to pursue revamping our configuration format to use
> git_config_from_file along with all its niceties. This would also put
> us somewhat more in line with upstream git and would allow all sorts
> of neat things -- for example instead of a per-repo cgitrc, a repo's
> bare gitconfig file could be used instead. We already support reading
> a few keys out of it, and moving to using git's config format for
> everything would allow us to unify all of this.

Also, starting with the 1.8.4 release (currently at 1.8.4-rc2), it's  
possible to do config_from_blob which may also make some of this easier.

[PATCH] configfile: Use git's internal config system instead.

[Jason]

This commit is completely broken and does not work but here's the
general idea if someone would like to clean it up and work out the
details.
---
 cgit.c       | 12 +++++----
 cgit.mk      |  1 -
 configfile.c | 87 ------------------------------------------------------------
 configfile.h |  8 ------
 scan-tree.c  |  7 ++---
 5 files changed, 11 insertions(+), 104 deletions(-)
 delete mode 100644 configfile.c
 delete mode 100644 configfile.h

diff --git a/cgit.c b/cgit.c
index 5ffd9e0..6e5798f 100644
--- a/cgit.c
+++ b/cgit.c
@@ -10,13 +10,13 @@
 #include "cgit.h"
 #include "cache.h"
 #include "cmd.h"
-#include "configfile.h"
 #include "html.h"
 #include "ui-shared.h"
 #include "ui-stats.h"
 #include "ui-blob.h"
 #include "ui-summary.h"
 #include "scan-tree.h"
+#include <cache.h>
 
 const char *cgit_version = CGIT_VERSION;
 
@@ -123,7 +123,7 @@ static void repo_config(struct cgit_repo *repo, const char *name, const char *va
 	}
 }
 
-static void config_cb(const char *name, const char *value)
+static int config_cb(const char *name, const char *value, void *data)
 {
 	if (!strcmp(name, "section") || !strcmp(name, "repo.group"))
 		ctx.cfg.section = xstrdup(value);
@@ -290,7 +290,9 @@ static void config_cb(const char *name, const char *value)
 	} else if (!prefixcmp(name, "mimetype."))
 		add_mimetype(name + 9, value);
 	else if (!strcmp(name, "include"))
-		parse_configfile(expand_macros(value), config_cb);
+		git_config_parse_parameter(expand_macros(value), config_cb, NULL);
+	
+	return 0;
 }
 
 static void querystring_cb(const char *name, const char *value)
@@ -838,7 +840,7 @@ static void process_cached_repolist(const char *path)
 		goto out;
 	}
 
-	parse_configfile(cached_rc.buf, config_cb);
+	git_config_parse_parameter(cached_rc.buf, config_cb, NULL);
 
 	/* If the cached configfile hasn't expired, lets exit now */
 	age = time(NULL) - st.st_mtime;
@@ -947,7 +949,7 @@ int main(int argc, const char **argv)
 	cgit_repolist.repos = NULL;
 
 	cgit_parse_args(argc, argv);
-	parse_configfile(expand_macros(ctx.env.cgit_config), config_cb);
+	git_config_parse_parameter(expand_macros(ctx.env.cgit_config), config_cb, NULL);
 	ctx.repo = NULL;
 	http_parse_querystring(ctx.qry.raw, querystring_cb);
 
diff --git a/cgit.mk b/cgit.mk
index 8af0041..ff24cf3 100644
--- a/cgit.mk
+++ b/cgit.mk
@@ -28,7 +28,6 @@ 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
diff --git a/configfile.c b/configfile.c
deleted file mode 100644
index e7e43a4..0000000
--- a/configfile.c
+++ /dev/null
@@ -1,87 +0,0 @@
-/* configfile.c: parsing of config files
- *
- * Copyright (C) 2008 Lars Hjemli
- * Copyright (C) 2013 Jason A. Donenfeld <Jason at zx2c4.com>. All Rights Reserved.
- *
- * Licensed under GNU General Public License v2
- *   (see COPYING for full license text)
- */
-
-#include <ctype.h>
-#include <stdio.h>
-#include "configfile.h"
-
-static int next_char(FILE *f)
-{
-	int c = fgetc(f);
-	if (c == '\r') {
-		c = fgetc(f);
-		if (c != '\n') {
-			ungetc(c, f);
-			c = '\r';
-		}
-	}
-	return c;
-}
-
-static void skip_line(FILE *f)
-{
-	int c;
-
-	while ((c = next_char(f)) && c != '\n' && c != EOF)
-		;
-}
-
-static int read_config_line(FILE *f, char *line, const char **value, int bufsize)
-{
-	int i = 0, isname = 0;
-
-	*value = NULL;
-	while (i < bufsize - 1) {
-		int c = next_char(f);
-		if (!isname && (c == '#' || c == ';')) {
-			skip_line(f);
-			continue;
-		}
-		if (!isname && isspace(c))
-			continue;
-
-		if (c == '=' && !*value) {
-			line[i] = 0;
-			*value = &line[i + 1];
-		} else if (c == '\n' && !isname) {
-			i = 0;
-			continue;
-		} else if (c == '\n' || c == EOF) {
-			break;
-		} else {
-			line[i] = c;
-		}
-		isname = 1;
-		i++;
-	}
-	line[i] = 0;
-	return i;
-}
-
-int parse_configfile(const char *filename, configfile_value_fn fn)
-{
-	static int nesting;
-	int len;
-	char line[256];
-	const char *value;
-	FILE *f;
-
-	/* cancel deeply nested include-commands */
-	if (nesting > 8)
-		return -1;
-	if (!(f = fopen(filename, "r")))
-		return -1;
-	nesting++;
-	while ((len = read_config_line(f, line, &value, sizeof(line))) > 0)
-		fn(line, value);
-	nesting--;
-	fclose(f);
-	return 0;
-}
-
diff --git a/configfile.h b/configfile.h
deleted file mode 100644
index 04235e5..0000000
--- a/configfile.h
+++ /dev/null
@@ -1,8 +0,0 @@
-#ifndef CONFIGFILE_H
-#define CONFIGFILE_H
-
-typedef void (*configfile_value_fn)(const char *name, const char *value);
-
-extern int parse_configfile(const char *filename, configfile_value_fn fn);
-
-#endif /* CONFIGFILE_H */
diff --git a/scan-tree.c b/scan-tree.c
index 2684b44..83ef4d9 100644
--- a/scan-tree.c
+++ b/scan-tree.c
@@ -9,8 +9,8 @@
 
 #include "cgit.h"
 #include "scan-tree.h"
-#include "configfile.h"
 #include "html.h"
+#include <cache.h>
 
 /* return 1 if path contains a objects/ directory and a HEAD file */
 static int is_git_dir(const char *path)
@@ -49,9 +49,10 @@ out:
 struct cgit_repo *repo;
 repo_config_fn config_fn;
 
-static void repo_config(const char *name, const char *value)
+static int repo_config(const char *name, const char *value, void *data)
 {
 	config_fn(repo, name, value);
+	return 0;
 }
 
 static int gitconfig_config(const char *key, const char *value, void *cb)
@@ -172,7 +173,7 @@ static void add_repo(const char *base, struct strbuf *path, repo_config_fn fn)
 
 	strbuf_addstr(path, "cgitrc");
 	if (!stat(path->buf, &st))
-		parse_configfile(xstrdup(path->buf), &repo_config);
+		git_config_parse_parameter(xstrdup(path->buf), &repo_config, NULL);
 
 	strbuf_release(&rel);
 }
-- 
1.8.2.1