[PATCH v2] index-pack: clarify the breached limit

From: "Matt Cooper via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: gitster@pobox.com, me@ttaylorr.com, derrickstolee@github.com,
	Matt Cooper <vtbassmatt@gmail.com>,
	Matt Cooper <vtbassmatt@gmail.com>
Subject: [PATCH v2] index-pack: clarify the breached limit
Date: Thu, 24 Feb 2022 00:07:20 +0000	[thread overview]
Message-ID: <pull.1158.v2.git.1645661240356.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1158.git.1645632193.gitgitgadget@gmail.com>

From: Matt Cooper <vtbassmatt@gmail.com>

As a small courtesy to users, report what limit was breached. This
is especially useful when a push exceeds a server-defined limit, since
the user is unlikely to have configured the limit (their host did).
Also demonstrate the human-readable message in a test.

Helped-by: Taylor Blau <me@ttaylorr.com>
Helped-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Matt Cooper <vtbassmatt@gmail.com>
---
    Specify the actual pack size limit which is breached
    
    Git allows configuring a maximum pack size. GitHub (like presumably
    other Git hosts) configures this setting to a generous but finite limit.
    When a user attempts to push an oversized pack, their connection is
    terminated with a message that they've exceeded the limit. The user has
    to find the limit value elsewhere, probably in the host's documentation.
    This change adds a small convenience -- specifying the limit itself in
    the error message -- so that users no longer have to search elsewhere to
    discover the limit.
    
    v2 squashes the changes into one commit and corrects the commit trailer
    misordering.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1158%2Fvtbassmatt%2Fmc%2Fhumanize-limit-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1158/vtbassmatt/mc/humanize-limit-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1158

Range-diff vs v1:

 1:  a2eb3956f3e ! 1:  abf21ec109a index-pack: clarify the breached limit
     @@ Commit message
          As a small courtesy to users, report what limit was breached. This
          is especially useful when a push exceeds a server-defined limit, since
          the user is unlikely to have configured the limit (their host did).
     +    Also demonstrate the human-readable message in a test.
      
     +    Helped-by: Taylor Blau <me@ttaylorr.com>
     +    Helped-by: Derrick Stolee <derrickstolee@github.com>
          Signed-off-by: Matt Cooper <vtbassmatt@gmail.com>
      
       ## builtin/index-pack.c ##
     @@ builtin/index-pack.c: static void use(int bytes)
       }
       
       static const char *open_pack_file(const char *pack_name)
     +
     + ## t/t5302-pack-index.sh ##
     +@@ t/t5302-pack-index.sh: test_expect_success 'index-pack -v --stdin produces progress for both phases' '
     + 	test_i18ngrep "Resolving deltas" err
     + '
     + 
     ++test_expect_success 'too-large packs report the breach' '
     ++	pack=$(git pack-objects --all pack </dev/null) &&
     ++	sz="$(test_file_size pack-$pack.pack)" &&
     ++	test "$sz" -gt 20 &&
     ++	test_must_fail git index-pack --max-input-size=20 pack-$pack.pack 2>err &&
     ++	grep "maximum allowed size (20 bytes)" err
     ++'
     ++
     + test_done
 2:  43990408a10 < -:  ----------- t5302: confirm that large packs mention limit


 builtin/index-pack.c  | 8 ++++++--
 t/t5302-pack-index.sh | 8 ++++++++
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 3c2e6aee3cc..c45273de3b1 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -323,8 +323,12 @@ static void use(int bytes)
 	if (signed_add_overflows(consumed_bytes, bytes))
 		die(_("pack too large for current definition of off_t"));
 	consumed_bytes += bytes;
-	if (max_input_size && consumed_bytes > max_input_size)
-		die(_("pack exceeds maximum allowed size"));
+	if (max_input_size && consumed_bytes > max_input_size) {
+		struct strbuf size_limit = STRBUF_INIT;
+		strbuf_humanise_bytes(&size_limit, max_input_size);
+		die(_("pack exceeds maximum allowed size (%s)"),
+		    size_limit.buf);
+	}
 }
 
 static const char *open_pack_file(const char *pack_name)
diff --git a/t/t5302-pack-index.sh b/t/t5302-pack-index.sh
index 8ee67df38f6..b0095ab41d3 100755
--- a/t/t5302-pack-index.sh
+++ b/t/t5302-pack-index.sh
@@ -284,4 +284,12 @@ test_expect_success 'index-pack -v --stdin produces progress for both phases' '
 	test_i18ngrep "Resolving deltas" err
 '
 
+test_expect_success 'too-large packs report the breach' '
+	pack=$(git pack-objects --all pack </dev/null) &&
+	sz="$(test_file_size pack-$pack.pack)" &&
+	test "$sz" -gt 20 &&
+	test_must_fail git index-pack --max-input-size=20 pack-$pack.pack 2>err &&
+	grep "maximum allowed size (20 bytes)" err
+'
+
 test_done

base-commit: e6ebfd0e8cbbd10878070c8a356b5ad1b3ca464e
-- 
gitgitgadget
