Re: Review request for 7192302 Remove JDBCRowSetImpl dependency on java.beans

2012-09-06 Thread Lance Andersen - Oracle
Here is the updated webrev http://cr.openjdk.java.net/~lancea/7192302/webrev.01

I know there is more clean-up that can be done to remove other Rave added code 
(such as the removal of set/getPreparedStatement/Connection/ResultSet), I want 
to keep the focus to just removing PropertyChangeSupport.  SQE and RowSet TCKs 
continue to pass with these changes.

Best
Lance
On Sep 5, 2012, at 5:17 PM, Alan Bateman wrote:

 On 05/09/2012 22:04, Lance Andersen - Oracle wrote:
 Hi all,
 
 Looking for a reviewer for the removal of PropertyChangeSupport from 
 JDBCRowSetImpl that was originally going to be used by the EOL Rave product. 
  As it is no longer needed the code has been removed.  The SQE and RowSet 
 TCK tests all continue to run without regression.
 
 The webrev can be found at 
 http://cr.openjdk.java.net/~lancea/7192302/webrev.00
 
 Thanks Lance, it's good to remove this dependency. In both commit and 
 rollback then it looks to me that the setting of oldVal can be removed. 
 Otherwise looks good to me.
 
 -Alan


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: Review request for 7192302 Remove JDBCRowSetImpl dependency on java.beans

2012-09-06 Thread Alan Bateman

On 06/09/2012 13:40, Lance Andersen - Oracle wrote:

Here is the updated webrev http://cr.openjdk.java.net/~lancea/7192302/webrev.01

I know there is more clean-up that can be done to remove other Rave added code 
(such as the removal of set/getPreparedStatement/Connection/ResultSet), I want 
to keep the focus to just removing PropertyChangeSupport.  SQE and RowSet TCKs 
continue to pass with these changes.


Your previous mail uses the word nuke, I was thinking the same thing :-)

The latest webrev looks okay except that in one of the constructors you 
have removed a call to ensure that the connection is established, I'm 
not sure about the significance of that.


I also see there are a couple of residual references to Rave that can 
probably be pulled, these seem to be related to the 
PropertyChangeListener support.


One method that looks like it could be removed too is setConcurrency but 
I agree that keeping focused on just removing the beans dependency is 
right for now.


-Alan.


Re: Review request for 7192302 Remove JDBCRowSetImpl dependency on java.beans

2012-09-06 Thread Lance Andersen - Oracle
Thank you for the comments Alan

On Sep 6, 2012, at 9:00 AM, Alan Bateman wrote:

 On 06/09/2012 13:40, Lance Andersen - Oracle wrote:
 Here is the updated webrev 
 http://cr.openjdk.java.net/~lancea/7192302/webrev.01
 
 I know there is more clean-up that can be done to remove other Rave added 
 code (such as the removal of set/getPreparedStatement/Connection/ResultSet), 
 I want to keep the focus to just removing PropertyChangeSupport.  SQE and 
 RowSet TCKs continue to pass with these changes.
 
 Your previous mail uses the word nuke, I was thinking the same thing :-)
 
 The latest webrev looks okay except that in one of the constructors you have 
 removed a call to ensure that the connection is established, I'm not sure 
 about the significance of that.

This is not needed here and given I have already tested with this removed, I 
figured I would OK to keep this as part of the change
 
 I also see there are a couple of residual references to Rave that can 
 probably be pulled, these seem to be related to the PropertyChangeListener 
 support.

I left those in as a reminder to go back as part of the rest of the Rave 
clean-up.  I would prefer to leave them for now and when I  make another pass 
for Rave, I will get rid of them
 
 One method that looks like it could be removed too is setConcurrency but I 
 agree that keeping focused on just removing the beans dependency is right for 
 now.

 I had thought about that but I have to think about this one a bit more as the 
getConcurrency() is leveraging the value returned from the active ResultSet 
which is why I did not remove this at this time.

In some cases, I must confess the original authors have left me scratching my 
head on some of this code but I want to be surgical in removing or addressing 
changes so that it would be easier to find any un-intended gotchas.

Best
Lance

 
 -Alan.


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: Review request for 7192302 Remove JDBCRowSetImpl dependency on java.beans

2012-09-06 Thread Alan Bateman

