On 2015-04-08 11:18, Erik Joelsson wrote:
Hello,
New webrev: http://cr.openjdk.java.net/~erikj/8073634/webrev.02/
Looks good to me now.
On 2015-04-07 13:26, Magnus Ihse Bursie wrote:
In general it looks good. I have a couple of remarks, though.
1) In Clean-docs, you do:
+ $(RM) -r $(SUPPORT_OUTPUTDIR)/docs
+ $(RM) -r $(IMAGES_OUTPUTDIR)/docs
I'm not quite certain cleaning images/docs the right thing to do. For
all other clean targets, we only clean the support part. If we want
to clean images, we run clean-images. And if you think this is the
right thing to do for images, wouldn't it then also be the right
thing for e.g. java.base-native? And then things quickly become
complicated.
The docs target generates docs directly into images. The directory in
support is for temporary/intermediate files. Building to support and
then just copy it all over to images seems like a waste of both time
and disk space. Also note that the clean targets aren't mutually
exclusive today. You can for example do clean-support or you can do
clean-java.base.
Alright.
2) The new configure-support, as well as the rather new make-support
seems to be a bit unfairly treated. There is a MAKESUPPORT_OUTPUTDIR
in spec.gmk but it's hardcoded there, so you can't use it in
configure, e.g. in your changes in build-performance.m4. The new
configure-support does not even have a corresponding
CONFIGURESUPPORT_OUTPUTDIR in the spec.gmk. Instead, it has a
variable CONFIGURE_SUPPORT that is present only in the configure files.
I'm not sure how much, if anything, of these inconsistencies that
should be fixed now. But I'd suggest that you rename
CONFIGURE_SUPPORT to CONFIGURESUPPORT_OUTPUTDIR at the very least.
(Or, maybe, MAKESUPPORT_OUTPUTDIR should be MAKE_SUPPORT_OUTPUTDIR.)
Renamed to CONFIGURESUPPORT_OUTPUTDIR and put it in spec. Removed
reference to make-support in configure and put the single use directly
in spec.gmk.in instead. There is no need for configure to know about
the sjavacservers dir. Changed dist-clean in Main.gmk to use the
variable CONFIGURESUPPORT_OUTPUTDIR.
Great!
/Magnus
