Re: Review for 7141242: build-infra merge: Rename CPP->CXX and LINK->LD

[Erik Joelsson]

New webrev posted:
http://cr.openjdk.java.net/~erikj/7141242/webrev.02/ 
<http://cr.openjdk.java.net/%7Eerikj/7141242/webrev.02/>

Fredriks change for ccache added this line to all vm.make files:

vm_version.o: CPPFLAGS += ${JRE_VERSION}

which had to be replaced by

vm_version.o: CXXFLAGS += ${JRE_VERSION}

/Erik

On 2012-02-03 02:43, David Holmes wrote:
Hi Erik,

I think this has gone as far as it needs for now. My visual inspection 
of these changes looks okay.

My lingering concern is the impact on external scripts etc that may 
set some of the renamed flags. Even I have a build script that sets 
things so that I don't get complaints about using the wrong compiler 
version on Solaris.

David

On 2/02/2012 7:59 PM, Erik Joelsson wrote:
Hello David,

Thanks for taking a look!

New webrev here: http://cr.openjdk.java.net/~erikj/7141242/webrev.01/
JPRT job running.

In this version a lot more has changes, see comments inline.

On 2012-02-02 03:33, David Holmes wrote:
Hi Erik,

Lots of CCC to CXX too :)

Right, it looked to me like CCC was used in rules.make by someone who
didn't like using CPP for the C++ compiler. I couldn't see any need for
an intermediate variable there, just extra confusion.

One compatibility concern: anyone currently setting CPP_FLAGS or
LINK_FLAGS etc, externally, will need to change to the new names.
Probably worth sending a wider email (jdk8-dev?) when this gets pushed.

Good point. We will need to send it out both to jdk8 and jdk7 consumers
as this will (unfortunately) also hit 7u4.
make/bsd/makefiles/gcc.make

- CPP = $(CXX)
+ CXX = $(CXX)
Thanks for spotting that. Fixed in new webrev. I think I've created
variations on this patch too many times now.

C++ flags passed to C compiler?

That looks weird yes. I don't dare changing it in the scope of this work
though.
make/*/makefiles/rules.make

-# $(CC) is the c compiler (cc/gcc), $(CCC) is the c++ compiler 
(CC/g++).
-C_COMPILE = $(CC) $(CPPFLAGS) $(CFLAGS)
-CC_COMPILE = $(CCC) $(CPPFLAGS) $(CFLAGS)
+# $(CC) is the c compiler (cc/gcc), $(CXX) is the c++ compiler 
(CC/g++).
+C_COMPILE = $(CC) $(CXXFLAGS) $(CFLAGS)
+CC_COMPILE = $(CXX) $(CXXFLAGS) $(CFLAGS)

The original code is confusing, given that CC is the C compiler it
makes no sense that a C++ compile be called CC_COMPILE. Is it worth
changing these to CC_COMPILE and CXX_COMPILE? Maybe a secondary 
cleanup?

Either a secondary cleanup or all at once. The new webrev deals with
these and the related COMPILE.CC. These changes aren't needed for
build-infra but they sure make the code clearer. Basically:

CC_COMPILE -> CXX_COMPILE
C_COMPILE -> CC_COMPILE
*.CC -> *.CXX
*.c -> *.CC
Removed *.cpp as they weren't used
(* is COMPILE, GENASM, LINK, LINK_LIB and PREPROCESS)

Question is, how far do we want to go? With these changes, we have
consistent naming of CC and CXX in all cases that I have found.

You missed a couple of scripts on Windows that use LINK_VER:

windows/get_msc_ver.sh
windows/build_vm_def.sh
I skipped the scripts as it didn't seem needed for my purposes, but
included them in the new webrev.

/Erik