Re: [PATCH v3 11/20] path: construct correct path to a worktree's index

[Junio C Hamano]

Jonathan Nieder  writes:

> Brandon Williams wrote:
>
>> So your suggestion is to completely avoid doing any location when asking
>> for a worktree_git_path, I guess those code paths which request those
>> paths should be aware enough that if they need something in commondir to
>> use git_common_path instead.  My only worry is that it may be difficult
>> to catch misuse of worktree_git_path during code review, at least that
>> was one of the motivating factors for originally respecting
>> GIT_INDEX_FILE and the like.
>
> Correct: I'm saying that when someone calls worktree_git_path, the
> intent is to resolve a path within the worktree git directory.  File
> relocation just gets in the way of that.
>
> I am not too worried about misuse because the only reason to call
> worktree_git_path is to access a worktree-specific file like HEAD or
> index.

Until somebody has a brilliant idea "git_path() can be implemented
in terms of worktree_git_path()---give it the current worktree!" ;-)

Just joking.  I agree with the general direction you've shown in the
thread.

Thanks.

Re: [PATCH v3 11/20] path: construct correct path to a worktree's index

[Jonathan Nieder]

Brandon Williams wrote:

> So your suggestion is to completely avoid doing any location when asking
> for a worktree_git_path, I guess those code paths which request those
> paths should be aware enough that if they need something in commondir to
> use git_common_path instead.  My only worry is that it may be difficult
> to catch misuse of worktree_git_path during code review, at least that
> was one of the motivating factors for originally respecting
> GIT_INDEX_FILE and the like.

Correct: I'm saying that when someone calls worktree_git_path, the
intent is to resolve a path within the worktree git directory.  File
relocation just gets in the way of that.

I am not too worried about misuse because the only reason to call
worktree_git_path is to access a worktree-specific file like HEAD or
index.

Thanks,
Jonathan

Re: [PATCH v3 11/20] path: construct correct path to a worktree's index

[Brandon Williams]

On 06/20, Jonathan Nieder wrote:
> Hi again,
> 
> Brandon Williams wrote:
> 
> > When working with worktrees the git directory is split into two part,
> > the per-worktree gitdir and a commondir which contains things which are
> > shared among all worktrees (like the object store).  With this notion of
> > having a split git directory, 557bd833b (git_path(): be aware of file
> > relocation in $GIT_DIR) and c7b3a3d2f ($GIT_COMMON_DIR: a new
> > environment variable) changed the way that 'git_path()' functioned so
> > that paths would be adjusted if they referred to files or directories
> > that are stored in the commondir or have been changed via an environment
> > variable.
> >
> > One interesting problem with this is the index file as it is not shared
> > across worktrees yet when asking for a path to a specific worktree's
> > index it will be replaced with a path to the current worktree's index.
> > In order to prevent this, teach 'adjuct_git_path' to replace the
> > path to the index with the path recorded in a repository (which would be
> > the current, active worktree) only when not asked to construct a path
> > for a specific worktree.
> >
> > Signed-off-by: Brandon Williams 
> > ---
> >  path.c | 11 +--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> Digging more into it with your help, I ran into v2.5.0-rc0~143^2~34
> and v2.11.0-rc0~15^2 (git-svn: "git worktree" awareness, 2016-10-14),
> which uses this function:
> 
>  -my $index = $ENV{GIT_INDEX_FILE} || "$ENV{GIT_DIR}/index";
>  +my $index = command_oneline(qw(rev-parse --git-path index));
> 
> That rules out my hope of making git_path stop being aware of the index
> at all.
> 
> That also made it a little clearer to me what is going on with this
> function.  I had previously misread !wt as meaning that git_dir is
> not under worktrees/ (or in other words as !wt->id).  Instead, it
> means that the caller does not want to use some other work tree in
> preference to git_dir.
> 
> With that piece in place, the patch starts making more sense to me.
> When !wt, repo->index_file is going to have the same value as
> getenv("GIT_INDEX_FILE") ?: buf->buf already, since repo->index_file
> refers to the current work tree git dir's index file.  When wt != NULL,
> we don't want to use that file and have requested to grab the index
> from a specific work tree git dir instead.
> 
> And patch 05/20 (environment: place key repository state in
> the_repository) had no effect since no one calls worktree_git_path
> with argument "index", nor with a user-specified path.
> 
> How about the following?  I found it a little easier to understand.

