From fb20d4b1268d97646ae24f07661892cf6da64c31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Mon, 21 Jun 2021 17:03:37 +0200 Subject: pack-objects tests: cover blindspots in stdin handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cover blindspots in the testing of stdin handling, including the "!len" condition added in b5d97e6b0a0 (pack-objects: run rev-list equivalent internally., 2006-09-04). The codepath taken with --revs and read_object_list_from_stdin() acts differently in some of these common cases, let's test for those. The "--stdin --revs" test being added here stresses the combination of --stdin-packs and the revision.c --stdin argument, some of this was covered in a test added in 339bce27f4f (builtin/pack-objects.c: add '--stdin-packs' option, 2021-02-22), but let's make sure that GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS=true keeps erroring out about --stdin, and it isn't picked up by the revision.c API's handling of that option. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- t/t5300-pack-object.sh | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) (limited to 't/t5300-pack-object.sh') diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh index 5c5e53f0be..65e991e370 100755 --- a/t/t5300-pack-object.sh +++ b/t/t5300-pack-object.sh @@ -34,6 +34,91 @@ test_expect_success 'setup' ' } >expect ' +test_expect_success 'setup pack-object in <<-EOF && + $(git -C pack-object-stdin rev-parse one) + EOF + + git -C pack-object-stdin pack-objects basic-stdin actual && + test_line_count = 1 actual && + + git -C pack-object-stdin pack-objects --revs basic-stdin-revs actual && + test_line_count = 3 actual +' + +test_expect_success 'pack-object in <<-EOF && + $(git -C pack-object-stdin rev-parse one) + garbage + $(git -C pack-object-stdin rev-parse two) + EOF + + sed "s/^> //g" >err.expect <<-EOF && + fatal: expected object ID, got garbage: + > garbage + + EOF + test_must_fail git -C pack-object-stdin pack-objects bad-line-stdin err.actual && + test_cmp err.expect err.actual && + + cat >err.expect <<-EOF && + fatal: bad revision '"'"'garbage'"'"' + EOF + test_must_fail git -C pack-object-stdin pack-objects --revs bad-line-stdin-revs err.actual && + test_cmp err.expect err.actual +' + +test_expect_success 'pack-object in <<-EOF && + $(git -C pack-object-stdin rev-parse one) + + $(git -C pack-object-stdin rev-parse two) + EOF + + sed -e "s/^> //g" -e "s/Z$//g" >err.expect <<-EOF && + fatal: expected object ID, got garbage: + > Z + + EOF + test_must_fail git -C pack-object-stdin pack-objects empty-line-stdin err.actual && + test_cmp err.expect err.actual && + + git -C pack-object-stdin pack-objects --revs empty-line-stdin-revs actual && + test_line_count = 3 actual +' + +test_expect_success 'pack-object in <<-EOF && + $(git -C pack-object-stdin rev-parse one) + $(git -C pack-object-stdin rev-parse two) + EOF + + # There is the "--stdin-packs is incompatible with --revs" + # test below, but we should make sure that the revision.c + # --stdin is not picked up + cat >err.expect <<-EOF && + fatal: disallowed abbreviated or ambiguous option '"'"'stdin'"'"' + EOF + test_must_fail git -C pack-object-stdin pack-objects stdin-with-stdin-option --stdin err.actual && + test_cmp err.expect err.actual && + + test_must_fail git -C pack-object-stdin pack-objects --stdin --revs stdin-with-stdin-option-revs 2>err.actual # e.g.: check_deltas stderr -gt 0 check_deltas() { -- cgit v1.2.3 From 561fa03529202a36e0d77548fdcb5d81422c1865 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Fri, 9 Jul 2021 12:13:48 +0200 Subject: pack-objects: fix segfault in --stdin-packs option MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix a segfault in the --stdin-packs option added in 339bce27f4f (builtin/pack-objects.c: add '--stdin-packs' option, 2021-02-22). The read_packs_list_from_stdin() function didn't check that the lines it was reading were valid packs, and thus when doing the QSORT() with pack_mtime_cmp() we'd have a NULL "util" field. The "util" field is used to associate the names of included/excluded packs with the packed_git structs they correspond to. The logic error was in assuming that we could iterate all packs and annotate the excluded and included packs we got, as opposed to checking the lines we got on stdin. There was a check for excluded packs, but included packs were simply assumed to be valid. As noted in the test we'll not report the first bad line, but whatever line sorted first according to the string-list.c API. In this case I think that's fine. There was further discussion of alternate approaches in [1]. Even though we're being lazy let's assert the line we do expect to get in the test, since whoever changes this code in the future might miss this case, and would want to update the test and comments. 1. http://lore.kernel.org/git/YND3h2l10PlnSNGJ@nand.local Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- t/t5300-pack-object.sh | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) (limited to 't/t5300-pack-object.sh') diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh index 65e991e370..e13a884207 100755 --- a/t/t5300-pack-object.sh +++ b/t/t5300-pack-object.sh @@ -119,6 +119,25 @@ test_expect_success 'pack-object in <<-EOF && + $(git -C pack-object-stdin rev-parse one) + $(git -C pack-object-stdin rev-parse two) + EOF + + # That we get "two" and not "one" has to do with OID + # ordering. It happens to be the same here under SHA-1 and + # SHA-256. See commentary in pack-objects.c + cat >err.expect <<-EOF && + fatal: could not find pack '"'"'$(git -C pack-object-stdin rev-parse two)'"'"' + EOF + test_must_fail git \ + -C pack-object-stdin \ + pack-objects stdin-with-stdin-option --stdin-packs \ + err.actual && + test_cmp err.expect err.actual +' + # usage: check_deltas # e.g.: check_deltas stderr -gt 0 check_deltas() { -- cgit v1.2.3