summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorEdward Thomson <ethomson@edwardthomson.com>2015-08-13 10:22:50 -0700
committerEdward Thomson <ethomson@edwardthomson.com>2015-08-13 10:22:50 -0700
commit9f1af7f279e9f5cbd03c9a9d78fcba77a1c2c64c (patch)
tree5250a876c2fe2ba49edcd7e662214baa53dd1f0f
parent1573acbbd9023b5c447caa7489f9579d8ffc810e (diff)
parent5340d63d3815ddbd1a7e1b5b9628fce10089e8a0 (diff)
downloadlibgit2-9f1af7f279e9f5cbd03c9a9d78fcba77a1c2c64c.tar.gz
Merge pull request #3168 from libgit2/cmn/config-tx
Locking and transactional/atomic updates for config
-rw-r--r--CHANGELOG.md9
-rw-r--r--include/git2/config.h18
-rw-r--r--include/git2/sys/config.h14
-rw-r--r--src/config.c35
-rw-r--r--src/config.h15
-rw-r--r--src/config_file.c109
-rw-r--r--src/config_file.h10
-rw-r--r--src/transaction.c42
-rw-r--r--src/transaction.h14
-rw-r--r--tests/config/write.c50
10 files changed, 298 insertions, 18 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 243b696d7..5dd4b3447 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,6 +9,11 @@ v0.23 + 1
### API additions
+* `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
### Breaking API changes
@@ -19,6 +24,10 @@ v0.23 + 1
with the reflog on ref deletion. The file-based backend must delete
it, a database-backed one may wish to archive it.
+* `git_config_backend` has gained two entries. `lock` and `unlock`
+ with which to implement the transactional/atomic semantics for the
+ configuration backend.
+
v0.23
------
diff --git a/include/git2/config.h b/include/git2/config.h
index 6d3fdb0c2..05c3ad622 100644
--- a/include/git2/config.h
+++ b/include/git2/config.h
@@ -689,6 +689,24 @@ GIT_EXTERN(int) git_config_backend_foreach_match(
void *payload);
+/**
+ * Lock the backend with the highest priority
+ *
+ * Locking disallows anybody else from writing to that backend. Any
+ * updates made after locking will not be visible to a reader until
+ * the file is unlocked.
+ *
+ * You can apply the changes by calling `git_transaction_commit()`
+ * before freeing the transaction. Either of these actions will unlock
+ * the config.
+ *
+ * @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_lock(git_transaction **tx, git_config *cfg);
+
/** @} */
GIT_END_DECL
#endif
diff --git a/include/git2/sys/config.h b/include/git2/sys/config.h
index 044e34417..4dad6da42 100644
--- a/include/git2/sys/config.h
+++ b/include/git2/sys/config.h
@@ -67,6 +67,20 @@ struct git_config_backend {
int (*iterator)(git_config_iterator **, struct git_config_backend *);
/** Produce a read-only version of this backend */
int (*snapshot)(struct git_config_backend **, struct git_config_backend *);
+ /**
+ * Lock this backend.
+ *
+ * Prevent any writes to the data store backing this
+ * backend. Any updates must not be visible to any other
+ * readers.
+ */
+ int (*lock)(struct git_config_backend *);
+ /**
+ * Unlock the data store backing this backend. If success is
+ * true, the changes should be committed, otherwise rolled
+ * back.
+ */
+ int (*unlock)(struct git_config_backend *, int success);
void (*free)(struct git_config_backend *);
};
#define GIT_CONFIG_BACKEND_VERSION 1
diff --git a/src/config.c b/src/config.c
index 77cf573e6..2df1fc80e 100644
--- a/src/config.c
+++ b/src/config.c
@@ -1144,6 +1144,41 @@ int git_config_open_default(git_config **out)
return error;
}
+int git_config_lock(git_transaction **out, git_config *cfg)
+{
+ int error;
+ git_config_backend *file;
+ file_internal *internal;
+
+ internal = git_vector_get(&cfg->files, 0);
+ if (!internal || !internal->file) {
+ giterr_set(GITERR_CONFIG, "cannot lock; the config has no backends/files");
+ return -1;
+ }
+ file = internal->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)
+{
+ git_config_backend *file;
+ file_internal *internal;
+
+ internal = git_vector_get(&cfg->files, 0);
+ if (!internal || !internal->file) {
+ giterr_set(GITERR_CONFIG, "cannot lock; the config has no backends/files");
+ return -1;
+ }
+
+ file = internal->file;
+
+ return file->unlock(file, commit);
+}
+
/***********
* Parsers
***********/
diff --git a/src/config.h b/src/config.h
index f257cc90f..ba745331a 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/config_file.c b/src/config_file.c
index 52a5376bd..a3fec1b34 100644
--- a/src/config_file.c
+++ b/src/config_file.c
@@ -105,6 +105,10 @@ typedef struct {
git_array_t(struct reader) readers;
+ bool locked;
+ git_filebuf locked_buf;
+ git_buf locked_content;
+
char *file_path;
} diskfile_backend;
@@ -685,6 +689,42 @@ static int config_snapshot(git_config_backend **out, git_config_backend *in)
return git_config_file__snapshot(out, b);
}
+static int config_lock(git_config_backend *_cfg)
+{
+ diskfile_backend *cfg = (diskfile_backend *) _cfg;
+ int error;
+
+ if ((error = git_filebuf_open(&cfg->locked_buf, cfg->file_path, 0, GIT_CONFIG_FILE_MODE)) < 0)
+ return error;
+
+ error = git_futils_readbuffer(&cfg->locked_content, cfg->file_path);
+ if (error < 0 && error != GIT_ENOTFOUND) {
+ git_filebuf_cleanup(&cfg->locked_buf);
+ return error;
+ }
+
+ cfg->locked = true;
+ return 0;
+
+}
+
+static int config_unlock(git_config_backend *_cfg, int success)
+{
+ diskfile_backend *cfg = (diskfile_backend *) _cfg;
+ int error = 0;
+
+ if (success) {
+ git_filebuf_write(&cfg->locked_buf, cfg->locked_content.ptr, cfg->locked_content.size);
+ error = git_filebuf_commit(&cfg->locked_buf);
+ }
+
+ git_filebuf_cleanup(&cfg->locked_buf);
+ git_buf_free(&cfg->locked_content);
+ cfg->locked = false;
+
+ return error;
+}
+
int git_config_file__ondisk(git_config_backend **out, const char *path)
{
diskfile_backend *backend;
@@ -706,6 +746,8 @@ int git_config_file__ondisk(git_config_backend **out, const char *path)
backend->header.parent.del_multivar = config_delete_multivar;
backend->header.parent.iterator = config_iterator_new;
backend->header.parent.snapshot = config_snapshot;
+ backend->header.parent.lock = config_lock;
+ backend->header.parent.unlock = config_unlock;
backend->header.parent.free = backend_free;
*out = (git_config_backend *)backend;
@@ -750,6 +792,21 @@ static int config_delete_readonly(git_config_backend *cfg, const char *name)
return config_error_readonly();
}
+static int config_lock_readonly(git_config_backend *_cfg)
+{
+ GIT_UNUSED(_cfg);
+
+ return config_error_readonly();
+}
+
+static int config_unlock_readonly(git_config_backend *_cfg, int success)
+{
+ GIT_UNUSED(_cfg);
+ GIT_UNUSED(success);
+
+ return config_error_readonly();
+}
+
static void backend_readonly_free(git_config_backend *_backend)
{
diskfile_backend *backend = (diskfile_backend *)_backend;
@@ -803,6 +860,8 @@ int git_config_file__snapshot(git_config_backend **out, diskfile_backend *in)
backend->header.parent.del = config_delete_readonly;
backend->header.parent.del_multivar = config_delete_multivar_readonly;
backend->header.parent.iterator = config_iterator_new;
+ backend->header.parent.lock = config_lock_readonly;
+ backend->header.parent.unlock = config_unlock_readonly;
backend->header.parent.free = backend_readonly_free;
*out = (git_config_backend *)backend;
@@ -1602,7 +1661,7 @@ static int config_read(git_strmap *values, diskfile_backend *cfg_file, struct re
return config_parse(reader, NULL, read_on_variable, NULL, NULL, &parse_data);
}
-static int write_section(git_filebuf *file, const char *key)
+static int write_section(git_buf *fbuf, const char *key)
{
int result;
const char *dot;
@@ -1626,7 +1685,7 @@ static int write_section(git_filebuf *file, const char *key)
if (git_buf_oom(&buf))
return -1;
- result = git_filebuf_write(file, git_buf_cstr(&buf), buf.size);
+ result = git_buf_put(fbuf, git_buf_cstr(&buf), buf.size);
git_buf_free(&buf);
return result;
@@ -1651,7 +1710,7 @@ static const char *quotes_for_value(const char *value)
}
struct write_data {
- git_filebuf *file;
+ git_buf *buf;
unsigned int in_section : 1,
preg_replaced : 1;
const char *section;
@@ -1662,10 +1721,10 @@ struct write_data {
static int write_line(struct write_data *write_data, const char *line, size_t line_len)
{
- int result = git_filebuf_write(write_data->file, line, line_len);
+ int result = git_buf_put(write_data->buf, line, line_len);
if (!result && line_len && line[line_len-1] != '\n')
- result = git_filebuf_printf(write_data->file, "\n");
+ result = git_buf_printf(write_data->buf, "\n");
return result;
}
@@ -1676,7 +1735,7 @@ static int write_value(struct write_data *write_data)
int result;
q = quotes_for_value(write_data->value);
- result = git_filebuf_printf(write_data->file,
+ result = git_buf_printf(write_data->buf,
"\t%s = %s%s%s\n", write_data->name, q, write_data->value, q);
/* If we are updating a single name/value, we're done. Setting `value`
@@ -1782,7 +1841,7 @@ static int write_on_eof(struct reader **reader, void *data)
* value.
*/
if ((!write_data->preg || !write_data->preg_replaced) && write_data->value) {
- if ((result = write_section(write_data->file, write_data->section)) == 0)
+ if ((result = write_section(write_data->buf, write_data->section)) == 0)
result = write_value(write_data);
}
@@ -1797,18 +1856,23 @@ static int config_write(diskfile_backend *cfg, const char *key, const regex_t *p
int result;
char *section, *name, *ldot;
git_filebuf file = GIT_FILEBUF_INIT;
+ git_buf buf = GIT_BUF_INIT;
struct reader *reader = git_array_get(cfg->readers, 0);
struct write_data write_data;
- /* Lock the file */
- if ((result = git_filebuf_open(
- &file, cfg->file_path, 0, GIT_CONFIG_FILE_MODE)) < 0) {
+ if (cfg->locked) {
+ result = git_buf_puts(&reader->buffer, git_buf_cstr(&cfg->locked_content));
+ } else {
+ /* Lock the file */
+ if ((result = git_filebuf_open(
+ &file, cfg->file_path, 0, GIT_CONFIG_FILE_MODE)) < 0) {
git_buf_free(&reader->buffer);
return result;
- }
+ }
- /* We need to read in our own config file */
- result = git_futils_readbuffer(&reader->buffer, cfg->file_path);
+ /* We need to read in our own config file */
+ result = git_futils_readbuffer(&reader->buffer, cfg->file_path);
+ }
/* Initialise the reading position */
if (result == GIT_ENOTFOUND) {
@@ -1827,7 +1891,7 @@ static int config_write(diskfile_backend *cfg, const char *key, const regex_t *p
name = ldot + 1;
section = git__strndup(key, ldot - key);
- write_data.file = &file;
+ write_data.buf = &buf;
write_data.section = section;
write_data.in_section = 0;
write_data.preg_replaced = 0;
@@ -1843,13 +1907,22 @@ static int config_write(diskfile_backend *cfg, const char *key, const regex_t *p
goto done;
}
- /* refresh stats - if this errors, then commit will error too */
- (void)git_filebuf_stats(&reader->file_mtime, &reader->file_size, &file);
+ if (cfg->locked) {
+ size_t len = buf.asize;
+ /* Update our copy with the modified contents */
+ git_buf_free(&cfg->locked_content);
+ git_buf_attach(&cfg->locked_content, git_buf_detach(&buf), len);
+ } else {
+ git_filebuf_write(&file, git_buf_cstr(&buf), git_buf_len(&buf));
- result = git_filebuf_commit(&file);
- git_buf_free(&reader->buffer);
+ /* refresh stats - if this errors, then commit will error too */
+ (void)git_filebuf_stats(&reader->file_mtime, &reader->file_size, &file);
+
+ result = git_filebuf_commit(&file);
+ }
done:
+ git_buf_free(&buf);
git_buf_free(&reader->buffer);
return result;
}
diff --git a/src/config_file.h b/src/config_file.h
index 0d8bf740f..1c52892c3 100644
--- a/src/config_file.h
+++ b/src/config_file.h
@@ -55,6 +55,16 @@ GIT_INLINE(int) git_config_file_foreach_match(
return git_config_backend_foreach_match(cfg, regexp, fn, data);
}
+GIT_INLINE(int) git_config_file_lock(git_config_backend *cfg)
+{
+ return cfg->lock(cfg);
+}
+
+GIT_INLINE(int) git_config_file_unlock(git_config_backend *cfg, int success)
+{
+ return cfg->unlock(cfg, success);
+}
+
extern int git_config_file_normalize_section(char *start, char *end);
#endif
diff --git a/src/transaction.c b/src/transaction.c
index e8331891c..e9639bf97 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 000000000..780c06830
--- /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 2e7b8182a..3d9b1a16a 100644
--- a/tests/config/write.c
+++ b/tests/config/write.c
@@ -1,6 +1,9 @@
#include "clar_libgit2.h"
#include "buffer.h"
#include "fileops.h"
+#include "git2/sys/config.h"
+#include "config_file.h"
+#include "config.h"
void test_config_write__initialize(void)
{
@@ -630,3 +633,50 @@ void test_config_write__to_file_with_only_comment(void)
git_buf_free(&result);
}
+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 */
+ cl_git_mkfile(filename, "[section]\n\tname = value\n");
+ cl_git_pass(git_config_open_ondisk(&cfg, filename));
+ 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(&tx, cfg));
+
+ /* Change entries in the locked backend */
+ cl_git_pass(git_config_set_string(cfg, "section.name", "other value"));
+ cl_git_pass(git_config_set_string(cfg, "section2.name3", "more value"));
+
+ /* We can see that the file we read from hasn't changed */
+ cl_git_pass(git_config_open_ondisk(&cfg2, filename));
+ cl_git_pass(git_config_get_entry(&entry, cfg2, "section.name"));
+ cl_assert_equal_s("value", entry->value);
+ git_config_entry_free(entry);
+ cl_git_fail_with(GIT_ENOTFOUND, git_config_get_entry(&entry, cfg2, "section2.name3"));
+ git_config_free(cfg2);
+
+ /* And we also get the old view when we read from the locked config */
+ 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_fail_with(GIT_ENOTFOUND, git_config_get_entry(&entry, cfg, "section2.name3"));
+
+ 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));
+ cl_git_pass(git_config_get_entry(&entry, cfg, "section.name"));
+ cl_assert_equal_s("other value", entry->value);
+ git_config_entry_free(entry);
+ cl_git_pass(git_config_get_entry(&entry, cfg, "section2.name3"));
+ cl_assert_equal_s("more value", entry->value);
+ git_config_entry_free(entry);
+
+ git_config_free(cfg);
+}