| Message ID | 20201013191729.2524700-2-smcallis@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/3] remote-curl: add testing for intelligent retry for HTTP | expand |
Sean McAllister <smcallis@google.com> writes: > CURLOPT_FILE has been deprecated since 2003. I thought that Dscho already mention this, but updating the above description to mention that _WRITEDATA was introduce to overtake _FILE as an equivalent in the same timeframe would be more helpful to readers. > Signed-off-by: Sean McAllister <smcallis@google.com> > --- > http-push.c | 6 +++--- > http-walker.c | 2 +- > http.c | 6 +++--- > remote-curl.c | 4 ++-- > 4 files changed, 9 insertions(+), 9 deletions(-) > > diff --git a/http-push.c b/http-push.c > index 6a4a43e07f..2e6fee3305 100644 > --- a/http-push.c > +++ b/http-push.c > @@ -894,7 +894,7 @@ static struct remote_lock *lock_remote(const char *path, long timeout) > slot->results = &results; > curl_setup_http(slot->curl, url, DAV_LOCK, &out_buffer, fwrite_buffer); > curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, dav_headers); > - curl_easy_setopt(slot->curl, CURLOPT_FILE, &in_buffer); > + curl_easy_setopt(slot->curl, CURLOPT_WRITEDATA, &in_buffer); > ...
On Tue, Oct 13, 2020 at 01:46:07PM -0700, Junio C Hamano wrote: > Sean McAllister <smcallis@google.com> writes: > > > CURLOPT_FILE has been deprecated since 2003. > > I thought that Dscho already mention this, but updating the above > description to mention that _WRITEDATA was introduce to overtake > _FILE as an equivalent in the same timeframe would be more helpful > to readers. Yes. But more important: - when is _FILE going away (or has it already in some versions)? - when did _WRITEDATA appear? IOW, as a reviewer I would want to make sure that we are not losing support for any reasonable version of libcurl, or that we are at least getting something in return (fixing an incompatibility with newer versions). From the link Dscho dug up it looks like the answer to the second one is "long enough not to care", but the commit message should make that plain. -Peff
On Tue, 13 Oct 2020, Jeff King wrote: Let me just inject myself here and comment on two curl things. > - when is _FILE going away (or has it already in some versions)? It will be kept around *at least* for as long as libcurl supports the version 7 API: for the forseeable future. Posssibly decades. > - when did _WRITEDATA appear? All curl symbols ever introduced are documented clearly in which version they appeared. See: https://curl.haxx.se/libcurl/c/symbols-in-versions.html To map the version numbers to release dates, this second table comes handy: https://curl.haxx.se/docs/releases.html CURLOPT_WRITEDATA was added in curl 7.9.7, shipped on May 10 2002. In the curl project we often see users use *very* old versions but 18 years is longer than even the most conservative users I've seen...
> > Sean McAllister <smcallis@google.com> writes: > > > CURLOPT_FILE has been deprecated since 2003. > > I thought that Dscho already mention this, but updating the above > description to mention that _WRITEDATA was introduce to overtake > _FILE as an equivalent in the same timeframe would be more helpful > to readers. > > Yes he did, I'll update the commit message to give more history on the change and why it's safe. > > Signed-off-by: Sean McAllister <smcallis@google.com> > > --- > > http-push.c | 6 +++--- > > http-walker.c | 2 +- > > http.c | 6 +++--- > > remote-curl.c | 4 ++-- > > 4 files changed, 9 insertions(+), 9 deletions(-) > > > > diff --git a/http-push.c b/http-push.c > > index 6a4a43e07f..2e6fee3305 100644 > > --- a/http-push.c > > +++ b/http-push.c > > @@ -894,7 +894,7 @@ static struct remote_lock *lock_remote(const char *path, long timeout) > > slot->results = &results; > > curl_setup_http(slot->curl, url, DAV_LOCK, &out_buffer, fwrite_buffer); > > curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, dav_headers); > > - curl_easy_setopt(slot->curl, CURLOPT_FILE, &in_buffer); > > + curl_easy_setopt(slot->curl, CURLOPT_WRITEDATA, &in_buffer); > > ...
On Tue, Oct 13, 2020 at 11:16:58PM +0200, Daniel Stenberg wrote: > Let me just inject myself here and comment on two curl things. > > > - when is _FILE going away (or has it already in some versions)? > > It will be kept around *at least* for as long as libcurl supports the > version 7 API: for the forseeable future. Posssibly decades. Thanks. That's exactly the level of carefulness I expected from libcurl. :) So this definitely isn't an urgent change, but given the date that WRITEDATA was introduced, there's very little downside to using it. So the argument for the commit message is mostly that it's a readability improvement. -Peff
> > Sean McAllister <smcallis@google.com> writes: > > > CURLOPT_FILE has been deprecated since 2003. > > I thought that Dscho already mention this, but updating the above > description to mention that _WRITEDATA was introduce to overtake > _FILE as an equivalent in the same timeframe would be more helpful > to readers. > > I've updated the commit message in v3 to explain more about the history and details here. > > Signed-off-by: Sean McAllister <smcallis@google.com> > > --- > > http-push.c | 6 +++--- > > http-walker.c | 2 +- > > http.c | 6 +++--- > > remote-curl.c | 4 ++-- > > 4 files changed, 9 insertions(+), 9 deletions(-) > > > > diff --git a/http-push.c b/http-push.c > > index 6a4a43e07f..2e6fee3305 100644 > > --- a/http-push.c > > +++ b/http-push.c > > @@ -894,7 +894,7 @@ static struct remote_lock *lock_remote(const char *path, long timeout) > > slot->results = &results; > > curl_setup_http(slot->curl, url, DAV_LOCK, &out_buffer, fwrite_buffer); > > curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, dav_headers); > > - curl_easy_setopt(slot->curl, CURLOPT_FILE, &in_buffer); > > + curl_easy_setopt(slot->curl, CURLOPT_WRITEDATA, &in_buffer); > > ...
diff --git a/http-push.c b/http-push.c index 6a4a43e07f..2e6fee3305 100644 --- a/http-push.c +++ b/http-push.c @@ -894,7 +894,7 @@ static struct remote_lock *lock_remote(const char *path, long timeout) slot->results = &results; curl_setup_http(slot->curl, url, DAV_LOCK, &out_buffer, fwrite_buffer); curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, dav_headers); - curl_easy_setopt(slot->curl, CURLOPT_FILE, &in_buffer); + curl_easy_setopt(slot->curl, CURLOPT_WRITEDATA, &in_buffer); lock = xcalloc(1, sizeof(*lock)); lock->timeout = -1; @@ -1151,7 +1151,7 @@ static void remote_ls(const char *path, int flags, curl_setup_http(slot->curl, url, DAV_PROPFIND, &out_buffer, fwrite_buffer); curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, dav_headers); - curl_easy_setopt(slot->curl, CURLOPT_FILE, &in_buffer); + curl_easy_setopt(slot->curl, CURLOPT_WRITEDATA, &in_buffer); if (start_active_slot(slot)) { run_active_slot(slot); @@ -1225,7 +1225,7 @@ static int locking_available(void) curl_setup_http(slot->curl, repo->url, DAV_PROPFIND, &out_buffer, fwrite_buffer); curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, dav_headers); - curl_easy_setopt(slot->curl, CURLOPT_FILE, &in_buffer); + curl_easy_setopt(slot->curl, CURLOPT_WRITEDATA, &in_buffer); if (start_active_slot(slot)) { run_active_slot(slot); diff --git a/http-walker.c b/http-walker.c index 4fb1235cd4..6c630711d1 100644 --- a/http-walker.c +++ b/http-walker.c @@ -384,7 +384,7 @@ static void fetch_alternates(struct walker *walker, const char *base) alt_req.walker = walker; slot->callback_data = &alt_req; - curl_easy_setopt(slot->curl, CURLOPT_FILE, &buffer); + curl_easy_setopt(slot->curl, CURLOPT_WRITEDATA, &buffer); curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer); curl_easy_setopt(slot->curl, CURLOPT_URL, url.buf); diff --git a/http.c b/http.c index 8b23a546af..b3c1669388 100644 --- a/http.c +++ b/http.c @@ -1921,7 +1921,7 @@ static int http_request(const char *url, curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1); } else { curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0); - curl_easy_setopt(slot->curl, CURLOPT_FILE, result); + curl_easy_setopt(slot->curl, CURLOPT_WRITEDATA, result); if (target == HTTP_REQUEST_FILE) { off_t posn = ftello(result); @@ -2337,7 +2337,7 @@ struct http_pack_request *new_direct_http_pack_request( } preq->slot = get_active_slot(); - curl_easy_setopt(preq->slot->curl, CURLOPT_FILE, preq->packfile); + curl_easy_setopt(preq->slot->curl, CURLOPT_WRITEDATA, preq->packfile); curl_easy_setopt(preq->slot->curl, CURLOPT_WRITEFUNCTION, fwrite); curl_easy_setopt(preq->slot->curl, CURLOPT_URL, preq->url); curl_easy_setopt(preq->slot->curl, CURLOPT_HTTPHEADER, @@ -2508,7 +2508,7 @@ struct http_object_request *new_http_object_request(const char *base_url, freq->slot = get_active_slot(); - curl_easy_setopt(freq->slot->curl, CURLOPT_FILE, freq); + curl_easy_setopt(freq->slot->curl, CURLOPT_WRITEDATA, freq); curl_easy_setopt(freq->slot->curl, CURLOPT_FAILONERROR, 0); curl_easy_setopt(freq->slot->curl, CURLOPT_WRITEFUNCTION, fwrite_sha1_file); curl_easy_setopt(freq->slot->curl, CURLOPT_ERRORBUFFER, freq->errorstr); diff --git a/remote-curl.c b/remote-curl.c index 32cc4a0c55..7f44fa30fe 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -847,7 +847,7 @@ static int probe_rpc(struct rpc_state *rpc, struct slot_results *results) curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE, 4); curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, headers); curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer); - curl_easy_setopt(slot->curl, CURLOPT_FILE, &buf); + curl_easy_setopt(slot->curl, CURLOPT_WRITEDATA, &buf); err = run_slot(slot, results); @@ -1012,7 +1012,7 @@ static int post_rpc(struct rpc_state *rpc, int stateless_connect, int flush_rece rpc_in_data.slot = slot; rpc_in_data.check_pktline = stateless_connect; memset(&rpc_in_data.pktline_state, 0, sizeof(rpc_in_data.pktline_state)); - curl_easy_setopt(slot->curl, CURLOPT_FILE, &rpc_in_data); + curl_easy_setopt(slot->curl, CURLOPT_WRITEDATA, &rpc_in_data); curl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 0);
CURLOPT_FILE has been deprecated since 2003. Signed-off-by: Sean McAllister <smcallis@google.com> --- http-push.c | 6 +++--- http-walker.c | 2 +- http.c | 6 +++--- remote-curl.c | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-)