On 06/09/2012 14:09, Lance Andersen - Oracle wrote:

:


The latest webrev looks okay except that in one of the constructors 
you have removed a call to ensure that the connection is established, 
I'm not sure about the significance of that.


This is not needed here and given I have already tested with this 
removed, I figured I would OK to keep this as part of the change
Okay, I'll have to trust you on this one but I am a little bit concerned 
that it could cause a NPE, say someone creates a JdbcRowSet and invokes 
a method such as comment, rollback or getAutoCommit without doing an 
explicit connect.





I left those in as a reminder to go back as part of the rest of the 
Rave clean-up.  I would prefer to leave them for now and when I  make 
another pass for Rave, I will get rid of them

Okay.



One method that looks like it could be removed too is setConcurrency 
but I agree that keeping focused on just removing the beans 
dependency is right for now.


 I had thought about that but I have to think about this one a bit 
more as the getConcurrency() is leveraging the value returned from the 
active ResultSet which is why I did not remove this at this time.


Okay although it looks to be just an optimization introduced as part of 
adding listener events.  I'm fine with leaving it as it is.


-Alan


Re: Review request for 7192302 Remove JDBCRowSetImpl dependency on java.beans

2012-09-06 Thread Lance Andersen - Oracle

On Sep 6, 2012, at 9:31 AM, Alan Bateman wrote:

 On 06/09/2012 14:09, Lance Andersen - Oracle wrote:
 :
 
 The latest webrev looks okay except that in one of the constructors you 
 have removed a call to ensure that the connection is established, I'm not 
 sure about the significance of that.
 
 This is not needed here and given I have already tested with this removed, I 
 figured I would OK to keep this as part of the change
 Okay, I'll have to trust you on this one but I am a little bit concerned that 
 it could cause a NPE, say someone creates a JdbcRowSet and invokes a method 
 such as comment, rollback or getAutoCommit without doing an explicit connect.

connect() is typically called to get a connection and it will validate that 
there is a non-null Connection.  I have unit tests which leverage that 
constructor and run clean with that change.

I agree that commit(), rollback, etc  should call checkState() and that is 
another fix I need to do but the potential for the NPE is there depending on 
which constructor was used and if you did something silly like making a 
rollback without doing any work.

I can revert that change if you like but I have not seen any failures under 
what I would term normal use.
 
 
 
 I left those in as a reminder to go back as part of the rest of the Rave 
 clean-up.  I would prefer to leave them for now and when I  make another 
 pass for Rave, I will get rid of them
 Okay.
 
 
 One method that looks like it could be removed too is setConcurrency but I 
 agree that keeping focused on just removing the beans dependency is right 
 for now.
 
 I had thought about that but I have to think about this one a bit more as 
 the getConcurrency() is leveraging the value returned from the active 
 ResultSet which is why I did not remove this at this time.
 
 Okay although it looks to be just an optimization introduced as part of 
 adding listener events

I am not sure I agree with the code either, just want to spend a bit more time 
looking at the code flow and see what tests if any we have here
  I'm fine with leaving it as it is.

thank you
 
 -Alan


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: Review request for 7192302 Remove JDBCRowSetImpl dependency on java.beans

2012-09-06 Thread Alan Bateman

On 06/09/2012 14:45, Lance Andersen - Oracle wrote:


:

connect() is typically called to get a connection and it will validate 
that there is a non-null Connection.  I have unit tests which leverage 
that constructor and run clean with that change.


I agree that commit(), rollback, etc  should call checkState() and 
that is another fix I need to do but the potential for the NPE is 
there depending on which constructor was used and if you did something 
silly like making a rollback without doing any work.


I'm okay with what you have if you make sure to submit a bug as a 
reminder to clean-up this code.


-Alan


Re: Review request for 7192302 Remove JDBCRowSetImpl dependency on java.beans

2012-09-06 Thread Mandy Chung

Lance,

On 9/6/2012 5:40 AM, Lance Andersen - Oracle wrote:

Here is the updated webrev http://cr.openjdk.java.net/~lancea/7192302/webrev.01


It's good to see this change that jdbc rowset doesn't depend on java.beans.

