git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jiang Xin <worldhello.net@gmail.com>
To: Junio C Hamano <gitster@pobox.com>,
	Git List <git@vger.kernel.org>, Jeff King <peff@peff.net>,
	"Daniel C . Klauer" <daniel.c.klauer@web.de>
Cc: Jiang Xin <zhiyou.jx@alibaba-inc.com>
Subject: [PATCH] pack-redundant: fix crash when one packfile in repo
Date: Wed, 16 Dec 2020 05:09:52 -0500	[thread overview]
Message-ID: <20201216100952.16890-1-worldhello.net@gmail.com> (raw)
In-Reply-To: <CANYiYbHOczhRoJkOofFNQ6VA3BiyOF=QjffFBDvLn43Ts8B67w@mail.gmail.com>

From: Jiang Xin <zhiyou.jx@alibaba-inc.com>

Command `git pack-redundant --all` will crash if there is only one
packfile in the repository.  This is because, if there is only one
packfile in local_packs, `cmp_local_packs` will do nothing and will
leave `pl->unique_objects` as uninitialized.

Junio will add additional option for pack-redundant for preparing
its removal.  So replace `git pack-redundant` command with variable
`$git_pack_redundant` in t5323.

Reported-by: Daniel C. Klauer <daniel.c.klauer@web.de>
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
---
 builtin/pack-redundant.c  |  6 ++++
 t/t5323-pack-redundant.sh | 65 +++++++++++++++++++++++++++++----------
 2 files changed, 54 insertions(+), 17 deletions(-)

diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
index 178e3409b7..690775fa82 100644
--- a/builtin/pack-redundant.c
+++ b/builtin/pack-redundant.c
@@ -473,6 +473,12 @@ static void cmp_local_packs(void)
 {
 	struct pack_list *subset, *pl = local_packs;
 
+	/* only one packfile */
+	if (!pl->next) {
+		llist_init(&pl->unique_objects);
+		return;
+	}
+
 	while ((subset = pl)) {
 		while ((subset = subset->next))
 			cmp_two_packs(pl, subset);
diff --git a/t/t5323-pack-redundant.sh b/t/t5323-pack-redundant.sh
index 6b4d1ca353..45436bd6e1 100755
--- a/t/t5323-pack-redundant.sh
+++ b/t/t5323-pack-redundant.sh
@@ -39,6 +39,8 @@ relationship between packs and objects is as follows:
 master_repo=master.git
 shared_repo=shared.git
 
+git_pack_redundant='git pack-redundant'
+
 # Create commits in <repo> and assign each commit's oid to shell variables
 # given in the arguments (A, B, and C). E.g.:
 #
@@ -112,19 +114,28 @@ test_expect_success 'setup master repo' '
 	create_commits_in "$master_repo" A B C D E F G H I J K L M N O P Q R
 '
 
+test_expect_success 'master: pack-redundant works with no packfile' '
+	(
+		cd "$master_repo" &&
+		cat >expect <<-EOF &&
+			fatal: Zero packs found!
+			EOF
+		test_must_fail $git_pack_redundant --all >actual 2>&1 &&
+		test_cmp expect actual
+	)
+'
+
 #############################################################################
 # Chart of packs and objects for this test case
 #
 #         | T A B C D E F G H I J K L M N O P Q R
 #     ----+--------------------------------------
 #     P1  | x x x x x x x                       x
-#     P2  |     x x x x   x x x
-#     P3  |             x     x x x x x
 #     ----+--------------------------------------
-#     ALL | x x x x x x x x x x x x x x         x
+#     ALL | x x x x x x x                       x
 #
 #############################################################################
-test_expect_success 'master: no redundant for pack 1, 2, 3' '
+test_expect_success 'master: pack-redundant works with one packfile' '
 	create_pack_in "$master_repo" P1 <<-EOF &&
 		$T
 		$A
@@ -135,6 +146,26 @@ test_expect_success 'master: no redundant for pack 1, 2, 3' '
 		$F
 		$R
 		EOF
+	(
+		cd "$master_repo" &&
+		$git_pack_redundant --all >out &&
+		test_must_be_empty out
+	)
+'
+
+#############################################################################
+# Chart of packs and objects for this test case
+#
+#         | T A B C D E F G H I J K L M N O P Q R
+#     ----+--------------------------------------
+#     P1  | x x x x x x x                       x
+#     P2  |     x x x x   x x x
+#     P3  |             x     x x x x x
+#     ----+--------------------------------------
+#     ALL | x x x x x x x x x x x x x x         x
+#
+#############################################################################
+test_expect_success 'master: no redundant for pack 1, 2, 3' '
 	create_pack_in "$master_repo" P2 <<-EOF &&
 		$B
 		$C
@@ -154,7 +185,7 @@ test_expect_success 'master: no redundant for pack 1, 2, 3' '
 		EOF
 	(
 		cd "$master_repo" &&
-		git pack-redundant --all >out &&
+		$git_pack_redundant --all >out &&
 		test_must_be_empty out
 	)
 '
@@ -192,7 +223,7 @@ test_expect_success 'master: one of pack-2/pack-3 is redundant' '
 		cat >expect <<-EOF &&
 			P3:$P3
 			EOF
-		git pack-redundant --all >out &&
+		$git_pack_redundant --all >out &&
 		format_packfiles <out >actual &&
 		test_cmp expect actual
 	)
@@ -231,7 +262,7 @@ test_expect_success 'master: pack 2, 4, and 6 are redundant' '
 			P4:$P4
 			P6:$P6
 			EOF
-		git pack-redundant --all >out &&
+		$git_pack_redundant --all >out &&
 		format_packfiles <out >actual &&
 		test_cmp expect actual
 	)
