<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><br></div><div>Nothing that can't be fixed or adjusted after the initial integration. So consider me a reviewer.</div><div>Just a few comments.</div><div><br></div>* I noticed that the copyright years were a little strange, saying "Copyright (c) 2007, 2011," instead of "Copyright (c) 2011, 2012," or "Copyright (c) 2012,".<div><br></div><div>* The "@GenerateNativeHeader" additions seem like they deserve some kind of comment, maybe a short one on the same line, like "No native methods here, but the constants are needed in the supporting JNI code" or something like that?</div><div><br></div><div>* The top repo's additions are a bit of a mind blower. Lots of stuff here. Haven't seem m4 files in a long time. ;^)</div><div><br></div><div> - In common/makefiles/IdlCompilation.gmk, lines 82-84 it says</div><div><div> 82 $(if $3,$1_$(strip $2))</div><div> 83 $(if $3,$1_$(strip $3))</div><div> 84 $(if $4,$1_$(strip $4))</div></div><div> Is line 82 right? $3 and not $2?</div><div><br></div><div> - In common/makefiles/MakeBase.gmk, this is very strange to me:</div><div><div> 134 compress_pre:=$(strip $(shell cat $(SRC_ROOT)/common/makefiles/compress.pre))</div><div> 135 compress_post:=$(strip $(shell cat $(SRC_ROOT)/common/makefiles/compress.post))</div></div><div> This path mapping logic seems like high maintenance. I understand what it is trying to get around,</div><div> and I don't have any suggestions for improvements at this time, but it does look very very touchy stuff. :^(</div><div> The good thing is that we have only one copy of it, and if anyone figures out a better way, we can change it, in one place.</div><div><br></div><div>It will take me quite a bit of time to understand this new makefile logic completely.</div><div><br></div><div>-kto</div><div><font class="Apple-style-span" face="monospace"><span class="Apple-style-span" style="white-space: pre; "><br></span></font></div><div><font class="Apple-style-span" face="monospace"><span class="Apple-style-span" style="white-space: pre;"><br></span></font></div><div><font class="Apple-style-span" face="monospace"><span class="Apple-style-span" style="white-space: pre;"><br></span></font></div><div><div><div>On Mar 21, 2012, at 7:07 AM, Erik Joelsson wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">
<meta http-equiv="content-type" content="text/html;
charset=ISO-8859-1">
<div bgcolor="#ffffff" text="#000000">
As outlined in [1], the build-infra project would like to push the
current work into jdk8 in order to expose it to a wider audience.
The webrevs are made against the jdk8/build forest. In each
repository, there are two kinds of changes: <br>
<br>
1. Changes to old makefiles and source code to be compatible with
the new build.<br>
2. The new makefiles<br>
<br>
For corba, jaxp and jaxws, all changes of category 1 have already
gone in. For langtools, we are awaiting one more change for
introducing the GenerateNativeHeader annotation. For hotspot, all
necessary changes have been pushed into hotspot-rt. For jdk, there
are two webrevs, one with everything and one with just the category
1 changes, to make it easier to see them. Finally for the root
repository there are only new files in the common subdir.<br>
<br>
root, configure script and makefiles:<br>
<a href="http://cr.openjdk.java.net/%7Eerikj/build-infra-m1/webrev-root-new/">http://cr.openjdk.java.net/~erikj/build-infra-m1/webrev-root-new/</a><br>
<br>
langtools, 1 new makefile:<br>
<a moz-do-not-send="true" href="http://cr.openjdk.java.net/%7Eerikj/build-infra-m1/webrev-langtools-new/">http://cr.openjdk.java.net/~erikj/build-infra-m1/webrev-langtools-new/</a><br>
<br>
langtools, GenerateNativeHeader annotation (this is already going in
through tools, but adding it here for reference as the jdk changes
depend on it)<br>
<a moz-do-not-send="true" href="http://cr.openjdk.java.net/%7Eerikj/build-infra-m1/webrev-langtools-nativeheader/">http://cr.openjdk.java.net/~erikj/build-infra-m1/webrev-langtools-nativeheader/</a><br>
<br>
corba, 1 new makefile:<br>
<a moz-do-not-send="true" href="http://cr.openjdk.java.net/%7Eerikj/build-infra-m1/webrev-corba-new/">http://cr.openjdk.java.net/~erikj/build-infra-m1/webrev-corba-new/</a><br>
<br>
jaxp, 1 new makefile<br>
<a moz-do-not-send="true" href="http://cr.openjdk.java.net/%7Eerikj/build-infra-m1/webrev-jaxp-new/">http://cr.openjdk.java.net/~erikj/build-infra-m1/webrev-jaxp-new/</a><br>
<br>
jaxws, 1 new makefile<br>
<a moz-do-not-send="true" href="http://cr.openjdk.java.net/%7Eerikj/build-infra-m1/webrev-jaxws-new/">http://cr.openjdk.java.net/~erikj/build-infra-m1/webrev-jaxws-new/</a><br>
<br>
jdk, just the changes to old files<br>
<a moz-do-not-send="true" href="http://cr.openjdk.java.net/%7Eerikj/build-infra-m1/webrev-jdk-other/">http://cr.openjdk.java.net/~erikj/build-infra-m1/webrev-jdk-other/</a><br>
<br>
jdk, all changes including a partial copy of the old makefiles.<br>
<a moz-do-not-send="true" href="http://cr.openjdk.java.net/%7Eerikj/build-infra-m1/webrev-jdk-new/">http://cr.openjdk.java.net/~erikj/build-infra-m1/webrev-jdk-new/</a><br>
<br>
Of course, if you prefer you can look at the new makefiles directly
in the build-infra/jdk8 repository forest too.<br>
<br>
These changes should not affect the old build at all. To build using
the new build system, change directory to "common/makefiles" and:<br>
<br>
../autoconf/configure<br>
make<br>
(make images)<br>
<br>
State of the new build (the old build should of course be
unaffected):<br>
<ul>
<li>Linux 32bit: Works<br>
</li>
<li>Linux 64bit: Works</li>
<li>Windows 32bit: Works<br>
</li>
<li>Windows 64bit: Works</li>
<li>Solaris i586: Builds but launchers currently unusable</li>
</ul>
Some notes:<br>
<ul>
<li>The old and new build (on linux x64) produce very close to
equal results. There is a comparison script in
common/bin/compareimage.sh with which this can be checked.</li>
<li>Not all makefiles in jdk have been converted yet, for those
that haven't been, a copy of the old files are used.</li>
<li>Not all promised features in the java compilation are active
and ready in this milestone. Most notably, it's still not using
more than one cpu and the nifty new dependency tracking is
disabled. A clean build is still pretty fast, but incremental
builds aren't as good as they will be yet.</li>
<li>On windows, only cygwin is currently supported. <br>
</li>
</ul>
Now please share your feedback!<br>
<br>
/Erik<br>
<br>
[1]
<a moz-do-not-send="true" class="moz-txt-link-freetext" href="http://mail.openjdk.java.net/pipermail/build-infra-dev/2012-March/000571.html">http://mail.openjdk.java.net/pipermail/build-infra-dev/2012-March/000571.html</a><br>
<br>
</div>
</blockquote></div><br></div></body></html>