This looks okay to me as you have explained why you remove the call to 
connect() in one of the constructors.  Alan and you discussed if 
setConcurrency could be removed in this fix. setType() is another one 
that could be removed.   I'm fine with leaving these cleanup in another 
fix and have this fix to focus on removing the dependency of 
PropertyChangeSupport.  It'd be good to file a CR as a follow-up of this 
work.


Thanks
Mandy


I know there is more clean-up that can be done to remove other Rave added code 
(such as the removal of set/getPreparedStatement/Connection/ResultSet), I want 
to keep the focus to just removing PropertyChangeSupport.  SQE and RowSet TCKs 
continue to pass with these changes.

Best
Lance
On Sep 5, 2012, at 5:17 PM, Alan Bateman wrote:


On 05/09/2012 22:04, Lance Andersen - Oracle wrote:

Hi all,

Looking for a reviewer for the removal of PropertyChangeSupport from 
JDBCRowSetImpl that was originally going to be used by the EOL Rave product.  
As it is no longer needed the code has been removed.  The SQE and RowSet TCK 
tests all continue to run without regression.

The webrev can be found at http://cr.openjdk.java.net/~lancea/7192302/webrev.00


Thanks Lance, it's good to remove this dependency. In both commit and rollback 
then it looks to me that the setting of oldVal can be removed. Otherwise looks 
good to me.

-Alan


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com



hg: jdk8/tl/jdk: 7195557: NPG: Unexpected number of memory pools

2012-09-06 Thread nils . loodin
Changeset: 076d0dafea5f
Author:mgerdin
Date:  2012-09-06 14:07 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/076d0dafea5f

7195557: NPG: Unexpected number of memory pools
Summary: Update management tests to work with a VM without a permanent 
generation memory pool
Reviewed-by: rbackman, brutisso, sla, dholmes

! test/java/lang/management/MemoryMXBean/CollectionUsageThreshold.java
! test/java/lang/management/MemoryMXBean/MemoryTest.java



Fwd: Need reviewers: more predictable binaries

2012-09-06 Thread Kelly O'Hair

Just FYI...

these build changes do touch sources in various teams, please let me know if 
you have issues with these changes.

-kto

Begin forwarded message:

 From: Kelly O'Hair kelly.oh...@oracle.com
 Subject: Need reviewers: more predictable binaries
 Date: September 5, 2012 9:08:53 PM PDT
 To: build-...@openjdk.java.net build-dev build-...@openjdk.java.net
 
 
 Need a reviewer for this change.
 
   http://cr.openjdk.java.net/~ohair/openjdk8/jdk8-this-file/webrev/
 
 It does change source, but it's effectively a build change.
 
 The goal here is to try and create more predictable binary files and remove 
 the possibility that
 a full source path (via __FILE__) gets baked into the shared libraries or 
 executables shipped.
 
 So two changes:
  * sort the .o files during links so they are always provided to the linker 
 in the same order.
  * replace use of __FILE__ to the macro THIS_FILE with just the basename of 
 the source file being compiled
 
 The __FILE__ issue is that it has an implementation defined string literal 
 value, depending on the compiler
 and the filename supplied on the compile line. By changing to the new 
 THIS_FILE macro, the object files
 will always have a consistent string literal in them, making it easier to 
 compare binaries between builds.
 Something we have never been able to do in the past. Granted it's just the 
 basename for the file, but should be enough.
 The tricky part is that __FILE__ only gets evaluated when it is used. In 
 normal C code, it will appear in
 macros but it only will get evaluated in the source file being compiled. It 
 is rare that macros using
 __FILE__  will get expanded in include files and I detected this not 
 happening in the jdk source at all.
 
 -kto



Re: Review request for 7192302 Remove JDBCRowSetImpl dependency on java.beans

2012-09-06 Thread Lance Andersen - Oracle
Hi Mandy,

Thank you also for the feedback along with Alan. I will push these back shortly 
and also create a CR.

