From: "Shawn O. Pearce" <spearce@spearce.org>
To: Junio C Hamano <gitster@pobox.com>,
Daniel Barkalow <barkalow@iabervon.org>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 5/5] Remove unnecessary 'fetch' argument from transport_get API
Date: Sun, 16 Sep 2007 04:11:29 -0400 [thread overview]
Message-ID: <20070916081129.GE3099@spearce.org> (raw)
In-Reply-To: <20070915072314.GE20346@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> wrote:
> We don't actually need to know at the time of transport_get if the
> caller wants to fetch, push, or do both on the returned object.
> It is easier to just delay the initialization of the HTTP walker
> until we know we will need it by providing a CURL specific fetch
> function in the curl_transport that makes sure the walker instance
> is initialized before use.
Daniel privately emailed me his rationale for why this fetch argument
was here in the first place and I mostly agree with him, but will
be working up a more clear API replacement in the near future.
The "1/0","fetch/push" thing is not the clearest way that we could
define the API.
Right now this patch is required to go along with my 1/5 bug fix,
as without this patch we get the sequence:
http_init()
http_init()
... use http ...
http_cleanup()
... try to use http again and barf ...
This 5/5 makes the sequence be in proper order by delaying creation
of the HTTP walker object (and thus one of the http_init() calls)
to after the first http_cleanup(), so we get the nice neat order of:
http_init()
... use http ...
http_cleanup()
http_init()
... use http just fine ...
http_cleanup()
I'll be honest here; I did not test 1/5 or 5/5 on their own.
I only tested the combined result of them, and that creates a
working HTTP fetch. But I'm pretty sure that one of these patches
alone will still cause SIGSEGV/SIGBUS errors during HTTP fetch due
to either the bad cleanup (1/5 fixes) or the bad init ordering I'm
talking about above (5/5 fixes).
Reinstating a replacement for the fetch parameter that I removed
in this patch isn't critical for functionality, but will be
necessary to do performance optimization in the form of reusing
the connection between ref discovery and pack transfer in the
native protocol. Right now I'm focusing on making builtin-fetch
stable and implementing prior behavior. Once its solid enough to
graduate to `next` we can start doing some of the optimization work.
--
Shawn.
prev parent reply other threads:[~2007-09-16 8:11 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-09-15 7:23 [PATCH 5/5] Remove unnecessary 'fetch' argument from transport_get API Shawn O. Pearce
2007-09-16 8:11 ` Shawn O. Pearce [this message]
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20070916081129.GE3099@spearce.org \
--to=spearce@spearce.org \
--cc=barkalow@iabervon.org \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/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.
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 NNTP newsgroup(s).