# [PATCH] git-remote: fixed missing .uploadpack usage for show command 4 messages from 2009-06-25 to 2009-06-25. Participants: Chris Frey, Junio C Hamano. Thread: https://gitlist.dev/t/19924 ## Chris Frey, 2009-06-25 09:00 Subject: [PATCH] git-remote: fixed missing .uploadpack usage for show command Message-ID: <20090625090036.GA32650@foursquare.net> URL: https://gitlist.dev/e/20090625090036.GA32650%40foursquare.net ``` When using 'git remote show ', the remote HEAD check did not use the uploadpack configuration setting. Signed-off-by: Chris Frey --- builtin-remote.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/builtin-remote.c b/builtin-remote.c index 658d578..ec1f903 100644 --- a/builtin-remote.c +++ b/builtin-remote.c @@ -787,7 +787,7 @@ static int get_remote_ref_states(const char *name, read_branches(); if (query) { - transport = transport_get(NULL, states->remote->url_nr > 0 ? + transport = transport_get(states->remote, states->remote->url_nr > 0 ? states->remote->url[0] : NULL); remote_refs = transport_get_remote_refs(transport); transport_disconnect(transport); -- 1.6.2.5 ``` ## Junio C Hamano, 2009-06-25 18:32 Subject: Re: [PATCH] git-remote: fixed missing .uploadpack usage for show command Message-ID: <7vmy7wcgge.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vmy7wcgge.fsf%40alter.siamese.dyndns.org In-Reply-To: <20090625090036.GA32650@foursquare.net> ``` Chris Frey writes: > When using 'git remote show ', the remote HEAD check > did not use the uploadpack configuration setting. > > Signed-off-by: Chris Frey Thanks. "X did not use Y" may be a good statement of the fact. From the patch text it can be seen that a NULL used to be passed and the patch makes it to pass states->remote instead, so "This patch make X use Y", even though left unsaid in the message, can be seen. But it does not answer a more important question. How was it a problem that "X did not use Y"? People who followed a recent discussion know the answer to this question, but ones who read this in the "git log" output 6 months down the line will not. Please make a habit of justifying the change by stating "why". "X should have used Y because of such and such reasons, but it didn't. Instead of showing the correct result W, it gave Z, which may happen to be the same as W in default settings but otherwise is wrong." ``` ## Chris Frey, 2009-06-25 21:21 Subject: [PATCH] git-remote: fixed missing .uploadpack usage for show command Message-ID: <20090625212135.GA28935@foursquare.net> URL: https://gitlist.dev/e/20090625212135.GA28935%40foursquare.net In-Reply-To: <7vmy7wcgge.fsf@alter.siamese.dyndns.org> ``` For users pulling from machines with self compiled git installs, in non-PATH locations, they can set the config option remote..uploadpack to set the location of git-upload-pack. When using 'git remote show ', the remote HEAD check did not use the uploadpack configuration setting, and would stall. In builtin-remote.c, the config setting is already loaded with the call to remote_get(), so this patch passes that remote along to transport_get(). Signed-off-by: Chris Frey --- A possibly clearer description... builtin-remote.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/builtin-remote.c b/builtin-remote.c index 658d578..ec1f903 100644 --- a/builtin-remote.c +++ b/builtin-remote.c @@ -787,7 +787,7 @@ static int get_remote_ref_states(const char *name, read_branches(); if (query) { - transport = transport_get(NULL, states->remote->url_nr > 0 ? + transport = transport_get(states->remote, states->remote->url_nr > 0 ? states->remote->url[0] : NULL); remote_refs = transport_get_remote_refs(transport); transport_disconnect(transport); -- 1.6.2.5 ``` ## Junio C Hamano, 2009-06-25 21:48 Subject: Re: [PATCH] git-remote: fix missing .uploadpack usage for show command Message-ID: <7vd48s2ddr.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vd48s2ddr.fsf%40alter.siamese.dyndns.org In-Reply-To: <20090625212135.GA28935@foursquare.net> ``` Chris Frey writes: > For users pulling from machines with self compiled git installs, > in non-PATH locations, they can set the config option > remote..uploadpack to set the location of git-upload-pack. > > When using 'git remote show ', the remote HEAD check > did not use the uploadpack configuration setting, and would > not use the configured program. > > In builtin-remote.c, the config setting is already loaded > with the call to remote_get(), so this patch passes that remote > along to transport_get(). > > Signed-off-by: Chris Frey > --- > > A possibly clearer description... Thanks, much clearer. Will queue, aiming to eventually merge to 'maint'. Do you have tests to protect this fix from getting broken in the future by other people? ```