Re: CFV: New jdk7u Committer: Kurchi Subhra Hazra

2012-05-10 Thread Joe Darcy

Vote: yes

-Joe


Alan Bateman wrote:


I hereby nominate Kurchi Subhra Hazra to jdk7u Committer.

Kurchi has committer role in the jdk8 project but only author role on 
jdk7u.


The following queries match most of her recent activity:

http://hg.openjdk.java.net/jdk8/tl/jdk/log?rev=khazra
http://hg.openjdk.java.net/jdk8/tl/jdk/log?rev=kurchi.subhra.ha...@oracle.com 


http://hg.openjdk.java.net/jdk7u/jdk7u-dev/jdk/log?rev=khazra
http://hg.openjdk.java.net/jdk7u/jdk7u-osx/jdk/log?rev=khazra

Votes are due by May 24, 2012, 09:00 PDT.

Only current jdk7u Committers [1] are eligible to vote on this 
nomination.


For Lazy Consensus voting instructions, see [2].

-Alan.

[1] http://openjdk.java.net/census
[2] http://openjdk.java.net/projects/#committer-vote




[7u10] Request for approval for CR 7181320 - javac NullPointerException for switch labels with cast to String expressions

2012-09-04 Thread Joe Darcy

Hello,

Please consider for 7u10 a backport of the fix for

7181320: javac NullPointerException for switch labels with cast to 
String expressions

http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7181320

from JDK 8:

http://hg.openjdk.java.net/jdk8/tl/langtools/rev/464f52f59f7d

The essence of the fix is to more uniformly preserve constant value 
information for string expressions so that the code generation for the 
string-in-switch statement can simply extract the constant value from 
the corresponding expression.


Thanks,

-Joe


[7u10] Request for approval for CR 7178324 - Crash when compiling for(i : x) try(AutoCloseable x = ...) {}

2012-09-04 Thread Joe Darcy

Hello,

Please consider for 7u10 a backport of the fix for

7178324: Crash when compiling for(i : x) try(AutoCloseable x = ...) {}
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7178324

from JDK 8:

http://hg.openjdk.java.net/jdk8/tl/langtools/rev/f071cd32d297

With the fix, inside the compiler the end position of a 
try-with-resources statement is properly recorded even when it has no 
catch blocks and does not appear in an enclosing block itself.


Thanks,

-Joe



Re: [7u10] Request for approval: 7166055: Javadoc for WeakHashMap contains misleading advice

2012-09-25 Thread Joe Darcy

Hello,

Catching up on email, I'm responding to this thread with my ccc chairman 
hat on.  The ccc is (currently) an Oracle-internal process which reviews 
API and other interfaces changes of the JDK.  The ccc is alluded to in 
the OpenJDK Developers' Guide [1] and among the ccc's roles is looking 
after the general evolution policy of the JDK [2].


For the proposed change for 7166055, I think it is clearly an 
*informative* change to the text and *not* a *normative* change to the 
specification of WeakHashMap.  The affected paragraph starts with 
"Implementation Note" and then goes on to give some usage advice.  
Therefor, this is not a specification change that would have conformance 
impact and on that matter it is fine for a 7 update release.


FWIW, my personal preference would be to have more such clarifications 
to the javadoc made between platform releases so that if the javadoc is 
regenerated, more helpful text is available.


Cheers,

-Joe

[1] http://openjdk.java.net/guide/changePlanning.html

[2] 
http://cr.openjdk.java.net/~darcy/OpenJdkDevGuide/OpenJdkDevelopersGuide.v0.777.html#general_evolution_policy


On 9/18/2012 10:30 AM, Seán Coffey wrote:
I'd have to agree with allowing minor/simple javadoc updates also 
where specification changes are not implied. Even though Oracle 
mightn't always update their javadocs it shouldn't stop others from 
doing so (again for minor/simple/typo updates)


I've run into arguments in past tough around what sort of javadoc 
updates do and do not imply spec. changes. Let's check with 
conformance team before deciding if this change is ok for an update 
release.


Regards,
Sean.

On 18/09/2012 15:51, Andrew Hughes wrote:


- Original Message -

