Re: [PATCH 2/2] transport-helper: mention helper name when it dies
On Wed, Apr 10, 2013 at 2:28 PM, Jeff King wrote: > Now that's the kind of whole-hearted endorsement I strive for. :) It's nothing wrong with your patch, the main problem is that there's not really a good place to point users at. > If you have better wording, I'm open to it. I do note that we don't > actually have a manpage for "git-remote-https", though we do for others. > Probably "man git-remote-helpers" is the most sensible thing to point > the user to. But I don't even think this is worthy of a big advice > message. It's a bug in the helper, it shouldn't really happen, and > giving the user a token they can use to report or google for the error > is probably good enough. Yeah, exactly. man git-remote-helpers is more a place for developers to read how to implement a git-remote-helper, not so much a place for users to read what they are, and/or how to use them. -- Cheers, Sverre Rabbelier -- 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 2/2] transport-helper: mention helper name when it dies
On Wed, Apr 10, 2013 at 02:23:56PM -0700, Sverre Rabbelier wrote: > On Wed, Apr 10, 2013 at 2:16 PM, Jeff King wrote: > > When we try to read from a remote-helper and get EOF or an > > error, we print a message indicating that the helper died. > > However, users may not know that a remote helper was in use > > (e.g., when using git-over-http), or even what a remote > > helper is. > > > > Let's print the name of the helper (e.g., "git-remote-https"); > > this makes it more obvious what the program is for, and > > provides a useful token for reporting bugs or searching for > > more information (e.g., in manpages). > > > > Signed-off-by: Jeff King > > Better than nothing: > > Acked-by: Sverre Rabbelier Now that's the kind of whole-hearted endorsement I strive for. :) If you have better wording, I'm open to it. I do note that we don't actually have a manpage for "git-remote-https", though we do for others. Probably "man git-remote-helpers" is the most sensible thing to point the user to. But I don't even think this is worthy of a big advice message. It's a bug in the helper, it shouldn't really happen, and giving the user a token they can use to report or google for the error is probably good enough. -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 2/2] transport-helper: mention helper name when it dies
On Wed, Apr 10, 2013 at 2:16 PM, Jeff King wrote: > When we try to read from a remote-helper and get EOF or an > error, we print a message indicating that the helper died. > However, users may not know that a remote helper was in use > (e.g., when using git-over-http), or even what a remote > helper is. > > Let's print the name of the helper (e.g., "git-remote-https"); > this makes it more obvious what the program is for, and > provides a useful token for reporting bugs or searching for > more information (e.g., in manpages). > > Signed-off-by: Jeff King Better than nothing: Acked-by: Sverre Rabbelier -- Cheers, Sverre Rabbelier -- 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
[PATCH 2/2] transport-helper: mention helper name when it dies
When we try to read from a remote-helper and get EOF or an
error, we print a message indicating that the helper died.
However, users may not know that a remote helper was in use
(e.g., when using git-over-http), or even what a remote
helper is.
Let's print the name of the helper (e.g., "git-remote-https");
this makes it more obvious what the program is for, and
provides a useful token for reporting bugs or searching for
more information (e.g., in manpages).
Signed-off-by: Jeff King
---
t/t5801-remote-helpers.sh | 2 +-
transport-helper.c| 8
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index aafc46a..8b2cb68 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -182,7 +182,7 @@ test_expect_success 'proper failure checks for pushing' '
cd local &&
test_must_fail git push --all 2> error &&
cat error &&
- grep -q "Reading from remote helper failed" error
+ grep -q "Reading from helper .git-remote-testgit. failed" error
)
'
diff --git a/transport-helper.c b/transport-helper.c
index 96081cc..3fc43b9 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -46,7 +46,7 @@ static void sendline(struct helper_data *helper, struct
strbuf *buffer)
die_errno("Full write to remote helper failed");
}
-static int recvline_fh(FILE *helper, struct strbuf *buffer)
+static int recvline_fh(FILE *helper, struct strbuf *buffer, const char *name)
{
strbuf_reset(buffer);
if (debug)
@@ -54,7 +54,7 @@ static int recvline_fh(FILE *helper, struct strbuf *buffer)
if (strbuf_getline(buffer, helper, '\n') == EOF) {
if (debug)
fprintf(stderr, "Debug: Remote helper quit.\n");
- die("Reading from remote helper failed");
+ die("Reading from helper 'git-remote-%s' failed", name);
}
if (debug)
@@ -64,7 +64,7 @@ static int recvline(struct helper_data *helper, struct strbuf
*buffer)
static int recvline(struct helper_data *helper, struct strbuf *buffer)
{
- return recvline_fh(helper->out, buffer);
+ return recvline_fh(helper->out, buffer, helper->name);
}
static void xchgline(struct helper_data *helper, struct strbuf *buffer)
@@ -536,7 +536,7 @@ static int process_connect_service(struct transport
*transport,
goto exit;
sendline(data, &cmdbuf);
- recvline_fh(input, &cmdbuf);
+ recvline_fh(input, &cmdbuf, name);
if (!strcmp(cmdbuf.buf, "")) {
data->no_disconnect_req = 1;
if (debug)
--
1.8.2.rc0.33.gd915649
--
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
