Request for review and approval to fix 8012366 in 7update

[Erik Joelsson]

Looks good to me though I'm not a jdk7 reviewer.

/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.
>>>>>>>
>>>>>
>>>>
>