thezbyg / gpick

Advanced color picker written in C++ using GTK+ toolkit
BSD 3-Clause "New" or "Revised" License
381 stars 34 forks source link

Scons build system doesn't use -std=c++14 #157

Closed ryandesign closed 6 years ago

ryandesign commented 6 years ago

The new cmake build system uses the -std=c++14 flag, but the scons build system doesn't, which causes build failures if the compiler doesn't default to C++14 or newer.

thezbyg commented 6 years ago

When using SCons build system, -std=c++14 flag is not set if one or more of the following environment variables is set: CFLAGS, CXXFLAGS, LDFLAGS. See SConscript line 110. This is done so that all compiler/linker flags could be controlled by setting environment variables.

I have added additional code to test if CXXFLAGS does not contain -std= flag, and set default value if that is the case (see commit ec1c7ab372511eee0a89eb6cd175bf56bb0b5fa0).

Let me know if this fixes your issue.

ryandesign commented 6 years ago

When using SCons build system, -std=c++14 flag is not set if one or more of the following environment variables is set: CFLAGS, CXXFLAGS, LDFLAGS. See SConscript line 110. This is done so that all compiler/linker flags could be controlled by setting environment variables.

Hmm, that's unexpected. As a user, I expect to be able to inform your build system of some things, such as the paths to headers and libraries (-I flags in CPPFLAGS, -L flags in LDFLAGS), the C++ library to use (-stdlib flag in CXXFLAGS), the architectures and optimization flags to use (-arch and -O flags in CFLAGS and CXXFLAGS). Meanwhile, I don't know what other flags your project requires. I as a user shouldn't have to know that your project requires C++14 or which compiler warnings your project is expected to compile cleanly with, etc., so you should add those flags even if I specify some flags of my own. You may need to evaluate on a flag-by-flag basis whether the flag should be appended after or prepended in front of user flags. So thanks for addressing this for the -std flag but keep it in mind for other flags you have now or might add in the future. And you might want to restructure the code so that you don't have to handle the same flag in two places.

I have added additional code to test if CXXFLAGS does not contain -std= flag, and set default value if that is the case (see commit ec1c7ab).

Let me know if this fixes your issue.

Thanks. The problem seems to be that you're adding the -std=c++14 flag to CPPFLAGS, which causes the build to fail once it tries to compile any C code:

error: invalid argument '-std=c++14' not allowed with 'C'

You should only add the flag to CXXFLAGS.

ryandesign commented 6 years ago

Thanks, looks like 3d79262f20b00a0f899a46096de7b1dcac763e04 fixed it.