Excellent work.
I think the comment in CreateSymbols could be made clearer w.r.t. Probe
- i.e. that Probe should be ran on top of the JDK N - i.e.
<JDK-8>/bin/java Probe --> classes-8
<JDK-7>/bin/java Probe --> classes-7
<JDK-6>/bin/java Probe --> classes-7
etc.
Maurizio
On 22/05/15 13:38, Jan Lahoda wrote:
Hi,
I've uploaded a new iteration of the patch(es):
top-level repository:
http://cr.openjdk.java.net/~jlahoda/8072480/webrev.01/top-level/
langtools:
http://cr.openjdk.java.net/~jlahoda/8072480/webrev.01/langtools/
(besides full webrevs, there are also webrevs showing the differences
between .00 and .01 available - see the "Delta webrev" link under
"Author's comments")
Summary of changes:
-applied Eric's build changes
-moved CreateSymbols into make/src/classes/build/tools/symbolgenerator
-tried to improve the specification of base platforms in
CreateSymbols, per Maurizio's comment
-other cleanup in langtools per Maurizio's comments.
Thanks,
Jan
On 21.5.2015 12:31, Maurizio Cimadamore wrote:
Hi Jan,
great work - couple of comments below:
* it seems like some of the routines in Arguments can be simplified
using lambdas - especially lookupPlatformProvider and checkOptionAllowed
* Why JDKPlatformProvider is not called JDKPlatformProvider*Factory* ?
* JavacProcessingEnvironment.JoiningIterator seems to have commonalities
with CompoundScopeIterator - any chance that we might refactor this a
bit?
* What's the rationale for giving an error if -platform is specified and
a non-StandardFileManager is provided? Can't we simply swallow that,
ignore the platform settings and issue a Lint 'options' warning?
* Would it make sense for some of the classfile generation logic in
CreateSymbols to be moved under the classfile API ?
* I had hard time reconciling some of the comments in CreateSymbols with
how the files were laid out. I think in the end, what you mean is that
if you have platforms 7, 8, 9 - you should pick one baseline (i.e. 8)
and then generate diffs for 9 and 7 against the 8 one. If that's the use
case, I think the command line can be simplified a bit in order to
explicitly state which of the platform is the baseline one, and the
remaining parameters can be inferred from the tool (as the
<base-platform-for-platform1,2 ... N> seem to be typically all identical
but one which is set to <none> - the one for the baseline). Maybe the
inference logic should be different (i.e. use 8 as a baseline for 7 and
7 as a baseline for 6) - but - whatever the logic, I think it should be
chosen once and for all, and live implicitly in the tool? Or are there
reasons as to why it might be handy to customize the baselines?
Maurizio
On 21/05/15 08:01, Jan Lahoda wrote:
Hi,
This is a patch adding a new option, -platform, to javac.
Patch for the top-level repository is here:
http://cr.openjdk.java.net/~jlahoda/8072480/webrev.00/top-level/
Patch for the langtools repository is here:
http://cr.openjdk.java.net/~jlahoda/8072480/webrev.00/langtools/
These changes also require additional langtools changes as their
prerequisite:
http://cr.openjdk.java.net/~jlahoda/8080675/webrev.00/
Overall, one can imagine '-platform N' to have the same effect as
'-source N -target N -bootclasspath <APIs-for-N>'. The possible values
for 'N' are pluggable in a limited way, though (see
langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/platform/PlatformProvider.java).
Note that this patch only introduces N=7 and N=8, which correspond to
Open JDK 7 and 8 GA.
A tricky problem to solve is where to get the APIs for platform N. The
proposal is to include history data in the textual format inside the
OpenJDK repositories (the input data), process them at build time and
create a lib/ct.sym file holding the data in the JDK image. javac
running with the -platform option then compiles against the lib/ct.sym
file. The input history data are placed
<top-level-repository>/make/data/symbols (the sym.txt files). This
patches only includes data for OpenJDK 7 and 8, and only for public
APIs (more or less Java SE and JDK @Exported APIs).
The size of the history data is currently ~6MB in the JDK checkout and
~650kB inside the .hg directory (the size could change significantly
if additional classes/elements, like non-public API classes, would
need to be included). The lib/ct.sym file is currently ~4.5MB.
The ct.sym file is a zip file containing signature files. The
signature files are similar to classfiles (so javac can read them as
classfiles), but are missing some attributes, most notably the Code
attribute. On the top-level, the ct.sym contains directories named
"7", "78" and "8". When compiling for version 'N', the bootclasspath
for that version is obtained by using directories which have 'N' in
their name.
The sigfiles for ct.sym are created by
<top-level-repository>/make/tools/symbolgenerator/CreateSymbols.java.
The same file can also be used to construct the sym.txt files. Concise
instructions are part of the CreateSymbols.java.
I am sending this as one review, as the changes together make one
feature, but the langtools and top-level changes are independent to a
great degree: the langtools changes add the "-platform" javac; and the
top-level changes add the history data and ability to build the ct.sym
file. If desired, these could be pushed and/or reviewed independently.
Many thanks go to Jon Gibbons, Joe Darcy, Magnus Ihse Bursie, Alan
Bateman and others who have provided advices and help on the matter
before.
Any insights/comments are wholeheartedly welcome.
Thanks,
Jan
