CDRIVER-4249 Apply _DEFAULT_SOURCE to all subdirectories #920
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.
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.
kevinAlbs left a comment
kevinAlbs left a comment There was a problem hiding this comment.
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
| # | ||
| # Check to see if we have large file support | ||
| # | ||
| cmake_push_check_state() |
There was a problem hiding this comment.
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.
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.
(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.
3 participants
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_SOURCEmacro 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_SOURCEor_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_SOURCEfor new programs. However,_GNU_SOURCEpreferring POSIX definitions over BSD seems to causestrerror_r()compatibility/detection issues on Darwin that are not present with_DEFAULT_SOURCE. Because_DEFAULT_SOURCEdoes not prompt these issues while still enabling the set of features required and used by the C driver,_DEFAULT_SOURCEwas selected and_GNU_SOURCEremoved.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_SOURCEduring compilation of the zlib bundle instead.