[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    Request for review (XS): 7140909 Visual Studio project builds broken: need to define INCLUDE_TRACE
From:       kelly.ohair () oracle ! com (Kelly O'Hair)
Date:       2012-01-30 17:30:07
Message-ID: D4E7AE07-004A-493D-A11E-8EE00692FEDA () oracle ! com
[Download RAW message or body]


On Jan 30, 2012, at 9:14 AM, Bengt Rutisson wrote:

> 
> Hi Kelly,
> 
> On 2012-01-30 17:55, Kelly O'Hair wrote:
> > Poking my nose in...
> 
> > -)
> 
> > It has been my experience that using global macros with very generic names is \
> > dangerous. If some day we update to Visual Studio 2032, and they decided to use \
> > the macro name INCLUDE_TRACE for something, we could have some interesting bug \
> > reports. 
> > If this is directly related to hotspot or jfr or anything we have created, I \
> > think it would be a much better idea, and also helpful to the code reader, to \
> > name it a little more uniquely. Like JFR_INCLUDE_TRACE or something that \
> > indicates what it is for or from.
> 
> I think you have a valid point, but I think we should ask the JFR team to handle \
> this as a separate task. With the change that I am proposing I am just adapting the \
> Visual Studio projects to what the JFR team did to the source and make files. 
> So, if it is ok with you I'd like to go ahead and push this change. Then we can \
> bring up the naming discussion with the JFR team as a separate issue.

That is fine.

Removing nose. ;^)

-kto

> 
> Thanks for looking at this!
> Bengt
> 
> > 
> > -kto
> > 
> > On Jan 30, 2012, at 6:21 AM, Bengt Rutisson wrote:
> > 
> > > Hi all,
> > > 
> > > Could I please have a couple of review for this really small change:
> > > http://cr.openjdk.java.net/~brutisso/7140909/webrev.01/
> > > 
> > > There is only one line changed, so I'll include the diff here as well:
> > > 
> > > Background:
> > > 
> > > The fix for "7115199: Add event tracing hooks and Java Flight Recorder \
> > > infrastructure" added this to make\windows\makefiles\vm.make: 
> > > !ifndef JAVASE_EMBEDDED
> > > CPP_FLAGS=$(CPP_FLAGS) /D "INCLUDE_TRACE"
> > > !endif
> > > 
> > > To keep the Visual Studio projects working the same type of change needs to be \
> > > made to src\share\tools\ProjectCreator\BuildConfig.java 
> > > This will only affect Visual Studio projects created with the ProjectCreator \
> > > tool (create.bat). This does not affect "normal" Windows builds or JPRT builds. \
> > >  Thanks,
> > > Bengt
> > > 
> 


[prev in list] [next in list] [prev in thread] [next in thread]