On Apr 14, 2013, at 10:14 PM, David Holmes <david.hol...@oracle.com> wrote:
> On 15/04/2013 2:56 PM, Christian Thalinger wrote: >> >> On Apr 14, 2013, at 9:54 PM, David Holmes <david.hol...@oracle.com> wrote: >> >>> On 15/04/2013 2:36 PM, Christian Thalinger wrote: >>>> >>>> On Apr 14, 2013, at 4:39 PM, David Holmes <david.hol...@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.hol...@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 >>>>>>> >>>>>> >>>> >>
