[PATCH v3] worktree repair: detect relative path in .git file correctly

From: Yoichi NAKAYAMA <yoichi.nakayama@gmail.com>

Given a state in which the cross-references between the worktree and the repository (specifically worktree/id/gitdir in the main repository and the .git file in the worktree) are recorded using absolute paths, setting 'worktree.useRelativePaths=true' and running 'git worktree repair' within the main worktree converts them to relative paths.

Conversely, given a state in which the cross-references are recorded using relative paths, one would expect that setting 'worktree.useRelativePaths=false' and running 'git worktree repair' would convert them to absolute paths. However, they remain as relative paths.

This is because we incorrectly use read_gitfile_gently(), which always returns an absolute path. To fix this, introduce read_gitfile_raw(), which is almost identical to read_gitfile_gently(), but skips checking the existence of the referenced repository and returns the path as-is from the .git file.

Signed-off-by: Yoichi NAKAYAMA <yoichi.nakayama@gmail.com>
---
    worktree repair: detect relative path in .git file correctly

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2205%2Fyoichi%2Fworktree-repair-relative-path-handling-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2205/yoichi/worktree-repair-relative-path-handling-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/2205

Range-diff vs v2:

 1:  5bcf19ef50 ! 1:  1cd25e315e worktree repair: detect relative path in .git file correctly
     @@ Metadata
       ## Commit message ##
          worktree repair: detect relative path in .git file correctly
      
     -    Given a state where the cross references between the worktree and the
     -    repository (specifically worktree/id/gitdir in the main repository and
     -    the .git file in the worktree) are recorded using absolute paths,
     +    Given a state in which the cross-references between the worktree and
     +    the repository (specifically worktree/id/gitdir in the main repository
     +    and the .git file in the worktree) are recorded using absolute paths,
          setting 'worktree.useRelativePaths=true' and running 'git worktree
          repair' within the main worktree converts them to relative paths.
      
     -    On the other hand, given a state where the cross references are
     -    recorded using relative paths, one would expect (by symmetry) that
     -    setting 'worktree.useRelativePath=false' and running 'git worktree
     -    repair' would convert them to absolute paths. However, they remain as
     -    relative paths.
     +    Conversely, given a state in which the cross-references are recorded
     +    using relative paths, one would expect that setting
     +    'worktree.useRelativePaths=false' and running 'git worktree repair'
     +    would convert them to absolute paths. However, they remain as relative
     +    paths.
      
     -    This is because we wrongly use read_gitfile_gently() which always
     -    returns an absolute path. To fix this, introduce read_gitfile_raw()
     -    that is almost same as read_gitfile_gently(), but it skips existence
     -    check of the referenced repository and returns the unmodified path
     -    read from .git file.
     +    This is because we incorrectly use read_gitfile_gently(), which always
     +    returns an absolute path. To fix this, introduce read_gitfile_raw(),
     +    which is almost identical to read_gitfile_gently(), but skips checking
     +    the existence of the referenced repository and returns the path as-is
     +    from the .git file.
      
          Signed-off-by: Yoichi NAKAYAMA <yoichi.nakayama@gmail.com>
      
       ## setup.c ##
      @@ setup.c: void read_gitfile_error_die(int error_code, const char *path)
     +  * cases).
        */
       const char *read_gitfile_gently(const char *path, int *return_error_code)
     - {
     --	const int max_file_size = 1 << 20;  /* 1MB */
     - 	int error_code = 0;
     - 	char *buf = NULL;
     --	char *dir = NULL;
     - 	const char *slash;
     ++{
     ++	int error_code = 0;
     ++	const char *slash;
     ++	struct strbuf contents = STRBUF_INIT;
      +	static struct strbuf realpath = STRBUF_INIT;
      +
     -+	buf = xstrdup_or_null(read_gitfile_raw(path, &error_code));
     ++	error_code = read_gitfile_raw(&contents, path);
      +	if (error_code)
      +		goto cleanup_return;
      +
     -+	if (!is_absolute_path(buf) && (slash = strrchr(path, '/'))) {
     ++	if (!is_absolute_path(contents.buf) && (slash = strrchr(path, '/'))) {
      +		size_t pathlen = slash+1 - path;
     -+		char *dir = xstrfmt("%.*s%.*s", (int)pathlen, path,
     -+				    (int)strlen(buf), buf);
     -+		free(buf);
     -+		buf = dir;
     ++		char *dir = xstrfmt("%.*s%s", (int)pathlen, path, contents.buf);
     ++		strbuf_reset(&contents);
     ++		strbuf_addstr(&contents, dir);
     ++		free(dir);
      +	}
     -+	if (!is_git_directory(buf)) {
     ++	if (!is_git_directory(contents.buf)) {
      +		error_code = READ_GITFILE_ERR_NOT_A_REPO;
      +		goto cleanup_return;
      +	}
      +
     -+	strbuf_realpath(&realpath, buf, 1);
     ++	strbuf_realpath(&realpath, contents.buf, 1);
      +
      +cleanup_return:
      +	if (return_error_code)
     @@ setup.c: void read_gitfile_error_die(int error_code, const char *path)
      +	else if (error_code)
      +		read_gitfile_error_die(error_code, path);
      +
     -+	free(buf);
     ++	strbuf_release(&contents);
      +	return error_code ? NULL : realpath.buf;
      +}
      +
     -+const char *read_gitfile_raw(const char *path, int *return_error_code)
     -+{
     -+	const int max_file_size = 1 << 20;  /* 1MB */
     -+	int error_code = 0;
     -+	char *buf = NULL;
     ++int read_gitfile_raw(struct strbuf *contents, const char *path)
     + {
     + 	const int max_file_size = 1 << 20;  /* 1MB */
     + 	int error_code = 0;
     + 	char *buf = NULL;
     +-	char *dir = NULL;
     +-	const char *slash;
       	struct stat st;
       	int fd;
       	ssize_t len;
      -	static struct strbuf realpath = STRBUF_INIT;
     -+	static struct strbuf contents = STRBUF_INIT;
       
       	if (stat(path, &st)) {
       		if (errno == ENOENT || errno == ENOTDIR)
     @@ setup.c: const char *read_gitfile_gently(const char *path, int *return_error_cod
      -
      -	strbuf_realpath(&realpath, dir, 1);
      -	path = realpath.buf;
     -+	strbuf_reset(&contents);
     -+	strbuf_add(&contents, buf+8, len-8);
     ++	strbuf_add(contents, buf+8, len-8);
       
       cleanup_return:
      -	if (return_error_code)
     @@ setup.c: const char *read_gitfile_gently(const char *path, int *return_error_cod
      -	else if (error_code)
      -		read_gitfile_error_die(error_code, path);
      -
     -+	*return_error_code = error_code;
       	free(buf);
      -	return error_code ? NULL : path;
     -+	return error_code ? NULL : contents.buf;
     ++	return error_code;
       }
       
       static void apply_gitdir_and_environment(struct repository *repo, const char *path)
     @@ setup.h: int is_nonbare_repository_dir(struct strbuf *path);
       #define READ_GITFILE_ERR_IS_A_DIR 10
       void read_gitfile_error_die(int error_code, const char *path);
       const char *read_gitfile_gently(const char *path, int *return_error_code);
     -+const char *read_gitfile_raw(const char *path, int *return_error_code);
     ++int read_gitfile_raw(struct strbuf *contents, const char *path);
       #define read_gitfile(path) read_gitfile_gently((path), NULL)
       const char *resolve_gitdir_gently(const char *suspect, int *return_error_code);
       #define resolve_gitdir(path) resolve_gitdir_gently((path), NULL)
     @@ t/t2406-worktree-repair.sh: test_expect_success 'repair worktree with relative p
       test_done
      
       ## worktree.c ##
     +@@ worktree.c: static void repair_gitfile(struct worktree *wt,
     + 	struct strbuf gitdir = STRBUF_INIT;
     + 	struct strbuf repo = STRBUF_INIT;
     + 	struct strbuf backlink = STRBUF_INIT;
     +-	char *dotgit_contents = NULL;
     ++	struct strbuf contents = STRBUF_INIT;
     ++	const char *dotgit_contents = NULL;
     + 	const char *repair = NULL;
     + 	char *path = NULL;
     + 	int err;
      @@ worktree.c: static void repair_gitfile(struct worktree *wt,
       	strbuf_realpath(&repo, path, 1);
       	strbuf_addf(&dotgit, "%s/.git", wt->path);
       	strbuf_addf(&gitdir, "%s/gitdir", repo.buf);
      -	dotgit_contents = xstrdup_or_null(read_gitfile_gently(dotgit.buf, &err));
     -+	dotgit_contents = xstrdup_or_null(read_gitfile_raw(dotgit.buf, &err));
     ++	err = read_gitfile_raw(&contents, dotgit.buf);
     ++	if (!err)
     ++		dotgit_contents = contents.buf;
       
       	if (dotgit_contents) {
       		if (is_absolute_path(dotgit_contents)) {
     @@ worktree.c: static void repair_gitfile(struct worktree *wt,
       		repair = _(".git file broken");
       	else if (fspathcmp(backlink.buf, repo.buf))
       		repair = _(".git file incorrect");
     +@@ worktree.c: static void repair_gitfile(struct worktree *wt,
     + 	}
     + 
     + done:
     +-	free(dotgit_contents);
     + 	free(path);
     + 	strbuf_release(&repo);
     + 	strbuf_release(&dotgit);
     + 	strbuf_release(&gitdir);
     + 	strbuf_release(&backlink);
     ++	strbuf_release(&contents);
     + }
     + 
     + static void repair_noop(int iserr UNUSED,
      @@ worktree.c: void repair_worktree_at_path(struct repository *repo,
       	strbuf_realpath_forgiving(&inferred_backlink, inferred_backlink.buf, 0);
       	dotgit_contents = xstrdup_or_null(read_gitfile_gently(dotgit.buf, &err));

 setup.c                    | 63 ++++++++++++++++++++++----------------
 setup.h                    |  1 +
 t/t2406-worktree-repair.sh | 54 ++++++++++++++++++++++++--------
 worktree.c                 | 20 +++++-------
 4 files changed, 88 insertions(+), 50 deletions(-)