headers
Erik Joelsson | 1 Feb 10:13
Picon
Favicon

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

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

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
Permalink | Reply | 14 comments |
headers
Fredrik Öhrström | 1 Feb 13:38
Picon
Favicon

Re: Patch for: 7132779: build-infra merge: Enable ccache to work for most developer builds.

2012-01-31 22:38, David Holmes skrev:
> I don't understand the comment on the new method though. 

Since jre_release_version() has a similar use case as vm_release(), ie
it is called from crash handlers and error reporting handlers,
I copied the comment for vm_release() to jre_release_version(). But
perhaps it is a bit too much information
since it is not that likely that the code will be refactored to have
stringStreams instead of char *.

> I'm assuming that the actual problem is the ccache compares the
> defines used for a compilation and so if the version string contains a
> timestamp and gets updated on each (incremental?) build then ccache
> thinks it has to recompile.

Yes, that is correct. As can be seen in most build server logs, the
ccache is rarely used. It is only used when a build number is explicitly
set, since the build number prevents a date and time to be inserted in
the JRE_RELEASE_VERSION.

//Fredrik
Permalink | Reply | 0 comments |
headers
Bengt Rutisson | 2 Feb 08:51
Picon
Favicon

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


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");
(Continue reading)

Permalink | Reply | 6 comments |
headers
Staffan Larsen | 2 Feb 09:01
Picon
Favicon

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

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.

/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));
(Continue reading)

Permalink | Reply | 5 comments |
headers
Erik Joelsson | 2 Feb 09:58
Picon
Favicon

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

Hello,

I've intentionally left out all VS project files. I'm not sure but I 
suspect that CPP is some kind of standard name for the compiler in that 
context. I'm happy to hear I didn't mess up the project creation!

/Erik

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.
>
> /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:
(Continue reading)

Permalink | Reply | 0 comments |
headers
Bengt Rutisson | 2 Feb 10:14
Picon
Favicon

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

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!
(Continue reading)

Permalink | Reply | 3 comments |
headers
Erik Joelsson | 2 Feb 10:59
Picon
Favicon

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

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.
(Continue reading)

Permalink | Reply | 5 comments |
headers
Staffan Larsen | 2 Feb 12:29
Picon
Favicon

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

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

/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:
>>
(Continue reading)

Permalink | Reply | 2 comments |
headers
Bengt Rutisson | 2 Feb 12:42
Picon
Favicon

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


2 feb 2012 kl. 12:29 skrev Staffan Larsen <staffan.larsen@...>:

> 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
>>
(Continue reading)

Permalink | Reply | 1 comment |
headers
Staffan Larsen | 2 Feb 13:07
Picon
Favicon

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

On 2 feb 2012, at 12:42, Bengt Rutisson wrote:

> 2 feb 2012 kl. 12:29 skrev Staffan Larsen <staffan.larsen@...>:
> 
>> 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?

Who knows… Old code that hasn't been updated, I guess.

/Staffan
Permalink | Reply | 0 comments |

Gmane