Commit 5340d63d3815ddbd1a7e1b5b9628fce10089e8a0

Carlos Martín Nieto 2015-07-12T12:50:23

config: perform unlocking via git_transaction This makes the API for commiting or discarding changes the same as for references.

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4442d0a..5dd4b34 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,11 +9,10 @@ v0.23 + 1
 
 ### API additions
 
-* `git_config_lock()` and `git_config_unlock()` have been added, which
-  allow for transactional/atomic complex updates to the configuration,
-  removing the opportunity for concurrent operations and not
-  committing any changes until the unlock.
-
+* `git_config_lock()` has been added, which allow for
+  transactional/atomic complex updates to the configuration, removing
+  the opportunity for concurrent operations and not committing any
+  changes until the unlock.
 
 ### API removals
 
diff --git a/include/git2/config.h b/include/git2/config.h
index 2550f8e..05c3ad6 100644
--- a/include/git2/config.h
+++ b/include/git2/config.h
@@ -696,25 +696,16 @@ GIT_EXTERN(int) git_config_backend_foreach_match(
  * updates made after locking will not be visible to a reader until
  * the file is unlocked.
  *
- * @param cfg the configuration in which to lock
- * @return 0 or an error code
- */
-GIT_EXTERN(int) git_config_lock(git_config *cfg);
-
-/**
- * Unlock the backend with the highest priority
+ * You can apply the changes by calling `git_transaction_commit()`
+ * before freeing the transaction. Either of these actions will unlock
+ * the config.
  *
- * Unlocking will allow other writers to updat the configuration
- * file. Optionally, any changes performed since the lock will be
- * applied to the configuration.
- *
- * @param cfg the configuration
- * @param commit boolean which indicates whether to commit any changes
- * done since locking
+ * @param tx the resulting transaction, use this to commit or undo the
+ * changes
+ * @param cfg the configuration in which to lock
  * @return 0 or an error code
  */
-GIT_EXTERN(int) git_config_unlock(git_config *cfg, int commit);
-
+GIT_EXTERN(int) git_config_lock(git_transaction **tx, git_config *cfg);
 
 /** @} */
 GIT_END_DECL
diff --git a/src/config.c b/src/config.c
index 937d00d..2df1fc8 100644
--- a/src/config.c
+++ b/src/config.c
@@ -1144,8 +1144,9 @@ int git_config_open_default(git_config **out)
 	return error;
 }
 
-int git_config_lock(git_config *cfg)
+int git_config_lock(git_transaction **out, git_config *cfg)
 {
+	int error;
 	git_config_backend *file;
 	file_internal *internal;
 
@@ -1156,7 +1157,10 @@ int git_config_lock(git_config *cfg)
 	}
 	file = internal->file;
 
-	return file->lock(file);
+	if ((error = file->lock(file)) < 0)
+		return error;
+
+	return git_transaction_config_new(out, cfg);
 }
 
 int git_config_unlock(git_config *cfg, int commit)
