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.

Just to add to what Igor said:

In fact, what JTREG does is fairly simple, but kind of hidden so it's not obvious what it does with the @library tags.

Let's say your test uses "@library /test/lib"

After your test completes, open the .jtr file (JTREG Report). It should have a command-line for javac, like this:

DISPLAY=:2 \\
HOME=/home/iklam \\
JTREG_COMPILEJDK=/home/iklam/jdk/bld/foobar/images/jdk \\
LANG=en_US.UTF-8 \\
LD_LIBRARY_PATH=/home/iklam/jdk/bld/foobar/images/jdk/../../images/test/hotspot/jtreg/native \\
PATH=/bin:/usr/bin \\
    /home/iklam/jdk/bld/foobar/images/jdk/bin/javac \\
-J-Dtest.src=/jdk/foobar/hotspot/test/runtime/SharedArchiveFile \\
-J-Dtest.src.path=/jdk/foobar/hotspot/test/runtime/SharedArchiveFile:/jdk/foobar/test/lib \\
-J-Dtest.classes=/jdk/tmp/jtreg/work/classes/14/runtime/SharedArchiveFile \\
-J-Dtest.class.path=/jdk/tmp/jtreg/work/classes/14/runtime/SharedArchiveFile:/jdk/tmp/jtreg/work/classes/14/test/lib \\
        -J-Dtest.vm.opts= \\
        -J-Dtest.tool.vm.opts= \\
        -J-Dtest.compiler.opts= \\
        -J-Dtest.java.opts= \\
        -J-Dtest.jdk=/home/iklam/jdk/bld/foobar-fastdebug/images/jdk \\
        -J-Dcompile.jdk=/home/iklam/jdk/bld/foobar/images/jdk \\
        -J-Dtest.timeout.factor=4.0 \\
        -J-Dtest.modules='java.base/jdk.internal.misc java.management' \\
-J-Dtest.nativepath=/home/iklam/jdk/bld/foobar/images/jdk/../../images/test/hotspot/jtreg/native \\
@/jdk/tmp/jtreg/work/runtime/SharedArchiveFile/SpaceUtilizationCheck.d/compile.0.jta

The gem is hidden in the compile.0.jta file. It contains something like:

      -sourcepath <blahblah>:/jdk/foobar/test/lib:<blahblah>

So if my test refers to a class under /test/lib, such as jdk.test.lib.process.ProcessTools, javac will be able to locate it under /jdk/foobar/test/lib/jdk/test/lib/process/ProcessTools.java, and will build it automatically.

So really, there's no reason why the test must explicitly do an @build of the library classes that it uses.

- 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

Reply via email to