Review for 7141242: build-infra merge: Rename CPP->CXX and LINK->LD
Erik Joelsson
erik.joelsson at oracle.com
Mon Feb 6 12:45:42 UTC 2012
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
More information about the build-dev
mailing list