[PATCH] fetch: don't try to update unfetched tracking refs
Since commit f269048 (fetch: opportunistically update tracking refs,
2013-05-11) we update tracking refs opportunistically when fetching
remote branches. However, if a refspec is given on the command line
that does not include a configured (non-pattern) refspec a fatal error
occurs.
Fix this by setting the missing_ok flag when calling get_fetch_map.
Signed-off-by: John Keeping j...@keeping.me.uk
---
builtin/fetch.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index e41cc0d..d15a734 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -183,7 +183,7 @@ static struct ref *get_ref_map(struct transport *transport,
old_tail = tail;
for (i = 0; i transport-remote-fetch_refspec_nr; i++)
get_fetch_map(ref_map, transport-remote-fetch[i],
- tail, 0);
+ tail, 1);
for (rm = *old_tail; rm; rm = rm-next)
rm-fetch_head_status = FETCH_HEAD_IGNORE;
} else {
--
1.8.3.rc3.438.gb3e4ae3
--
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
Re: [PATCH] fetch: don't try to update unfetched tracking refs
On Mon, May 27, 2013 at 12:40:25PM +0100, John Keeping wrote: Since commit f269048 (fetch: opportunistically update tracking refs, 2013-05-11) we update tracking refs opportunistically when fetching remote branches. However, if a refspec is given on the command line that does not include a configured (non-pattern) refspec a fatal error occurs. I'm not sure I understand what the last sentence means. I tried to add a test like: diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index ff43e08..02e30e1 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -422,6 +422,16 @@ test_expect_success 'configured fetch updates tracking' ' ) ' +test_expect_success 'non-configured ref does not confuse tracking update' ' + cd $D + git update-ref refs/odd/location HEAD + ( + cd three + git fetch origin refs/odd/location + git rev-parse --verify FETCH_HEAD + ) +' + test_expect_success 'pushing nonexistent branch by mistake should not segv' ' cd $D but it does not fail with the existing code. Can you give an example that fails? -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
Re: [PATCH] fetch: don't try to update unfetched tracking refs
On Mon, May 27, 2013 at 11:42:52AM -0400, Jeff King wrote: On Mon, May 27, 2013 at 12:40:25PM +0100, John Keeping wrote: Since commit f269048 (fetch: opportunistically update tracking refs, 2013-05-11) we update tracking refs opportunistically when fetching remote branches. However, if a refspec is given on the command line that does not include a configured (non-pattern) refspec a fatal error occurs. I'm not sure I understand what the last sentence means. I tried to add a test like: diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index ff43e08..02e30e1 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -422,6 +422,16 @@ test_expect_success 'configured fetch updates tracking' ' ) ' +test_expect_success 'non-configured ref does not confuse tracking update' ' + cd $D + git update-ref refs/odd/location HEAD + ( + cd three + git fetch origin refs/odd/location + git rev-parse --verify FETCH_HEAD + ) +' + test_expect_success 'pushing nonexistent branch by mistake should not segv' ' cd $D 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 Then doing git fetch origin master fails because: fatal: Couldn't find remote ref refs/notes/amlog The following test fails for me (and passes with my patch) - note that in two, remote.one.fetch is configured as refs/heads/master:refs/heads/one. -- 8 -- diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index ff43e08..c540257 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -422,6 +422,19 @@ test_expect_success 'configured fetch updates tracking' ' ) ' +test_expect_success 'configured ref does not confuse tracking' ' + + cd $D + ( + cd one + git branch -f side + ) + ( + cd two + git fetch one side + ) +' + test_expect_success 'pushing nonexistent branch by mistake should not segv' ' cd $D -- 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
Re: [PATCH] fetch: don't try to update unfetched tracking refs
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
