Re: BUG: For first push to a bare repo, using "--tags" prevents creation of master branch

[Junio C Hamano <gitster@pobox.com>]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> I recall "fetch --tags" was updated to become your "--tags-also" by
>> e66ef7ae (Merge branch 'mh/fetch-tags-in-addition-to-normal-refs',
>> 2013-12-12).  Perhaps we should do the same on the push side.
>
> Would we still have a way to push tags only with this?
>
> fetch and push are a bit different: it's almost never a bad idea to run
> fetch (I see remote-tracking as a kind of cache, and fetch is the way to
> refresh this cache), but pushing too much can be harmful. Not a strong
> objection, but I'm not sure that this change is 100% safe for everyone.

Yeah, a possible fallout can be seen in the updates to the tests in
this patch.  I am undecided if "pushing can be harmful" should trump
"--tags should behave in a similar way for fetch and push that are
the reverse of each other".

If we are to introduce --tags-also (or --tags-too), we should add it
to fetch, too.


-- >8 --
Subject: [PATCH] push --tags: push tags *in addition to* other stuff

Earlier, we changed "git fetch --tags" from "fetch all tags instead
of what we would fetch without --tags argument" to "fetch all tags
in addition to what we would fetch without --tags argument" with
c5a84e92 (fetch --tags: fetch tags *in addition to* other stuff,
2013-10-30), which was merged as a part of a series at e66ef7ae.

This does the same to "git push --tags"; instead of pushing all tags
(and nothing else, as the command line does not have any other
refspec), the command now pushes all tags in addition to what "git
push" would have pushed.

The changes to the tests shows a possible fallout.  Both of them do
not set up the default push refspec, causing "git push" without any
refspecs on the command line to fail, which was OK because they do
not intend to do "git push" without any explicit refspec on the
command line.  But now "--tags" no longer is considered an explicit
refspec, the logic to find the refs to be pushed by default is
triggered to find what "other stuff" needs to be pushed, and causes
the "git push --tags" to fail.  As it is to push tags and nothing
else what these pushes want to do, replace these "git push --tags"
invocations with "git push" with an explicit "refs/tags/\*" refspec.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/push.c                   | 18 ++++++++++--------
 t/t5509-fetch-push-namespaces.sh |  2 +-
 t/t5516-fetch-push.sh            |  2 +-
 3 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/builtin/push.c b/builtin/push.c
index 57c138b..ec215d7 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -382,7 +382,7 @@ static int push_with_options(struct transport *transport, int flags)
 	return 1;
 }
 
-static int do_push(const char *repo, int flags)
+static int do_push(const char *repo, int flags, int tags_too)
 {
 	int i, errs;
 	struct remote *remote = pushremote_get(repo);
@@ -429,6 +429,10 @@ static int do_push(const char *repo, int flags)
 		} else if (!(flags & TRANSPORT_PUSH_MIRROR))
 			setup_default_push_refspecs(remote);
 	}
+
+	if (tags_too)
+		add_refspec("refs/tags/*");
+
 	errs = 0;
 	url_nr = push_url_of_remote(remote, &url);
 	if (url_nr) {
@@ -494,7 +498,7 @@ static int git_push_config(const char *k, const char *v, void *cb)
 int cmd_push(int argc, const char **argv, const char *prefix)
 {
 	int flags = 0;
-	int tags = 0;
+	int tags_too = 0;
 	int rc;
 	const char *repo = NULL;	/* default repository */
 	struct option options[] = {
@@ -504,7 +508,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		OPT_BIT( 0 , "mirror", &flags, N_("mirror all refs"),
 			    (TRANSPORT_PUSH_MIRROR|TRANSPORT_PUSH_FORCE)),
 		OPT_BOOL( 0, "delete", &deleterefs, N_("delete refs")),
-		OPT_BOOL( 0 , "tags", &tags, N_("push tags (can't be used with --all or --mirror)")),
+		OPT_BOOL( 0 , "tags", &tags_too, N_("push tags (can't be used with --all or --mirror)")),
 		OPT_BIT('n' , "dry-run", &flags, N_("dry run"), TRANSPORT_PUSH_DRY_RUN),
 		OPT_BIT( 0,  "porcelain", &flags, N_("machine-readable output"), TRANSPORT_PUSH_PORCELAIN),
 		OPT_BIT('f', "force", &flags, N_("force updates"), TRANSPORT_PUSH_FORCE),
@@ -535,20 +539,18 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 	git_config(git_push_config, &flags);
 	argc = parse_options(argc, argv, prefix, options, push_usage, 0);
 
-	if (deleterefs && (tags || (flags & (TRANSPORT_PUSH_ALL | TRANSPORT_PUSH_MIRROR))))
+	if (deleterefs &&
+	    (tags_too || (flags & (TRANSPORT_PUSH_ALL | TRANSPORT_PUSH_MIRROR))))
 		die(_("--delete is incompatible with --all, --mirror and --tags"));
 	if (deleterefs && argc < 2)
 		die(_("--delete doesn't make sense without any refs"));
 
-	if (tags)
-		add_refspec("refs/tags/*");
-
 	if (argc > 0) {
 		repo = argv[0];
 		set_refspecs(argv + 1, argc - 1, repo);
 	}
 
-	rc = do_push(repo, flags);
+	rc = do_push(repo, flags, tags_too);
 	if (rc == -1)
 		usage_with_options(push_usage, options);
 	else
diff --git a/t/t5509-fetch-push-namespaces.sh b/t/t5509-fetch-push-namespaces.sh
index cc0b31f..2f9b0f0 100755
--- a/t/t5509-fetch-push-namespaces.sh
+++ b/t/t5509-fetch-push-namespaces.sh
@@ -30,7 +30,7 @@ test_expect_success 'pushing into a repository using a ref namespace' '
 		git ls-remote pushee-namespaced >actual &&
 		printf "$commit1\trefs/heads/master\n" >expected &&
 		test_cmp expected actual &&
-		git push pushee-namespaced --tags &&
+		git push pushee-namespaced "refs/tags/*" &&
 		git ls-remote pushee-namespaced >actual &&
 		printf "$commit0\trefs/tags/0\n" >>expected &&
 		printf "$commit1\trefs/tags/1\n" >>expected &&
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 8a5f236..9d54186 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -802,7 +802,7 @@ test_expect_success 'allow deleting a ref using --delete' '
 test_expect_success 'allow deleting a tag using --delete' '
 	mk_test testrepo heads/master &&
 	git tag -a -m dummy_message deltag heads/master &&
-	git push testrepo --tags &&
+	git push testrepo "refs/tags/*" &&
 	(cd testrepo && git rev-parse --verify -q refs/tags/deltag) &&
 	git push testrepo --delete tag deltag &&
 	(cd testrepo && test_must_fail git rev-parse --verify refs/tags/deltag)
-- 
2.4.2-540-gc413a76