On Mon, 2019-07-29 at 16:32 +0100, Andrew John Hughes wrote: > > On 29/07/2019 11:10, Severin Gehwolf wrote: > > On Mon, 2019-07-29 at 10:19 +0100, Andrew Dinn wrote: > > > On 26/07/2019 18:32, Andrew John Hughes wrote: > > > > On 26/07/2019 16:53, Severin Gehwolf wrote: > > [...] > > > > > > What exactly is being pushed > > > > > > here? > > > > > > > > > > The following 4 patches: > > > > > > > > > > jdk: > > > > > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8222737/04/jdk/webrev/ > > > > > hotspot: > > > > > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8222737/03/hotspot/webrev/ > > > > > langtools: > > > > > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8222737/03/langtools/webrev/ > > > > > top: > > > > > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8222737/02/top/webrev/ > > > > > > > > > > Each of them has a reference to a plain patch file if you prefer to > > > > > review that. > > > > > > > > > > Thanks, > > > > > Severin > > > > > > > > > > > > > Ok. HotSpot, JDK & top look fine, but there seem to be a lot of changes > > > > in langtools. Are these original changes to this patch or are they > > > > backports? Is there a reason langtools needs so much more work than the > > > > others? > > > The langtools Makefile changes appear to be larger because they required > > > adding a load of definitions/macros that are already present in the > > > Makefiles in the other trees (i.e. AWK, CAT etc, ZIP_UP_RESULTS, > > > BUNDLE_UP_AND_EXIT etc). These changes look ok to me. > > > > Yes, exactly. The langools Makefile looks *very* different from > > jdk/hotspot. In order to get some unified status of test results across > > all three, these changes were needed. I've opted to make the langools > > Makefile look more like the ones from jdk and hotspot. > > > > Old "make test" from top-level continues to work with these changes > > (modulo -verbose:nopass=> summary change). > > > > Anyway, I had another look at this and made 8075546[1] another pre- > > requisite which makes the langtools changes slightly smaller. Updated > > webrevs set, thus, is: > > > > jdk: > > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8222737/04/jdk/webrev/ > > hotspot: > > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8222737/03/hotspot/webrev/ > > langtools: > > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8222737/05/langtools/webrev/ > > top: > > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8222737/02/top/webrev/ > > > > Thanks, > > Severin > > > > [1] https://bugs.openjdk.java.net/browse/JDK-8075546 > > > > Right. That doesn't answer the question as to the origin of the changes. > If I look at the langtools Makefile in OpenJDK 11, it doesn't have these > changes. Should they not go there first if they are needed?
It looks like test/langtools/Makefile in JDK 11 is not used. At least make run-test-tier1 doesn't use it in JDK 11. I've just tried the following which works fine: $ rm test/langtools/Makefile $ hg status R test/langtools/Makefile $ make JAVAC_FLAGS=-g DISABLE_INTREE_EC=true LOG=debug run-test-tier1 Old make files have been removed with JDK-8217638 in JDK 13. To answer your question: I don't think these changes are needed for OpenJDK 11 as it has the "run-tests" facility (JDK-8176084) which deprecates the old way of running tests. Having said that, it seems more intrusive to try to get that facility ported to JDK 8u, than proceeding with the proposed change. JDK-8217638 suggests that old make files were kept for preserving some old workflows. Due to deprecation of "make test" from JDK 8 in JDK 9+, most of it is different from JDK 9+ onwards. For some time, both approaches seem to have kept working. I'm not sure in what way "make run-test" depends on the new build system in JDK 9. It may do. Briefly looking at the run-test patches it seems to open a can of worms. Incidentally, JDK-8170629, as one of the changes to the build/test infrastructure which happened in JDK 9 timeframe, seems to confirm that langtools is the "odd-man-out" in terms of make files and code duplication :) > Update releases are primarily for backports, so I'm dubious of anything > that brings in a bunch of apparently new changes without making it clear > why there are needed and why a backport is not appropriate. I realize that. There is a balance to strike, though. Backports-only vs. small enhancements. Considering that OpenJDK 8u will go on for some time to come it seems beneficial to have some way for developers of running a known-to-be-working set of tests. That's going to be the bar which every new contribution will have to pass. Current facility only runs jdk and langtools tests, does not print a summary, doesn't work well in CI environments, etc. The proposal preserves the old functionality, but also allows for a new "tier1" target. So, in light of the changed build system in JDK 9+, and the rather small changes in this patch (on the grand scheme of things), it seems reasonable to implement this feature in the way proposed. It's a JDK 8u *enhancement* after all. What would you propose as an alternative? Thanks, Severin