Re: [PATCH 06/17] Let fetch_pack() inform caller about number of unique heads
From: Jeff King <hidden>
Date: 2016-06-15 22:54:33
On Thu, Aug 23, 2012 at 10:10:31AM +0200, mhagger@alum.mit.edu wrote:
From: Michael Haggerty <redacted> fetch_pack() remotes duplicates from the list (nr_heads, heads), thereby shrinking the list. But previously, the caller was not informed about the shrinkage. This would cause a spurious error message to be emitted by cmd_fetch_pack() if "git fetch-pack" is called with duplicate refnames. So change the signature of fetch_pack() to accept nr_heads by reference, and if any duplicates were removed then modify it to reflect the number of remaining references. The last test of t5500 inexplicably *required* "git fetch-pack" to fail when fetching a list of references that contains duplicates; i.e., it insisted on the buggy behavior. So change the test to expect the correct behavior.
Eek, yeah, the current behavior is obviously wrong. The remove_duplicates code comes from 310b86d (fetch-pack: do not barf when duplicate re patterns are given, 2006-11-25) and clearly meant for fetch-pack to handle this case gracefully.
quoted hunk ↗ jump to hunk
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh index 3cc3346..0d4edcb 100755 --- a/t/t5500-fetch-pack.sh +++ b/t/t5500-fetch-pack.sh@@ -391,7 +391,7 @@ test_expect_success 'fetch mixed refs from cmdline and stdin' ' test_expect_success 'test duplicate refs from stdin' ' ( cd client && - test_must_fail git fetch-pack --stdin --no-progress .. <../input.dup + git fetch-pack --stdin --no-progress .. <../input.dup ) >output && cut -d " " -f 2 <output | sort >actual && test_cmp expect actual
It's interesting that the output was the same before and after the fix. I guess that is because the error comes at the very end, when we are making sure all of the provided heads have been consumed. -Peff