On Mon, May 27, 2013 at 05:01:29PM +0100, John Keeping wrote: > > I'm not sure I understand what the last sentence means. I tried to add a > > test like: > [...] > > but it does not fail with the existing code. Can you give an example > > that fails? > > I have this in my .git/config for git.git: > > [remote "origin"] > url = git://github.com/gitster/git > fetch = +refs/heads/*:refs/remotes/origin/* > fetch = +refs/notes/amlog:refs/notes/amlog
Ah, I see. It is not "the refspec on the command-line does not match a configured refspec", but rather "there exists a configured non-pattern refspec that does not match what was on the command-line (even if what was on the command-line did match another refspec)". So your fix makes perfect sense. Do you mind squashing in this test below? I think it is a little less subtle than what you posted, as it sets up the situation explicitly in the test. It also checks that the refs we _did_ match still get updated ("master" in this case). diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index ff43e08..fde6891 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -422,6 +422,22 @@ test_expect_success 'configured fetch updates tracking' ' ) ' +test_expect_success 'non-matching refspecs do not confuse tracking update' ' + cd "$D" && + git update-ref refs/odd/location HEAD && + ( + cd three && + git update-ref refs/remotes/origin/master base-origin-master && + git config --add remote.origin.fetch \ + refs/odd/location:refs/remotes/origin/odd && + o=$(git rev-parse --verify refs/remotes/origin/master) && + git fetch origin master && + n=$(git rev-parse --verify refs/remotes/origin/master) && + test "$o" != "$n" && + test_must_fail git rev-parse --verify refs/remotes/origin/odd + ) +' + test_expect_success 'pushing nonexistent branch by mistake should not segv' ' cd "$D" && Thanks for the fix. Acked-by: Jeff King <p...@peff.net> -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html