diff --git a/src/config.h b/src/config.h
index f257cc9..ba74533 100644
--- a/src/config.h
+++ b/src/config.h
@@ -88,4 +88,19 @@ extern int git_config__cvar(
  */
 int git_config_lookup_map_enum(git_cvar_t *type_out, const char **str_out,
 			       const git_cvar_map *maps, size_t map_n, int enum_val);
+
+/**
+ * Unlock the backend with the highest priority
+ *
+ * Unlocking will allow other writers to updat the configuration
+ * file. Optionally, any changes performed since the lock will be
+ * applied to the configuration.
+ *
+ * @param cfg the configuration
+ * @param commit boolean which indicates whether to commit any changes
+ * done since locking
+ * @return 0 or an error code
+ */
+GIT_EXTERN(int) git_config_unlock(git_config *cfg, int commit);
+
 #endif
diff --git a/src/transaction.c b/src/transaction.c
index e833189..e9639bf 100644
--- a/src/transaction.c
+++ b/src/transaction.c
@@ -12,6 +12,7 @@
 #include "pool.h"
 #include "reflog.h"
 #include "signature.h"
+#include "config.h"
 
 #include "git2/transaction.h"
 #include "git2/signature.h"
@@ -20,6 +21,12 @@
 
 GIT__USE_STRMAP
 
+typedef enum {
+	TRANSACTION_NONE,
+	TRANSACTION_REFS,
+	TRANSACTION_CONFIG,
+} transaction_t;
+
 typedef struct {
 	const char *name;
 	void *payload;
@@ -39,13 +46,29 @@ typedef struct {
 } transaction_node;
 
 struct git_transaction {
+	transaction_t type;
 	git_repository *repo;
 	git_refdb *db;
+	git_config *cfg;
 
 	git_strmap *locks;
 	git_pool pool;
 };
 
+int git_transaction_config_new(git_transaction **out, git_config *cfg)
+{
+	git_transaction *tx;
+	assert(out && cfg);
+
+	tx = git__calloc(1, sizeof(git_transaction));
+	GITERR_CHECK_ALLOC(tx);
+
+	tx->type = TRANSACTION_CONFIG;
+	tx->cfg = cfg;
+	*out = tx;
+	return 0;
+}
+
 int git_transaction_new(git_transaction **out, git_repository *repo)
 {
 	int error;
@@ -71,6 +94,7 @@ int git_transaction_new(git_transaction **out, git_repository *repo)
 	if ((error = git_repository_refdb(&tx->db, repo)) < 0)
 		goto on_error;
 
+	tx->type = TRANSACTION_REFS;
 	memcpy(&tx->pool, &pool, sizeof(git_pool));
 	tx->repo = repo;
 	*out = tx;
@@ -305,6 +329,14 @@ int git_transaction_commit(git_transaction *tx)
 
 	assert(tx);
 
+	if (tx->type == TRANSACTION_CONFIG) {
+		error = git_config_unlock(tx->cfg, true);
+		git_config_free(tx->cfg);
+		tx->cfg = NULL;
+
+		return error;
+	}
+
 	for (pos = kh_begin(tx->locks); pos < kh_end(tx->locks); pos++) {
 		if (!git_strmap_has_data(tx->locks, pos))
 			continue;
@@ -332,6 +364,16 @@ void git_transaction_free(git_transaction *tx)
 
 	assert(tx);
 
+	if (tx->type == TRANSACTION_CONFIG) {
+		if (tx->cfg) {
+			git_config_unlock(tx->cfg, false);
+			git_config_free(tx->cfg);
+		}
+
+		git__free(tx);
+		return;
+	}
+
 	/* start by unlocking the ones we've left hanging, if any */
 	for (pos = kh_begin(tx->locks); pos < kh_end(tx->locks); pos++) {
 		if (!git_strmap_has_data(tx->locks, pos))
diff --git a/src/transaction.h b/src/transaction.h
new file mode 100644
index 0000000..780c068
--- /dev/null
+++ b/src/transaction.h
@@ -0,0 +1,14 @@
+/*
+ * Copyright (C) the libgit2 contributors. All rights reserved.
+ *
+ * This file is part of libgit2, distributed under the GNU GPL v2 with
+ * a Linking Exception. For full terms see the included COPYING file.
+ */
+#ifndef INCLUDE_transaction_h__
+#define INCLUDE_transaction_h__
+
+#include "common.h"
+
+int git_transaction_config_new(git_transaction **out, git_config *cfg);
+
+#endif
diff --git a/tests/config/write.c b/tests/config/write.c
index e43c26b..3d9b1a1 100644
--- a/tests/config/write.c
+++ b/tests/config/write.c
@@ -637,6 +637,7 @@ void test_config_write__locking(void)
 {
 	git_config *cfg, *cfg2;
 	git_config_entry *entry;
+	git_transaction *tx;
 	const char *filename = "locked-file";
 
 	/* Open the config and lock it */
@@ -645,7 +646,7 @@ void test_config_write__locking(void)
 	cl_git_pass(git_config_get_entry(&entry, cfg, "section.name"));
 	cl_assert_equal_s("value", entry->value);
 	git_config_entry_free(entry);
-	cl_git_pass(git_config_lock(cfg));
+	cl_git_pass(git_config_lock(&tx, cfg));
 
 	/* Change entries in the locked backend */
 	cl_git_pass(git_config_set_string(cfg, "section.name", "other value"));
@@ -665,8 +666,8 @@ void test_config_write__locking(void)
 	git_config_entry_free(entry);
 	cl_git_fail_with(GIT_ENOTFOUND, git_config_get_entry(&entry, cfg, "section2.name3"));
 
-	git_config_unlock(cfg, true);
-	git_config_free(cfg);
+	cl_git_pass(git_transaction_commit(tx));
+	git_transaction_free(tx);
 
 	/* Now that we've unlocked it, we should see both updates */
 	cl_git_pass(git_config_open_ondisk(&cfg, filename));