Best
Lance
On Sep 6, 2012, at 11:04 AM, Mandy Chung wrote:

 Lance,
 
 On 9/6/2012 5:40 AM, Lance Andersen - Oracle wrote:
 Here is the updated webrev 
 http://cr.openjdk.java.net/~lancea/7192302/webrev.01
 
 It's good to see this change that jdbc rowset doesn't depend on java.beans.
 
 This looks okay to me as you have explained why you remove the call to 
 connect() in one of the constructors.  Alan and you discussed if 
 setConcurrency could be removed in this fix. setType() is another one that 
 could be removed.   I'm fine with leaving these cleanup in another fix and 
 have this fix to focus on removing the dependency of PropertyChangeSupport.  
 It'd be good to file a CR as a follow-up of this work.
 
 Thanks
 Mandy
 
 I know there is more clean-up that can be done to remove other Rave added 
 code (such as the removal of set/getPreparedStatement/Connection/ResultSet), 
 I want to keep the focus to just removing PropertyChangeSupport.  SQE and 
 RowSet TCKs continue to pass with these changes.
 
 Best
 Lance
 On Sep 5, 2012, at 5:17 PM, Alan Bateman wrote:
 
 On 05/09/2012 22:04, Lance Andersen - Oracle wrote:
 Hi all,
 
 Looking for a reviewer for the removal of PropertyChangeSupport from 
 JDBCRowSetImpl that was originally going to be used by the EOL Rave 
 product.  As it is no longer needed the code has been removed.  The SQE 
 and RowSet TCK tests all continue to run without regression.
 
 The webrev can be found at 
 http://cr.openjdk.java.net/~lancea/7192302/webrev.00
 
 Thanks Lance, it's good to remove this dependency. In both commit and 
 rollback then it looks to me that the setting of oldVal can be removed. 
 Otherwise looks good to me.
 
 -Alan
 
 Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
 Oracle Java Engineering
 1 Network Drive
 Burlington, MA 01803
 lance.ander...@oracle.com
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



hg: jdk8/tl/jdk: 7192302: Remove JDBCRowSetImpl dependency on java.beans

2012-09-06 Thread lance . andersen
Changeset: 8c6895afe204
Author:lancea
Date:  2012-09-06 13:16 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/8c6895afe204

7192302: Remove JDBCRowSetImpl dependency on java.beans
Reviewed-by: alanb, mchung

! src/share/classes/com/sun/rowset/JdbcRowSetImpl.java



Re: Review request for 7192302 Remove JDBCRowSetImpl dependency on java.beans

2012-09-06 Thread Alan Bateman

Lance,

I see you've just pushed this but one thing I didn't spot initially is 
that in the second webrev you changed the protected connect method to be 
private. Is this a supported API and could this cause problems for JDBC 
drivers that extend this?


-Alan


Re: Review request for 7192302 Remove JDBCRowSetImpl dependency on java.beans

2012-09-06 Thread Lance Andersen - Oracle
Hi Alan,

The connect method is used by the RI not the RowSet spec.  It was made 
protected for Rave.

Best
Lance
On Sep 6, 2012, at 1:21 PM, Alan Bateman wrote:

 Lance,
 
 I see you've just pushed this but one thing I didn't spot initially is that 
 in the second webrev you changed the protected connect method to be private. 
 Is this a supported API and could this cause problems for JDBC drivers that 
 extend this?
 
 -Alan


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: Review request for 7192302 Remove JDBCRowSetImpl dependency on java.beans

2012-09-06 Thread Mandy Chung
As the comment noted, the connect method was made protected for Rave 
requirement but it's not defined in JdbcRowSet and BaseRowSet.  It's a 
method specific to the RI and changing it to private is okay if they get 
an JdbcRowSet instance by calling the rowset factory.  Alan's question 
prompts me to look if anyone references JdbcRowSetImpl directly - I find 
that the javax.sql.rowset.JdbcRowSet spec has the example to instantiate 
JdbcRowSetImpl.  Is it still a supported or recommended way?  If so, 
there is a potential compatibility risk; otherwise, it'd be good to 
update the spec to remove reference to JdbcRowSetImpl.


Mandy

On 9/6/2012 10:21 AM, Alan Bateman wrote:

Lance,

I see you've just pushed this but one thing I didn't spot initially is 
that in the second webrev you changed the protected connect method to 
be private. Is this a supported API and could this cause problems for 
JDBC drivers that extend this?


-Alan


Re: Review request for 7192302 Remove JDBCRowSetImpl dependency on java.beans

2012-09-06 Thread Lance Andersen - Oracle
The recommended way to access a rowset is via the factory now.For whatever 
reason the original authors chose not to provide a factory.