So your suggestion is to completely avoid doing any location when asking
for a worktree_git_path, I guess those code paths which request those
paths should be aware enough that if they need something in commondir to
use git_common_path instead.  My only worry is that it may be difficult
to catch misuse of worktree_git_path during code review, at least that
was one of the motivating factors for originally respecting
GIT_INDEX_FILE and the like.

> 
> -- >8 --
> From: Brandon Williams 
> Subject: worktree_git_path: do not let GIT_INDEX_FILE override path to index
> 
> git_path (and variants like strbuf_git_path) is designed to behave
> like a convenience function that produces a string $GIT_DIR/.
> In the process, callers automatically get support for path relocation
> variables like $GIT_OBJECT_DIRECTORY:
> 
> - git_path("index") is $GIT_INDEX_FILE when set
> - git_path("info/grafts") is $GIT_GRAFTS_FILE when set
> - git_path("objects/") is $GIT_OBJECT_DIRECTORY/ when set
> - git_path("hooks/") is  under core.hookspath when set
> - git_path("refs/") etc (see path.c::common_list) is relative
>   to $GIT_COMMON_DIR instead of $GIT_DIR
> 
> worktree_git_path, by comparison, is designed to resolve files in a
> specific worktree's git dir and should not perform such relocation.
> Do so by skipping the relocation step in worktree_git_path.
> 
> Fortunately no current callers of worktree_git_path pass such
> arguments.  Noticed due to an interaction between two patches under
> review, one of which introduced such a caller:
> 
> * One made "git prune" check the index file in each worktree's git dir
>   (using worktree_git_path(wt, "index")) for objects not to prune,
>   triggering the unwanted relocation code by mistake and allowing
>   objects reachable from worktree indices to be pruned if
>   GIT_INDEX_FILE is set.
> 
> * The other simplified the relocation logic for index, info/grafts,
>   objects, and hooks to happen unconditionally instead of based on
>   whether environment or configuration variables are set, causing the
>   relocation to trigger even when GIT_INDEX_FILE is not set.
> 
> [jn: rewrote commit message; skipped all relocations instead of just
>  the index]
> 
> Change-Id: I2ba0a48a48b7e9a9c2e3ef97648cf53cb913bdd9
> Signed-off-by: Brandon Williams 

Re: [PATCH v3 11/20] path: construct correct path to a worktree's index

[Jonathan Nieder]

Subject: worktree_git_path() should not use file relocation

git_path is a convenience function that usually produces a string
$GIT_DIR/.  Since v2.5.0-rc0~143^2~35 (git_path(): be aware of
file relocation in $GIT_DIR, 2014-11-30), as a side benefit callers
get support for path relocation variables like $GIT_OBJECT_DIRECTORY:

- git_path("index") is $GIT_INDEX_FILE when set
- git_path("info/grafts") is $GIT_GRAFTS_FILE when set
- git_path("objects/") is $GIT_OBJECT_DIRECTORY/ when set
- git_path("hooks/") is  under core.hookspath when set
- git_path("refs/") etc (see path.c::common_list) is relative
  to $GIT_COMMON_DIR instead of $GIT_DIR

worktree_git_path, by comparison, is designed to resolve files in a
specific worktree's git dir.  Unfortunately, it shares code with
git_path and performs the same relocation.  The result is that paths
that are meant to be relative to the specified worktree's git dir end
up replaced by paths from environment variables within the current git
dir.

Luckily, no current callers pass such arguments.  The relocation was
noticed when testing the result of merging two patches under review,
one of which introduces a caller:

* The first patch made git prune check the index file in each
  worktree's git dir (using worktree_git_path(wt, "index")) for
  objects not to prune.  This would trigger the unwanted relocation
  when GIT_INDEX_FILE is set, causing objects reachable from the
  index to be pruned.

