Re: [PATCH] transport-helper: enforce atomic in push_refs_with_push

[Emily Shaffer <emilyshaffer@google.com>]

On Wed, Jul 03, 2019 at 11:13:45AM -0700, Junio C Hamano wrote:
> SZEDER Gábor <szeder.dev@gmail.com> writes:
> 
> > On Tue, Jul 02, 2019 at 02:37:42PM -0700, Junio C Hamano wrote:
> >> > +test_expect_success 'push --atomic shows all failed refs' '
> >> > +	# Make up/master, up/allrefs
> >> > +	d=$HTTPD_DOCUMENT_ROOT_PATH/atomic-failed-refs.git &&
> >> > +	git init --bare "$d" &&
> >> > +	git --git-dir="$d" config http.receivepack true &&
> >> > +	up="$HTTPD_URL"/smart/atomic-failed-refs.git &&
> >> > +	test_commit allrefs1 &&
> >> > +	test_commit allrefs2 &&
> >> > +	git branch allrefs &&
> >> > +	git push "$up" master allrefs &&
> >> > +	# Make master and allrefs incompatible with up/master, up/allrefs
> >> > +	git checkout allrefs &&
> >> > +	git reset --hard HEAD^ &&
> >> > +	git checkout master &&
> >> > +	git reset --hard HEAD^ &&
> >> > +	# --atomic should complain about both master and allrefs
> >> > +	test_must_fail git push --atomic "$up" master allrefs >&output &&
> >> 
> >> Don't rely on ">&output", which is an unnecessary bash-ism here.  It
> >> breaks test run under shells like dash.
> >> 
> >> 	>output 2>&1
> >> 
> >> should be OK.
> >
> > '2>output' would be a tad better, because those refs should be printed
> > to stderr.
> 
> Yeah; there are many existing uses of ">output 2>&1" in the same
> script and I was following the suit.  There also are 2>err and I
> agree that it is more appropriate in this case.

Oh, this is a good point. I'll change it, thanks both.