Build on GNU/kfreeBSD and GNU/Hurd

[ginggs]

Debian's GNU/kFreeBSD port consists of a GNU userland using the GNU C library on top of a FreeBSD kernel. Similarly, Debian's GNU/Hurd runs on top of the GNU Mach microkernel.

[EdSchouten]

__FreeBSD_kernel__ means that the system has a FreeBSD kernel, and we don't care about the userland, right? As in, it should be valid if even FreeBSD itself defined this, right? If so, then this change is wrong. FreeBSD does not have <features.h>, <endian.h>, etc.

[ginggs]

Debian/kFreeBSD has /usr/include/features.h, /usr/include/endian.h and /usr/include/machine/endian.h.

I tried building both ways, but including <machine/endian.h> resulted in the library having an executable stack.

[EdSchouten]

If I understand the code correctly, we want to test whether the system uses glibc. Instead, we now test whether the system has a Linux kernel, a FreeBSD kernel or a HURD kernel. If your system has a FreeBSD kernel, it may not necessarily have the headers you're trying to include. FreeBSD itself does not have these headers.

[ginggs]

@EdSchouten I'm not sure now whether __FreeBSD_kernel__ is defined on real FreeBSD, but we obviously don't want to break that build.

Another option is to use __GLIBC__ here which should be defined on Linux, GNU/kFreeBSD and GNU/Hurd.

[EdSchouten]

Please do. In this case it's far more accurate. 👍

[ginggs]

@EdSchouten using __GLIBC__ here instead of __linux worked for me on Linux, GNU/kFreeBSD and GNU/Hurd.

[ginggs]

I was wrong about the executable stack, I am still seeing it on GNU/kFreeBSD.
I'm guessing it may be caused by this trailer at the end of every *.S file:

/* Enable stack protection */
#if defined(__linux__) && defined(__ELF__)
.section .note.GNU-stack,"",%progbits
#endif

Would we want #if defined(__GLIBC__) && defined(__ELF__) here instead?

[EdSchouten]

I think that stanza is also required on FreeBSD itself. Maybe we should just use defined(__ELF__) here? My guess is that it's always safe to add this to ELF files, regardless of whether the linker and operating system do something with it.

[ViralBShah]

Please squash when this is ready to go. @ginggs Also do let me know if you need me to tag new versions to make packaging upstream easier.

[ginggs]

I think this is ready to go. Let me know if I should squash this into one commit, or keep the ELF stuff in a separate commit, or even move it to a separate pull request.

[ViralBShah]

Yes squashing + separate elf commit is good. If simpler you could just close this and do a new PR. Either way is ok

[ginggs]

Good to go.

[tkelman]

the comment is referring to the if statement on line 80 and still applies

[ginggs]

I commented when I first committed this, but it disappeared after I squashed.
https://github.com/ginggs/openlibm/commit/49059f7ee6b9d72cdeb962bc34c14d56432ade07#commitcomment-14003490

There is a comment #keep these if statements separate, but it would make sense to have a sane default, otherwise the clean target in Makefile:
rm -fr $$dir/*.o $$dir/*.a $$dir/*.$(SHLIB_EXT)*; \
evaluates to:
rm -fr $$dir/*.o $$dir/*.a $$dir/*.*; \
and cleans everything!

I noticed this was changed in openspecfun and it now has a default.
See: JuliaMath/openspecfun@e91925a

[tkelman]

that also should not have removed the comment. the "separate" part is referring to

ifneq (,$(findstring MINGW,$(OS)))
override OS=WINNT
endif

#keep these if statements separate
ifeq ($(OS), WINNT)

[ginggs]

I think I've got it now

[tkelman]

👍