* The second patch simplified the relocation logic for index,
  info/grafts, objects, and hooks to happen unconditionally instead of
  based on whether environment or configuration variables are set.
  This caused the relocation to trigger even when GIT_INDEX_FILE is
  not set.

[jn: rewrote commit message; skipping all relocation instead of just
 GIT_INDEX_FILE]

Signed-off-by: Brandon Williams 
Signed-off-by: Jonathan Nieder 
---
Jonathan Nieder wrote:

> How about the following?  I found it a little easier to understand.
>
> -- >8 --
> From: Brandon Williams 
> Subject: worktree_git_path: do not let GIT_INDEX_FILE override path to index
[...]
> Change-Id: I2ba0a48a48b7e9a9c2e3ef97648cf53cb913bdd9

Gah, sorry about the stray Change-Id line.  While we're fixing that,
here's a version with a slightly clearer commit message.

 path.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/path.c b/path.c
index c1cb1cf62..4c3a27a8e 100644
--- a/path.c
+++ b/path.c
@@ -397,7 +397,8 @@ static void do_git_path(const struct worktree *wt, struct 
strbuf *buf,
strbuf_addch(buf, '/');
gitdir_len = buf->len;
strbuf_vaddf(buf, fmt, args);
-   adjust_git_path(buf, gitdir_len);
+   if (!wt)
+   adjust_git_path(buf, gitdir_len);
strbuf_cleanup_path(buf);
 }
 
-- 
2.13.1.611.g7e3b11ae1-goog

Re: [PATCH v3 11/20] path: construct correct path to a worktree's index

[Jonathan Nieder]

Hi again,

Brandon Williams wrote:

> When working with worktrees the git directory is split into two part,
> the per-worktree gitdir and a commondir which contains things which are
> shared among all worktrees (like the object store).  With this notion of
> having a split git directory, 557bd833b (git_path(): be aware of file
> relocation in $GIT_DIR) and c7b3a3d2f ($GIT_COMMON_DIR: a new
> environment variable) changed the way that 'git_path()' functioned so
> that paths would be adjusted if they referred to files or directories
> that are stored in the commondir or have been changed via an environment
> variable.
>
> One interesting problem with this is the index file as it is not shared
> across worktrees yet when asking for a path to a specific worktree's
> index it will be replaced with a path to the current worktree's index.
> In order to prevent this, teach 'adjuct_git_path' to replace the
> path to the index with the path recorded in a repository (which would be
> the current, active worktree) only when not asked to construct a path
> for a specific worktree.
>
> Signed-off-by: Brandon Williams 
> ---
>  path.c | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)

Digging more into it with your help, I ran into v2.5.0-rc0~143^2~34
and v2.11.0-rc0~15^2 (git-svn: "git worktree" awareness, 2016-10-14),
which uses this function:

 -  my $index = $ENV{GIT_INDEX_FILE} || "$ENV{GIT_DIR}/index";
 +  my $index = command_oneline(qw(rev-parse --git-path index));

That rules out my hope of making git_path stop being aware of the index
at all.

That also made it a little clearer to me what is going on with this
function.  I had previously misread !wt as meaning that git_dir is
not under worktrees/ (or in other words as !wt->id).  Instead, it
means that the caller does not want to use some other work tree in
preference to git_dir.

With that piece in place, the patch starts making more sense to me.
When !wt, repo->index_file is going to have the same value as
getenv("GIT_INDEX_FILE") ?: buf->buf already, since repo->index_file
refers to the current work tree git dir's index file.  When wt != NULL,
we don't want to use that file and have requested to grab the index
from a specific work tree git dir instead.

And patch 05/20 (environment: place key repository state in
the_repository) had no effect since no one calls worktree_git_path
with argument "index", nor with a user-specified path.

How about the following?  I found it a little easier to understand.

-- >8 --
From: Brandon Williams 
Subject: worktree_git_path: do not let GIT_INDEX_FILE override path to index

git_path (and variants like strbuf_git_path) is designed to behave
like a convenience function that produces a string $GIT_DIR/.
In the process, callers automatically get support for path relocation
variables like $GIT_OBJECT_DIRECTORY:

