From: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Jeff Hostetler <git@jeffhostetler.com>,
Johannes Schindelin <johannes.schindelin@gmx.de>,
Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: [PATCH v2 2/4] split-index; stop abusing the `base_oid` to strip the "link" extension
Date: Sun, 26 Mar 2023 22:45:41 +0000 [thread overview]
Message-ID: <8cc075f63252206972880d0167b694ed86ec0311.1679870743.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1497.v2.git.1679870743.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
When a split-index is in effect, the `$GIT_DIR/index` file needs to
contain a "link" extension that contains all the information about the
split-index, including the information about the shared index.
However, in some cases Git needs to suppress writing that "link"
extension (i.e. to fall back to writing a full index) even if the
in-memory index structure _has_ a `split_index` configured. This is the
case e.g. when "too many not shared" index entries exist.
In such instances, the current code sets the `base_oid` field of said
`split_index` structure to all-zero to indicate that `do_write_index()`
should skip writing the "link" extension.
This can lead to problems later on, when the in-memory index is still
used to perform other operations and eventually wants to write a
split-index, detects the presence of the `split_index` and reuses that,
too (under the assumption that it has been initialized correctly and
still has a non-null `base_oid`).
Let's stop zeroing out the `base_oid` to indicate that the "link"
extension should not be written.
One might be tempted to simply call `discard_split_index()` instead,
under the assumption that Git decided to write a non-split index and
therefore the `split_index` structure might no longer be wanted.
However, that is not possible because that would release index entries
in `split_index->base` that are likely to still be in use. Therefore we
cannot do that.
The next best thing we _can_ do is to introduce a bit field to indicate
specifically which index extensions (not) to write. So that's what we do
here.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
read-cache.c | 49 +++++++++++++++++++++++-------------
t/t7527-builtin-fsmonitor.sh | 2 +-
2 files changed, 33 insertions(+), 18 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index b09128b1884..7c9a0eeeac8 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2868,6 +2868,16 @@ static int record_ieot(void)
return !git_config_get_index_threads(&val) && val != 1;
}
+enum write_extensions {
+ WRITE_NO_EXTENSION = 0,
+ WRITE_SPLIT_INDEX_EXTENSION = 1<<0,
+ WRITE_CACHE_TREE_EXTENSION = 1<<1,
+ WRITE_RESOLVE_UNDO_EXTENSION = 1<<2,
+ WRITE_UNTRACKED_CACHE_EXTENSION = 1<<3,
+ WRITE_FSMONITOR_EXTENSION = 1<<4,
+};
+#define WRITE_ALL_EXTENSIONS ((enum write_extensions)-1)
+
/*
* On success, `tempfile` is closed. If it is the temporary file
* of a `struct lock_file`, we will therefore effectively perform
@@ -2876,7 +2886,7 @@ static int record_ieot(void)
* rely on it.
*/
static int do_write_index(struct index_state *istate, struct tempfile *tempfile,
- int strip_extensions, unsigned flags)
+ enum write_extensions write_extensions, unsigned flags)
{
uint64_t start = getnanotime();
struct hashfile *f;
@@ -3045,8 +3055,8 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,
return -1;
}
- if (!strip_extensions && istate->split_index &&
- !is_null_oid(&istate->split_index->base_oid)) {
+ if (write_extensions & WRITE_SPLIT_INDEX_EXTENSION &&
+ istate->split_index) {
struct strbuf sb = STRBUF_INIT;
if (istate->sparse_index)
@@ -3060,7 +3070,8 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,
if (err)
return -1;
}
- if (!strip_extensions && !drop_cache_tree && istate->cache_tree) {
+ if (write_extensions & WRITE_CACHE_TREE_EXTENSION &&
+ !drop_cache_tree && istate->cache_tree) {
struct strbuf sb = STRBUF_INIT;
cache_tree_write(&sb, istate->cache_tree);
@@ -3070,7 +3081,8 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,
if (err)
return -1;
}
- if (!strip_extensions && istate->resolve_undo) {
+ if (write_extensions & WRITE_RESOLVE_UNDO_EXTENSION &&
+ istate->resolve_undo) {
struct strbuf sb = STRBUF_INIT;
resolve_undo_write(&sb, istate->resolve_undo);
@@ -3081,7 +3093,8 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,
if (err)
return -1;
}
- if (!strip_extensions && istate->untracked) {
+ if (write_extensions & WRITE_UNTRACKED_CACHE_EXTENSION &&
+ istate->untracked) {
struct strbuf sb = STRBUF_INIT;
write_untracked_extension(&sb, istate->untracked);
@@ -3092,7 +3105,8 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,
if (err)
return -1;
}
- if (!strip_extensions && istate->fsmonitor_last_update) {
+ if (write_extensions & WRITE_FSMONITOR_EXTENSION &&
+ istate->fsmonitor_last_update) {
struct strbuf sb = STRBUF_INIT;
write_fsmonitor_extension(&sb, istate);
@@ -3166,8 +3180,10 @@ static int commit_locked_index(struct lock_file *lk)
return commit_lock_file(lk);
}
-static int do_write_locked_index(struct index_state *istate, struct lock_file *lock,
- unsigned flags)
+static int do_write_locked_index(struct index_state *istate,
+ struct lock_file *lock,
+ unsigned flags,
+ enum write_extensions write_extensions)
{
int ret;
int was_full = istate->sparse_index == INDEX_EXPANDED;
@@ -3185,7 +3201,7 @@ static int do_write_locked_index(struct index_state *istate, struct lock_file *l
*/
trace2_region_enter_printf("index", "do_write_index", the_repository,
"%s", get_lock_file_path(lock));
- ret = do_write_index(istate, lock->tempfile, 0, flags);
+ ret = do_write_index(istate, lock->tempfile, write_extensions, flags);
trace2_region_leave_printf("index", "do_write_index", the_repository,
"%s", get_lock_file_path(lock));
@@ -3214,7 +3230,7 @@ static int write_split_index(struct index_state *istate,
{
int ret;
prepare_to_write_split_index(istate);
- ret = do_write_locked_index(istate, lock, flags);
+ ret = do_write_locked_index(istate, lock, flags, WRITE_ALL_EXTENSIONS);
finish_writing_split_index(istate);
return ret;
}
@@ -3289,7 +3305,7 @@ static int write_shared_index(struct index_state *istate,
trace2_region_enter_printf("index", "shared/do_write_index",
the_repository, "%s", get_tempfile_path(*temp));
- ret = do_write_index(si->base, *temp, 1, flags);
+ ret = do_write_index(si->base, *temp, WRITE_NO_EXTENSION, flags);
trace2_region_leave_printf("index", "shared/do_write_index",
the_repository, "%s", get_tempfile_path(*temp));
@@ -3366,9 +3382,8 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock,
if ((!si && !test_split_index_env) ||
alternate_index_output ||
(istate->cache_changed & ~EXTMASK)) {
- if (si)
- oidclr(&si->base_oid);
- ret = do_write_locked_index(istate, lock, flags);
+ ret = do_write_locked_index(istate, lock, flags,
+ ~WRITE_SPLIT_INDEX_EXTENSION);
goto out;
}
@@ -3394,8 +3409,8 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock,
/* Same initial permissions as the main .git/index file */
temp = mks_tempfile_sm(git_path("sharedindex_XXXXXX"), 0, 0666);
if (!temp) {
- oidclr(&si->base_oid);
- ret = do_write_locked_index(istate, lock, flags);
+ ret = do_write_locked_index(istate, lock, flags,
+ ~WRITE_SPLIT_INDEX_EXTENSION);
goto out;
}
ret = write_shared_index(istate, &temp, flags);
diff --git a/t/t7527-builtin-fsmonitor.sh b/t/t7527-builtin-fsmonitor.sh
index cbafdd69602..9fab9a2ab38 100755
--- a/t/t7527-builtin-fsmonitor.sh
+++ b/t/t7527-builtin-fsmonitor.sh
@@ -1003,7 +1003,7 @@ test_expect_success !UNICODE_COMPOSITION_SENSITIVE 'Unicode nfc/nfd' '
egrep "^event: nfd/d_${utf8_nfc}/?$" ./unicode.trace
'
-test_expect_failure 'split-index and FSMonitor work well together' '
+test_expect_success 'split-index and FSMonitor work well together' '
git init split-index &&
test_when_finished "git -C \"$PWD/split-index\" \
fsmonitor--daemon stop" &&
--
gitgitgadget
next prev parent reply other threads:[~2023-03-26 22:47 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-22 16:00 [PATCH 0/4] Fix a few split-index bugs Johannes Schindelin via GitGitGadget
2023-03-22 16:00 ` [PATCH 1/4] split-index & fsmonitor: demonstrate a bug Johannes Schindelin via GitGitGadget
2023-03-23 13:39 ` Jeff Hostetler
2023-03-22 16:00 ` [PATCH 2/4] split-index; stop abusing the `base_oid` to strip the "link" extension Johannes Schindelin via GitGitGadget
2023-03-22 21:24 ` Junio C Hamano
2023-03-23 13:59 ` Jeff Hostetler
2023-03-23 16:06 ` Junio C Hamano
2023-03-23 15:22 ` Jeff Hostetler
2023-03-22 16:00 ` [PATCH 3/4] fsmonitor: avoid overriding `cache_changed` bits Johannes Schindelin via GitGitGadget
2023-03-22 16:00 ` [PATCH 4/4] unpack-trees: take care to propagate the split-index flag Johannes Schindelin via GitGitGadget
2023-03-23 14:32 ` Jeff Hostetler
2023-03-22 16:22 ` [PATCH 0/4] Fix a few split-index bugs Junio C Hamano
2023-03-26 22:45 ` [PATCH v2 " Johannes Schindelin via GitGitGadget
2023-03-26 22:45 ` [PATCH v2 1/4] split-index & fsmonitor: demonstrate a bug Johannes Schindelin via GitGitGadget
2023-03-26 22:45 ` Johannes Schindelin via GitGitGadget [this message]
2023-03-26 22:45 ` [PATCH v2 3/4] fsmonitor: avoid overriding `cache_changed` bits Johannes Schindelin via GitGitGadget
2023-03-26 22:45 ` [PATCH v2 4/4] unpack-trees: take care to propagate the split-index flag Johannes Schindelin via GitGitGadget
2023-03-27 14:48 ` [PATCH v2 0/4] Fix a few split-index bugs Jeff Hostetler
2023-03-27 16:42 ` Junio C Hamano
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=8cc075f63252206972880d0167b694ed86ec0311.1679870743.git.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail.com \
--cc=git@jeffhostetler.com \
--cc=git@vger.kernel.org \
--cc=johannes.schindelin@gmx.de \
/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).