<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<p>Hi Igor and Ioi,</p>
<p> I partially agree with you. As initially stated in the
proposal and bug(<a class="issue-link"
data-issue-key="JDK-8181299"
href="https://bugs.openjdk.java.net/browse/JDK-8181299"
id="key-val" rel="4930496">JDK-8181299</a>), I don't think this
patch is a fix but a quick workaround to make them runnable.</p>
<p> "explicit" is reasonable for me. But "explicit" should not be
restricted as "explicit all, including dependencies", as it is not
productive or even realistic in the long term.<br>
</p>
Thanks,<br>
Felix<br>
<div class="moz-cite-prefix">On 2017/6/2 7:58, Igor Ignatyev wrote:<br>
</div>
<blockquote type="cite"
cite="mid:BEA86CC1-341F-45AA-AC27-7FB84EE01295@oracle.com">
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<blockquote type="cite" class=""><span class="" style="float:
none; display: inline !important;">For example: doing this may
be enough for now:</span><br class="">
<br class="">
<span class="" style="float: none; display: inline !important;"> *
@build jdk.test.lib.process.*</span><br class="">
<br class="">
<span class="" style="float: none; display: inline !important;">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:</span><br
class="">
<br class="">
<span class="" style="float: none; display: inline !important;"> *
@build jdk.test.lib.process.* jdk.test.lib.process.hidden.*</span></blockquote>
<div class=""><br class="">
</div>
and in fact, there is already similar problem and <a
href="http://cr.openjdk.java.net/%7Exiaofeya/8181299/webrev.01/"
class="" moz-do-not-send="true">http://cr.openjdk.java.net/~xiaofeya/8181299/webrev.01/</a>
does not address it.
<div class="">
<div class="">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.
<div class=""><br class="">
</div>
<div class="">Cheers,<br class="">
<div class="">
<div class="">
<div class="">-- Igor<br class="">
<div class=""><br class="">
<div>
<blockquote type="cite" class="">
<div class="">On Jun 1, 2017, at 3:37 PM, Ioi
Lam <<a href="mailto:ioi.lam@oracle.com"
class="" moz-do-not-send="true">ioi.lam@oracle.com</a>>
wrote:</div>
<br class="Apple-interchange-newline">
<div class=""><br style="font-family: Helvetica;
font-size: 12px; font-style: normal;
font-variant-caps: normal; font-weight:
normal; letter-spacing: normal; text-align:
start; text-indent: 0px; text-transform:
none; white-space: normal; word-spacing:
0px; -webkit-text-stroke-width: 0px;"
class="">
<br style="font-family: Helvetica; font-size:
12px; font-style: normal; font-variant-caps:
normal; font-weight: normal; letter-spacing:
normal; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal;
word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<span style="font-family: Helvetica;
font-size: 12px; font-style: normal;
font-variant-caps: normal; font-weight:
normal; letter-spacing: normal; text-align:
start; text-indent: 0px; text-transform:
none; white-space: normal; word-spacing:
0px; -webkit-text-stroke-width: 0px; float:
none; display: inline !important;" class="">On
6/1/17 1:17 PM, Igor Ignatyev wrote:</span><br
style="font-family: Helvetica; font-size:
12px; font-style: normal; font-variant-caps:
normal; font-weight: normal; letter-spacing:
normal; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal;
word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<blockquote type="cite" style="font-family:
Helvetica; font-size: 12px; font-style:
normal; font-variant-caps: normal;
font-weight: normal; letter-spacing: normal;
orphans: auto; text-align: start;
text-indent: 0px; text-transform: none;
white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-size-adjust:
auto; -webkit-text-stroke-width: 0px;"
class="">
<blockquote type="cite" class="">On Jun 1,
2017, at 1:20 AM, Chris Hegarty <<a
href="mailto:chris.hegarty@oracle.com"
class="" moz-do-not-send="true">chris.hegarty@oracle.com</a>>
wrote:<br class="">
<br class="">
Igor,<br class="">
<br class="">
<blockquote type="cite" class="">On 1 Jun
2017, at 04:32, Igor Ignatyev <<a
href="mailto:igor.ignatyev@oracle.com"
class="" moz-do-not-send="true">igor.ignatyev@oracle.com</a>>
wrote:<br class="">
<br class="">
Hi Felix,<br class="">
<br class="">
I have suggested the exact opposite
change[1-3] to fix the same problem.<br
class="">
</blockquote>
I’m sorry, but this is all just too
confusing. After your change, who, or
what, is<br class="">
responsible for building/compiling the
test library dependencies?<br class="">
</blockquote>
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.<br class="">
</blockquote>
<br style="font-family: Helvetica; font-size:
12px; font-style: normal; font-variant-caps:
normal; font-weight: normal; letter-spacing:
normal; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal;
word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<br style="font-family: Helvetica; font-size:
12px; font-style: normal; font-variant-caps:
normal; font-weight: normal; letter-spacing:
normal; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal;
word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<span style="font-family: Helvetica;
font-size: 12px; font-style: normal;
font-variant-caps: normal; font-weight:
normal; letter-spacing: normal; text-align:
start; text-indent: 0px; text-transform:
none; white-space: normal; word-spacing:
0px; -webkit-text-stroke-width: 0px; float:
none; display: inline !important;" class="">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</span><br
style="font-family: Helvetica; font-size:
12px; font-style: normal; font-variant-caps:
normal; font-weight: normal; letter-spacing:
normal; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal;
word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<br style="font-family: Helvetica; font-size:
12px; font-style: normal; font-variant-caps:
normal; font-weight: normal; letter-spacing:
normal; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal;
word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<a
href="https://bugs.openjdk.java.net/browse/CODETOOLS-7901986"
style="font-family: Helvetica; font-size:
12px; font-style: normal; font-variant-caps:
normal; font-weight: normal; letter-spacing:
normal; orphans: auto; text-align: start;
text-indent: 0px; text-transform: none;
white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-size-adjust:
auto; -webkit-text-stroke-width: 0px;"
class="" moz-do-not-send="true">https://bugs.openjdk.java.net/browse/CODETOOLS-7901986</a><br
style="font-family: Helvetica; font-size:
12px; font-style: normal; font-variant-caps:
normal; font-weight: normal; letter-spacing:
normal; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal;
word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<br style="font-family: Helvetica; font-size:
12px; font-style: normal; font-variant-caps:
normal; font-weight: normal; letter-spacing:
normal; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal;
word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<span style="font-family: Helvetica;
font-size: 12px; font-style: normal;
font-variant-caps: normal; font-weight:
normal; letter-spacing: normal; text-align:
start; text-indent: 0px; text-transform:
none; white-space: normal; word-spacing:
0px; -webkit-text-stroke-width: 0px; float:
none; display: inline !important;" class="">In
the bug report, there is test case that can
reliably reproduce the NoClassDefFoundError
problem.</span><br style="font-family:
Helvetica; font-size: 12px; font-style:
normal; font-variant-caps: normal;
font-weight: normal; letter-spacing: normal;
text-align: start; text-indent: 0px;
text-transform: none; white-space: normal;
word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<br style="font-family: Helvetica; font-size:
12px; font-style: normal; font-variant-caps:
normal; font-weight: normal; letter-spacing:
normal; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal;
word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<span style="font-family: Helvetica;
font-size: 12px; font-style: normal;
font-variant-caps: normal; font-weight:
normal; letter-spacing: normal; text-align:
start; text-indent: 0px; text-transform:
none; white-space: normal; word-spacing:
0px; -webkit-text-stroke-width: 0px; float:
none; display: inline !important;" class="">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.</span><br style="font-family:
Helvetica; font-size: 12px; font-style:
normal; font-variant-caps: normal;
font-weight: normal; letter-spacing: normal;
text-align: start; text-indent: 0px;
text-transform: none; white-space: normal;
word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<br style="font-family: Helvetica; font-size:
12px; font-style: normal; font-variant-caps:
normal; font-weight: normal; letter-spacing:
normal; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal;
word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<span style="font-family: Helvetica;
font-size: 12px; font-style: normal;
font-variant-caps: normal; font-weight:
normal; letter-spacing: normal; text-align:
start; text-indent: 0px; text-transform:
none; white-space: normal; word-spacing:
0px; -webkit-text-stroke-width: 0px; float:
none; display: inline !important;" class="">For
example: doing this may be enough for now:</span><br
style="font-family: Helvetica; font-size:
12px; font-style: normal; font-variant-caps:
normal; font-weight: normal; letter-spacing:
normal; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal;
word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<br style="font-family: Helvetica; font-size:
12px; font-style: normal; font-variant-caps:
normal; font-weight: normal; letter-spacing:
normal; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal;
word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<span style="font-family: Helvetica;
font-size: 12px; font-style: normal;
font-variant-caps: normal; font-weight:
normal; letter-spacing: normal; text-align:
start; text-indent: 0px; text-transform:
none; white-space: normal; word-spacing:
0px; -webkit-text-stroke-width: 0px; float:
none; display: inline !important;" class=""> *
@build jdk.test.lib.process.*</span><br
style="font-family: Helvetica; font-size:
12px; font-style: normal; font-variant-caps:
normal; font-weight: normal; letter-spacing:
normal; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal;
word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<br style="font-family: Helvetica; font-size:
12px; font-style: normal; font-variant-caps:
normal; font-weight: normal; letter-spacing:
normal; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal;
word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<span style="font-family: Helvetica;
font-size: 12px; font-style: normal;
font-variant-caps: normal; font-weight:
normal; letter-spacing: normal; text-align:
start; text-indent: 0px; text-transform:
none; white-space: normal; word-spacing:
0px; -webkit-text-stroke-width: 0px; float:
none; display: inline !important;" class="">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:</span><br
style="font-family: Helvetica; font-size:
12px; font-style: normal; font-variant-caps:
normal; font-weight: normal; letter-spacing:
normal; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal;
word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<br style="font-family: Helvetica; font-size:
12px; font-style: normal; font-variant-caps:
normal; font-weight: normal; letter-spacing:
normal; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal;
word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<span style="font-family: Helvetica;
font-size: 12px; font-style: normal;
font-variant-caps: normal; font-weight:
normal; letter-spacing: normal; text-align:
start; text-indent: 0px; text-transform:
none; white-space: normal; word-spacing:
0px; -webkit-text-stroke-width: 0px; float:
none; display: inline !important;" class=""> *
@build jdk.test.lib.process.*
jdk.test.lib.process.hidden.*</span><br
style="font-family: Helvetica; font-size:
12px; font-style: normal; font-variant-caps:
normal; font-weight: normal; letter-spacing:
normal; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal;
word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<br style="font-family: Helvetica; font-size:
12px; font-style: normal; font-variant-caps:
normal; font-weight: normal; letter-spacing:
normal; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal;
word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<span style="font-family: Helvetica;
font-size: 12px; font-style: normal;
font-variant-caps: normal; font-weight:
normal; letter-spacing: normal; text-align:
start; text-indent: 0px; text-transform:
none; white-space: normal; word-spacing:
0px; -webkit-text-stroke-width: 0px; float:
none; display: inline !important;" class="">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.</span><br
style="font-family: Helvetica; font-size:
12px; font-style: normal; font-variant-caps:
normal; font-weight: normal; letter-spacing:
normal; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal;
word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<br style="font-family: Helvetica; font-size:
12px; font-style: normal; font-variant-caps:
normal; font-weight: normal; letter-spacing:
normal; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal;
word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<span style="font-family: Helvetica;
font-size: 12px; font-style: normal;
font-variant-caps: normal; font-weight:
normal; letter-spacing: normal; text-align:
start; text-indent: 0px; text-transform:
none; white-space: normal; word-spacing:
0px; -webkit-text-stroke-width: 0px; float:
none; display: inline !important;" class="">By
the way, we made a fix in the HotSpot tests
(see<span class="Apple-converted-space"> </span></span><a
href="https://bugs.openjdk.java.net/browse/JDK-8157957"
style="font-family: Helvetica; font-size:
12px; font-style: normal; font-variant-caps:
normal; font-weight: normal; letter-spacing:
normal; orphans: auto; text-align: start;
text-indent: 0px; text-transform: none;
white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-size-adjust:
auto; -webkit-text-stroke-width: 0px;"
class="" moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8157957</a><span
style="font-family: Helvetica; font-size:
12px; font-style: normal; font-variant-caps:
normal; font-weight: normal; letter-spacing:
normal; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal;
word-spacing: 0px;
-webkit-text-stroke-width: 0px; float: none;
display: inline !important;" class="">) that
got rid of many (but not all) of the
NoClassDefFoundErrors by *removing* the
@build lines .....</span><br
style="font-family: Helvetica; font-size:
12px; font-style: normal; font-variant-caps:
normal; font-weight: normal; letter-spacing:
normal; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal;
word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<br style="font-family: Helvetica; font-size:
12px; font-style: normal; font-variant-caps:
normal; font-weight: normal; letter-spacing:
normal; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal;
word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<span style="font-family: Helvetica;
font-size: 12px; font-style: normal;
font-variant-caps: normal; font-weight:
normal; letter-spacing: normal; text-align:
start; text-indent: 0px; text-transform:
none; white-space: normal; word-spacing:
0px; -webkit-text-stroke-width: 0px; float:
none; display: inline !important;" class="">My
proposal is, instead of just adding @build
for band-aid, we should fix
CODETOOLS-7901986 instead.</span><br
style="font-family: Helvetica; font-size:
12px; font-style: normal; font-variant-caps:
normal; font-weight: normal; letter-spacing:
normal; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal;
word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<br style="font-family: Helvetica; font-size:
12px; font-style: normal; font-variant-caps:
normal; font-weight: normal; letter-spacing:
normal; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal;
word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<span style="font-family: Helvetica;
font-size: 12px; font-style: normal;
font-variant-caps: normal; font-weight:
normal; letter-spacing: normal; text-align:
start; text-indent: 0px; text-transform:
none; white-space: normal; word-spacing:
0px; -webkit-text-stroke-width: 0px; float:
none; display: inline !important;" class="">Thanks</span><br
style="font-family: Helvetica; font-size:
12px; font-style: normal; font-variant-caps:
normal; font-weight: normal; letter-spacing:
normal; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal;
word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<span style="font-family: Helvetica;
font-size: 12px; font-style: normal;
font-variant-caps: normal; font-weight:
normal; letter-spacing: normal; text-align:
start; text-indent: 0px; text-transform:
none; white-space: normal; word-spacing:
0px; -webkit-text-stroke-width: 0px; float:
none; display: inline !important;" class="">-
Ioi</span><br style="font-family: Helvetica;
font-size: 12px; font-style: normal;
font-variant-caps: normal; font-weight:
normal; letter-spacing: normal; text-align:
start; text-indent: 0px; text-transform:
none; white-space: normal; word-spacing:
0px; -webkit-text-stroke-width: 0px;"
class="">
<br style="font-family: Helvetica; font-size:
12px; font-style: normal; font-variant-caps:
normal; font-weight: normal; letter-spacing:
normal; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal;
word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<br style="font-family: Helvetica; font-size:
12px; font-style: normal; font-variant-caps:
normal; font-weight: normal; letter-spacing:
normal; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal;
word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<blockquote type="cite" style="font-family:
Helvetica; font-size: 12px; font-style:
normal; font-variant-caps: normal;
font-weight: normal; letter-spacing: normal;
orphans: auto; text-align: start;
text-indent: 0px; text-transform: none;
white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-size-adjust:
auto; -webkit-text-stroke-width: 0px;"
class="">
<blockquote type="cite" class=""><br
class="">
Test library code has no @modules tags, so
does not explicitly declare its<br
class="">
module dependencies. Instead module
dependencies, required by test<br class="">
library code, are declared in the test
using the library. If we wildcard, or<br
class="">
otherwise leave broad build dependencies,
from tests then there is no<br class="">
way to know what new module dependencies
may be added in the future.<br class="">
That is, one of, the reason(s) I asked
Felix to be explicit about the build<br
class="">
dependencies.<br class="">
</blockquote>
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.<br class="">
<br class="">
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.<br
class="">
<br class="">
-- Igor<br class="">
<blockquote type="cite" class="">-Chris.<br
class="">
<br class="">
<blockquote type="cite" class="">[1] <a
href="https://bugs.openjdk.java.net/browse/JDK-8181391"
class="" moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8181391</a><br
class="">
[2] <a
href="http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-June/048012.html"
class="" moz-do-not-send="true">http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-June/048012.html</a><br
class="">
[3] <a
href="http://cr.openjdk.java.net/%7Eiignatyev//8181391/webrev.00/index.html"
class="" moz-do-not-send="true">http://cr.openjdk.java.net/~iignatyev//8181391/webrev.00/index.html</a></blockquote>
</blockquote>
</blockquote>
</div>
</blockquote>
</div>
<br class="">
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
<br>
</body>
</html>