- git_path("index") is $GIT_INDEX_FILE when set
- git_path("info/grafts") is $GIT_GRAFTS_FILE when set
- git_path("objects/") is $GIT_OBJECT_DIRECTORY/ when set
- git_path("hooks/") is  under core.hookspath when set
- git_path("refs/") etc (see path.c::common_list) is relative
  to $GIT_COMMON_DIR instead of $GIT_DIR

worktree_git_path, by comparison, is designed to resolve files in a
specific worktree's git dir and should not perform such relocation.
Do so by skipping the relocation step in worktree_git_path.

Fortunately no current callers of worktree_git_path pass such
arguments.  Noticed due to an interaction between two patches under
review, one of which introduced such a caller:

* One made "git prune" check the index file in each worktree's git dir
  (using worktree_git_path(wt, "index")) for objects not to prune,
  triggering the unwanted relocation code by mistake and allowing
  objects reachable from worktree indices to be pruned if
  GIT_INDEX_FILE is set.

* The other simplified the relocation logic for index, info/grafts,
  objects, and hooks to happen unconditionally instead of based on
  whether environment or configuration variables are set, causing the
  relocation to trigger even when GIT_INDEX_FILE is not set.

[jn: rewrote commit message; skipped all relocations instead of just
 the index]

Change-Id: I2ba0a48a48b7e9a9c2e3ef97648cf53cb913bdd9
Signed-off-by: Brandon Williams 
Signed-off-by: Jonathan Nieder 
---
 path.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/path.c b/path.c
index c1cb1cf62..4c3a27a8e 100644
--- a/path.c
+++ b/path.c
@@ -397,7 +397,8 @@ static void do_git_path(const struct worktree *wt, struct 
strbuf *buf,
strbuf_addch(buf, '/');
gitdir_len = buf->len;
strbuf_vaddf(buf, fmt, args);
-   adjust_git_path(buf, gitdir_len);
+   if (!wt)
+   adjust_git_path(buf, gitdir_len);
strbuf_cleanup_path(buf);
 }
 
-- 
2.13.1.611.g7e3b11ae1

Re: [PATCH v3 11/20] path: construct correct path to a worktree's index

[Brandon Williams]

