Make zstd build out of the box properly on FreeBSD #475
Conversation
| @@ -41,6 +41,10 @@ CPPFLAGS += -I./legacy -DZSTD_LEGACY_SUPPORT=1 | |||
| ZSTD_FILES+= $(wildcard legacy/*.c) | |||
| endif | |||
|
|
|||
| ifeq ($(shell uname), FreeBSD) | |||
| CPPFLAGS += -D_POSIX_C_SOURCE=200112L | |||
There was a problem hiding this comment.
It seems this detection macro is not useful in /lib.
Feel free to explain if you believe differently.
| @@ -52,6 +52,10 @@ CPPFLAGS += -I$(ZSTDDIR)/legacy | |||
| ZSTDLEGACY_FILES:= $(ZSTDDIR)/legacy/*.c | |||
| endif | |||
|
|
|||
| ifeq ($(shell uname), FreeBSD) | |||
| CPPFLAGS += -D_POSIX_C_SOURCE=200112L | |||
There was a problem hiding this comment.
I tend to prefer such detection macro to be set from source code itself. My understanding is that this macro should be enabled from utils.h. Is there a safe way to do so ?
cc @inikep , he has some experience on this topic.
There was a problem hiding this comment.
-
The issue concerns the availability of
-r(recursive) option that usesopendir/closedirfunctions (http://man.he.net/man3/closedir) conforming to SVr4, POSIX.1-2001, 4.3BSD -
The
_POSIX_C_SOURCE=200112Lmacro is used to detect availability ofopendir/closedirfunctions on Linux. My understanding is that_POSIX_C_SOURCEshouldn't be defined by user at all because it may not work e.g. with older versions of an operating system andglibc. It's better to use--std=gnu99or-D_GNU_SOURCEwhich should be more portable as they define a proper version of_POSIX_C_SOURCE. -
The proposed changes force the C parser to turn on '-r' on FreeBSD. But it's better to do auto detection of FreeBSD only for relevant file that is
util.h. It seems that all BSD-forks are based on 4.4BSD therefore we need only to check availability of one of:
__DragonFly__
__FreeBSD__
__NetBSD__
__OpenBSD__
|
Totally agree on the objective. |
|
This PR should turn on the '-r' option for *BSD and Solaris: |
|
With latest patch by @inikep , recursive mode |
* Remove last bashism from tests/playTests.sh * Use gmd5sum from the sysutils/coreutils port
c23833a to
83cc2fb
Compare
|
@Cyan4973, I have rebased, and split this up in two separate commits:
|
| @@ -98,7 +98,7 @@ extern "C" { | |||
| # define SET_HIGH_PRIORITY /* disabled */ | |||
| # endif | |||
| # define UTIL_sleep(s) sleep(s) | |||
| # if defined(_POSIX_C_SOURCE) && (_POSIX_C_SOURCE >= 199309L) | |||
| # if defined(__DragonFly__) || defined(__FreeBSD__) || defined(__NetBSD__) || defined(__OpenBSD__) || (defined(_POSIX_C_SOURCE) && (_POSIX_C_SOURCE >= 199309L)) | |||
There was a problem hiding this comment.
http://man.openbsd.org/nanosleep.2 says:
The nanosleep() system call has been available since NetBSD 1.3 and was ported to OpenBSD 2.1 and FreeBSD 3.0.
It means that this change will not work e.g. with FreeBSD older than 3.0 (Fri, 16 Oct 1998).
There was a problem hiding this comment.
Well, in practice nobody runs these old versions anymore. The oldest supported version of FreeBSD is 9.3, and even that will reach end-of-life on 2016-12-31. In practice, there may be a few equipment vendors that run older forked versions of FreeBSD, but certainly not older than FreeBSD 7.x or 8.x.
There was a problem hiding this comment.
I'm sure you are right but I wanted to make it clear for @Cyan4973.
There are some OSes (e.g. Windows, CentOS/Red Hat Enterprise Linux) with 10-years maintenance updates but FreeBSD 3.0 is 18-years old.
There was a problem hiding this comment.
You would be surprised how old some systems can be out there...
Nevertheless, there are portability risks that are reasonable to assume, and not supporting FreeBSD < 3.0 is one of them. We'll learn from user feedbacks if there is any need to take care of this use case.
-D_POSIX_C_SOURCE=200112Lto enable POSIX functionalitytests/playTests.shgmd5sumfrom the sysutils/coreutils port