-
-
Notifications
You must be signed in to change notification settings - Fork 6.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
configure, cmake, lib: more form api deprecation #9621
Conversation
I don't think we can disable it by default without bumping the SONAME number. This would affect a lot of users. |
Technically speaking, yes we can. The API is still there but returns Now, for users needing it, they still can build with Considering #9116, it seems obvious that deprecation in the documentation only is not enough: the solution implemented in this PR may seem to come abrubtly because you just read the PR. If it had been submitted (with forms enabled) together with the mime api introduction, it would appear more natural to disable it by default now. After all, we already suppressed (not only disabled) features like Kerberos4 and char code conversion. They were less used, I admit. That said, we can still have the form api enabled by default for some months or until a big change (version 8?), but I don't feel stretching the delay for disabling it by default will help. A change in SONAME, although not needed, will emphasize a theoric incompatibility that can even exist between 2 different builds today (using |
I disagree. SONAME implies ABI compatibility, and ABI includes behavior. This is functionality that is still very much in use by people. Having the option there is good, but disabling it by default is not.
It becomes an exercise in philosophy at times: when nobody uses a feature then it actually doesn't change behavior when it is removed. A tough balance and call to make.
A user can destroy the compatibility in many ways, but we maintain it for the same build options, and in particular when you do not disable features in a new way. |
7e5c0de
to
fd82766
Compare
Updated to enable by default. |
I don't quite understand the warning. What is that going to do for users? The form API will be provided for as long as we remain using this SONAME, which very well may be another decade or two. |
This PR is not only another nth + 1 disabling conditional, but a push for users to use mime instead of form api by emphasizing deprecation. |
I think the doc warning is enough. We could put it in bold, diff --git a/docs/libcurl/curl_formadd.3 b/docs/libcurl/curl_formadd.3
index 92135bf..04e10b3 100644
--- a/docs/libcurl/curl_formadd.3
+++ b/docs/libcurl/curl_formadd.3
@@ -32,7 +32,7 @@ CURLFORMcode curl_formadd(struct curl_httppost **firstitem,
struct curl_httppost **lastitem, ...);
.fi
.SH DESCRIPTION
-This function is deprecated. Do not use. See \fIcurl_mime_init(3)\fP instead.
+\fBThis function is deprecated.\fP Use \fIcurl_mime_init(3)\fP instead.
curl_formadd() is used to append sections when building a multipart form
post. Append one section at a time until you have added all the sections you |
Good idea. But I fear a lot of people do cut n' paste old programs and do not read the doc :-( |
Ref: curl#9621 Closes #xxxx
fd82766
to
0cf44b9
Compare
@@ -654,7 +654,7 @@ CURLcode Curl_vsetopt(struct Curl_easy *data, CURLoption option, va_list param) | |||
data->set.upload = FALSE; | |||
break; | |||
|
|||
#ifndef CURL_DISABLE_MIME | |||
#ifndef CURL_DISABLE_FORM_API |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't disabling MIME also imply disabling the form API? This makes them look like they are independent from each other, but surely the form API doesn't work if you disable MIME does it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't disabling MIME also imply disabling the form API?
Yes. The case is tested in configure.ac
and CURL_DISABLE_FORM_API
is always set when CURL_DISABLE_MIME
is. CMakeLists.txt
doesn't check it though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is now also handled in CMake.
0165bb4
to
7be0c85
Compare
7be0c85
to
4e8173d
Compare
3926257
to
6f60ab6
Compare
Now that the deprecation patch has been applied, I've removed the compilation-time warning. |
6f60ab6
to
fb2dd31
Compare
fb2dd31
to
5865a3b
Compare
5865a3b
to
d5dd367
Compare
d5dd367
to
ab940b7
Compare
7eea330
to
b0b3638
Compare
b0b3638
to
ee4c730
Compare
ee4c730
to
c230703
Compare
c230703
to
593724d
Compare
593724d
to
5c4c37f
Compare
5c4c37f
to
be78058
Compare
de9b7d8
to
7083a88
Compare
7083a88
to
cc5b5e7
Compare
it would be very nice to give some examples of how to convert curl_formadd to curl_mime_init .... as the logic is completely different. thanks |
Well... this is a doc request and should be set in a separate issue, as it is not related to introducing conditional compilation for form api. FYI, there are files |
cc5b5e7
to
f07fdb4
Compare
f07fdb4
to
c2d2788
Compare
Introduce a --enable-form-api configure option to control its inclusion in builds. The condition name defined for it is CURL_DISABLE_FORM_API. Form api code is dependent of MIME: configure and CMake handle this dependency automatically: CMake by making it a dependent option explicitly, configure by inheriting the MIME value by default and rejecting explicit incompatible values. "form-api" is now a new hidden test feature. Update libcurl modules to respect this option and adjust tests accordingly.
c2d2788
to
599f633
Compare
Thanks, and sorry for taking so long. |
Introduce a --enable-form-api configure option to control its inclusion in builds. The condition name defined for it is CURL_DISABLE_FORM_API. Form api code is dependent of MIME: configure and CMake handle this dependency automatically: CMake by making it a dependent option explicitly, configure by inheriting the MIME value by default and rejecting explicit incompatible values. "form-api" is now a new hidden test feature. Update libcurl modules to respect this option and adjust tests accordingly. Closes curl#9621
Introduce a
--enable-form-api
configure option to control its inclusion in builds. The condition name defined for it isCURL_DISABLE_FORM_API
.Form api code is dependent of MIME: configure and CMake handle this dependency automatically: CMake by making it a
dependent option explicitly, configure by inheriting the MIME value by default and rejecting explicit incompatible values.
form-api
is now a new hidden test feature.Update libcurl modules to respect this option and adjust tests accordingly.