-
Notifications
You must be signed in to change notification settings - Fork 455
CDRIVER-4249 Apply _DEFAULT_SOURCE to all subdirectories #920
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
_BSD_SOURCE is a deprecated alias for _DEFAULT_SOURCE.
@jmikola Regarding PHPC, the changes in this PR suggest that defining |
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.
Regarding PHPC, the changes in this PR suggest that defining _DEFAULT_SOURCE alone should (hopefully) suffice for building the entire C driver via autotools. Please let me know if you observe otherwise.
I can confirm that replacing -D_XOPEN_SOURCE=700
with -D_DEFAULT_SOURCE
in our bundled CFLAGS allows libmongoc to once again compile without error. I'll defer to Kevin on the rest of this PR, but this LGTM.
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.
Thank you for the drive-by fix for Darwin tasks with zlib! I request not modifying the zlib source directly, but otherwise LGTM.
src/zlib-1.2.11/CMakeLists.txt
Outdated
@@ -27,6 +27,7 @@ check_include_file(stddef.h HAVE_STDDEF_H) | |||
# | |||
# Check to see if we have large file support | |||
# | |||
cmake_push_check_state() |
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.
To permit validating changes on Darwin, this PR includes a drive-by fix of build failures on Darwin due to #890. Since this fix requires modifying the zlib bundle (last updated in 2018), I would appreciate some confirmation as to whether the changes made are appropriate. If we wish to leave the zlib sources unmodified, we can choose to selectively exclude _DEFAULT_SOURCE during compilation of the zlib bundle instead.
I prefer leaving the zlib sources unmodified. It more easily enables updating the bundled zlib. E.g. if zlib has a priority patch release (e.g. to fix a security bug) it may simplify updating the bundled zlib.
endif () | ||
if (HAVE_STDARG_H) | ||
add_definitions (-DHAVE_STDARG_H) | ||
endif () |
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.
Offhand, PHP was already defining HAVE_UNISTD_H
for bundled zlib. It was trivial to make the change for HAVE_STDARG_H
, although I didn't notice any build failure omitting it on Ubuntu 20.04.
LGTM!
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.
(Lack of) <stdarg.h>
was not causing issues, as far as I could observe. HAVE_STDARG_H
was just added to compliment the addition of HAVE_UNISTD_H
for consistency. The (lack of) <unistd.h>
was the relevant issue being addressed.
This allows PHPC to defer entirely to libmongoc for cross-option URI validation Bump libmongoc to 1.21-dev Build changes are ported from upstream CMake changes (see: CDRIVER-4249)
Reverts a change introduced in ab44b0c. This was previously in mongodb/mongo-c-driver#920 but removed before merging in mongodb/mongo-c-driver@9d2d8b1.
Per CDRIVER-4249, this PR cleans up feature test macros used to enable non-conforming C language extensions such as POSIX and BSD library functions.
The
_BSD_SOURCE
macro is a deprecated alias of_DEFAULT_SOURCE
, so it was removed in favor of the already-present_DEFAULT_SOURCE
.There was a choice available as to whether to use
_DEFAULT_SOURCE
or_GNU_SOURCE
. The difference being (according to GNU documentation):_GNU_SOURCE
: everything is included: ISO C89, ISO C99, POSIX.1, POSIX.2, BSD, SVID, X/Open, LFS, and GNU extensions. In the cases where POSIX.1 conflicts with BSD, the POSIX definitions take precedence._DEFAULT_SOURCE
: most features are included apart from X/Open, LFS and GNU extensions: the effect is to enable features from the 2008 edition of POSIX, as well as certain BSD and SVID features without a separate feature test macro to control them.The documentation recommends
_GNU_SOURCE
for new programs. However,_GNU_SOURCE
preferring POSIX definitions over BSD seems to causestrerror_r()
compatibility/detection issues on Darwin that are not present with_DEFAULT_SOURCE
. Because_DEFAULT_SOURCE
does not prompt these issues while still enabling the set of features required and used by the C driver,_DEFAULT_SOURCE
was selected and_GNU_SOURCE
removed.To permit validating changes on Darwin, this PR includes a drive-by fix of build failures on Darwin due to #890. Since this fix requires modifying the zlib bundle (last updated in 2018), I would appreciate some confirmation as to whether the changes made are appropriate. If we wish to leave the zlib sources unmodified, we can choose to selectively exclude
_DEFAULT_SOURCE
during compilation of the zlib bundle instead.