Re: [PATCH v10 1/8] transport: not report a non-head push as a branch

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

Jiang Xin <worldhello.net@gmail.com> writes:

>  create mode 100755 t/t5411-proc-receive-hook.sh
>  create mode 100644 t/t5411/common-functions.sh
>  create mode 100644 t/t5411/common-test-cases.sh
>  create mode 100755 t/t5412-proc-receive-hook-http-protocol.sh
>
> diff --git a/t/t5411-proc-receive-hook.sh b/t/t5411-proc-receive-hook.sh
> new file mode 100755
> index 0000000000..ef289fe92a
> --- /dev/null
> +++ b/t/t5411-proc-receive-hook.sh
> @@ -0,0 +1,75 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2020 Jiang Xin
> +#
> +
> +test_description='Test proc-receive hook'
> +
> +. ./test-lib.sh
> +
> +. "$TEST_DIRECTORY"/t5411/common-functions.sh
> +
> +# Format the output of git-push, git-show-ref and other commands to make a
> +# user-friendly and stable text.  In addition to the common format method,
> +# we also replace the URL of different protocol for the upstream repository
> +# with a fixed pattern.
> +make_user_friendly_and_stable_output () {
> +	make_user_friendly_and_stable_output_common | sed \
> +		-e "s#To ../upstream.git#To <URL/of/upstream.git>#"

Break the line immediately after the pipe, and begin the next line
with the command.  That way, the shell knows that you haven't
finished giving the command at the end of the first line, and you do
not have to use a backslash, i.e.

    make_user_friendly |
    sed -e 's/from/to/'

> +		cd workbench &&
> +		# Try to make a stable fixed width for abbreviated commit ID,
> +		# this fixed-width oid will be replaced with "<OID>".
> +		git config core.abbrev 7 &&
> +		git remote add origin ../upstream.git &&
> +		git update-ref refs/heads/master $A &&
> +		git tag -m "v123" v123 $A &&
> +		git push origin \
> +			$B:refs/heads/master \
> +			$A:refs/heads/next
> +	) &&
> +	TAG=$(git -C workbench rev-parse v123) &&
> +
> +	# setup pre-receive hook
> +	cat >upstream.git/hooks/pre-receive <<-\EOF &&

Use write_script and you don't need the chmod at the end.

> +	#!/bin/sh
> +
> +	echo >&2 "# pre-receive hook"
> +
> +	while read old new ref
> +	do
> +		echo >&2 "pre-receive< $old $new $ref"
> +	done
> +	EOF

Perhaps "exec >&2" upfront?

> +	# setup post-receive hook
> +	cat >upstream.git/hooks/post-receive <<-\EOF &&
> +	#!/bin/sh
> +
> +	echo >&2 "# post-receive hook"
> +
> +	while read old new ref
> +	do
> +		echo >&2 "post-receive< $old $new $ref"
> +	done
> +	EOF
> +
> +	chmod a+x \
> +		upstream.git/hooks/pre-receive \
> +		upstream.git/hooks/post-receive &&

Ditto.

> +	upstream=upstream.git
> +'
> +
> +# Include test cases for both file and HTTP protocol
> +. "$TEST_DIRECTORY"/t5411/common-test-cases.sh
> +
> +test_done


> diff --git a/t/t5411/common-functions.sh b/t/t5411/common-functions.sh
> new file mode 100644
> index 0000000000..6e400c0625
> --- /dev/null
> +++ b/t/t5411/common-functions.sh
> @@ -0,0 +1,50 @@

It is good that this omits "#!" and leaves the file without +x bit.

> +# Format the output of git-push, git-show-ref and other commands to make a
> +# user-friendly and stable text.  We can easily prepare the expect text
> +# without having to worry about future changes of the commit ID and spaces
> +# of the output.  We also replce single quotes with double quotes, because
> +# it is boring to prepare unquoted single quotes in expect txt.

s/txt/text/

> +make_user_friendly_and_stable_output_common () {
> +	sed \
> +		-e "s/  *\$//" \
> +		-e "s/   */ /g" \
> +		-e "s/'/\"/g" \
> +		-e "s/$A/<COMMIT-A>/g" \
> +		-e "s/$B/<COMMIT-B>/g" \
> +		-e "s/$TAG/<TAG-v123>/g" \
> +		-e "s/$ZERO_OID/<ZERO-OID>/g" \
> +		-e "s/$(echo $A | cut -c1-7)[0-9a-f]*/<OID-A>/g" \
> +		-e "s/$(echo $B | cut -c1-7)[0-9a-f]*/<OID-B>/g"
> +}

So a full object name for commit "A" becomes <COMMIT-A> while an
abbreviated one becomes <OID-A>?  OK.  I wonder if we force the full
length (i.e. no abbreviation), the tests become simpler, but it is
OK.  As long as A and B does not share the same 7 hexdigits in any
future hash function we choose, the above will do the right thing
;-)

