From patchwork Tue Mar 26 07:53:18 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Denton Liu X-Patchwork-Id: 10870593 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id EAB0417E0 for ; Tue, 26 Mar 2019 07:53:26 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id D26E928D18 for ; Tue, 26 Mar 2019 07:53:26 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id C66F729087; Tue, 26 Mar 2019 07:53:26 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id D715028D18 for ; Tue, 26 Mar 2019 07:53:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731238AbfCZHxW (ORCPT ); Tue, 26 Mar 2019 03:53:22 -0400 Received: from mail-pg1-f193.google.com ([209.85.215.193]:40609 "EHLO mail-pg1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731114AbfCZHxV (ORCPT ); Tue, 26 Mar 2019 03:53:21 -0400 Received: by mail-pg1-f193.google.com with SMTP id u9so7866676pgo.7 for ; Tue, 26 Mar 2019 00:53:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=vFvM7T9db3xFiORrUDhqHHiMW9gAeGCCoKzPNt0VbjQ=; b=Snnz+Owvye4Iwbttko71ZWtkwWyflWnrfjc9VGOqj7fVMvqKhVu0kbRoll2T7ej7kF Q7VkvXx0y7sif5d7Mcx4z2e9Xec/GjZmwYThXZbCa6Er9ZekViTEC9ljriOSEk4fMD1e DIJtNT/FTa+cpjTVzQbmRx5wn4C7QTBPcjNJde9zr1LZoDm3uCZLztlqZdBYAWnbg354 xfhScvrLdsy0UybkdR0X9y299tW18vkquFR8zVChofTM7BYRJfR7MugZPDZFxeLR49N0 hSWahZ7ppJttJ4NeNPvw7alZAH4TDVYQKYcErlPFcZQyr0W499kPQ44Ky7LLmBVa9Awh UFaQ== 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:content-transfer-encoding :in-reply-to:user-agent; bh=vFvM7T9db3xFiORrUDhqHHiMW9gAeGCCoKzPNt0VbjQ=; b=SbPvR7p428AvPC0BeSgL9qYxQYHxeiLpzkv60WOgaMQMRpAz55wN98x88qtrhuZZa/ O9qWqECTt9bfppetU2/Uf0lXdA9UxxT4a60xprKQMr8gLbPh6fiZTod2xObURkcNsKWW 6/NgOC8dWLdrlMYh4CAa4LCLG1BWykR0JK5Faun3GY0kNVytaOyLp0yyZt14iGvcqXFA 9PZtOkbl6gaYefeBxMqOKyHJglY7iPCbEF2NrcuR+LyQCVLU0JnSuKQO7OydxEhExduu +hMC5tkWa4r6TiJRtf4VtsGS34YbnYIEEjAYRR5n8V156L/6sPrC4H25WGP4ieWM2AgA StHw== X-Gm-Message-State: APjAAAXQMCOf1aSKjg0MdT8YmojTLVx4nabGKoIxe/5D0uaTA4gBrsKr 75YFSzC3VxApzno7J2+Wfx7mDozP X-Google-Smtp-Source: APXvYqx21e+M3/STptbOyLz60997TolvRFS02E8nBhjQQwKZr09IPU1uUId+8Rf/CNQlJUwMw37PKg== X-Received: by 2002:aa7:8615:: with SMTP id p21mr5220418pfn.98.1553586800351; Tue, 26 Mar 2019 00:53:20 -0700 (PDT) Received: from archbookpro.localdomain (c-73-222-73-77.hsd1.ca.comcast.net. [73.222.73.77]) by smtp.gmail.com with ESMTPSA id 25sm22835735pfo.96.2019.03.26.00.53.19 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 26 Mar 2019 00:53:19 -0700 (PDT) Date: Tue, 26 Mar 2019 00:53:18 -0700 From: Denton Liu To: Git Mailing List Cc: Jeff King , =?iso-8859-1?q?=C6var_Arnfj=F6r=F0?= Bjarmason , Robert Dailey Subject: [PATCH 1/3] tag: prevent recursive tags Message-ID: References: <20190325233723.GD23728@sigill.intra.peff.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.11.4 (2019-03-13) Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Robert Dailey reported confusion on the mailing list about a recursive tag which was most likely created by mistake. Jeff King noted that this isn't a very common case so, most likely, creating a tag-to-a-tag is a user-error. Prevent mistakes by erroring and providing advice on recursive tags, unless "--allow-recursive-tag" is specified. Fix tests that fail as a result of this change. Reported-by: Robert Dailey Helped-by: Jeff King Helped-by: Ævar Arnfjörð Bjarmason Signed-off-by: Denton Liu --- advice.c | 2 ++ advice.h | 1 + builtin/tag.c | 30 ++++++++++++++++++++++++++---- t/annotate-tests.sh | 2 +- t/t0410-partial-clone.sh | 2 +- t/t4205-log-pretty-formats.sh | 2 +- t/t5305-include-tag.sh | 2 +- t/t5500-fetch-pack.sh | 2 +- t/t6302-for-each-ref-filter.sh | 4 ++-- t/t7004-tag.sh | 4 ++-- t/t9350-fast-export.sh | 4 ++-- 11 files changed, 40 insertions(+), 15 deletions(-) diff --git a/advice.c b/advice.c index 567209aa79..f31889e6de 100644 --- a/advice.c +++ b/advice.c @@ -26,6 +26,7 @@ int advice_ignored_hook = 1; int advice_waiting_for_editor = 1; int advice_graft_file_deprecated = 1; int advice_checkout_ambiguous_remote_branch_name = 1; +int advice_recursive_tag = 1; static int advice_use_color = -1; static char advice_colors[][COLOR_MAXLEN] = { @@ -81,6 +82,7 @@ static struct { { "waitingForEditor", &advice_waiting_for_editor }, { "graftFileDeprecated", &advice_graft_file_deprecated }, { "checkoutAmbiguousRemoteBranchName", &advice_checkout_ambiguous_remote_branch_name }, + { "recursiveTag", &advice_recursive_tag }, /* make this an alias for backward compatibility */ { "pushNonFastForward", &advice_push_update_rejected } diff --git a/advice.h b/advice.h index f875f8cd8d..66aa39757c 100644 --- a/advice.h +++ b/advice.h @@ -26,6 +26,7 @@ extern int advice_ignored_hook; extern int advice_waiting_for_editor; extern int advice_graft_file_deprecated; extern int advice_checkout_ambiguous_remote_branch_name; +extern int advice_recursive_tag; int git_default_advice_config(const char *var, const char *value); __attribute__((format (printf, 1, 2))) diff --git a/builtin/tag.c b/builtin/tag.c index 02f6bd1279..0b44a3cbc1 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -22,10 +22,11 @@ #include "ref-filter.h" static const char * const git_tag_usage[] = { - N_("git tag [-a | -s | -u ] [-f] [-m | -F ] []"), + N_("git tag [-a | -s | -u ] [-f] [-m | -F ] [--allow-recursive-tag]\n" + "\t\t []"), N_("git tag -d ..."), - N_("git tag -l [-n[]] [--contains ] [--no-contains ] [--points-at ]" - "\n\t\t[--format=] [--[no-]merged []] [...]"), + N_("git tag -l [-n[]] [--contains ] [--no-contains ] [--points-at ]\n" + "\t\t[--format=] [--[no-]merged []] [...]"), N_("git tag -v [--format=] ..."), NULL }; @@ -197,6 +198,7 @@ static int build_tag_object(struct strbuf *buf, int sign, struct object_id *resu struct create_tag_options { unsigned int message_given:1; unsigned int use_editor:1; + unsigned int allow_recursive_tag; unsigned int sign; enum { CLEANUP_NONE, @@ -205,6 +207,17 @@ struct create_tag_options { } cleanup_mode; }; +static const char message_advice_recursive_tag[] = + N_("The object '%s' referred to by your new tag is already a tag.\n" + "\n" + "If you meant to create a tag of a tag, use:\n" + "\n" + "\tgit tag --allow-recursive-tag %s\n" + "\n" + "If you meant to tag the object that it points to, use:\n" + "\n" + "\tgit tag %s^{}"); + static void create_tag(const struct object_id *object, const char *tag, struct strbuf *buf, struct create_tag_options *opt, struct object_id *prev, struct object_id *result) @@ -215,7 +228,14 @@ static void create_tag(const struct object_id *object, const char *tag, type = oid_object_info(the_repository, object, NULL); if (type <= OBJ_NONE) - die(_("bad object type.")); + die(_("bad object type.")); + + if (type == OBJ_TAG && !opt->allow_recursive_tag) { + error(_("refusing to make a recursive tag")); + if (advice_recursive_tag) + advise(_(message_advice_recursive_tag), tag, tag, tag); + exit(1); + } strbuf_addf(&header, "object %s\n" @@ -403,6 +423,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix) N_("use another key to sign the tag")), OPT__FORCE(&force, N_("replace the tag if exists"), 0), OPT_BOOL(0, "create-reflog", &create_reflog, N_("create a reflog")), + OPT_BOOL(0, "allow-recursive-tag", &opt.allow_recursive_tag, + N_("allow recursive tags to be made")), OPT_GROUP(N_("Tag listing options")), OPT_COLUMN(0, "column", &colopts, N_("show tag list in columns")), diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh index 6da48a2e0a..841f922e07 100644 --- a/t/annotate-tests.sh +++ b/t/annotate-tests.sh @@ -70,7 +70,7 @@ test_expect_success 'blame 1 author' ' test_expect_success 'blame by tag objects' ' git tag -m "test tag" testTag && - git tag -m "test tag #2" testTag2 testTag && + git tag -m "test tag #2" --allow-recursive-tag testTag2 testTag && check_count -h testTag A 2 && check_count -h testTag2 A 2 ' diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh index bce02788e6..5f06c2d76f 100755 --- a/t/t0410-partial-clone.sh +++ b/t/t0410-partial-clone.sh @@ -16,7 +16,7 @@ pack_as_from_promisor () { promise_and_delete () { HASH=$(git -C repo rev-parse "$1") && - git -C repo tag -a -m message my_annotated_tag "$HASH" && + git -C repo tag -a -m message my_annotated_tag --allow-recursive-tag "$HASH" && git -C repo rev-parse my_annotated_tag | pack_as_from_promisor && # tag -d prints a message to stdout, so redirect it git -C repo tag -d my_annotated_tag >/dev/null && diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh index f42a69faa2..018550f3b2 100755 --- a/t/t4205-log-pretty-formats.sh +++ b/t/t4205-log-pretty-formats.sh @@ -511,7 +511,7 @@ test_expect_success 'set up log decoration tests' ' test_expect_success 'log decoration properly follows tag chain' ' git tag -a tag1 -m tag1 && - git tag -a tag2 -m tag2 tag1 && + git tag -a tag2 -m tag2 --allow-recursive-tag tag1 && git tag -d tag1 && git commit --amend -m shorter && git log --no-walk --tags --pretty="%H %d" --decorate=full >actual && diff --git a/t/t5305-include-tag.sh b/t/t5305-include-tag.sh index a5eca210b8..c99850c1c0 100755 --- a/t/t5305-include-tag.sh +++ b/t/t5305-include-tag.sh @@ -68,7 +68,7 @@ test_expect_success 'check unpacked result (have commit, have tag)' ' test_expect_success 'create hidden inner tag' ' test_commit commit && git tag -m inner inner HEAD && - git tag -m outer outer inner && + git tag -m outer --allow-recursive-tag outer inner && git tag -d inner ' diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh index 49c540b1e1..c549b37aec 100755 --- a/t/t5500-fetch-pack.sh +++ b/t/t5500-fetch-pack.sh @@ -562,7 +562,7 @@ test_expect_success 'test --all wrt tag to non-commits' ' hello tag EOF ) && - git tag -a -m "tag -> tag" tag-to-tag $tag && + git tag -a -m "tag -> tag" --allow-recursive-tag tag-to-tag $tag && # `fetch-pack --all` should succeed fetching all those objects. mkdir fetchall && diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh index fc067ed672..f7b56ae195 100755 --- a/t/t6302-for-each-ref-filter.sh +++ b/t/t6302-for-each-ref-filter.sh @@ -12,7 +12,7 @@ test_expect_success 'setup some history and refs' ' git checkout -b side && test_commit four && git tag -m "An annotated tag" annotated-tag && - git tag -m "Annonated doubly" doubly-annotated-tag annotated-tag && + git tag -m "Annonated doubly" --allow-recursive-tag doubly-annotated-tag annotated-tag && # Note that these "signed" tags might not actually be signed. # Tests which care about the distinction should be marked @@ -24,7 +24,7 @@ test_expect_success 'setup some history and refs' ' sign= fi && git tag $sign -m "A signed tag" signed-tag && - git tag $sign -m "Signed doubly" doubly-signed-tag signed-tag && + git tag $sign -m "Signed doubly" --allow-recursive-tag doubly-signed-tag signed-tag && git checkout master && git update-ref refs/odd/spot master diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh index 0b01862c23..7a7c0ccee9 100755 --- a/t/t7004-tag.sh +++ b/t/t7004-tag.sh @@ -1265,7 +1265,7 @@ echo "A message for another tag" >>expect echo '-----BEGIN PGP SIGNATURE-----' >>expect test_expect_success GPG \ 'creating a signed tag pointing to another tag should succeed' ' - git tag -s -m "A message for another tag" tag-signed-tag signed-tag && + git tag -s -m "A message for another tag" --allow-recursive-tag tag-signed-tag signed-tag && get_tag_msg tag-signed-tag >actual && test_cmp expect actual ' @@ -1690,7 +1690,7 @@ test_expect_success '--points-at finds annotated tags of commits' ' ' test_expect_success '--points-at finds annotated tags of tags' ' - git tag -m "describing the v4.0 tag object" \ + git tag -m "describing the v4.0 tag object" --allow-recursive-tag \ annotated-again-v4.0 annotated-v4.0 && cat >expect <<-\EOF && annotated-again-v4.0 diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh index 5690fe2810..b5ed7e119a 100755 --- a/t/t9350-fast-export.sh +++ b/t/t9350-fast-export.sh @@ -441,8 +441,8 @@ test_expect_success 'set-up a few more tags for tag export tests' ' HEAD_TREE=$(git show -s --pretty=raw HEAD | grep tree | sed "s/tree //") && git tag tree_tag -m "tagging a tree" $HEAD_TREE && git tag -a tree_tag-obj -m "tagging a tree" $HEAD_TREE && - git tag tag-obj_tag -m "tagging a tag" tree_tag-obj && - git tag -a tag-obj_tag-obj -m "tagging a tag" tree_tag-obj + git tag tag-obj_tag -m "tagging a tag" --allow-recursive-tag tree_tag-obj && + git tag -a tag-obj_tag-obj -m "tagging a tag" --allow-recursive-tag tree_tag-obj ' test_expect_success 'tree_tag' '