-
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
test1960: verify CURL_SOCKOPT_ALREADY_CONNECTED #10651
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
Conversation
When returned from the CURLOPT_SOCKOPTFUNCTION, like when we have a custom socket connected in the app, passed in to libcurl. Verifies the fix in #10648
tests/libtest/lib1960.c
Outdated
#include "test.h" | ||
|
||
#ifdef WIN32 | ||
#undef _WIN32_WINNT |
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.
Hmm, seems like my comment got applied only to the commit and not shown in the PR:
Or maybe just make HAVE_INET_PTON
a requirement for executing this test? The Windows headers might already be included indirectly by test.h, so this might be too late to redefine the value.
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.
ah hurt by that again ...
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.
but then, where/when is mingw.h included which is the header that apparently sets the "bad" define?
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.
According to: msys2/MINGW-packages#6191
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.
While here: should we perhaps drop mingw v1 support and live a happier life?
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.
I'd just make the test always succeed if HAVE_INET_PTON
is not set?
Dropping MinGW v1 might fire back, unfortunately. In another open source project I'm involved in, when that was broken, there were always many complaints by people new to software development even though it was not officially supported. The original MinGW was often the thing tutorials pointed to for Windows, or they mentioned just "MinGW" and a Google search for that points to its Wikipedia page, linking to the original MinGW homepage. Not sure if that's still the case, but that was not so many years ago when original MinGW was already very much outdated.
So I wouldn't care too much about finding workarounds for it to make features work, but being able to get a clean default compile might make our lives easier than not having to care for it at all.
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.
I'd just make the test always succeed if HAVE_INET_PTON is not set?
Not terribly easy since the test verifies protocol parts and I think it makes sense to have it do that. I will have to also invent some way for how this test can be skipped for mingw v1...
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.
trying a precheck to skip the test if inet_pton is not present
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.
Replying to https://curl.se/mail/lib-2023-03/0006.html:
"original" MinGW has been a constant source of issues :( (I've spent many days digging for issues unique to it) Its Windows headers are seriously outdated. The compiler as well. It is (and always was) difficult to install. It only supports Intel 32-bit, which by itself is an almost dead platform. Now even its original website (mingw.org) is gone.
It's never an easy decision to drop a platform, compiler, feature, but in this case I'd vote to do so. It takes off a ton of complications, leaving largely a modern mingw-w64 and MSVC to worry about on Windows.
[ Meanwhile mingw-w64 can be installed via MSYS2 or standalone, or using most unixy package managers as well. Its Wine headers are up to date. It's actively developed. ]
Success!! 🚀 |
When returned from the CURLOPT_SOCKOPTFUNCTION, like when we have a custom socket connected in the app, passed in to libcurl. Verifies the fix in curl#10648 Closes curl#10651
Otherwise, it might find the binary in .libs which can cause it to use the system libcurl which can fail. This error is only visible by noticing that the test is skipped. Follow-up to e4dfe6f Ref: curl#10651
When returned from the CURLOPT_SOCKOPTFUNCTION, like when we have a custom socket connected in the app, passed in to libcurl.
Verifies the fix in #10648