Merge pull request #4217 from pks-t/pks/readonly-cfg-backend

Honor read-only flag when writing to config backends

src/config.c

@@ -576,22 +576,50 @@ int git_config_foreach_match(
  * Setters
  **************/
 
-static int config_error_nofiles(const char *name)
+typedef enum {
+	BACKEND_USE_SET,
+	BACKEND_USE_DELETE
+} backend_use;
+
+static const char *uses[] = {
+    "set",
+    "delete"
+};
+
+static int get_backend_for_use(git_config_backend **out,
+	git_config *cfg, const char *name, backend_use use)
 {
+	size_t i;
+	file_internal *f;
+
+	*out = NULL;
+
+	if (git_vector_length(&cfg->files) == 0) {
+		giterr_set(GITERR_CONFIG,
+			"cannot %s value for '%s' when no config files exist",
+			uses[use], name);
+		return GIT_ENOTFOUND;
+	}
+
+	git_vector_foreach(&cfg->files, i, f) {
+		if (!f->file->readonly) {
+			*out = f->file;
+			return 0;
+		}
+	}
+
 	giterr_set(GITERR_CONFIG,
-		"cannot set value for '%s' when no config files exist", name);
+		"cannot %s value for '%s' when all config files are readonly",
+		uses[use], name);
 	return GIT_ENOTFOUND;
 }
 
 int git_config_delete_entry(git_config *cfg, const char *name)
 {
 	git_config_backend *file;
-	file_internal *internal;
 
-	internal = git_vector_get(&cfg->files, 0);
-	if (!internal || !internal->file)
-		return config_error_nofiles(name);
-	file = internal->file;
+	if (get_backend_for_use(&file, cfg, name, BACKEND_USE_DELETE) < 0)
+		return GIT_ENOTFOUND;
 
 	return file->del(file, name);
 }
@@ -617,17 +645,14 @@ int git_config_set_string(git_config *cfg, const char *name, const char *value)
 {
 	int error;
 	git_config_backend *file;
-	file_internal *internal;
 
 	if (!value) {
 		giterr_set(GITERR_CONFIG, "the value to set cannot be NULL");
 		return -1;
 	}
 
-	internal = git_vector_get(&cfg->files, 0);
-	if (!internal || !internal->file)
-		return config_error_nofiles(name);
-	file = internal->file;
+	if (get_backend_for_use(&file, cfg, name, BACKEND_USE_SET) < 0)
+		return GIT_ENOTFOUND;
 
 	error = file->set(file, name, value);
 
@@ -1032,25 +1057,19 @@ int git_config_multivar_iterator_new(git_config_iterator **out, const git_config
 int git_config_set_multivar(git_config *cfg, const char *name, const char *regexp, const char *value)
 {
 	git_config_backend *file;
-	file_internal *internal;
 
-	internal = git_vector_get(&cfg->files, 0);
-	if (!internal || !internal->file)
-		return config_error_nofiles(name);
-	file = internal->file;
+	if (get_backend_for_use(&file, cfg, name, BACKEND_USE_DELETE) < 0)
+		return GIT_ENOTFOUND;
 
 	return file->set_multivar(file, name, regexp, value);
 }
 
 int git_config_delete_multivar(git_config *cfg, const char *name, const char *regexp)
 {
 	git_config_backend *file;
-	file_internal *internal;
 
-	internal = git_vector_get(&cfg->files, 0);
-	if (!internal || !internal->file)
-		return config_error_nofiles(name);
-	file = internal->file;
+	if (get_backend_for_use(&file, cfg, name, BACKEND_USE_DELETE) < 0)
+		return GIT_ENOTFOUND;
 
 	return file->del_multivar(file, name, regexp);
 }

src/config_file.h

@@ -7,6 +7,7 @@
 #ifndef INCLUDE_config_file_h__
 #define INCLUDE_config_file_h__
 
+#include "git2/sys/config.h"
 #include "git2/config.h"
 
 GIT_INLINE(int) git_config_file_open(git_config_backend *cfg, unsigned int level)

tests/config/readonly.c

@@ -0,0 +1,64 @@
+#include "clar_libgit2.h"
+#include "config_file.h"
+#include "config.h"
+
+static git_config *cfg;
+
+void test_config_readonly__initialize(void)
+{
+	cl_git_pass(git_config_new(&cfg));
+}
+
+void test_config_readonly__cleanup(void)
+{
+	git_config_free(cfg);
+	cfg = NULL;
+}
+
+void test_config_readonly__writing_to_readonly_fails(void)
+{
+	git_config_backend *backend;
+
+	cl_git_pass(git_config_file__ondisk(&backend, "global"));
+	backend->readonly = 1;
+	cl_git_pass(git_config_add_backend(cfg, backend, GIT_CONFIG_LEVEL_GLOBAL, 0));
+
+	cl_git_fail_with(GIT_ENOTFOUND, git_config_set_string(cfg, "foo.bar", "baz"));
+	cl_assert(!git_path_exists("global"));
+}
+
+void test_config_readonly__writing_to_cfg_with_rw_precedence_succeeds(void)
+{
+	git_config_backend *backend;
+
+	cl_git_pass(git_config_file__ondisk(&backend, "global"));
+	backend->readonly = 1;
+	cl_git_pass(git_config_add_backend(cfg, backend, GIT_CONFIG_LEVEL_GLOBAL, 0));
+
+	cl_git_pass(git_config_file__ondisk(&backend, "local"));
+	cl_git_pass(git_config_add_backend(cfg, backend, GIT_CONFIG_LEVEL_LOCAL, 0));
+
+	cl_git_pass(git_config_set_string(cfg, "foo.bar", "baz"));
+
+	cl_assert(git_path_exists("local"));
+	cl_assert(!git_path_exists("global"));
+	cl_git_pass(p_unlink("local"));
+}
+
+void test_config_readonly__writing_to_cfg_with_ro_precedence_succeeds(void)
+{
+	git_config_backend *backend;
+
+	cl_git_pass(git_config_file__ondisk(&backend, "local"));
+	backend->readonly = 1;
+	cl_git_pass(git_config_add_backend(cfg, backend, GIT_CONFIG_LEVEL_LOCAL, 0));
+
+	cl_git_pass(git_config_file__ondisk(&backend, "global"));
+	cl_git_pass(git_config_add_backend(cfg, backend, GIT_CONFIG_LEVEL_GLOBAL, 0));
+
+	cl_git_pass(git_config_set_string(cfg, "foo.bar", "baz"));
+
+	cl_assert(!git_path_exists("local"));
+	cl_assert(git_path_exists("global"));
+	cl_git_pass(p_unlink("global"));
+}