RfR and RfA: Backport of JDK-8055831 Open Source Java Access Bridge
Erik Joelsson
erik.joelsson at oracle.com
Tue Mar 31 10:45:53 UTC 2015
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