On 06/20, Jonathan Nieder wrote:
> Hi,
> 
> Brandon Williams wrote:
> 
> > When working with worktrees the git directory is split into two part,
> > the per-worktree gitdir and a commondir which contains things which are
> > shared among all worktrees (like the object store).  With this notion of
> > having a split git directory, 557bd833b (git_path(): be aware of file
> > relocation in $GIT_DIR) and c7b3a3d2f ($GIT_COMMON_DIR: a new
> > environment variable) changed the way that 'git_path()' functioned so
> > that paths would be adjusted if they referred to files or directories
> > that are stored in the commondir or have been changed via an environment
> > variable.
> >
> > One interesting problem with this is the index file as it is not shared
> > across worktrees yet when asking for a path to a specific worktree's
> > index it will be replaced with a path to the current worktree's index.
> > In order to prevent this, teach 'adjuct_git_path' to replace the
> > path to the index with the path recorded in a repository (which would be
> > the current, active worktree) only when not asked to construct a path
> > for a specific worktree.
> >
> > Signed-off-by: Brandon Williams 
> > ---
> >  path.c | 11 +--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> Thanks --- this is subtle.  I don't think that what this patch does is
> right.  Commenting below.
> 
> > --- a/path.c
> > +++ b/path.c
> > @@ -372,13 +372,20 @@ void report_linked_checkout_garbage(void)
> >  }
> >  
> >  static void adjust_git_path(const struct repository *repo,
> > +   const struct worktree *wt,
> > struct strbuf *buf, int git_dir_len)
> >  {
> > const char *base = buf->buf + git_dir_len;
> > if (is_dir_file(base, "info", "grafts"))
> > strbuf_splice(buf, 0, buf->len,
> >   repo->graft_file, strlen(repo->graft_file));
> > -   else if (!strcmp(base, "index"))
> > +   /*
> > +* Only try to replace the path '$gitdir/index' with the index file
> > +* recorded in the repository when not constructing a path for a
> > +* worktree.  This way we can retrieve the correct path to a particular
> > +* worktree's index file.
> > +*/
> > +   else if (!wt && !strcmp(base, "index"))
> > strbuf_splice(buf, 0, buf->len,
> >   repo->index_file, strlen(repo->index_file));
> 
> Some context that may have been missing: GIT_INDEX_FILE is a low-level
> tool to allow script authors to specify an alternate index file to use
> when running commands like "git read-tree" or "git checkout-index".
> 
> The above would make it not take effect for git_path() callers when
> 'wt != NULL'.  As a result, if any caller reaches this code path, then
> scripts specifying GIT_INDEX_FILE would stop working when run from a
> worktree that borrows refs and objects from a separate repository.
> I'm pretty sure that such a subtle flip in behavior based on whether
> they are in "git worktree" created worktree or the main working tree
> is not what the end user would intend, so this looks like a step in
> the wrong direction.
> 
> Fortunately this code path doesn't actually get called.
> 
> In fact, rewriting git_path("index") in this function feels to me like
> a layering violation.  Shouldn't callers be using get_index_file() to
> express their intent more clearly?  That's what all current callers
> do.
> 
> IIUC this came up when a patch from nd/prune-in-worktree (e7a6a3b15,
> revision.c: --indexed-objects add objects from all worktrees), which
> is currently not in pu, introduced a caller that does call
> git_path("index").  The old behavior of replacing git_path("index")
> with $GIT_INDEX_FILE when the latter is set was mostly harmless
> because typically GIT_INDEX_FILE is not set, especially when people
> are running "git prune".  Patch 05/20 (environment: place key
> repository state in the_repository) made the substitution harmful: now
> we would use repo->index_file unconditionally instead of allowing the
> ordinary worktree-relative resolution as a fallback when
> GIT_INDEX_FILE is unset.
> 
> Possible next steps:
> 
>  1. I think we should make git_path less magical and discourage
> callers from relying on it to handle the GIT_INDEX_FILE envvar.
> We can do that by removing the !strcmp(base, "index") case
> completely.

>From our conversation off-line it seems like we agree that this is
probably the best course of action.  So I'll plan to drop the case
completely since it isn't even used currently.  I'll also go ahead and
drop the graft_file one as well since it doesn't make much sense to keep
it around when it is only queried for once.

> 
>  2. Optionally, it is possible to be more cautious by keeping the
> !strcmp(base, "index") case and making it call BUG() to force
> people not to use it.  This would help steer callers toward
> get_index_file().  But given that the only 

Re: [PATCH v3 11/20] path: construct correct path to a worktree's index

[Jonathan Nieder]

Hi,

Brandon Williams wrote:

> When working with worktrees the git directory is split into two part,
> the per-worktree gitdir and a commondir which contains things which are
> shared among all worktrees (like the object store).  With this notion of
> having a split git directory, 557bd833b (git_path(): be aware of file
> relocation in $GIT_DIR) and c7b3a3d2f ($GIT_COMMON_DIR: a new
> environment variable) changed the way that 'git_path()' functioned so
> that paths would be adjusted if they referred to files or directories
> that are stored in the commondir or have been changed via an environment
> variable.
>
> One interesting problem with this is the index file as it is not shared
> across worktrees yet when asking for a path to a specific worktree's
> index it will be replaced with a path to the current worktree's index.
> In order to prevent this, teach 'adjuct_git_path' to replace the
> path to the index with the path recorded in a repository (which would be
> the current, active worktree) only when not asked to construct a path
> for a specific worktree.
>
> Signed-off-by: Brandon Williams 
> ---
>  path.c | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)

Thanks --- this is subtle.  I don't think that what this patch does is
right.  Commenting below.