The connect() method has always been a method to be used internally by 
JdbcRowSetRIImpl and Rave.  There is no reason for a user of the API to ever 
have to call this method.   I can certainly update the javadocs for JdbcRowSet 
to use the factory but all of the examples as well as the tutorial always 
leveraged using the defined API.

Best
Lance
On Sep 6, 2012, at 1:54 PM, Mandy Chung wrote:

 As the comment noted, the connect method was made protected for Rave 
 requirement but it's not defined in JdbcRowSet and BaseRowSet.  It's a method 
 specific to the RI and changing it to private is okay if they get an 
 JdbcRowSet instance by calling the rowset factory.  Alan's question prompts 
 me to look if anyone references JdbcRowSetImpl directly - I find that the 
 javax.sql.rowset.JdbcRowSet spec has the example to instantiate 
 JdbcRowSetImpl.  Is it still a supported or recommended way?  If so, there is 
 a potential compatibility risk; otherwise, it'd be good to update the spec to 
 remove reference to JdbcRowSetImpl.
 
 Mandy
 
 On 9/6/2012 10:21 AM, Alan Bateman wrote:
 Lance,
 
 I see you've just pushed this but one thing I didn't spot initially is that 
 in the second webrev you changed the protected connect method to be private. 
 Is this a supported API and could this cause problems for JDBC drivers that 
 extend this?
 
 -Alan


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: [OpenJDK 2D-Dev] Fwd: Need reviewers: more predictable binaries

2012-09-06 Thread Scott Kovatch
I feel like I should be able to find the answer to this, but does Objective-C 
use CPPFLAGS? I can't tell if these changes would apply to .m or .mm files. It 
certainly appears to be the intent of the change, since a number of .m files in 
the AWT were changed to use THIS_FILE.

-- Scott K.

On Sep 6, 2012, at 9:30 AM, Kelly O'Hair kelly.oh...@oracle.com wrote:

 
 Just FYI...
 
 these build changes do touch sources in various teams, please let me know if 
 you have issues with these changes.
 
 -kto
 
 Begin forwarded message:
 
 From: Kelly O'Hair kelly.oh...@oracle.com
 Subject: Need reviewers: more predictable binaries
 Date: September 5, 2012 9:08:53 PM PDT
 To: build-...@openjdk.java.net build-dev build-...@openjdk.java.net
 
 
 Need a reviewer for this change.
 
   http://cr.openjdk.java.net/~ohair/openjdk8/jdk8-this-file/webrev/
 
 It does change source, but it's effectively a build change.
 
 The goal here is to try and create more predictable binary files and remove 
 the possibility that
 a full source path (via __FILE__) gets baked into the shared libraries or 
 executables shipped.
 
 So two changes:
  * sort the .o files during links so they are always provided to the linker 
 in the same order.
  * replace use of __FILE__ to the macro THIS_FILE with just the basename of 
 the source file being compiled
 
 The __FILE__ issue is that it has an implementation defined string literal 
 value, depending on the compiler
 and the filename supplied on the compile line. By changing to the new 
 THIS_FILE macro, the object files
 will always have a consistent string literal in them, making it easier to 
 compare binaries between builds.
 Something we have never been able to do in the past. Granted it's just the 
 basename for the file, but should be enough.
 The tricky part is that __FILE__ only gets evaluated when it is used. In 
 normal C code, it will appear in
 macros but it only will get evaluated in the source file being compiled. It 
 is rare that macros using
 __FILE__  will get expanded in include files and I detected this not 
 happening in the jdk source at all.
 
 -kto
 



[8] Review request for 7196354 check-in jdk.tbom file to openjdk repo

2012-09-06 Thread Michael Fang

Hello,

Please help to review the new JDK8 file for the following CR:
7196354 check-in jdk.tbom file to openjdk repo

The webrev is located at:
http://cr.openjdk.java.net/~mfang/7196354/webrev.00/

Build-dev:
The file will be pushed to the top level openjdk repository 
http://hg.openjdk.java.net/jdk8/build. This file is to be used as 
translation bill of material file for automated translation drop system 
to zip up the list of English resource files to be delivered to 
translation team.


Mark:
Since the dev team will need to maintain this file in the future 
(modifying it if you add or delete resource files), I temporarily put 
down your name as contact for the file. Please figure out the proper 
owner and we can update it again.


