From patchwork Tue Jan 4 18:15:41 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 12703765 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 1034AC433EF for ; Tue, 4 Jan 2022 18:15:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236153AbiADSPo (ORCPT ); Tue, 4 Jan 2022 13:15:44 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50740 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234586AbiADSPn (ORCPT ); Tue, 4 Jan 2022 13:15:43 -0500 Received: from mail-qv1-xf36.google.com (mail-qv1-xf36.google.com [IPv6:2607:f8b0:4864:20::f36]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 522E5C061761 for ; Tue, 4 Jan 2022 10:15:43 -0800 (PST) Received: by mail-qv1-xf36.google.com with SMTP id ke6so35148064qvb.1 for ; Tue, 04 Jan 2022 10:15:43 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ttaylorr-com.20210112.gappssmtp.com; s=20210112; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=xtMowjcH9kIdYnLdb2LZFMrjUkwJVwMgrlKKUGywxo4=; b=dZYzaGG9/FaqpdnqGMDHOYC25CZdvir+ltLuoUZ7iO9RAddr2+uYbumJ/CFMwIXaZe x3//By7H0uRSFnTNjCpU+jaw3yQULU4glVNWVB5CfRURmXGugfL22mLT+LLdmXZHPYPO WjYVIElGIMDxcdXp7jM4qAJJGzjc6dWd0aYXnaUYIFQ9hK2PjsYvYBMlmwI7grd7hWGc 7NE8ZyqGPvPpGYgSo0vNcAHH/naXoodbEl0t0e1GNiBc9fg69GBdX24Of/lTrbNs1VhO pwQxZGtG1AF+84Wzk4gK7la74VSJxec+OA49moNbDVYyKEHEJSBKquvJI7jp5yhPdA2f BeBg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=xtMowjcH9kIdYnLdb2LZFMrjUkwJVwMgrlKKUGywxo4=; b=v7zuu0wkycaf9rKYMxQZj/K+gFM+7SBvyoV2g3s+EPq8Ls+lEv9A1M1nSXJ3sGvPwI s4kuP9aC0kqwLt41oxvkkjbARuwBpYkmudZ+C/rh4OvqfEWOkl2hpP7JrxRVg0HDfNDK WoRhlEB8DjMpJuHqWzS7rMwaWMNJoe+6ktSqjTN+FGeO46TbkmKwuuuYmWt9MYJu0wNz 0t722eZxpn8//ZrRzSIB/EO+FTtnRFEtomFy0xRBXGCRVBiFEEgQxDBL8omC1zpey2gH Znxo7IXdVZRWpSv8Pu6Lw9URs3dk8NAupS4855oIpKz/Giq5t55grDjK/2e0T0STxj3V o8rA== X-Gm-Message-State: AOAM532nFq22o7RJ0uX/0VYEpe69YzPJRt4AGQ67+moFcvpmaHplik+8 iZg+TOZdNEtijIH9R9y4qKThkdwwlskno5fK X-Google-Smtp-Source: ABdhPJzoqwwwlr0VODA8OnTcZMYx1ZSR4ZRNziOJaAdT4KH2EooYpy8EKMTzI3Kqq0p1bqAOmjKbpQ== X-Received: by 2002:a05:6214:c4a:: with SMTP id r10mr44809561qvj.95.1641320142261; Tue, 04 Jan 2022 10:15:42 -0800 (PST) Received: from localhost (104-178-186-189.lightspeed.milwwi.sbcglobal.net. [104.178.186.189]) by smtp.gmail.com with ESMTPSA id x25sm29984100qkf.91.2022.01.04.10.15.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 04 Jan 2022 10:15:41 -0800 (PST) Date: Tue, 4 Jan 2022 13:15:41 -0500 From: Taylor Blau To: git@vger.kernel.org Cc: gitster@pobox.com, stolee@gmail.com Subject: [PATCH v3 1/9] t5326: demonstrate bitmap corruption after permutation Message-ID: References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org This patch demonstrates a cause of bitmap corruption that can occur when the contents of the multi-pack index does not change, but the underlying object order does. In this example, we have a MIDX containing two packs, each with a distinct set of objects (pack A corresponds to the tree, blob, and commit from the first patch, and pack B corresponds to the second patch). First, a MIDX is written where the 'A' pack is preferred. As expected, the bitmaps generated there are in-tact. But then, we generate an identical MIDX with a different object order: this time preferring pack 'B'. Due to a bug which will be explained and fixed in the following commit, the MIDX is updated, but the .rev file is not, causing the .bitmap file to be read incorrectly. Specifically, the .bitmap file will contain correct data, but the auxiliary object order in the .rev file is stale, causing readers to get confused by reading the new bitmaps using the old object order. Signed-off-by: Taylor Blau --- t/t5326-multi-pack-bitmaps.sh | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/t/t5326-multi-pack-bitmaps.sh b/t/t5326-multi-pack-bitmaps.sh index e187f90f29..0ca2868b0b 100755 --- a/t/t5326-multi-pack-bitmaps.sh +++ b/t/t5326-multi-pack-bitmaps.sh @@ -395,4 +395,35 @@ test_expect_success 'hash-cache values are propagated from pack bitmaps' ' ) ' +test_expect_failure 'changing the preferred pack does not corrupt bitmaps' ' + rm -fr repo && + git init repo && + test_when_finished "rm -fr repo" && + ( + cd repo && + + test_commit A && + test_commit B && + + git rev-list --objects --no-object-names HEAD^ >A.objects && + git rev-list --objects --no-object-names HEAD^.. >B.objects && + + A=$(git pack-objects $objdir/pack/pack indexes <<-EOF && + pack-$A.idx + pack-$B.idx + EOF + + git multi-pack-index write --bitmap --stdin-packs \ + --preferred-pack=pack-$A.pack X-Patchwork-Id: 12703766 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 B0420C433EF for ; Tue, 4 Jan 2022 18:15:48 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236160AbiADSPs (ORCPT ); Tue, 4 Jan 2022 13:15:48 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50760 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236154AbiADSPq (ORCPT ); Tue, 4 Jan 2022 13:15:46 -0500 Received: from mail-qv1-xf35.google.com (mail-qv1-xf35.google.com [IPv6:2607:f8b0:4864:20::f35]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0CEF1C061761 for ; Tue, 4 Jan 2022 10:15:46 -0800 (PST) Received: by mail-qv1-xf35.google.com with SMTP id fo11so35102975qvb.4 for ; Tue, 04 Jan 2022 10:15:46 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ttaylorr-com.20210112.gappssmtp.com; s=20210112; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=dGyKq6LLyxjkfV7RG7MCJYo/FORq6V0w0bGAQBSmZXA=; b=ganIP8NJ2p7sQitQG9DSDn/t8Jtx48Ede7WuxcSE0pENdpZFcDWCG6XpDuiwJBWpiX jaE6F7CW0eWiWl3zvtjsDaitmrORq7vIQuSpQsoMD3G4jHduBjRb5B7QB/Rg7hz5rwkV HnaA/2o9be2YWEtZWzc7r5WWcrQ0n7BK51onTGadUCvxVTlUckS45Z9cUkYrVhBNHSwG V2WHYD1dRn6EeJq5QvQM7yDj3rkFripJbhoc2QrYsrDOVTp1jR1qMJGFNrTrSS8BqirS Y4whwxxe7QY2H8M9br9c3zNVfPg6HPe9592w6VI/q2HZjFlp6LM06sG8ZlEYN9Ob0VUo 2K5Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=dGyKq6LLyxjkfV7RG7MCJYo/FORq6V0w0bGAQBSmZXA=; b=pGDVw73eG5SpR9cberMyx7fvHFhUZDz+bP8KGtzrCDCEDAQpCpgGnuC3GSafGJD7kG E/zLqKwhcJf/Pdx+q8MUuHXZttCAscUAenFK1PYloDtcllRXhuuJe31tl7RazoZzBaTm eJve9wjIjscPO0SHl+5XpE9U0QStfdvDlukPkn9xLSRRaMG+0wkfzsBY9dQjkAKhoif6 bAhz/ay2kQYdKk18aoD48ngaLwyrubXpPJhoTPEug/pWR71VMKP7TLDQP/00bhQUT7t+ cApVdDkfObOfam/CfwZHeURTwKQiTEE87bf6GCZ/NCqk9HTN+44B4bXCkJ8sOhMPwL5/ V1gQ== X-Gm-Message-State: AOAM532pXOPaz9vdPOJmHj/dudPNjrVJE11+AQZ14sSUB1BA2d6fV21m 07+0IfHE3q8e7JwE+a3OkRCzihgCKzGeZsmS X-Google-Smtp-Source: ABdhPJwGlWRFNygAY4ddYUg7bKdjcKO6/JHuGnuPeGvuk7wGcBsuS+XG2TrFenn7PeKYHtFRH+Lz6A== X-Received: by 2002:a05:6214:2a88:: with SMTP id jr8mr47240013qvb.79.1641320144993; Tue, 04 Jan 2022 10:15:44 -0800 (PST) Received: from localhost (104-178-186-189.lightspeed.milwwi.sbcglobal.net. [104.178.186.189]) by smtp.gmail.com with ESMTPSA id q21sm30346521qkl.52.2022.01.04.10.15.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 04 Jan 2022 10:15:44 -0800 (PST) Date: Tue, 4 Jan 2022 13:15:43 -0500 From: Taylor Blau To: git@vger.kernel.org Cc: gitster@pobox.com, stolee@gmail.com Subject: [PATCH v3 2/9] midx.c: make changing the preferred pack safe Message-ID: <7d20c13f8b48d2aef45c2c8c40efb6ecdb865aa8.1641320129.git.me@ttaylorr.com> References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org The previous patch demonstrates a bug where a MIDX's auxiliary object order can become out of sync with a MIDX bitmap. This is because of two confounding factors: - First, the object order is stored in a file which is named according to the multi-pack index's checksum, and the MIDX does not store the object order. This means that the object order can change without altering the checksum. - But the .rev file is moved into place with finalize_object_file(), which link(2)'s the file into place instead of renaming it. For us, that means that a modified .rev file will not be moved into place if MIDX's checksum was unchanged. The fix here is two-fold. First, we need to stop linking the file into place and instead rename it. It's likely we were using finalize_object_file() instead of a pure rename() because the former also adjusts shared permissions. But that is unnecessary, because we already do so in write_rev_file_order(), so rename alone is safe. But we also need to make the MIDX's checksum change in some way when the preferred pack changes without altering the set of packs stored in a MIDX to prevent a race where the new .rev file is moved into place before the MIDX is updated. Here, you'd get the opposite effect: reading old bitmaps with the new object order. But this race bites us even here: suppose that we didn't change the MIDX checksum, but only renamed the auxiliary object order into place instead of hardlinking it. Then when we go to generate the new bitmap, we'll load the old MIDX bitmap, along with the MIDX that it references. That's fine, since the new MIDX isn't moved into place until after the new bitmap is generated. But the new object order *has* been moved into place. So we'll read the old bitmaps in the new order when generating the new bitmap file, meaning that without this secondary change, bitmap generation itself would become a victim of the race described here. This can all be prevented by forcing the MIDX's checksum to change when the object order changes. We could include the entire object order in the MIDX, but doing so is somewhat awkward. (For example, the code that writes a .rev file expects to know the checksum of the associated pack or MIDX, but writing that data into the MIDX itself makes that a circular dependency). Instead, make the object order used during bitmap generation part of the MIDX itself. That means that the new test in t5326 will cause the MIDX's checksum to update, preventing the stale read problem. In theory, it is possible to store a "fingerprint" of the full object order here, so long as that fingerprint changes at least as often as the full object order does. Some possibilities here include storing the identity of the preferred pack, along with the mtimes of the non-preferred packs in a consistent order. But storing a limited part of the information makes it difficult to reason about whether or not there are gaps between the two that would cause us to get bitten by this bug again. Signed-off-by: Taylor Blau --- Documentation/technical/multi-pack-index.txt | 1 + Documentation/technical/pack-format.txt | 13 +++++----- midx.c | 25 ++++++++++++++++---- t/t5326-multi-pack-bitmaps.sh | 2 +- 4 files changed, 30 insertions(+), 11 deletions(-) diff --git a/Documentation/technical/multi-pack-index.txt b/Documentation/technical/multi-pack-index.txt index b39c69da8c..f2221d2b44 100644 --- a/Documentation/technical/multi-pack-index.txt +++ b/Documentation/technical/multi-pack-index.txt @@ -24,6 +24,7 @@ and their offsets into multiple packfiles. It contains: ** An offset within the jth packfile for the object. * If large offsets are required, we use another list of large offsets similar to version 2 pack-indexes. +- An optional list of objects in pseudo-pack order (used with MIDX bitmaps). Thus, we can provide O(log N) lookup time for any number of packfiles. diff --git a/Documentation/technical/pack-format.txt b/Documentation/technical/pack-format.txt index 8d2f42f29e..6d3efb7d16 100644 --- a/Documentation/technical/pack-format.txt +++ b/Documentation/technical/pack-format.txt @@ -376,6 +376,11 @@ CHUNK DATA: [Optional] Object Large Offsets (ID: {'L', 'O', 'F', 'F'}) 8-byte offsets into large packfiles. + [Optional] Bitmap pack order (ID: {'R', 'I', 'D', 'X'}) + A list of MIDX positions (one per object in the MIDX, num_objects in + total, each a 4-byte unsigned integer in network byte order), sorted + according to their relative bitmap/pseudo-pack positions. + TRAILER: Index checksum of the above contents. @@ -456,9 +461,5 @@ In short, a MIDX's pseudo-pack is the de-duplicated concatenation of objects in packs stored by the MIDX, laid out in pack order, and the packs arranged in MIDX order (with the preferred pack coming first). -Finally, note that the MIDX's reverse index is not stored as a chunk in -the multi-pack-index itself. This is done because the reverse index -includes the checksum of the pack or MIDX to which it belongs, which -makes it impossible to write in the MIDX. To avoid races when rewriting -the MIDX, a MIDX reverse index includes the MIDX's checksum in its -filename (e.g., `multi-pack-index-xyz.rev`). +The MIDX's reverse index is stored in the optional 'RIDX' chunk within +the MIDX itself. diff --git a/midx.c b/midx.c index 837b46b2af..d3179e9c02 100644 --- a/midx.c +++ b/midx.c @@ -33,6 +33,7 @@ #define MIDX_CHUNKID_OIDLOOKUP 0x4f49444c /* "OIDL" */ #define MIDX_CHUNKID_OBJECTOFFSETS 0x4f4f4646 /* "OOFF" */ #define MIDX_CHUNKID_LARGEOFFSETS 0x4c4f4646 /* "LOFF" */ +#define MIDX_CHUNKID_REVINDEX 0x52494458 /* "RIDX" */ #define MIDX_CHUNK_FANOUT_SIZE (sizeof(uint32_t) * 256) #define MIDX_CHUNK_OFFSET_WIDTH (2 * sizeof(uint32_t)) #define MIDX_CHUNK_LARGE_OFFSET_WIDTH (sizeof(uint64_t)) @@ -833,6 +834,18 @@ static int write_midx_large_offsets(struct hashfile *f, return 0; } +static int write_midx_revindex(struct hashfile *f, + void *data) +{ + struct write_midx_context *ctx = data; + uint32_t i; + + for (i = 0; i < ctx->entries_nr; i++) + hashwrite_be32(f, ctx->pack_order[i]); + + return 0; +} + struct midx_pack_order_data { uint32_t nr; uint32_t pack; @@ -891,7 +904,7 @@ static void write_midx_reverse_index(char *midx_name, unsigned char *midx_hash, tmp_file = write_rev_file_order(NULL, ctx->pack_order, ctx->entries_nr, midx_hash, WRITE_REV); - if (finalize_object_file(tmp_file, buf.buf)) + if (rename(tmp_file, buf.buf)) die(_("cannot store reverse index file")); strbuf_release(&buf); @@ -1403,15 +1416,19 @@ static int write_midx_internal(const char *object_dir, (size_t)ctx.num_large_offsets * MIDX_CHUNK_LARGE_OFFSET_WIDTH, write_midx_large_offsets); + if (flags & (MIDX_WRITE_REV_INDEX | MIDX_WRITE_BITMAP)) { + ctx.pack_order = midx_pack_order(&ctx); + add_chunk(cf, MIDX_CHUNKID_REVINDEX, + ctx.entries_nr * sizeof(uint32_t), + write_midx_revindex); + } + write_midx_header(f, get_num_chunks(cf), ctx.nr - dropped_packs); write_chunkfile(cf, &ctx); finalize_hashfile(f, midx_hash, CSUM_FSYNC | CSUM_HASH_IN_STREAM); free_chunkfile(cf); - if (flags & (MIDX_WRITE_REV_INDEX | MIDX_WRITE_BITMAP)) - ctx.pack_order = midx_pack_order(&ctx); - if (flags & MIDX_WRITE_REV_INDEX) write_midx_reverse_index(midx_name.buf, midx_hash, &ctx); if (flags & MIDX_WRITE_BITMAP) { diff --git a/t/t5326-multi-pack-bitmaps.sh b/t/t5326-multi-pack-bitmaps.sh index 0ca2868b0b..353282310d 100755 --- a/t/t5326-multi-pack-bitmaps.sh +++ b/t/t5326-multi-pack-bitmaps.sh @@ -395,7 +395,7 @@ test_expect_success 'hash-cache values are propagated from pack bitmaps' ' ) ' -test_expect_failure 'changing the preferred pack does not corrupt bitmaps' ' +test_expect_success 'changing the preferred pack does not corrupt bitmaps' ' rm -fr repo && git init repo && test_when_finished "rm -fr repo" && From patchwork Tue Jan 4 18:15:46 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 12703767 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 061ECC433F5 for ; Tue, 4 Jan 2022 18:15:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236163AbiADSPu (ORCPT ); Tue, 4 Jan 2022 13:15:50 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50778 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236155AbiADSPs (ORCPT ); Tue, 4 Jan 2022 13:15:48 -0500 Received: from mail-qk1-x734.google.com (mail-qk1-x734.google.com [IPv6:2607:f8b0:4864:20::734]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id AF85EC061761 for ; Tue, 4 Jan 2022 10:15:48 -0800 (PST) Received: by mail-qk1-x734.google.com with SMTP id i187so33581954qkf.5 for ; Tue, 04 Jan 2022 10:15:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ttaylorr-com.20210112.gappssmtp.com; s=20210112; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=StB/LK9blxjkJOwppFu/JcsUXmSSKIHCZYIhAhYeOQM=; b=LFlDh/H+BCTRi0hbHD8SlS02+326P2aMvfb5sxLa/3dHuk1WXWA9tx6bwP2hwVs/3N iGwaJsX3gk0iiHjqiwhkjH+ODVc2ZugSN0eh0aDoKp/9ecFXgrvCWy8uKG968FZ/2qYX ZNAMRh+vF0bglZV9xfrhK0xMQj1XxIBWAR3+d8kx4OCsHzA9xhB5hMAE8CmWW6mxX4Rs tH0WRssAerpsfKDlWoQKqrVw47we4078/yCDsz7wkY7eXR23ia0sCEy0WNW61l7eUCvm YK1IE7Kh/jiprBFSjDCKQcH8n1OaZ+nYKMPjdPVJFkuDlk/cB7Va1Xm6dvC5dqwnlalI BiWg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=StB/LK9blxjkJOwppFu/JcsUXmSSKIHCZYIhAhYeOQM=; b=4A70akzm1crC7c0sn58Vex8LAt8cLLaLIiQIKncbr4C0UFdkeqc2mtlBLO5p9XE1wN LhutoE77glRyvW5/LgUWbj0NF/3bhe2tK5BSuDOPG3RUUqBCKoktgqz0+RAfqSRs9paU b9y6kfRRy5x/va6pbfqLOTV08RaF8K61xfFbr9lj+l2w/JFrvBpad7g5nN3yuxxfZBiR AdY8q9JdC20ON/ZoMBnKC3ZCnoZ9UW3F0D4hNXnzs96be3b4f0d9HC+nhgpbI5XAcYVG La4iEY95Pwb6X5MQFVTX5AEFxf7sIdEkaWOVeRvXfbkX7Fk3lzV4TVao7VGzwsy6RvKB P3gg== X-Gm-Message-State: AOAM532c2PYN+Rhykma5ZfE0giOXGDe+fkdfQ3PUxSEz/vq4ebvwvvTP VZ9VEoro+V8X1bkzMtOnGfTckZx9vqqHuitN X-Google-Smtp-Source: ABdhPJyl7Rz7LwV6+2tM9upekPBH12Ayc938sv6OyO19grNJ6MWgLJg3EsMwnX1AyZIUhNMfP+V+rA== X-Received: by 2002:a05:620a:2848:: with SMTP id h8mr36357956qkp.610.1641320147660; Tue, 04 Jan 2022 10:15:47 -0800 (PST) Received: from localhost (104-178-186-189.lightspeed.milwwi.sbcglobal.net. [104.178.186.189]) by smtp.gmail.com with ESMTPSA id f12sm32201281qtj.93.2022.01.04.10.15.47 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 04 Jan 2022 10:15:47 -0800 (PST) Date: Tue, 4 Jan 2022 13:15:46 -0500 From: Taylor Blau To: git@vger.kernel.org Cc: gitster@pobox.com, stolee@gmail.com Subject: [PATCH v3 3/9] pack-revindex.c: instrument loading on-disk reverse index Message-ID: <3279e2eb9b7bbfcc930e4ee146a3bd2476ef91eb.1641320129.git.me@ttaylorr.com> References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org In a subsequent commit, we'll use the MIDX's new 'RIDX' chunk as a source for the reverse index's data. But it will be useful for tests to be able to determine whether the reverse index was loaded from the separate .rev file, or from a chunk within the MIDX. To instrument this, add a trace2 event which the tests can look for in order to determine the reverse index's source. Signed-off-by: Taylor Blau --- pack-revindex.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pack-revindex.c b/pack-revindex.c index 70d0fbafcb..bd15ebad03 100644 --- a/pack-revindex.c +++ b/pack-revindex.c @@ -301,6 +301,9 @@ int load_midx_revindex(struct multi_pack_index *m) if (m->revindex_data) return 0; + trace2_data_string("load_midx_revindex", the_repository, + "source", "rev"); + get_midx_rev_filename(&revindex_name, m); ret = load_revindex_from_disk(revindex_name.buf, From patchwork Tue Jan 4 18:15:49 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 12703768 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 D0490C433F5 for ; Tue, 4 Jan 2022 18:15:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236165AbiADSPz (ORCPT ); Tue, 4 Jan 2022 13:15:55 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50792 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236158AbiADSPv (ORCPT ); Tue, 4 Jan 2022 13:15:51 -0500 Received: from mail-qv1-xf33.google.com (mail-qv1-xf33.google.com [IPv6:2607:f8b0:4864:20::f33]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1F211C061799 for ; Tue, 4 Jan 2022 10:15:51 -0800 (PST) Received: by mail-qv1-xf33.google.com with SMTP id kc16so35192200qvb.3 for ; Tue, 04 Jan 2022 10:15:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ttaylorr-com.20210112.gappssmtp.com; s=20210112; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=LMumUQ2hzoDKMXFY3lwDuDhafwwkP0GDPscap3ppkSk=; b=hkVQm6Yk3kPHzqyIXELe1On9ydagw0NtAtdJFdJ4YACX451OZmQmEYwiP4hKPZYJTX 9SCCf6nfE3Z1Pb7mho0BjCAO2zq0Tju9VB+3/4Bj0L7xu7ApdPhVkvyWBXaGPkwqK1HE T3C8JTMQEHjZwetz3gWuwz0wa4Lgm+ZJqBe9M8zoBnV2VvXPSmwagXCZxMmrVbsfFuyu bfuXQo4mTtHngoYDB1WxvLWUxZUOtt4YdEb+qNm3aQg7B/U2wOxCMkHWqGB+2iNyJpVr bDLwzn336BspGyyBzhmomBH+G3+7Yw7hxS641pZ7yMLjkx4cY9V66Vkqkhga4V9Mw74J sdVw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=LMumUQ2hzoDKMXFY3lwDuDhafwwkP0GDPscap3ppkSk=; b=mktvX7LAZTI0Mo6RoVbKSsgtAirGREs2LRu7mQyPkT4eR0E5wliYOnO/3oKqK3rj4J GripTcep1XfV3mvPwGtkeD1O+dVJytcgD4LD7GXEMMsuSPvFH0C2Wo4llSr652+uiqW/ 6eHHBZX+hI29aPvkqimoUvDnFAxiNV1aGa6O98v4RC8Oi1ibJKEFv4ZMkx/z5QlBxDf/ UoeD/rXZAY6xbDhCTDwVhCKjHWM2t1bu9DE0QcFK4Se05UxFqBKL3jwuIz/gmwyEti8E blyEcsSHhWDj6dlbgD+Sfm+GcIqFtknHDB6/M1H2rxOiGL6CIxnFAwl4OgMnbHRef6aU A4Qg== X-Gm-Message-State: AOAM5315qw9qVbiMGTyzn2w47H9LNxhtody5RoVmkW944J1rjN7jU+fP dSWTUJ3kPNysGeplGd3yhvM0wyYNvgcI2aIZ X-Google-Smtp-Source: ABdhPJw4JaeZMm8kvqdB2q8rVbJf5s1QeWZeb8ht+OIeOLQE4mBQ+pcRO38VorRPNS4a4C9cuCD8Zw== X-Received: by 2002:a05:6214:4110:: with SMTP id kc16mr46695780qvb.49.1641320150065; Tue, 04 Jan 2022 10:15:50 -0800 (PST) Received: from localhost (104-178-186-189.lightspeed.milwwi.sbcglobal.net. [104.178.186.189]) by smtp.gmail.com with ESMTPSA id t30sm30255954qkj.125.2022.01.04.10.15.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 04 Jan 2022 10:15:49 -0800 (PST) Date: Tue, 4 Jan 2022 13:15:49 -0500 From: Taylor Blau To: git@vger.kernel.org Cc: gitster@pobox.com, stolee@gmail.com Subject: [PATCH v3 4/9] t5326: drop unnecessary setup Message-ID: <5818621ea8ae6cb4bb1364621c33016edc208438.1641320129.git.me@ttaylorr.com> References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org The core.multiPackIndex config became true by default back in 18e449f86b (midx: enable core.multiPackIndex by default, 2020-09-25), so it is no longer necessary to enable it explicitly. Signed-off-by: Taylor Blau --- t/t5326-multi-pack-bitmaps.sh | 4 ---- 1 file changed, 4 deletions(-) diff --git a/t/t5326-multi-pack-bitmaps.sh b/t/t5326-multi-pack-bitmaps.sh index 353282310d..1a9581af30 100755 --- a/t/t5326-multi-pack-bitmaps.sh +++ b/t/t5326-multi-pack-bitmaps.sh @@ -19,10 +19,6 @@ midx_pack_source () { setup_bitmap_history -test_expect_success 'enable core.multiPackIndex' ' - git config core.multiPackIndex true -' - test_expect_success 'create single-pack midx with bitmaps' ' git repack -ad && git multi-pack-index write --bitmap && From patchwork Tue Jan 4 18:15:51 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 12703769 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 71EDFC433EF for ; Tue, 4 Jan 2022 18:15:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236171AbiADSPz (ORCPT ); Tue, 4 Jan 2022 13:15:55 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50826 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236177AbiADSPy (ORCPT ); Tue, 4 Jan 2022 13:15:54 -0500 Received: from mail-qv1-xf30.google.com (mail-qv1-xf30.google.com [IPv6:2607:f8b0:4864:20::f30]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 68206C061395 for ; Tue, 4 Jan 2022 10:15:53 -0800 (PST) Received: by mail-qv1-xf30.google.com with SMTP id fq10so35120529qvb.10 for ; Tue, 04 Jan 2022 10:15:53 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ttaylorr-com.20210112.gappssmtp.com; s=20210112; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=duo42mmwjDG7xzHwpCbjszexAEaF0rYVp05lq6fruSo=; b=i0VQg2XKHZwLatgDtAHQhwmmXsRP0CYdMiVMrmxtd1qPQkwjstDi0ehateCUP74SyN KfpI5woGkYPDtebujM5o8OTab8ZIhLN1Kg/q1KCV5jG1XGDy3Oz5FQALRYweO4tG6pIr H/V8gyvepvdjs+fXIqV/p78jpIgEnK+79O+4JfHz2h4ucNdTF+9o255ubKvHDRFg0t8Y zmaXOwrM+hJBUiRVdKaGx+7tVflGjO98xlciDrvvLnJoyUIqXEIddVah56s7UimJtR/K l4Vw+yN0guYrGZCm2bZhF0Ctench7wi8PEseOs5F1hyt5EDOaxKPf+sm3Xoac/8JjUho 8EkQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=duo42mmwjDG7xzHwpCbjszexAEaF0rYVp05lq6fruSo=; b=KS4j9MVCqbeSMmdg9xJswxFKpeFOs2ZRQKkxp/s+H6Ce9/inzhv1pIKfn5T6jImc0+ ctvyhiplBnOrZaziixzBRBRUEW1mFfAUMrT+ro1tYBfr+hsMBlpFGyiPiyx8lyrhDDo/ 8E7pg+sTF55DCl6ba3gpMEIaXKo7koDQdep+kD+IUG8vi1zdSs7fIaDhJCx/Kov/wK/1 Gbam66AgqI9ifFK/QhQ3+9nkAdBv6bGz2H6zfjSN/d90ktR5LdFGu31YW+AYIuXXV8Cn SvFCGK18CTeT6IPJN07Fda5XM6CJ1Hf/34qUZcQUuLLvEpsaArlz0QhckZjZILmhm3nm qmvw== X-Gm-Message-State: AOAM533VgvhkPz3g6+TPjsFWFeMO6CSLzX/8W60fEgxsPSwxYNVuqc3H UKz9C+hHRPWlsC+/GbiL5hHb/rMwqiS5sPqf X-Google-Smtp-Source: ABdhPJxrDtdinURh6gspw/JEPMaExyYqJ2d2gsKOgtQkrZ3fn7EoZ79Wll4kyFIbdQ+X0hfDlzd5fA== X-Received: by 2002:ad4:5be5:: with SMTP id k5mr47247822qvc.19.1641320152440; Tue, 04 Jan 2022 10:15:52 -0800 (PST) Received: from localhost (104-178-186-189.lightspeed.milwwi.sbcglobal.net. [104.178.186.189]) by smtp.gmail.com with ESMTPSA id p16sm30741697qtx.19.2022.01.04.10.15.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 04 Jan 2022 10:15:52 -0800 (PST) Date: Tue, 4 Jan 2022 13:15:51 -0500 From: Taylor Blau To: git@vger.kernel.org Cc: gitster@pobox.com, stolee@gmail.com Subject: [PATCH v3 5/9] t5326: extract `test_rev_exists` Message-ID: <33502d6a17b6b927d43a015859e40e6cdaab6667.1641320129.git.me@ttaylorr.com> References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org To determine which source of data is used for the MIDX's reverse index cache, introduce a helper which forces loading the reverse index, and then looks for the special trace2 event introduced in a previous commit. For now, this helper just looks for when the legacy MIDX .rev file was loaded, but in a subsequent commit will become parameterized over the the reverse index's source. This function replaces checking for the existence of the .rev file. We could write a similar helper to ensure that the .rev file is cleaned up after repacking, but it will make subsequent tests more difficult to write, and provides marginal value since we already check that the MIDX .bitmap file is removed. Signed-off-by: Taylor Blau --- t/t5326-multi-pack-bitmaps.sh | 37 +++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/t/t5326-multi-pack-bitmaps.sh b/t/t5326-multi-pack-bitmaps.sh index 1a9581af30..999740f8c7 100755 --- a/t/t5326-multi-pack-bitmaps.sh +++ b/t/t5326-multi-pack-bitmaps.sh @@ -17,16 +17,29 @@ midx_pack_source () { test-tool read-midx --show-objects .git/objects | grep "^$1 " | cut -f2 } +test_rev_exists () { + commit="$1" + + test_expect_success 'reverse index exists' ' + GIT_TRACE2_EVENT=$(pwd)/event.trace \ + git rev-list --test-bitmap "$commit" && + + test_path_is_file $midx-$(midx_checksum $objdir).rev && + grep "\"category\":\"load_midx_revindex\",\"key\":\"source\",\"value\":\"rev\"" event.trace + ' +} + setup_bitmap_history test_expect_success 'create single-pack midx with bitmaps' ' git repack -ad && git multi-pack-index write --bitmap && test_path_is_file $midx && - test_path_is_file $midx-$(midx_checksum $objdir).bitmap && - test_path_is_file $midx-$(midx_checksum $objdir).rev + test_path_is_file $midx-$(midx_checksum $objdir).bitmap ' +test_rev_exists HEAD + basic_bitmap_tests test_expect_success 'create new additional packs' ' @@ -52,10 +65,11 @@ test_expect_success 'create multi-pack midx with bitmaps' ' test_line_count = 25 packs && test_path_is_file $midx && - test_path_is_file $midx-$(midx_checksum $objdir).bitmap && - test_path_is_file $midx-$(midx_checksum $objdir).rev + test_path_is_file $midx-$(midx_checksum $objdir).bitmap ' +test_rev_exists HEAD + basic_bitmap_tests test_expect_success '--no-bitmap is respected when bitmaps exist' ' @@ -66,7 +80,6 @@ test_expect_success '--no-bitmap is respected when bitmaps exist' ' test_path_is_file $midx && test_path_is_file $midx-$(midx_checksum $objdir).bitmap && - test_path_is_file $midx-$(midx_checksum $objdir).rev && git multi-pack-index write --no-bitmap && @@ -206,10 +219,11 @@ test_expect_success 'setup partial bitmaps' ' test_commit loose && git multi-pack-index write --bitmap 2>err && test_path_is_file $midx && - test_path_is_file $midx-$(midx_checksum $objdir).bitmap && - test_path_is_file $midx-$(midx_checksum $objdir).rev + test_path_is_file $midx-$(midx_checksum $objdir).bitmap ' +test_rev_exists HEAD~ + basic_bitmap_tests HEAD~ test_expect_success 'removing a MIDX clears stale bitmaps' ' @@ -224,7 +238,6 @@ test_expect_success 'removing a MIDX clears stale bitmaps' ' # Write a MIDX and bitmap; remove the MIDX but leave the bitmap. stale_bitmap=$midx-$(midx_checksum $objdir).bitmap && - stale_rev=$midx-$(midx_checksum $objdir).rev && rm $midx && # Then write a new MIDX. @@ -234,9 +247,7 @@ test_expect_success 'removing a MIDX clears stale bitmaps' ' test_path_is_file $midx && test_path_is_file $midx-$(midx_checksum $objdir).bitmap && - test_path_is_file $midx-$(midx_checksum $objdir).rev && - test_path_is_missing $stale_bitmap && - test_path_is_missing $stale_rev + test_path_is_missing $stale_bitmap ) ' @@ -257,7 +268,6 @@ test_expect_success 'pack.preferBitmapTips' ' git multi-pack-index write --bitmap && test_path_is_file $midx && test_path_is_file $midx-$(midx_checksum $objdir).bitmap && - test_path_is_file $midx-$(midx_checksum $objdir).rev && test-tool bitmap list-commits | sort >bitmaps && comm -13 bitmaps commits >before && @@ -267,7 +277,6 @@ test_expect_success 'pack.preferBitmapTips' ' snapshot && rm -fr $midx-$(midx_checksum $objdir).bitmap && - rm -fr $midx-$(midx_checksum $objdir).rev && rm -fr $midx && git multi-pack-index write --bitmap --refs-snapshot=snapshot && From patchwork Tue Jan 4 18:15:53 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 12703770 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 C1A81C433F5 for ; Tue, 4 Jan 2022 18:15:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236174AbiADSP5 (ORCPT ); Tue, 4 Jan 2022 13:15:57 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50842 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236168AbiADSP4 (ORCPT ); Tue, 4 Jan 2022 13:15:56 -0500 Received: from mail-qk1-x736.google.com (mail-qk1-x736.google.com [IPv6:2607:f8b0:4864:20::736]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 21845C061761 for ; Tue, 4 Jan 2022 10:15:56 -0800 (PST) Received: by mail-qk1-x736.google.com with SMTP id t66so24243859qkb.4 for ; Tue, 04 Jan 2022 10:15:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ttaylorr-com.20210112.gappssmtp.com; s=20210112; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=E12vkO/ps71LLId4vEM7DU+LX8fmvF1bBpY+mPbeZZA=; b=lr75691Av15gP8Wfg6jc3xgV4peIqvsdzFT0cwfbzk0qrZcvFFoP7Ik9fND8ZN1mV7 PKx3w5y7CJ8KpVEE7Cnywd/gc6v+rlsnfdUZ5xlCh4pzdrejF64BgJjNYVcrmINrfBpB 0AMipQY+zGLO87Xcv23oBF8LcA+NdQ2S4i1ECRx8ZQ6G/PLptSHq3XxLlT6s2PwBjBUf WUe0BRWTH/eJI5GL7mpfm9pLTwblfeHMzWH9oBqdY61BCk+1w7u/jwkDUeu4UmXWM+cv OS794ExCS8cMOrXQKyIjlPKKfb7JdQ/Xrjmtyjxq1+MfUn8XjcoBGV2PZtP0HydXtLYC auYQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=E12vkO/ps71LLId4vEM7DU+LX8fmvF1bBpY+mPbeZZA=; b=l2IdufXQ5kPefS/wVBm7k+q28p0qFlhZo2ZCScabEcd7NXdQpDnLbSZCou/zCPeHqk I0lCseYMCRoVNqI3p4X7pfxT8H8as4FmVtuo/2xZCe72ObvKHfzXIZKAl6njpLl+FfyW QtXNVI2pfNUs2BuVNZxYwoj/OQu67FK8u0oe5tmmY9CeJXmLtAVqL2fBbgqBKq5l5sE1 OEYIbsNHwyOSzy41FxWCHgf8JF4tOGoqGpAG31JjH6VffIsy7fEGq39EaEBSPSPL9vlO /5MZ6lT6HsbagsKwH3/Bti3j7vQZpsb8gjaJcEhOjGwVYYFWNnIUglIwWLmW58AojfZy +PXA== X-Gm-Message-State: AOAM532yZyLpnp6NaFOeyoG5u4c0TO3nsiN5sTZCeWZ1SSunfExzGJte 9KPhMXwFFE1gYnYiWKll4YKmZyZp2qpePf7w X-Google-Smtp-Source: ABdhPJz6hSs6uAVngZWgd15i9H97JnkhULDio+SvyFmyZxkJWO7Y18KFJlcbXz4xq8U8q7qmwdDxWA== X-Received: by 2002:a05:620a:bc3:: with SMTP id s3mr36811218qki.197.1641320155002; Tue, 04 Jan 2022 10:15:55 -0800 (PST) Received: from localhost (104-178-186-189.lightspeed.milwwi.sbcglobal.net. [104.178.186.189]) by smtp.gmail.com with ESMTPSA id c7sm34744279qtx.67.2022.01.04.10.15.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 04 Jan 2022 10:15:54 -0800 (PST) Date: Tue, 4 Jan 2022 13:15:53 -0500 From: Taylor Blau To: git@vger.kernel.org Cc: gitster@pobox.com, stolee@gmail.com Subject: [PATCH v3 6/9] t5326: move tests to t/lib-bitmap.sh Message-ID: <76e23cae0f42afef982a4e94ec551335774be6f2.1641320129.git.me@ttaylorr.com> References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org In t5326, we have a handful of tests that we would like to run twice: once using the MIDX's new `RIDX` chunk as the source of the reverse-index cache, and once using the separate `.rev` file. But because these tests mutate the state of the underlying repository, and then make assumptions about those mutations occurring in a certain sequence, simply running the tests twice in the same repository is awkward. Instead, extract the core of interesting tests into t/lib-bitmap.sh to prepare for them to be run twice, each in a separate test script. This means that they can each operate on a separate repository, removing any concerns about mutating state. For now, this patch is a strict cut-and-paste of some tests from t5326. The tests which did not move are not interesting with respect to the source of their reverse index data. Signed-off-by: Taylor Blau --- t/lib-bitmap.sh | 177 ++++++++++++++++++++++++++++++++++ t/t5326-multi-pack-bitmaps.sh | 173 +-------------------------------- 2 files changed, 179 insertions(+), 171 deletions(-) diff --git a/t/lib-bitmap.sh b/t/lib-bitmap.sh index 21d0392dda..48a8730a13 100644 --- a/t/lib-bitmap.sh +++ b/t/lib-bitmap.sh @@ -1,6 +1,9 @@ # Helpers for scripts testing bitmap functionality; see t5310 for # example usage. +objdir=.git/objects +midx=$objdir/pack/multi-pack-index + # Compare a file containing rev-list bitmap traversal output to its non-bitmap # counterpart. You can't just use test_cmp for this, because the two produce # subtly different output: @@ -264,3 +267,177 @@ have_delta () { midx_checksum () { test-tool read-midx --checksum "$1" } + +# midx_pack_source +midx_pack_source () { + test-tool read-midx --show-objects .git/objects | grep "^$1 " | cut -f2 +} + +test_rev_exists () { + commit="$1" + + test_expect_success 'reverse index exists' ' + GIT_TRACE2_EVENT=$(pwd)/event.trace \ + git rev-list --test-bitmap "$commit" && + + test_path_is_file $midx-$(midx_checksum $objdir).rev && + grep "\"category\":\"load_midx_revindex\",\"key\":\"source\",\"value\":\"rev\"" event.trace + ' +} + +midx_bitmap_core () { + setup_bitmap_history + + test_expect_success 'create single-pack midx with bitmaps' ' + git repack -ad && + git multi-pack-index write --bitmap && + test_path_is_file $midx && + test_path_is_file $midx-$(midx_checksum $objdir).bitmap + ' + + test_rev_exists HEAD + + basic_bitmap_tests + + test_expect_success 'create new additional packs' ' + for i in $(test_seq 1 16) + do + test_commit "$i" && + git repack -d || return 1 + done && + + git checkout -b other2 HEAD~8 && + for i in $(test_seq 1 8) + do + test_commit "side-$i" && + git repack -d || return 1 + done && + git checkout second + ' + + test_expect_success 'create multi-pack midx with bitmaps' ' + git multi-pack-index write --bitmap && + + ls $objdir/pack/pack-*.pack >packs && + test_line_count = 25 packs && + + test_path_is_file $midx && + test_path_is_file $midx-$(midx_checksum $objdir).bitmap + ' + + test_rev_exists HEAD + + basic_bitmap_tests + + test_expect_success '--no-bitmap is respected when bitmaps exist' ' + git multi-pack-index write --bitmap && + + test_commit respect--no-bitmap && + git repack -d && + + test_path_is_file $midx && + test_path_is_file $midx-$(midx_checksum $objdir).bitmap && + + git multi-pack-index write --no-bitmap && + + test_path_is_file $midx && + test_path_is_missing $midx-$(midx_checksum $objdir).bitmap && + test_path_is_missing $midx-$(midx_checksum $objdir).rev + ' + + test_expect_success 'setup midx with base from later pack' ' + # Write a and b so that "a" is a delta on top of base "b", since Git + # prefers to delete contents out of a base rather than add to a shorter + # object. + test_seq 1 128 >a && + test_seq 1 130 >b && + + git add a b && + git commit -m "initial commit" && + + a=$(git rev-parse HEAD:a) && + b=$(git rev-parse HEAD:b) && + + # In the first pack, "a" is stored as a delta to "b". + p1=$(git pack-objects .git/objects/pack/pack <<-EOF + $a + $b + EOF + ) && + + # In the second pack, "a" is missing, and "b" is not a delta nor base to + # any other object. + p2=$(git pack-objects .git/objects/pack/pack <<-EOF + $b + $(git rev-parse HEAD) + $(git rev-parse HEAD^{tree}) + EOF + ) && + + git prune-packed && + # Use the second pack as the preferred source, so that "b" occurs + # earlier in the MIDX object order, rendering "a" unusable for pack + # reuse. + git multi-pack-index write --bitmap --preferred-pack=pack-$p2.idx && + + have_delta $a $b && + test $(midx_pack_source $a) != $(midx_pack_source $b) + ' + + rev_list_tests 'full bitmap with backwards delta' + + test_expect_success 'clone with bitmaps enabled' ' + git clone --no-local --bare . clone-reverse-delta.git && + test_when_finished "rm -fr clone-reverse-delta.git" && + + git rev-parse HEAD >expect && + git --git-dir=clone-reverse-delta.git rev-parse HEAD >actual && + test_cmp expect actual + ' + + test_expect_success 'changing the preferred pack does not corrupt bitmaps' ' + rm -fr repo && + git init repo && + test_when_finished "rm -fr repo" && + ( + cd repo && + + test_commit A && + test_commit B && + + git rev-list --objects --no-object-names HEAD^ >A.objects && + git rev-list --objects --no-object-names HEAD^.. >B.objects && + + A=$(git pack-objects $objdir/pack/pack indexes <<-EOF && + pack-$A.idx + pack-$B.idx + EOF + + git multi-pack-index write --bitmap --stdin-packs \ + --preferred-pack=pack-$A.pack err && + test_path_is_file $midx && + test_path_is_file $midx-$(midx_checksum $objdir).bitmap + ' + + test_rev_exists HEAD~ + + basic_bitmap_tests HEAD~ +} diff --git a/t/t5326-multi-pack-bitmaps.sh b/t/t5326-multi-pack-bitmaps.sh index 999740f8c7..100ac90d15 100755 --- a/t/t5326-multi-pack-bitmaps.sh +++ b/t/t5326-multi-pack-bitmaps.sh @@ -9,134 +9,7 @@ test_description='exercise basic multi-pack bitmap functionality' GIT_TEST_MULTI_PACK_INDEX=0 GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP=0 -objdir=.git/objects -midx=$objdir/pack/multi-pack-index - -# midx_pack_source -midx_pack_source () { - test-tool read-midx --show-objects .git/objects | grep "^$1 " | cut -f2 -} - -test_rev_exists () { - commit="$1" - - test_expect_success 'reverse index exists' ' - GIT_TRACE2_EVENT=$(pwd)/event.trace \ - git rev-list --test-bitmap "$commit" && - - test_path_is_file $midx-$(midx_checksum $objdir).rev && - grep "\"category\":\"load_midx_revindex\",\"key\":\"source\",\"value\":\"rev\"" event.trace - ' -} - -setup_bitmap_history - -test_expect_success 'create single-pack midx with bitmaps' ' - git repack -ad && - git multi-pack-index write --bitmap && - test_path_is_file $midx && - test_path_is_file $midx-$(midx_checksum $objdir).bitmap -' - -test_rev_exists HEAD - -basic_bitmap_tests - -test_expect_success 'create new additional packs' ' - for i in $(test_seq 1 16) - do - test_commit "$i" && - git repack -d || return 1 - done && - - git checkout -b other2 HEAD~8 && - for i in $(test_seq 1 8) - do - test_commit "side-$i" && - git repack -d || return 1 - done && - git checkout second -' - -test_expect_success 'create multi-pack midx with bitmaps' ' - git multi-pack-index write --bitmap && - - ls $objdir/pack/pack-*.pack >packs && - test_line_count = 25 packs && - - test_path_is_file $midx && - test_path_is_file $midx-$(midx_checksum $objdir).bitmap -' - -test_rev_exists HEAD - -basic_bitmap_tests - -test_expect_success '--no-bitmap is respected when bitmaps exist' ' - git multi-pack-index write --bitmap && - - test_commit respect--no-bitmap && - git repack -d && - - test_path_is_file $midx && - test_path_is_file $midx-$(midx_checksum $objdir).bitmap && - - git multi-pack-index write --no-bitmap && - - test_path_is_file $midx && - test_path_is_missing $midx-$(midx_checksum $objdir).bitmap && - test_path_is_missing $midx-$(midx_checksum $objdir).rev -' - -test_expect_success 'setup midx with base from later pack' ' - # Write a and b so that "a" is a delta on top of base "b", since Git - # prefers to delete contents out of a base rather than add to a shorter - # object. - test_seq 1 128 >a && - test_seq 1 130 >b && - - git add a b && - git commit -m "initial commit" && - - a=$(git rev-parse HEAD:a) && - b=$(git rev-parse HEAD:b) && - - # In the first pack, "a" is stored as a delta to "b". - p1=$(git pack-objects .git/objects/pack/pack <<-EOF - $a - $b - EOF - ) && - - # In the second pack, "a" is missing, and "b" is not a delta nor base to - # any other object. - p2=$(git pack-objects .git/objects/pack/pack <<-EOF - $b - $(git rev-parse HEAD) - $(git rev-parse HEAD^{tree}) - EOF - ) && - - git prune-packed && - # Use the second pack as the preferred source, so that "b" occurs - # earlier in the MIDX object order, rendering "a" unusable for pack - # reuse. - git multi-pack-index write --bitmap --preferred-pack=pack-$p2.idx && - - have_delta $a $b && - test $(midx_pack_source $a) != $(midx_pack_source $b) -' - -rev_list_tests 'full bitmap with backwards delta' - -test_expect_success 'clone with bitmaps enabled' ' - git clone --no-local --bare . clone-reverse-delta.git && - test_when_finished "rm -fr clone-reverse-delta.git" && - - git rev-parse HEAD >expect && - git --git-dir=clone-reverse-delta.git rev-parse HEAD >actual && - test_cmp expect actual -' +midx_bitmap_core bitmap_reuse_tests() { from=$1 @@ -213,18 +86,7 @@ test_expect_success 'missing object closure fails gracefully' ' ) ' -test_expect_success 'setup partial bitmaps' ' - test_commit packed && - git repack && - test_commit loose && - git multi-pack-index write --bitmap 2>err && - test_path_is_file $midx && - test_path_is_file $midx-$(midx_checksum $objdir).bitmap -' - -test_rev_exists HEAD~ - -basic_bitmap_tests HEAD~ +midx_bitmap_partial_tests test_expect_success 'removing a MIDX clears stale bitmaps' ' rm -fr repo && @@ -398,35 +260,4 @@ test_expect_success 'hash-cache values are propagated from pack bitmaps' ' ) ' -test_expect_success 'changing the preferred pack does not corrupt bitmaps' ' - rm -fr repo && - git init repo && - test_when_finished "rm -fr repo" && - ( - cd repo && - - test_commit A && - test_commit B && - - git rev-list --objects --no-object-names HEAD^ >A.objects && - git rev-list --objects --no-object-names HEAD^.. >B.objects && - - A=$(git pack-objects $objdir/pack/pack indexes <<-EOF && - pack-$A.idx - pack-$B.idx - EOF - - git multi-pack-index write --bitmap --stdin-packs \ - --preferred-pack=pack-$A.pack X-Patchwork-Id: 12703771 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 660E0C433F5 for ; Tue, 4 Jan 2022 18:16:02 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236158AbiADSQB (ORCPT ); Tue, 4 Jan 2022 13:16:01 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50866 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236179AbiADSP6 (ORCPT ); Tue, 4 Jan 2022 13:15:58 -0500 Received: from mail-qv1-xf36.google.com (mail-qv1-xf36.google.com [IPv6:2607:f8b0:4864:20::f36]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 74D08C061761 for ; Tue, 4 Jan 2022 10:15:58 -0800 (PST) Received: by mail-qv1-xf36.google.com with SMTP id kd9so35052328qvb.11 for ; Tue, 04 Jan 2022 10:15:58 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ttaylorr-com.20210112.gappssmtp.com; s=20210112; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=amRhedGuSHl1b2MYxVwt+xMWJ0GNSUpVqqCAdkmPxnQ=; b=6j3aKgg+qsdq8d9alseGPmRGQBhoHNsmnsNfOpPRHakmKoQvJK19qopuBfBaj/0UgB Oul4Ywj6RuW9o2TabQUBLh3kw3tdmUSMfH7kFVL/LPmnDOb/4relM1DFUf+hSq8ktKMl z66OKBgacp6xn0t0EM2WAPgSetX3M19v1bxpo9XPPhNqYWVD2Q5szfi2wKwelh4jHwTB cTGIff3YgFEM3ew87i4nCGp9tm+bHy7tC+7GvCumfSL//oNwyekbCPjRB5FydiqekmqC AbMwn0dxo9NT3Jbf7xM4B7+WunMv09HySWEASvXJTZS4cxYGmoDiRh8JQWEUj7NgC69P dJtw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=amRhedGuSHl1b2MYxVwt+xMWJ0GNSUpVqqCAdkmPxnQ=; b=r8W8QOvBGv16d+fl3NibOeGIEs7AEFF2igVpxAcV8yJr0bmpNIX2gsGh3BH4Y/JmPI u+wVn1gYJQINGBY5LsAD0DLBbxsWd0abCt8MnXEW4IK4h4f4wpPKx1tZOLWi8jIW5FX3 04rF7BUXSSpD8LQg84/oQEUia2zBNrIldISR1eR+Hkz5ejxAR1+oq0k3JJgsFvkttl6t 6yuzTIFu0iEIEftJaDGh8DXQSkivouY/IPN8IspzdTe6cXfYvXR+uBHQGfSg6z5RGDBg ZL/q7d588zn7ZCc5TdYxEUWiiw7Mjm7elIJix08JpQt4hXcJlTlFuWd5yMlchYB0u7YT HLaw== X-Gm-Message-State: AOAM530D9aMZuUHrEhuGX1vLg7KrMzCEl/m3C6lbjhYw+QniINM8mJ0o 8rC4K8eoZVWuwNFqFz6vWcb29E01i1cG5CFJ X-Google-Smtp-Source: ABdhPJyG3+b97BegTRLd9NbikEGCe4reshLK1yXzIiwJgr5ZWTCSUCYkzwFE3q50ggwC6vgzt0KK2A== X-Received: by 2002:a05:6214:29c1:: with SMTP id gh1mr46298575qvb.108.1641320157526; Tue, 04 Jan 2022 10:15:57 -0800 (PST) Received: from localhost (104-178-186-189.lightspeed.milwwi.sbcglobal.net. [104.178.186.189]) by smtp.gmail.com with ESMTPSA id j9sm31935089qkp.111.2022.01.04.10.15.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 04 Jan 2022 10:15:57 -0800 (PST) Date: Tue, 4 Jan 2022 13:15:56 -0500 From: Taylor Blau To: git@vger.kernel.org Cc: gitster@pobox.com, stolee@gmail.com Subject: [PATCH v3 7/9] t/lib-bitmap.sh: parameterize tests over reverse index source Message-ID: <7ce3dc60f93d5154943a63f1c09345bd47fbc7c9.1641320129.git.me@ttaylorr.com> References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org To prepare for reading the reverse index data out of the MIDX itself, teach the `test_rev_exists` function to take an expected "source" for the reverse index data. When given "rev", it asserts that the MIDX's `.rev` file exists, and is loaded when verifying the integrity of its bitmaps. Otherwise, it ensures that trace2 reports the source of the reverse index data as the same string which was given to test_rev_exists(). The following patch will implement reading the reverse index data from the MIDX itself. Signed-off-by: Taylor Blau --- t/lib-bitmap.sh | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/t/lib-bitmap.sh b/t/lib-bitmap.sh index 48a8730a13..77b5f46a03 100644 --- a/t/lib-bitmap.sh +++ b/t/lib-bitmap.sh @@ -275,17 +275,23 @@ midx_pack_source () { test_rev_exists () { commit="$1" + kind="$2" test_expect_success 'reverse index exists' ' GIT_TRACE2_EVENT=$(pwd)/event.trace \ git rev-list --test-bitmap "$commit" && - test_path_is_file $midx-$(midx_checksum $objdir).rev && - grep "\"category\":\"load_midx_revindex\",\"key\":\"source\",\"value\":\"rev\"" event.trace + if test "rev" = "$kind" + then + test_path_is_file $midx-$(midx_checksum $objdir).rev + fi && + grep "\"category\":\"load_midx_revindex\",\"key\":\"source\",\"value\":\"$kind\"" event.trace ' } midx_bitmap_core () { + rev_kind="${1:-rev}" + setup_bitmap_history test_expect_success 'create single-pack midx with bitmaps' ' @@ -295,7 +301,7 @@ midx_bitmap_core () { test_path_is_file $midx-$(midx_checksum $objdir).bitmap ' - test_rev_exists HEAD + test_rev_exists HEAD "$rev_kind" basic_bitmap_tests @@ -325,7 +331,7 @@ midx_bitmap_core () { test_path_is_file $midx-$(midx_checksum $objdir).bitmap ' - test_rev_exists HEAD + test_rev_exists HEAD "$rev_kind" basic_bitmap_tests @@ -428,6 +434,8 @@ midx_bitmap_core () { } midx_bitmap_partial_tests () { + rev_kind="${1:-rev}" + test_expect_success 'setup partial bitmaps' ' test_commit packed && git repack && @@ -437,7 +445,7 @@ midx_bitmap_partial_tests () { test_path_is_file $midx-$(midx_checksum $objdir).bitmap ' - test_rev_exists HEAD~ + test_rev_exists HEAD~ "$rev_kind" basic_bitmap_tests HEAD~ } From patchwork Tue Jan 4 18:16:00 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 12703772 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 1FB09C433EF for ; Tue, 4 Jan 2022 18:16:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236176AbiADSQD (ORCPT ); Tue, 4 Jan 2022 13:16:03 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50902 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236168AbiADSQC (ORCPT ); Tue, 4 Jan 2022 13:16:02 -0500 Received: from mail-qt1-x82d.google.com (mail-qt1-x82d.google.com [IPv6:2607:f8b0:4864:20::82d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EF687C061784 for ; Tue, 4 Jan 2022 10:16:01 -0800 (PST) Received: by mail-qt1-x82d.google.com with SMTP id z9so34932237qtj.9 for ; Tue, 04 Jan 2022 10:16:01 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ttaylorr-com.20210112.gappssmtp.com; s=20210112; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=0DwxVptzMT8NKeVdXjQG/9jsjfNgut0iEpUqbDNaH2c=; b=LMYrE9/RxURJysScm6SUc+aXrDaRfjsAiwYtXfr0qiVhxJAK8661KTc28YwrfMZiyQ SOzlm3xWsWLKUn+iJWjyGorjxJQHw4/rYi7hEl1LDvX/mzXscHItq7Hft3J/3qsxTWOY QqREzP/U/wXnizAhy9f3Z+XbKrfECpbcdEI1t3AAGl4iO42ps0ho3ed7U6HA3YAOUIQx OUx2eYwogFU6UIMG50MHFzoLOo4Eg5xvKUOeRJmyZ0C8nhuP/axNwOn1jjB2fc3OAER3 Xu5ipon76K+GShsPlNIShCloRHpmGA9hgyX/FcfdQqgyf8+cr5eAaUQ7wsnpkhBVtueg j2vQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=0DwxVptzMT8NKeVdXjQG/9jsjfNgut0iEpUqbDNaH2c=; b=rgNVZ7W+aiqAlfMaxzfh/SOZOHpGnZpY2KnrwlJf6sYGhnDCTnp9oz+GKNN+4J3041 GfJFPnUaahtDFOErof/EQuuKbMmSaIonhgXw18Np/ULk/DQ2PY/oRBt0fNqxnUonjPdI XNvId8qVdu5Ej+WQ66dDmlFE4yZ/9eYeplQeVFVHRhQG9BKf+VDueZ9b3HkQMjU0bMs4 ynb1E0eTzFgG6sIuscUnXxWTOTP+2WWHY6/u1QTs2X0QPLLvWbiiaAs8c0JCL4I6yKyp HaarfTxGsF6SEQ9varcc1AlKhOVn5eIxZgnDqlbgV5XDgn4heyS62wumnhVYiUUXrNO6 UZ4g== X-Gm-Message-State: AOAM5314rpwdDMMGbIf2fTOMiS5PPwd/b/AsSFdIZmZPzIL17en6A9zK WiFQkfR9xJippHiGf0X/WgviCPcIUmGejFBQ X-Google-Smtp-Source: ABdhPJwa36i9OFacKNn9tI5UqKQZo0DKuNqUPxKl5+TaxC/HWCoIcsSwP38rodCHtmDrHMB/V1Qlgw== X-Received: by 2002:ac8:4514:: with SMTP id q20mr45056738qtn.202.1641320160951; Tue, 04 Jan 2022 10:16:00 -0800 (PST) Received: from localhost (104-178-186-189.lightspeed.milwwi.sbcglobal.net. [104.178.186.189]) by smtp.gmail.com with ESMTPSA id l13sm32660970qkp.109.2022.01.04.10.16.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 04 Jan 2022 10:16:00 -0800 (PST) Date: Tue, 4 Jan 2022 13:16:00 -0500 From: Taylor Blau To: git@vger.kernel.org Cc: gitster@pobox.com, stolee@gmail.com Subject: [PATCH v3 8/9] midx: read `RIDX` chunk when present Message-ID: <55aa69de12c5f82a66836e829f915363cc73b421.1641320129.git.me@ttaylorr.com> References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org When a MIDX contains the new `RIDX` chunk, ensure that the reverse index is read from it instead of the on-disk .rev file. Since we need to encode the object order in the MIDX itself for correctness reasons, there is no point in storing the same data again outside of the MIDX. So, this patch stops writing separate .rev files, and reads it out of the MIDX itself. This is possible to do with relatively little new code, since the format of the RIDX chunk is identical to the data in the .rev file. In other words, we can implement this by pointing the `revindex_data` field at the reverse index chunk of the MIDX instead of the .rev file without any other changes. Note that we have two knobs that are adjusted for the new tests: GIT_TEST_MIDX_WRITE_REV and GIT_TEST_MIDX_READ_RIDX. The former controls whether the MIDX .rev is written at all, and the latter controls whether we read the MIDX's RIDX chunk. Both are necessary to ensure that the test added at the beginning of this series continues to work. This is because we always need to write the RIDX chunk in the MIDX in order to change its checksum, but we want to make sure reading the existing .rev file still works (since the RIDX chunk takes precedence by default). Arguably this isn't a very interesting mode to test, because the precedence rules mean that we'll always read the RIDX chunk over the .rev file. But it makes it impossible for a user to induce corruption in their repository by adjusting the test knobs (since if we had an either/or knob they could stop writing the RIDX chunk, allowing them to tweak the MIDX's object order without changing its checksum). Signed-off-by: Taylor Blau --- midx.c | 6 +++++- midx.h | 1 + pack-revindex.c | 17 +++++++++++++++++ t/lib-bitmap.sh | 4 ++-- t/t5326-multi-pack-bitmaps.sh | 6 ++++++ t/t5327-multi-pack-bitmaps-rev.sh | 23 +++++++++++++++++++++++ t/t7700-repack.sh | 4 ---- 7 files changed, 54 insertions(+), 7 deletions(-) create mode 100755 t/t5327-multi-pack-bitmaps-rev.sh diff --git a/midx.c b/midx.c index d3179e9c02..9aba13b5b1 100644 --- a/midx.c +++ b/midx.c @@ -162,6 +162,9 @@ struct multi_pack_index *load_multi_pack_index(const char *object_dir, int local pair_chunk(cf, MIDX_CHUNKID_LARGEOFFSETS, &m->chunk_large_offsets); + if (git_env_bool("GIT_TEST_MIDX_READ_RIDX", 1)) + pair_chunk(cf, MIDX_CHUNKID_REVINDEX, &m->chunk_revindex); + m->num_objects = ntohl(m->chunk_oid_fanout[255]); CALLOC_ARRAY(m->pack_names, m->num_packs); @@ -1429,7 +1432,8 @@ static int write_midx_internal(const char *object_dir, finalize_hashfile(f, midx_hash, CSUM_FSYNC | CSUM_HASH_IN_STREAM); free_chunkfile(cf); - if (flags & MIDX_WRITE_REV_INDEX) + if (flags & MIDX_WRITE_REV_INDEX && + git_env_bool("GIT_TEST_MIDX_WRITE_REV", 0)) write_midx_reverse_index(midx_name.buf, midx_hash, &ctx); if (flags & MIDX_WRITE_BITMAP) { if (write_midx_bitmap(midx_name.buf, midx_hash, &ctx, diff --git a/midx.h b/midx.h index b7d79a515c..22e8e53288 100644 --- a/midx.h +++ b/midx.h @@ -36,6 +36,7 @@ struct multi_pack_index { const unsigned char *chunk_oid_lookup; const unsigned char *chunk_object_offsets; const unsigned char *chunk_large_offsets; + const unsigned char *chunk_revindex; const char **pack_names; struct packed_git **packs; diff --git a/pack-revindex.c b/pack-revindex.c index bd15ebad03..08dc160167 100644 --- a/pack-revindex.c +++ b/pack-revindex.c @@ -298,9 +298,26 @@ int load_midx_revindex(struct multi_pack_index *m) { struct strbuf revindex_name = STRBUF_INIT; int ret; + if (m->revindex_data) return 0; + if (m->chunk_revindex) { + /* + * If the MIDX `m` has a `RIDX` chunk, then use its contents for + * the reverse index instead of trying to load a separate `.rev` + * file. + * + * Note that we do *not* set `m->revindex_map` here, since we do + * not want to accidentally call munmap() in the middle of the + * MIDX. + */ + trace2_data_string("load_midx_revindex", the_repository, + "source", "midx"); + m->revindex_data = (const uint32_t *)m->chunk_revindex; + return 0; + } + trace2_data_string("load_midx_revindex", the_repository, "source", "rev"); diff --git a/t/lib-bitmap.sh b/t/lib-bitmap.sh index 77b5f46a03..365d990ce3 100644 --- a/t/lib-bitmap.sh +++ b/t/lib-bitmap.sh @@ -290,7 +290,7 @@ test_rev_exists () { } midx_bitmap_core () { - rev_kind="${1:-rev}" + rev_kind="${1:-midx}" setup_bitmap_history @@ -434,7 +434,7 @@ midx_bitmap_core () { } midx_bitmap_partial_tests () { - rev_kind="${1:-rev}" + rev_kind="${1:-midx}" test_expect_success 'setup partial bitmaps' ' test_commit packed && diff --git a/t/t5326-multi-pack-bitmaps.sh b/t/t5326-multi-pack-bitmaps.sh index 100ac90d15..c0924074c4 100755 --- a/t/t5326-multi-pack-bitmaps.sh +++ b/t/t5326-multi-pack-bitmaps.sh @@ -9,6 +9,12 @@ test_description='exercise basic multi-pack bitmap functionality' GIT_TEST_MULTI_PACK_INDEX=0 GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP=0 +# This test exercise multi-pack bitmap functionality where the object order is +# stored and read from a special chunk within the MIDX, so use the default +# behavior here. +sane_unset GIT_TEST_MIDX_WRITE_REV +sane_unset GIT_TEST_MIDX_READ_RIDX + midx_bitmap_core bitmap_reuse_tests() { diff --git a/t/t5327-multi-pack-bitmaps-rev.sh b/t/t5327-multi-pack-bitmaps-rev.sh new file mode 100755 index 0000000000..d30ba632c8 --- /dev/null +++ b/t/t5327-multi-pack-bitmaps-rev.sh @@ -0,0 +1,23 @@ +#!/bin/sh + +test_description='exercise basic multi-pack bitmap functionality (.rev files)' + +. ./test-lib.sh +. "${TEST_DIRECTORY}/lib-bitmap.sh" + +# We'll be writing our own midx and bitmaps, so avoid getting confused by the +# automatic ones. +GIT_TEST_MULTI_PACK_INDEX=0 +GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP=0 + +# Unlike t5326, this test exercise multi-pack bitmap functionality where the +# object order is stored in a separate .rev file. +GIT_TEST_MIDX_WRITE_REV=1 +GIT_TEST_MIDX_READ_RIDX=0 +export GIT_TEST_MIDX_WRITE_REV +export GIT_TEST_MIDX_READ_RIDX + +midx_bitmap_core rev +midx_bitmap_partial_tests rev + +test_done diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh index 4693f8dc2b..02a6633a16 100755 --- a/t/t7700-repack.sh +++ b/t/t7700-repack.sh @@ -311,16 +311,13 @@ test_expect_success 'cleans up MIDX when appropriate' ' checksum=$(midx_checksum $objdir) && test_path_is_file $midx && test_path_is_file $midx-$checksum.bitmap && - test_path_is_file $midx-$checksum.rev && test_commit repack-3 && GIT_TEST_MULTI_PACK_INDEX=0 git repack -Adb --write-midx && test_path_is_file $midx && test_path_is_missing $midx-$checksum.bitmap && - test_path_is_missing $midx-$checksum.rev && test_path_is_file $midx-$(midx_checksum $objdir).bitmap && - test_path_is_file $midx-$(midx_checksum $objdir).rev && test_commit repack-4 && GIT_TEST_MULTI_PACK_INDEX=0 git repack -Adb && @@ -353,7 +350,6 @@ test_expect_success '--write-midx with preferred bitmap tips' ' test_line_count = 1 before && rm -fr $midx-$(midx_checksum $objdir).bitmap && - rm -fr $midx-$(midx_checksum $objdir).rev && rm -fr $midx && # instead of constructing the snapshot ourselves (c.f., the test From patchwork Tue Jan 4 18:16:02 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 12703773 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 75235C433F5 for ; Tue, 4 Jan 2022 18:16:08 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236177AbiADSQI (ORCPT ); Tue, 4 Jan 2022 13:16:08 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50930 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236184AbiADSQE (ORCPT ); Tue, 4 Jan 2022 13:16:04 -0500 Received: from mail-qv1-xf30.google.com (mail-qv1-xf30.google.com [IPv6:2607:f8b0:4864:20::f30]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 58557C061761 for ; Tue, 4 Jan 2022 10:16:04 -0800 (PST) Received: by mail-qv1-xf30.google.com with SMTP id kc16so35192774qvb.3 for ; Tue, 04 Jan 2022 10:16:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ttaylorr-com.20210112.gappssmtp.com; s=20210112; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=tYyExQra/AYrSHJ9ncfj6OUd6kL1FAK9ugP68Yl3AMU=; b=DBHvSI3EpshEnUp8kOp6MT7OyaT+vwyWo+A4gtOCea49WKDMRN8OsIpgnBTopvOa3G NdktQDxK2AR5C7iP8YB6jdUwRyoiC0zxbQOdwgQDj0yyZgf9D0B1RwHm+XM8PAgnXlvz J8sB6Doji/T75jrFYpSvfPcSuX38Ln7T8QA8CVOoiJmeChYZNWDWYo7jAkLqrF58lYNw icco7+uYCOozamv4bF+/G+Hmau1k9bTD6o8UfKYSCC2SVZNaLu9b1S/nRCAAhkrs2LDg pIFCWEqa3xZJUII35hPm1WtHcXxFIXbkqeUK3EK6B/6znRzGG8Z6dNFbU4JoUN5FXnc3 3+Dg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=tYyExQra/AYrSHJ9ncfj6OUd6kL1FAK9ugP68Yl3AMU=; b=NDKuiEWuXOCmXy7lF3sGz7T3jbcSs8yUbm+3HDBGfocwCO7g1DL88/xb7S7RbjhnXM 8nCaeFO2qMPzwXgGebzlCrdU1Zk7Nc4NcMdwLyugiSMfrcmvAZqshzxENYSBSP+uBRoW UfFKov1YhfSfwbOomJjUrTRrXDCQyLboqfGlapxOY4L7jWZ1PgnI2OiKVmAQM4jd4qNB /+/mPXhEakafCk+Ld5Ekw3k4I1OY3Q4waNaEX/prfy/GS8qvZG8NxkT2aVJZNISrT9aX GgdIr/Vkmv4UZGgbXt9WA5dZpGSMsy70nBhU7DM0QPt99/gV5hagAX4UR+n5koZLRTGR FxLw== X-Gm-Message-State: AOAM533uK4H3gFaUKUM4si2WN/ZFWe1FKDK/OjuvvyHPx4Txhk9KAJ8o wzUyLRaiO2zan8EsMIAKGEfyZgxVlWaiekKL X-Google-Smtp-Source: ABdhPJzbX23JCDb/1w1Lmo/Iq1/b6WUf54pOODJT1Wher8ZIdByruYirNLv87ir49sst1dj4HaPVFw== X-Received: by 2002:a05:6214:248a:: with SMTP id gi10mr42979931qvb.101.1641320163412; Tue, 04 Jan 2022 10:16:03 -0800 (PST) Received: from localhost (104-178-186-189.lightspeed.milwwi.sbcglobal.net. [104.178.186.189]) by smtp.gmail.com with ESMTPSA id h16sm35323087qtx.20.2022.01.04.10.16.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 04 Jan 2022 10:16:03 -0800 (PST) Date: Tue, 4 Jan 2022 13:16:02 -0500 From: Taylor Blau To: git@vger.kernel.org Cc: gitster@pobox.com, stolee@gmail.com Subject: [PATCH v3 9/9] pack-bitmap.c: gracefully fallback after opening pack/MIDX Message-ID: <9707d5ea4433d9a5c7f8581dbb2d8a05f410efd3.1641320129.git.me@ttaylorr.com> References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org When opening a MIDX/pack-bitmap, we call open_midx_bitmap_1() or open_pack_bitmap_1() respectively in a loop over the set of MIDXs/packs. By design, these functions are supposed to be called over every pack and MIDX, since only one of them should have a valid bitmap. Ordinarily we return '0' from these two functions in order to indicate that we successfully loaded a bitmap To signal that we couldn't load a bitmap corresponding to the MIDX/pack (either because one doesn't exist, or because there was an error with loading it), we can return '-1'. In either case, the callers each enumerate all MIDXs/packs to ensure that at most one bitmap per-kind is present. But when we fail to load a bitmap that does exist (for example, loading a MIDX bitmap without finding a corresponding reverse index), we'll return -1 but leave the 'midx' field non-NULL. So when we fallback to loading a pack bitmap, we'll complain that the bitmap we're trying to populate already is "opened", even though it isn't. Rectify this by setting the '->pack' and '->midx' field back to NULL as appropriate. Two tests are added: one to ensure that the MIDX-to-pack bitmap fallback works, and another to ensure we still complain when there are multiple pack bitmaps in a repository. Signed-off-by: Taylor Blau --- pack-bitmap.c | 4 ++++ t/t5310-pack-bitmaps.sh | 28 ++++++++++++++++++++++++++++ t/t5326-multi-pack-bitmaps.sh | 19 +++++++++++++++++++ 3 files changed, 51 insertions(+) diff --git a/pack-bitmap.c b/pack-bitmap.c index f772d3cb7f..9c666cdb8b 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -358,7 +358,9 @@ static int open_midx_bitmap_1(struct bitmap_index *bitmap_git, cleanup: munmap(bitmap_git->map, bitmap_git->map_size); bitmap_git->map_size = 0; + bitmap_git->map_pos = 0; bitmap_git->map = NULL; + bitmap_git->midx = NULL; return -1; } @@ -405,6 +407,8 @@ static int open_pack_bitmap_1(struct bitmap_index *bitmap_git, struct packed_git munmap(bitmap_git->map, bitmap_git->map_size); bitmap_git->map = NULL; bitmap_git->map_size = 0; + bitmap_git->map_pos = 0; + bitmap_git->pack = NULL; return -1; } diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh index d05ab716f6..f775fc1ce6 100755 --- a/t/t5310-pack-bitmaps.sh +++ b/t/t5310-pack-bitmaps.sh @@ -397,4 +397,32 @@ test_expect_success 'pack.preferBitmapTips' ' ) ' +test_expect_success 'complains about multiple pack bitmaps' ' + rm -fr repo && + git init repo && + test_when_finished "rm -fr repo" && + ( + cd repo && + + test_commit base && + + git repack -adb && + bitmap="$(ls .git/objects/pack/pack-*.bitmap)" && + mv "$bitmap" "$bitmap.bak" && + + test_commit other && + git repack -ab && + + mv "$bitmap.bak" "$bitmap" && + + find .git/objects/pack -type f -name "*.pack" >packs && + find .git/objects/pack -type f -name "*.bitmap" >bitmaps && + test_line_count = 2 packs && + test_line_count = 2 bitmaps && + + git rev-list --use-bitmap-index HEAD 2>err && + grep "ignoring extra bitmap file" err + ) +' + test_done diff --git a/t/t5326-multi-pack-bitmaps.sh b/t/t5326-multi-pack-bitmaps.sh index c0924074c4..3c1ecc7e25 100755 --- a/t/t5326-multi-pack-bitmaps.sh +++ b/t/t5326-multi-pack-bitmaps.sh @@ -266,4 +266,23 @@ test_expect_success 'hash-cache values are propagated from pack bitmaps' ' ) ' +test_expect_success 'graceful fallback when missing reverse index' ' + rm -fr repo && + git init repo && + test_when_finished "rm -fr repo" && + ( + cd repo && + + test_commit base && + + # write a pack and MIDX bitmap containing base + git repack -adb && + git multi-pack-index write --bitmap && + + GIT_TEST_MIDX_READ_RIDX=0 \ + git rev-list --use-bitmap-index HEAD 2>err && + ! grep "ignoring extra bitmap file" err + ) +' + test_done