> --- a/path.c
> +++ b/path.c
> @@ -372,13 +372,20 @@ void report_linked_checkout_garbage(void)
>  }
>  
>  static void adjust_git_path(const struct repository *repo,
> + const struct worktree *wt,
>   struct strbuf *buf, int git_dir_len)
>  {
>   const char *base = buf->buf + git_dir_len;
>   if (is_dir_file(base, "info", "grafts"))
>   strbuf_splice(buf, 0, buf->len,
> repo->graft_file, strlen(repo->graft_file));
> - else if (!strcmp(base, "index"))
> + /*
> +  * Only try to replace the path '$gitdir/index' with the index file
> +  * recorded in the repository when not constructing a path for a
> +  * worktree.  This way we can retrieve the correct path to a particular
> +  * worktree's index file.
> +  */
> + else if (!wt && !strcmp(base, "index"))
>   strbuf_splice(buf, 0, buf->len,
> repo->index_file, strlen(repo->index_file));

Some context that may have been missing: GIT_INDEX_FILE is a low-level
tool to allow script authors to specify an alternate index file to use
when running commands like "git read-tree" or "git checkout-index".

The above would make it not take effect for git_path() callers when
'wt != NULL'.  As a result, if any caller reaches this code path, then
scripts specifying GIT_INDEX_FILE would stop working when run from a
worktree that borrows refs and objects from a separate repository.
I'm pretty sure that such a subtle flip in behavior based on whether
they are in "git worktree" created worktree or the main working tree
is not what the end user would intend, so this looks like a step in
the wrong direction.

Fortunately this code path doesn't actually get called.

In fact, rewriting git_path("index") in this function feels to me like
a layering violation.  Shouldn't callers be using get_index_file() to
express their intent more clearly?  That's what all current callers
do.

IIUC this came up when a patch from nd/prune-in-worktree (e7a6a3b15,
revision.c: --indexed-objects add objects from all worktrees), which
is currently not in pu, introduced a caller that does call
git_path("index").  The old behavior of replacing git_path("index")
with $GIT_INDEX_FILE when the latter is set was mostly harmless
because typically GIT_INDEX_FILE is not set, especially when people
are running "git prune".  Patch 05/20 (environment: place key
repository state in the_repository) made the substitution harmful: now
we would use repo->index_file unconditionally instead of allowing the
ordinary worktree-relative resolution as a fallback when
GIT_INDEX_FILE is unset.

Possible next steps:

 1. I think we should make git_path less magical and discourage
callers from relying on it to handle the GIT_INDEX_FILE envvar.
We can do that by removing the !strcmp(base, "index") case
completely.

 2. Optionally, it is possible to be more cautious by keeping the
!strcmp(base, "index") case and making it call BUG() to force
people not to use it.  This would help steer callers toward
get_index_file().  But given that the only caller did not actually
want GIT_INDEX_FILE substitution, I don't think that that is
necessary or useful.

 3. A docstring for git_path should explain the substitutions it
currently makes and more straightforward alternatives that callers
can use.

 4. Specifying GIT_INDEX_FILE when running "git prune" is a
meaningless combination.  It would be nice for "git prune" to
error out to save the user from confusion.

 5. There are likely other commands that don't make sense with
GIT_INDEX_FILE. 

Re: [PATCH v3 11/20] path: construct correct path to a worktree's index

[Stefan Beller]

On Tue, Jun 20, 2017 at 12:19 PM, Brandon Williams  wrote:
> When working with worktrees the git directory is split into two part,
> the per-worktree gitdir and a commondir which contains things which are
> shared among all worktrees (like the object store).  With this notion of
> having a split git directory, 557bd833b (git_path(): be aware of file
> relocation in $GIT_DIR) and c7b3a3d2f ($GIT_COMMON_DIR: a new
> environment variable) changed the way that 'git_path()' functioned so
> that paths would be adjusted if they referred to files or directories
> that are stored in the commondir or have been changed via an environment
> variable.
>
> One interesting problem with this is the index file as it is not shared
> across worktrees yet when asking for a path to a specific worktree's
> index it will be replaced with a path to the current worktree's index.
> In order to prevent this, teach 'adjuct_git_path' to replace the

adjust_git_path

> path to the index with the path recorded in a repository (which would be
> the current, active worktree) only when not asked to construct a path
> for a specific worktree.
>
> Signed-off-by: Brandon Williams 
> ---
>  path.c | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/path.c b/path.c
> index 76a872297..c6832a30e 100644
> --- a/path.c
> +++ b/path.c
> @@ -372,13 +372,20 @@ void report_linked_checkout_garbage(void)
>  }
>
>  static void adjust_git_path(const struct repository *repo,
> +   const struct worktree *wt,
> struct strbuf *buf, int git_dir_len)
>  {
> const char *base = buf->buf + git_dir_len;
> if (is_dir_file(base, "info", "grafts"))
> strbuf_splice(buf, 0, buf->len,
>   repo->graft_file, strlen(repo->graft_file));
> -   else if (!strcmp(base, "index"))
> +   /*
> +* Only try to replace the path '$gitdir/index' with the index file
> +* recorded in the repository when not constructing a path for a
> +* worktree.  This way we can retrieve the correct path to a 
> particular
> +* worktree's index file.
> +*/
> +   else if (!wt && !strcmp(base, "index"))
> strbuf_splice(buf, 0, buf->len,
>   repo->index_file, strlen(repo->index_file));
> else if (dir_prefix(base, "objects"))
> @@ -411,7 +418,7 @@ static void do_git_path(const struct repository *repo,
> strbuf_addch(buf, '/');
> gitdir_len = buf->len;
> strbuf_vaddf(buf, fmt, args);
> -   adjust_git_path(repo, buf, gitdir_len);
> +   adjust_git_path(repo, wt, buf, gitdir_len);
> strbuf_cleanup_path(buf);
>  }
>
> --
> 2.13.1.611.g7e3b11ae1-goog
>

[PATCH v3 11/20] path: construct correct path to a worktree's index

[Brandon Williams]

When working with worktrees the git directory is split into two part,
the per-worktree gitdir and a commondir which contains things which are
shared among all worktrees (like the object store).  With this notion of
having a split git directory, 557bd833b (git_path(): be aware of file
relocation in $GIT_DIR) and c7b3a3d2f ($GIT_COMMON_DIR: a new
environment variable) changed the way that 'git_path()' functioned so
that paths would be adjusted if they referred to files or directories
that are stored in the commondir or have been changed via an environment
variable.

One interesting problem with this is the index file as it is not shared
across worktrees yet when asking for a path to a specific worktree's
index it will be replaced with a path to the current worktree's index.
In order to prevent this, teach 'adjuct_git_path' to replace the
path to the index with the path recorded in a repository (which would be
the current, active worktree) only when not asked to construct a path
for a specific worktree.

Signed-off-by: Brandon Williams 
---
 path.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/path.c b/path.c
index 76a872297..c6832a30e 100644
--- a/path.c
+++ b/path.c
@@ -372,13 +372,20 @@ void report_linked_checkout_garbage(void)
 }
 
 static void adjust_git_path(const struct repository *repo,
+   const struct worktree *wt,
struct strbuf *buf, int git_dir_len)
 {
const char *base = buf->buf + git_dir_len;
if (is_dir_file(base, "info", "grafts"))
strbuf_splice(buf, 0, buf->len,
  repo->graft_file, strlen(repo->graft_file));
-   else if (!strcmp(base, "index"))
+   /*
+* Only try to replace the path '$gitdir/index' with the index file
+* recorded in the repository when not constructing a path for a
+* worktree.  This way we can retrieve the correct path to a particular
+* worktree's index file.
+*/
+   else if (!wt && !strcmp(base, "index"))
strbuf_splice(buf, 0, buf->len,
  repo->index_file, strlen(repo->index_file));
else if (dir_prefix(base, "objects"))
@@ -411,7 +418,7 @@ static void do_git_path(const struct repository *repo,
strbuf_addch(buf, '/');
gitdir_len = buf->len;
strbuf_vaddf(buf, fmt, args);
-   adjust_git_path(repo, buf, gitdir_len);
+   adjust_git_path(repo, wt, buf, gitdir_len);
strbuf_cleanup_path(buf);
 }
 
-- 
2.13.1.611.g7e3b11ae1-goog