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

[Bengt Rutisson]

2 feb 2012 kl. 12:29 skrev Staffan Larsen <staffan.larsen at oracle.com>:

> The .dsp files are all VS 6 artifacts and not used.

Ok. Thanks! That makes me happy.

But why are they copied around when we build VS 2010 projects?

Bengt


> 
> /Staffan
> 
> On 2 feb 2012, at 10:14, Bengt Rutisson wrote:
> 
>> On 2012-02-02 09:01, Staffan Larsen wrote:
>>> Those generated files are Visual Studio projects for VS version 6 (I think). Really old stuff. I don't think these are used (nor is VS 6 supported), so we should eventually clean out that code. I wouldn't bother fixing it.
>> 
>> Right. I thought WinGammaPlatformVC10 inherited from WinGammaPlatformVC6, but it seems to inherit from WinGammaPlatformVC7 which in turn does _not_ inherit form WinGammaPlatformVC6.
>> 
>> Still, my generated ADLC files contain CPP=cl.exe. So, I figured that they came from a call to the old implemntation. But maybe they are copied from this location?
>> 
>> make/windows/projectfiles/compiler2/ADLCompiler.dsp:28:CPP=cl.exe
>> make/windows/projectfiles/tiered/ADLCompiler.dsp:28:CPP=cl.exe
>> 
>> In that case, do these files need to be updated?
>> 
>> Bengt
>> 
>>> 
>>> /Staffan
>>> 
>>> 
>>> On 2 feb 2012, at 08:51, Bengt Rutisson wrote:
>>> 
>>>> Hi Erik,
>>>> 
>>>> I have not looked closely at your changes, so don't consider this a review. What I did do was apply your patch and try to create a Visual Studio project with the create.bat script. That still works. Nice!
>>>> 
>>>> One thing I noticed is that the ProjectCreator tool generates some files for the ADLC builds. These files still use the CPP name. Since it still works to create a project I don't know if this needs to be changed. But maybe it is good to be consistent.
>>>> 
>>>> Here's where we use the CPP name:
>>>> 
>>>> src\share\tools\ProjectCreator/WinGammaPlatformVC6.java:
>>>> 
>>>> 68:        printWriter.println("CPP=cl.exe");
>>>> 145:            printWriter.println("# SUBTRACT CPP /YX /Yc /Yu");
>>>> 149:            printWriter.println("# ADD CPP /Yc\"incls/_precompiled.incl\"");
>>>> 210:        rv.add("ADD CPP /nologo /MT /W3 /WX /GX /YX /Fr /FD /c");
>>>> 217:        rv.add("ADD BASE CPP "+Util.prefixed_join(" /I ", includes, true));
>>>> 218:        rv.add("ADD CPP "+Util.prefixed_join(" /I ", includes, true));
>>>> 219:        rv.add("ADD BASE CPP "+Util.prefixed_join(" /D ", defines, true));
>>>> 220:        rv.add("ADD CPP "+Util.prefixed_join(" /D ", defines, true));
>>>> 221:        rv.add("ADD CPP /Yu\"incls/_precompiled.incl\"");
>>>> 230:        rv.add("ADD BASE CPP /MD");
>>>> 231:        rv.add("ADD CPP /MD");
>>>> 252:        rv.add("ADD BASE CPP /Gm /Zi /O"+opt);
>>>> 272:        rv.add("ADD CPP /O"+getOptFlag());
>>>> 
>>>> And these are the generated files:
>>>> 
>>>> build\vs-amd64/compiler2/generated/ADLCompiler.dsp
>>>> build\vs-amd64/tiered/generated/ADLCompiler.dsp
>>>> 
>>>> 
>>>> Bengt
>>>> 
>>>> On 2012-02-02 03:33, David Holmes wrote:
>>>>> Hi Erik,
>>>>> 
>>>>> On 1/02/2012 7:13 PM, Erik Joelsson wrote:
>>>>>> http://cr.openjdk.java.net/~erikj/7141242/webrev.00/
>>>>>> 240 lines changed: 0 ins; 19 del; 221 mod; 6363 unchg
>>>>>> 
>>>>>> 7141242: build-infra merge: Rename CPP->CXX and LINK->LD
>>>>> Lots of CCC to CXX too :)
>>>>> 
>>>>> 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.
>>>>> 
>>>>> ---
>>>>> 
>>>>> make/bsd/makefiles/gcc.make
>>>>> 
>>>>> - CPP = $(CXX)
>>>>> + CXX = $(CXX)
>>>>> 
>>>>> infinite recursion or a tautology? :)
>>>>> 
>>>>> ---
>>>>> 
>>>>> make/*/makefiles/launcher.make
>>>>> 
>>>>> Not your doing but this has highlighted some strange rules eg:
>>>>> 
>>>>> + $(QUIETLY) $(CC) -g -o $@ -c $<  -MMD $(LAUNCHERFLAGS) $(CXXFLAGS)
>>>>> 
>>>>> C++ flags passed to C compiler?
>>>>> 
>>>>> ---
>>>>> 
>>>>> 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?
>>>>> 
>>>>> And again C++ flags passed to C compiler :(
>>>>> 
>>>>> ---
>>>>> 
>>>>> You missed a couple of scripts on Windows that use LINK_VER:
>>>>> 
>>>>> windows/get_msc_ver.sh
>>>>> windows/build_vm_def.sh
>>>>> 
>>>>> Cheers,
>>>>> David
>>>>> -----
>>>>> 
>>>>> 
>>>>>> The build-infra project is starting to move into jdk8. For the hotspot
>>>>>> build to stay compatible with the changes, the naming of standard make
>>>>>> variables, like CC and LD need to be standardized across the build.
>>>>>> Currently hotspot names the C++ compiler CPP, which is traditionally the
>>>>>> name of the preprocessor. The windows nmake files name the linker LINK.
>>>>>> We would like to rename the C++ compiler to CXX and have the linker
>>>>>> named LD everywhere.
>>>>>> 
>>>>>> Patch is tested with hsx/hotspot-rt. Testing with jdk7u is in progress.
>>>>>> 
>>>>>> /Erik
>>>>>> 
>>>>>> 
>> 
>