Commit 4bf136b0c33c0066e1a4da8c236b42d4f655073f

Nicolas Cavallari 2021-06-23T16:53:53

config: fix included configs not refreshed more than once If an included config is refreshed twice, the second update is not taken into account. This is because the list of included files is cleared after re-reading the new configuration, instead of being cleared before. Fix it and add a test case to check for this bug.

diff --git a/src/config_file.c b/src/config_file.c
index 7fba71a..5267beb 100644
--- a/src/config_file.c
+++ b/src/config_file.c
@@ -164,23 +164,27 @@ out:
 	return error;
 }
 
+static void config_file_clear_includes(config_file_backend* cfg)
+{
+	config_file *include;
+	uint32_t i;
+
+	git_array_foreach(cfg->file.includes, i, include)
+		config_file_clear(include);
+	git_array_clear(cfg->file.includes);
+}
+
 static int config_file_set_entries(git_config_backend *cfg, git_config_entries *entries)
 {
 	config_file_backend *b = GIT_CONTAINER_OF(cfg, config_file_backend, parent);
 	git_config_entries *old = NULL;
-	config_file *include;
 	int error;
-	uint32_t i;
 
 	if (b->parent.readonly) {
 		git_error_set(GIT_ERROR_CONFIG, "this backend is read-only");
 		return -1;
 	}
 
-	git_array_foreach(b->file.includes, i, include)
-		config_file_clear(include);
-	git_array_clear(b->file.includes);
-
 	if ((error = git_mutex_lock(&b->values_mutex)) < 0) {
 		git_error_set(GIT_ERROR_OS, "failed to lock config backend");
 		goto out;
@@ -202,6 +206,8 @@ static int config_file_refresh_from_buffer(git_config_backend *cfg, const char *
 	git_config_entries *entries = NULL;
 	int error;
 
+	config_file_clear_includes(b);
+
 	if ((error = git_config_entries_new(&entries)) < 0 ||
 	    (error = config_file_read_buffer(entries, b->repo, &b->file,
 					     b->level, 0, buf, buflen)) < 0 ||
@@ -229,6 +235,8 @@ static int config_file_refresh(git_config_backend *cfg)
 	if (!modified)
 		return 0;
 
+	config_file_clear_includes(b);
+
 	if ((error = git_config_entries_new(&entries)) < 0 ||
 	    (error = config_file_read(entries, b->repo, &b->file, b->level, 0)) < 0 ||
 	    (error = config_file_set_entries(cfg, entries)) < 0)
diff --git a/tests/config/include.c b/tests/config/include.c
index e2b0fc9..b702d62 100644
--- a/tests/config/include.c
+++ b/tests/config/include.c
@@ -178,6 +178,30 @@ void test_config_include__rewriting_include_refreshes_values(void)
 	cl_git_pass(p_unlink("second"));
 }
 
+void test_config_include__rewriting_include_twice_refreshes_values(void)
+{
+	cl_git_mkfile("top-level", "[include]\npath = included");
+	cl_git_mkfile("included", "[foo]\nbar = first-value");
+
+	cl_git_pass(git_config_open_ondisk(&cfg, "top-level"));
+	cl_git_pass(git_config_get_string_buf(&buf, cfg, "foo.bar"));
+
+	git_buf_clear(&buf);
+	cl_git_mkfile("included", "[foo]\nother = value2");
+	cl_git_fail(git_config_get_string_buf(&buf, cfg, "foo.bar"));
+	cl_git_pass(git_config_get_string_buf(&buf, cfg, "foo.other"));
+	cl_assert_equal_s(buf.ptr, "value2");
+
+	git_buf_clear(&buf);
+	cl_git_mkfile("included", "[foo]\nanother = bar");
+	cl_git_fail(git_config_get_string_buf(&buf, cfg, "foo.other"));
+	cl_git_pass(git_config_get_string_buf(&buf, cfg, "foo.another"));
+	cl_assert_equal_s(buf.ptr, "bar");
+
+	cl_git_pass(p_unlink("top-level"));
+	cl_git_pass(p_unlink("included"));
+}
+
 void test_config_include__included_variables_cannot_be_deleted(void)
 {
 	cl_git_mkfile("top-level", "[include]\npath = included\n");