Review for 7141242: build-infra merge: Rename CPP->CXX and LINK->LD (original) (raw)
Erik Joelsson erik.joelsson at oracle.com
Mon Feb 6 12:45:42 UTC 2012
- Previous message (by thread): Review for 7141242: build-infra merge: Rename CPP->CXX and LINK->LD
- Next message (by thread): Patch to fix build breakage with GCC 4.7
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
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 CPPFLAGS or LINKFLAGS 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)istheccompiler(cc/gcc),(CC) is the c compiler (cc/gcc), (CC)istheccompiler(cc/gcc),(CCC) is the c++ compiler (CC/g++). -CCOMPILE = (CC)(CC) (CC)(CPPFLAGS) $(CFLAGS) -CCCOMPILE = (CCC)(CCC) (CCC)(CPPFLAGS) $(CFLAGS) +# (CC)istheccompiler(cc/gcc),(CC) is the c compiler (cc/gcc), (CC)istheccompiler(cc/gcc),(CXX) is the c++ compiler (CC/g++). +CCOMPILE = (CC)(CC) (CC)(CXXFLAGS) $(CFLAGS) +CCCOMPILE = (CXX)(CXX) (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 CCCOMPILE. Is it worth changing these to CCCOMPILE and CXXCOMPILE? 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: CCCOMPILE -> CXXCOMPILE CCOMPILE -> CCCOMPILE *.CC -> *.CXX *.c -> *.CC Removed *.cpp as they weren't used (* is COMPILE, GENASM, LINK, LINKLIB 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 LINKVER: windows/getmscver.sh windows/buildvmdef.sh I skipped the scripts as it didn't seem needed for my purposes, but included them in the new webrev. /Erik
- Previous message (by thread): Review for 7141242: build-infra merge: Rename CPP->CXX and LINK->LD
- Next message (by thread): Patch to fix build breakage with GCC 4.7
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]