4 files changed, 70 insertions, 6 deletions

diff --git a/source/cgit.c b/source/cgit.cindex 41bbad3..17a79a2 100644--- a/source/cgit.c+++ b/source/cgit.c@@ -481,6 +481,24 @@ out: 	strbuf_release(&cached_rc); } +/*+ * Unlike the cgitrc a scanned repository may carry, cgitrc and the files it+ * includes are meant to be there, and one that cannot be read would otherwise+ * leave nothing but an empty index to go on.+ */+static void parse_cgitrc(const char *filename)+{+	// Copied, since the name usually sits in the buffer macro expansion+	// reuses, which a line in the file can overwrite.+	char *file = xstrdup(filename);+	int err = config_file_parse(file, apply_config);++	if (err)+		fprintf(stderr, "[cgit] Unable to read config file %s: %s (%d)\n",+			file, strerror(err), err);+	free(file);+}+ static void add_mimetype(const char *name, const char *value) { 	struct string_list_item *item;@@ -736,7 +754,7 @@ static void apply_config(const char *name, const char *value) 	} else if (skip_prefix(name, "mimetype.", &arg)) 		add_mimetype(arg, value); 	else if (!strcmp(name, "include"))-		config_file_parse(cgit_expand_macros(value), apply_config);+		parse_cgitrc(cgit_expand_macros(value)); 	else if (skip_prefix(name, "repo.", &arg)) 		fprintf(stderr, "[cgit] Ignoring %s before any repo.url\n", name); 	else@@ -1365,7 +1383,7 @@ int cmd_main(int argc, const char **argv) 	prepare_context();  	parse_args(argc, argv);-	config_file_parse(cgit_expand_macros(ctx.env.cgit_config), apply_config);+	parse_cgitrc(cgit_expand_macros(ctx.env.cgit_config)); 	ctx.repo = NULL; 	http_parse_querystring(ctx.qry.raw, apply_query_param); 
diff --git a/source/cgit.c b/source/cgit.c
index 41bbad3..17a79a2 100644
--- a/source/cgit.c
+++ b/source/cgit.c
@@ -481,6 +481,24 @@ out:
 	strbuf_release(&cached_rc);
 }
 
+/*
+ * Unlike the cgitrc a scanned repository may carry, cgitrc and the files it
+ * includes are meant to be there, and one that cannot be read would otherwise
+ * leave nothing but an empty index to go on.
+ */
+static void parse_cgitrc(const char *filename)
+{
+	// Copied, since the name usually sits in the buffer macro expansion
+	// reuses, which a line in the file can overwrite.
+	char *file = xstrdup(filename);
+	int err = config_file_parse(file, apply_config);
+
+	if (err)
+		fprintf(stderr, "[cgit] Unable to read config file %s: %s (%d)\n",
+			file, strerror(err), err);
+	free(file);
+}
+
 static void add_mimetype(const char *name, const char *value)
 {
 	struct string_list_item *item;
@@ -736,7 +754,7 @@ static void apply_config(const char *name, const char *value)
 	} else if (skip_prefix(name, "mimetype.", &arg))
 		add_mimetype(arg, value);
 	else if (!strcmp(name, "include"))
-		config_file_parse(cgit_expand_macros(value), apply_config);
+		parse_cgitrc(cgit_expand_macros(value));
 	else if (skip_prefix(name, "repo.", &arg))
 		fprintf(stderr, "[cgit] Ignoring %s before any repo.url\n", name);
 	else
@@ -1365,7 +1383,7 @@ int cmd_main(int argc, const char **argv)
 	prepare_context();
 
 	parse_args(argc, argv);
-	config_file_parse(cgit_expand_macros(ctx.env.cgit_config), apply_config);
+	parse_cgitrc(cgit_expand_macros(ctx.env.cgit_config));
 	ctx.repo = NULL;
 	http_parse_querystring(ctx.qry.raw, apply_query_param);
 
