> For example: doing this may be enough for now: > > * @build jdk.test.lib.process.* > > But what if in the future, jdk.test.lib.process is restructured to have a > private package jdk.test.lib.process.hidden? To work around > CODETOOLS-7901986, all the test cases that must be modified to the following, > which unnecessarily exposes library implementation details to the library > users: > > * @build jdk.test.lib.process.* jdk.test.lib.process.hidden.*
and in fact, there is already similar problem and http://cr.openjdk.java.net/~xiaofeya/8181299/webrev.01/ does not address it. jdk/test/lib/process/ProcessTools depends on jdk/test/lib/Utils so all tests which have '@build jdk.test.lib.process.ProcessTools' will have to have '@build jdk.test.lib.Utils'. then we have OutputAnalyzer which depends on ProcessTools so all tests which @build jdk.test.lib.process.OutputAnalyzer will @build ProcessTools and Utils explicitly. many testlibrary classes which on jdk.test.lib.process.OutputAnalyzer, so one will have to specify OutputAnalyzer ProcessTools and Utils in the tests which depends on other testlibrary classes. to make things even worse, Utils depends on OutputAnalyzer and there are lots of tests and test library classes which depend on Utils, so all of them will have to have at least '@build jdk.test.lib.Utils jdk.test.lib.process.OutputAnalyzer jdk.test.lib.process.ProcessTools'. and they will work stable till someone refactors them and extract some new classes. that is to say, it's nearly impossible to have all explicit @build actions. Cheers, -- Igor > On Jun 1, 2017, at 3:37 PM, Ioi Lam <ioi....@oracle.com> wrote: > > > > On 6/1/17 1:17 PM, Igor Ignatyev wrote: >>> On Jun 1, 2017, at 1:20 AM, Chris Hegarty <chris.hega...@oracle.com> wrote: >>> >>> Igor, >>> >>>> On 1 Jun 2017, at 04:32, Igor Ignatyev <igor.ignat...@oracle.com> wrote: >>>> >>>> Hi Felix, >>>> >>>> I have suggested the exact opposite change[1-3] to fix the same problem. >>> I’m sorry, but this is all just too confusing. After your change, who, or >>> what, is >>> responsible for building/compiling the test library dependencies? >> jtreg is responsible, there is an implicit build for each @run, and jtreg >> will analyze a test class to get transitive closure for static dependencies, >> hence you have to have @build only for classes which are not in constant >> pool, e.g. used only by reflection or whose classnames are only used to >> spawn a new java instance. > > > I suspect the problem is caused by a long standing bug in jtreg that results > in library classes being partially compiled. Please see my evaluation in > > https://bugs.openjdk.java.net/browse/CODETOOLS-7901986 > <https://bugs.openjdk.java.net/browse/CODETOOLS-7901986> > > In the bug report, there is test case that can reliably reproduce the > NoClassDefFoundError problem. > > I think adding all the @build commands in the tests are just band-aids. > Things will break unless every test explicitly uses @build to build every > class in every library that they use, including all the private classes that > are not directly accessible by the test cases. > > For example: doing this may be enough for now: > > * @build jdk.test.lib.process.* > > But what if in the future, jdk.test.lib.process is restructured to have a > private package jdk.test.lib.process.hidden? To work around > CODETOOLS-7901986, all the test cases that must be modified to the following, > which unnecessarily exposes library implementation details to the library > users: > > * @build jdk.test.lib.process.* jdk.test.lib.process.hidden.* > > Just imagine this -- "in order to use malloc() you must explicitly build not > only malloc(), but also sbrk() ... and every other function in libc". That > seems unreasonable to me. > > By the way, we made a fix in the HotSpot tests (see > https://bugs.openjdk.java.net/browse/JDK-8157957 > <https://bugs.openjdk.java.net/browse/JDK-8157957>) that got rid of many (but > not all) of the NoClassDefFoundErrors by *removing* the @build lines ..... > > My proposal is, instead of just adding @build for band-aid, we should fix > CODETOOLS-7901986 instead. > > Thanks > - Ioi > > >>> >>> Test library code has no @modules tags, so does not explicitly declare its >>> module dependencies. Instead module dependencies, required by test >>> library code, are declared in the test using the library. If we wildcard, or >>> otherwise leave broad build dependencies, from tests then there is no >>> way to know what new module dependencies may be added in the future. >>> That is, one of, the reason(s) I asked Felix to be explicit about the build >>> dependencies. >> having explicit builds does not really help w/ module dependency, if someone >> change a testlibrary class so it starts to depend on another testlibrary >> class, jtreg will implicitly build it and if this class has some module >> dependencies, you will have to reflect them in the test. >> >> generally speaking, I don't like having explicit build actions because build >> actions themselves are implicit, so they don't really help, it's still will >> be hard to spot missed explicit builds. not having (unneeded) explicit >> builds is an easy rule to follow and we can easily find all places which >> don't follow this rule by grep. >> >> -- Igor >>> -Chris. >>> >>>> [1] https://bugs.openjdk.java.net/browse/JDK-8181391 >>>> [2] >>>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-June/048012.html >>>> [3] http://cr.openjdk.java.net/~iignatyev//8181391/webrev.00/index.html
