Re: RFR: (T) 8241144 Javadoc is not generated for new module jdk.nio.mapmode
On 25/03/2020 3:49 am, Florian Weimer wrote: * Magnus Ihse Bursie: On 2020-03-24 09:59, Andrew Dinn wrote: On 23/03/2020 18:38, Erik Joelsson wrote: Looks good. Thanks for the review, Erik. I'm assuming that also implies it is trivial (because, copyright update a side, it really is a 1-liner :-). For code in the build system, we do not have the Hotspot rules of multiple reviewers, waiting period or trtiviality. A single OK review is enough to be allowed to push it. Where are these rules documented? I looked for them on openjdk.java.net, but could not find them unfortunately. Hotspot rules are buried in here: https://wiki.openjdk.java.net/display/HotSpot/HotSpot+How+To "Before pushing" You must be a Committer in the JDK project You need a non-JEP JBS issue for tracking Your change must have been available for review at least 24 hours to accommodate for all time zones Your change must have been approved by two Committers out of which at least one is also a Reviewer Your change must have passed through the hs tier 1 testing provided by the submit-hs repository with zero failures You must run all relevant testing to make sure your actual change is working You must be available the next few hours, and the next day and ready to follow up with any fix needed in case your change causes problems in later tiers There is a notion of trivial changes that can be pushed sooner than 24 hours. It should be clearly stated in the review mail that the intention is to push as a trivial change. How to actually define "trivial" is decided on a case-by-case basis but in general it would be things like fixing a comment, or moving code without changing it. Backing out a change is also considered trivial as the change itself in that case is generated by mercurial. Cheers, David
Re: RFR: (T) 8241144 Javadoc is not generated for new module jdk.nio.mapmode
* Magnus Ihse Bursie: > On 2020-03-24 09:59, Andrew Dinn wrote: >> On 23/03/2020 18:38, Erik Joelsson wrote: >>> Looks good. >> Thanks for the review, Erik. >> >> I'm assuming that also implies it is trivial (because, copyright update >> a side, it really is a 1-liner :-). > > For code in the build system, we do not have the Hotspot rules of > multiple reviewers, waiting period or trtiviality. A single OK review is > enough to be allowed to push it. Where are these rules documented? I looked for them on openjdk.java.net, but could not find them unfortunately.
Re: RFR: JDK-8241463 Move build tools to respective modules
On 2020-03-23 23:15, naoto.s...@oracle.com wrote: Hi Magnus, I looked at i18n related changes: make/CopyInterimTZDB.gmk make/ToolsJdk.gmk make/gendata/Gendata-java.base.gmk make/gendata/GendataBreakIterator.gmk make/gendata/GendataTZDB.gmk make/gensrc/GensrcCharacterData.gmk make/gensrc/GensrcEmojiData.gmk They look ok to me. Thank you! The *.java changes should have copyright year update. Ok, I'll update them. As to charsetmapping and cldrconverter, I believe they can reside in java.base, as jdk.charsets and jdk.localedata modules depend on it. Okay. It's not ideal, but I think you're right. I'll move them as well. I'll publish an updated webrev with these changes when there's agreement on where in the source code tree to move the files. /Magnus Naoto On 3/23/20 12:03 PM, Magnus Ihse Bursie wrote: The build tools (small java tools that are run during the build to generate source code, or data, needed in the JDK) have historically been placed in the "make" directory. This maybe made sense long time ago, but does not do so anymore. Instead, the build tools source code should move the the module that needs them. For instance, compilefontconfig should move to java.desktop, etc. There are multiple reasons for this: * Currently we build *all* build tools at once, which mean that we cannot compile java.base until e.g. the compilefontconfig tool is compiled, even though it is not needed. * If a build tool, e.g. compilefontconfig is modified, all build tools are recompiled, which triggers a rebuild of more or less the entire JDK. This makes development of the build tools unnecessary tedious. * When the build tools are modified, the group owning the corresponding module is the proper review instance, not the build team. But since they reside under "make", the review mails often include build-dev, but this is mostly noise for us. With this move, the ownership is made clear. In this patch, I have not modified how and when the build tools are compiled, but this shuffle is the prerequisite for continuing with that in a follow-up patch. I have also moved the build tools to the org.openjdk.buildtools.* package name space (inspired by Skara), instead of the strangely named build.tools.* name space. A few build tools are not moved in this patch. Two of them, charsetmapping and cldrconverter, are shared between two modules. (I think they should move to modules nevertheless, but they need some more thought to make sure I do this right.) The rest are tools that are needed for the build in general, like linking or javadoc support. I'll move this to a better location too, but in a separate patch. Bug: https://bugs.openjdk.java.net/browse/JDK-8241463 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8241463-move-build-tools-to-modules/webrev.01 /Magnus
Re: RFR: JDK-8241463 Move build tools to respective modules
On 2020-03-23 21:19, Mandy Chung wrote:
Hi Magnus,
Modularizing the build tools is a good move.
Thanks!
This patch suggests to place the build tools under
src/$MODULE/share/tools/$PACKAGE/*.java
I think the modular source location of the build tools needs more
discussion, including jigsaw-dev for this discussion.
Ok, I've pruned the recipient list down to just jigsaw-dev and
build-dev. Let's keep this thread for discussing where to put the code
in the source tree, and when we agree on that I can make an updated
webrev and get buy-in from the component owners.
The JDK source as specified in JEP 201 is under:
src/$MODULE/{share,$OS}/classes/$PACKAGE/*.java
Compiling the source files from the `src` directory are the
intermediate input to build the resulting image. Build tools are
used to generate additional intermediate input (that is not part of
the `src` directory) to build the image. So I wonder if
make/$MODULE/share/tools or make/tools/$MODULE may be better location
for the build tools.
That's certainly a possibility. Ever since the big jigsaw restructuring,
Erik and I have discussed how to modularize the build system. I did not
even like the original jigsaw push, but there was not enough time to fix
the build system, and as a compromise we opened a bunch of JBS issues to
fix this in a follow-up fashion. Two are still remaining: JDK-8055192
and JDK-8055193, and JDK-8241463 is a part of that, even though it seems
to have not been written down in a bug report at that time.
At the core, we'd like to "invert" the current structure where we have
files like:
make/lib/Lib-java.base.gmk
make/lib/Lib-java.desktop.gmk
make/gensrc/Gensrc-java.base.gmk
make/gensrc/Gensrc-java.desktop.gmk
... etc
and instead have like:
make/modules/java.base/lib.gmk
make/modules/java.base/gensrc.gmk
make/modules/java.desktop/lib.gmk
make/modules/java.desktop/gensrc.gmk
However, this stuff is clearly a core part of the build system, so there
is no question that this belongs under "make".
In contrast, the build tools are only interacting with the build system
on the surface. There is some kind of an API here -- the build tools
have some kind of calling convention, wants some input and produces some
output, which is consumed by the build system. But the actual workings
of the build tools have 100% to do with the component. In some cases,
the original developer found it more suitable to create a tool for
generating programmatically a number of classes, rather than doing that
once, for hand, and check in the code. (I'm a bit skeptical towards
those build tools, btw, even though I understand the reason.) But this
clearly is just another way to express the core functionality of that
module. If anything is causing confusion right now, it's when the
component teams do not know about or take responsibility for the build
tools.
So I think the build tools are just like much of the rest of the product
-- the build system needs to be aware of them and how they work, but
they are really the expression -- and responsibility -- of the component
owners. And thus, I believe they are much better suited in the src tree.
Furthermore, we have JDK-8055193, about modularizing the make/data
directory. This is something I care even more about. And here I'm quite
firm in my conviction that this has nothing to do in the make directory.
It's just a remnant of the old thinking of "Oh, I don't know where to
put this. Let's just continue using 'make' as our misc trashcan". Things
like make/data/macosxicons should move to
src/java.desktop/data/macosxicons, and make/data/publicsuffixlist to
src/java.base/data/publicsuffixlist, and so forth. There's absolutely
nothing here that has anything to do with the build system. (With the
possible exception of product-wide stuff, I know these exists among the
tools, I'm not sure about the data.)
And since there is a strong correlation between the data files and the
build tools -- many build tools are custom made to process things in the
data directory -- I think it makes much sense to move the source code of
the tools along with them.
We already have collected everything else that belongs to a module under
src/$module/share. Apart from classes, and native, we have:
* conf
* lib
* legal
* man
for those modules that require them.
My suggestion is that we add, for those module that require them:
* data
* tools
If the name of the latter is not good enough, I'm open for suggestions.
Maybe "build-tools", "buildtools", "tools-src", "tools-classes",
"classes-tools"...? My idea for picking "tools" was that it was short,
and seemed to fit in well with the style of the other names. An
additional benefit was that it was not as limiting as "buildtools", and
thus allowing for the modules to put other kinds of tools there that
might not be needed at build time -- for instance, tools that are needed
every once in a while to update some checked-in
Re: RFR: (T) 8241144 Javadoc is not generated for new module jdk.nio.mapmode
On 24/03/2020 09:10, Magnus Ihse Bursie wrote:
> On 2020-03-24 09:59, Andrew Dinn wrote:
>> I'm assuming that also implies it is trivial (because, copyright update
>> a side, it really is a 1-liner :-).
>
> For code in the build system, we do not have the Hotspot rules of
> multiple reviewers, waiting period or trtiviality. A single OK review is
> enough to be allowed to push it.
Ah ok, thanks for the advice. I'll push as soon as hg.openjdk.java.net
comes back to life.
> (And for the record, you can add me as reviewer as well, if you wish :))
You are on the list :-)
regards,
Andrew Dinn
---
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill
Re: RFR: (T) 8241144 Javadoc is not generated for new module jdk.nio.mapmode
On 2020-03-24 09:59, Andrew Dinn wrote:
On 23/03/2020 18:38, Erik Joelsson wrote:
Looks good.
Thanks for the review, Erik.
I'm assuming that also implies it is trivial (because, copyright update
a side, it really is a 1-liner :-).
For code in the build system, we do not have the Hotspot rules of
multiple reviewers, waiting period or trtiviality. A single OK review is
enough to be allowed to push it.
(And for the record, you can add me as reviewer as well, if you wish :))
/Magnus
I will push to the dev tree and request a backport to jdk14u.
regards,
Andrew Dinn
---
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill
On 2020-03-23 10:56, Andrew Dinn wrote:
Could I please have a review for this trivial fix to the module make
file which ensures that javadoc is generated for new module
jdk.nio.mapmode created as part of the implementation of JEP 352. The
original patch added the module to the BOOT_MODULES list but was not to
the DOCS_MODULES list.
JIRA: https://bugs.openjdk.java.net/browse/JDK-8241144
webrev: http://cr.openjdk.java.net/~adinn/8241144/webrev
Testing:
Built image and compiled + ran Hello World.
Built make target docs-jdk-api-javadoc and checked module
jdk.nio.mapmode was included in output
regards,
Andrew Dinn
---
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill
Re: RFR: (T) 8241144 Javadoc is not generated for new module jdk.nio.mapmode
On 23/03/2020 18:38, Erik Joelsson wrote:
> Looks good.
Thanks for the review, Erik.
I'm assuming that also implies it is trivial (because, copyright update
a side, it really is a 1-liner :-).
I will push to the dev tree and request a backport to jdk14u.
regards,
Andrew Dinn
---
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill
> On 2020-03-23 10:56, Andrew Dinn wrote:
>> Could I please have a review for this trivial fix to the module make
>> file which ensures that javadoc is generated for new module
>> jdk.nio.mapmode created as part of the implementation of JEP 352. The
>> original patch added the module to the BOOT_MODULES list but was not to
>> the DOCS_MODULES list.
>>
>> JIRA: https://bugs.openjdk.java.net/browse/JDK-8241144
>> webrev: http://cr.openjdk.java.net/~adinn/8241144/webrev
>>
>> Testing:
>>
>> Built image and compiled + ran Hello World.
>>
>> Built make target docs-jdk-api-javadoc and checked module
>> jdk.nio.mapmode was included in output
>>
>> regards,
>>
>>
>> Andrew Dinn
>> ---
>> Senior Principal Software Engineer
>> Red Hat UK Ltd
>> Registered in England and Wales under Company Registration No. 03798903
>> Directors: Michael Cunningham, Michael ("Mike") O'Neill
>>
>
