diff options
author | Carlos Martín Nieto <cmn@dwim.me> | 2015-06-01 19:17:03 +0200 |
---|---|---|
committer | Carlos Martín Nieto <cmn@dwim.me> | 2015-08-12 04:09:09 +0200 |
commit | b1667039640ba3464ea0e2d13ad28c9244d80b4d (patch) | |
tree | e394808939d7545258ee1a757358a286d281a124 | |
parent | 3ce9e4d23572718deeab438ce149013eece57371 (diff) | |
download | libgit2-b1667039640ba3464ea0e2d13ad28c9244d80b4d.tar.gz |
config: implement basic transactional support
When a configuration file is locked, any updates made to it will be done
to the in-memory copy of the file. This allows for multiple updates to
happen while we hold the lock, preventing races during complex
config-file manipulation.
-rw-r--r-- | include/git2/sys/config.h | 14 | ||||
-rw-r--r-- | src/config_file.c | 93 | ||||
-rw-r--r-- | src/config_file.h | 10 | ||||
-rw-r--r-- | tests/config/write.c | 46 |
4 files changed, 151 insertions, 12 deletions
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_file.c b/src/config_file.c index fd03c200f..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; @@ -1801,15 +1860,19 @@ static int config_write(diskfile_backend *cfg, const char *key, const regex_t *p 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) { @@ -1840,20 +1903,26 @@ static int config_write(diskfile_backend *cfg, const char *key, const regex_t *p git__free(section); if (result < 0) { - git_buf_free(&buf); git_filebuf_cleanup(&file); goto done; } - git_filebuf_write(&file, git_buf_cstr(&buf), git_buf_len(&buf)); + 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)); - /* refresh stats - if this errors, then commit will error too */ - (void)git_filebuf_stats(&reader->file_mtime, &reader->file_size, &file); + /* 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); - git_buf_free(&reader->buffer); + 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/tests/config/write.c b/tests/config/write.c index 2e7b8182a..5446b95c3 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,46 @@ void test_config_write__to_file_with_only_comment(void) git_buf_free(&result); } +void test_config_write__locking(void) +{ + git_config_backend *cfg, *cfg2; + git_config_entry *entry; + 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_file__ondisk(&cfg, filename)); + cl_git_pass(git_config_file_open(cfg, GIT_CONFIG_LEVEL_APP)); + cl_git_pass(git_config_file_get_string(&entry, cfg, "section.name")); + cl_assert_equal_s("value", entry->value); + git_config_entry_free(entry); + cl_git_pass(git_config_file_lock(cfg)); + + /* Change entries in the locked backend */ + cl_git_pass(git_config_file_set_string(cfg, "section.name", "other value")); + cl_git_pass(git_config_file_set_string(cfg, "section2.name3", "more value")); + + /* We can see that the file we read from hasn't changed */ + cl_git_pass(git_config_file__ondisk(&cfg2, filename)); + cl_git_pass(git_config_file_open(cfg2, GIT_CONFIG_LEVEL_APP)); + cl_git_pass(git_config_file_get_string(&entry, cfg2, "section.name")); + cl_assert_equal_s("value", entry->value); + git_config_entry_free(entry); + cl_git_fail_with(GIT_ENOTFOUND, git_config_file_get_string(&entry, cfg2, "section2.name3")); + git_config_file_free(cfg2); + + git_config_file_unlock(cfg, true); + git_config_file_free(cfg); + + /* Now that we've unlocked it, we should see both updates */ + cl_git_pass(git_config_file__ondisk(&cfg, filename)); + cl_git_pass(git_config_file_open(cfg, GIT_CONFIG_LEVEL_APP)); + cl_git_pass(git_config_file_get_string(&entry, cfg, "section.name")); + cl_assert_equal_s("other value", entry->value); + git_config_entry_free(entry); + cl_git_pass(git_config_file_get_string(&entry, cfg, "section2.name3")); + cl_assert_equal_s("more value", entry->value); + git_config_entry_free(entry); + + git_config_file_free(cfg); +} |