[PATCH] git-remote: fixed missing .uploadpack usage for show command

[PATCH] git-remote: fixed missing .uploadpack usage for show command

[Chris Frey]

When using 'git remote show <name>', the remote HEAD check
did not use the uploadpack configuration setting.

Signed-off-by: Chris Frey <cdfrey@foursquare.net>
---
 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

Re: [PATCH] git-remote: fixed missing .uploadpack usage for show command

[Junio C Hamano]

Chris Frey <cdfrey@foursquare.net> writes:

> When using 'git remote show <name>', the remote HEAD check
> did not use the uploadpack configuration setting.
>
> Signed-off-by: Chris Frey <cdfrey@foursquare.net>

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."

[PATCH] git-remote: fixed missing .uploadpack usage for show command

[Chris Frey]

For users pulling from machines with self compiled git installs,
in non-PATH locations, they can set the config option
remote.<name>.uploadpack to set the location of git-upload-pack.

When using 'git remote show <name>', 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 <cdfrey@foursquare.net>
---

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

Re: [PATCH] git-remote: fix missing .uploadpack usage for show command

[Junio C Hamano]

Chris Frey <cdfrey@foursquare.net> writes:

> For users pulling from machines with self compiled git installs,
> in non-PATH locations, they can set the config option
> remote.<name>.uploadpack to set the location of git-upload-pack.
>
> When using 'git remote show <name>', 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 <cdfrey@foursquare.net>
> ---
>
> 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?