-stdlib=libc++ should be conditional on compiler and not hardcoded on macOS

[barracuda156]

These flags in Makefile should be conditional on compiler and not just OS:

ifeq ($(UNAME),Darwin)
    CFLAGS += -stdlib=libc++
    CXXFLAGS += -stdlib=libc++
    LDFLAGS += -stdlib=libc++
endif

Otherwise build with GCC is broken:

/opt/local/bin/gcc-mp-12 -pipe -Os -arch ppc  -O2 -I ./include  -stdlib=libc++ -fPIC -c -o src/cencode.o src/cencode.c
gcc-mp-12: error: unrecognized command-line option '-stdlib=libc++'
make[1]: *** [src/cencode.o] Error 1
make: *** [libsass/lib/libsass.a] Error 2
ERROR: compilation failed for package ‘sass’

[wch]

This is similar to #116 (which was fixed) but I think it's a separate issue, since the lines you're referring to are in the Makefile from libsass itself: https://github.com/sass/libsass/blob/2102188d21d2b7577c2b3edb12832e90786a2831/Makefile#L112-L116

What compiler are you using?

[barracuda156]

What compiler are you using?

@wch It is seen in the log above, GCC 12.

P. S. I am aware that latest versions of GCC can be configured to support libc++, but a) that is not default behavior, AFAIU, b) it does not work on some archs (ppc, ppcc64), c) older version of GCC do not support that at all, but can be used with R (C++11 support is there), d) using libc++ with GCC is largely untested even on Intel.

[wch]

Right, I see it's GCC 12. I'm wondering, though, what does the mp mean in gcc-mp-12?

I also see that you are building for ppc. It may be true that, strictly speaking, the flags in the Makefile should be conditional on the compiler. I'll note, though, that Makefile comes directly from a very widely used (though now dormant) project, libsass, and we would really prefer not to maintain our own fork of the code. There would have to be a very compelling reason for us to do that, and I don't think that supporting a dead platform like PowerPC merits modifying, testing, and maintaining our own version of libsass.

[barracuda156]

@wch mp is for Macports.

The problem is not PPC-specific though. Most of versions of GCC on Intel won’t support linking to libc++, and even those which do, it is an untested and undesirable behavior. The code gets away with these wrong settings simply because most people use Clang on MacOS. That does not make the code correct though. The test should not be OS-based, but either test for compiler used or C++ library directly.

[cpsievert]

It's seems that #130 might fix this. It'd be much appreciated if you could confirm @barracuda156 by installing the latest dev version -- remotes::install_github("rstudio/sass")

[barracuda156]

@cpsievert Thank you, I will try this out soon (and also re-run tests to see if that example still fails).

[barracuda156]

It is still broken, since it still hardcodes the wrong runtime for C++, and the source uses C++:

* installing *source* package ‘sass’ ...
** using staged installation
** libs
/opt/local/Library/Frameworks/R.framework/Resources/share/make/shlib.mk:18: warning: overriding commands for target `shlib-clean'
Makevars:12: warning: ignoring old commands for target `shlib-clean'
using C compiler: ‘gcc-mp-12 (MacPorts gcc12 12.3.0_0) 12.3.0’
using C++ compiler: ‘g++-mp-12 (MacPorts gcc12 12.3.0_0) 12.3.0’
Warning in system2("xcrun", "--show-sdk-path", TRUE, TRUE) :
  running command ''xcrun' --show-sdk-path 2>&1' had status 64
using SDK: ‘NA’‘NA’‘NA’‘NA’‘NA’‘NA’
/opt/local/Library/Frameworks/R.framework/Resources/share/make/shlib.mk:18: warning: overriding commands for target `shlib-clean'
Makevars:12: warning: ignoring old commands for target `shlib-clean'
/opt/local/bin/gcc-mp-12 -I"/opt/local/Library/Frameworks/R.framework/Resources/include" -DNDEBUG -I./libsass/include  -isystem/opt/local/include/LegacySupport -I/opt/local/include    -fPIC  -pipe -Os -arch ppc  -c compile.c -o compile.o
/opt/local/bin/g++-mp-12 -std=gnu++17 -I"/opt/local/Library/Frameworks/R.framework/Resources/include" -DNDEBUG -I./libsass/include  -isystem/opt/local/include/LegacySupport -I/opt/local/include    -fPIC  -pipe -Os -arch ppc  -c init.cpp -o init.o
MAKEFLAGS= CC="/opt/local/bin/gcc-mp-12" CFLAGS="-pipe -Os -arch ppc " CXX="/opt/local/bin/g++-mp-12 -std=gnu++17" AR="ar" LDFLAGS="-Wl,-headerpad_max_install_names -Wl,-rpath,/opt/local/lib/libgcc -L/opt/local/lib -lMacportsLegacySupport -arch ppc" make -C libsass
/opt/local/bin/gcc-mp-12 -pipe -Os -arch ppc  -O2 -I ./include  -fPIC -c -o src/cencode.o src/cencode.c
/opt/local/bin/g++-mp-12 -std=gnu++17 -Wall -O2 -std=c++11 -I ./include  -stdlib=libc++ -fPIC -c -o src/ast.o src/ast.cpp
g++-mp-12: error: unrecognized command-line option '-stdlib=libc++'
make[1]: *** [src/ast.o] Error 1
make: *** [libsass/lib/libsass.a] Error 2
ERROR: compilation failed for package ‘sass’

[barracuda156]

Why is that flag even hardcoded? Any sane compiler adds its primary runtime libs automatically. I think this is the only R package out of some 3000+ tested so far which has this bug.