Make the build logic more robust for BSD systems #382

ararslan · 2018-04-13T23:14:29Z

This properly sets MAKE (when undefined) on BSDs to gmake rather than make, which refers to the incompatible BSD Make. Further, it betters detection of Clang as the default compiler, which is the case on FreeBSD 11.0+ and OpenBSD 6.0+.

This has been verified to work on FreeBSD with gmake and gmake -f makefile.shared.

Companion to libtom/libtommath#108.

sjaeckel · 2018-04-13T23:36:10Z

makefile_include.mk


-ifeq ($(CC),cc)
+ifeq ($(origin CC),default)
+ifneq (,$(shell echo $$"\#ifdef __clang__\nCLANG\n\#endif\n" | $(CC) -E - | grep CLANG))


uhm, this creates the following for me:

$ make <stdin>:3:2: error: #endif without #if ... $ gcc -dumpversion 5.4.0 $ cc -dumpversion 5.4.0 $ clang -dumpversion 4.2.1

Ubuntu Xenial-based distro

Ack, I can reproduce that on Elementary OS, which is based on Ubuntu 16.04. Interestingly the problem seems to be with the string escaping. On FreeBSD, the $$" is required, otherwise piping to $(CC) -E fails with "error: unterminated conditional directive." On Linux, it appears that the $$" is what's messing it up...

To add to the confusion, macOS, which uses Clang as the default compiler, seems to be fine both with and without $$. 🤔

Would that be an option?

-ifneq (,$(shell echo $$"\#ifdef __clang__\nCLANG\n\#endif\n" | $(CC) -E - | grep CLANG)) +ifneq (,$(shell $(CC) --version | grep -i clang))

The problem there is that Clang's version info doesn't tell you it's Clang on every platform. For example, Clang on macOS calls itself "Apple LLVM."

I was chatting with a colleague about the $$ conundrum and he said that it stems from the fact that shell in a Makefile refers to a different shell on different systems, and that it's always POSIX sh on FreeBSD and dash on Ubuntu. While an interesting tidbit, it doesn't provide an immediate solution. 😉

I think the easiest way forward here would be to put the check in a file, e.g. just make a file containing

#ifdef __clang__ CLANG #endif

and in the Makefile call $(shell $(CC) -E file | grep CLANG). The downside of that of course is then there's just some annoying little file sitting floating around in the source tree...

Figured it out. We can just conditionally add extra escaping on FreeBSD.

sjaeckel

@buggywhip can you please test on Mac?
@karel-m can you please test as well on the platforms where you use makefile or makefile.shared?

ararslan · 2018-04-18T14:46:19Z

I tried this out on my Mac, both with make and make -f makefile.shared. The default compiler is correctly detected as Clang, and tests pass with both make invocations. As an aside, Travis supports macOS, so you could add that to your testing matrix if you wanted.

rofl0r · 2018-04-22T20:52:52Z

The default compiler is correctly detected as Clang

that's not how it works. in a properly configured environment, the default compiler is detected as follows:

if CC env var is set, this denotes the compiler to use
if CC is not set, a command called cc will be used (or c99 if the compilee wants to use C99 features)

that means that a properly configured system should have symlinks /usr/bin/cc -> /usr/bin/clang or whatever in place.

no makefile should ever do bogus stuff like using uname to decide which OS it runs on and then use an if-else abomination to check whether MAC uses clang or gcc.

ararslan · 2018-04-22T23:14:46Z

That's not what this does. Currently, the logic here says "if your $(CC) is cc, we assume it's GCC and prepend the cross-compilation stuff." This is incorrect for systems that use Clang as the default compiler. This PR changes that to "if you didn't specify CC, we'll check whether it refers to GCC or Clang, then prepend the cross-compilation stuff appropriately." The problem with just using $(CC) as-is is that most cross compilation toolchains that I've seen have gcc or clang as part of the executable name rather than cc.

rofl0r · 2018-04-23T00:45:38Z

That's not what this does

yeah, i'm talking about how it should be according to conventions

sjaeckel · 2018-04-23T20:35:02Z

@rofl0r okay, so how should we solve this? ~~resp. do we have to do anything?~~

rofl0r · 2018-04-26T19:05:16Z

looking into one of my cross-compiler builds:

$ ls -la ~/musl-cross-make-6.3.0/mips-linux-musl/bin | grep cc | head -n 1
lrwxrwxrwx    1 rofl     rofl            19 Jan 21  2017 mips-linux-musl-cc -> mips-linux-musl-gcc

that means what i said above applies also to CROSS_COMPILE scenarios i.e.: there should be a $(CROSS_COMPILE)cc which is a symlink to either $(CROSS_COMPILE)gcc or $(CROSS_COMPILE)clang.

so:

if $(CC) is set, we always use it like that
if $(CROSS_COMPILE) is set but not $(CC), CC becomes $(CROSS_COMPILE)cc
if neither $(CROSS_COMPILE) nor $(CC) are set CC becomes cc

or simplified (pseudocode):

