From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Denton Liu <liu.denton@gmail.com>
Cc: Git Mailing List <git@vger.kernel.org>, Jeff King <peff@peff.net>,
Robert Dailey <rcdailey.lists@gmail.com>
Subject: Re: [PATCH 1/3] tag: prevent recursive tags
Date: Tue, 26 Mar 2019 11:10:15 +0100 [thread overview]
Message-ID: <87imw6aquw.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <c371a653b4049256f3427e467b144385ee47ef43.1553586707.git.liu.denton@gmail.com>
On Tue, Mar 26 2019, Denton Liu wrote:
> 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
Better to split the refactoring part of this into a leading
change. I.e. the latter 2 removed/2 added here, and the former could
also start with a wrap to 79 chars...
> type = oid_object_info(the_repository, object, NULL);
> if (type <= OBJ_NONE)
> - die(_("bad object type."));
> + die(_("bad object type."));
Ditto this 4 space -> tab change.
> +
> + 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);
> + }
This patch of mine didn't end up making it in, but shows how instead of
"tag, tag, tag" here you can just pass it once and use positional printf
arguments in the message. It makes things a lot more self-explanatory:
https://public-inbox.org/git/20181026192734.9609-8-avarab@gmail.com/
>
> 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' '
At least some of these tests should change the existing test line to a
"test_must_fail", i.e. assert that without the new option these aren't
created, maybe for good measure test --force too.
next prev parent reply other threads:[~2019-03-26 10:10 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
2019-03-26 10:10 ` Ævar Arnfjörð Bjarmason [this message]
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87imw6aquw.fsf@evledraar.gmail.com \
--to=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=liu.denton@gmail.com \
--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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.