From: Thomas Gummerer <t.gummerer@gmail.com>
To: Brandon Williams <bmwill@google.com>
Cc: git@vger.kernel.org, "Christian Couder" <chriscool@tuxfamily.org>,
"Lars Schneider" <larsxschneider@gmail.com>,
"Jeff King" <peff@peff.net>, "Junio C Hamano" <gitster@pobox.com>,
"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: Re: [PATCH 1/3] repository: fix repo_read_index with submodules
Date: Mon, 11 Dec 2017 20:37:07 +0000 [thread overview]
Message-ID: <20171211203707.GB25616@hank> (raw)
In-Reply-To: <20171211185406.GA177995@google.com>
On 12/11, Brandon Williams wrote:
> On 12/10, Thomas Gummerer wrote:
> > repo_read_index calls read_index_from, which takes an path argument for
> > the location of the index file. For the split index however it relies
> > on the current working directory to construct the path using git_path.
> >
> > repo_read_index calls read_index_from with the full path for the index
> > file, however it doesn't change the cwd, so when split index mode is
> > turned on, read_index_from can't find the file for the split index.
> >
> > For example t3007-ls-files-recurse-submodules.sh was broken with
> > GIT_TEST_SPLIT_INDEX set in 188dce131f ("ls-files: use repository
> > object", 2017-06-22), and t7814-grep-recurse-submodules.sh was also
> > broken in a similar manner, probably by introducing struct repository
> > there, although I didn't track down the exact commit for that.
> >
> > Fix this by introducing a new read_index_for_repo function, which knows
> > about the correct paths for the submodules.
> >
> > The alternative would have been to make the callers pass in the base
> > path for the split index, however that ended up being more complicated,
> > and I think we want to converge towards using struct repository for
> > things like these anyway.
>
> Thanks for catching this, I'm not a user of split index myself which is
> why I unfortunately overlooked this. Definitely a good change. I
> really only have one nit below.
Me neither, I just remember to run the split index tests every once in
a while, which is also why it's taken so long to catch this.
Hopefully after we fixed this we can get travis to run the tests,
which should help :)
> >
> > Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
> > ---
> > cache.h | 1 +
> > read-cache.c | 19 +++++++++++++++++--
> > repository.c | 2 +-
> > 3 files changed, 19 insertions(+), 3 deletions(-)
> >
> > diff --git a/cache.h b/cache.h
> > index cb5db7bf83..d42bea1ef7 100644
> > --- a/cache.h
> > +++ b/cache.h
> > @@ -614,6 +614,7 @@ extern int read_index_preload(struct index_state *, const struct pathspec *paths
> > extern int do_read_index(struct index_state *istate, const char *path,
> > int must_exist); /* for testting only! */
> > extern int read_index_from(struct index_state *, const char *path);
> > +extern int read_index_for_repo(const struct repository *);
> > extern int is_index_unborn(struct index_state *);
> > extern int read_index_unmerged(struct index_state *);
> >
> > diff --git a/read-cache.c b/read-cache.c
> > index 2eb81a66b9..4d5c4ad79b 100644
> > --- a/read-cache.c
> > +++ b/read-cache.c
> > @@ -20,6 +20,7 @@
> > #include "split-index.h"
> > #include "utf8.h"
> > #include "fsmonitor.h"
> > +#include "repository.h"
> >
> > /* Mask for the name length in ce_flags in the on-disk index */
> >
> > @@ -1871,7 +1872,8 @@ static void freshen_shared_index(char *base_sha1_hex, int warn)
> > free(shared_index);
> > }
> >
> > -int read_index_from(struct index_state *istate, const char *path)
> > +static int do_read_index_from(struct index_state *istate, const char *path,
> > + const struct repository *repo)
> > {
> > struct split_index *split_index;
> > int ret;
> > @@ -1896,7 +1898,10 @@ 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);
> > + if (repo)
> > + base_path = repo_git_path(repo, "sharedindex.%s", base_sha1_hex);
> > + else
> > + base_path = git_path("sharedindex.%s", base_sha1_hex);
> > ret = do_read_index(split_index->base, base_path, 1);
> > if (hashcmp(split_index->base_sha1, split_index->base->sha1))
> > die("broken index, expect %s in %s, got %s",
> > @@ -1909,6 +1914,16 @@ int read_index_from(struct index_state *istate, const char *path)
> > return ret;
> > }
> >
> > +int read_index_for_repo(const struct repository *repo)
> > +{
> > + return do_read_index_from(repo->index, repo->index_file, repo);
> > +}
> > +
> > +int read_index_from(struct index_state *istate, const char *path)
> > +{
> > + return do_read_index_from(istate, path, NULL);
> > +}
>
> Instead of passing NULL and having to special case it in
> 'do_read_index_from()', how about we pass in 'the_repository'?
I think that makes sense the only way this function used to work with
split index turned on is if it's called from the main repository, so
just passing through 'the_repository' would have the function behave
the exact same way as before.
> > +
> > int is_index_unborn(struct index_state *istate)
> > {
> > return (!istate->cache_nr && !istate->timestamp.sec);
> > diff --git a/repository.c b/repository.c
> > index bb2fae5446..928b1f553d 100644
> > --- a/repository.c
> > +++ b/repository.c
> > @@ -229,5 +229,5 @@ int repo_read_index(struct repository *repo)
> > if (!repo->index)
> > repo->index = xcalloc(1, sizeof(*repo->index));
> >
> > - return read_index_from(repo->index, repo->index_file);
> > + return read_index_for_repo(repo);
> > }
> > --
> > 2.15.1.504.g5279b80103
> >
>
> --
> Brandon Williams
next prev parent reply other threads:[~2017-12-11 20:35 UTC|newest]
Thread overview: 95+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-10 21:21 [PATCH 0/3] fixes for split index mode Thomas Gummerer
2017-12-10 21:22 ` [PATCH 1/3] repository: fix repo_read_index with submodules Thomas Gummerer
2017-12-11 18:54 ` Brandon Williams
2017-12-11 20:37 ` Thomas Gummerer [this message]
2017-12-10 21:22 ` [PATCH 2/3] prune: fix pruning with multiple worktrees and split index Thomas Gummerer
2017-12-11 19:09 ` Brandon Williams
2017-12-11 21:39 ` Thomas Gummerer
2017-12-10 21:22 ` [PATCH 3/3] travis: run tests with GIT_TEST_SPLIT_INDEX Thomas Gummerer
2017-12-10 23:37 ` Eric Sunshine
2017-12-11 21:09 ` SZEDER Gábor
2017-12-11 21:42 ` Thomas Gummerer
2017-12-12 15:54 ` Lars Schneider
2017-12-12 19:15 ` Junio C Hamano
2017-12-12 20:15 ` Thomas Gummerer
2017-12-12 20:51 ` Junio C Hamano
2017-12-13 23:21 ` Thomas Gummerer
2017-12-13 17:21 ` Lars Schneider
2017-12-13 17:38 ` Junio C Hamano
2017-12-13 17:46 ` Lars Schneider
2017-12-13 23:28 ` Thomas Gummerer
2017-12-17 22:51 ` [PATCH v2 0/3] fixes for split index mode Thomas Gummerer
2017-12-17 22:51 ` [PATCH v2 1/3] repository: fix repo_read_index with submodules Thomas Gummerer
2017-12-18 18:01 ` Brandon Williams
2017-12-18 23:05 ` Thomas Gummerer
2017-12-18 23:05 ` Brandon Williams
2017-12-17 22:51 ` [PATCH v2 2/3] prune: fix pruning with multiple worktrees and split index Thomas Gummerer
2017-12-18 18:19 ` Brandon Williams
2018-01-03 22:18 ` Thomas Gummerer
2018-01-04 19:12 ` Junio C Hamano
2017-12-17 22:51 ` [PATCH v2 3/3] travis: run tests with GIT_TEST_SPLIT_INDEX Thomas Gummerer
2017-12-18 18:16 ` Lars Schneider
2018-01-04 20:13 ` Thomas Gummerer
2018-01-05 11:03 ` Lars Schneider
2018-01-07 20:02 ` Thomas Gummerer
2018-01-07 22:30 ` [PATCH v3 0/3] fixes for split index mode Thomas Gummerer
2018-01-07 22:30 ` [PATCH v3 1/3] read-cache: fix reading the shared index for other repos Thomas Gummerer
2018-01-08 10:41 ` Duy Nguyen
2018-01-08 22:41 ` Thomas Gummerer
2018-01-13 22:33 ` Thomas Gummerer
2018-01-08 23:38 ` Brandon Williams
2018-01-09 1:24 ` Duy Nguyen
2018-01-16 21:42 ` Brandon Williams
2018-01-17 0:16 ` Duy Nguyen
2018-01-17 0:32 ` Brandon Williams
2018-01-17 18:16 ` Jonathan Nieder
2018-01-18 10:19 ` Duy Nguyen
2018-01-19 21:57 ` Junio C Hamano
2018-01-20 11:58 ` Thomas Gummerer
2018-01-22 6:14 ` Junio C Hamano
2018-01-27 12:18 ` Thomas Gummerer
2018-02-07 22:41 ` Junio C Hamano
2018-01-07 22:30 ` [PATCH v3 2/3] split-index: don't write cache tree with null oid entries Thomas Gummerer
2018-01-07 22:30 ` [PATCH v3 3/3] travis: run tests with GIT_TEST_SPLIT_INDEX Thomas Gummerer
2018-01-13 22:37 ` [PATCH v3 4/3] read-cache: don't try to write index if we can't write shared index Thomas Gummerer
2018-01-14 9:36 ` Duy Nguyen
2018-01-14 10:18 ` [PATCH 1/3] read-cache.c: change type of "temp" in write_shared_index() Nguyễn Thái Ngọc Duy
2018-01-14 10:18 ` [PATCH 2/3] read-cache.c: move tempfile creation/cleanup out of write_shared_index Nguyễn Thái Ngọc Duy
2018-01-14 10:18 ` [PATCH 3/3] read-cache: don't write index twice if we can't write shared index Nguyễn Thái Ngọc Duy
2018-01-18 11:36 ` SZEDER Gábor
2018-01-18 12:47 ` Duy Nguyen
2018-01-18 13:29 ` Jeff King
2018-01-18 13:36 ` Duy Nguyen
2018-01-18 15:00 ` Duy Nguyen
2018-01-18 21:37 ` Jeff King
2018-01-18 22:32 ` SZEDER Gábor
2018-01-19 0:30 ` Duy Nguyen
2018-01-22 13:32 ` [PATCH 0/5] Travis CI: don't run the test suite as root in the 32 bit Linux build SZEDER Gábor
2018-01-22 13:32 ` [PATCH 1/5] travis-ci: use 'set -x' for the commands under 'su' " SZEDER Gábor
2018-01-22 13:32 ` [PATCH 2/5] travis-ci: use 'set -e' in the 32 bit Linux build job SZEDER Gábor
2018-01-23 16:26 ` Jeff King
2018-01-23 16:32 ` Jeff King
2018-01-24 12:12 ` SZEDER Gábor
2018-01-24 15:49 ` Jeff King
2018-01-22 13:32 ` [PATCH 3/5] travis-ci: don't repeat the path of the cache directory SZEDER Gábor
2018-01-23 16:30 ` Jeff King
2018-01-24 13:14 ` SZEDER Gábor
2018-01-22 13:32 ` [PATCH 4/5] travis-ci: don't run the test suite as root in the 32 bit Linux build SZEDER Gábor
2018-01-23 16:43 ` Jeff King
2018-01-24 13:45 ` SZEDER Gábor
2018-01-24 15:56 ` Jeff King
2018-01-24 18:01 ` Jeff King
2018-01-24 19:51 ` Jeff King
2018-01-22 13:32 ` [PATCH 5/5] travis-ci: don't fail if user already exists on 32 bit Linux build job SZEDER Gábor
2018-01-23 16:46 ` Jeff King
2018-01-24 0:32 ` Duy Nguyen
2018-01-24 19:39 ` SZEDER Gábor
2018-01-22 18:27 ` [PATCH 3/3] read-cache: don't write index twice if we can't write shared index SZEDER Gábor
2018-01-22 19:46 ` Eric Sunshine
2018-01-22 22:10 ` SZEDER Gábor
2018-01-24 9:11 ` Duy Nguyen
2018-01-26 22:44 ` Lars Schneider
2018-01-14 14:29 ` [PATCH v3 4/3] read-cache: don't try to write index " Thomas Gummerer
2018-01-18 21:53 ` [PATCH v3 0/3] fixes for split index mode Thomas Gummerer
2018-01-19 18:34 ` Junio C Hamano
2018-01-19 21:11 ` Thomas Gummerer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171211203707.GB25616@hank \
--to=t.gummerer@gmail.com \
--cc=bmwill@google.com \
--cc=chriscool@tuxfamily.org \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=larsxschneider@gmail.com \
--cc=pclouds@gmail.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).