Core-libs-dev:
I believe most of the resource files are used for your product area. 
Please take a look and let me know if you have any concerns, or know of 
any files to be added or deleted at this time.


In the future, if any bug/rfe requires adding/deleting resource files, 
the dev work should include updating this file to reflect the correct 
resource file list. (and please ask me to review it).


I18n-dev:
Also included the group mailing list since this is part of l10n team's 
deliverable.


thanks,

-michael


Re: AWT Dev [OpenJDK 2D-Dev] Fwd: Need reviewers: more predictable binaries

2012-09-06 Thread Mike Swingler
My strong suspicion is that the JDK Makefiles only use CFLAGS, not CPPFLAGS for 
.m files. CPPFLAGS should be used for .mm files (but those should be really 
rare).

Regards,
Mike Swingler
Apple Inc.

On Sep 6, 2012, at 11:35 AM, Scott Kovatch scott.kova...@oracle.com wrote:

 I feel like I should be able to find the answer to this, but does Objective-C 
 use CPPFLAGS? I can't tell if these changes would apply to .m or .mm files. 
 It certainly appears to be the intent of the change, since a number of .m 
 files in the AWT were changed to use THIS_FILE.
 
 -- Scott K.
 
 On Sep 6, 2012, at 9:30 AM, Kelly O'Hair kelly.oh...@oracle.com wrote:
 
 
 Just FYI...
 
 these build changes do touch sources in various teams, please let me know if 
 you have issues with these changes.
 
 -kto
 
 Begin forwarded message:
 
 From: Kelly O'Hair kelly.oh...@oracle.com
 Subject: Need reviewers: more predictable binaries
 Date: September 5, 2012 9:08:53 PM PDT
 To: build-...@openjdk.java.net build-dev build-...@openjdk.java.net
 
 
 Need a reviewer for this change.
 
   http://cr.openjdk.java.net/~ohair/openjdk8/jdk8-this-file/webrev/
 
 It does change source, but it's effectively a build change.
 
 The goal here is to try and create more predictable binary files and remove 
 the possibility that
 a full source path (via __FILE__) gets baked into the shared libraries or 
 executables shipped.
 
 So two changes:
  * sort the .o files during links so they are always provided to the linker 
 in the same order.
  * replace use of __FILE__ to the macro THIS_FILE with just the basename of 
 the source file being compiled
 
 The __FILE__ issue is that it has an implementation defined string literal 
 value, depending on the compiler
 and the filename supplied on the compile line. By changing to the new 
 THIS_FILE macro, the object files
 will always have a consistent string literal in them, making it easier to 
 compare binaries between builds.
 Something we have never been able to do in the past. Granted it's just the 
 basename for the file, but should be enough.
 The tricky part is that __FILE__ only gets evaluated when it is used. In 
 normal C code, it will appear in
 macros but it only will get evaluated in the source file being compiled. It 
 is rare that macros using
 __FILE__  will get expanded in include files and I detected this not 
 happening in the jdk source at all.
 
 -kto
 
 



Re: AWT Dev [OpenJDK 2D-Dev] Fwd: Need reviewers: more predictable binaries

2012-09-06 Thread Kelly O'Hair
Typically, the macros COMPILE.c, COMPILE.CC, and COMPILE.m will include 
$(CPPFLAGS)
which is the preprocessing flags for any compilations using a preprocessor.

I was trying to make the minimum changes needed to get this additional -D 
option on all the
compile lines. I am pretty sure this works, but the way I have changed the 
sources, if I missed any,
then the worse case is that you still get __FILE__.

It has been suggested that at some point the #ifndef THIS_FILE be removed and 
we expect THIS_FILE
to be defined on all compiles, but initially I wasn't willing to be so strict 
on this.

-kto

