@matheusbsbs / matheus-bagliano / commits / 3198237bf3

connect: use "service" enum for "name" argument

The git_connect() function takes a "name" argument which is a bit confusing. It is _not_ the program to run on the remote repo, which is specified by the "prog" argument. It should instead be one of a few well-known strings specifying the type of operation (e.g., "git-upload-pack"). But to add to the confusion, unless otherwise configured, those well-known strings will also be the same as the programs we run, making it easy to mistake which variable is which. This confusion comes from eaa0fd6584 (git_connect(): fix corner cases in downgrading v2 to v0, 2023-03-17), though in its defense, the term "name" and the use of a string are found in other connect code, going all the way back to b236752a87 (Support remote archive from all smart transports, 2009-12-09). But let's see if we can clean things up a bit. The term "name" is overly vague. We use "service" in other places, including in the smart-http protocol, so let's use it here, too. Using a string invites the notion that it can be anything, not one of a defined set. Let's instead introduce an enum, which has the added bonus that the compiler can catch typos for us, rather than quietly choosing the wrong service from an unexpected strcmp() result. We do still have to turn our enum into those well-known strings to pass along in the remote-helper protocol (e.g., for a stateless-connect directive). But now we do so explicitly and in a way that I think is much more obvious to follow. This is a pure cleanup; there should be no behavior change. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed May 19, 2026 at 01:22 UTC 3198237bf3ce3c1eae845ac8e13a2da813800c77

9 files changed +58 -28

builtin/archive.c

+1 -1

