diff mbox series

[2/3] split-index: accept that a base index can be empty

Message ID 81118ce106222993ef17586fb0f249d8319f3b90.1688044991.git.gitgitgadget@gmail.com (mailing list archive)
State Accepted
Commit 7667f4f0a3c2002940c0b03930597fddc8599277
Headers show
Series commit -a -m: allow the top-level tree to become empty again | expand

Commit Message

Johannes Schindelin June 29, 2023, 1:23 p.m. UTC
From: Johannes Schindelin <johannes.schindelin@gmx.de>

We are about to fix an ancient bug where `do_read_index()` pretended
that the index was not initialized when there are no index entries.

Before the `index_state` structure gained the `initialized` flag in
913e0e99b6a (unpack_trees(): protect the handcrafted in-core index from
read_cache(), 2008-08-23), that was the best we could do (even if it was
incorrect: it is totally possible to read a Git index file that contains
no index entries).

This pattern was repeated also in 998330ac2e7 (read-cache: look for
shared index files next to the index, too, 2021-08-26), which we fix
here by _not_ mistaking an empty base index for a missing
`sharedindex.*` file.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 read-cache.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

Comments

Junio C Hamano June 29, 2023, 7:02 p.m. UTC | #1
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> We are about to fix an ancient bug where `do_read_index()` pretended
> that the index was not initialized when there are no index entries.
>
> Before the `index_state` structure gained the `initialized` flag in
> 913e0e99b6a (unpack_trees(): protect the handcrafted in-core index from
> read_cache(), 2008-08-23), that was the best we could do (even if it was
> incorrect: it is totally possible to read a Git index file that contains
> no index entries).

Yeah, I very much remember how that single bit made our live much
easier.

> This pattern was repeated also in 998330ac2e7 (read-cache: look for
> shared index files next to the index, too, 2021-08-26), which we fix
> here by _not_ mistaking an empty base index for a missing
> `sharedindex.*` file.

Ahh, this is in the codepath to deal with a separate worktree.  We
allow sharing of the "sharedindex.*" file across worktrees and
entries read from the "index" files from individual worktrees to
overlay it.  But we also do allow worktrees to have their own
"sharedindex.*" file, which is what the commit in question wanted to
do, and the way it (wanted to) implement was

 - check the "gitdir" version first, as before
 - if that did not exist, then look at the one next to "index"

but "if that did not exist" was implemented incorrectly and did not
account for the case where that "gitdir" version was an empty index.

So, instead, updated code checks and reads the "gitdir" version *if*
the file exists, regardless of how many entries there are in it.

Makes sense.

> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  read-cache.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/read-cache.c b/read-cache.c
> index b10caa9831c..e15a472f54f 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -2455,12 +2455,14 @@ int read_index_from(struct index_state *istate, const char *path,
>  
>  	base_oid_hex = oid_to_hex(&split_index->base_oid);
>  	base_path = xstrfmt("%s/sharedindex.%s", gitdir, base_oid_hex);
> -	trace2_region_enter_printf("index", "shared/do_read_index",
> -				   the_repository, "%s", base_path);
> -	ret = do_read_index(split_index->base, base_path, 0);
> -	trace2_region_leave_printf("index", "shared/do_read_index",
> -				   the_repository, "%s", base_path);
> -	if (!ret) {
> +	if (file_exists(base_path)) {
> +		trace2_region_enter_printf("index", "shared/do_read_index",
> +					the_repository, "%s", base_path);
> +
> +		ret = do_read_index(split_index->base, base_path, 0);
> +		trace2_region_leave_printf("index", "shared/do_read_index",
> +					the_repository, "%s", base_path);
> +	} else {
>  		char *path_copy = xstrdup(path);
>  		char *base_path2 = xstrfmt("%s/sharedindex.%s",
>  					   dirname(path_copy), base_oid_hex);
diff mbox series

Patch

diff --git a/read-cache.c b/read-cache.c
index b10caa9831c..e15a472f54f 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2455,12 +2455,14 @@  int read_index_from(struct index_state *istate, const char *path,
 
 	base_oid_hex = oid_to_hex(&split_index->base_oid);
 	base_path = xstrfmt("%s/sharedindex.%s", gitdir, base_oid_hex);
-	trace2_region_enter_printf("index", "shared/do_read_index",
-				   the_repository, "%s", base_path);
-	ret = do_read_index(split_index->base, base_path, 0);
-	trace2_region_leave_printf("index", "shared/do_read_index",
-				   the_repository, "%s", base_path);
-	if (!ret) {
+	if (file_exists(base_path)) {
+		trace2_region_enter_printf("index", "shared/do_read_index",
+					the_repository, "%s", base_path);
+
+		ret = do_read_index(split_index->base, base_path, 0);
+		trace2_region_leave_printf("index", "shared/do_read_index",
+					the_repository, "%s", base_path);
+	} else {
 		char *path_copy = xstrdup(path);
 		char *base_path2 = xstrfmt("%s/sharedindex.%s",
 					   dirname(path_copy), base_oid_hex);