From patchwork Fri Mar 5 15:21:37 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 12118663 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id BA045C433E0 for ; Fri, 5 Mar 2021 15:22:42 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 7835E65090 for ; Fri, 5 Mar 2021 15:22:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229669AbhCEPWL (ORCPT ); Fri, 5 Mar 2021 10:22:11 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45604 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229982AbhCEPVl (ORCPT ); Fri, 5 Mar 2021 10:21:41 -0500 Received: from mail-qt1-x830.google.com (mail-qt1-x830.google.com [IPv6:2607:f8b0:4864:20::830]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id F2328C061574 for ; Fri, 5 Mar 2021 07:21:40 -0800 (PST) Received: by mail-qt1-x830.google.com with SMTP id w6so1955569qti.6 for ; Fri, 05 Mar 2021 07:21:40 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ttaylorr-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=yxu++oTaPW9TpZXN2sKeiBmTS3Ujf/7mqu1jVuZQlSo=; b=iOmrpg32W0QsiIQZhzw7QqBO2MFjpkRL4xg7GtXwzG5uVEiBfFGQGe1PIn9EA635wg di0AnTAwWZhTEboijG4W4GR4NZ2IfZquYj13sdmn40DF7AisRWiK1LfcqWa2UgLLEhdL DKADxFonf/mYwVIjJn80Lt/naBEbsUWKDY0SVx2j88e9X4CH/GqOxnerZDZLiVUqgafH EunddHYWKXu2c9gUWxwwVwkgDwGac6XDXWLUidvgVJI5NHQF6om/aB0ny2O293XDi3GR RCLbNwCMy/Hed5RYLoHDjSB5zXk2bHj2Lksn7u3KT96MpCj+R3BEWF9rJjaPR9LQf/n5 SPEg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=yxu++oTaPW9TpZXN2sKeiBmTS3Ujf/7mqu1jVuZQlSo=; b=Jl04NuBde6RVA1/T1ikFeTecX1Yhw1ZKm/D5aB3Ga2D0cPxw+Zpe5AkbKlwpiyMzgH cLQuQlaACnG1cTK+GN3kQT0lHq9rDo33iWHyYO84wUwPbe922b5aFxdQ16m7uPJ4oanL CMmBuwjlsTMQyCSRHa1xRPGXwH5NY1BOCcZoJwns0ctC0inaottTnEauaK6BbEqbk0pX Mh/IOl2YrWOGG5rjUaozO4t6KR5ViUk25MokgdmByfBZ9U6xbqmINvn3hUnsYI5Ct5yD 3K63I7dZHgdh7ax8paD6ZrTx6F80sBeyfmKRmOulFyIjjoN0Dc7mcPFDnwWJb8Oedi9Z PBMw== X-Gm-Message-State: AOAM533xHuqPiPYTPV+Oohxs7er0svZelmugX4wCyOCeF2Cubqj6jxc1 +LUkB8p6CNQdvxhm23KEYTWo0iD/gfiRYSia X-Google-Smtp-Source: ABdhPJwbcPAr4TS88wr3pqTPP4DorznsIhSl/53SfpVEJG29Ta0Sz5IDoUPmCZpHJuvkOcoOxseHLg== X-Received: by 2002:a05:622a:102:: with SMTP id u2mr9170398qtw.37.1614957699903; Fri, 05 Mar 2021 07:21:39 -0800 (PST) Received: from localhost ([2605:9480:22e:ff10:4ce8:219:f731:dbf5]) by smtp.gmail.com with ESMTPSA id 46sm2063401qte.7.2021.03.05.07.21.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 05 Mar 2021 07:21:39 -0800 (PST) Date: Fri, 5 Mar 2021 10:21:37 -0500 From: Taylor Blau To: git@vger.kernel.org Cc: gitster@pobox.com, peff@peff.net Subject: [PATCH 1/5] builtin/repack.c: do not repack single packs with --geometric Message-ID: <80bc7fa8397491d015b80a39168813d2019e262d.1614957681.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 0fabafd0b9 (builtin/repack.c: add '--geometric' option, 2021-02-22), the 'git repack --geometric' code aborts early when there is zero or one pack. When there are no packs, this code does the right thing by placing the split at "0". But when there is exactly one pack, the split is placed at "1", which means that "git repack --geometric" (with any factor) repacks all of the objects in a single pack. This is wasteful, and the remaining code in split_pack_geometry() does the right thing (not repacking the objects in a single pack) even when only one pack is present. Loosen the guard to only stop when there aren't any packs, and let the rest of the code do the right thing. Add a test to ensure that this is the case. Noticed-by: Junio C Hamano Signed-off-by: Taylor Blau --- builtin/repack.c | 2 +- t/t7703-repack-geometric.sh | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/builtin/repack.c b/builtin/repack.c index bcf280b10d..4ca2f647b4 100644 --- a/builtin/repack.c +++ b/builtin/repack.c @@ -351,7 +351,7 @@ static void split_pack_geometry(struct pack_geometry *geometry, int factor) uint32_t split; off_t total_size = 0; - if (geometry->pack_nr <= 1) { + if (!geometry->pack_nr) { geometry->split = geometry->pack_nr; return; } diff --git a/t/t7703-repack-geometric.sh b/t/t7703-repack-geometric.sh index 96917fc163..4a1952a054 100755 --- a/t/t7703-repack-geometric.sh +++ b/t/t7703-repack-geometric.sh @@ -20,6 +20,21 @@ test_expect_success '--geometric with no packs' ' ) ' +test_expect_success '--geometric with one pack' ' + git init geometric && + test_when_finished "rm -fr geometric" && + ( + cd geometric && + + test_commit "base" && + git repack -d && + + git repack --geometric 2 >out && + + test_i18ngrep "Nothing new to pack" out + ) +' + test_expect_success '--geometric with an intact progression' ' git init geometric && test_when_finished "rm -fr geometric" && From patchwork Fri Mar 5 15:21:43 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 12118665 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D2161C433DB for ; Fri, 5 Mar 2021 15:22:42 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 9D5E365093 for ; Fri, 5 Mar 2021 15:22:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229982AbhCEPWL (ORCPT ); Fri, 5 Mar 2021 10:22:11 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45628 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230496AbhCEPVq (ORCPT ); Fri, 5 Mar 2021 10:21:46 -0500 Received: from mail-qk1-x735.google.com (mail-qk1-x735.google.com [IPv6:2607:f8b0:4864:20::735]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3E490C061574 for ; Fri, 5 Mar 2021 07:21:46 -0800 (PST) Received: by mail-qk1-x735.google.com with SMTP id z190so2259253qka.9 for ; Fri, 05 Mar 2021 07:21:46 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ttaylorr-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=NqZE+1NU2pbwR6Xfi6vWCpphssMaMePk2IJI5dO+HRg=; b=pm4o4UuopSUzPnxx97/mpEj9F6tT4gqBgYveobiGuQBdUFl566kzTNUs+MRnS9vOev qr5CwfJaU5EOfXj8RgfyZcSagyTo7ORkuWtUB2bq6c/0pjcfQr/E4bbde8sx3OioxbJp DH8HIqREUa0pV9waorRklH9A2kQ+ryyVsufFeLxgDXQuuRqYRKNgaWXUcPIQbIsF8Jds aYJp115PEsHwu6yo1HrJl9Sz1Ei+rs0pAF3HqH5DC8bpGSGZBYswjST8X/rjsRNpsaJK 0fBlpCYV+Ziap+gyqpnpKUaIHaAPo5fEjiMfhqUc9W4NirPkNX/g6r3W84c3gC1tzKBD b8ZA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=NqZE+1NU2pbwR6Xfi6vWCpphssMaMePk2IJI5dO+HRg=; b=pg2mcm0SjzqT+AGK3hMzFCxkTKUEC4ESGWmouVl5mGk/+ValYg9IuF+homfPOY1AdM D2kqGPyYO1u/6RV6JmOLGqXrik3d5Vbl35YEvidbhiiaY+QDH6UQGGyZh54GobTfzb8J /GyKukryZUwkIMMshlk5i95jEMZgw91xrMDd2CViFZjvNwW6jC7xyOdOGamu45Cv8Xem G6k5F3kXNrIpSNpbwsUzv0DVw42d1KReoelxDTbhhSe7/fQoBgEXdvx98m5XpV/mHGD4 +yk2xQfc42tRSsOeqnJb3Qr8rzstohfJBZVKsDw4jmmLBRLB3dYkW55JjclUL2zx76q5 b/MQ== X-Gm-Message-State: AOAM5324JFiGOYHzQhDrY3kLOT8HEpLEPodZlh0fUjYTU3hFDXgqcu04 h8PmaZyjY28AlujnBq2En0YVNMQyF4JGcq6y X-Google-Smtp-Source: ABdhPJxKxKIidGiHQXuNSuTZU5HeNQwfQxca8VRqBiKb4mKi88VzX7oLclwp4jRUgY6T+fRLUiGRhQ== X-Received: by 2002:a05:620a:31a:: with SMTP id s26mr9504626qkm.355.1614957705229; Fri, 05 Mar 2021 07:21:45 -0800 (PST) Received: from localhost ([2605:9480:22e:ff10:4ce8:219:f731:dbf5]) by smtp.gmail.com with ESMTPSA id g2sm1868516qkd.124.2021.03.05.07.21.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 05 Mar 2021 07:21:44 -0800 (PST) Date: Fri, 5 Mar 2021 10:21:43 -0500 From: Taylor Blau To: git@vger.kernel.org Cc: gitster@pobox.com, peff@peff.net Subject: [PATCH 2/5] t7703: test --geometric repack with loose objects Message-ID: References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org We don't currently have a test that demonstrates the non-idempotent behavior of 'git repack --geometric' with loose objects, so add one here to make sure we don't regress in this area. Signed-off-by: Taylor Blau --- t/t7703-repack-geometric.sh | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/t/t7703-repack-geometric.sh b/t/t7703-repack-geometric.sh index 4a1952a054..5ccaa440e0 100755 --- a/t/t7703-repack-geometric.sh +++ b/t/t7703-repack-geometric.sh @@ -54,6 +54,37 @@ test_expect_success '--geometric with an intact progression' ' ) ' +test_expect_success '--geometric with loose objects' ' + git init geometric && + test_when_finished "rm -fr geometric" && + ( + cd geometric && + + # These packs already form a geometric progression. + test_commit_bulk --start=1 1 && # 3 objects + test_commit_bulk --start=2 2 && # 6 objects + # The loose objects are packed together, breaking the + # progression. + test_commit loose && # 3 objects + + find $objdir/pack -name "*.pack" | sort >before && + git repack --geometric 2 -d && + find $objdir/pack -name "*.pack" | sort >after && + + comm -13 before after >new && + comm -23 before after >removed && + + test_line_count = 1 new && + test_must_be_empty removed && + + git repack --geometric 2 -d && + find $objdir/pack -name "*.pack" | sort >after && + + # The progression (3, 3, 6) is combined into one new pack. + test_line_count = 1 after + ) +' + test_expect_success '--geometric with small-pack rollup' ' git init geometric && test_when_finished "rm -fr geometric" && From patchwork Fri Mar 5 15:21:50 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 12118671 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0D4DEC433E6 for ; Fri, 5 Mar 2021 15:22:43 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C673A6508D for ; Fri, 5 Mar 2021 15:22:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230487AbhCEPWM (ORCPT ); Fri, 5 Mar 2021 10:22:12 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45660 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230511AbhCEPVy (ORCPT ); Fri, 5 Mar 2021 10:21:54 -0500 Received: from mail-qt1-x830.google.com (mail-qt1-x830.google.com [IPv6:2607:f8b0:4864:20::830]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D34BFC061574 for ; Fri, 5 Mar 2021 07:21:53 -0800 (PST) Received: by mail-qt1-x830.google.com with SMTP id 18so1973506qty.3 for ; Fri, 05 Mar 2021 07:21:53 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ttaylorr-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=/U3pYSgUtgqQM+PBSge337RhCEGhTiHYrdje1g3WRM0=; b=BnZLdYN7TDPt2UWrvwUphpSC44OeJNxFi7lZJ/zfuK+grHr6qMWLfVZURP7yNLRx4Y OKebTKbT8OHFk5erhcz1CFCDtsCrSxnEk6pW9XpYI8i/9mob5aD/3e5a5Bq4iGBczdpo nKn2su5sRacao4TvHaSs80cAoXM4rgcU1Zbx+FYwSVb7Asv9Xa/88b4naWBWcBwDNl1Z pBkjlWyKlEZkgUdkbt97VWt42Jx2gISgUeOpnAf+Giy28QQiOUDtImKk+KkPcAlUKRie DxA15Vtufmw8Knf54GpAMMXix7hWuX1OCQrp7CMYmplOz9GQHHH6+6Yrdj6aqngDpD5B qeVw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=/U3pYSgUtgqQM+PBSge337RhCEGhTiHYrdje1g3WRM0=; b=WOeAVv1GRRTn/3rKPXUjygiW5KX3+tIgE4jRZazxS6UuKNL4bZb7RB6GAtVtiqt8N5 48zJkVrGcpO+YXdi6Se4TJHfpzl0iQOL0WCkB8w7iZZTdNvZCjDq3fQB1opkqSVFzjm2 ICQOVE90bkx4fXsAZOBUYxx0nmI59E05oT0lwsy3em8sm5QCXtNWynbDJcLw5Y7VkNFz OhyjSv5scHqEkfO2hdQPiy+BZ17rFqEsCezkBOM8L6Pf8iHSawu8xVigzUUhRD6jCZU5 A9KM576a6MMvE7X9fsj7tE0sCSemhbJVdBTtSGLFoD/oOt/FQHR0fXaBuJinU3K5w6GR leGA== X-Gm-Message-State: AOAM532znQI1RNAtQPh6i6i0eqwgVAShn87MGEH3Lq8rKw6LYTP7dVUQ 8goeyvWpq2isQcZ7V9vnQ+qJ3QqoVL4EOIDq X-Google-Smtp-Source: ABdhPJwTe7gp+zhLaGkXDsss8+yAJK1zOD6JSxqhKJksp9XsuQ+KtqpM/U2lSIkjQrfMJ+8pb3Owsw== X-Received: by 2002:a05:622a:506:: with SMTP id l6mr9234188qtx.134.1614957712790; Fri, 05 Mar 2021 07:21:52 -0800 (PST) Received: from localhost ([2605:9480:22e:ff10:4ce8:219:f731:dbf5]) by smtp.gmail.com with ESMTPSA id g11sm1992651qkk.5.2021.03.05.07.21.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 05 Mar 2021 07:21:52 -0800 (PST) Date: Fri, 5 Mar 2021 10:21:50 -0500 From: Taylor Blau To: git@vger.kernel.org Cc: gitster@pobox.com, peff@peff.net Subject: [PATCH 3/5] builtin/repack.c: assign pack split later Message-ID: <60f13524bd602517c41554898b213161e9d603ce.1614957681.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 the where to place the split when repacking with the '--geometric' option, split_pack_geometry() assigns the "split" variable and then decrements it in a loop. It would be equivalent (and more readable) to assign the split to the loop position after exiting the loop, so do that instead. Suggested-by: Junio C Hamano Signed-off-by: Taylor Blau --- builtin/repack.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/builtin/repack.c b/builtin/repack.c index 4ca2f647b4..21a5778e73 100644 --- a/builtin/repack.c +++ b/builtin/repack.c @@ -356,8 +356,6 @@ static void split_pack_geometry(struct pack_geometry *geometry, int factor) return; } - split = geometry->pack_nr - 1; - /* * First, count the number of packs (in descending order of size) which * already form a geometric progression. @@ -365,12 +363,12 @@ static void split_pack_geometry(struct pack_geometry *geometry, int factor) for (i = geometry->pack_nr - 1; i > 0; i--) { struct packed_git *ours = geometry->pack[i]; struct packed_git *prev = geometry->pack[i - 1]; - if (geometry_pack_weight(ours) >= factor * geometry_pack_weight(prev)) - split--; - else + if (geometry_pack_weight(ours) < factor * geometry_pack_weight(prev)) break; } + split = i; + if (split) { /* * Move the split one to the right, since the top element in the From patchwork Fri Mar 5 15:21:56 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 12118667 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 21577C43381 for ; Fri, 5 Mar 2021 15:22:43 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id E7A4565090 for ; Fri, 5 Mar 2021 15:22:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230511AbhCEPWN (ORCPT ); Fri, 5 Mar 2021 10:22:13 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45682 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230519AbhCEPV7 (ORCPT ); Fri, 5 Mar 2021 10:21:59 -0500 Received: from mail-qk1-x72b.google.com (mail-qk1-x72b.google.com [IPv6:2607:f8b0:4864:20::72b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 51E29C061574 for ; Fri, 5 Mar 2021 07:21:59 -0800 (PST) Received: by mail-qk1-x72b.google.com with SMTP id g185so2281678qkf.6 for ; Fri, 05 Mar 2021 07:21:59 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ttaylorr-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=s7hlQUvDHq4CPNNuQV1QFKvrPQVVOTVCw1T1CbYiepM=; b=rbkVmHdWDrh3R1B1Ic/sYumz+SKu3OoNdQn513Tf33Zdrr5NvCPHTl/rzYMZgJbMP3 Pl12VZoyfS9T9hTEj4KvwQipOrF6wpOC1cexmey/Yjw0ojqyYE+EA4y9Yvwtu4dsFqcC bpCjj4YDJ+WoyBfjw/GPKmPTE9uUqlu5/nHB8kTgFCjJmVFrlO/g2BrNgwtzgQChpN1c 2WLr8u8M0KEyw81L+AwW+A34qyy0/tEnArN3a2WM84ub4cxmf5rmmX+Wo7zxTGYgw+dJ UEdKkQpHsGvwgkRPXSvPbtq1W6fMqz59QL1CCJkAO3Uu/Dm9LJ0gU/hey4uT+5Riuamt Hrug== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=s7hlQUvDHq4CPNNuQV1QFKvrPQVVOTVCw1T1CbYiepM=; b=BfCQMO5+cSeJ5v/L5QTKVSCqAXHhF06D/LfqYUc4YiCGSLtSXx/lnCchQesNDMak/S j4bOTKivflYB+ArvGmQxz5rLwzsoIWczMKBWqnvmrBNDwRz7thwMSl6UHzBfUUc3tmrj ujqo88cfAOzGbCDHmE02yqNVs3COXwDL4GQ7hGQBqxDox8Ml5fnNXxfRqqfhq9uFqSRs IY/b1KAOF5SVmXeDt+yoXfGO8QdmOsI5mtsW3KPNbZs7iQyLAn5/NIXko024Ge+E2hMy M1g2Y6DEvIgBPpIu6dXHc0mj4XwM/dF/up2+Bk2STWuTrUeyHYgAfx1gBFVCXs2hxirR gWmw== X-Gm-Message-State: AOAM531/bUfvbhkqqfUtE82RCccNACZBRlZBlySoESAsrfM729kCFbJf aOCicBW7gko7zse+Mq/WEnapp7DZNlTsFHxc X-Google-Smtp-Source: ABdhPJzzvQRoWTiZ0PV2lnN330qYXEe8fQBVH2PYNmZOq8NIq+e9hb0ixwWX+eB3hqt+r7puF9z+1A== X-Received: by 2002:a05:620a:1133:: with SMTP id p19mr9929600qkk.340.1614957718090; Fri, 05 Mar 2021 07:21:58 -0800 (PST) Received: from localhost ([2605:9480:22e:ff10:4ce8:219:f731:dbf5]) by smtp.gmail.com with ESMTPSA id b63sm1925850qkd.84.2021.03.05.07.21.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 05 Mar 2021 07:21:57 -0800 (PST) Date: Fri, 5 Mar 2021 10:21:56 -0500 From: Taylor Blau To: git@vger.kernel.org Cc: gitster@pobox.com, peff@peff.net Subject: [PATCH 4/5] builtin/repack.c: be more conservative with unsigned overflows Message-ID: References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org There are a number of places in the geometric repack code where we multiply the number of objects in a pack by another unsigned value. We trust that the number of objects in a pack is always representable by a uint32_t, but we don't necessarily trust that that number can be multiplied without overflow. Sprinkle some unsigned_add_overflows() and unsigned_mult_overflows() in split_pack_geometry() to check that we never overflow any unsigned types when adding or multiplying them. Arguably these checks are a little too conservative, and certainly they do not help the readability of this function. But they are serving a useful purpose, so I think they are worthwhile overall. Suggested-by: Junio C Hamano Signed-off-by: Taylor Blau --- builtin/repack.c | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/builtin/repack.c b/builtin/repack.c index 21a5778e73..677c6b75c1 100644 --- a/builtin/repack.c +++ b/builtin/repack.c @@ -363,6 +363,12 @@ static void split_pack_geometry(struct pack_geometry *geometry, int factor) for (i = geometry->pack_nr - 1; i > 0; i--) { struct packed_git *ours = geometry->pack[i]; struct packed_git *prev = geometry->pack[i - 1]; + + if (unsigned_mult_overflows(factor, geometry_pack_weight(prev))) + die(_("pack %s too large to consider in geometric " + "progression"), + prev->pack_name); + if (geometry_pack_weight(ours) < factor * geometry_pack_weight(prev)) break; } @@ -388,11 +394,25 @@ static void split_pack_geometry(struct pack_geometry *geometry, int factor) * packs in the heavy half need to be joined into it (if any) to restore * the geometric progression. */ - for (i = 0; i < split; i++) - total_size += geometry_pack_weight(geometry->pack[i]); + for (i = 0; i < split; i++) { + struct packed_git *p = geometry->pack[i]; + + if (unsigned_add_overflows(total_size, geometry_pack_weight(p))) + die(_("pack %s too large to roll up"), p->pack_name); + total_size += geometry_pack_weight(p); + } for (i = split; i < geometry->pack_nr; i++) { struct packed_git *ours = geometry->pack[i]; + + if (unsigned_mult_overflows(factor, total_size)) + die(_("pack %s too large to roll up"), ours->pack_name); + if (geometry_pack_weight(ours) < factor * total_size) { + if (unsigned_add_overflows(total_size, + geometry_pack_weight(ours))) + die(_("pack %s too large to roll up"), + ours->pack_name); + split++; total_size += geometry_pack_weight(ours); } else From patchwork Fri Mar 5 15:22:02 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 12118669 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 81347C433E9 for ; Fri, 5 Mar 2021 15:22:43 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 3B45465015 for ; Fri, 5 Mar 2021 15:22:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230518AbhCEPWO (ORCPT ); Fri, 5 Mar 2021 10:22:14 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45706 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229793AbhCEPWG (ORCPT ); Fri, 5 Mar 2021 10:22:06 -0500 Received: from mail-qv1-xf29.google.com (mail-qv1-xf29.google.com [IPv6:2607:f8b0:4864:20::f29]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B8B1DC061574 for ; Fri, 5 Mar 2021 07:22:05 -0800 (PST) Received: by mail-qv1-xf29.google.com with SMTP id r5so1128762qvv.9 for ; Fri, 05 Mar 2021 07:22:05 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ttaylorr-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=88MkexIJZ7VpGMDZp9DCeqY4uEnqyzGiUxC7bKMLQHY=; b=zwYjybWiVqMyBtqy3JcY0pSmIOawaLQQYa27wdbMCl4BG5rWorSa6+653U3pvqi3H+ sId3Dyf0mWhcD8RD1FZS4arfeQphO6Ynb8FGaInYUu4/gSn171ReqXbQx+cbmSwab2SK MjVyzgsnCX3W2ScrhtJfo29VFu/g2bYkZB6bHbXXXqMUglglTuC3TtitI+XQzdMAs4aX PqwsDHQMhh9Wgim6pExcxdBsqt5wsVbnqwRntqKPL8i4dzOyPjCon3EY+OdUCd8FRZLS +2nBV/Nmh/iBis8wwtJ3YkZG0+jAVTPSV9rHg6okUMkI1fBC9eGTvuEHU/ZJu1i7tsLr Msfw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=88MkexIJZ7VpGMDZp9DCeqY4uEnqyzGiUxC7bKMLQHY=; b=lfwmg22XmR2UCp1tJQnnHghBu2JdiHc+qkbk2uNFm86YUv8Nsm0Qcj8epFFPw57KeG pJE9aO65Y97z4APUphBr2aVlcKPAkNOVHNDVNJKu1snDtuspddbw1wNWzRcDBCiNPoGo HEvjtUpwK6Xv94Ik3005fk1ZEjHpHNzwQQAy2nWEr/6oPdA94YVxt0+VlBPbsakenmjk 1WnQxv9ym0En7ljlZzcuTisXuWKA8Lr3S9GDFij5XHKB8RvAeSkmYSuTlf/k32w0z9R0 d15/eB8mn8rYvi82ugfPZePVqEhENB+4oOjctoiT/7o3Gw3ZbRUtXLj6gKNiXR98kKmD nF8g== X-Gm-Message-State: AOAM531eIWrgu8fdfty7jWHH7uv401ZYeFgULb5hkJk2ivvsWcdkoq+G qHkevYlMX3gl8+nNJGQoWcjMHpOF4FjOwbx4 X-Google-Smtp-Source: ABdhPJyGQSGnco0fX3iHlWJG4Ae3xM0BCIAO62rqCEvvqc5W9G/V9/gC7Z6QREWRw125I1zFmoNF8A== X-Received: by 2002:a05:6214:b27:: with SMTP id w7mr9457981qvj.34.1614957724696; Fri, 05 Mar 2021 07:22:04 -0800 (PST) Received: from localhost ([2605:9480:22e:ff10:4ce8:219:f731:dbf5]) by smtp.gmail.com with ESMTPSA id e15sm2104410qtp.58.2021.03.05.07.22.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 05 Mar 2021 07:22:04 -0800 (PST) Date: Fri, 5 Mar 2021 10:22:02 -0500 From: Taylor Blau To: git@vger.kernel.org Cc: gitster@pobox.com, peff@peff.net Subject: [PATCH 5/5] builtin/repack.c: reword comment around pack-objects flags Message-ID: References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Junio C Hamano The comment in this block is meant to indicate that passing '--all', '--reflog', and so on aren't necessary when repacking with the '--geometric' option. But, it has two problems: first, it is factually incorrect ('--all' is *not* incompatible with '--stdin-packs' as the comment suggests); second, it is quite focused on the geometric case for a block that is guarding against it. Reword this comment to address both issues. Signed-off-by: Junio C Hamano Signed-off-by: Taylor Blau --- builtin/repack.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/builtin/repack.c b/builtin/repack.c index 677c6b75c1..6ce2556c9e 100644 --- a/builtin/repack.c +++ b/builtin/repack.c @@ -545,12 +545,14 @@ int cmd_repack(int argc, const char **argv, const char *prefix) strvec_push(&cmd.args, "--non-empty"); if (!geometry) { /* - * 'git pack-objects' will up all objects loose or packed - * (either rolling them up or leaving them alone), so don't pass - * these options. + * We need to grab all reachable objects, including those that + * are reachable from reflogs and the index. * - * The implementation of 'git pack-objects --stdin-packs' - * makes them redundant (and the two are incompatible). + * When repacking into a geometric progression of packs, + * however, we ask 'git pack-objects --stdin-packs', and it is + * not about packing objects based on reachability but about + * repacking all the objects in specified packs and loose ones + * (indeed, --stdin-packs is incompatible with these options). */ strvec_push(&cmd.args, "--all"); strvec_push(&cmd.args, "--reflog");