On Sep 6, 2012, at 11:42 AM, Mike Swingler wrote:

 My strong suspicion is that the JDK Makefiles only use CFLAGS, not CPPFLAGS 
 for .m files. CPPFLAGS should be used for .mm files (but those should be 
 really rare).
 
 Regards,
 Mike Swingler
 Apple Inc.
 
 On Sep 6, 2012, at 11:35 AM, Scott Kovatch scott.kova...@oracle.com wrote:
 
 I feel like I should be able to find the answer to this, but does 
 Objective-C use CPPFLAGS? I can't tell if these changes would apply to .m or 
 .mm files. It certainly appears to be the intent of the change, since a 
 number of .m files in the AWT were changed to use THIS_FILE.
 
 -- Scott K.
 
 On Sep 6, 2012, at 9:30 AM, Kelly O'Hair kelly.oh...@oracle.com wrote:
 
 
 Just FYI...
 
 these build changes do touch sources in various teams, please let me know 
 if you have issues with these changes.
 
 -kto
 
 Begin forwarded message:
 
 From: Kelly O'Hair kelly.oh...@oracle.com
 Subject: Need reviewers: more predictable binaries
 Date: September 5, 2012 9:08:53 PM PDT
 To: build-...@openjdk.java.net build-dev build-...@openjdk.java.net
 
 
 Need a reviewer for this change.
 
   http://cr.openjdk.java.net/~ohair/openjdk8/jdk8-this-file/webrev/
 
 It does change source, but it's effectively a build change.
 
 The goal here is to try and create more predictable binary files and 
 remove the possibility that
 a full source path (via __FILE__) gets baked into the shared libraries or 
 executables shipped.
 
 So two changes:
  * sort the .o files during links so they are always provided to the 
 linker in the same order.
  * replace use of __FILE__ to the macro THIS_FILE with just the basename 
 of the source file being compiled
 
 The __FILE__ issue is that it has an implementation defined string literal 
 value, depending on the compiler
 and the filename supplied on the compile line. By changing to the new 
 THIS_FILE macro, the object files
 will always have a consistent string literal in them, making it easier to 
 compare binaries between builds.
 Something we have never been able to do in the past. Granted it's just the 
 basename for the file, but should be enough.
 The tricky part is that __FILE__ only gets evaluated when it is used. In 
 normal C code, it will appear in
 macros but it only will get evaluated in the source file being compiled. 
 It is rare that macros using
 __FILE__  will get expanded in include files and I detected this not 
 happening in the jdk source at all.
 
 -kto
 
 
 



Re: [8] Review request for 7196354 check-in jdk.tbom file to openjdk repo

2012-09-06 Thread mark . reinhold
2012/9/5 14:08 -0700, michael.f...@oracle.com:
 Please help to review the new JDK8 file for the following CR:
 7196354 check-in jdk.tbom file to openjdk repo
 
 The webrev is located at:
 http://cr.openjdk.java.net/~mfang/7196354/webrev.00/

This file needs a more descriptive name, especially if it's going to be
in the root of the source tree.  Suggestion: translated-files.xml .

Is there a DTD or a schema for this file?  I can guess what most of it
means, but I might guess incorrectly.

[line 8] OpenJDK isn't a component, it's a community.  I think you mean
JDK here.

The JDK / JRE division in this file is somewhat artificial and likely
to become incorrect over time -- not every developer knows exactly which
files are in the JRE vs. the full JDK.  I suggest doing away with that
division and simply sorting the file-set elements by source file name.

At a glance it looks like the source and target attributes for any given
file are identical.  Do you expect there to be cases where they're
different?

 ...
 
 Mark:
 Since the dev team will need to maintain this file in the future (modifying it
 if you add or delete resource files), I temporarily put down your name as
 contact for the file. Please figure out the proper owner and we can update it
 again.

We don't put contact names in source code.  Please remove my name, and do
not add another.

 ...
 
 In the future, if any bug/rfe requires adding/deleting resource files, the dev
 work should include updating this file to reflect the correct resource file
 list. (and please ask me to review it).

If you expect other people to update this file over time then you need
to document that expectation somewhere and, as importantly, you need to
document the syntax and semantics of the file.  Fortunately we have a
way to do that, namely the JEP process (http://openjdk.java.net/jeps/).
I suggest you draft a JEP for this and circulate it for review along
with the webrev.

- Mark


hg: jdk8/tl/jdk: 7196677: diff compares same file to itself in PaddingTest regression test.

2012-09-06 Thread weijun . wang
Changeset: 833f4630f3a1
Author:weijun
Date:  2012-09-07 10:24 +0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/833f4630f3a1

7196677: diff compares same file to itself in PaddingTest regression test.
Reviewed-by: xuelei

! test/com/sun/crypto/provider/Cipher/DES/PaddingTest.java