Re: [PATCH v3 1/3] read-cache: fix reading the shared index for other repos

[Duy Nguyen <pclouds@gmail.com>]

On Mon, Jan 8, 2018 at 5:30 AM, Thomas Gummerer <t.gummerer@gmail.com> wrote:
> @@ -1896,16 +1895,17 @@ int read_index_from(struct index_state *istate, const char *path)
>                 split_index->base = xcalloc(1, sizeof(*split_index->base));
>
>         base_sha1_hex = sha1_to_hex(split_index->base_sha1);
> -       base_path = git_path("sharedindex.%s", base_sha1_hex);
> +       base_path = xstrfmt("%s/sharedindex.%s", gitdir, base_sha1_hex);

Personally I prefer the repo_git_path() from v2 (sorry I was away and
could not comment anything). The thing is, git_path() and friends
could do some path translation underneath to support multiple
worktrees. Think of the given path here as a "virtual path" that may
be translated to something else, not exactly <git_dir> + "/" +
"sharedindex.%s". But in practice, we're not breaking the relationship
between $GIT_DIR/index and $GIT_DIR/sharedindex.* any time soon, doing
manual path transformation here is fine.

What about the other git_path() in this file? With patch applied I still get

> ~/w/git/temp $ git grep git_path read-cache.c
read-cache.c:           shared_index_path = git_path("%s", de->d_name);
read-cache.c:   temp = mks_tempfile(git_path("sharedindex_XXXXXX"));
read-cache.c:                         git_path("sharedindex.%s",
sha1_to_hex(si->base->sha1)));
read-cache.c:           const char *shared_index = git_path("sharedindex.%s",

I suppose submodule has not triggered any of these code paths yet. Not
sure if we should deal with them now or wait until later.

Perhaps if we add a "struct repository *" pointer inside index_state,
we could retrieve back the_repository (or others) and call
repo_git_path() everywhere without changing index api too much. I
don't know. I like the  'struct repository' concept but couldn't
follow its development so I don't if this is what it should become.
-- 
Duy