git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: "brian m. carlson" <sandals@crustytoothpaste.net>,
	"Eric W. Biederman" <ebiederm@gmail.com>
Subject: [PATCH 1/3] transport-helper: use write helpers more consistently
Date: Wed, 20 Mar 2024 05:34:17 -0400	[thread overview]
Message-ID: <20240320093417.GA2445682@coredump.intra.peff.net> (raw)
In-Reply-To: <20240320093226.GA2445531@coredump.intra.peff.net>

The transport-helper code provides some functions for writing to the
helper process, but there are a few spots that don't use them. We should
do so consistently because:

  1. They detect errors on write (though in practice this means the
     helper process went away, and we'd see the problem as soon as we
     try to read the response).

  2. They dump the written bytes to the GIT_TRANSPORT_HELPER_DEBUG
     stream. It's doubly confusing to miss some writes but not others,
     as you see a partial conversation.

The "list" ones go all the way back to the beginning of the transport
helper code; they were just missed when most writes were converted in
bf3c523c3f (Add remote helper debug mode, 2009-12-09). The nearby
"object-format" write presumably just cargo-culted them, as it's only a
few lines away.

Signed-off-by: Jeff King <peff@peff.net>
---
I also find the output kind of verbose (especially the constant
"waiting" lines), and because it's not GIT_TRACE_TRANSPORT_HELPER, it's
annoying to use with the test scripts (it gets eaten by the test
harness, and you can't even redirect it to an alternative file).
So I was tempted to convert it, but it felt like too deep a rabbit hole
for today.

 transport-helper.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/transport-helper.c b/transport-helper.c
index b660b7942f..7f6bbd06bb 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -1211,15 +1211,15 @@ static struct ref *get_refs_list_using_list(struct transport *transport,
 	helper = get_helper(transport);
 
 	if (data->object_format) {
-		write_str_in_full(helper->in, "option object-format\n");
+		write_constant(helper->in, "option object-format\n");
 		if (recvline(data, &buf) || strcmp(buf.buf, "ok"))
 			exit(128);
 	}
 
 	if (data->push && for_push)
-		write_str_in_full(helper->in, "list for-push\n");
+		write_constant(helper->in, "list for-push\n");
 	else
-		write_str_in_full(helper->in, "list\n");
+		write_constant(helper->in, "list\n");
 
 	while (1) {
 		char *eov, *eon;
-- 
2.44.0.650.g4615f65fe0



  reply	other threads:[~2024-03-20  9:34 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-07  8:47 [RFC/PATCH 0/2] some transport-helper "option object-format" confusion Jeff King
2024-03-07  8:51 ` [PATCH 1/2] t5801: fix object-format handling in git-remote-testgit Jeff King
2024-03-07  8:56 ` [PATCH 2/2] doc/gitremote-helpers: match object-format option docs to code Jeff King
2024-03-07 22:20   ` brian m. carlson
2024-03-12  7:45     ` Jeff King
2024-03-13 21:11       ` brian m. carlson
2024-03-14 12:47         ` Eric W. Biederman
2024-03-14 21:21           ` brian m. carlson
2024-03-15 15:41             ` Eric W. Biederman
2024-03-16  6:04               ` Jeff King
2024-03-17 20:47                 ` Eric W. Biederman
2024-03-18  8:49                   ` Jeff King
2024-03-14 15:33         ` Junio C Hamano
2024-03-14 21:54           ` brian m. carlson
2024-03-20  9:32 ` [PATCH 0/3] some transport-helper "option object-format" confusion Jeff King
2024-03-20  9:34   ` Jeff King [this message]
2024-03-20  9:37   ` [PATCH 2/3] transport-helper: drop "object-format <algo>" option Jeff King
2024-03-20  9:41   ` [PATCH 3/3] transport-helper: send "true" value for object-format option Jeff King
2024-03-20 17:23     ` Junio C Hamano
2024-03-20 17:05   ` [PATCH 0/3] some transport-helper "option object-format" confusion Eric W. Biederman
2024-03-27  9:48     ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240320093417.GA2445682@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=ebiederm@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=sandals@crustytoothpaste.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).