if isempty(CC)
  CC:=$(CROSS_COMPILE)cc
fi

sjaeckel · 2018-04-27T20:26:40Z

if isempty(CC)
CC:=$(CROSS_COMPILE)cc
fi

this would require to set CC and CROSS_COMPILE in case I want to cross-compile or not?
while searching for the reason why this is even there I realized that none of my cross-compilation toolchains has a $(CROSS_COMPILE)cc symlink, they all only have $(CROSS_COMPILE)gcc ...

rofl0r · 2018-04-27T22:36:51Z

this would require to set CC and CROSS_COMPILE in case I want to cross-compile or not?

no, CROSS_COMPILE is user set, you shouldnt try to set it if not given

while searching for the reason why this is even there I realized that none of my cross-compilation toolchains has a $(CROSS_COMPILE)cc symlink, they all only have $(CROSS_COMPILE)gcc

that's odd. where did you get your cross toolchains from ?

EDIT: anyway, even if there are some toolchains out there that don't set $(CROSS_COMPILE)cc as a symlink to the right compiler, we should lobby them into doing it, as this takes away the entire need for system-specific and non-crosscompile-compatible uname hacks.

sjaeckel · 2018-04-28T10:29:39Z

no, CROSS_COMPILE is user set, you shouldnt try to set it if not given

sorry - that's what I meant by I, the user, not the libtom maintainer :)

that's odd. where did you get your cross toolchains from ?

EDIT: anyway, even if there are some toolchains out there that don't set $(CROSS_COMPILE)cc as a symlink to the right compiler, we should lobby them into doing it, as this takes away the entire need for system-specific and non-crosscompile-compatible uname hacks.

well I haven't seen any cross toolchain that provides a cc ... I've some from linaro, some from debian/ubuntu packages, some in yocto...

Edit: oh, I just found one, buildroot seems to provide a $(CROSS_COMPILE)cc

... and yes it's a good idea to 'lobby them into doing it' but for now that's not real yet and so we should go with one of those 'temporary solutions'^TM that lets us anyways do what we want :)

sjaeckel · 2018-05-19T18:36:34Z

makefile_include.mk

 endif

-ifeq ($(CC),cc)
+ifeq ($(origin CC),default)


Before merging this... I just realized that the original check for CC was already wrong, this path should only be taken if CROSS_COMPILE is defined... or not? :)

Sorry, been busy with other stuff. I'll revisit this.

This properly sets MAKE (when undefined) on BSDs to gmake rather than make, which refers to the incompatible BSD Make. Further, it betters detection of Clang as the default compiler, which is the case on FreeBSD 11.0+ and OpenBSD 6.0+.

Make the build logic more robust for BSD systems (cherry picked from commit 5ab8dcf)

ararslan mentioned this pull request Apr 13, 2018

Make the build logic more robust for BSD systems libtom/libtommath#108

Merged

sjaeckel reviewed Apr 13, 2018

View reviewed changes

ararslan force-pushed the aa/freebsd branch from e504dbd to adb610f Compare April 14, 2018 17:11

sjaeckel approved these changes Apr 18, 2018

View reviewed changes

sjaeckel reviewed May 19, 2018

View reviewed changes

Make the build logic more robust for BSD systems

f5c665b

This properly sets MAKE (when undefined) on BSDs to gmake rather than make, which refers to the incompatible BSD Make. Further, it betters detection of Clang as the default compiler, which is the case on FreeBSD 11.0+ and OpenBSD 6.0+.

ararslan force-pushed the aa/freebsd branch from adb610f to f5c665b Compare May 29, 2018 21:05

sjaeckel merged commit 5ab8dcf into libtom:develop May 31, 2018

ararslan deleted the aa/freebsd branch May 31, 2018 19:51

sjaeckel added a commit that referenced this pull request Jun 21, 2018

Merge pull request #382 from ararslan/aa/freebsd

4c2bb90

Make the build logic more robust for BSD systems (cherry picked from commit 5ab8dcf)

Make the build logic more robust for BSD systems #382

Make the build logic more robust for BSD systems #382

Uh oh!

Conversation

ararslan commented Apr 13, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ararslan Apr 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sjaeckel left a comment

Choose a reason for hiding this comment

Uh oh!

ararslan commented Apr 18, 2018

Uh oh!

rofl0r commented Apr 22, 2018

Uh oh!

ararslan commented Apr 22, 2018

Uh oh!

rofl0r commented Apr 23, 2018

Uh oh!

sjaeckel commented Apr 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rofl0r commented Apr 26, 2018

Uh oh!

sjaeckel commented Apr 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rofl0r commented Apr 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sjaeckel commented Apr 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ararslan Apr 14, 2018 •

edited

Loading

sjaeckel commented Apr 23, 2018 •

edited

Loading

sjaeckel commented Apr 27, 2018 •

edited

Loading

rofl0r commented Apr 27, 2018 •

edited

Loading

sjaeckel commented Apr 28, 2018 •

edited

Loading