Request for review and approval to fix 8012366 in 7update

[Tim Bell]

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