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
build: delete checks for C89 standard headers #11940
Conversation
stdio.h
|
7f2af51
to
59cfdbe
Compare
stdio.h
@ncop: Do you think this solution would also work?: check_symbol_exists(fseeko "${CURL_INCLUDES};stdio.h" HAVE_FSEEKO)
check_symbol_exists(_fseeki64 "${CURL_INCLUDES};stdio.h" HAVE__FSEEKI64) This also self-documents that stdio.h is necessary for detection, without adding it for every other check by default. |
@bagder: I extended this PR with other C89 headers that we're checking for, most likely unnecessarily. I'm looking into |
ea8fbb5 (from 2008) suggests that a system called "Cell" might not actually come with a C89 |
59cfdbe
to
0e07821
Compare
`HAVE_STDIO_H` is not used in the source. `stdio.h` is a standard C89 header and we use it everywhere unconditionally. Ref: https://github.com/curl/curl/pull/11918/files#r1336337304 Regression from 9c7165e curl#11918 Closes #xxxxx
0e07821
to
fe35197
Compare
I plan to follow this up with a patch to move CMake global standard headers ( |
Yes, I would expect that work work, and according the logs it does:
I agree that it is better this way. |
I only discovered the broken test of fseeko, because I was looking at the logs. It is fragile because the fallback code also works, so a broken test can easily go undetected. Maybe it would be an idea to add a test to CI to run both |
That's a good idea. It will of course only test the exact options used in the test and we can run a few different ones, but it should still help us. |
Such detection comparison test would be very useful indeed. |
I missed it in 96c2990 curl#11940
Before this patch we added standard headers unconditionally to the global list of headers used for feature checks. This is unnecessary and also doesn't help CMake 'Generate' performance. This patch moves these headers to each feature check where they are actually needed. Stop using `stddef.h`, as it seems unnecessary. I've used autotools' `m4/curl-functions.m4` to figure out these dependencies. Also delete checking for the C89 standard header `time.h`, that I missed in the earlier commit. Ref: 96c2990 #11940 Closes #11951
#ifdef HAVE_STRINGS_H | ||
# include <strings.h> | ||
#endif | ||
/* includes end */" | ||
AC_CHECK_HEADERS( | ||
sys/types.h string.h strings.h, |
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 seems to break strerror_r
detection on Solaris, visible as warnings in the autobuilds since yesterday:
https://curl.se/dev/log.cgi?id=20230927074238-1773871#prob1
configure: WARNING: cannot determine strerror_r() style: edit lib/curl_config.h manually.
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.
Is it possible Solaris doesn't have a string.h? Or it something else? Is there a way to pull or view config.log of a failed build?
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 this, strerror_r is declared in string.h: https://docs.oracle.com/cd/E86824_01/html/E54766/strerror-r-3c.html
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.
Possibly related (caused by my bad edit): d14089d
Follow-up to 96c2990 (curl#11940) Closes #xxxxx
We removed C89 `setjmp.h` and `signal.h` detections and excluded them from the global header list we use for detecting functions [1]. Then missed to re-add these headers to the specific functions which need them to be detected [2]. Fix this omission in this patch. `sigsetjmp` has a second detection attempt documented as 'special' in the comment. However this detection uses the same method as the first attempt, with the difference of using `setjmp.h` header only, without including the rest of non-standard headers detected. It detects both function and macro like the first attempt. This second attempt seems redundant. It's been there since the initial commit of CMake support. [1] Follow-up to 3795fcd curl#11951 [2] Follow-up to 96c2990 curl#11940 Closes #xxxxx
We removed C89 `setjmp.h` and `signal.h` detections and excluded them from the global header list we use when detecting functions [1]. Then missed to re-add these headers to the specific functions which need them to be detected [2]. Fix this omission in this patch. [1] Follow-up to 3795fcd #11951 [2] Follow-up to 96c2990 #11940 Closes #12043
Delete checks and guards for standard C89 headers and assume these are available: `stdio.h`, `string.h`, `time.h`, `setjmp.h`, `stdlib.h`, `stddef.h`, `signal.h`. Some of these we already used unconditionally, some others we only used for feature checks. Follow-up to 9c7165e curl#11918 (for `stdio.h` in CMake) Closes curl#11940
Before this patch we added standard headers unconditionally to the global list of headers used for feature checks. This is unnecessary and also doesn't help CMake 'Generate' performance. This patch moves these headers to each feature check where they are actually needed. Stop using `stddef.h`, as it seems unnecessary. I've used autotools' `m4/curl-functions.m4` to figure out these dependencies. Also delete checking for the C89 standard header `time.h`, that I missed in the earlier commit. Ref: 96c2990 curl#11940 Closes curl#11951
This made the getaddrinfo detection fail, but we did not spot it in the CI because it graciously falled back to using legacy functions instead! Follow-up to 96c2990 (curl#11940) Closes curl#11965
Follow-up to 96c2990 curl#11940 Closes curl#11966
We removed C89 `setjmp.h` and `signal.h` detections and excluded them from the global header list we use when detecting functions [1]. Then missed to re-add these headers to the specific functions which need them to be detected [2]. Fix this omission in this patch. [1] Follow-up to 3795fcd curl#11951 [2] Follow-up to 96c2990 curl#11940 Closes curl#12043
Delete checks and guards for standard C89 headers and assume these are
available:
stdio.h
,string.h
,time.h
,setjmp.h
,stdlib.h
,stddef.h
,signal.h
Some of these we already used unconditionally, some others we only used
for feature checks.
Follow-up to 9c7165e #11918 (for
stdio.h
in CMake)Closes #11940