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@xxxxxxxxx>
Helped-by: Jeff King <peff@xxxxxxxx>
Helped-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
Signed-off-by: Denton Liu <liu.denton@xxxxxxxxx>
---
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"
+ "\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