@@ -31,7 +31,7 @@ static int run_remote_archiver(int argc, const char **argv,

32 _remote = remote_get(remote);

33 transport = transport_get(_remote, _remote->url.v[0]);

34 - transport_connect(transport, "git-upload-archive", exec, fd);

34 + transport_connect(transport, GIT_CONNECT_UPLOAD_ARCHIVE, exec, fd);

36 /*

37 * Inject a fake --format field at the beginning of the

builtin/fetch-pack.c

+1 -1

@@ -223,7 +223,7 @@ int cmd_fetch_pack(int argc,

223 int flags = args.verbose ? CONNECT_VERBOSE : 0;

224 if (args.diag_url)

225 flags |= CONNECT_DIAG_URL;

226 - conn = git_connect(fd, dest, "git-upload-pack",

226 + conn = git_connect(fd, dest, GIT_CONNECT_UPLOAD_PACK,

227 args.uploadpack, flags);

228 if (!conn)

229 return args.diag_url ? 0 : 1;

builtin/send-pack.c

+3 -2

@@ -273,8 +273,9 @@ int cmd_send_pack(int argc,

273 fd[0] = 0;

274 fd[1] = 1;

275 } else {

276 - conn = git_connect(fd, dest, "git-receive-pack", receivepack,

277 - args.verbose ? CONNECT_VERBOSE : 0);

276 + conn = git_connect(fd, dest, GIT_CONNECT_RECEIVE_PACK,

277 + receivepack,

278 + args.verbose ? CONNECT_VERBOSE : 0);

279 }

280

281 packet_reader_init(&reader, fd[0], NULL, 0,

connect.c

+2 -2

@@ -1429,7 +1429,7 @@ static void fill_ssh_args(struct child_process *conn, const char *ssh_host,

1429 * the connection failed).

1430 */

1431 struct child_process *git_connect(int fd[2], const char *url,

1432 - const char *name,

1432 + enum git_connect_service service,

1433 const char *prog, int flags)

1434 {

1435 char *hostandport, *path;

@@ -1443,7 +1443,7 @@ struct child_process *git_connect(int fd[2], const char *url,

1443 * fetch, ls-remote, etc), then fallback to v0 since we don't know how

1444 * to do anything else (like push or remote archive) via v2.

1445 */

1446 - if (version == protocol_v2 && strcmp("git-upload-pack", name))

1446 + if (version == protocol_v2 && service != GIT_CONNECT_UPLOAD_PACK)

1447 version = protocol_v0;

1448

1449 /* Without this we cannot rely on waitpid() to tell

connect.h

+6 -1

@@ -7,7 +7,12 @@

7 #define CONNECT_DIAG_URL (1u << 1)

8 #define CONNECT_IPV4 (1u << 2)

9 #define CONNECT_IPV6 (1u << 3)

10 -struct child_process *git_connect(int fd[2], const char *url, const char *name, const char *prog, int flags);

10 +enum git_connect_service {

11 + GIT_CONNECT_UPLOAD_PACK,

12 + GIT_CONNECT_RECEIVE_PACK,

13 + GIT_CONNECT_UPLOAD_ARCHIVE,

14 +};

15 +struct child_process *git_connect(int fd[2], const char *url, enum git_connect_service, const char *prog, int flags);

16 int finish_connect(struct child_process *conn);

17 int git_connection_is_socket(struct child_process *conn);

18 int server_supports(const char *feature);

transport-helper.c

+30 -13

@@ -622,8 +622,22 @@ static int run_connect(struct transport *transport, struct strbuf *cmdbuf)

622 return ret;

623 }

624

625 +static const char *connect_service_cmd(enum git_connect_service service)

626 +{

627 + switch (service) {

628 + case GIT_CONNECT_UPLOAD_PACK:

629 + return "git-upload-pack";

630 + case GIT_CONNECT_RECEIVE_PACK:

631 + return "git-receive-pack";

632 + case GIT_CONNECT_UPLOAD_ARCHIVE:

633 + return "git-upload-archive";

634 + }

635 + BUG("unknown git_connect_type: %d", service);

636 +}

637 +

638 static int process_connect_service(struct transport *transport,

626 - const char *name, const char *exec)

639 + enum git_connect_service service,

640 + const char *exec)

641 {

642 struct helper_data *data = transport->data;

643 struct strbuf cmdbuf = STRBUF_INIT;

@@ -633,7 +647,7 @@ static int process_connect_service(struct transport *transport,

647 * Handle --upload-pack and friends. This is fire and forget...

648 * just warn if it fails.

649 */

636 - if (strcmp(name, exec)) {

650 + if (strcmp(connect_service_cmd(service), exec)) {

651 int r = set_helper_option(transport, "servpath", exec);

652 if (r > 0)

653 warning(_("setting remote service path not supported by protocol"));

@@ -642,13 +656,15 @@ static int process_connect_service(struct transport *transport,

656 }

657

658 if (data->connect) {

645 - strbuf_addf(&cmdbuf, "connect %s\n", name);

659 + strbuf_addf(&cmdbuf, "connect %s\n",

660 + connect_service_cmd(service));

661 ret = run_connect(transport, &cmdbuf);

662 } else if (data->stateless_connect &&

663 (get_protocol_version_config() == protocol_v2) &&

649 - (!strcmp("git-upload-pack", name) ||

650 - !strcmp("git-upload-archive", name))) {

651 - strbuf_addf(&cmdbuf, "stateless-connect %s\n", name);

664 + (service == GIT_CONNECT_UPLOAD_PACK ||

665 + service == GIT_CONNECT_UPLOAD_ARCHIVE)) {

666 + strbuf_addf(&cmdbuf, "stateless-connect %s\n",

667 + connect_service_cmd(service));

668 ret = run_connect(transport, &cmdbuf);

669 if (ret)

670 transport->stateless_rpc = 1;

@@ -662,32 +678,33 @@ static int process_connect(struct transport *transport,

678 int for_push)

679 {

680 struct helper_data *data = transport->data;

665 - const char *name;

681 + enum git_connect_service service;

682 const char *exec;

683 int ret;

684

669 - name = for_push ? "git-receive-pack" : "git-upload-pack";

685 + service = for_push ? GIT_CONNECT_RECEIVE_PACK : GIT_CONNECT_UPLOAD_PACK;

686 if (for_push)

687 exec = data->transport_options.receivepack;

688 else

689 exec = data->transport_options.uploadpack;

690

675 - ret = process_connect_service(transport, name, exec);

691 + ret = process_connect_service(transport, service, exec);

692 if (ret)

693 do_take_over(transport);

694 return ret;

695 }

696

681 -static int connect_helper(struct transport *transport, const char *name,

682 - const char *exec, int fd[2])

697 +static int connect_helper(struct transport *transport, enum git_connect_service service,

698 + const char *exec, int fd[2])

699 {

700 struct helper_data *data = transport->data;

701

702 /* Get_helper so connect is inited. */

703 get_helper(transport);

704

689 - if (!process_connect_service(transport, name, exec))

690 - die(_("can't connect to subservice %s"), name);

705 + if (!process_connect_service(transport, service, exec))

706 + die(_("can't connect to subservice %s"),

707 + connect_service_cmd(service));

708

709 fd[0] = data->helper->out;

710 fd[1] = data->helper->in;

transport-internal.h

+4 -1

@@ -1,6 +1,8 @@

1 #ifndef TRANSPORT_INTERNAL_H

2 #define TRANSPORT_INTERNAL_H

4 +#include "connect.h"

5 +

6 struct ref;

7 struct transport;

8 struct strvec;

@@ -58,7 +60,8 @@ struct transport_vtable {

60 * process involved generating new commits.

61 **/

62 int (*push_refs)(struct transport *transport, struct ref *refs, int flags);

61 - int (*connect)(struct transport *connection, const char *name,

63 + int (*connect)(struct transport *connection,

64 + enum git_connect_service service,

65 const char *executable, int fd[2]);

67 /** get_refs_list(), fetch(), and push_refs() can keep

transport.c

+8 -6

@@ -308,8 +308,8 @@ static int connect_setup(struct transport *transport, int for_push)

308

309 data->conn = git_connect(data->fd, transport->url,

310 for_push ?

311 - "git-receive-pack" :

312 - "git-upload-pack",

311 + GIT_CONNECT_RECEIVE_PACK :

312 + GIT_CONNECT_UPLOAD_PACK,

313 for_push ?

314 data->options.receivepack :

315 data->options.uploadpack,

@@ -956,12 +956,13 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re

956 return ret;

957 }

958

959 -static int connect_git(struct transport *transport, const char *name,

959 +static int connect_git(struct transport *transport,

960 + enum git_connect_service service,

961 const char *executable, int fd[2])

962 {

963 struct git_transport_data *data = transport->data;

964 data->conn = git_connect(data->fd, transport->url,

964 - name, executable, 0);

965 + service, executable, 0);

966 fd[0] = data->fd[0];

967 fd[1] = data->fd[1];

968 return 0;

@@ -1651,11 +1652,12 @@ void transport_unlock_pack(struct transport *transport, unsigned int flags)

1652 string_list_clear(&transport->pack_lockfiles, 0);

1653 }

1654

1654 -int transport_connect(struct transport *transport, const char *name,

1655 +int transport_connect(struct transport *transport,

1656 + enum git_connect_service service,

1657 const char *exec, int fd[2])

1658 {

1659 if (transport->vtable->connect)

1658 - return transport->vtable->connect(transport, name, exec, fd);

1660 + return transport->vtable->connect(transport, service, exec, fd);

1661 else

1662 die(_("operation not supported by protocol"));

1663 }

transport.h

+3 -1

@@ -5,6 +5,7 @@

5 #include "remote.h"

6 #include "list-objects-filter-options.h"

7 #include "string-list.h"

8 +#include "connect.h"

10 struct git_transport_options {

11 unsigned thin : 1;

@@ -324,7 +325,8 @@ char *transport_anonymize_url(const char *url);

325 void transport_take_over(struct transport *transport,

326 struct child_process *child);

327

327 -int transport_connect(struct transport *transport, const char *name,

328 +int transport_connect(struct transport *transport,

329 + enum git_connect_service service,

330 const char *exec, int fd[2]);

331

332 /* Transport methods defined outside transport.c */