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

[Christian Thalinger]

On Apr 15, 2013, at 10:52 AM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:

> Christian,
> 
> First, you should start using webrev versions so we can follow your changes during reviews.

I should.  Where is our new review system? ;-)

> 
> Changes are fine. I wish there was a way to avoid duplicated code (functions in Makefiles?).

That would be nice but the problem as I see it is the way how we are doing this.  We add all the targets to EXPORT_LIST and then:

generic_export: $(EXPORT_LIST)

So all the targets in EXPORT_LIST must exists.  If not, you get these mysterious make failures saying it can't find a target.

Again, I'm not a make expert...

-- Chris

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