From: Denton Liu <liu.denton@gmail.com>
To: Git Mailing List <git@vger.kernel.org>
Cc: "Jeff King" <peff@peff.net>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
"Robert Dailey" <rcdailey.lists@gmail.com>
Subject: Re: [PATCH 1/3] tag: prevent recursive tags
Date: Tue, 26 Mar 2019 01:51:47 -0700 [thread overview]
Message-ID: <20190326085147.GA3837@archbookpro.localdomain> (raw)
In-Reply-To: <c371a653b4049256f3427e467b144385ee47ef43.1553586707.git.liu.denton@gmail.com>
On Tue, Mar 26, 2019 at 12:53:18AM -0700, Denton Liu wrote:
> 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 <rcdailey.lists@gmail.com>
> Helped-by: Jeff King <peff@peff.net>
> Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
> 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 <key-id>] [-f] [-m <msg> | -F <file>] <tagname> [<head>]"),
> + N_("git tag [-a | -s | -u <key-id>] [-f] [-m <msg> | -F <file>] [--allow-recursive-tag]\n"
> + "\t\t<tagname> [<head>]"),
> N_("git tag -d <tagname>..."),
> - N_("git tag -l [-n[<num>]] [--contains <commit>] [--no-contains <commit>] [--points-at <object>]"
> - "\n\t\t[--format=<format>] [--[no-]merged [<commit>]] [<pattern>...]"),
> + N_("git tag -l [-n[<num>]] [--contains <commit>] [--no-contains <commit>] [--points-at <object>]\n"
> + "\t\t[--format=<format>] [--[no-]merged [<commit>]] [<pattern>...]"),
> N_("git tag -v [--format=<format>] <tagname>..."),
> 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"
My bad, left an extra space before the quote.
> + "\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' '
> --
> 2.21.0.512.g57bf1b23e1
>
next prev parent reply other threads:[~2019-03-26 8:51 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-21 16:59 Strange annotated tag issue Robert Dailey
2019-03-21 19:04 ` Bryan Turner
2019-03-21 19:39 ` Jeff King
2019-03-21 19:29 ` Jeff King
2019-03-25 13:50 ` Robert Dailey
2019-03-25 14:49 ` Jeff King
2019-03-25 15:31 ` Robert Dailey
2019-03-25 18:43 ` Bryan Turner
2019-03-25 23:36 ` Jeff King
2019-03-25 19:19 ` Ævar Arnfjörð Bjarmason
2019-03-25 23:37 ` Jeff King
2019-03-26 7:53 ` [PATCH 0/3] tag: prevent recursive tags Denton Liu
2019-03-26 7:53 ` [PATCH 1/3] " Denton Liu
2019-03-26 8:51 ` Denton Liu [this message]
2019-03-26 10:10 ` Ævar Arnfjörð Bjarmason
2019-03-27 4:57 ` Elijah Newren
2019-03-27 10:27 ` Ævar Arnfjörð Bjarmason
2019-03-28 19:02 ` Robert Dailey
2019-03-26 7:53 ` [PATCH 2/3] t7004: ensure recursive tag behavior is working Denton Liu
2019-03-26 10:11 ` Ævar Arnfjörð Bjarmason
2019-03-26 7:53 ` [PATCH 3/3] git-tag.txt: document --allow-recursive-tag option Denton Liu
2019-03-26 10:16 ` Ævar Arnfjörð Bjarmason
2019-03-26 16:18 ` [PATCH 0/3] tag: prevent recursive tags Jeff King
2019-04-02 5:38 ` [PATCH v2 0/2] tag: prevent nested tags Denton Liu
2019-04-02 5:38 ` [PATCH v2 1/2] tag: fix formatting Denton Liu
2019-04-02 5:38 ` [PATCH v2 2/2] tag: prevent nested tags Denton Liu
2019-04-02 23:03 ` [PATCH v2.5 " Denton Liu
2019-04-03 7:32 ` Junio C Hamano
2019-04-03 8:49 ` Junio C Hamano
2019-04-03 18:26 ` Robert Dailey
2019-04-04 9:32 ` Junio C Hamano
2019-04-04 13:47 ` Robert Dailey
2019-04-04 21:50 ` Junio C Hamano
2019-04-05 2:51 ` Robert Dailey
2019-04-03 18:16 ` Johannes Sixt
2019-04-03 21:33 ` Denton Liu
2019-04-04 2:02 ` Jeff King
2019-04-04 9:31 ` Junio C Hamano
2019-04-04 12:27 ` Jeff King
2019-04-04 21:54 ` Junio C Hamano
2019-04-04 22:12 ` Jeff King
2019-04-11 18:40 ` Eckhard Maaß
2019-04-12 3:21 ` Junio C Hamano
2019-04-05 0:36 ` Elijah Newren
2019-04-05 5:29 ` Junio C Hamano
2019-04-04 18:25 ` [PATCH v3 0/2] tag: advise on recursive tagging Denton Liu
2019-04-04 18:25 ` [PATCH v3 1/2] tag: fix formatting Denton Liu
2019-04-04 18:25 ` [PATCH v3 2/2] tag: advise on nested tags Denton Liu
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=20190326085147.GA3837@archbookpro.localdomain \
--to=liu.denton@gmail.com \
--cc=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=peff@peff.net \
--cc=rcdailey.lists@gmail.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).