Re: [PATCH v6 4/9] ref: support multiple worktrees check for refs

[Patrick Steinhardt <ps@pks.im>]

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