Re: CFV: New jdk7u Committer: Kurchi Subhra Hazra
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
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 = ...) {}
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
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)
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
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
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
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
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
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
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
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
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
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; ...
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
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
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 ?
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
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
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
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