[
https://issues.apache.org/jira/browse/TWILL-215?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15859837#comment-15859837
]
Martin Serrano commented on TWILL-215:
--------------------------------------
Here are the diffs. With these changes, a user would get an exception at
submit time if a dependency could not be found. And all tests pass. But it
turns out there are many detected dependencies which are not used. In the
diffs below there a special cases for these. To be clear, I'm not proposing
these diffs as the solution, I just wanted to share how things shake out.
{code:language=diff}
diff --git
a/twill-core/src/main/java/org/apache/twill/internal/ApplicationBundler.java
b/twill-core/src/main/java/org/apache/twill/internal/ApplicationBundler.java
index 8f82dbd..2eaee84 100644
--- a/twill-core/src/main/java/org/apache/twill/internal/ApplicationBundler.java
+++ b/twill-core/src/main/java/org/apache/twill/internal/ApplicationBundler.java
@@ -27,6 +27,7 @@ import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import com.google.common.io.ByteStreams;
import com.google.common.io.Files;
+
import org.apache.twill.api.ClassAcceptor;
import org.apache.twill.filesystem.Location;
import org.apache.twill.internal.utils.Dependencies;
@@ -252,6 +253,30 @@ public final class ApplicationBundler {
Dependencies.findClassDependencies(classLoader, new ClassAcceptor() {
@Override
public boolean accept(String className, URL classUrl, URL classPathUrl) {
+
+ if (classPathUrl == null) {
+ /*
+ * ignore annotation runtime dependencies and the janino deps from
logback, and others.
+ * this just stops and exception being thrown if these don't exist
on classpath.
+ */
+ if (className.startsWith("javax.annotation") ||
+ className.contains("janino") ||
className.equals("javax.interceptor.InvocationContext")
+ || className.contains("ejb") ||
className.contains("javax.enterprise")
+ || className.contains("org.mockito.")) {
+ return false;
+ }
+ /* let findClassDependencies deal with anything else not found,
probably will throw an exception. */
+ return true;
+ }
+
+ /* - filter out gaffer (logback for gradle configuration
+ * - filter out logback IfAction which brings in commons-compiler
+ * even if the jar for these exists on the classpath it will not be
added.
+ */
+ if (className.contains("gaffer") ||
+ className.equals("ch.qos.logback.core.joran.conditional.IfAction")) {
+ return false;
+ }
if (bootstrapClassPaths.contains(classPathUrl.getFile())) {
return false;
}
diff --git
a/twill-core/src/main/java/org/apache/twill/internal/utils/Dependencies.java
b/twill-core/src/main/java/org/apache/twill/internal/utils/Dependencies.java
index eb55557..8e82950 100644
--- a/twill-core/src/main/java/org/apache/twill/internal/utils/Dependencies.java
+++ b/twill-core/src/main/java/org/apache/twill/internal/utils/Dependencies.java
@@ -22,6 +22,7 @@ import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import com.google.common.io.ByteStreams;
+
import org.apache.twill.api.ClassAcceptor;
import org.objectweb.asm.AnnotationVisitor;
import org.objectweb.asm.ClassReader;
@@ -72,7 +73,9 @@ public final class Dependencies {
while (!classes.isEmpty()) {
String className = classes.remove();
URL classUrl = getClassURL(className, classLoader);
- if (classUrl == null) {
+
+ /* don't worry about inaccessible inner classes. */
+ if (classUrl == null && className.contains("$")) {
continue;
}
@@ -81,6 +84,10 @@ public final class Dependencies {
continue;
}
+ if (classUrl == null) {
+ throw new IOException("Dependency class " + className + " not found on
classpath");
+ }
+
try (InputStream is = classUrl.openStream()) {
// Visit the bytecode to lookup classes that the visiting class is
depended on.
new ClassReader(ByteStreams.toByteArray(is)).accept(new
DependencyClassVisitor(new DependencyAcceptor() {
@@ -106,6 +113,10 @@ public final class Dependencies {
}
private static URL getClassPathURL(String className, URL classUrl) {
+ if (classUrl == null) {
+ return null;
+ }
+
try {
if ("file".equals(classUrl.getProtocol())) {
String path = classUrl.getFile();
diff --git
a/twill-core/src/test/java/org/apache/twill/internal/utils/ApplicationBundlerTest.java
b/twill-core/src/test/java/org/apache/twill/internal/utils/ApplicationBundlerTest.java
index 4609ebd..8314434 100644
---
a/twill-core/src/test/java/org/apache/twill/internal/utils/ApplicationBundlerTest.java
+++
b/twill-core/src/test/java/org/apache/twill/internal/utils/ApplicationBundlerTest.java
@@ -21,6 +21,7 @@ import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
import com.google.common.io.ByteStreams;
import com.google.common.io.Files;
+
import org.apache.twill.filesystem.LocalLocationFactory;
import org.apache.twill.filesystem.Location;
import org.apache.twill.internal.ApplicationBundler;
{code}
> Dependencies not on classpath lead to runtime startup error
> -----------------------------------------------------------
>
> Key: TWILL-215
> URL: https://issues.apache.org/jira/browse/TWILL-215
> Project: Apache Twill
> Issue Type: Bug
> Components: core
> Affects Versions: 0.9.0
> Reporter: Martin Serrano
> Priority: Critical
> Fix For: 0.10.0
>
>
> We do not use logback in our environment but it is a dependency of
> {{ApplicationMasterMain}}. When {{YarnTwillPreparer.createTwillJar}} is
> called in our environment, the logback jar is not on our classpath. For a
> class not in the classpath, the {{Dependencies.findClassDependencies}} method
> ignores it. This leads to a runtime startup error when the app master tries
> to start.
> This is easily fixed unless there some use case for ignoring the dependency
> when it is not on the classpath. An exception should be thrown and no yarn
> job should be submitted.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
