On 7/12/13 8:57 AM, Erik Joelsson wrote:
> Looks good to me though I'm not a jdk7 reviewer.

Looks good to me, too. Local OpenJDK build on OS X and an OpenJDK linux build 
on JPRT both passed.

Please send in a fresh phase 2 approval request to the jdk7u-dev list.

cheers,
dalibor topic

> /Erik
> 
> On 2013-07-12 07:22, Tim Bell wrote:
>> David, Erik -
>>
>> Thanks for the feedback:
>>
>>> Does this also need to check for OPENJDK in case you are doing an OPENJDK 
>>> build with a full forest present? Or is this logic already by-passed for 
>>> OPENJDK builds? David 
>>
>>
>>> Setting OPENJDK=true on the command line will force BUILD_INSTALL=false, 
>>> even if the source is available, and INSTALL_SRC_AVAILABLE will still be 
>>> true. As a user, if I requested an OPENJDK build with the full forest 
>>> available, I would not expect the build to head down into the install repo, 
>>> even for debug bundles. For this reason I think the check should be against 
>>> BUILD_INSTALL instead of INSTALL_SRC_AVAILABLE.
>>>
>>> /Erik
>>
>> The revised change looks for BUILD_INSTALL instead, which I believe handles 
>> both points.  The webrev is here:
>>
>> http://cr.openjdk.java.net/~tbell/8012366/webrev.02/
>>
>> Tim
>>
>>
>>
>> On 07/11/13 01:19 AM, Erik Joelsson wrote:
>>> On 2013-07-11 07:30, David Holmes wrote:
>>>> Tim,
>>>>
>>>> On 11/07/2013 6:47 AM, Tim Bell wrote:
>>>>> On 07/10/13 12:37 PM, Tim Bell wrote:
>>>>>> Hi Kelly
>>>>>>
>>>>>>> How did BUILD_INSTALL ever become true on an OpenJDK build?
>>>>>>
>>>>>> From the 'generic_debug_build' rule in the top level 7update Makefile [1]
>>>>>>
>>>>>> generic_debug_build:
>>>>>>     $(MAKE) \
>>>>>>         ALT_OUTPUTDIR=$(ABS_OUTPUTDIR)/$(REL_JDK_OUTPUTDIR) \
>>>>>>             DEBUG_NAME=$(DEBUG_NAME) \
>>>>>>         GENERATE_DOCS=false \
>>>>>>         BUILD_INSTALL_BUNDLES=true \
>>>>>>         CREATE_DEBUGINFO_BUNDLES=false \
>>>>>>             $(BOOT_CYCLE_DEBUG_SETTINGS) \
>>>>>>         generic_build_repo_series
>>>>>
>>>>>
>>>>> After giving this more study, let me modify what I wrote a bit -
>>>>> generic_debug_build sets "BUILD_INSTALL_BUNDLES=true", which causes the
>>>>> 'install-binaries-jdk-debug' rule in the same Makefile to also run. This
>>>>> dives into install-rules.gmk [2] in a place where BUILD_INSTALL is not
>>>>> checked.
>>>>>
>>>>> Here is a revised fix and webrev:
>>>>>
>>>>>    http://cr.openjdk.java.net/~tbell/8012366/webrev.01/
>>>>
>>>> Does this also need to check for OPENJDK in case you are doing an OPENJDK 
>>>> build with a full forest present? Or is this logic already by-passed for 
>>>> OPENJDK builds?
>>>>
>>> Setting OPENJDK=true on the command line will force BUILD_INSTALL=false, 
>>> even if the source is available, and INSTALL_SRC_AVAILABLE will still be 
>>> true. As a user, if I requested an OPENJDK build with the full forest 
>>> available, I would not expect the build to head down into the install repo, 
>>> even for debug bundles. For this reason I think the check should be against 
>>> BUILD_INSTALL instead of INSTALL_SRC_AVAILABLE.
>>>
>>> /Erik
>>>
>>>> David
>>>>
>>>>> Tim
>>>>>
>>>>>> [1] http://hg.openjdk.java.net/jdk7u/jdk7u/file/tip/Makefile
>>>>>
>>>>> [2] http://hg.openjdk.java.net/jdk7u/jdk7u/file/tip/make/install-rules.gmk
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>>> -kto
>>>>>>>
>>>>>>> On Jul 8, 2013, at 12:03 PM, Tim Bell wrote:
>>>>>>>
>>>>>>>> All-
>>>>>>>>
>>>>>>>> My fix for 8007815 [1] did not handle the case of an OpenJDK build
>>>>>>>> that does not have the closed source trees.  If a user tries to
>>>>>>>> build that far, the build will fail with "cd: can't cd to
>>>>>>>> ./install/make/installer/binaries/linux"
>>>>>>>>
>>>>>>>> This is a request for review and approval to fix 8012366 in 7u.
>>>>>>>> These changes will not throw a build error if the closed directories
>>>>>>>> do not exist.  Makefile changes only; no product code has been
>>>>>>>> modified:
>>>>>>>>
>>>>>>>> Link to the bug report:
>>>>>>>>
>>>>>>>>    http://bugs.sun.com/view_bug.do?bug_id=8012366
>>>>>>>>
>>>>>>>> webrev of the Makefile changes:
>>>>>>>>
>>>>>>>>   http://cr.openjdk.java.net/~tbell/8012366/webrev.00/
>>>>>>>>
>>>>>>>> Thanks in advance-
>>>>>>>>
>>>>>>>> Tim
>>>>>>>> [1]  There is no external link for 8007815, sorry. These changes
>>>>>>>> had to do with building and packaging in the install tree.
>>>>>>>>
>>>>>>
>>>>>
>>



-- 
Oracle <http://www.oracle.com>
Dalibor Topic | Principal Product Manager
Phone: +494089091214 <tel:+494089091214> | Mobile: +491737185961 
<tel:+491737185961>
Oracle Java Platform Group

ORACLE Deutschland B.V. & Co. KG | Kühnehöfe 5 | 22761 Hamburg

ORACLE Deutschland B.V. & Co. KG
Hauptverwaltung: Riesstr. 25, D-80992 München
Registergericht: Amtsgericht München, HRA 95603
Geschäftsführer: Jürgen Kunz

Komplementärin: ORACLE Deutschland Verwaltung B.V.
Hertogswetering 163/167, 3543 AS Utrecht, Niederlande
Handelsregister der Handelskammer Midden-Niederlande, Nr. 30143697
Geschäftsführer: Alexander van der Ven, Astrid Kepper, Val Maher

Green Oracle <http://www.oracle.com/commitment> Oracle is committed to 
developing practices and products that help protect the environment

Reply via email to