On 08/23/2012 10:54 AM, Jeff King wrote: > On Thu, Aug 23, 2012 at 10:10:31AM +0200, mhagger@xxxxxxxxxxxx wrote: > >> From: Michael Haggerty <mhagger@xxxxxxxxxxxx> >> >> 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. > >> 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. "git fetch-pack" emits information about successfully-received references regardless of whether some requested references were not received. The "no such remote ref %s" output goes to stderr. So the only difference between before/after fix should be what is written to stderr, whereas the test only looks at stdout. Michael -- Michael Haggerty mhagger@xxxxxxxxxxxx http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html