From: Jonathan Nieder <jrnieder@gmail.com>
Date: Wed, 31 Oct 2012 00:11:17 GMT
Subject: Re: [PATCH v2 4/4] fast-export: make sure refs are updated properly
Message-ID: <20121031001117.GA29486@elie.Belkin>
In-Reply-To: <1351623987-21012-5-git-send-email-felipe.contreras@gmail.com>

(cc-ing the git list)
Felipe Contreras wrote:

> When an object has already been exported (and thus is in the marks) it
> is flagged as SHOWN, so it will not be exported again, even if this time
> it's exported through a different ref.
>
> We don't need the object to be exported again, but we want the ref
> updated

Yes, makes perfect sense.

For what it's worth,
Acked-by: Jonathan Nieder <jrnieder@gmail.com>

[...]
> --- a/t/t5800-remote-helpers.sh
> +++ b/t/t5800-remote-helpers.sh
> @@ -145,4 +145,15 @@ test_expect_failure 'push new branch with old:new refspec' '
>  	compare_refs clone HEAD server refs/heads/new-refspec
>  '
>  
> +test_expect_success 'push ref with existing object' '
> +	(cd localclone &&
> +	git branch point-to-master master &&
> +	git push origin point-to-master
> +	) &&
> +
> +	(cd server &&
> +	git show-ref refs/heads/point-to-master
> +	)

Style: if you indent like this, the test becomes clearer:

	(
		cd localclone &&
		git branch point-to-master master &&
		git push origin point-to-master
	) &&
	(
		cd server &&
		git rev-parse --verify refs/heads/point-to-master
	)

[...]
> +test_expect_success 'refs are updated even if no commits need to be exported' '
> +	git fast-export --import-marks=tmp-marks \
> +		--export-marks=tmp-marks master > /dev/null &&

The redirect just makes the test log with "-v" less informative, so
I'd drop it.

> +	git fast-export --import-marks=tmp-marks \
> +		--export-marks=tmp-marks master > actual &&
> +	test_cmp expected actual

Redirections in git shell scripts are generally spelled as
"do_something >actual", without a space between the operator and
filename.

Hope that helps,
Jonathan