Re: [PATCH 3/8] push tests: add more testing for forced tag pushing

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

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Improve the tests added in dbfeddb12e ("push: require force for refs
> under refs/tags/", 2012-11-29) to assert that the same behavior
> applies various forms other refspecs, and that "+" in a refspec will
> override the "--no-force" option (but not the other way around).

For some reason this fell out of my radar; sorry about that.

I like the general idea to ensure non-ff pushes are rejected, unless
forced, to update a light-weight tag with another.  I am unsure what
should happen when trying to update a light-weight tag with an
object with different type (or vice versa), and haven't read in this
series what your opinion is yet.  Let's read on and see how it goes.

I have a moderately strong preference that

	$ git push --no-force child2

with a configured refspec

	[remote "child2"]
		url = ../child2
		push = +refs/tags/*:refs/tags/*

should behave as a non-forced push (regardless of the refs hierarchy
involved, not limited to tags/).  I have a mild preference against

	$ git push --no-force ../child2 +refs/tags/*:refs/tags/*

that forces, just because command line options look a lot more
explicit than the prefix '+', and choosing it not to force would
make it consistent with the desired behaviour for configured forcing
refspec.

I couldn't quite get what you meant by "(but not the other way
around)".  Did you mean

	$ git push --force ../child2 refs/tags/*:refs/tags/*

should not become non-forcing version because of the (lack of)
prefix on the refspec does not trump the --force command line
option?  If so, making

	$ git push --no-force ../child2 +refs/tags/*:refs/tags/*

not to force would make things more consistent, I suspect, i.e. we
can simply declare that presence or absense of '+' prefix in the
refspec determines the forced-ness of the push/fetch when there is
no command line option to decide it, but an explicit command line
option will always override it.  

Am I missing something obvious?

> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  t/t5516-fetch-push.sh | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
> index 15c8d5a734..c9a2011915 100755
> --- a/t/t5516-fetch-push.sh
> +++ b/t/t5516-fetch-push.sh
> @@ -981,7 +981,17 @@ test_expect_success 'push requires --force to update lightweight tag' '
>  		git push --force ../child2 Tag &&
>  		git tag -f Tag HEAD~ &&
>  		test_must_fail git push ../child2 Tag &&
> -		git push --force ../child2 Tag
> +		git push --force ../child2 Tag &&
> +		git tag -f Tag &&
> +		test_must_fail git push ../child2 "refs/tags/*:refs/tags/*" &&
> +		git push --force ../child2 "refs/tags/*:refs/tags/*" &&
> +		git tag -f Tag HEAD~ &&
> +		git push ../child2 "+refs/tags/*:refs/tags/*" &&
> +		git tag -f Tag &&
> +		git push --no-force ../child2 "+refs/tags/*:refs/tags/*" &&
> +		git tag -f Tag HEAD~ &&
> +		test_must_fail git push ../child2 tag Tag &&
> +		git push --force ../child2 tag Tag
>  	)
>  '