From patchwork Tue Dec 14 01:55:28 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 12675233 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 C1A4BC433EF for ; Tue, 14 Dec 2021 01:55:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S243430AbhLNBzc (ORCPT ); Mon, 13 Dec 2021 20:55:32 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59168 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237640AbhLNBzb (ORCPT ); Mon, 13 Dec 2021 20:55:31 -0500 Received: from mail-qk1-x733.google.com (mail-qk1-x733.google.com [IPv6:2607:f8b0:4864:20::733]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C81D5C061574 for ; Mon, 13 Dec 2021 17:55:30 -0800 (PST) Received: by mail-qk1-x733.google.com with SMTP id t83so15614335qke.8 for ; Mon, 13 Dec 2021 17:55:30 -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=PADQrfQoa+1+9IP738e0xBN43JOW51CDciMFik14pdH9bjtFCSu0J5WQ8m4sZ6sp36 ooCujeR17JKsAvU2IcIcUqlIRy9Q0nCVHEcNIEeAnC2uzLpM7amkFXEWK5931Z+dSZI9 2CljjV/Y4LapbKTnnN7KQtRIhwgJjC4qy/cBQx4SMTvEsRy9Sq/EHRGkIeddqOzg8T9w D58EBLHEs1Gw6+qmFxtL0Q58Db0gYL/AGU34oZmFSD6WwjriHKDVkEvtGsYWaCNk4iJt PbozmAbkf7jmUAmWyArQGv6snOKYqXECp5UM5B0FEv+LzyJBYNcAuCZrCyHoBIBni6Of AVyQ== 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=z5DWQ1pxes5JHoP4BJxiUouPjjx5AdygsR5eLUwaGewbLy4o0T6hOjXD44bpAhfLld 25UALYJ0MjV7uz4b6wT+sAvjgEqEKGjErVSoQMC/QUzLz/PwjZOZIS0FaKbjCxbv+oOd HAIYy54kOYt0jxVo7Erps5mTXju8W7gWT7ALDUgdjqgwlCBK/AKEkoUEjsIKZvqoZNeu f0Ao7rwzeufv4Kjs/G+Gr4Vwb9o3rQksUgMQttAAsRbwEOSj0jkmF+/6pKNLUx4VksHi rwPI4xPgQkrpxY2kU/85QoToJ2fHVf83S/ECJZwNZNDH2IQjoNgdnb/3n/m4lph/7Hvk /P5A== X-Gm-Message-State: AOAM533VwvSAXh3nme0DougIcLSxIQulwza57DYpcqAgzdDtQcUtTOjG JZyOHtvRwy1I4V8g7MqjL8ciVYpY6cfp16BC X-Google-Smtp-Source: ABdhPJwb5BGXVD+oTjyefSmmwSHxEu969Tu4pF10nVHh/Q0OJZ93WBmq93eGIAEkF/acg+OztmxAQw== X-Received: by 2002:a05:620a:4452:: with SMTP id w18mr1743525qkp.625.1639446929801; Mon, 13 Dec 2021 17:55:29 -0800 (PST) Received: from localhost (104-178-186-189.lightspeed.milwwi.sbcglobal.net. [104.178.186.189]) by smtp.gmail.com with ESMTPSA id t11sm10893035qtx.48.2021.12.13.17.55.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 13 Dec 2021 17:55:29 -0800 (PST) Date: Mon, 13 Dec 2021 20:55:28 -0500 From: Taylor Blau To: git@vger.kernel.org Cc: gitster@pobox.com, peff@peff.net, stolee@gmail.com Subject: [PATCH v2 1/8] 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: 12675235 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 1D49CC433EF for ; Tue, 14 Dec 2021 01:55:35 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S243513AbhLNBze (ORCPT ); Mon, 13 Dec 2021 20:55:34 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59182 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S243484AbhLNBzd (ORCPT ); Mon, 13 Dec 2021 20:55:33 -0500 Received: from mail-qk1-x72f.google.com (mail-qk1-x72f.google.com [IPv6:2607:f8b0:4864:20::72f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9DC00C061574 for ; Mon, 13 Dec 2021 17:55:33 -0800 (PST) Received: by mail-qk1-x72f.google.com with SMTP id 132so15607711qkj.11 for ; Mon, 13 Dec 2021 17:55:33 -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=/u+3UqWkyZrY+h7xDMbCB6JOKa3YT+Bu5dB8xuN1RIc=; b=0ktg5smCimN6GnJhKI17ZKP7QY6G6G2xXid9DoGiGSybUoHwbhJLQH9tKkdMO+1QJc E7qTQ0qeB1tJ707CJibIihMHvBrzSal2b+70DHwB4etSSHpO57k/wdimjOoCy3Sp5iMU ms7/oFfduIu9C4hHREzAuJ0vzbg7htJd9e3IgpXbX9kQ0Xp/8REMkjjPzqfHPgMbPq8k 0eLo9h5qzr27idvCCx3Zp5Tnq03QkpF8kIl+x8N+nUOAle0BvMisiB9nHr4pP0+jdwLV jgnb5/e9ZvYqIZiL6eXwNDobLpZNG0z2B6m6bzdk1Mqjri6athEIuVgABQrX7Pr/fKw3 ek4w== 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=/u+3UqWkyZrY+h7xDMbCB6JOKa3YT+Bu5dB8xuN1RIc=; b=XC9aghBvZ5vY8uv0MEKC0Fzdyv96lQmgomY3HRt5miACbJgn1OEV15INxHkZUqe8y3 +RHwYu6RkJHh9fdJNuXqcu8VvU8oYFCxSZyP8Y3wY29+05+r/tMQGL94lWcPLbnNvFph T+Jk26jIrtxOOsadFW5hhkNUbwgiwfnXI2igRCFmShOWUQ5rY24QAFOOg4t15SBY7Fpg ncHxax621v2b7PtJVwnFEYfr2vg7u2VTpek4PQpN2I6kr5V638OhuCoF48Ak85mdrtA+ lEND47KNYQSPEr82TT2+eYIQ/oAW7UCcVlGGC8tPZzgwNiObJ0dSB1zlJZC/kC72jiAM kpXQ== X-Gm-Message-State: AOAM531ljSO6x0xGptzTPiHlEoUT5hbaozlMVmsw3ieMknUS+nL2dhIz X5N8xQhqsGDU6RlecjdhJTTdT5TGSeq4BcUO X-Google-Smtp-Source: ABdhPJz3GI5/kbnkcyXZbNUtltOGuxPmuJVIWOXiC9TXdGuzKZQLIqRfqRwiPyI7PxjjQa1XOwKY/w== X-Received: by 2002:a05:620a:d94:: with SMTP id q20mr1751987qkl.482.1639446932354; Mon, 13 Dec 2021 17:55:32 -0800 (PST) Received: from localhost (104-178-186-189.lightspeed.milwwi.sbcglobal.net. [104.178.186.189]) by smtp.gmail.com with ESMTPSA id g12sm11341774qtk.69.2021.12.13.17.55.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 13 Dec 2021 17:55:32 -0800 (PST) Date: Mon, 13 Dec 2021 20:55:31 -0500 From: Taylor Blau To: git@vger.kernel.org Cc: gitster@pobox.com, peff@peff.net, stolee@gmail.com Subject: [PATCH v2 2/8] midx.c: make changing the preferred pack safe Message-ID: <4ea52e66dd6cbc5ca32c36c60266e9757b784a04.1639446906.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 86f40f2490..6e1fb092c6 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 Dec 14 01:55:33 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 12675237 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 62162C433EF for ; Tue, 14 Dec 2021 01:55:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S243563AbhLNBzh (ORCPT ); Mon, 13 Dec 2021 20:55:37 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59204 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S243506AbhLNBzg (ORCPT ); Mon, 13 Dec 2021 20:55:36 -0500 Received: from mail-qv1-xf32.google.com (mail-qv1-xf32.google.com [IPv6:2607:f8b0:4864:20::f32]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D9F57C061751 for ; Mon, 13 Dec 2021 17:55:35 -0800 (PST) Received: by mail-qv1-xf32.google.com with SMTP id i12so16112872qvh.11 for ; Mon, 13 Dec 2021 17:55:35 -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=lI6ZErtc3HHhHyh0yxzMo9ChxxLU3ThWQhTxtp/dr98cx4oQ+H0dpc6pIQR6xw/aK4 taKhDDdrimepAD07MBWJEhUVgSHwjHgvJaZTh2agAEwto6VY5s+3YdJnP2VHLD1Yu5GH YEE+N6iR3e4hufnYrc/FHXePFDZs8n7ZsND7YCdhyFk+c9bTAMcFZAA9lOnRKuakaQSw sQ8WjvtZKLAM65NJytSZU/jfOfQYpSdjBT5m5FL+7Y3OTv4vB3JIy/U+9SvUyKkePFu1 gKZbbCxrHZIXbxdy75xcHcyNXCLqlNKELjPd2z2zSCqrQ99TeEqYHOtoY+sEXXBlW8so k7rw== 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=zVxNfKDZy+VxAw3REV4QL4JwOpHezpBmah9qByv2xXynQn3wmQLSVO5+B8VFckQ3kR rj9nBnxcU1WfJWL7N6R3rmsbQWfhklihCl0cg1GIeCvkKCbkdCGy2xi3R5OwmRRFTKo7 apWhDGrqqk3pdhd8rER1Qa0K2Ik/v/NQJDPq5Dc7ZbzOEhXjqqj/9D2cEO8/x/f9n6Y4 Fh5QDXHYH/IU1cy7JmL67Dj8ZBsHqtWfLfupvWV7N8zp7WVJCqrRweavlSxHCgqEPHe5 UVcGCY0Z/doZqL8x5KnOwTE3ePZJRP+wst2GbTF3LC94nMouTZAwBbB2bXEaXW7hIduO 5UCQ== X-Gm-Message-State: AOAM531aHkWuBpS0aEU1eOspy2KqeQwSyWZugvRun1UNDYI8kGcvm8lm bdnTlOKX/RWlg31w02lpCqt9Vt35FlelMx21 X-Google-Smtp-Source: ABdhPJyAwRNc+ze/2g9sS1tZEff1qM+KeVEFWl5p/TPGJjewDu/eV8d6WZFyk1rxKVOa/IPSGFxGxg== X-Received: by 2002:a05:6214:508f:: with SMTP id kk15mr2248813qvb.3.1639446934953; Mon, 13 Dec 2021 17:55:34 -0800 (PST) Received: from localhost (104-178-186-189.lightspeed.milwwi.sbcglobal.net. [104.178.186.189]) by smtp.gmail.com with ESMTPSA id x16sm6936166qko.15.2021.12.13.17.55.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 13 Dec 2021 17:55:34 -0800 (PST) Date: Mon, 13 Dec 2021 20:55:33 -0500 From: Taylor Blau To: git@vger.kernel.org Cc: gitster@pobox.com, peff@peff.net, stolee@gmail.com Subject: [PATCH v2 3/8] pack-revindex.c: instrument loading on-disk reverse index Message-ID: 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 Dec 14 01:55:36 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 12675239 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 EE05AC433EF for ; Tue, 14 Dec 2021 01:55:40 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S243484AbhLNBzk (ORCPT ); Mon, 13 Dec 2021 20:55:40 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59216 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S243861AbhLNBzi (ORCPT ); Mon, 13 Dec 2021 20:55:38 -0500 Received: from mail-il1-x12c.google.com (mail-il1-x12c.google.com [IPv6:2607:f8b0:4864:20::12c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0D662C061574 for ; Mon, 13 Dec 2021 17:55:38 -0800 (PST) Received: by mail-il1-x12c.google.com with SMTP id i13so5714847ilk.13 for ; Mon, 13 Dec 2021 17:55:38 -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=j4ytsUXw/WXHsy81vZRE30BvqUWXS8QMu1j0GGyDnymelkVGwpEsiCtg2A6QCdkLOE eT8baMB2U49L8GhYX7QPnVG8nvKknLjPwOeGV0H5QyAQXh7Vg/Zi9CQ0T602+I6YZrol X+ZDZL8mudnmjM8n+LghJ2+w0LFw7n7l22xnMt6dIGmdaPp4K98rUAaq+sT+jfl/of/1 pUtXGnez6+CVSOfdVDpFNpx4fasUhPxPzff2W5fLQA+puRflCR1ofxndXCKlS3zaecHt 35VBN9Y6ciWiqHM/IoN4Xhna9/yIuQSNoGnQboiNzdAYGfpd9BH1K2M6hSrok6f/Y66f 9KEg== 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=nhI51P7lrwn0VBGbMF2DoKrPWtpKu+CMX66rI/j7eLD2SfPfOwymE0Zy/D+4ZujofP vh9XID+BhRdU65f127v1VSalD94QsDUtM6Tal+hECG4PED43KqyV9J0A7G4z6qVZy8Wx Acfxkv4LPpSAnIlyZCRXqXrpYJjUvSQr1PPruCnM5RweGPyvC6mEMr1DTCkv7Yo8O6OW phvzP5y5hUkRoS0jHsHWexyitO2q8r4sFZ1R+H98fFXLU1G/Z+C4N4rd4KzeZe1X0UKt iMGasdoJ86pWeFKnOpu83F3CbHu0hs/jpq6fPY8FFXZHh7KOPXsdEI3t9AlJGZ+oH/vb nuoA== X-Gm-Message-State: AOAM531J7o2uHrbq9zRGDHUo1o0oeqlWvPg/B4QLMX7NucClIfDbeS9R A7QMDAYdmir//jCCXmnYDRqt41SAqtjGm2vm X-Google-Smtp-Source: ABdhPJzEZko69uanejYp4r8aw1ahrTLqJRCzDNrQAZ56ROVe/68i2pwQHksjYCEPdnJmedD5WyZ3vw== X-Received: by 2002:a05:6e02:216b:: with SMTP id s11mr1561268ilv.228.1639446937223; Mon, 13 Dec 2021 17:55:37 -0800 (PST) Received: from localhost (104-178-186-189.lightspeed.milwwi.sbcglobal.net. [104.178.186.189]) by smtp.gmail.com with ESMTPSA id 8926c6da1cb9f-2faa6b4dab7sm186799173.205.2021.12.13.17.55.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 13 Dec 2021 17:55:36 -0800 (PST) Date: Mon, 13 Dec 2021 20:55:36 -0500 From: Taylor Blau To: git@vger.kernel.org Cc: gitster@pobox.com, peff@peff.net, stolee@gmail.com Subject: [PATCH v2 4/8] t5326: drop unnecessary setup Message-ID: 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 Dec 14 01:55:38 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 12675241 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 45A7CC433F5 for ; Tue, 14 Dec 2021 01:55:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S244169AbhLNBzm (ORCPT ); Mon, 13 Dec 2021 20:55:42 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59248 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S244296AbhLNBzl (ORCPT ); Mon, 13 Dec 2021 20:55:41 -0500 Received: from mail-io1-xd2d.google.com (mail-io1-xd2d.google.com [IPv6:2607:f8b0:4864:20::d2d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 40D71C0613F8 for ; Mon, 13 Dec 2021 17:55:40 -0800 (PST) Received: by mail-io1-xd2d.google.com with SMTP id e128so21485378iof.1 for ; Mon, 13 Dec 2021 17:55:40 -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=6RimBHqEjmYfmIr76gFc1c7hkCw71cBB5Tu8e3M8pIg=; b=ektnd6kGDxSdGXuJlCi0EuuxPJ6UdOyqrIvQrqLIuoPKuwveIRkscyeSdFuOTcptbs /CvKDrHq2BbD28SqyHH4FE9zP9aqbg9aGlePqa9U8d46lGj7sjzdV9wM6FhyzgGN5rtO UBBqZzCcBL/2K74IO6L1fkdwb3uBf02qPThW4YJgQakxZwO1PEDvaisgAjzAEbsXpvRl uhCaV4H9RdLY1+2VDCf+mzuAWZC0W+E33M/X4FidiWsgdlPMSxmRQOnmoWwXZSBxpSZj f4YLJX+gXRJ/To6BpbPZ/hfVCmVp9MmkksyQEZeLeB9lgnk7QOFJlDYG600Olbeqlu0S ow+A== 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=6RimBHqEjmYfmIr76gFc1c7hkCw71cBB5Tu8e3M8pIg=; b=h3CoBxPT46unEi0RuwcXMVZGPUZldjeESRUiroa5Cl05kl6qUWRirUsfd4aTyrbf04 NbT60wAQK7F6eCjLtSzrIkHhn3xM5dkJixjShpRFeQQxSOKv9nD/IFHpNPEFtEOJ7Sy0 zbqWYFOP8fberH09TxOpz5p5fdS2WGNlv+9IGC7NIA5BX4TYUnbQXefcz1PGINJRhTME ufYCuoobsgu+AEkF1H6EHx2A+fwEj/RGeooGFu6w/cYaBBKkMu8DuFfsvL45Hoiij1Nb GaTx2pi+yYa1Mq8BK6RVgGm+0RnxU0LEluPry9SPDG0ZQoeaaZVzGPQHetuS2II+2AYD aJSw== X-Gm-Message-State: AOAM531lK5SJ6/qxtwSvUQWnnjBuuYQQ8KvSjZqv70iKLnyiwOxUJseW bBPjcN2qvBSur8Vda6exHuASIjif1aBCiBDw X-Google-Smtp-Source: ABdhPJyEbIJ3r9ZR9QEOMf/uu61fXjk16oktFzgVR6gFKchWDZrrNfSqxyPybGAITp8e3/Gro+2EFg== X-Received: by 2002:a05:6602:150b:: with SMTP id g11mr1627124iow.119.1639446939556; Mon, 13 Dec 2021 17:55:39 -0800 (PST) Received: from localhost (104-178-186-189.lightspeed.milwwi.sbcglobal.net. [104.178.186.189]) by smtp.gmail.com with ESMTPSA id 8926c6da1cb9f-2faa6b4daeasm187111173.274.2021.12.13.17.55.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 13 Dec 2021 17:55:39 -0800 (PST) Date: Mon, 13 Dec 2021 20:55:38 -0500 From: Taylor Blau To: git@vger.kernel.org Cc: gitster@pobox.com, peff@peff.net, stolee@gmail.com Subject: [PATCH v2 5/8] t5326: extract `test_rev_exists` Message-ID: <73faab9f429221b7be88ea88ce59bc47afb57348.1639446906.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 | 38 +++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/t/t5326-multi-pack-bitmaps.sh b/t/t5326-multi-pack-bitmaps.sh index 1a9581af30..85a91c2675 100755 --- a/t/t5326-multi-pack-bitmaps.sh +++ b/t/t5326-multi-pack-bitmaps.sh @@ -17,16 +17,30 @@ 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_NESTING=10 \ + 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 +66,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 +81,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 +220,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 +239,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 +248,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 +269,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 +278,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 Dec 14 01:55:41 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 12675243 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 B6C90C433EF for ; Tue, 14 Dec 2021 01:55:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S244717AbhLNBzp (ORCPT ); Mon, 13 Dec 2021 20:55:45 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59272 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S244598AbhLNBzn (ORCPT ); Mon, 13 Dec 2021 20:55:43 -0500 Received: from mail-io1-xd30.google.com (mail-io1-xd30.google.com [IPv6:2607:f8b0:4864:20::d30]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 04635C061748 for ; Mon, 13 Dec 2021 17:55:43 -0800 (PST) Received: by mail-io1-xd30.google.com with SMTP id x6so21287230iol.13 for ; Mon, 13 Dec 2021 17:55:42 -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=UxKTn++M2CnEK0r4vZqINfkKGZzuhEWcsoyAegcxI0g=; b=Qfftd2gSANY/KQG5314TdBfb/oFTr6cm657D7+132YLjfMu16xofZNXBgWGZCau1Kn QoCjqqsatkoO43jxQ5wXsEbX0d1dAnxY8018RRbDT/Cs3WK11WICGtwJy6ylfpRJxSJE OqkqrhS5ujvhEFELf33/USpxMTLKHa+OjDRRpi5yLI4qLGDhR2ZivRi02a1xnQWcKDtI rCHv3QR/ua3sDl0/gdwyNA038aLl5Pf9Xrq4HmlPoAOfUvaup+Zlz89Dog0z/vkAKaM1 CimX31AUbTlACIXVuFruD3U4+QwpUXugGawrHWhGmKm1zdSGqrfwb8pwxykZDc4cbZdP KXMg== 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=UxKTn++M2CnEK0r4vZqINfkKGZzuhEWcsoyAegcxI0g=; b=5vwqOlGrpFDlrjFYXo5uk+VqghQE126Xnj1FIdbU5z0nu6TgQp2I+JZ4e1hBhT7qK8 6+VS0/6caOqIQdkEOTJp6cJR3T6/xF6SxrMnU5IFplkh1ThQ2CJIACtFqZpDxFhxdQZW /ccBl5o98hDIBlKWs9XvUtkSPDDhUGmFZbjeyHpuSsp0gMuxVZjEuDPVU7Z8YMzMJQAY 88QdGdlVqpt5GuNJRg83ZXWFLPtQ6SVH0yxow+8Jggqcy2X4j50J22VABdntjwFPYEs8 5wmmoAa1FOLlISTPfzkia0tG6C/R/BOeSXps83io/x82v8lPhqiqMOtr3Kc1D68f9WrN 3khA== X-Gm-Message-State: AOAM533z5dWHOXRdt6OUXg5yYnCjSq/ufKVMz6G9TQ2p/apPJf384Vao EwYGBHdszX+esdaHX65GsBg1Pyq7DSlmKuBy X-Google-Smtp-Source: ABdhPJyK25QHKAN3PXz9HFDsZWaYRh2q5QQg5k9DM3+szBc69ERsVZ+qMgI5EqeL5oEnbDtzx/t3iw== X-Received: by 2002:a05:6602:13d3:: with SMTP id o19mr1683307iov.4.1639446941914; Mon, 13 Dec 2021 17:55:41 -0800 (PST) Received: from localhost (104-178-186-189.lightspeed.milwwi.sbcglobal.net. [104.178.186.189]) by smtp.gmail.com with ESMTPSA id j16sm686831ilr.60.2021.12.13.17.55.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 13 Dec 2021 17:55:41 -0800 (PST) Date: Mon, 13 Dec 2021 20:55:41 -0500 From: Taylor Blau To: git@vger.kernel.org Cc: gitster@pobox.com, peff@peff.net, stolee@gmail.com Subject: [PATCH v2 6/8] t5326: move tests to t/lib-bitmap.sh Message-ID: 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 | 178 ++++++++++++++++++++++++++++++++++ t/t5326-multi-pack-bitmaps.sh | 174 +-------------------------------- 2 files changed, 180 insertions(+), 172 deletions(-) diff --git a/t/lib-bitmap.sh b/t/lib-bitmap.sh index 21d0392dda..771c41c2ea 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,178 @@ 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_NESTING=10 \ + 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 85a91c2675..100ac90d15 100755 --- a/t/t5326-multi-pack-bitmaps.sh +++ b/t/t5326-multi-pack-bitmaps.sh @@ -9,135 +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_NESTING=10 \ - 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 @@ -214,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 && @@ -399,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: 12675245 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 80B72C433FE for ; Tue, 14 Dec 2021 01:55:47 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S244281AbhLNBzq (ORCPT ); Mon, 13 Dec 2021 20:55:46 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59292 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S243991AbhLNBzp (ORCPT ); Mon, 13 Dec 2021 20:55:45 -0500 Received: from mail-io1-xd33.google.com (mail-io1-xd33.google.com [IPv6:2607:f8b0:4864:20::d33]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D9DDDC061371 for ; Mon, 13 Dec 2021 17:55:44 -0800 (PST) Received: by mail-io1-xd33.google.com with SMTP id b187so21428900iof.11 for ; Mon, 13 Dec 2021 17:55:44 -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=Ua2pp3357xioWAVxo2bJptKzW277e02pttZKmcGXvAk=; b=dj9j8GR7vM2eJSWY+xUvSqU0dhBZh+C0Pr/j3JQR0y/Jma11w2QExD34RY17PsfBlT 2Dp7tckpJho03a8BxeWkqZzWQm2aFfFoKHRtyo5QBReIl6HJReM2VNUIqnhex7GgljIY MyqsKq+xCMmN7BoEzPwt0m8DQxvNOeYkrnotyV6sb1v0s8QpE67AxxZsRqGlpQFzJqa6 uK99VrSm6MB+NYmfX5Lf/fZUfLO/7NZhohrxwBNoSWTyTSdmS0OtV4vSfuziVDx4ARxh E8/3cNspKSG5pjksIbk8twCmZQDnJG7LGLqh3OKQM2YQcCui+2bWAUmKkayELmQ1s4Ag W4NQ== 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=Ua2pp3357xioWAVxo2bJptKzW277e02pttZKmcGXvAk=; b=E3n8b0ifPsoOTgoVpKfxaEa5PH8SU7BUzB5qYp5/F7A4R6pXgHDP7z0/QtEO8yFCr7 hM0O/+bOEVFFFXkBO/YIiNVP5QbuX+hhkrYsz0mGxIkzh8tu3M2KtFaHdoN4+MvgCh9m a9qgBxtMQvQ4cUXCRm5ioWx04xexiFKcU4Lt318h60/dkfWJCnzRbjTqBVxoySlCxaVI Xrpn7D9ZDZ9nDleo5usRQKnhtwYoi0kBrEvVOqcnzW1wZGtv+lXG0Cubc3RAp26ucuoL 5omT69leCw0VgwRRoszNv+NNUOPy504/5c6Hyi9/QD6+OWwaSYrRr8y3dKzwjQIWzfqk ZOkg== X-Gm-Message-State: AOAM532HduOGB+xQedq2BMl/4H/p+gZpjTzWfKg3IXOLtefkcVjKTtT8 TL1C/plCKuQeiKRnt+DjH7Suvd8Kd/iGuP8l X-Google-Smtp-Source: ABdhPJyjiksAF5oxcC2XpG2FhR0LZ7P7X5fWvycXrO4MKpIQiekWu2gf2D208o6J8NJpLXZCFYFbvQ== X-Received: by 2002:a02:6901:: with SMTP id e1mr1239810jac.0.1639446944192; Mon, 13 Dec 2021 17:55: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 8926c6da1cb9f-2faa6b4da28sm185444173.52.2021.12.13.17.55.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 13 Dec 2021 17:55:43 -0800 (PST) Date: Mon, 13 Dec 2021 20:55:43 -0500 From: Taylor Blau To: git@vger.kernel.org Cc: gitster@pobox.com, peff@peff.net, stolee@gmail.com Subject: [PATCH v2 7/8] t/lib-bitmap.sh: parameterize tests over reverse index source Message-ID: 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 771c41c2ea..0a35daf939 100644 --- a/t/lib-bitmap.sh +++ b/t/lib-bitmap.sh @@ -275,18 +275,24 @@ midx_pack_source () { test_rev_exists () { commit="$1" + kind="$2" test_expect_success 'reverse index exists' ' GIT_TRACE2_EVENT_NESTING=10 \ 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' ' @@ -296,7 +302,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 @@ -326,7 +332,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 @@ -429,6 +435,8 @@ midx_bitmap_core () { } midx_bitmap_partial_tests () { + rev_kind="${1:-rev}" + test_expect_success 'setup partial bitmaps' ' test_commit packed && git repack && @@ -438,7 +446,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 Dec 14 01:55:45 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 12675247 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 262C8C433EF for ; Tue, 14 Dec 2021 01:55:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S244290AbhLNBzs (ORCPT ); Mon, 13 Dec 2021 20:55:48 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59314 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S243560AbhLNBzs (ORCPT ); Mon, 13 Dec 2021 20:55:48 -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 C7603C061574 for ; Mon, 13 Dec 2021 17:55:47 -0800 (PST) Received: by mail-qk1-x736.google.com with SMTP id d2so15582687qki.12 for ; Mon, 13 Dec 2021 17:55:47 -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=4/3GqYZimI8Q7oV2z7favpU5VYXh8rajLCSGRRI7MHM=; b=ZsAFfr9/5eq+1WbjABg0w60Xi3HFEAxWdq1G3HWZ3Ch3zuCQxkVcjfWXTCgUOivK/L IKu068b2N6zJ3/EvmMlE16ZKyHzT+VKBn8p3chxPKf+CPZeLllBXxptme9lKH/iWAmz1 kXdZHBsrS7ARFtJSS7foSlsrs+pOPB0KvOH8EeMzCTLOVo6DaEHVEK+cUYwWMpasttJl Dvk1+pggB9+KJ7G6XQnzXsRQfi7SKPggDK19docVkq+4fBRIaMAW6k1woMKnvc5m5AOa 2Pe4fD9Eahd4g7JeXI5v9BRNONWXM0Cq7QIhwHkwVOG32/3NbKjbdia+y5MuCbAY8hqQ eQ9w== 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=4/3GqYZimI8Q7oV2z7favpU5VYXh8rajLCSGRRI7MHM=; b=KOcMsn7ck2Brj/4pgvz3TeMJyu19ETmcNY5DESAaPADwwEl961DO0vm6XoBIauyCvQ puttQJkHw3SRAOFOeBiqn7zpgKBX6yodkY4oTyDXX6VXUJonT94x032pJ6My3AdmTe4z cVQC0S7eq4icSjOdy681Fz+TcxM59eEmTQ5lj/SdeYhbeB/xkAIMZqkdRsPpC0trOeYz SW24fDTuckDZVOqDmoyEs2Ga+DGEOWa2aVMs8ceeAtS2KgoBLF7hVKSUD0C+809gjPUQ Vg84h4NM/7B+K+Zy6UzZwdY7DbZicRr8h75dYiV9xFbbXprcKNwHhePzPn0tKYNPG/xA K7fQ== X-Gm-Message-State: AOAM5314BSyQB2jvkQViUYuYom7qTAMQfEOzqIf6JylDQLIUT64iDhQf CLDFKu1zU3ZDXHt5BHH3bVqSNTRwSIa8mTVA X-Google-Smtp-Source: ABdhPJxXpxmd1lLh0PcKvzl9Ft+9pz4j5iWru8TAXHIioeCoMrOoWhLIwQHhsiKFng1FfZe27+lszQ== X-Received: by 2002:a05:620a:2413:: with SMTP id d19mr1795143qkn.82.1639446946722; Mon, 13 Dec 2021 17:55:46 -0800 (PST) Received: from localhost (104-178-186-189.lightspeed.milwwi.sbcglobal.net. [104.178.186.189]) by smtp.gmail.com with ESMTPSA id s6sm7084651qko.43.2021.12.13.17.55.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 13 Dec 2021 17:55:46 -0800 (PST) Date: Mon, 13 Dec 2021 20:55:45 -0500 From: Taylor Blau To: git@vger.kernel.org Cc: gitster@pobox.com, peff@peff.net, stolee@gmail.com Subject: [PATCH v2 8/8] midx: read `RIDX` chunk when present Message-ID: <993bfa8dd8480e74d64f657539b0c518ad155e5c.1639446906.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 | 5 +++++ t/t5327-multi-pack-bitmaps-rev.sh | 23 +++++++++++++++++++++++ t/t7700-repack.sh | 4 ---- 7 files changed, 53 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 0a35daf939..f5eaa9cf68 100644 --- a/t/lib-bitmap.sh +++ b/t/lib-bitmap.sh @@ -291,7 +291,7 @@ test_rev_exists () { } midx_bitmap_core () { - rev_kind="${1:-rev}" + rev_kind="${1:-midx}" setup_bitmap_history @@ -435,7 +435,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..8c92acb0ce 100755 --- a/t/t5326-multi-pack-bitmaps.sh +++ b/t/t5326-multi-pack-bitmaps.sh @@ -9,6 +9,11 @@ test_description='exercise basic multi-pack bitmap functionality' GIT_TEST_MULTI_PACK_INDEX=0 GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP=0 +GIT_TEST_MIDX_WRITE_REV=0 +GIT_TEST_MIDX_READ_RIDX=1 +export GIT_TEST_MIDX_WRITE_REV +export 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 0260ad6f0e..3ee56cefd3 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