From patchwork Wed Mar 22 16:00:56 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Johannes Schindelin X-Patchwork-Id: 13184250 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6FDA7C6FD1F for ; Wed, 22 Mar 2023 16:01:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229799AbjCVQBG (ORCPT ); Wed, 22 Mar 2023 12:01:06 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33906 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229986AbjCVQBF (ORCPT ); Wed, 22 Mar 2023 12:01:05 -0400 Received: from mail-wr1-x434.google.com (mail-wr1-x434.google.com [IPv6:2a00:1450:4864:20::434]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 970B050F93 for ; Wed, 22 Mar 2023 09:01:03 -0700 (PDT) Received: by mail-wr1-x434.google.com with SMTP id v1so11524695wrv.1 for ; Wed, 22 Mar 2023 09:01:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1679500862; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=J8QhUNEFReiCSgkZDDHAbi0jGT1Xpb/cllleVu/3f1k=; b=HCqUANxfLmxh0V7nzptTEkLT+b5W0F3WvSXTir/2Gxli3qXgDMl/igGQwHsafOd4aI 4skI5N/Gqf32RVy7//aOK+k7BdQswkl+MaxBgWq8Epp82VCcAT9TtLDi3QS4+Cf7Ddeu bpunGw04ZeHx9bXUljRbpjKaz9KpIR/kdFUfOmEDy+W/eS9B13OfWCGn98Fdiz5wj3dp 4ZfZLuFuomfkuAsfv0hGP+XNBRMyjicIWfuJvCuSZ4AxqUAyDQhw0v8bBuFwChxts18Y ITW8tgLPmFJqqSHx2r/+0FBKvFsXlxi+csYsb0ney3aUBDyma8knngcatQLqD7PN9Sgq 3fwQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679500862; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=J8QhUNEFReiCSgkZDDHAbi0jGT1Xpb/cllleVu/3f1k=; b=mu6ExDa925qWIMydGPc+98DrEXQ4rl+Q8A5sklvOvlqI770AY/7NH14Bg8HlgcfTPy o6BoYxkAGhNcwyHoMAA0Ix5v9Tb9qWr81KCmay7NzC5udO4/x2ZYR/TQsA9BCF0DpK8/ HrLtsX59JYvfFUlDxqDGEtUs79euDM/dO3hhzvTMG9a1PJd85hJiWp5zJtqwsrbAz5NA o74qIl5ch/hCRIUcKLhWUcLMhZ/gIfxwOzupyK1bwDK57Wx76cAzpJVqsBTVAejFeYa8 AHt3tfADbNFGXzMPzQsNjeo28saWSqpn47acxzDjACvlZysgVvI4KD+wetuYbI8R/994 FreA== X-Gm-Message-State: AAQBX9cjrjIDhsQDmQNzwQO9isRVGZCX/KcI/Opwr3/LjDO2EnXSqsYW /dd3oWcKOEVjCp03PIkxpTdSQpA55/0= X-Google-Smtp-Source: AKy350bSqv5ypHeam1HRHdYHedkidDcUCoQO7gXd3Mr/9ghuqh5Eq9rAZ0tItd+9I0hx2TuMPVL11g== X-Received: by 2002:a5d:6510:0:b0:2c5:4ca3:d56c with SMTP id x16-20020a5d6510000000b002c54ca3d56cmr185140wru.0.1679500861958; Wed, 22 Mar 2023 09:01:01 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id k2-20020a5d6e82000000b002c55b0e6ef1sm14346380wrz.4.2023.03.22.09.01.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 22 Mar 2023 09:01:01 -0700 (PDT) Message-Id: In-Reply-To: References: Date: Wed, 22 Mar 2023 16:00:56 +0000 Subject: [PATCH 1/4] split-index & fsmonitor: demonstrate a bug Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Jeff Hostetler , Johannes Schindelin , Johannes Schindelin Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Johannes Schindelin From: Johannes Schindelin This commit adds a new test case that demonstrates a bug in the split-index code that is triggered under certain circumstances when the FSMonitor is enabled, and its symptom manifests in the form of one of the following error messages: BUG: fsmonitor.c:20: fsmonitor_dirty has more entries than the index (2 > 1) BUG: unpack-trees.c:776: pos doesn't point to the first entry of / in index error: invalid path '' error: The following untracked working tree files would be overwritten by reset: initial.t Which of these error messages appears depends on timing-dependent conditions. Technically the root cause lies with a bug in the split-index code that has nothing to do with FSMonitor, but for the sake of this new test case it was the easiest way to trigger the bug. The bug is this: Under specific conditions, Git needs to skip writing the "link" extension (which is the index extension containing the information pertaining to the split-index). To do that, the `base_oid` attribute of the `split_index` structure in the in-memory index is zeroed out, and `do_write_index()` specifically checks for a "null" `base_oid` to understand that the "link" extension should not be written. However, this violates the consistency of the in-memory index structure, but that does not cause problems in most cases because the process exits without using the in-memory index structure anymore, anyway. But: _When_ the in-memory index is still used (which is the case e.g. in `git rebase`), subsequent writes of `the_index` are at risk of writing out a bogus index file, one that _should_ have a "link" extension but does not. In many cases, the `SPLIT_INDEX_ORDERED` flag _happens_ to be set for subsequent writes, forcing the shared index to be written, which re-initializes `base_oid` to a non-bogus state, and all is good. When it is _not_ set, however, all kinds of mayhem ensue, resulting in above-mentioned error messages, and often enough putting worktrees in a totally broken state where the only recourse is to manually delete the `index` and the `index.lock` files and then call `git reset` manually. Not something to ask users to do. The reason why it is comparatively easy to trigger the bug with FSMonitor is that there is _another_ bug in the FSMonitor code: `mark_fsmonitor_valid()` sets `cache_changed` to 1, i.e. treating that variable as a Boolean. But it is a bit field, and 1 happens to be the `SOMETHING_CHANGED` bit that forces the "link" extension to be skipped when writing the index, among other things. "Comparatively easy" is a relative term in this context, for sure. The essence of how the new test case triggers the bug is as following: 1. The `git rebase` invocation will first reset the worktree to a commit that contains only the `one.t` file, and then execute a rebase script that starts with the following commands (commit hashes skipped): label onto reset initial pick two label two reset two pick three [...] 2. Before executing the `label` command, a split index is written, as well as the shared index. 3. The `reset initial` command in the rebase script writes out a new split index but skips writing the shared index, as intended. 4. The `pick two` command updates the worktree and refreshes the index, marking the `two.t` entry as valid via the FSMonitor, which sets the `SOMETHING_CHANGED` bit in `cache_changed`, which in turn causes the `base_oid` attribute to be zeroed out and a full (non-split) index to be written (making sure _not_ to write the "link" extension). 5. Now, the `reset two` command will leave the worktree alone, but still write out a new split index, not writing the shared index (because `base_oid` is still zeroed out, and there is no index entry update requiring it to be written, either). 6. When it is turn to run `pick three`, the index is read, but it is too short: It only contains a single entry when there should be two, because the "link" extension is missing from the written-out index file. There are three bugs at play, actually, which will be fixed over the course of the next commits: - The `base_oid` attribute should not be zeroed out to indicate when the "link" extension should not be written, as it puts the in-memory index structure into an inconsistent state. - The FSMonitor should not overwrite bits in `cache_changed`. - The `unpack_trees()` function tries to reuse the `split_index` structure from the source index, if any, but does not propagate the `SPLIT_INDEX_ORDERED` flag. While a fix for the second bug would let this test case pass, there are other conditions where the `SOMETHING_CHANGED` bit is set. Therefore, the bug that most crucially needs to be fixed is the first one. Signed-off-by: Johannes Schindelin --- t/t7527-builtin-fsmonitor.sh | 37 ++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/t/t7527-builtin-fsmonitor.sh b/t/t7527-builtin-fsmonitor.sh index d419085379c..cbafdd69602 100755 --- a/t/t7527-builtin-fsmonitor.sh +++ b/t/t7527-builtin-fsmonitor.sh @@ -1003,4 +1003,41 @@ 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' ' + git init split-index && + test_when_finished "git -C \"$PWD/split-index\" \ + fsmonitor--daemon stop" && + ( + cd split-index && + git config core.splitIndex true && + # force split-index in most cases + git config splitIndex.maxPercentChange 99 && + git config core.fsmonitor true && + + # Create the following commit topology: + # + # * merge three + # |\ + # | * three + # * | merge two + # |\| + # | * two + # * | one + # |/ + # * 5a5efd7 initial + + test_commit initial && + test_commit two && + test_commit three && + git reset --hard initial && + test_commit one && + test_tick && + git merge two && + test_tick && + git merge three && + + git rebase --force-rebase -r one + ) +' + test_done From patchwork Wed Mar 22 16:00:57 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Johannes Schindelin X-Patchwork-Id: 13184251 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7FF3FC76195 for ; Wed, 22 Mar 2023 16:01:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230012AbjCVQBH (ORCPT ); Wed, 22 Mar 2023 12:01:07 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33930 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230047AbjCVQBF (ORCPT ); Wed, 22 Mar 2023 12:01:05 -0400 Received: from mail-wm1-x332.google.com (mail-wm1-x332.google.com [IPv6:2a00:1450:4864:20::332]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6108037707 for ; Wed, 22 Mar 2023 09:01:04 -0700 (PDT) Received: by mail-wm1-x332.google.com with SMTP id o40-20020a05600c512800b003eddedc47aeso6263464wms.3 for ; Wed, 22 Mar 2023 09:01:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1679500863; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=4y5Cx5Ly972ypwoXXjEuCJjboDkjsHrxc4c8nogbdt8=; b=W2fP6zAr0C+FAMsErCdgs5fVmG0LXaszrsNyiaPEADUC0zZNR3Dkj2XLTlOW1dBzlI gnQl/fe1moddP5WlbysV5Z7mk8JnPVt3jH0FCuianarK1/QSdczs17X6oHGXLD20PZUJ vAcqWicly6GuCP93SGYfmvU6GG5WirwLVJ3bKTmGBIb1zssO8nl7hYbi39tbvK8rEWY+ nt/Q3jIfLtamf1HHM/w9+hglNbD1/bR9p7tJYGSlzPjLEf8tPdMxVZ7qtQOBL9gfMJGB hH+e1IYWFW1nV0Wx7GWapNaJ4cRB14SnUekuoI4PxvuK0z4VtIiEGaoQ1YxSMzT6U8tO 6y8Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679500863; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=4y5Cx5Ly972ypwoXXjEuCJjboDkjsHrxc4c8nogbdt8=; b=TA3OLkZQNb1uc41xUq7MNlyXNSuf0qsvLBOh3sceGE498tvRgoxvZQluZTHohPkEx4 Td87yw5xzHFgyNEeWdSkvlOky8+loGpTT0yzLNM4hE+ND7JMGfS5szLJb5PsxFASRNyo 2VBacA4Bkac89A3z1ctpiJY4T5dk2q4mrqjAAXqQFFWSYNO78Yl33dFZCpIB3XBFcfty pbK+SRFbnljhpzHlBn67oKgxFoOAgcmIfcQRDBHYT1baT7Dv26fCjIOp8TWjCFZyGWTq gCnZUMX0donZSPYnOUKgDMq98LtAXculDxTvA4ljIywkoXAkyAJaoRo2wHCILnjPmaUl iJSQ== X-Gm-Message-State: AO0yUKVDso2h0BL0osarERjMLmbwKrrvkuz25pvnKHNoNJURIpHqNTQU p7lFi8qbis6H8CoF66Ri0v8N/q+xUms= X-Google-Smtp-Source: AK7set+mUzKzomb0FHxm/OJ1dJEJNlt4dhJChn6yDRn0sb4BZpl1xcUoXdNOyDnrRxeJ9yRaN/Wz/Q== X-Received: by 2002:a05:600c:218:b0:3ee:2552:7512 with SMTP id 24-20020a05600c021800b003ee25527512mr43997wmi.13.1679500862554; Wed, 22 Mar 2023 09:01:02 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id iv16-20020a05600c549000b003ee1acdaf95sm7488990wmb.36.2023.03.22.09.01.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 22 Mar 2023 09:01:02 -0700 (PDT) Message-Id: In-Reply-To: References: Date: Wed, 22 Mar 2023 16:00:57 +0000 Subject: [PATCH 2/4] split-index; stop abusing the `base_oid` to strip the "link" extension Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Jeff Hostetler , Johannes Schindelin , Johannes Schindelin Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Johannes Schindelin From: Johannes Schindelin 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 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 flag, specifically indicating when the "link" extension should be skipped. So that's what we do here. Signed-off-by: Johannes Schindelin --- read-cache.c | 37 ++++++++++++++++++++++-------------- t/t7527-builtin-fsmonitor.sh | 2 +- 2 files changed, 24 insertions(+), 15 deletions(-) diff --git a/read-cache.c b/read-cache.c index b09128b1884..8fcb2d54c05 100644 --- a/read-cache.c +++ b/read-cache.c @@ -2868,6 +2868,12 @@ static int record_ieot(void) return !git_config_get_index_threads(&val) && val != 1; } +enum strip_extensions { + WRITE_ALL_EXTENSIONS = 0, + STRIP_ALL_EXTENSIONS = 1, + STRIP_LINK_EXTENSION_ONLY = 2 +}; + /* * On success, `tempfile` is closed. If it is the temporary file * of a `struct lock_file`, we will therefore effectively perform @@ -2876,7 +2882,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 strip_extensions strip_extensions, unsigned flags) { uint64_t start = getnanotime(); struct hashfile *f; @@ -3045,7 +3051,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile, return -1; } - if (!strip_extensions && istate->split_index && + if (strip_extensions == WRITE_ALL_EXTENSIONS && istate->split_index && !is_null_oid(&istate->split_index->base_oid)) { struct strbuf sb = STRBUF_INIT; @@ -3060,7 +3066,7 @@ 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 (strip_extensions != STRIP_ALL_EXTENSIONS && !drop_cache_tree && istate->cache_tree) { struct strbuf sb = STRBUF_INIT; cache_tree_write(&sb, istate->cache_tree); @@ -3070,7 +3076,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile, if (err) return -1; } - if (!strip_extensions && istate->resolve_undo) { + if (strip_extensions != STRIP_ALL_EXTENSIONS && istate->resolve_undo) { struct strbuf sb = STRBUF_INIT; resolve_undo_write(&sb, istate->resolve_undo); @@ -3081,7 +3087,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile, if (err) return -1; } - if (!strip_extensions && istate->untracked) { + if (strip_extensions != STRIP_ALL_EXTENSIONS && istate->untracked) { struct strbuf sb = STRBUF_INIT; write_untracked_extension(&sb, istate->untracked); @@ -3092,7 +3098,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile, if (err) return -1; } - if (!strip_extensions && istate->fsmonitor_last_update) { + if (strip_extensions != STRIP_ALL_EXTENSIONS && istate->fsmonitor_last_update) { struct strbuf sb = STRBUF_INIT; write_fsmonitor_extension(&sb, istate); @@ -3166,8 +3172,14 @@ static int commit_locked_index(struct lock_file *lk) return commit_lock_file(lk); } +/* + * Write the Git index into a `.lock` file + * + * If `strip_link_extension` is non-zero, avoid writing any "link" extension + * (used by the split-index feature). + */ static int do_write_locked_index(struct index_state *istate, struct lock_file *lock, - unsigned flags) + unsigned flags, int strip_link_extension) { int ret; int was_full = istate->sparse_index == INDEX_EXPANDED; @@ -3185,7 +3197,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, strip_link_extension ? STRIP_LINK_EXTENSION_ONLY : 0, flags); trace2_region_leave_printf("index", "do_write_index", the_repository, "%s", get_lock_file_path(lock)); @@ -3214,7 +3226,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, 0); finish_writing_split_index(istate); return ret; } @@ -3366,9 +3378,7 @@ 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, 1); goto out; } @@ -3394,8 +3404,7 @@ 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, 1); 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" && From patchwork Wed Mar 22 16:00:58 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Johannes Schindelin X-Patchwork-Id: 13184252 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C4E30C6FD1C for ; Wed, 22 Mar 2023 16:01:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230291AbjCVQBJ (ORCPT ); Wed, 22 Mar 2023 12:01:09 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33940 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229906AbjCVQBG (ORCPT ); Wed, 22 Mar 2023 12:01:06 -0400 Received: from mail-wr1-x431.google.com (mail-wr1-x431.google.com [IPv6:2a00:1450:4864:20::431]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 52B9D65446 for ; Wed, 22 Mar 2023 09:01:05 -0700 (PDT) Received: by mail-wr1-x431.google.com with SMTP id t15so17615342wrz.7 for ; Wed, 22 Mar 2023 09:01:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1679500863; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=6HWgix6MtXPwLulkf+Lqdg0AYUoo+ggdD0mNFQepnrc=; b=gcf+I1h5UGTYMpx7wEcVyOpRlws08uwOYvHQ4k42Nyhni37vjy8k2xpi+TBuao2eHZ yf6MvYCDYMs3hUHb6OVL5unyInmkQKDw3IK6AL35hGEE/SNPY7dwnwT83yNhwAopiKZP N/zYkkD6w9s3zrJRdytrQPl/bxE4xD34FhLZcWXUkY5+g1VtCoB3sEmHo6bdKnJABmj6 wZgSCIKKWdJtQvFS/ZSm9N6vkmaFeOSkVObqY0nMsm3zJdhV0b7VDdsEfNhkX5aCGLvD PcTkEFJ30ex6dOxrmLPx9YY3zJDMSD/LdWYqirjCXdg1q2loSdfA9cNZwayVqsO7XtmD UHNA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679500863; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=6HWgix6MtXPwLulkf+Lqdg0AYUoo+ggdD0mNFQepnrc=; b=GrdCY/mu6LeORQZKNIqmigwITW3pDB25MBQito8xv+KY9lDmwmsmW6oUkZvbGOTOqu 95Gl1yJlx+hP9Zev8HVixIgA91zAMXGO5qQj5A6h5JV4eS/iG/8OZnqVa9Tu5dyaZOER UDStZEOeth0fiQjkr0ENWTkO+r8yNORyYOvqwLa1uawQUOBPe1bNXIABZ1rYgINYteqb ODVxw7uqJp2bDvleQKZMKHqt2Je+hmNV26t8c8YS5JWjBJKvkg9dWJ0IzTb4rdcm11C2 4v6GlIT4G1GWE2GGx5BKZNSPRbIoKtPkD3T+uKSlYnulwoa+SVkmZSii4WQ0M9+k8elg JOFg== X-Gm-Message-State: AAQBX9chq4US17P5bQS0Y5HEpUppkOxhtVjaGD4QP4hQ3ASWryEvdxET ji0X6bNEVKzAWtTYDiXATxnmI6NJo9k= X-Google-Smtp-Source: AKy350aLSzGVW+g3+IqzTjMNQOtQ1js/iv5v1lAWrBX7clOEKpGASIkXIkuk9oRFPejHnFodnyWbfw== X-Received: by 2002:adf:e5ca:0:b0:2ce:9877:84ae with SMTP id a10-20020adfe5ca000000b002ce987784aemr274051wrn.16.1679500863664; Wed, 22 Mar 2023 09:01:03 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id l5-20020adfe585000000b002cea9d931e6sm14186281wrm.78.2023.03.22.09.01.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 22 Mar 2023 09:01:03 -0700 (PDT) Message-Id: In-Reply-To: References: Date: Wed, 22 Mar 2023 16:00:58 +0000 Subject: [PATCH 3/4] fsmonitor: avoid overriding `cache_changed` bits Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Jeff Hostetler , Johannes Schindelin , Johannes Schindelin Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Johannes Schindelin From: Johannes Schindelin As of e636a7b4d030 (read-cache: be specific what part of the index has changed, 2014-06-13), the paradigm `cache_changed = 1` fell out of fashion and it became a bit field instead. This is important because some bits have specific meaning and should not be unset without care, e.g. `SPLIT_INDEX_ORDERED`. However, b5a816975206 (mark_fsmonitor_valid(): mark the index as changed if needed, 2019-05-24) did use the `cache_changed` attribute as if it were a Boolean instead of a bit field. That not only would override the `SPLIT_INDEX_ORDERED` bit when marking index entries as valid via the FSMonitor, but worse: it would set the `SOMETHING_OTHER` bit (whose value is 1). This means that Git would unnecessarily force a full index to be written out when a split index was asked for. Let's instead use the bit that is specifically intended to indicate FSMonitor-triggered changes, allowing the split-index feature to work as designed. Noticed-by: Jeff Hostetler Signed-off-by: Johannes Schindelin --- fsmonitor.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fsmonitor.h b/fsmonitor.h index edf7ce5203b..778707b131b 100644 --- a/fsmonitor.h +++ b/fsmonitor.h @@ -86,7 +86,7 @@ static inline void mark_fsmonitor_valid(struct index_state *istate, struct cache !(ce->ce_flags & CE_FSMONITOR_VALID)) { if (S_ISGITLINK(ce->ce_mode)) return; - istate->cache_changed = 1; + istate->cache_changed |= FSMONITOR_CHANGED; ce->ce_flags |= CE_FSMONITOR_VALID; trace_printf_key(&trace_fsmonitor, "mark_fsmonitor_clean '%s'", ce->name); } From patchwork Wed Mar 22 16:00:59 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Johannes Schindelin X-Patchwork-Id: 13184253 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3CCF9C6FD1C for ; Wed, 22 Mar 2023 16:01:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230047AbjCVQBL (ORCPT ); Wed, 22 Mar 2023 12:01:11 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33958 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230099AbjCVQBH (ORCPT ); Wed, 22 Mar 2023 12:01:07 -0400 Received: from mail-wm1-x336.google.com (mail-wm1-x336.google.com [IPv6:2a00:1450:4864:20::336]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C75ED50F93 for ; Wed, 22 Mar 2023 09:01:05 -0700 (PDT) Received: by mail-wm1-x336.google.com with SMTP id v4-20020a05600c470400b003ee4f06428fso2289260wmo.4 for ; Wed, 22 Mar 2023 09:01:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1679500864; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=A3ib7yuv25Yk7MrLfx2gVEaVtF2eRGWMtyxxUs9Xe7A=; b=JommsP5ttJfF3/cFtILd19aO4te9RkopzZVADwE/yj4qcWOdrWQbFoOyVGhqtX/9Oa v7eGt2Xmxq92f+2WsOcVqLFoI4TsvUDByNKi2RXoGFX5584Ty6i2Vpz5XTKJv1eSVjJ6 iNRjNmX3Laa4vybIfsbYl1K4Yn7s5j8RLg6Sci77o/RCNNqdYIPszEcI7OZ9iyoQheF+ Bxvu9ICTzsnGjDDUtlMU9YZUb4as/JOELjDz6rwB6YaHY9C303iwUX3etpLwxd9c34S7 RMiuBfmhj0Qc9sacJYKrQ9nRExVnpUDqADpBubXT58GFwmMgmHFB0l456X02th5PyNGT jW9Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679500864; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=A3ib7yuv25Yk7MrLfx2gVEaVtF2eRGWMtyxxUs9Xe7A=; b=FFj0lRNwkFI8xMwws639vMVE/XDJGcokOOyhcjeQeVlDb58HjYAOp6phCLAIe8OpCz jbjWuyt01uDuhwLgZVW0tCA9FqUAvv8YmIhJKvzY3vq9i5DbYaI2nBYemvpiSBjMw2cD cmLjXYLHOzkRmpt9rvGKPJcnMvgZfXuA9Y2NkzjCRlMUsNQwKfjpPSSj9Xz398b8FxgW hn3intARil8sIswr3bR45wrNPiDHG7+TZxPCwh/fYovaJO4/xc6EDPH21BZglj90w5GK eUBKHM09ZspOad5UEA7R0BiIxISBxUszp35XkQ/riz6a1N/sw/wAEa+WriKiWQURwema EYIA== X-Gm-Message-State: AO0yUKXHu/BBhS7290g43a8OC0mU39VxyZ/HpUi3QJeFglQvHFJUWWK5 JdTrPRDWefCVC8pLxTo0T5WRusgVB5E= X-Google-Smtp-Source: AK7set+WCalcw8swzHHFL0tn1QvxgiVqHjSDnM810NOKMwuhp9SCDr6UvC3on5iKJvDWU08pXSC3Rg== X-Received: by 2002:a7b:c850:0:b0:3ed:2f1a:883c with SMTP id c16-20020a7bc850000000b003ed2f1a883cmr41623wml.14.1679500864178; Wed, 22 Mar 2023 09:01:04 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id l2-20020a05600c4f0200b003ee610d1ce9sm2302045wmq.34.2023.03.22.09.01.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 22 Mar 2023 09:01:04 -0700 (PDT) Message-Id: <3963d3e542896b9cdf871dc7ea13330ddac87795.1679500859.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Wed, 22 Mar 2023 16:00:59 +0000 Subject: [PATCH 4/4] unpack-trees: take care to propagate the split-index flag Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Jeff Hostetler , Johannes Schindelin , Johannes Schindelin Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Johannes Schindelin From: Johannes Schindelin When copying the `split_index` structure from one index structure to another, we need to propagate the `SPLIT_INDEX_ORDERED` flag, too, if it is set, otherwise Git might forget to write the shared index when that is actually needed. It just so _happens_ that in many instances when `unpack_trees()` is called, the result causes the shared index to be written anyway, but there are edge cases when that is not so. Signed-off-by: Johannes Schindelin --- unpack-trees.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/unpack-trees.c b/unpack-trees.c index 90b92114be8..ca5e47c77c0 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -1916,6 +1916,8 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options * create a new one. */ o->result.split_index = o->src_index->split_index; + if (o->src_index->cache_changed & SPLIT_INDEX_ORDERED) + o->result.cache_changed |= SPLIT_INDEX_ORDERED; o->result.split_index->refcount++; } else { o->result.split_index = init_split_index(&o->result);