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

[David Holmes]

On 16/04/2013 5:30 AM, Christian Thalinger wrote:
>
> 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.

We should be able to use similar techniques as used in the new build to 
dynamically generate targets, dependencies and recipes.

However as I mention in the bug report a further problem is that we 
bundle some VM independent build artifacts into the VM specific build 
directory. and we will need to clean that up I think if we are to 
macrofy this part of the build logic.

I have JDK-7195896 as a cleanup task that might be able to deal with 
this problem. But no idea when I'll get to it.

David


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