diff --git a/source/config.c b/source/config.cindex 49cd398..ca7c688 100644--- a/source/config.c+++ b/source/config.c@@ -85,24 +85,29 @@ static int read_entry(FILE *f, struct strbuf *name, struct strbuf *value) 	} } -void config_file_parse(const char *filename, config_file_value_fn fn)+int config_file_parse(const char *filename, config_file_value_fn fn) { 	static int nesting; 	struct strbuf name = STRBUF_INIT; 	struct strbuf value = STRBUF_INIT; 	FILE *f;+	int err = 0;  	// An include line calls back into here, so an include cycle would 	// recurse until the stack gave out. 	if (nesting > MAX_INCLUDE_NESTING)-		return;+		return 0; 	if (!(f = fopen(filename, "r")))-		return;+		return errno; 	nesting++; 	while (read_entry(f, &name, &value)) 		fn(name.buf, value.buf);+	// A directory opens like a file and fails only once it is read.+	if (ferror(f))+		err = errno; 	nesting--; 	fclose(f); 	strbuf_release(&name); 	strbuf_release(&value);+	return err; }
diff --git a/source/config.c b/source/config.c
index 49cd398..ca7c688 100644
--- a/source/config.c
+++ b/source/config.c
@@ -85,24 +85,29 @@ static int read_entry(FILE *f, struct strbuf *name, struct strbuf *value)
 	}
 }
 
-void config_file_parse(const char *filename, config_file_value_fn fn)
+int config_file_parse(const char *filename, config_file_value_fn fn)
 {
 	static int nesting;
 	struct strbuf name = STRBUF_INIT;
 	struct strbuf value = STRBUF_INIT;
 	FILE *f;
+	int err = 0;
 
 	// An include line calls back into here, so an include cycle would
 	// recurse until the stack gave out.
 	if (nesting > MAX_INCLUDE_NESTING)
-		return;
+		return 0;
 	if (!(f = fopen(filename, "r")))
-		return;
+		return errno;
 	nesting++;
 	while (read_entry(f, &name, &value))
 		fn(name.buf, value.buf);
+	// A directory opens like a file and fails only once it is read.
+	if (ferror(f))
+		err = errno;
 	nesting--;
 	fclose(f);
 	strbuf_release(&name);
 	strbuf_release(&value);
+	return err;
 }
diff --git a/source/config.h b/source/config.hindex 3ddf974..30cf74d 100644--- a/source/config.h+++ b/source/config.h@@ -9,6 +9,10 @@  typedef void (*config_file_value_fn)(const char *name, const char *value); -extern void config_file_parse(const char *filename, config_file_value_fn fn);+/*+ * Returns 0, or an errno value when filename could not be opened or read. A+ * missing file is ordinary for some callers, so reporting one is left to them.+ */+extern int config_file_parse(const char *filename, config_file_value_fn fn);  #endif // CGIT_CONFIG_H
diff --git a/source/config.h b/source/config.h
index 3ddf974..30cf74d 100644
--- a/source/config.h
+++ b/source/config.h
@@ -9,6 +9,10 @@
 
 typedef void (*config_file_value_fn)(const char *name, const char *value);
 
-extern void config_file_parse(const char *filename, config_file_value_fn fn);
+/*
+ * Returns 0, or an errno value when filename could not be opened or read. A
+ * missing file is ordinary for some callers, so reporting one is left to them.
+ */
+extern int config_file_parse(const char *filename, config_file_value_fn fn);
 
 #endif // CGIT_CONFIG_H
