On Tue, Nov 05, 2024 at 08:52:19PM +0800, shejialuo wrote:
> On Tue, Nov 05, 2024 at 08:11:49AM +0100, Patrick Steinhardt wrote:
> > On Mon, Oct 21, 2024 at 09:34:40PM +0800, shejialuo wrote:
> > > @@ -3558,6 +3560,9 @@ static int files_fsck_refs_dir(struct ref_store *ref_store,
> > > } else if (S_ISREG(iter->st.st_mode) ||
> > > S_ISLNK(iter->st.st_mode)) {
> > > strbuf_reset(&target_name);
> > > +
> > > + if (!is_main_worktree(wt))
> > > + strbuf_addf(&target_name, "worktrees/%s/", wt->id);
> > > strbuf_addf(&target_name, "%s/%s", refs_check_dir,
> > > iter->relative_path);
> > >
> >
> > Hm. Isn't it somewhat duplicate to pass both the prepended target name
> > _and_ the worktree to the callback? I imagine that we'd have to
> > eventually strip the worktree prefix to find the correct ref, unless we
> > end up using the main ref store to look up the ref.
> >
>
> Actually, the worktree won't be passed to the callback. The
> `fsck_refs_fn` function will never use worktree `wt`. The reason why I
> use `wt` is that we need to print _full_ path information to the user
> when error happens for the situation where worktree A and worktree B has
> the same ref name "refs/worktree/foo".
>
> I agree that we will strip the worktree prefix to find the correct ref
> in the file system. This is done by the following statement:
>
> strbuf_addf(&sb, "%s/%s", ref_store->gitdir, refs_check_dir);
>
> For worktree, `ref_store->gitdir` will automatically be
> `.git/worktrees/<id>`.
>
> In the v5, I didn't print the full path and we even didn't need the
> parameter `wt`. However, if we want to print the following info:
>
> worktrees/<id>/refs/worktree/a
>
> So, just because we need the `worktrees/<id>` information. Actually, we
> could also get the information by using "ref_store->gitdir" and
> "ref_store->repo->gitdir". However, this is cumbersome and it's a bad
> idea. So I change the prototype of "fsck_fn" to add a new parameter
> "struct worktree *".
In practice you can also derive that full refname from the worktree
itself, as the ID is stored in `struct worktree::id`. Would that maybe
be a better solution?
Patrick
