From 5334758ba7309438686c779e4ff943a5f84b6868 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 1 Aug 2016 10:43:49 -0400 Subject: [PATCH] repo: Make ostree_repo_create() nonfatal on existing repos In general we want to support "idempotentcy" or "state synchronization" across interruption. If a repo is only partially created due to a crash or whatever, it's hard for a user to know that. Let's just make `ostree_repo_create()` idempotent. Since all we're doing is a set of `mkdirat()` invocations, it's quite simple. This also involved porting to fd-relative, which IMO makes the code a lot clearer. Closes: #422 Approved by: 14rcole --- src/libostree/ostree-repo.c | 110 +++++++++++++++++------------------- tests/basic-test.sh | 8 ++- 2 files changed, 60 insertions(+), 58 deletions(-) diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 89d976b2..c48c139e 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -1842,8 +1842,15 @@ ostree_repo_mode_from_string (const char *mode, * @cancellable: Cancellable * @error: Error * - * Create the underlying structure on disk for the - * repository. + * Create the underlying structure on disk for the repository, and call + * ostree_repo_open() on the result, preparing it for use. + + * Since version 2016.8, this function will succeed on an existing + * repository, and finish creating any necessary files in a partially + * created repository. However, this function cannot change the mode + * of an existing repository, and will silently ignore an attempt to + * do so. + * */ gboolean ostree_repo_create (OstreeRepo *self, @@ -1851,76 +1858,65 @@ ostree_repo_create (OstreeRepo *self, GCancellable *cancellable, GError **error) { - gboolean ret = FALSE; - GString *config_data = NULL; - g_autoptr(GFile) child = NULL; - g_autoptr(GFile) grandchild = NULL; - const char *mode_str; + const char *repopath = gs_file_get_path_cached (self->repodir); + glnx_fd_close int dfd = -1; + struct stat stbuf; + const char *state_dirs[] = { "objects", "tmp", "extensions", "state", + "refs", "refs/heads", "refs/remotes" }; - if (!ostree_repo_mode_to_string (mode, &mode_str, error)) - goto out; - - if (mkdir (gs_file_get_path_cached (self->repodir), 0755) != 0) + if (mkdir (repopath, 0755) != 0) { - if (errno != EEXIST) + if (G_UNLIKELY (errno != EEXIST)) { glnx_set_error_from_errno (error); - goto out; + return FALSE; } } - config_data = g_string_new (DEFAULT_CONFIG_CONTENTS); - g_string_append_printf (config_data, "mode=%s\n", mode_str); + if (!glnx_opendirat (AT_FDCWD, repopath, TRUE, &dfd, error)) + return FALSE; - if (!g_file_replace_contents (self->config_file, - config_data->str, - config_data->len, - NULL, FALSE, 0, NULL, - cancellable, error)) - goto out; + if (fstatat (dfd, "config", &stbuf, 0) < 0) + { + if (errno == ENOENT) + { + const char *mode_str; + g_autoptr(GString) config_data = g_string_new (DEFAULT_CONFIG_CONTENTS); - if (!g_file_make_directory (self->objects_dir, cancellable, error)) - goto out; + if (!ostree_repo_mode_to_string (mode, &mode_str, error)) + return FALSE; - if (!g_file_make_directory (self->tmp_dir, cancellable, error)) - goto out; + g_string_append_printf (config_data, "mode=%s\n", mode_str); - { - g_autoptr(GFile) extensions_dir = - g_file_resolve_relative_path (self->repodir, "extensions"); - if (!g_file_make_directory (extensions_dir, cancellable, error)) - goto out; - } + if (!glnx_file_replace_contents_at (dfd, "config", + (guint8*)config_data->str, config_data->len, + 0, cancellable, error)) + return FALSE; + } + else + { + glnx_set_error_from_errno (error); + return FALSE; + } + } - g_clear_object (&child); - child = g_file_get_child (self->repodir, "refs"); - if (!g_file_make_directory (child, cancellable, error)) - goto out; - - g_clear_object (&grandchild); - grandchild = g_file_get_child (child, "heads"); - if (!g_file_make_directory (grandchild, cancellable, error)) - goto out; - - g_clear_object (&grandchild); - grandchild = g_file_get_child (child, "remotes"); - if (!g_file_make_directory (grandchild, cancellable, error)) - goto out; - - g_clear_object (&child); - child = g_file_get_child (self->repodir, "state"); - if (!g_file_make_directory (child, cancellable, error)) - goto out; + for (guint i = 0; i < G_N_ELEMENTS (state_dirs); i++) + { + const char *elt = state_dirs[i]; + if (mkdirat (dfd, elt, 0755) == -1) + { + if (G_UNLIKELY (errno != EEXIST)) + { + glnx_set_error_from_errno (error); + return FALSE; + } + } + } if (!ostree_repo_open (self, cancellable, error)) - goto out; + return FALSE; - ret = TRUE; - - out: - if (config_data) - g_string_free (config_data, TRUE); - return ret; + return TRUE; } static gboolean diff --git a/tests/basic-test.sh b/tests/basic-test.sh index 003df893..d4c9aaf2 100755 --- a/tests/basic-test.sh +++ b/tests/basic-test.sh @@ -19,7 +19,7 @@ set -euo pipefail -echo "1..57" +echo "1..58" $OSTREE checkout test2 checkout-test2 echo "ok checkout" @@ -41,6 +41,12 @@ echo "ok shortened checksum" (cd repo && ${CMD_PREFIX} ostree rev-parse test2) echo "ok repo-in-cwd" +rm test-repo -rf +$OSTREE --repo=test-repo init --mode=bare-user +$OSTREE --repo=test-repo init --mode=bare-user +rm test-repo -rf +echo "ok repo-init on existing repo" + cd checkout-test2 assert_has_file firstfile assert_has_file baz/cow