diff --git a/tests/t0205-config.sh b/tests/t0205-config.shindex 32c260e..12087e7 100755--- a/tests/t0205-config.sh+++ b/tests/t0205-config.sh@@ -106,4 +106,41 @@ test_expect_success 'a NUL inside a comment does not end it' ' 	! grep Smuggled tmp ' +# cgitrc and the files it includes are meant to be there, so one that cannot+# be read is logged rather than left to show an empty index.+test_expect_success 'a config file that cannot be read is reported' '+	CGIT_CONFIG="$PWD/no-such-rc" QUERY_STRING="url=" cgit >tmp 2>err &&+	grep "No repositories found" tmp &&+	grep "Unable to read config file $PWD/no-such-rc: No such file or directory" err &&+	mkdir rc-dir &&+	CGIT_CONFIG="$PWD/rc-dir" QUERY_STRING="url=" cgit >tmp 2>err &&+	grep "Unable to read config file $PWD/rc-dir: Is a directory" err &&+	{+		echo "include=$PWD/no-such-include" &&+		cat diagrc+	} >includerc &&+	cgit_rc includerc "url=diag/" >tmp 2>err &&+	grep "Status: 200" tmp &&+	grep "Unable to read config file $PWD/no-such-include: No such file or directory" err+'++# A scanned repository's own cgitrc is optional, so a scan says nothing when+# one is missing, and neither does reading the scan back from the cache.+test_expect_success 'a scan says nothing of the files it may lack' '+	git clone -q --bare repos/foo/.git quiet/foo.git &&+	mkdir quiet-cache &&+	{+		echo "virtual-root=/" &&+		echo "cache-root=$PWD/quiet-cache" &&+		echo "cache-size=64" &&+		echo "cache-index-ttl=0" &&+		echo "scan-path=$PWD/quiet"+	} >quietrc &&+	cgit_rc quietrc "url=" >tmp 2>err &&+	grep ">foo.git</a>" tmp &&+	cgit_rc quietrc "url=" >tmp 2>>err &&+	grep ">foo.git</a>" tmp &&+	test_must_be_empty err+'+ test_done
diff --git a/tests/t0205-config.sh b/tests/t0205-config.sh
index 32c260e..12087e7 100755
--- a/tests/t0205-config.sh
+++ b/tests/t0205-config.sh
@@ -106,4 +106,41 @@ test_expect_success 'a NUL inside a comment does not end it' '
 	! grep Smuggled tmp
 '
 
+# cgitrc and the files it includes are meant to be there, so one that cannot
+# be read is logged rather than left to show an empty index.
+test_expect_success 'a config file that cannot be read is reported' '
+	CGIT_CONFIG="$PWD/no-such-rc" QUERY_STRING="url=" cgit >tmp 2>err &&
+	grep "No repositories found" tmp &&
+	grep "Unable to read config file $PWD/no-such-rc: No such file or directory" err &&
+	mkdir rc-dir &&
+	CGIT_CONFIG="$PWD/rc-dir" QUERY_STRING="url=" cgit >tmp 2>err &&
+	grep "Unable to read config file $PWD/rc-dir: Is a directory" err &&
+	{
+		echo "include=$PWD/no-such-include" &&
+		cat diagrc
+	} >includerc &&
+	cgit_rc includerc "url=diag/" >tmp 2>err &&
+	grep "Status: 200" tmp &&
+	grep "Unable to read config file $PWD/no-such-include: No such file or directory" err
+'
+
+# A scanned repository's own cgitrc is optional, so a scan says nothing when
+# one is missing, and neither does reading the scan back from the cache.
+test_expect_success 'a scan says nothing of the files it may lack' '
+	git clone -q --bare repos/foo/.git quiet/foo.git &&
+	mkdir quiet-cache &&
+	{
+		echo "virtual-root=/" &&
+		echo "cache-root=$PWD/quiet-cache" &&
+		echo "cache-size=64" &&
+		echo "cache-index-ttl=0" &&
+		echo "scan-path=$PWD/quiet"
+	} >quietrc &&
+	cgit_rc quietrc "url=" >tmp 2>err &&
+	grep ">foo.git</a>" tmp &&
+	cgit_rc quietrc "url=" >tmp 2>>err &&
+	grep ">foo.git</a>" tmp &&
+	test_must_be_empty err
+'
+
 test_done