RfR and RfA: Backport of JDK-8055831 Open Source Java Access Bridge

Pete Brunet peter.brunet at oracle.com
Tue Mar 31 11:12:10 UTC 2015


Hi Erik, Thank you for the review and assistance.  -Pete

On 3/31/15 5:45 AM, Erik Joelsson wrote:
> Hello Pete,
>
> The webrev.02 linked below is broken so I will not approve it. The
> "ifdef OPENJDK" means that those parts of the build will not get built
> when closed source is available, i.e. for Oracle builds. There are
> also formatting issues that, while not critical, I would normally not
> let through either.
>
> Since this seems urgent to you, I have prepared a webrev of the
> makefiles, corrected the way I would like them. I have built this on
> windows both 32 and 64 bit, both open and closed and verified that all
> accessbridge related build artifacts appear in all of them.
>
> http://cr.openjdk.java.net/~erikj/8076182/webrev.01/
>
> I have started two JPRT jobs to verify nothing broke on the other
> platforms.
>
> /Erik
>
> On 2015-03-31 05:21, Pete Brunet wrote:
>> Thanks Sean, The JPRT open build pointed out some problems which I have
>> resolved but need Erik's review of those changes which are here:
>> http://cr.openjdk.java.net/~ptbrunet/JDK-8076182/webrev.02/
>>
>> Alexander Zvegintsev reviewed the source so once Erik finishes his
>> review hopefully you'll be able to approve it for push into 8u-dev.
>>
>> Erik, There are some places where I changed ifndef OpenJDK to ifdef
>> OpenJDK.  I didn't think of it until too late in the evening, but I
>> probably should have just removed that.  If so, I can do that later in
>> the week.  Since this patch has to be pushed on Tuesday (likely the day
>> you are reading this) I don't want to add any risk due to even small
>> changes.  The change will be one place in each of:
>> - jdk/make/src/GensrcMisc.gmk
>> - jdk/make/CompileJavaClasses.gmk
>> - jdk/make/CreateJars.gmk
>>
>> Pete
>>
>> p.s. Apparently there was a cockpit error on my part on my first attempt
>> at the open only build (which should have failed but didn't).  After
>> JPRT found the problem I was able to reproduce the issue locally and now
>> both local and JPRT open builds are OK.
>>
>> On 3/30/15 2:05 PM, Seán Coffey wrote:
>>> Pete,
>>>
>>> I'm not a reviewer for this but I notice that some files have the
>>> incorrect copyright header (PROPRIETARY/CONFIDENTIAL) - Please correct
>>> that before any push. I think it's also safer if you run both an
>>> OpenJDK build and a closed/open src build through JPRT. It'll handle
>>> both.
>>>
>>> regards,
>>> Sean.
>>>
>>> On 30/03/2015 19:39, Pete Brunet wrote:
>>>> p.s. The 7 JPRT builds ran OK: 2 win, 2 linux, 2 solaris, 1 mac
>>>>
>>>> On 3/30/15 11:10 AM, Pete Brunet wrote:
>>>>> Erik, I made the changes that you pointed out.  Builds worked OK for
>>>>> local closed and open.  I also started a JPRT build for all
>>>>> platforms.  That's 7 targets and since that will take a while I
>>>>> wanted
>>>>> to publish the new patch before you left for the day:
>>>>>
>>>>> http://cr.openjdk.java.net/~ptbrunet/JDK-8076182/webrev.01/
>>>>>
>>>>> Pete
>>>>>
>>>>> On 3/30/15 2:43 AM, Erik Joelsson wrote:
>>>>>> Hello,
>>>>>>
>>>>>> In jdk/make/CompileJavaClasses.gmk:
>>>>>>
>>>>>> You removed
>>>>>> $(JDK_TOPDIR)/src/closed/$(OPENJDK_TARGET_OS_API_DIR)/classes, but
>>>>>> for linux and solaris, that source dir is still needed. Is it giving
>>>>>> you an error when the directory doesn't exist? In that case you will
>>>>>> need to make it conditional on platform.
>>>>>>
>>>>>> jdk/make/CompileLaunchers.gmk:
>>>>>>
>>>>>> If jabswitch is open source, then it shouldn't be built under the
>>>>>> condition "ifndef OPENJDK".
>>>>>>
>>>>>> jdk/make/CopyFiles.gmk:
>>>>>>
>>>>>> ifndef OPENJDK
>>>>>>
>>>>>> jdk/make/gensrc/GensrcMisc.gmk:
>>>>>>
>>>>>> ifndef OPENJDK
>>>>>>
>>>>>> jdk/make/lib/PlatformLibraries.gmk:
>>>>>>
>>>>>> ifndef OPENJDK
>>>>>>
>>>>>>
>>>>>> I would recommend getting an open only forest, apply your patch and
>>>>>> verify that you get everything built properly.
>>>>>>
>>>>>> /Erik
>>>>>>
>>>>>> On 2015-03-28 01:30, Pete Brunet wrote:
>>>>>>> Please review and approve a backport of JDK-8055831 to open source
>>>>>>> the
>>>>>>> Java Access Bridge.  The original patch has been pushed to 9 and
>>>>>>> the
>>>>>>> backport now needs to be pushed to 8u.
>>>>>>>
>>>>>>> Due to the significant directory structure changes in 9 due to
>>>>>>> modularization I could not simply apply the patch.  However the
>>>>>>> effect
>>>>>>> is functionally the same.  Here are the changes:
>>>>>>>
>>>>>>> - moved the JAB files from closed to open
>>>>>>> - change the copyright headers to open style
>>>>>>> - adjust the make files accordingly
>>>>>>> - create javadoc for com.sun.java.accessibility.util; creates it in
>>>>>>> docs/jre/api/accessibility/jaccess/spec
>>>>>>> - removed extraneous unused files; these are tools inadvertently
>>>>>>> picked
>>>>>>> up in 7u6 when the JAB was first integrated and which will be
>>>>>>> added later
>>>>>>> - removed some dead code
>>>>>>> - cleaned up comments/documentation
>>>>>>> - cleaned up some code indentation issues
>>>>>>> - in package-private EventQueueMonitor.addTopLevelWindow the return
>>>>>>> value was not used so I changed the return from boolean to void
>>>>>>>
>>>>>>> The open part of the patch is here:
>>>>>>> http://cr.openjdk.java.net/~ptbrunet/JDK-8076182/webrev.00/
>>>>>>>
>>>>>>> (I see I have five copyright dates to fix in jdk/make.)
>>>>>>>
>>>>>>> There is a closed part but it's just the removal of the source
>>>>>>> files
>>>>>>> from the closed repository.
>>>>>>>
>>>>>>> The patch has been through a successful 32/64 bit JPRT build and
>>>>>>> subsequent testing on my machine
>>>>>>>
>>>>>>> This patch needs to be pushed by Tuesday so I'll appreciate your
>>>>>>> review.
>>>>>>>
>>>>>>> Thanks, Pete
>



More information about the jdk8u-dev mailing list