On 16/09/2012 1:26 AM, Phil Race wrote:

On 9/15/12 3:46 AM, David Holmes wrote:

Phil,

On 15/09/2012 2:57 AM, Phil Race wrote:

I really don't think its appropriate to push javadoc changes into
an
update release without
a really, really compelling reason that I don't see here.

That is certainly true if they represent a specification change,
but
there is no semantic change here this is a simple clarification.

That would just rule it out completely. But we don't even
regenerate
javadoc for
the update releases and we have never randomly backported doc
comments, for
no obvious reason. So my reasoning and position stands.

This is OpenJDK, it doesn't matter if "we" don't regenerate javadoc
for
update releases. And I have long thought that "we" should! I
understand
the issue with spec changes in update releases but I never understood
a
policy that would allow errors, misconceptions and mis-guidance to be
set in stone instead of correcting them for the benefit of the user
community.


+1

GNU/Linux distributions will make use of this new documentation in 
new builds,
even if the copies on the Oracle website aren't updated.  The fact 
that you don't
want to jump through whatever hoops are needed to update your own 
copies should not
stop people from making minor updates (clarifications, typo fixes) at 
the OpenJDK level.


I don't know how often jdk7u builds with docs are done at Oracle but 
there are currently

a number of warnings being thrown out by the build:

../../src/share/classes/java/awt/color/ICC_Profile.java:1069: warning 
- Tag @see: missing '#': "activateDeferredProfile()"
../../src/share/classes/java/lang/invoke/MethodHandle.java:392: 
warning - Tag @link: reference not found: Objects.equals 
java.util.Objects#equals
../../src/share/classes/java/util/Calendar.java:1717: warning - Tag 
@see: can't find setInternallySetState(int) in java.util.Calendar
../../src/share/classes/java/util/Currency.java:685: warning - 
@throws tag has no arguments.
../../src/share/classes/javax/swing/plaf/nimbus/NimbusStyle.java:854: 
warning - @return tag has no arguments.
../../src/share/classes/javax/swing/plaf/nimbus/NimbusStyle.java:926: 
warning - @return tag has no arguments.
/home/andrew/builder/icedtea-jdk7/impsrc/javax/xml/bind/JAXBContext.java:262: 
warning - Tag @see: reference not found: S 7.4.1 "Named Packages" in 
Java Language Specification

7 warnings

Are we supposed to retain these too?  I can probably provide webrevs 
to fix these, but there's

no point if they aren't going to be accepted.


David


-phil.


David
--


A reminder: Update releases aren't a free-for-all. You need to
exercise
judgement in what
has to go in and what is the case for it. We are up to 7u10 now.
We need
to be dialling
back the rate of change and focusing on JDK 8.

-phil.


On 9/14/2012 12:56 AM, Shi Jun Zhang wrote:

Hi all,

I'd like to request for approval to push the following change
into
7u10.

Changeset in jdk8
http://hg.openjdk.java.net/jdk8/tl/jdk/rev/237e27c7ddc3

Webrev
http://cr.openjdk.java.net/~zhangshj/jdk7u/7166055/webrev.00/

Reviewed by dholmes, mduigou

Review thread
http://mail.openjdk.java.net/pipermail/core-libs-dev/2012-May/010322.html 










Re: Backports to jdk7u / was : (Re: Request for approval:7151427: Fix the potential memory leak in error handling code in X11SurfaceData.c)

2012-09-25 Thread Joe Darcy

Hello,

While it is certainly true in the limit that effort on JDK 7 update 
should be reduced to focus on JDK 8 and other future releases, my own 
estimation is that we are still in the phase of the lifetime of JDK 7 
train where it is sensible and prudent to accept proactive fixes into 
the release, especially through a low-overhead process like the one now 
being used for 7 updates.


Cheers,

-Joe

On 9/18/2012 11:09 AM, Phil Race wrote:
Every fix adds risk. And some things don't add enough value to make 7u 
detectably better.

Some fixes may be low risk but add minimal value.
The focus for making things better needs to be JDK 8.

-phil.

On 9/18/2012 11:03 AM, Seán Coffey wrote:

Phil -

