githwxi / ATS-Postiats

ATS2: Unleashing the Potentials of Types and Templates
www.ats-lang.org
Other
352 stars 54 forks source link

GCC toolchain is hardcoded in distribution makefile #259

Closed avanov closed 3 years ago

avanov commented 3 years ago

Hi!

It turns out that this line in the dist makefile hardcodes GCC toolchain for all target platforms, which makes the following makefile target fail on Mac OS, which uses Clang toolchain by default: make -j4 -C src/CBOOT patsopt CCOMP=gcc GCFLAG= LDFLAGS=

Here's an example of the issue when ATS2 is built on MacOS with Nix.At the moment, in order to mitigate it we have to explicitly depend on GCC when we build the distribution on MacOS, but in principle I think this should not be a requirement, as ATS docs say that it should be possible to use Clang as the underlying toolchain for ATS2.

It would be good to lift this requirement and to make GCC optional.

githwxi commented 3 years ago

I just changed this line to:

CCOMP=$(CC)

By default, CC is gcc. You can do:

make CC=clang -f Makefile_dist

to compile ATS2 with clang. I just tried it and it went smoothly.

Without this change, you could just try:

make CCOMP=clang -f Makefile_dist

avanov commented 3 years ago

Hi @githwxi , thanks for the quick feedback! Do you mind if we change CC=gcc to CC ?= gcc. The difference between them lies in how make will treat external environment variables prefixed to the command. Here's an example:

Makefile (current =)

ABC = default
all:
    @echo $(ABC)

Prefix vs make param:

$ make all
default

$ make ABC=123 all
123

$ ABC=123 make all
default

Makefile (suggested ?=)

ABC ?= default
all:
    @echo $(ABC)

Prefix vs make param:

$ make all
default

$ make ABC=123 all
123

$ ABC=123 make all
123

The benefit that we gain here is that the make command becomes immutable in its form, yet dependent on the environment it runs in, which helps in automating the project build pipeline with a higher level build environments like Nix, which may set appropriate environment variables as prefixes, before running project makefiles.

githwxi commented 3 years ago

Very good. I have just made the change. Thank you for your input!

avanov commented 3 years ago

Thank you!

githwxi commented 3 years ago

Yes, this had been reported earlier. It should be fixed when the next release is out.