Re: [PATCH 12/12] receive-pack: avoid duplicates between our refs and alternates

Jeff King [off-list ref] writes:

We de-duplicate ".have" refs among themselves, but never
check if they are duplicates of our local refs. It's not
unreasonable that they would be if we are a "--shared" or
"--reference" clone of a similar repository; we'd have all
the same tags.

We can handle this by inserting our local refs into the
oidset, but obviously not suppressing duplicates (since the
refnames are important).

Makes sense.

+extract_ref_advertisement () {
+	perl -lne '
+		# \\ is there to skip capabilities after \0
+		/push< ([^\\]+)/ or next;
+		exit 0 if $1 eq "0000";
+		print $1;
+	'

Parsing TRACE_PACKET output?  Yuck.  But I think this has to do, as
any other solution will bound to be uglier.

+test_expect_success 'receive-pack de-dupes .have lines' '
+	git init shared &&
+	git -C shared commit --allow-empty -m both &&
+	git clone -s shared fork &&
+	(
+		cd shared &&
+		git checkout -b only-shared &&
+		git commit --allow-empty -m only-shared &&
+		git update-ref refs/heads/foo HEAD
+	) &&
+
+	# Notable things in this expectation:
+	#  - local refs are not de-duped
+	#  - .have does not duplicate locals
+	#  - .have does not duplicate itself
+	local=$(git -C fork rev-parse HEAD) &&
+	shared=$(git -C shared rev-parse only-shared) &&
+	cat >expect <<-EOF &&
+	$local refs/heads/master
+	$local refs/remotes/origin/HEAD
+	$local refs/remotes/origin/master
+	$shared .have
+	EOF

We may want to sort this thing and the extracted one when comparing;
the order of the entries is not part of the feature we cast in stone.

+
+	GIT_TRACE_PACKET=$(pwd)/trace git push fork HEAD:foo &&
+	extract_ref_advertisement <trace >refs &&
+	test_cmp expect refs
+'
+
 test_done

On Wed, Jan 25, 2017 at 12:02:30PM -0800, Junio C Hamano wrote:

quoted

+extract_ref_advertisement () {
+	perl -lne '
+		# \\ is there to skip capabilities after \0
+		/push< ([^\\]+)/ or next;
+		exit 0 if $1 eq "0000";
+		print $1;
+	'

Parsing TRACE_PACKET output?  Yuck.  But I think this has to do, as
any other solution will bound to be uglier.

Agreed. My initial attempt was to just run "git receive-pack </dev/null"
and parse the advertisement. But then you have to parse pktlines. :)

quoted

+	# Notable things in this expectation:
+	#  - local refs are not de-duped
+	#  - .have does not duplicate locals
+	#  - .have does not duplicate itself
+	local=$(git -C fork rev-parse HEAD) &&
+	shared=$(git -C shared rev-parse only-shared) &&
+	cat >expect <<-EOF &&
+	$local refs/heads/master
+	$local refs/remotes/origin/HEAD
+	$local refs/remotes/origin/master
+	$shared .have
+	EOF

We may want to sort this thing and the extracted one when comparing;
the order of the entries is not part of the feature we cast in stone.

True (and in fact the .have moved in the previous patch). I wondered,
though, if it might not be a reasonable thing for somebody to have to
look at the output and verify it when the order _does_ change.

I dunno.

-Peff