This fix has soaked in jdk8 for ~5 months. It was initially 
contributed by Sean Chou who has an interest in jdk7u. Why can't it 
be backported if he's willing to make JDK7u better ?


I'm not getting your argument around us needing to ramp down 7u 
fixes. If folks want to contribute tried and tested fixes to jdk7u 
which would appear low risk, then isn't it a win, win for all JDK 
users ? Yes, there are risks to each fix but we have a large number 
of tests run for each update release.


jdk7u is going to be around for many years to come. It's by no means 
a product which we need to start decreasing fixes on in my view! 
Fixing in jdk8 is also a requirement for jdk7u integration.


I do see validity in your point around deciding risk assessment of 
backports. It's possibly something that we need to scrutinize more ? 
That brings more work for reviewers of course.


regards,
Sean.

On 18/09/2012 18:44, Phil Race wrote:
Actually this clearly falls into the 'not remotely important enough 
to backport' bucket.

So I would not approve this backport.

-phil.

On 9/18/12 10:39 AM, Seán Coffey wrote:
Approved for jdk7u-dev.  Note that this most likely means the fix 
will end up in 7u12. If there's a strong justification for 7u10 
inclusion, let me know and one of the jdk7u maintainers can help 
you work through a phase2 request[1] for 7u10.


I'll create a bug record to track this fix in jdk7u.

Regards,
Sean.

[1] http://openjdk.java.net/projects/jdk7u/phase2/phase2-process.html

On 14/09/2012 09:55, Sean Chou wrote:

Hi all,

Here's a request of porting fix 7151427 back to JDK7, could you 
please help

to review?

Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7151427

Change set: http://hg.openjdk.java.net/jdk8/2d/jdk/rev/b1af41b86f9f

Thread where it was reviewed:
http://mail.openjdk.java.net/pipermail/2d-dev/2012-March/002403.html
*
*













Re: Approval request for 8004316

2013-01-15 Thread Joe Darcy

On 1/15/2013 3:13 AM, Artem Ananiev wrote:


On 1/8/2013 10:56 PM, Seán Coffey wrote:

Phil,

Yes - people "should" cc relevant parties when such backports are taking
place. Not mandatory though. Rule 5 in code review guidelines :
http://openjdk.java.net/projects/jdk7u/codereview.html

It's probably a good time to remind OpenJDK 7u contributors to carry out
such checks where applicable.


there's basically no justification of the need for a backport
and I heartily disapprove of backporting  8004316

I can't understand why you're against such a backport. It looks like
printing functionality is broken on some OSes without this fix. Given
that Jayashree backported this fix, I hope she can take responsibility
for any potential regressions that may be encountered.


I completely agree with Phil here.

JDK8 is not the same as JDK7u, sometimes reviewers approve risky fixes 
for 8, in assumption there will be enough time to resolve all the 
regressions without breaking anyone's applications. We can't afford 
doing the same for 7u, which is already used in many production systems.


And those people using 7u in production often don't want to wait until 
they can adopt JDK 8, say, a year after it ships, to have a fix for a 
problem.


As always, there are tradeoffs between stability and progress.  In my 
estimation, the tradeoffs that are being made in the 7 update train in 
that regard have been the right ones.


-Joe


RFR for 7u14: build change to allow 7131459: [Fmt-De] DecimalFormat produces wrong format() results when close to a tie

2013-01-22 Thread Joe Darcy

Hello,

Please review the patch below for inclusion into 7u14.  Some background 
to explain what is going on.  The issue 7131459 [1] has already been 
fixed in JDK 8 to address some long-standing numerical irregularities in 
certain double -> string conversions:


http://hg.openjdk.java.net/jdk8/tl/jdk/rev/bc1f16f5566

However, the patch below does not represent a straight backport of this 
work.  In the 7 update train, this fix is only going to be used when 
7050528 [2] is used and 7050528 is only used in closed code. 7050528 is 
a performance optimization with the improved numerical behavior and 
7131459 provides consistency in the non-optimized cases.


For the 7 update train, to make this happen some changes are needed in 
closed code (not sent to this list), but one small build changes is 
needed to an open makefile which controls the building of a jar file 
only used in the closed product; patch below:


