tag: advise on nested tags

Robert Dailey reported confusion on the mailing list about a nested tag which was most likely created by mistake. Jeff King noted that this isn't a very common case and creating a tag-to-a-tag can be a user-error. Suggest that it may be a mistake with an advice message when creating such a tag. Those who do want to create a tag that point at another tag regularly can turn it off with the usual advice mechanism. 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> [jc: fixed test style and tweaked the log message] Signed-off-by: Junio C Hamano <gitster@pobox.com>

Denton Liu committed Apr 4, 2019 at 11:25 UTC eea9c1e78ffd878cbaaf2c1e346e186a1e58a460
5 files changed +28 -2
Documentation/config/advice.txt
+2
@@ -90,4 +90,6 @@ advice.*::
90 waitingForEditor::
91 Print a message to the terminal whenever Git is waiting for
92 editor input from the user.
93 + nestedTag::
94 + Advice shown if a user attempts to recursively tag a tag object.
95 --
advice.c
+2
@@ -26,6 +26,7 @@ int advice_ignored_hook = 1;
26 int advice_waiting_for_editor = 1;
27 int advice_graft_file_deprecated = 1;
28 int advice_checkout_ambiguous_remote_branch_name = 1;
29 +int advice_nested_tag = 1;
30
31 static int advice_use_color = -1;
32 static char advice_colors[][COLOR_MAXLEN] = {
@@ -81,6 +82,7 @@ static struct {
82 { "waitingForEditor", &advice_waiting_for_editor },
83 { "graftFileDeprecated", &advice_graft_file_deprecated },
84 { "checkoutAmbiguousRemoteBranchName", &advice_checkout_ambiguous_remote_branch_name },
85 + { "nestedTag", &advice_nested_tag },
86
87 /* make this an alias for backward compatibility */
88 { "pushNonFastForward", &advice_push_update_rejected }
advice.h
+1
@@ -26,6 +26,7 @@ extern int advice_ignored_hook;
26 extern int advice_waiting_for_editor;
27 extern int advice_graft_file_deprecated;
28 extern int advice_checkout_ambiguous_remote_branch_name;
29 +extern int advice_nested_tag;
30
31 int git_default_advice_config(const char *var, const char *value);
32 __attribute__((format (printf, 1, 2)))
builtin/tag.c
+12 -2
@@ -206,7 +206,14 @@ struct create_tag_options {
206 } cleanup_mode;
207 };
208
209 -static void create_tag(const struct object_id *object, const char *tag,
209 +static const char message_advice_nested_tag[] =
210 + N_("You have created a nested tag. The object referred to by your new is\n"
211 + "already a tag. If you meant to tag the object that it points to, use:\n"
212 + "\n"
213 + "\tgit tag -f %s %s^{}");
214 +
215 +static void create_tag(const struct object_id *object, const char *object_ref,
216 + const char *tag,
217 struct strbuf *buf, struct create_tag_options *opt,
218 struct object_id *prev, struct object_id *result)
219 {
@@ -218,6 +225,9 @@ static void create_tag(const struct object_id *object, const char *tag,
225 if (type <= OBJ_NONE)
226 die(_("bad object type."));
227
228 + if (type == OBJ_TAG && advice_nested_tag)
229 + advise(_(message_advice_nested_tag), tag, object_ref);
230 +
231 strbuf_addf(&header,
232 "object %s\n"
233 "type %s\n"
@@ -551,7 +561,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
561 if (create_tag_object) {
562 if (force_sign_annotate && !annotate)
563 opt.sign = 1;
554 - create_tag(&object, tag, &buf, &opt, &prev, &object);
564 + create_tag(&object, object_ref, tag, &buf, &opt, &prev, &object);
565 }
566
567 transaction = ref_transaction_begin(&err);
t/t7004-tag.sh
+11
@@ -1700,6 +1700,17 @@ test_expect_success '--points-at finds annotated tags of tags' '
1700 test_cmp expect actual
1701 '
1702
1703 +test_expect_success 'recursive tagging should give advice' '
1704 + sed -e "s/|$//" <<-EOF >expect &&
1705 + hint: You have created a nested tag. The object referred to by your new is
1706 + hint: already a tag. If you meant to tag the object that it points to, use:
1707 + hint: |
1708 + hint: git tag -f nested annotated-v4.0^{}
1709 + EOF
1710 + git tag -m nested nested annotated-v4.0 2>actual &&
1711 + test_i18ncmp expect actual
1712 +'
1713 +
1714 test_expect_success 'multiple --points-at are OR-ed together' '
1715 cat >expect <<-\EOF &&
1716 v2.0