From: Jeff King <peff@peff.net> To: Jonathan Tan <jonathantanmy@google.com> Cc: gitster@pobox.com, git@vger.kernel.org Subject: Re: [RFC PATCH] Modify fetch-pack to no longer die on error? Date: Tue, 28 Jul 2020 16:08:47 -0400 [thread overview] Message-ID: <20200728200847.GA1019822@coredump.intra.peff.net> (raw) In-Reply-To: <20200728192350.352978-1-jonathantanmy@google.com> On Tue, Jul 28, 2020 at 12:23:50PM -0700, Jonathan Tan wrote: > > For this particular case, with the performance and all, I agree that > > the stupid and robust approach would be the best. > > I'm concerned that we will be painting ourselves into a corner here - I > have been appreciating the richer interface that a C call provides us, > compared to sub-processes where we have to communicate through a single > input stream and a single output stream. For example, "wanted-refs" and > [...] Yeah, that's a compelling reason. I'd have thought for this use case you could just say "hey, make sure these objects exist", which doesn't require a lot of communication. But often when I think things like that and start coding, it turns out to be much more complicated. So I am perfectly willing to believe you that it doesn't apply here. And even if it did, you're right that we may run into other spots that do need to pass back more information, but benefit from more lib-ified code that doesn't die(). Just to play devil's advocate for a moment... > (Also, I think that debugging within a process is easier than debugging > across processes, but that might not be a concern that other people > share.) This is definitely true sometimes, but I think is sometimes the opposite. When we push things out to a sub-process, then the interface between the two processes has to be well-defined (e.g., writing results to a file with a particular format). And that can make debugging easier, because you can pick up from that intermediate state (munging it in the middle, or even generating it from scratch for testing). Likewise, that can result in a more flexible and robust system from the perspective of users. If we had invented "git log" first, we probably wouldn't have "git rev-list | git diff-tree --stdin" at all. But having that as two separate tools is sometimes useful for people doing things _besides_ log, since it gives different entry points to the code. That said, I think I could buy the argument that "fetch" works pretty well as a basic building block for users. It's pretty rare to actually use fetch-pack as a distinct operation. This is all a monolith vs module tradeoff question, and the tradeoff around modularity for fetch isn't that compelling. -Peff
next prev parent reply other threads:[~2020-07-28 20:09 UTC|newest] Thread overview: 57+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-07-24 22:38 Jonathan Tan 2020-07-24 23:07 ` Junio C Hamano 2020-07-24 23:11 ` Junio C Hamano 2020-07-25 21:41 ` Jeff King 2020-07-25 23:01 ` Junio C Hamano 2020-07-27 17:11 ` Jeff King 2020-07-28 19:23 ` Jonathan Tan 2020-07-28 20:08 ` Jeff King [this message] 2020-07-29 18:53 ` Jonathan Tan 2020-07-29 19:29 ` Jeff King 2020-07-29 19:02 ` Junio C Hamano 2020-07-29 22:55 ` Jonathan Tan 2020-08-05 1:20 ` [RFC PATCH 0/7] Lazy fetch with subprocess Jonathan Tan 2020-08-05 1:20 ` [RFC PATCH 1/7] fetch-pack: allow NULL negotiator->add_tip Jonathan Tan 2020-08-05 19:53 ` Junio C Hamano 2020-08-07 20:53 ` Jonathan Tan 2020-08-05 1:20 ` [RFC PATCH 2/7] fetch-pack: allow NULL negotiator->known_common Jonathan Tan 2020-08-05 20:08 ` Junio C Hamano 2020-08-05 22:11 ` Junio C Hamano 2020-08-07 20:59 ` Jonathan Tan 2020-08-05 1:20 ` [RFC PATCH 3/7] negotiator/null: add null fetch negotiator Jonathan Tan 2020-08-05 1:20 ` [RFC PATCH 4/7] fetch: --stdin Jonathan Tan 2020-08-05 20:33 ` Junio C Hamano 2020-08-07 21:10 ` Jonathan Tan 2020-08-07 21:58 ` Junio C Hamano 2020-08-07 21:10 ` Jonathan Tan 2020-08-05 1:20 ` [RFC PATCH 5/7] fetch: submodule config Jonathan Tan 2020-08-05 1:20 ` [RFC PATCH 6/7] fetch: only populate existing_refs if needed Jonathan Tan 2020-08-05 1:20 ` [RFC PATCH 7/7] promisor-remote: use subprocess to fetch Jonathan Tan 2020-08-11 22:52 ` [PATCH v2 0/7] Lazy fetch with subprocess Jonathan Tan 2020-08-11 22:52 ` [PATCH v2 1/7] negotiator/null: add null fetch negotiator Jonathan Tan 2020-08-12 12:55 ` Derrick Stolee 2020-08-12 16:44 ` Junio C Hamano 2020-08-12 17:29 ` Jonathan Tan 2020-08-11 22:52 ` [PATCH v2 2/7] fetch: allow refspecs specified through stdin Jonathan Tan 2020-08-11 22:52 ` [PATCH v2 3/7] fetch: avoid reading submodule config until needed Jonathan Tan 2020-08-12 17:34 ` Junio C Hamano 2020-08-11 22:52 ` [PATCH v2 4/7] fetch: only populate existing_refs if needed Jonathan Tan 2020-08-12 18:06 ` Junio C Hamano 2020-08-11 22:52 ` [PATCH v2 5/7] fetch-pack: do not lazy-fetch during ref iteration Jonathan Tan 2020-08-12 18:25 ` Junio C Hamano 2020-08-11 22:52 ` [PATCH v2 6/7] promisor-remote: lazy-fetch objects in subprocess Jonathan Tan 2020-08-12 18:28 ` Junio C Hamano 2020-08-11 22:52 ` [PATCH v2 7/7] fetch-pack: remove no_dependents code Jonathan Tan 2020-08-12 12:51 ` [PATCH v2 0/7] Lazy fetch with subprocess Derrick Stolee 2020-08-18 4:01 ` [PATCH v3 " Jonathan Tan 2020-08-18 4:01 ` [PATCH v3 1/7] negotiator/noop: add noop fetch negotiator Jonathan Tan 2020-08-18 4:01 ` [PATCH v3 2/7] fetch: allow refspecs specified through stdin Jonathan Tan 2020-08-18 4:01 ` [PATCH v3 3/7] fetch: avoid reading submodule config until needed Jonathan Tan 2020-08-18 4:01 ` [PATCH v3 4/7] fetch: only populate existing_refs if needed Jonathan Tan 2020-08-18 4:01 ` [PATCH v3 5/7] fetch-pack: do not lazy-fetch during ref iteration Jonathan Tan 2020-08-18 4:01 ` [PATCH v3 6/7] promisor-remote: lazy-fetch objects in subprocess Jonathan Tan 2020-08-18 4:01 ` [PATCH v3 7/7] fetch-pack: remove no_dependents code Jonathan Tan 2020-08-18 19:56 ` [PATCH v3 0/7] Lazy fetch with subprocess Junio C Hamano 2020-08-18 22:32 ` Junio C Hamano 2020-08-18 23:36 ` [PATCH] fixup! promisor-remote: lazy-fetch objects in subprocess Jonathan Tan 2020-08-18 23:57 ` Junio C Hamano
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=20200728200847.GA1019822@coredump.intra.peff.net \ --to=peff@peff.net \ --cc=git@vger.kernel.org \ --cc=gitster@pobox.com \ --cc=jonathantanmy@google.com \ --subject='Re: [RFC PATCH] Modify fetch-pack to no longer die on error?' \ /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
Code repositories for project(s) associated with this 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).