diff --git a/make/altclasses/Makefile b/make/altclasses/Makefile
--- a/make/altclasses/Makefile
+++ b/make/altclasses/Makefile
@@ -1,5 +1,5 @@
 #
-# Copyright (c) 2010, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2010, 2013, Oracle and/or its affiliates. All rights 
reserved.

 # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
 #
 # This code is free software; you can redistribute it and/or modify it
@@ -42,6 +42,7 @@ ALTRT_JAR_FILE = $(LIBDIR)/alt-rt.jar
 ALTRT_JAR_FILE = $(LIBDIR)/alt-rt.jar
 ALTRT_JAR_SOURCE_FILE = $(TEMPDIR)/alt-rt.jarsrclist
 ALTRT_JAR_SOURCES = $(wildcard $(ALTCLASSES_SRCDIR)/java/*/*.java)
+ALTRT_JAR_SOURCES += $(wildcard $(ALTCLASSES_SRCDIR)/sun/misc/*.java)

Thanks,

-Joe

[1] http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7131459

[2] http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7050528


Re: [7u] Request for approval: 8012004: JINTERNALFRAME NOT BEING FINALIZED AFTER CLOSING

2013-05-01 Thread Joe Darcy

On 04/29/2013 08:36 AM, Seán Coffey wrote:

Mikhail,

This is a new method you're proposing to introduce in a public API for 
an update release. Granted, it's an override.


cc'ing Joe Darcy here to confirm that it's OK and that it doesn't 
introduce a source/binary compatibility concern. - Joe - can you 
comment if this change is OK for 7u-dev ?


From a quick look, adding an explicit override for a previously 
inherited method (with no spec change) is fine.


Cheers,

-Joe



regards,
Sean.

On 29/04/13 15:13, mikhail cherkasov wrote:

Hello All,

Requesting approval to push a fix to 7u-dev, it absolutely the same 
as for jdk8:

http://hg.openjdk.java.net/jdk8/awt/jdk/rev/2b8bd577257c
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8012004

Thanks,
Mikhail.






7u40 Request for approval for CR 8012044 - Give more information about self-suppression from Throwable.addSuppressed

2013-05-06 Thread Joe Darcy

Hello,

I'd like to backport the changes for

8012044 - Give more information about self-suppression from 
Throwable.addSuppressed

http://bugs.sun.com/view_bug.do?bug_id=8012044

from JDK 8 to the 7 update train. In JDK 8, the changeset is at

http://hg.openjdk.java.net/jdk8/tl/jdk/rev/4da1d43f5843

and the patch of this changeset applies cleanly to the 7 update repo 
(other than the copyright date on Throwable).


See core-libs-dev threads about this change:
"Throwable.addSuppressed error conditions -- use the exception as the 
cause?"

http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-April/015796.html

Review thread:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-April/015947.html

Thanks,

-Joe


Re: Review request: New man page files

2013-06-18 Thread Joe Darcy

Hello Alexey,

On 6/18/2013 12:49 AM, alexey zhebel wrote:
We are moving to a new process for generating man pages. I checked in 
the two that changed for 7u40 (java and jcmd):

http://cr.openjdk.java.net/~azhebel/8016767/

Please review.

Note that the copyright/license information format was approved by 
Donald Smith who worked with legal.




On that final point, under the proposed change, the copyright line and 
the start of the license are not contiguous. There are a number of 
intervening lines:


   3 .\" Arch: generic
   4 .\" Software: JDK 7
   5 .\" Date: 6 August 2013
   6 .\" SectDesc: Basic Tools
   7 .\" Title: java.1

The convention for basically every other file with a copyright and 
license statement under source code control in the OpenJDK repositories 
is that the copyright + license must be at the top of the file and be 
contiguous.


From an engineering perspective, I think it is neither necessary nor 
desirable to grant to an exception to this rule for man pages even if it 
is acceptable to Don.


-Joe


Re: Review request for JDK-8016767: Provide man pages generated from DARB for OpenJDK

2013-07-19 Thread Joe Darcy

On 07/19/2013 03:39 PM, Stuart Marks wrote:

Hi Alexey,

The new man pages are a big improvement over the previous ones.

There are still a number of formatting issues that need to be 
corrected, but these can be postponed until later, as I understand the 
difficulties of trying to fix up the toolchain under a release deadline.


I think I may have mentioned this in a different review thread, but 
I'll mention some issues here so that we can revisit them in the 
future when we have more time to work on the toolchain.


1. Indentation is now too large, whereas previously it was negative. I 
think some pairs of .RS .RE can be removed. This is mainly around 
options listings.


2. En-dash markup \- is used in a bunch of places where a bare hyphen 
- is appropriate.


3. The tags for the tagged paragraphs description options, e.g. 
-client, -server, -classpath, -d32 should be emboldened. Where text is 
a placeholder to be filled in, e.g. -Dproperty=value, "property" and 
"value" should be italicized.


4. Formatting in the example under -agentlib:libname[=options] could 
use work. Less spacing between the lines, and more spacing before and 
after.


5. Occasional unnecessarily repeated .LP directives. Unnecessary .fl 
and .br directives.


6. Use of 'o' instead of \(bu for bullet items.

Again, it seems sensible to proceed with the man pages as they stand, 
and to work on fixing up these finer points of markup for future 
releases.


I agree that is a reasonable way to proceed.

Cheers,

-Joe




hg: jdk7u/jdk7u-dev/langtools: 8068639: Make certain annotation classfile warnings opt-in

2015-01-08 Thread joe . darcy
Changeset: 08c11954963e
Author:darcy
Date:  2015-01-08 11:16 -0800
URL:   http://hg.openjdk.java.net/jdk7u/jdk7u-dev/langtools/rev/08c11954963e

8068639: Make certain annotation classfile warnings opt-in
Reviewed-by: jjg

! src/share/classes/com/sun/tools/javac/jvm/ClassReader.java
! test/tools/javac/annotations/6214965/T6214965.java
! test/tools/javac/annotations/6365854/T6365854.java



Request for approval for CR 6476261 - (reflect) GenericSignatureFormatError When signature includes nested inner classes

2011-09-01 Thread Joe Darcy

Hello.

I hereby request approval to backport a trio of signature parsing fixes 
from JDK 8 to 7 update:


   6476261: (reflect) GenericSignatureFormatError When signature 
includes nested inner classes

   http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6476261

   6832374: (reflect) malformed signature can cause parser to go into 
infinite loop

   http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6832374

   7052898: (reflect) SignatureParser will accept strings outside of 
the grammar

   http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7052898

   Webrev:
   http://cr.openjdk.java.net/~darcy/6476261.0/

The code for 7 update is trivially different than the code for JDK 8 
since the JDK 8 code takes advantage of a new constructor added to 
GenericSignatureFormatError; here is the diff to SignatureParser.java:


123,126c123,124
< if (DEBUG)
< System.out.println("Signature Parse error: " + errorMsg +
<"\n\tRemaining input: " + remainder());
< return new GenericSignatureFormatError();
---
> return new GenericSignatureFormatError("Signature Parse 
error: " + errorMsg +
>"\n\tRemaining input: 
" + remainder());


Thanks,

-Joe


Re: Request for approval for CR 6476261 - (reflect) GenericSignatureFormatError When signature includes nested inner classes

2011-09-02 Thread Joe Darcy

Dalibor Topic wrote:

On 9/1/11 1:11 PM, Joe Darcy wrote:
  

Hello.

I hereby request approval to backport a trio of signature parsing fixes from 
JDK 8 to 7 update:

   6476261: (reflect) GenericSignatureFormatError When signature includes 
nested inner classes
   http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6476261

   6832374: (reflect) malformed signature can cause parser to go into infinite 
loop
   http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6832374

   7052898: (reflect) SignatureParser will accept strings outside of the grammar
   http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7052898



Unfortunately, the last bug comes up as not available for me. Could you 
describe it in your own words,
and let us know whether this is a regression, and what its priority is?
  


Sure; the legal syntax for a throws clause is a proper subset of the 
full FieldTypeSignature production.  The old code did not make that 
distinction, improperly allowing arrays in the throws section.  The new 
code fixes this issue by adding a b0olean argument in 
parseFieldTypeSignature(boolean allowArrays).  The problem is not a 
regression, but is is included as a unit with the other two fixes.


  

   Webrev:
   http://cr.openjdk.java.net/~darcy/6476261.0/

The code for 7 update is trivially different than the code for JDK 8 since the 
JDK 8 code takes advantage of a new constructor added to 
GenericSignatureFormatError; here is the diff to SignatureParser.java:

123,126c123,124
< if (DEBUG)
< System.out.println("Signature Parse error: " + errorMsg +
<"\n\tRemaining input: " + remainder());
< return new GenericSignatureFormatError();
---


return new GenericSignatureFormatError("Signature Parse error: " + 
errorMsg +
   "\n\tRemaining input: " + 
remainder());
  


Thanks - I am a bit puzzled since the new jdk 8 constructor seems to still be used in the 
posted webrev.
  


Sorry; I've posted the 7 update version of the webrev at:

http://cr.openjdk.java.net/~darcy/6476261.7u/

-Joe




hg: jdk7u/jdk7u-dev/jdk: 6476261: (reflect) GenericSignatureFormatError When signature includes nested inner classes; ...

2011-09-02 Thread joe . darcy
Changeset: 370e70f40ea7
Author:darcy
Date:  2011-09-02 12:23 -0700
URL:   http://hg.openjdk.java.net/jdk7u/jdk7u-dev/jdk/rev/370e70f40ea7

6476261: (reflect) GenericSignatureFormatError When signature includes nested 
inner classes
6832374: (reflect) malformed signature can cause parser to go into infinite loop
7052898: (reflect) SignatureParser will accept strings outside of the grammar
Summary: Various signature parsing fixes; additional review by 
sonali.g...@oracle.com
Reviewed-by: alanb, robilad

! src/share/classes/sun/reflect/generics/parser/SignatureParser.java
! test/java/lang/reflect/Generics/Probe.java
+ test/java/lang/reflect/Generics/SignatureTest.java
+ test/java/lang/reflect/Generics/TestBadSignatures.java



Request for approval for CR 7075098: Remove unused fdlibm files

2011-09-02 Thread Joe Darcy

Hello.

For your consideration, I submit a request to backport from JDK 8 the 
fix for


   7075098: Remove unused fdlibm files
   http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7075098
  
   Changeset:

   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/b9fffbe98230

The patch applies cleanly to 7 update; the effect of the change will be 
to removed some unused files and thus reduce the download and install size.


Cheers,

-Joe


hg: jdk7u/jdk7u-dev/jdk: 7075098: Remove unused fdlibm files

2011-09-04 Thread joe . darcy
Changeset: b02f83934216
Author:darcy
Date:  2011-09-04 09:43 -0700
URL:   http://hg.openjdk.java.net/jdk7u/jdk7u-dev/jdk/rev/b02f83934216

7075098: Remove unused fdlibm files
Reviewed-by: alanb, mduigou

! make/java/fdlibm/FILES_c.gmk
! src/share/native/java/lang/fdlibm/include/fdlibm.h
! src/share/native/java/lang/fdlibm/include/jfdlibm.h



Re: Bake time in JDK8 before 7u ?

2011-09-06 Thread Joe Darcy

David Holmes wrote:
Most fixes to 7u must go to 8 first. I assume at least part of the 
reason for this is to have fixes "bake" in 8 before coming down into 
7u. But I'm seeing a lot of pushes to 8 followed (sometimes within 
minutes) by requests to push to 7u. I know there are various schedules 
to adhere to but it seems to me that the requirement to push to 8 
before 7u is not serving any purpose in these cases.


It is making sure that JDK 8 does not develop a fix bug deficit compared 
to the 7 update train.


Most fixes from most engineers are good most of the time; I think we 
should take advantage of that in making our policies :-)


-Joe



Re: [7u4-osx] Request for approval for 7123392 (launcher) fix MacOSX launcher failures

2012-01-05 Thread Joe Darcy

Hi Kumar,

Looks fine.  In test/tools/launcher/Test7029048.java,  I suggest rephrasing

289 System.out.println("Note: not applicable on Windows and 
MacOSX");


as "Note: applicable on neither Windows nor MacOSX".

-Joe


On 1/5/2012 8:49 AM, Kumar Srinivasan wrote:

Hi,

Please approve,

CR: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7123392
Webrev: http://cr.openjdk.java.net/~ksrini/7123392/

Fixes for the launcher tests which fail with MacOSX and OpenJDK, a 
precursor to

the re-factoring launcher work in progress.

Thanks
Kumar



Re: [7ux-osx] Please review: 7124089: launcher refactoring

2012-01-18 Thread Joe Darcy

On 01/18/2012 07:02 AM, Anthony Petrov wrote:

On 1/18/2012 4:33 AM, Kumar Srinivasan wrote:
Was a specification (ccc) filed for these flags ? Will this be 
documented in the help
section ? though X flags are unsupported, in the past we have 
documented them, both
in the man pages as well as "java -X", if these will be left 
undocumented should we take

the vm approach and mark it -XX ?


Yes, we should have ccc requests to cover any new flags and any 
platform-specific flags.


If a flag is generally sensible, I'd prefer to see the flag 
supported across platforms.


Yes I agree, a ccc needs to be filed asap, and IMO the awt team should
be filing this, as they are aware of all the nuances of these flags.


I've filed 7131038 to address this.

Joe, do you think it absolutely needs an official -help output and a 
CCC approval in 7u4 time-frame, or could it wait till e.g. 7u6?


If flags are officially supported in 7u4, then the ccc request should be 
done by 7u4; I don't expect much contention about this request so it 
should be approved quickly.


-Joe


Re: [7u4-osx] Please review: 7124089: launcher refactoring v2.0

2012-01-23 Thread Joe Darcy

Launcher changes looks good,

-Joe

On 01/21/2012 10:05 AM, Kumar Srinivasan wrote:

Hi Kelly et. al.,

 I have beautified/fixed the Makefiles addressing Kellys' comments below:

1. Indented the Makefiles correctly.
2. Annotated with more  trailing comments to the if/else/endif clauses
3. Removed the offending \ escapes
4. Removed the * from Release.gmk, it turns out the files being copied
   were not quite right (missing files), fixed it such that all the 
appropriate files

   are copied.
5. Added comments for the MacOSX specific cflags.

The incremental webrev is here:
http://cr.openjdk.java.net/~ksrini/7124089/webrev.2/webrev.delta/index.html 



The full webrev is here:
 http://cr.openjdk.java.net/~ksrini/7124089/webrev.2/index.html

Thanks
Kumar


On the Makefiles

Please refrain from using any wildcards (e.g. * ) in the make rules. 
Better to be explicit, or if necessary
use something like FILES=$(wildcard $(SOMEDIR)/*) and a cp $(FILES) 
$(SOMEPLACE)

so that we can at least see in the Makefile log what it really copied.

Please indent the Makefile if/else/endif statements.

Thank you for the trailing comments on the endif's. ;^)

Please try to avoid escaped quotes on the compile lines, use this  
-DX='"abc"' rather than this -DX=\"abc\"
escaped quotes are very problematic on Windows and I know this isn't 
Windows, but it tempts windows
people to use it, it will not work in all situations. Where '"abc"' 
does.


Please add a comment on what the -Os compiler option means, and also 
the -x objective-c, I could guess

but would be better to document it in the makefile.

-kto

On Jan 20, 2012, at 8:24 AM, Kumar Srinivasan wrote:


Hi All,

Based on all the comments from Anthony, Joe and David,
here is the modified version:

Highlights:
1. re-factored code in solaris directory  to be shared with macosx,
reducing duplication across the *nixes.

2. adjusted the makefilesto allow the above

2. eliminated all conditionals from the shared  java.c

3. added a new launcher regression test for the macosx specific -X 
options


For those who have already reviewed the 0th version, here is an
incremental webrev to make it easier reviewing the differences.
http://cr.openjdk.java.net/~ksrini/7124089/webrev.1/webrev.delta/index.html 



Here is the complete webrev:
http://cr.openjdk.java.net/~ksrini/7124089/webrev.1/index.html

Thanks
Kumar