Message ID | 20241007-wt_relative_paths-v3-2-622cf18c45eb@pm.me (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | Link worktrees with relative paths | expand |
Caleb White via B4 Relay <devnull+cdwhite3.pm.me@kernel.org> writes: > From: Caleb White <cdwhite3@pm.me> > > Git currently stores absolute paths to both the main repository and As always, drop "currently". The usual way to compose a log message of this project is to - Give an observation on how the current system work in the present tense (so no need to say "Currently X is Y", just "X is Y"), and discuss what you perceive as a problem in it. - Propose a solution (optional---often, problem description trivially leads to an obvious solution in reader's minds). - Give commands to the codebase to "become like so". in this order. > linked worktrees. However, this causes problems when moving repositories The above claim is way too strong. When relative references are used, you can move directories without problems only if you move all the worktrees and the repository in unison without breaking their relative locations, which is an exception and not the norm. The repository knows where its extra worktrees are by their absolute paths (and vice versa) to allow discovery of each other. When a directory is moved, "git worktree repair" must be used to adjust these references. It however becomes possible, in a narrow special case, to do without "git worktree repair". When the repository and the worktrees move in parallel without breaking their relative location, the repair operation becomes unneeded if we made them refer to each other using relative paths (think: tarring up both the reposity and the worktrees at the same time, and then untarring the result at a different place). or something, perhaps. > Although Git now writes relative paths, existing repositories with > absolute paths are still supported. There are no breaking changes > to workflows based on absolute paths, ensuring backward compatibility. Good. Being able to work with existing repositories is an absolute requirement. Are there good test cases? I would imagine that you would need to force a worktree and its repository to refer to each other using absolute paths and then try to access with the updated code, perhaps moving one of such "old-style" directory and then using the updated "git worktree repair" and observing the result. To allow such a test possible, you might even need an option to "git worktree" to allow it to create these linkages using absolute paths, not relative (and if you need to keep both possibilities anyway, you might make the newer "relative" layout an optional feature, triggered by a command line option given to "git worktree add" and friends). Have we considered how badly existing third-party tools, that has learned to assume that the paths are aboslute, may break with this change, though? Or is this "we won't know until we inflict it on real users and see who screams"? Thanks.
Hi Caleb On 08/10/2024 04:12, Caleb White via B4 Relay wrote: > From: Caleb White <cdwhite3@pm.me> > > Git currently stores absolute paths to both the main repository and > linked worktrees. However, this causes problems when moving repositories > or working in containerized environments where absolute paths differ > between systems. The worktree links break, and users are required to > manually execute `worktree repair` to repair them, leading to workflow > disruptions. Additionally, mapping repositories inside of containerized > environments renders the repository unusable inside the containers, and > this is not repairable as repairing the worktrees inside the containers > will result in them being broken outside the containers. > > To address this, this patch makes Git always write relative paths when > linking worktrees. Relative paths increase the resilience of the > worktree links across various systems and environments, particularly > when the worktrees are self-contained inside the main repository (such > as when using a bare repository with worktrees). This improves > portability, workflow efficiency, and reduces overall breakages. This is quite a sweeping claim, it would be helpful to describe the trade off that this patch is making. I've not been following the discussion closely but as I understand it while there are cases such as sharing a drive between Windows and linux or moving all the worktrees including the main one together where using relative paths prevents problems there are other cases such as moving a single worktree that are fixable by running "git worktree repair" when using absolute paths but not with relative paths. It was suggested in another thread I saw recently that storing both would be more resiliant. Another possibility would be to store the an absolute path in the worktree's .git file and relative paths in worktrees/*/gitdir. That would enable "git worktree repair" run in a moved worktree to find the main worktree if the main worktree has not been moved as it does now. It would also allow "git worktree repair" run in the main worktree that has been moved to find the linked worktrees that were moved in tandam with it. Best Wishes Phillip > Although Git now writes relative paths, existing repositories with > absolute paths are still supported. There are no breaking changes > to workflows based on absolute paths, ensuring backward compatibility. > > At a low level, the changes involve modifying functions in `worktree.c` > and `builtin/worktree.c` to use `relative_path()` when writing the > worktree’s `.git` file and the main repository’s `gitdir` reference. > Instead of hardcoding absolute paths, Git now computes the relative path > between the worktree and the repository, ensuring that these links are > portable. Locations where these respective file are read have also been > updated to properly handle both absolute and relative paths. Generally, > relative paths are always resolved into absolute paths before any > operations or comparisons are performed. > > Additionally, `repair_worktrees_after_gitdir_move()` has been introduced > to address the case where both the `<worktree>/.git` and > `<repo>/worktrees/<id>/gitdir` links are broken after the gitdir is > moved (such as during a re-initialization). This function repairs both > sides of the worktree link using the old gitdir path to reestablish the > correct paths after a move. > > The `worktree.path` struct member has also been updated to always store > the absolute path of a worktree. This ensures that worktree consumers > never have to worry about trying to resolve the absolute path themselves. > > Signed-off-by: Caleb White <cdwhite3@pm.me> > --- > builtin/worktree.c | 16 ++-- > setup.c | 2 +- > t/t2408-worktree-relative.sh | 39 ++++++++ > worktree.c | 207 ++++++++++++++++++++++++++++++++++--------- > worktree.h | 10 +++ > 5 files changed, 223 insertions(+), 51 deletions(-) > > diff --git a/builtin/worktree.c b/builtin/worktree.c > index fc31d072a620d7b455d7f150bd3a9e773ee9d4ed..dae63dedf4cac2621f51f95a39aa456b33acd894 100644 > --- a/builtin/worktree.c > +++ b/builtin/worktree.c > @@ -414,7 +414,8 @@ static int add_worktree(const char *path, const char *refname, > const struct add_opts *opts) > { > struct strbuf sb_git = STRBUF_INIT, sb_repo = STRBUF_INIT; > - struct strbuf sb = STRBUF_INIT, realpath = STRBUF_INIT; > + struct strbuf sb = STRBUF_INIT, sb_tmp = STRBUF_INIT; > + struct strbuf sb_path_realpath = STRBUF_INIT, sb_repo_realpath = STRBUF_INIT; > const char *name; > struct strvec child_env = STRVEC_INIT; > unsigned int counter = 0; > @@ -490,11 +491,10 @@ static int add_worktree(const char *path, const char *refname, > > strbuf_reset(&sb); > strbuf_addf(&sb, "%s/gitdir", sb_repo.buf); > - strbuf_realpath(&realpath, sb_git.buf, 1); > - write_file(sb.buf, "%s", realpath.buf); > - strbuf_realpath(&realpath, repo_get_common_dir(the_repository), 1); > - write_file(sb_git.buf, "gitdir: %s/worktrees/%s", > - realpath.buf, name); > + strbuf_realpath(&sb_path_realpath, path, 1); > + strbuf_realpath(&sb_repo_realpath, sb_repo.buf, 1); > + write_file(sb.buf, "%s/.git", relative_path(sb_path_realpath.buf, sb_repo_realpath.buf, &sb_tmp)); > + write_file(sb_git.buf, "gitdir: %s", relative_path(sb_repo_realpath.buf, sb_path_realpath.buf, &sb_tmp)); > strbuf_reset(&sb); > strbuf_addf(&sb, "%s/commondir", sb_repo.buf); > write_file(sb.buf, "../.."); > @@ -578,11 +578,13 @@ static int add_worktree(const char *path, const char *refname, > > strvec_clear(&child_env); > strbuf_release(&sb); > + strbuf_release(&sb_tmp); > strbuf_release(&symref); > strbuf_release(&sb_repo); > + strbuf_release(&sb_repo_realpath); > strbuf_release(&sb_git); > + strbuf_release(&sb_path_realpath); > strbuf_release(&sb_name); > - strbuf_release(&realpath); > free_worktree(wt); > return ret; > } > diff --git a/setup.c b/setup.c > index 94e79b2e487f3faa537547e190acf9b7ea0be3b5..7b648de0279116b381eea46800ad130606926103 100644 > --- a/setup.c > +++ b/setup.c > @@ -2420,7 +2420,7 @@ static void separate_git_dir(const char *git_dir, const char *git_link) > > if (rename(src, git_dir)) > die_errno(_("unable to move %s to %s"), src, git_dir); > - repair_worktrees(NULL, NULL); > + repair_worktrees_after_gitdir_move(src); > } > > write_file(git_link, "gitdir: %s", git_dir); > diff --git a/t/t2408-worktree-relative.sh b/t/t2408-worktree-relative.sh > new file mode 100755 > index 0000000000000000000000000000000000000000..a3136db7e28cb20926ff44211e246ce625a6e51a > --- /dev/null > +++ b/t/t2408-worktree-relative.sh > @@ -0,0 +1,39 @@ > +#!/bin/sh > + > +test_description='test worktrees linked with relative paths' > + > +TEST_PASSES_SANITIZE_LEAK=true > +. ./test-lib.sh > + > +test_expect_success 'links worktrees with relative paths' ' > + test_when_finished rm -rf repo && > + git init repo && > + ( > + cd repo && > + test_commit initial && > + git worktree add wt1 && > + echo "../../../wt1/.git" >expected_gitdir && > + cat .git/worktrees/wt1/gitdir >actual_gitdir && > + echo "gitdir: ../.git/worktrees/wt1" >expected_git && > + cat wt1/.git >actual_git && > + test_cmp expected_gitdir actual_gitdir && > + test_cmp expected_git actual_git > + ) > +' > + > +test_expect_success 'move repo without breaking relative internal links' ' > + test_when_finished rm -rf repo moved && > + git init repo && > + ( > + cd repo && > + test_commit initial && > + git worktree add wt1 && > + cd .. && > + mv repo moved && > + cd moved/wt1 && > + git status >out 2>err && > + test_must_be_empty err > + ) > +' > + > +test_done > diff --git a/worktree.c b/worktree.c > index 0cba0d6e6e9ad02ace04a0301104a04a07cbef65..77ff484d3ec48c547ee4e3d958dfa28a52c1eaa7 100644 > --- a/worktree.c > +++ b/worktree.c > @@ -110,6 +110,12 @@ struct worktree *get_linked_worktree(const char *id, > strbuf_rtrim(&worktree_path); > strbuf_strip_suffix(&worktree_path, "/.git"); > > + if (!is_absolute_path(worktree_path.buf)) { > + strbuf_strip_suffix(&path, "gitdir"); > + strbuf_addbuf(&path, &worktree_path); > + strbuf_realpath_forgiving(&worktree_path, path.buf, 0); > + } > + > CALLOC_ARRAY(worktree, 1); > worktree->repo = the_repository; > worktree->path = strbuf_detach(&worktree_path, NULL); > @@ -373,18 +379,29 @@ int validate_worktree(const struct worktree *wt, struct strbuf *errmsg, > void update_worktree_location(struct worktree *wt, const char *path_) > { > struct strbuf path = STRBUF_INIT; > + struct strbuf repo = STRBUF_INIT; > + struct strbuf file = STRBUF_INIT; > + struct strbuf tmp = STRBUF_INIT; > > if (is_main_worktree(wt)) > BUG("can't relocate main worktree"); > > + strbuf_realpath(&repo, git_common_path("worktrees/%s", wt->id), 1); > strbuf_realpath(&path, path_, 1); > if (fspathcmp(wt->path, path.buf)) { > - write_file(git_common_path("worktrees/%s/gitdir", wt->id), > - "%s/.git", path.buf); > + strbuf_addf(&file, "%s/gitdir", repo.buf); > + write_file(file.buf, "%s/.git", relative_path(path.buf, repo.buf, &tmp)); > + strbuf_reset(&file); > + strbuf_addf(&file, "%s/.git", path.buf); > + write_file(file.buf, "gitdir: %s", relative_path(repo.buf, path.buf, &tmp)); > + > free(wt->path); > wt->path = strbuf_detach(&path, NULL); > } > strbuf_release(&path); > + strbuf_release(&repo); > + strbuf_release(&file); > + strbuf_release(&tmp); > } > > int is_worktree_being_rebased(const struct worktree *wt, > @@ -564,38 +581,52 @@ static void repair_gitfile(struct worktree *wt, > { > struct strbuf dotgit = STRBUF_INIT; > struct strbuf repo = STRBUF_INIT; > - char *backlink; > + struct strbuf backlink = STRBUF_INIT; > + struct strbuf tmp = STRBUF_INIT; > + char *dotgit_contents = NULL; > const char *repair = NULL; > int err; > > /* missing worktree can't be repaired */ > if (!file_exists(wt->path)) > - return; > + goto done; > > if (!is_directory(wt->path)) { > fn(1, wt->path, _("not a directory"), cb_data); > - return; > + goto done; > } > > strbuf_realpath(&repo, git_common_path("worktrees/%s", wt->id), 1); > strbuf_addf(&dotgit, "%s/.git", wt->path); > - backlink = xstrdup_or_null(read_gitfile_gently(dotgit.buf, &err)); > + dotgit_contents = xstrdup_or_null(read_gitfile_gently(dotgit.buf, &err)); > + > + if (dotgit_contents) { > + if (is_absolute_path(dotgit_contents)) { > + strbuf_addstr(&backlink, dotgit_contents); > + } else { > + strbuf_addf(&backlink, "%s/%s", wt->path, dotgit_contents); > + strbuf_realpath_forgiving(&backlink, backlink.buf, 0); > + } > + } > > if (err == READ_GITFILE_ERR_NOT_A_FILE) > fn(1, wt->path, _(".git is not a file"), cb_data); > else if (err) > repair = _(".git file broken"); > - else if (fspathcmp(backlink, repo.buf)) > + else if (fspathcmp(backlink.buf, repo.buf)) > repair = _(".git file incorrect"); > > if (repair) { > fn(0, wt->path, repair, cb_data); > - write_file(dotgit.buf, "gitdir: %s", repo.buf); > + write_file(dotgit.buf, "gitdir: %s", relative_path(repo.buf, wt->path, &tmp)); > } > > - free(backlink); > +done: > + free(dotgit_contents); > strbuf_release(&repo); > strbuf_release(&dotgit); > + strbuf_release(&backlink); > + strbuf_release(&tmp); > } > > static void repair_noop(int iserr UNUSED, > @@ -618,6 +649,59 @@ void repair_worktrees(worktree_repair_fn fn, void *cb_data) > free_worktrees(worktrees); > } > > +void repair_worktree_after_gitdir_move(struct worktree *wt, const char *old_path) > +{ > + struct strbuf path = STRBUF_INIT; > + struct strbuf repo = STRBUF_INIT; > + struct strbuf gitdir = STRBUF_INIT; > + struct strbuf dotgit = STRBUF_INIT; > + struct strbuf olddotgit = STRBUF_INIT; > + struct strbuf tmp = STRBUF_INIT; > + > + if (is_main_worktree(wt)) > + goto done; > + > + strbuf_realpath(&repo, git_common_path("worktrees/%s", wt->id), 1); > + strbuf_addf(&gitdir, "%s/gitdir", repo.buf); > + > + if (strbuf_read_file(&olddotgit, gitdir.buf, 0) < 0) > + goto done; > + > + strbuf_rtrim(&olddotgit); > + if (is_absolute_path(olddotgit.buf)) { > + strbuf_addbuf(&dotgit, &olddotgit); > + } else { > + strbuf_addf(&dotgit, "%s/worktrees/%s/%s", old_path, wt->id, olddotgit.buf); > + strbuf_realpath_forgiving(&dotgit, dotgit.buf, 0); > + } > + > + if (!file_exists(dotgit.buf)) > + goto done; > + > + strbuf_addbuf(&path, &dotgit); > + strbuf_strip_suffix(&path, "/.git"); > + > + write_file(dotgit.buf, "gitdir: %s", relative_path(repo.buf, path.buf, &tmp)); > + write_file(gitdir.buf, "%s", relative_path(dotgit.buf, repo.buf, &tmp)); > +done: > + strbuf_release(&path); > + strbuf_release(&repo); > + strbuf_release(&gitdir); > + strbuf_release(&dotgit); > + strbuf_release(&olddotgit); > + strbuf_release(&tmp); > +} > + > +void repair_worktrees_after_gitdir_move(const char *old_path) > +{ > + struct worktree **worktrees = get_worktrees_internal(1); > + struct worktree **wt = worktrees + 1; /* +1 skips main worktree */ > + > + for (; *wt; wt++) > + repair_worktree_after_gitdir_move(*wt, old_path); > + free_worktrees(worktrees); > +} > + > static int is_main_worktree_path(const char *path) > { > struct strbuf target = STRBUF_INIT; > @@ -684,6 +768,8 @@ void repair_worktree_at_path(const char *path, > struct strbuf inferred_backlink = STRBUF_INIT; > struct strbuf gitdir = STRBUF_INIT; > struct strbuf olddotgit = STRBUF_INIT; > + struct strbuf realolddotgit = STRBUF_INIT; > + struct strbuf tmp = STRBUF_INIT; > char *dotgit_contents = NULL; > const char *repair = NULL; > int err; > @@ -701,9 +787,17 @@ void repair_worktree_at_path(const char *path, > } > > infer_backlink(realdotgit.buf, &inferred_backlink); > + strbuf_realpath_forgiving(&inferred_backlink, inferred_backlink.buf, 0); > dotgit_contents = xstrdup_or_null(read_gitfile_gently(realdotgit.buf, &err)); > if (dotgit_contents) { > - strbuf_addstr(&backlink, dotgit_contents); > + if (is_absolute_path(dotgit_contents)) { > + strbuf_addstr(&backlink, dotgit_contents); > + } else { > + strbuf_addbuf(&backlink, &realdotgit); > + strbuf_strip_suffix(&backlink, ".git"); > + strbuf_addstr(&backlink, dotgit_contents); > + strbuf_realpath_forgiving(&backlink, backlink.buf, 0); > + } > } else if (err == READ_GITFILE_ERR_NOT_A_FILE) { > fn(1, realdotgit.buf, _("unable to locate repository; .git is not a file"), cb_data); > goto done; > @@ -721,7 +815,7 @@ void repair_worktree_at_path(const char *path, > fn(1, realdotgit.buf, _("unable to locate repository; .git file does not reference a repository"), cb_data); > goto done; > } > - } else if (err) { > + } else { > fn(1, realdotgit.buf, _("unable to locate repository; .git file broken"), cb_data); > goto done; > } > @@ -753,90 +847,117 @@ void repair_worktree_at_path(const char *path, > repair = _("gitdir unreadable"); > else { > strbuf_rtrim(&olddotgit); > - if (fspathcmp(olddotgit.buf, realdotgit.buf)) > + if (is_absolute_path(olddotgit.buf)) { > + strbuf_addbuf(&realolddotgit, &olddotgit); > + } else { > + strbuf_addf(&realolddotgit, "%s/%s", backlink.buf, olddotgit.buf); > + strbuf_realpath_forgiving(&realolddotgit, realolddotgit.buf, 0); > + } > + if (fspathcmp(realolddotgit.buf, realdotgit.buf)) > repair = _("gitdir incorrect"); > } > > if (repair) { > fn(0, gitdir.buf, repair, cb_data); > - write_file(gitdir.buf, "%s", realdotgit.buf); > + write_file(gitdir.buf, "%s", relative_path(realdotgit.buf, backlink.buf, &tmp)); > } > done: > free(dotgit_contents); > strbuf_release(&olddotgit); > + strbuf_release(&realolddotgit); > strbuf_release(&backlink); > strbuf_release(&inferred_backlink); > strbuf_release(&gitdir); > strbuf_release(&realdotgit); > strbuf_release(&dotgit); > + strbuf_release(&tmp); > } > > int should_prune_worktree(const char *id, struct strbuf *reason, char **wtpath, timestamp_t expire) > { > struct stat st; > - char *path; > + struct strbuf dotgit = STRBUF_INIT; > + struct strbuf gitdir = STRBUF_INIT; > + struct strbuf repo = STRBUF_INIT; > + struct strbuf file = STRBUF_INIT; > + char *path = NULL; > + int rc = 0; > int fd; > size_t len; > ssize_t read_result; > > *wtpath = NULL; > - if (!is_directory(git_path("worktrees/%s", id))) { > + strbuf_realpath(&repo, git_common_path("worktrees/%s", id), 1); > + strbuf_addf(&gitdir, "%s/gitdir", repo.buf); > + if (!is_directory(repo.buf)) { > strbuf_addstr(reason, _("not a valid directory")); > - return 1; > + rc = 1; > + goto done; > } > - if (file_exists(git_path("worktrees/%s/locked", id))) > - return 0; > - if (stat(git_path("worktrees/%s/gitdir", id), &st)) { > + strbuf_addf(&file, "%s/locked", repo.buf); > + if (file_exists(file.buf)) { > + goto done; > + } > + if (stat(gitdir.buf, &st)) { > strbuf_addstr(reason, _("gitdir file does not exist")); > - return 1; > + rc = 1; > + goto done; > } > - fd = open(git_path("worktrees/%s/gitdir", id), O_RDONLY); > + fd = open(gitdir.buf, O_RDONLY); > if (fd < 0) { > strbuf_addf(reason, _("unable to read gitdir file (%s)"), > strerror(errno)); > - return 1; > + rc = 1; > + goto done; > } > len = xsize_t(st.st_size); > path = xmallocz(len); > > read_result = read_in_full(fd, path, len); > + close(fd); > if (read_result < 0) { > strbuf_addf(reason, _("unable to read gitdir file (%s)"), > strerror(errno)); > - close(fd); > - free(path); > - return 1; > - } > - close(fd); > - > - if (read_result != len) { > + rc = 1; > + goto done; > + } else if (read_result != len) { > strbuf_addf(reason, > _("short read (expected %"PRIuMAX" bytes, read %"PRIuMAX")"), > (uintmax_t)len, (uintmax_t)read_result); > - free(path); > - return 1; > + rc = 1; > + goto done; > } > while (len && (path[len - 1] == '\n' || path[len - 1] == '\r')) > len--; > if (!len) { > strbuf_addstr(reason, _("invalid gitdir file")); > - free(path); > - return 1; > + rc = 1; > + goto done; > } > path[len] = '\0'; > - if (!file_exists(path)) { > - if (stat(git_path("worktrees/%s/index", id), &st) || > - st.st_mtime <= expire) { > + if (is_absolute_path(path)) { > + strbuf_addstr(&dotgit, path); > + } else { > + strbuf_addf(&dotgit, "%s/%s", repo.buf, path); > + strbuf_realpath_forgiving(&dotgit, dotgit.buf, 0); > + } > + if (!file_exists(dotgit.buf)) { > + strbuf_reset(&file); > + strbuf_addf(&file, "%s/index", repo.buf); > + if (stat(file.buf, &st) || st.st_mtime <= expire) { > strbuf_addstr(reason, _("gitdir file points to non-existent location")); > - free(path); > - return 1; > - } else { > - *wtpath = path; > - return 0; > + rc = 1; > + goto done; > } > } > - *wtpath = path; > - return 0; > + *wtpath = strbuf_detach(&dotgit, NULL); > +done: > + free(path); > + strbuf_release(&dotgit); > + strbuf_release(&gitdir); > + strbuf_release(&repo); > + strbuf_release(&file); > + return rc; > } > > static int move_config_setting(const char *key, const char *value, > diff --git a/worktree.h b/worktree.h > index 11279d0c8fe249bb30642563bf221a8de7f3b0a3..e96118621638667d87c5d7e0452ed10bd1ddf606 100644 > --- a/worktree.h > +++ b/worktree.h > @@ -131,6 +131,16 @@ typedef void (* worktree_repair_fn)(int iserr, const char *path, > */ > void repair_worktrees(worktree_repair_fn, void *cb_data); > > +/* > + * Repair the linked worktrees after the gitdir has been moved. > + */ > +void repair_worktrees_after_gitdir_move(const char *old_path); > + > +/* > + * Repair the linked worktree after the gitdir has been moved. > + */ > +void repair_worktree_after_gitdir_move(struct worktree *wt, const char *old_path); > + > /* > * Repair administrative files corresponding to the worktree at the given path. > * The worktree's .git file pointing at the repository must be intact for the >
On Tuesday, October 8th, 2024 at 18:09, Junio C Hamano <gitster@pobox.com> wrote: > Caleb White via B4 Relay devnull+cdwhite3.pm.me@kernel.org writes: > > > From: Caleb White cdwhite3@pm.me > > > > Git currently stores absolute paths to both the main repository and > > > As always, drop "currently". > > The usual way to compose a log message of this project is to > > - Give an observation on how the current system work in the present > tense (so no need to say "Currently X is Y", just "X is Y"), and > discuss what you perceive as a problem in it. > > - Propose a solution (optional---often, problem description > trivially leads to an obvious solution in reader's minds). > > - Give commands to the codebase to "become like so". > > in this order. Thank you for the feedback, I'll reword this. This is my first patch series to this project (and my first mailing list patch submission, so I've been learning a bunch), thank you for bearing with me! :) > > linked worktrees. However, this causes problems when moving repositories > > The above claim is way too strong. When relative references are > used, you can move directories without problems only if you move all > the worktrees and the repository in unison without breaking their > relative locations, which is an exception and not the norm. > > The repository knows where its extra worktrees are by their > absolute paths (and vice versa) to allow discovery of each > other. When a directory is moved, "git worktree repair" must be > used to adjust these references. > > It however becomes possible, in a narrow special case, to do > without "git worktree repair". When the repository and the > worktrees move in parallel without breaking their relative > location, the repair operation becomes unneeded if we made them > refer to each other using relative paths (think: tarring up both > the reposity and the worktrees at the same time, and then > untarring the result at a different place). > > or something, perhaps. I can reduce the strength of the language and add some better examples, however, I believe this scenario is becoming more common. There's been a rise in popularity with using worktrees in conjunction with bare repositories that take the following form: /path/to/repo/ .git/ (bare repo) master/ develop/ feature1/ hotfix/ Essentially, you create a directory to act as the the "repository", then you `clone --bare` the actual repository to `.git`, and you create worktrees (there's some other steps that need to be performed such as updating the refs to point to origin, but this is essentially it) as needed. This pattern has the following advantages: 1. For high volume projects, this allows you to use worktrees to quickly switch between contexts. I might be working on a feature at work when I quickly need to switch focus to hotfix something that broke in production and I don't have to worry about committing / stashing. 2. This avoids polluting the parent directory with the different worktrees. I tend to have a directory that houses the various repos I work on, and I don't want to have worktrees intermixed with other repos. 3. This keeps the `repo` directory clean so you don't have worktrees intermixed with your source code on the main working tree. In this pattern, everything is self contained inside the `repo` dir. Moving this directory or tarring / untarring it (as you mentioned) should not cause the links to break and require repair. Granted, this is not the only use-case which is why both absolute and relative paths should be supported. > > Although Git now writes relative paths, existing repositories with > > absolute paths are still supported. There are no breaking changes > > to workflows based on absolute paths, ensuring backward compatibility. > > Good. Being able to work with existing repositories is an absolute > requirement. Are there good test cases? I would imagine that you > would need to force a worktree and its repository to refer to each > other using absolute paths and then try to access with the updated > code, perhaps moving one of such "old-style" directory and then > using the updated "git worktree repair" and observing the result. > To allow such a test possible, you might even need an option to "git > worktree" to allow it to create these linkages using absolute paths, > not relative (and if you need to keep both possibilities anyway, you > might make the newer "relative" layout an optional feature, > triggered by a command line option given to "git worktree add" and > friends). The tests passed before and after, but there's no tests that explicitly use absolute paths. I can add a `worktree.useRelativePaths` config/cli option. I was trying to avoid this, but this may be best to offer users the most flexibility. With the addition of this config value, I might split patch [2/3] into two patches: the first will handle reading potentially relative paths from the files, and the second will add the config option and handle the writing of relative paths to the files. What's the best way to parameterize the worktree tests? I would like to run the same tests for both absolute and relative paths and I'm not particularly a fan of just copying them all into new *-relative.sh files. > Have we considered how badly existing third-party tools, that has > learned to assume that the paths are aboslute, may break with this > change, though? Or is this "we won't know until we inflict it on > real users and see who screams"? The `worktree` struct that's used internally is populated with the absolute path, and `worktree list` and the porcelain options return the absolute paths to the user, so third-party tools should not notice a difference if they're using the api. The worktree docs even include the following warning: The rule of thumb is do not make any assumption about whether a path belongs to $GIT_DIR or $GIT_COMMON_DIR when you need to directly access something inside $GIT_DIR. Use git rev-parse --git-path to get the final path. so third-party tools really shouldn't be reading the files, and I'm not aware of any that do, but we can monitor for feedback once users start using the change. Best,
On Wednesday, October 9th, 2024 at 05:11, Phillip Wood <phillip.wood123@gmail.com> wrote: > This is quite a sweeping claim, it would be helpful to describe the > trade off that this patch is making. Thank you for your feedback, I'll adjust the language and add some additional examples. > but as I understand it while there are cases such as > sharing a drive between Windows and linux or moving all the worktrees > including the main one together where using relative paths prevents problems This is true, there are several cases where relative paths prevent breakages. > there are other cases such as moving a single worktree that are > fixable by running "git worktree repair" when using absolute paths but > not with relative paths. This is not exactly true, if only the **repository** is moved then yes it will no longer be able to find it's worktrees without the absolute path, however, `git worktree repair` can still repair the worktree when provided the path to the worktree (or the directory containing the worktrees I believe). > It was suggested in another thread I saw recently that storing both > would be more resiliant. I'm not too sure what you mean here, but storing both the absolute and relative paths in both files seems like an over-complication (not sure if that's what you were talking about or not). However, this patch series does support the linking files containing either absolute or relative paths. > Another possibility would be to store the an absolute path in the > worktree's .git file and relative paths in worktrees/*/gitdir. That > would enable "git worktree repair" run in a moved worktree to find the > main worktree if the main worktree has not been moved as it does now. It > would also allow "git worktree repair" run in the main worktree that has > been moved to find the linked worktrees that were moved in tandam with it. Storing the absolute path in the `.git` and relative paths in the `gitdir` will not work---it will still suffer from the same limitations that we are trying to avoid. And all the test cases related to `worktree repair` are passing with relative paths. Best,
Caleb White <cdwhite3@pm.me> writes: > What's the best way to parameterize the worktree tests? I would like > to run the same tests for both absolute and relative paths and I'm > not particularly a fan of just copying them all into new *-relative.sh > files. What I meant by interoperability tests are a lot smaller scale. A test that creates worktree/repository pair without the option to use relative, and then tries to use such a worktree/repository pair with the option would simulate "how well the newer Git handles an existing repository", and another test that creates with the option to use relative and uses the worktree/repository without the option would simulate "how well existing versions of Git works when seeing a worktree made with the newer git with the relative option". By "parameterise", if you mean running a set of worktree/repository tests without the "relative" option enabled, and run the same set of tests with the option enabled, you could model it after how t8001 and t8002 (or t5560 and t5561) share a lot of same tests that are in a file that is included by both of them. In smaller scale, it is common to have an ad-hoc construct like: for conf in relative absolute do test_expect_success ... test_expect_success ... test_expect_success ... done that has bunch of test_expect_success, which may change the behaviour depending on the value of $conf, not &&-chained inside the for loop. You can use a nested loop (one for preparing, the other for testing the use of worktree) if you want to test the full matrix. I do not offhand know if such parametralized tests are necessary in the context of this change, though. Thanks.
> Caleb White cdwhite3@pm.me writes: > > > What's the best way to parameterize the worktree tests? I would like > > to run the same tests for both absolute and relative paths and I'm > > not particularly a fan of just copying them all into new *-relative.sh > > files. > > > What I meant by interoperability tests are a lot smaller scale. > > A test that creates worktree/repository pair without the option to > use relative, and then tries to use such a worktree/repository pair > with the option would simulate "how well the newer Git handles an > existing repository", and another test that creates with the option > to use relative and uses the worktree/repository without the option > would simulate "how well existing versions of Git works when seeing > a worktree made with the newer git with the relative option". > > By "parameterise", if you mean running a set of worktree/repository > tests without the "relative" option enabled, and run the same set of > tests with the option enabled, you could model it after how t8001 > and t8002 (or t5560 and t5561) share a lot of same tests that are in > a file that is included by both of them. In smaller scale, it is > common to have an ad-hoc construct like: > > for conf in relative absolute > do > test_expect_success ... > test_expect_success ... > test_expect_success ... > done > > that has bunch of test_expect_success, which may change the > behaviour depending on the value of $conf, not &&-chained inside the > for loop. You can use a nested loop (one for preparing, the other > for testing the use of worktree) if you want to test the full > matrix. > > I do not offhand know if such parametralized tests are necessary in > the context of this change, though. > > Thanks. Ah, I see what you mean, thank you for the details! I'll be sure to look at the examples and try to determine which would be the best paths forward. > existing repository", and another test that creates with the option > to use relative and uses the worktree/repository without the option > would simulate "how well existing versions of Git works when seeing > a worktree made with the newer git with the relative option". I can already tell you that this particular case is not going to work because existing versions of git expect the path to be absolute. Most of the changes in this patch revolve around properly reading/handling the relative paths, not writing the relative paths. Best,
Caleb White <cdwhite3@pm.me> writes: >> existing repository", and another test that creates with the option >> to use relative and uses the worktree/repository without the option >> would simulate "how well existing versions of Git works when seeing >> a worktree made with the newer git with the relative option". > > I can already tell you that this particular case is not going to work > because existing versions of git expect the path to be absolute. Most > of the changes in this patch revolve around properly reading/handling > the relative paths, not writing the relative paths. If we are talking about making irreversible change to an existing repository, we may need to grab one extensions bit (cf. Documentation/technical/repository-version.txt and then refer also to Documentation/config/extensions.txt [*]) and flip it when we wrote a relative link to refer to an worktree and repository. [Footnote] * The repository-version document claims that any extensions invented must be registered there, but config/extensions.txt that came later ignored it and seems to have acquired a few more than the "master list". We should clean up the mess.
On Wednesday, October 9th, 2024 at 18:37, Junio C Hamano <gitster@pobox.com> wrote: > Caleb White cdwhite3@pm.me writes: > > > > existing repository", and another test that creates with the option > > > to use relative and uses the worktree/repository without the option > > > would simulate "how well existing versions of Git works when seeing > > > a worktree made with the newer git with the relative option". > > > > I can already tell you that this particular case is not going to work > > because existing versions of git expect the path to be absolute. Most > > of the changes in this patch revolve around properly reading/handling > > the relative paths, not writing the relative paths. > > > If we are talking about making irreversible change to an existing > repository, we may need to grab one extensions bit (cf. > Documentation/technical/repository-version.txt and then refer also > to Documentation/config/extensions.txt [*]) and flip it when we > wrote a relative link to refer to an worktree and repository. Thanks, I'll take a look at the references. > [Footnote] > > * The repository-version document claims that any extensions > invented must be registered there, but config/extensions.txt that > came later ignored it and seems to have acquired a few more than > the "master list". We should clean up the mess. Would you like the contents of config/extensions.txt moved into the repository-version document and then deleted?
Empty Message
diff --git a/builtin/worktree.c b/builtin/worktree.c index fc31d072a620d7b455d7f150bd3a9e773ee9d4ed..dae63dedf4cac2621f51f95a39aa456b33acd894 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -414,7 +414,8 @@ static int add_worktree(const char *path, const char *refname, const struct add_opts *opts) { struct strbuf sb_git = STRBUF_INIT, sb_repo = STRBUF_INIT; - struct strbuf sb = STRBUF_INIT, realpath = STRBUF_INIT; + struct strbuf sb = STRBUF_INIT, sb_tmp = STRBUF_INIT; + struct strbuf sb_path_realpath = STRBUF_INIT, sb_repo_realpath = STRBUF_INIT; const char *name; struct strvec child_env = STRVEC_INIT; unsigned int counter = 0; @@ -490,11 +491,10 @@ static int add_worktree(const char *path, const char *refname, strbuf_reset(&sb); strbuf_addf(&sb, "%s/gitdir", sb_repo.buf); - strbuf_realpath(&realpath, sb_git.buf, 1); - write_file(sb.buf, "%s", realpath.buf); - strbuf_realpath(&realpath, repo_get_common_dir(the_repository), 1); - write_file(sb_git.buf, "gitdir: %s/worktrees/%s", - realpath.buf, name); + strbuf_realpath(&sb_path_realpath, path, 1); + strbuf_realpath(&sb_repo_realpath, sb_repo.buf, 1); + write_file(sb.buf, "%s/.git", relative_path(sb_path_realpath.buf, sb_repo_realpath.buf, &sb_tmp)); + write_file(sb_git.buf, "gitdir: %s", relative_path(sb_repo_realpath.buf, sb_path_realpath.buf, &sb_tmp)); strbuf_reset(&sb); strbuf_addf(&sb, "%s/commondir", sb_repo.buf); write_file(sb.buf, "../.."); @@ -578,11 +578,13 @@ static int add_worktree(const char *path, const char *refname, strvec_clear(&child_env); strbuf_release(&sb); + strbuf_release(&sb_tmp); strbuf_release(&symref); strbuf_release(&sb_repo); + strbuf_release(&sb_repo_realpath); strbuf_release(&sb_git); + strbuf_release(&sb_path_realpath); strbuf_release(&sb_name); - strbuf_release(&realpath); free_worktree(wt); return ret; } diff --git a/setup.c b/setup.c index 94e79b2e487f3faa537547e190acf9b7ea0be3b5..7b648de0279116b381eea46800ad130606926103 100644 --- a/setup.c +++ b/setup.c @@ -2420,7 +2420,7 @@ static void separate_git_dir(const char *git_dir, const char *git_link) if (rename(src, git_dir)) die_errno(_("unable to move %s to %s"), src, git_dir); - repair_worktrees(NULL, NULL); + repair_worktrees_after_gitdir_move(src); } write_file(git_link, "gitdir: %s", git_dir); diff --git a/t/t2408-worktree-relative.sh b/t/t2408-worktree-relative.sh new file mode 100755 index 0000000000000000000000000000000000000000..a3136db7e28cb20926ff44211e246ce625a6e51a --- /dev/null +++ b/t/t2408-worktree-relative.sh @@ -0,0 +1,39 @@ +#!/bin/sh + +test_description='test worktrees linked with relative paths' + +TEST_PASSES_SANITIZE_LEAK=true +. ./test-lib.sh + +test_expect_success 'links worktrees with relative paths' ' + test_when_finished rm -rf repo && + git init repo && + ( + cd repo && + test_commit initial && + git worktree add wt1 && + echo "../../../wt1/.git" >expected_gitdir && + cat .git/worktrees/wt1/gitdir >actual_gitdir && + echo "gitdir: ../.git/worktrees/wt1" >expected_git && + cat wt1/.git >actual_git && + test_cmp expected_gitdir actual_gitdir && + test_cmp expected_git actual_git + ) +' + +test_expect_success 'move repo without breaking relative internal links' ' + test_when_finished rm -rf repo moved && + git init repo && + ( + cd repo && + test_commit initial && + git worktree add wt1 && + cd .. && + mv repo moved && + cd moved/wt1 && + git status >out 2>err && + test_must_be_empty err + ) +' + +test_done diff --git a/worktree.c b/worktree.c index 0cba0d6e6e9ad02ace04a0301104a04a07cbef65..77ff484d3ec48c547ee4e3d958dfa28a52c1eaa7 100644 --- a/worktree.c +++ b/worktree.c @@ -110,6 +110,12 @@ struct worktree *get_linked_worktree(const char *id, strbuf_rtrim(&worktree_path); strbuf_strip_suffix(&worktree_path, "/.git"); + if (!is_absolute_path(worktree_path.buf)) { + strbuf_strip_suffix(&path, "gitdir"); + strbuf_addbuf(&path, &worktree_path); + strbuf_realpath_forgiving(&worktree_path, path.buf, 0); + } + CALLOC_ARRAY(worktree, 1); worktree->repo = the_repository; worktree->path = strbuf_detach(&worktree_path, NULL); @@ -373,18 +379,29 @@ int validate_worktree(const struct worktree *wt, struct strbuf *errmsg, void update_worktree_location(struct worktree *wt, const char *path_) { struct strbuf path = STRBUF_INIT; + struct strbuf repo = STRBUF_INIT; + struct strbuf file = STRBUF_INIT; + struct strbuf tmp = STRBUF_INIT; if (is_main_worktree(wt)) BUG("can't relocate main worktree"); + strbuf_realpath(&repo, git_common_path("worktrees/%s", wt->id), 1); strbuf_realpath(&path, path_, 1); if (fspathcmp(wt->path, path.buf)) { - write_file(git_common_path("worktrees/%s/gitdir", wt->id), - "%s/.git", path.buf); + strbuf_addf(&file, "%s/gitdir", repo.buf); + write_file(file.buf, "%s/.git", relative_path(path.buf, repo.buf, &tmp)); + strbuf_reset(&file); + strbuf_addf(&file, "%s/.git", path.buf); + write_file(file.buf, "gitdir: %s", relative_path(repo.buf, path.buf, &tmp)); + free(wt->path); wt->path = strbuf_detach(&path, NULL); } strbuf_release(&path); + strbuf_release(&repo); + strbuf_release(&file); + strbuf_release(&tmp); } int is_worktree_being_rebased(const struct worktree *wt, @@ -564,38 +581,52 @@ static void repair_gitfile(struct worktree *wt, { struct strbuf dotgit = STRBUF_INIT; struct strbuf repo = STRBUF_INIT; - char *backlink; + struct strbuf backlink = STRBUF_INIT; + struct strbuf tmp = STRBUF_INIT; + char *dotgit_contents = NULL; const char *repair = NULL; int err; /* missing worktree can't be repaired */ if (!file_exists(wt->path)) - return; + goto done; if (!is_directory(wt->path)) { fn(1, wt->path, _("not a directory"), cb_data); - return; + goto done; } strbuf_realpath(&repo, git_common_path("worktrees/%s", wt->id), 1); strbuf_addf(&dotgit, "%s/.git", wt->path); - backlink = xstrdup_or_null(read_gitfile_gently(dotgit.buf, &err)); + dotgit_contents = xstrdup_or_null(read_gitfile_gently(dotgit.buf, &err)); + + if (dotgit_contents) { + if (is_absolute_path(dotgit_contents)) { + strbuf_addstr(&backlink, dotgit_contents); + } else { + strbuf_addf(&backlink, "%s/%s", wt->path, dotgit_contents); + strbuf_realpath_forgiving(&backlink, backlink.buf, 0); + } + } if (err == READ_GITFILE_ERR_NOT_A_FILE) fn(1, wt->path, _(".git is not a file"), cb_data); else if (err) repair = _(".git file broken"); - else if (fspathcmp(backlink, repo.buf)) + else if (fspathcmp(backlink.buf, repo.buf)) repair = _(".git file incorrect"); if (repair) { fn(0, wt->path, repair, cb_data); - write_file(dotgit.buf, "gitdir: %s", repo.buf); + write_file(dotgit.buf, "gitdir: %s", relative_path(repo.buf, wt->path, &tmp)); } - free(backlink); +done: + free(dotgit_contents); strbuf_release(&repo); strbuf_release(&dotgit); + strbuf_release(&backlink); + strbuf_release(&tmp); } static void repair_noop(int iserr UNUSED, @@ -618,6 +649,59 @@ void repair_worktrees(worktree_repair_fn fn, void *cb_data) free_worktrees(worktrees); } +void repair_worktree_after_gitdir_move(struct worktree *wt, const char *old_path) +{ + struct strbuf path = STRBUF_INIT; + struct strbuf repo = STRBUF_INIT; + struct strbuf gitdir = STRBUF_INIT; + struct strbuf dotgit = STRBUF_INIT; + struct strbuf olddotgit = STRBUF_INIT; + struct strbuf tmp = STRBUF_INIT; + + if (is_main_worktree(wt)) + goto done; + + strbuf_realpath(&repo, git_common_path("worktrees/%s", wt->id), 1); + strbuf_addf(&gitdir, "%s/gitdir", repo.buf); + + if (strbuf_read_file(&olddotgit, gitdir.buf, 0) < 0) + goto done; + + strbuf_rtrim(&olddotgit); + if (is_absolute_path(olddotgit.buf)) { + strbuf_addbuf(&dotgit, &olddotgit); + } else { + strbuf_addf(&dotgit, "%s/worktrees/%s/%s", old_path, wt->id, olddotgit.buf); + strbuf_realpath_forgiving(&dotgit, dotgit.buf, 0); + } + + if (!file_exists(dotgit.buf)) + goto done; + + strbuf_addbuf(&path, &dotgit); + strbuf_strip_suffix(&path, "/.git"); + + write_file(dotgit.buf, "gitdir: %s", relative_path(repo.buf, path.buf, &tmp)); + write_file(gitdir.buf, "%s", relative_path(dotgit.buf, repo.buf, &tmp)); +done: + strbuf_release(&path); + strbuf_release(&repo); + strbuf_release(&gitdir); + strbuf_release(&dotgit); + strbuf_release(&olddotgit); + strbuf_release(&tmp); +} + +void repair_worktrees_after_gitdir_move(const char *old_path) +{ + struct worktree **worktrees = get_worktrees_internal(1); + struct worktree **wt = worktrees + 1; /* +1 skips main worktree */ + + for (; *wt; wt++) + repair_worktree_after_gitdir_move(*wt, old_path); + free_worktrees(worktrees); +} + static int is_main_worktree_path(const char *path) { struct strbuf target = STRBUF_INIT; @@ -684,6 +768,8 @@ void repair_worktree_at_path(const char *path, struct strbuf inferred_backlink = STRBUF_INIT; struct strbuf gitdir = STRBUF_INIT; struct strbuf olddotgit = STRBUF_INIT; + struct strbuf realolddotgit = STRBUF_INIT; + struct strbuf tmp = STRBUF_INIT; char *dotgit_contents = NULL; const char *repair = NULL; int err; @@ -701,9 +787,17 @@ void repair_worktree_at_path(const char *path, } infer_backlink(realdotgit.buf, &inferred_backlink); + strbuf_realpath_forgiving(&inferred_backlink, inferred_backlink.buf, 0); dotgit_contents = xstrdup_or_null(read_gitfile_gently(realdotgit.buf, &err)); if (dotgit_contents) { - strbuf_addstr(&backlink, dotgit_contents); + if (is_absolute_path(dotgit_contents)) { + strbuf_addstr(&backlink, dotgit_contents); + } else { + strbuf_addbuf(&backlink, &realdotgit); + strbuf_strip_suffix(&backlink, ".git"); + strbuf_addstr(&backlink, dotgit_contents); + strbuf_realpath_forgiving(&backlink, backlink.buf, 0); + } } else if (err == READ_GITFILE_ERR_NOT_A_FILE) { fn(1, realdotgit.buf, _("unable to locate repository; .git is not a file"), cb_data); goto done; @@ -721,7 +815,7 @@ void repair_worktree_at_path(const char *path, fn(1, realdotgit.buf, _("unable to locate repository; .git file does not reference a repository"), cb_data); goto done; } - } else if (err) { + } else { fn(1, realdotgit.buf, _("unable to locate repository; .git file broken"), cb_data); goto done; } @@ -753,90 +847,117 @@ void repair_worktree_at_path(const char *path, repair = _("gitdir unreadable"); else { strbuf_rtrim(&olddotgit); - if (fspathcmp(olddotgit.buf, realdotgit.buf)) + if (is_absolute_path(olddotgit.buf)) { + strbuf_addbuf(&realolddotgit, &olddotgit); + } else { + strbuf_addf(&realolddotgit, "%s/%s", backlink.buf, olddotgit.buf); + strbuf_realpath_forgiving(&realolddotgit, realolddotgit.buf, 0); + } + if (fspathcmp(realolddotgit.buf, realdotgit.buf)) repair = _("gitdir incorrect"); } if (repair) { fn(0, gitdir.buf, repair, cb_data); - write_file(gitdir.buf, "%s", realdotgit.buf); + write_file(gitdir.buf, "%s", relative_path(realdotgit.buf, backlink.buf, &tmp)); } done: free(dotgit_contents); strbuf_release(&olddotgit); + strbuf_release(&realolddotgit); strbuf_release(&backlink); strbuf_release(&inferred_backlink); strbuf_release(&gitdir); strbuf_release(&realdotgit); strbuf_release(&dotgit); + strbuf_release(&tmp); } int should_prune_worktree(const char *id, struct strbuf *reason, char **wtpath, timestamp_t expire) { struct stat st; - char *path; + struct strbuf dotgit = STRBUF_INIT; + struct strbuf gitdir = STRBUF_INIT; + struct strbuf repo = STRBUF_INIT; + struct strbuf file = STRBUF_INIT; + char *path = NULL; + int rc = 0; int fd; size_t len; ssize_t read_result; *wtpath = NULL; - if (!is_directory(git_path("worktrees/%s", id))) { + strbuf_realpath(&repo, git_common_path("worktrees/%s", id), 1); + strbuf_addf(&gitdir, "%s/gitdir", repo.buf); + if (!is_directory(repo.buf)) { strbuf_addstr(reason, _("not a valid directory")); - return 1; + rc = 1; + goto done; } - if (file_exists(git_path("worktrees/%s/locked", id))) - return 0; - if (stat(git_path("worktrees/%s/gitdir", id), &st)) { + strbuf_addf(&file, "%s/locked", repo.buf); + if (file_exists(file.buf)) { + goto done; + } + if (stat(gitdir.buf, &st)) { strbuf_addstr(reason, _("gitdir file does not exist")); - return 1; + rc = 1; + goto done; } - fd = open(git_path("worktrees/%s/gitdir", id), O_RDONLY); + fd = open(gitdir.buf, O_RDONLY); if (fd < 0) { strbuf_addf(reason, _("unable to read gitdir file (%s)"), strerror(errno)); - return 1; + rc = 1; + goto done; } len = xsize_t(st.st_size); path = xmallocz(len); read_result = read_in_full(fd, path, len); + close(fd); if (read_result < 0) { strbuf_addf(reason, _("unable to read gitdir file (%s)"), strerror(errno)); - close(fd); - free(path); - return 1; - } - close(fd); - - if (read_result != len) { + rc = 1; + goto done; + } else if (read_result != len) { strbuf_addf(reason, _("short read (expected %"PRIuMAX" bytes, read %"PRIuMAX")"), (uintmax_t)len, (uintmax_t)read_result); - free(path); - return 1; + rc = 1; + goto done; } while (len && (path[len - 1] == '\n' || path[len - 1] == '\r')) len--; if (!len) { strbuf_addstr(reason, _("invalid gitdir file")); - free(path); - return 1; + rc = 1; + goto done; } path[len] = '\0'; - if (!file_exists(path)) { - if (stat(git_path("worktrees/%s/index", id), &st) || - st.st_mtime <= expire) { + if (is_absolute_path(path)) { + strbuf_addstr(&dotgit, path); + } else { + strbuf_addf(&dotgit, "%s/%s", repo.buf, path); + strbuf_realpath_forgiving(&dotgit, dotgit.buf, 0); + } + if (!file_exists(dotgit.buf)) { + strbuf_reset(&file); + strbuf_addf(&file, "%s/index", repo.buf); + if (stat(file.buf, &st) || st.st_mtime <= expire) { strbuf_addstr(reason, _("gitdir file points to non-existent location")); - free(path); - return 1; - } else { - *wtpath = path; - return 0; + rc = 1; + goto done; } } - *wtpath = path; - return 0; + *wtpath = strbuf_detach(&dotgit, NULL); +done: + free(path); + strbuf_release(&dotgit); + strbuf_release(&gitdir); + strbuf_release(&repo); + strbuf_release(&file); + return rc; } static int move_config_setting(const char *key, const char *value, diff --git a/worktree.h b/worktree.h index 11279d0c8fe249bb30642563bf221a8de7f3b0a3..e96118621638667d87c5d7e0452ed10bd1ddf606 100644 --- a/worktree.h +++ b/worktree.h @@ -131,6 +131,16 @@ typedef void (* worktree_repair_fn)(int iserr, const char *path, */ void repair_worktrees(worktree_repair_fn, void *cb_data); +/* + * Repair the linked worktrees after the gitdir has been moved. + */ +void repair_worktrees_after_gitdir_move(const char *old_path); + +/* + * Repair the linked worktree after the gitdir has been moved. + */ +void repair_worktree_after_gitdir_move(struct worktree *wt, const char *old_path); + /* * Repair administrative files corresponding to the worktree at the given path. * The worktree's .git file pointing at the repository must be intact for the