@@ -266,7 +297,7 @@ test_expect_success 'master: pack-8 (subset of pack-1) is also redundant' '
 			P6:$P6
 			P8:$P8
 			EOF
-		git pack-redundant --all >out &&
+		$git_pack_redundant --all >out &&
 		format_packfiles <out >actual &&
 		test_cmp expect actual
 	)
@@ -284,9 +315,9 @@ test_expect_success 'master: clean loose objects' '
 test_expect_success 'master: remove redundant packs and pass fsck' '
 	(
 		cd "$master_repo" &&
-		git pack-redundant --all | xargs rm &&
+		$git_pack_redundant --all | xargs rm &&
 		git fsck &&
-		git pack-redundant --all >out &&
+		$git_pack_redundant --all >out &&
 		test_must_be_empty out
 	)
 '
@@ -304,7 +335,7 @@ test_expect_success 'setup shared.git' '
 test_expect_success 'shared: all packs are redundant, but no output without --alt-odb' '
 	(
 		cd "$shared_repo" &&
-		git pack-redundant --all >out &&
+		$git_pack_redundant --all >out &&
 		test_must_be_empty out
 	)
 '
@@ -343,7 +374,7 @@ test_expect_success 'shared: show redundant packs in stderr for verbose mode' '
 			P5:$P5
 			P7:$P7
 			EOF
-		git pack-redundant --all --verbose >out 2>out.err &&
+		$git_pack_redundant --all --verbose >out 2>out.err &&
 		test_must_be_empty out &&
 		grep "pack$" out.err | format_packfiles >actual &&
 		test_cmp expect actual
@@ -356,9 +387,9 @@ test_expect_success 'shared: remove redundant packs, no packs left' '
 		cat >expect <<-EOF &&
 			fatal: Zero packs found!
 			EOF
-		git pack-redundant --all --alt-odb | xargs rm &&
+		$git_pack_redundant --all --alt-odb | xargs rm &&
 		git fsck &&
-		test_must_fail git pack-redundant --all --alt-odb >actual 2>&1 &&
+		test_must_fail $git_pack_redundant --all --alt-odb >actual 2>&1 &&
 		test_cmp expect actual
 	)
 '
@@ -386,7 +417,7 @@ test_expect_success 'shared: create new objects and packs' '
 test_expect_success 'shared: no redundant without --alt-odb' '
 	(
 		cd "$shared_repo" &&
-		git pack-redundant --all >out &&
+		$git_pack_redundant --all >out &&
 		test_must_be_empty out
 	)
 '
@@ -417,7 +448,7 @@ test_expect_success 'shared: no redundant without --alt-odb' '
 test_expect_success 'shared: one pack is redundant with --alt-odb' '
 	(
 		cd "$shared_repo" &&
-		git pack-redundant --all --alt-odb >out &&
+		$git_pack_redundant --all --alt-odb >out &&
 		format_packfiles <out >actual &&
 		test_line_count = 1 actual
 	)
@@ -454,7 +485,7 @@ test_expect_success 'shared: ignore unique objects and all two packs are redunda
 			Px1:$Px1
 			Px2:$Px2
 			EOF
-		git pack-redundant --all --alt-odb >out <<-EOF &&
+		$git_pack_redundant --all --alt-odb >out <<-EOF &&
 			$X
 			$Y
 			$Z
-- 
2.30.0.rc0


  reply	other threads:[~2020-12-16 10:12 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-15 17:34 bug report: "git pack-redundant --all" crash in minimize() Daniel C. Klauer
2020-12-15 18:21 ` Jeff King
2020-12-15 22:32   ` Junio C Hamano
2020-12-16  9:21   ` Jiang Xin
2020-12-16 10:09     ` Jiang Xin [this message]
2020-12-16 17:32       ` [PATCH] pack-redundant: fix crash when one packfile in repo Junio C Hamano
2020-12-17  1:57         ` [PATCH v2] " Jiang Xin
2020-12-17  6:08           ` Junio C Hamano
2020-12-16 18:46       ` [PATCH] " Jeff King
2020-12-16 13:22   ` bug report: "git pack-redundant --all" crash in minimize() Daniel Klauer
2020-12-16 19:00     ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201216100952.16890-1-worldhello.net@gmail.com \
    --to=worldhello.net@gmail.com \
    --cc=daniel.c.klauer@web.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=zhiyou.jx@alibaba-inc.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).