RFR (S): 7172922: export_ makefile targets do not work unless all supported variants are built

[David Holmes]

On 15/04/2013 2:56 PM, Christian Thalinger wrote:
>
> On Apr 14, 2013, at 9:54 PM, David Holmes <david.holmes at oracle.com> wrote:
>
>> On 15/04/2013 2:36 PM, Christian Thalinger wrote:
>>>
>>> On Apr 14, 2013, at 4:39 PM, David Holmes <david.holmes at oracle.com> wrote:
>>>
>>>> Hi Chris,
>>>>
>>>> On 14/04/2013 2:03 PM, Christian Thalinger wrote:
>>>>>
>>>>> On Apr 12, 2013, at 5:10 PM, David Holmes <david.holmes at oracle.com> wrote:
>>>>>
>>>>>> Hi Chris,
>>>>>>
>>>>>> On 13/04/2013 4:58 AM, Christian Thalinger wrote:
>>>>>>> http://cr.openjdk.java.net/~twisti/7172922
>>>>>>>
>>>>>>> 7172922: export_ makefile targets do not work unless all supported variants are built
>>>>>>> Reviewed-by:
>>>>>>>
>>>>>>> GEN_DIR can be overwritten by other configurations if multiple JVM_VARIANT_*s are defined. The fix is to use the *_BASE_DIRs directly to install the correct files.
>>>>>>>
>>>>>>> make/Makefile
>>>>>>
>>>>>> This looks like a simple temporary solution - thanks.
>>>>>
>>>>> Yes, it's not perfect but good enough for now.
>>>>>
>>>>>>
>>>>>> More long term I hope we should be able to generate the set of targets based on the selected JVM_VARIANTS, without needing all those duplicated blocks.
>>>>>>
>>>>>> One query with the current situation: why doesn't MISC_DIR cause us a problem? It would seem to have the same issue as GEN_DIR. ???
>>>>>
>>>>> MISC_DIR has the same problem but I didn't want to mess with Windows.
>>>>>
>>>>> How about this one?
>>>>>
>>>>> http://cr.openjdk.java.net/~twisti/7172922
>>>>
>>>> I like the addition simplification of getting rid of BASE_DIR and MISC_DIR.
>>>>
>>>> However I think you still need conditionals for Windows otherwise this:
>>>>
>>>> 315 $(EXPORT_JRE_BIN_DIR)/%.diz:                    $(C2_DIR)/%.diz
>>>> 316         $(install-file)
>>>>
>>>> for example, is going to be executed for all platforms and dump the diz files into the bin directory.
>>>
>>> Only if a $(EXPORT_JRE_BIN_DIR)/*.diz file is on the EXPORT_LIST.
>>
>> Oops! My bad.
>>
>> I still think I prefer seeing platform specific targets in platform specific conditionals, rather than using comments.
>
> I don't have a strong opinion about this.  Will add the ifdefs tomorrow.

Don't bother unless someone else feels strongly about it. (I don't want 
to have to re-review :) )

David
-----

> -- Chris
>
>> But if we can macrofy this as the next step (different CR) then that can be handled once within the macro.
>>
>> Thanks,
>> David
>>
>>> -- Chris
>>>
>>>>
>>>> David
>>>> -----
>>>>
>>>>> -- Chris
>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> David
>>>>>>
>>>>>
>>>
>