> diff --git a/t/t5412-proc-receive-hook-http-protocol.sh b/t/t5412-proc-receive-hook-http-protocol.sh
> new file mode 100755
> index 0000000000..e2446d4d32
> --- /dev/null
> +++ b/t/t5412-proc-receive-hook-http-protocol.sh
> @@ -0,0 +1,86 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2020 Jiang Xin
> +#
> +
> +test_description='Test proc-receive hook for HTTP protocol'
> +
> +. ./test-lib.sh
> +
> +ROOT_PATH="$PWD"
> +. "$TEST_DIRECTORY"/lib-gpg.sh
> +. "$TEST_DIRECTORY"/lib-httpd.sh
> +. "$TEST_DIRECTORY"/lib-terminal.sh
> +start_httpd
> +
> +. "$TEST_DIRECTORY"/t5411/common-functions.sh
> +
> +# Format the output of git-push, git-show-ref and other commands to make a
> +# user-friendly and stable text.  In addition to the common format method,
> +# we also replace the URL of different protocol for the upstream repository
> +# with a fixed pattern.
> +make_user_friendly_and_stable_output () {
> +	make_user_friendly_and_stable_output_common | sed \
> +		-e "s#To http:.*/upstream.git#To <URL/of/upstream.git>#"
> +}

Ditto.

> +# Refs of upstream : master(B)  next(A)
> +# Refs of workbench: master(A)           tags/v123
> +test_expect_success "setup" '
> +	git init --bare upstream.git &&
> +	git -C upstream.git config http.receivepack true &&
> +	git init workbench &&
> +	create_commits_in workbench A B &&
> +	(
> +		cd workbench &&
> +		# Try to make a stable fixed width for abbreviated commit ID,
> +		# this fixed-width oid will be replaced with "<OID>".
> +		git config core.abbrev 7 &&
> +		git remote add origin ../upstream.git &&
> +		git update-ref refs/heads/master $A &&
> +		git tag -m "v123" v123 $A &&
> +		git push origin \
> +			$B:refs/heads/master \
> +			$A:refs/heads/next
> +	) &&
> +	TAG=$(git -C workbench rev-parse v123) &&
> +
> +	# setup pre-receive hook
> +	cat >upstream.git/hooks/pre-receive <<-\EOF &&
> +	#!/bin/sh

Ditto.

> +
> +	echo >&2 "# pre-receive hook"
> +
> +	while read old new ref
> +	do
> +		echo >&2 "pre-receive< $old $new $ref"
> +	done
> +	EOF
> +
> +	# setup post-receive hook
> +	cat >upstream.git/hooks/post-receive <<-\EOF &&
> +	#!/bin/sh
> +
> +	echo >&2 "# post-receive hook"
> +
> +	while read old new ref
> +	do
> +		echo >&2 "post-receive< $old $new $ref"
> +	done
> +	EOF
> +
> +	chmod a+x \
> +		upstream.git/hooks/pre-receive \
> +		upstream.git/hooks/post-receive &&
> +
> +	upstream="$HTTPD_DOCUMENT_ROOT_PATH/upstream.git" &&
> +	mv upstream.git "$upstream" &&
> +	git -C workbench remote set-url origin $HTTPD_URL/smart/upstream.git
> +'
> +
> +setup_askpass_helper
> +
> +# Include test cases for both file and HTTP protocol
> +. "$TEST_DIRECTORY"/t5411/common-test-cases.sh
> +
> +test_done
> diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
> index 9ff041a093..9e4b9313b5 100755
> --- a/t/t5516-fetch-push.sh
> +++ b/t/t5516-fetch-push.sh
> @@ -1039,7 +1039,7 @@ test_force_fetch_tag "annotated tag" "-f -a -m'tag message'"
>  test_expect_success 'push --porcelain' '
>  	mk_empty testrepo &&
>  	echo >.git/foo  "To testrepo" &&
> -	echo >>.git/foo "*	refs/heads/master:refs/remotes/origin/master	[new branch]"  &&
> +	echo >>.git/foo "*	refs/heads/master:refs/remotes/origin/master	[new reference]"  &&
>  	echo >>.git/foo "Done" &&
>  	git push >.git/bar --porcelain  testrepo refs/heads/master:refs/remotes/origin/master &&
>  	(
> diff --git a/transport.c b/transport.c
> index 1fdc7dac1a..272c0f4046 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -500,9 +500,12 @@ static void print_ok_ref_status(struct ref *ref, int porcelain, int summary_widt
>  				 porcelain, summary_width);
>  	else if (is_null_oid(&ref->old_oid))

This block is about an update to missing "old", i.e. a creation.

>  		print_ref_status('*',
> -			(starts_with(ref->name, "refs/tags/") ? "[new tag]" :
> -			"[new branch]"),
> -			ref, ref->peer_ref, NULL, porcelain, summary_width);

We used to say "if outside refs/tags/, let's call it branch".

> +				 (starts_with(ref->name, "refs/tags/")
> +				  ? "[new tag]"
> +				  : (starts_with(ref->name, "refs/heads/")
> +				     ? "[new branch]"
> +				     : "[new reference]")),
> +				 ref, ref->peer_ref, NULL, porcelain, summary_width);

Now we say "outside refs/heads/ and refs/tags/, that's just a
reference, different from branch".  OK.

Do we ever update refs/remotes/ with this codepath?  Would we want
to label it differently, i.e. as "remote-tracking branch"?

>  	else {
>  		struct strbuf quickref = STRBUF_INIT;
>  		char type;

Thanks.