Igor
On 2017/6/2 10:11, Igor Ignatyev wrote:
Hi Felix,
none of the jdk tests which fail w/ NCDFE:
jdk/test/lib/process/StreamPumper depend on StreamPumper directly,
they get this dependency transitively
from jdk/test/lib/process/ProcessTools,
That is why I think it is a bug too.
so I don't see how you will find this good definition of "explicit"
even for the failures at hand.
Just meant "expected behavior", as it makes test code clear for me. Of
course it fails, otherwise there will be no such discussion at all.
-Felix
I recommend to work around this the same way we did it in hotspot,
which reliably removed almost all our NCDFE failures, -- remove
explicit @build, if not all for all classes, then at least for
jdk/test/lib/** classes.
-- Igor
On Jun 1, 2017, at 6:52 PM, Felix Yang <felix.y...@oracle.com
<mailto:felix.y...@oracle.com>> wrote:
Hi Igor and Ioi,
I partially agree with you. As initially stated in the proposal
and bug(JDK-8181299
<https://bugs.openjdk.java.net/browse/JDK-8181299>), I don't think
this patch is a fix but a quick workaround to make them runnable.
"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.
Thanks,
Felix
On 2017/6/2 7:58, Igor Ignatyev wrote:
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/
<http://cr.openjdk.java.net/%7Exiaofeya/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
<mailto: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 <mailto:chris.hega...@oracle.com>> wrote:
Igor,
On 1 Jun 2017, at 04:32, Igor Ignatyev <igor.ignat...@oracle.com
<mailto: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
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
(seehttps://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
<http://cr.openjdk.java.net/%7Eiignatyev//8181391/webrev.00/index.html>
