On 12/19, Stefan Beller wrote:
> Relocating a git directory consists of 3 steps:
> 1) Move the directory.
> 2) Update the gitlink file.
> 3) Set core.worktree correctly.
>
> In an ideal world all three steps would be part of one transaction, such
> that either all of them happen correctly or none of them.
> However currently we just execute these three steps sequentially and die
> in case of an error in any of these 3 steps.
>
> Dying is ok in 1) as the transaction hasn't started and the state is
> recoverable.
>
> When dying in 2), this is a problem as the repo state is left in an
> inconsistent state, e.g. the git link file of a submodule could be
> empty and hence even the superproject appears to be broken as basic
> commands such as git-status would die as there is it cannot tell the
> state of the submodule.
> So in that case try to undo 1) to be in a less sufferable state.
I now see why an atomic filesystem swap operation would be useful :)
>
> 3) is less of an issue as experiments with submodules show. When
> core.worktree is unset or set incorrectly, git-status still works
> both in the superproject as well as the working tree of the submodule.
>
> Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx>
> ---
> dir.c | 94 +++++++++++++++++++++++++++++++++++++++++++++++++++++++------
> dir.h | 6 ++--
> submodule.c | 3 +-
> 3 files changed, 91 insertions(+), 12 deletions(-)
>
> diff --git a/dir.c b/dir.c
> index b2cb23fe88..e4e3f69869 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -2749,30 +2749,66 @@ void untracked_cache_add_to_index(struct index_state *istate,
> untracked_cache_invalidate_path(istate, path);
> }
>
> -static void point_gitlink_file_to(const char *work_tree, const char *git_dir)
> +/*
> + * Just like write_file, we try hard to write the full content to the file.
> + * If there is suspicion the write did not work correctly, make sure the file
> + * is removed again.
> + * Return 0 if the write succeeded, -1 if the file was removed,
> + * -2 if the (partial) file is still there.
> + */
> +static int write_file_or_remove(const char *path, const char *buf, size_t len)
> +{
> + int retries = 3;
> + int fd = xopen(path, O_WRONLY | O_CREAT | O_TRUNC, 0666);
> + if (write_in_full(fd, buf, len) != len) {
> + warning_errno(_("could not write '%s'"), path);
> + goto err;
> + }
> + if (close(fd)) {
> + warning_errno(_("could not close '%s'"), path);
> + goto err;
> + }
> + return 0;
> +err:
> + while (retries-- > 0) {
> + if (file_exists(path))
> + unlink_or_warn(path);
> + else
> + return -1;
> + }
> + return -2;
> +}
Any reason why on attempt 2 or 3 this would succeed if it failed on try
1?
> +
> +static int point_gitlink_file_to(const char *work_tree, const char *git_dir)
> {
> struct strbuf file_name = STRBUF_INIT;
> struct strbuf rel_path = STRBUF_INIT;
> + struct strbuf content = STRBUF_INIT;
> + int ret;
>
> strbuf_addf(&file_name, "%s/.git", work_tree);
> - write_file(file_name.buf, "gitdir: %s",
> - relative_path(git_dir, work_tree, &rel_path));
> + strbuf_addf(&content, "gitdir: %s",
> + relative_path(git_dir, work_tree, &rel_path));
> + ret = write_file_or_remove(file_name.buf, content.buf, content.len);
>
> strbuf_release(&file_name);
> strbuf_release(&rel_path);
> + return ret;
> }
>
> -static void set_core_work_tree_to_connect(const char *work_tree, const char *git_dir)
> +static int set_core_work_tree_to_connect(const char *work_tree, const char *git_dir)
> {
> struct strbuf file_name = STRBUF_INIT;
> struct strbuf rel_path = STRBUF_INIT;
> + int ret;
>
> strbuf_addf(&file_name, "%s/config", git_dir);
> - git_config_set_in_file(file_name.buf, "core.worktree",
> + ret = git_config_set_in_file_gently(file_name.buf, "core.worktree",
> relative_path(work_tree, git_dir, &rel_path));
>
> strbuf_release(&file_name);
> strbuf_release(&rel_path);
> + return ret;
> }
>
> /* Update gitfile and core.worktree setting to connect work tree and git dir */
> @@ -2790,12 +2826,54 @@ void connect_work_tree_and_git_dir(const char *work_tree_, const char *git_dir_)
>
> /*
> * Migrate the git directory of the given path from old_git_dir to new_git_dir.
> + * Return 0 on success and -1 on a minor issue. Die in case the repository is
> + * fatally messed up.
> */
> -void relocate_gitdir(const char *path, const char *old_git_dir, const char *new_git_dir)
> +int relocate_gitdir(const char *path, const char *old_git_dir, const char *new_git_dir)
> {
> + char *git_dir = xstrdup(real_path(new_git_dir));
> + char *work_tree = xstrdup(real_path(path));
> + int c;
I guess in a later patch we could clean up these calls to real_path to
use real_pathdup from bw/realpath-wo-chdir.
> +
> if (rename(old_git_dir, new_git_dir) < 0)
> - die_errno(_("could not migrate git directory from '%s' to '%s'"),
> + die_errno(_("could not rename git directory from '%s' to '%s'"),
> old_git_dir, new_git_dir);
>
> - connect_work_tree_and_git_dir(path, new_git_dir);
> + c = point_gitlink_file_to(work_tree, git_dir);
> + if (c < 0) {
> + warning(_("tried to move the git directory from '%s' to '%s'"),
> + old_git_dir, new_git_dir);
> + warning(_("problems with creating a .git file in '%s' to point to '%s'"),
> + work_tree, new_git_dir);
> + if (c == -1) {
> + warning(_("try to undo the move"));
> + if (rename(new_git_dir, old_git_dir) < 0)
> + die_errno(_("could not rename git directory from '%s' to '%s'"),
> + new_git_dir, old_git_dir);
> + return -1;
> + } else if (c == -2) {
> + warning(_("The .git file is still there, "
> + "where the undo operation would move the git "
> + "directory"));
> + die(_("failed to undo the git directory relocation"));
> + }
> + };
> +
> + if (set_core_work_tree_to_connect(work_tree, git_dir) < 0) {
> + /*
> + * At this point the git dir was moved and the gitlink file
> + * was updated correctly, this leaves the repository in a
> + * state that is not totally broken. Setting the core.worktree
> + * correctly would have been the last step to perform a
> + * complete git directory relocation, but this is good enough
> + * to not die.
> + */
> + warning(_("could not set core.worktree in '%s' to point at '%s'"),
> + git_dir, work_tree);
> + return -1;
> + }
> +
> + free(work_tree);
> + free(git_dir);
> + return 0;
> }
> diff --git a/dir.h b/dir.h
> index bf23a470af..86f1cf790f 100644
> --- a/dir.h
> +++ b/dir.h
> @@ -336,7 +336,7 @@ void write_untracked_extension(struct strbuf *out, struct untracked_cache *untra
> void add_untracked_cache(struct index_state *istate);
> void remove_untracked_cache(struct index_state *istate);
> extern void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);
> -extern void relocate_gitdir(const char *path,
> - const char *old_git_dir,
> - const char *new_git_dir);
> +extern int relocate_gitdir(const char *path,
> + const char *old_git_dir,
> + const char *new_git_dir);
> #endif
> diff --git a/submodule.c b/submodule.c
> index 45ccfb7ab4..fa1f44bb5a 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -1277,7 +1277,8 @@ static void relocate_single_git_dir_into_superproject(const char *prefix,
> prefix ? prefix : "", path,
> real_old_git_dir, real_new_git_dir);
>
> - relocate_gitdir(path, real_old_git_dir, real_new_git_dir);
> + if (relocate_gitdir(path, real_old_git_dir, real_new_git_dir))
> + die(_("could not relocate git directory of '%s'"), path);
>
> free(old_git_dir);
> free(real_old_git_dir);
> --
> 2.11.0.rc2.53.gb7b3fba.dirty
>
--
Brandon Williams