Fixed CFLAGS and CXXFLAGS propagation for platform.mk to use

[thadguidry]

I tested this Brian and it seemed to propagate the CFLAGS and CXXFLAGS correctly now.

[yichoi]

I think no need to add above 3 lines

there is no CFLAGS, CXXFLAGS flags exported from config.mk
if user environments added like this, user environment flags automatically added may affect compile result.

[thadguidry]

I think you might have misunderstood the actual intent.
Without adding this fix, then the following command does not pass the flag -m32 during the build:

make VERBOSE=1 CFLAGS=-m32

Brian confirmed that this was a bug in the make files from what he saw as well, and this fix seemed the appropriate place for me to make the modification that allows the flags to pass. In my testing it seemed to work, but you guys are the experts, I'm still learning the make files and Make itself.

[yichoi]

using CFLAGS like make CFLAGS=xxxx could know explicitly which flags added.
however if user environments has like that (typically, CFLAGS is frequently defined like that.)

[jet@~] env
...
CFLAGS=XXXX
...

this CFLAGS will be automatically propagate rust build system accidentally.
I do not want to be used accidentally these kind of values which is sensitive to build.

If you insist to have cflag modification feature in command line with make.
I suggest to define not just CFLAGS but XXX_CFLAGS to minimize above risk.

However, I think most of CFLAGS of build should be well defined in build script and don't need to be changed.

[pnkfelix]

I think we can satisfy both @yichoi and @thadguidry by using the GNU make origin function.

Illustrated below:

% cat Makefile
default: printMSG

MSG="Hello World"
WHO="someone"

ifeq "$(origin WHO)" "environment"
  WHO_FROM_ENV = $(WHO)
endif

ifeq "$(origin WHO)" "environment override"
  WHO_FROM_ENV_OVERRIDE = $(WHO)
  MSG += $(WHO)
endif

ifeq "$(origin WHO)" "command line"
  WHO_FROM_CMD = $(WHO)
  MSG += $(WHO)
endif

printMSG:
    @echo $(MSG)
% make
Hello World
% make WHO=you
Hello World you
% WHO=you make
Hello World
% WHO=forced-via-dash-e make -e
Hello World forced-via-dash-e
% 

So, someone who wants their environment's CFLAGS to be included in the run can ask for it via the -e option to make, if we write the Makefile accordingly.

[yichoi]

@pnkfelix looks great !

I think below code will be good enough

ifeq "$(origin CFLAGS)" "command line"
    CFG_GCCISH_CFLAGS += $(CFLAGS)
endif
ifeq "$(origin CXXFLAGS)" "command line"
    CFG_GCCISH_CXXFLAGS += $(CXXFLAGS)
endif

[thadguidry]

I like the idea also.

For Windows users such as myself, modifying the CFLAGS is not going to be a
big deal, since our native environment does not really utilize them.
Giving Windows users this option to specify them at make command line will
be important for those trying to Build Rust on their Windows systems with
the scattering of toolchains and simulated Posix environments that we have
available.

And its good to know that there is a way that solves it without impacting
Linux or OSX users.

Kudos!

On Mon, Jul 1, 2013 at 7:55 AM, yichoi notifications@github.com wrote:

@pnkfelix https://github.com/pnkfelix looks great !

I think below code will be good enough

ifeq "$(origin CFLAGS)" "command line"
CFG_GCCISH_CFLAGS += $(CFLAGS)
endif
ifeq "$(origin CXXFLAGS)" "command line"
CFG_GCCISH_CXXFLAGS += $(CXXFLAGS)
endif

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/7494#issuecomment-20279741
.

-Thad
Thad on Freebase.com http://www.freebase.com/view/en/thad_guidry
Thad on LinkedIn http://www.linkedin.com/in/thadguidry/

[pnkfelix]

@thadguidry do you think you'll be able to incorporate the changes to use the origin function as discussed? I could try to do it myself, but I'd be more comfortable with you doing it since you know what concrete use case you have in mind for overriding CFLAGS and CXXFLAGS

[pnkfelix]

@thadguidry (or is this PR obsolete now that #7547 landed?)

[thadguidry]

@pnkfelix Yes, I think this PR is now obsolete since #7547 has got us further with gcc and g++ toolchains on Windows 64bit and MinGW.

[thadguidry]

Note, that in my testing adding @yichoi 's code snippet to platform.mk worked against a make statement of:

make CFLAGS="-Wall -Werror -g -m64 -D_WIN32_WINNT=0x0600"

So we can always add that back in to platform.mk if we really need command line arguments passed for the CFLAGS and CXXFLAGS.