RFR (S): 7172922: export_ makefile targets do not work unless all supported variants are built
Vladimir Kozlov
vladimir.kozlov at oracle.com
Mon Apr 15 10:52:29 PDT 2013
Christian,
First, you should start using webrev versions so we can follow your
changes during reviews.
Changes are fine. I wish there was a way to avoid duplicated code
(functions in Makefiles?).
Thanks,
Vladimir
On 4/14/13 10:17 PM, Christian Thalinger wrote:
>
> On Apr 14, 2013, at 10:14 PM, David Holmes <david.holmes at oracle.com> wrote:
>
>> 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 :) )
>
> Even better :-) If there are no objections tomorrow I will push it. Thanks for reviewing.
>
> -- Chris
>
>>
>> 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
>>>>>>>>
>>>>>>>
>>>>>
>>>
>
More information about the hotspot-dev
mailing list