You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@twill.apache.org by ch...@apache.org on 2015/04/23 23:43:20 UTC

incubator-twill git commit: (TWILL-120) Make Twill Java8 compatible

Repository: incubator-twill
Updated Branches:
  refs/heads/master 17a8ba64f -> ae24a0498


(TWILL-120) Make Twill Java8 compatible

- Make Dependencies to use ASM5 visitor to find class dependencies
- Update MethodVisitor to use the right visitMethodInsn instead of the deprecated one
  - The deprecated one will miss static initializer <clinit>, hence causing missing runtime dependencies
- Also fixed the tracing to includes all Annotation classes that are visible at runtime
- Added an unit-test in a separate java8-test to test Java8 compatibility

Signed-off-by: Terence Yim <ch...@apache.org>

This closes #30 on GitHub.


Project: http://git-wip-us.apache.org/repos/asf/incubator-twill/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-twill/commit/ae24a049
Tree: http://git-wip-us.apache.org/repos/asf/incubator-twill/tree/ae24a049
Diff: http://git-wip-us.apache.org/repos/asf/incubator-twill/diff/ae24a049

Branch: refs/heads/master
Commit: ae24a0498fabb34ab33689ff5d651552ff49a4cd
Parents: 17a8ba6
Author: Terence Yim <ch...@apache.org>
Authored: Thu Apr 9 17:03:18 2015 -0700
Committer: Terence Yim <ch...@apache.org>
Committed: Thu Apr 23 14:42:28 2015 -0700

----------------------------------------------------------------------
 .travis.yml                                     |  22 ++++
 pom.xml                                         |  13 +++
 .../twill/internal/utils/Dependencies.java      | 107 +++++++++++++++----
 twill-java8-test/pom.xml                        |  81 ++++++++++++++
 .../java/org/apache/twill/test/Java8Test.java   |  91 ++++++++++++++++
 twill-yarn/pom.xml                              |  15 +++
 .../internal/yarn/AbstractYarnAMClient.java     |   3 +-
 7 files changed, 309 insertions(+), 23 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-twill/blob/ae24a049/.travis.yml
----------------------------------------------------------------------
diff --git a/.travis.yml b/.travis.yml
index b33f47f..8cb390e 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -20,6 +20,7 @@ language: java
 
 jdk:
   - oraclejdk7
+  - oraclejdk8
 
 branches:
   only:
@@ -39,6 +40,27 @@ env:
   - PROFILE='hadoop-2.5'
   - PROFILE='cdh-4.4.0'
   - PROFILE='mapr-hadoop-2.4'
+  - PROFILE='hadoop-2.5,java8-test'
+
+# Only runs JDK8 on hadoop-2.5 profile
+matrix:
+  exclude:
+    - jdk: oraclejdk8
+      env: PROFILE='hadoop-2.0'
+    - jdk: oraclejdk8
+      env: PROFILE='hadoop-2.1'
+    - jdk: oraclejdk8
+      env: PROFILE='hadoop-2.2'
+    - jdk: oraclejdk8
+      env: PROFILE='hadoop-2.4'
+    - jdk: oraclejdk8
+      env: PROFILE='hadoop-2.5'
+    - jdk: oraclejdk8
+      env: PROFILE='cdh-4.4.0'
+    - jdk: oraclejdk8
+      env: PROFILE='mapr-hadoop-2.4'
+    - jdk: oraclejdk7
+      env: PROFILE='hadoop-2.5,java8-test'
 
 sudo: false
 

http://git-wip-us.apache.org/repos/asf/incubator-twill/blob/ae24a049/pom.xml
----------------------------------------------------------------------
diff --git a/pom.xml b/pom.xml
index 83f2a95..9f923c3 100644
--- a/pom.xml
+++ b/pom.xml
@@ -241,6 +241,13 @@
                     <groupId>org.apache.maven.plugins</groupId>
                     <artifactId>maven-checkstyle-plugin</artifactId>
                     <version>2.11</version>
+                    <dependencies>
+                        <dependency>
+                            <groupId>com.puppycrawl.tools</groupId>
+                            <artifactId>checkstyle</artifactId>
+                            <version>6.5</version>
+                        </dependency>
+                    </dependencies>
                     <executions>
                         <execution>
                             <id>validate</id>
@@ -722,6 +729,12 @@
                 </plugins>
             </build>
         </profile>
+        <profile>
+            <id>java8-test</id>
+            <modules>
+                <module>twill-java8-test</module>
+            </modules>
+        </profile>
     </profiles>
 
     <dependencyManagement>

http://git-wip-us.apache.org/repos/asf/incubator-twill/blob/ae24a049/twill-core/src/main/java/org/apache/twill/internal/utils/Dependencies.java
----------------------------------------------------------------------
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 015b9f5..90f9d18 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
@@ -30,6 +30,7 @@ import org.objectweb.asm.Label;
 import org.objectweb.asm.MethodVisitor;
 import org.objectweb.asm.Opcodes;
 import org.objectweb.asm.Type;
+import org.objectweb.asm.TypePath;
 import org.objectweb.asm.signature.SignatureReader;
 import org.objectweb.asm.signature.SignatureVisitor;
 
@@ -158,25 +159,14 @@ public final class Dependencies {
   private static final class DependencyClassVisitor extends ClassVisitor {
 
     private final SignatureVisitor signatureVisitor;
+    private final AnnotationVisitor annotationVisitor;
     private final DependencyAcceptor acceptor;
 
     public DependencyClassVisitor(DependencyAcceptor acceptor) {
-      super(Opcodes.ASM4);
+      super(Opcodes.ASM5);
       this.acceptor = acceptor;
-      this.signatureVisitor = new SignatureVisitor(Opcodes.ASM4) {
-        private String currentClass;
-
-        @Override
-        public void visitClassType(String name) {
-          currentClass = name;
-          addClass(name);
-        }
-
-        @Override
-        public void visitInnerClassType(String name) {
-          addClass(currentClass + "$" + name);
-        }
-      };
+      this.signatureVisitor = createSignatureVisitor();
+      this.annotationVisitor = createAnnotationVisitor();
     }
 
     @Override
@@ -198,8 +188,11 @@ public final class Dependencies {
 
     @Override
     public AnnotationVisitor visitAnnotation(String desc, boolean visible) {
+      if (!visible) {
+        return null;
+      }
       addType(Type.getType(desc));
-      return null;
+      return annotationVisitor;
     }
 
     @Override
@@ -215,11 +208,14 @@ public final class Dependencies {
         addType(Type.getType(desc));
       }
 
-      return new FieldVisitor(Opcodes.ASM4) {
+      return new FieldVisitor(Opcodes.ASM5) {
         @Override
         public AnnotationVisitor visitAnnotation(String desc, boolean visible) {
+          if (!visible) {
+            return null;
+          }
           addType(Type.getType(desc));
-          return null;
+          return annotationVisitor;
         }
       };
     }
@@ -233,17 +229,24 @@ public final class Dependencies {
       }
       addClasses(exceptions);
 
-      return new MethodVisitor(Opcodes.ASM4) {
+      return new MethodVisitor(Opcodes.ASM5) {
+
         @Override
         public AnnotationVisitor visitAnnotation(String desc, boolean visible) {
+          if (!visible) {
+            return null;
+          }
           addType(Type.getType(desc));
-          return null;
+          return annotationVisitor;
         }
 
         @Override
         public AnnotationVisitor visitParameterAnnotation(int parameter, String desc, boolean visible) {
+          if (!visible) {
+            return null;
+          }
           addType(Type.getType(desc));
-          return null;
+          return annotationVisitor;
         }
 
         @Override
@@ -258,7 +261,7 @@ public final class Dependencies {
         }
 
         @Override
-        public void visitMethodInsn(int opcode, String owner, String name, String desc) {
+        public void visitMethodInsn(int opcode, String owner, String name, String desc, boolean itf) {
           addType(Type.getObjectType(owner));
           addMethod(desc);
         }
@@ -271,6 +274,16 @@ public final class Dependencies {
         }
 
         @Override
+        public AnnotationVisitor visitLocalVariableAnnotation(int typeRef, TypePath typePath, Label[] start,
+                                                              Label[] end, int[] index, String desc, boolean visible) {
+          if (!visible) {
+            return null;
+          }
+          addType(Type.getType(desc));
+          return annotationVisitor;
+        }
+
+        @Override
         public void visitMultiANewArrayInsn(String desc, int dims) {
           addType(Type.getType(desc));
         }
@@ -316,6 +329,56 @@ public final class Dependencies {
         addType(type);
       }
     }
+
+    /**
+     * Creates a {@link SignatureVisitor} for gathering dependency information from class signature.
+     */
+    private SignatureVisitor createSignatureVisitor() {
+      return new SignatureVisitor(Opcodes.ASM5) {
+        private String currentClass;
+
+        @Override
+        public void visitClassType(String name) {
+          currentClass = name;
+          addClass(name);
+        }
+
+        @Override
+        public void visitInnerClassType(String name) {
+          addClass(currentClass + "$" + name);
+        }
+      };
+    }
+
+    /**
+     * Creates an {@link AnnotationVisitor} for gathering dependency information from annotations.
+     */
+    private AnnotationVisitor createAnnotationVisitor() {
+      return new AnnotationVisitor(Opcodes.ASM5) {
+        @Override
+        public void visit(String name, Object value) {
+          if (value instanceof Type) {
+            addType((Type) value);
+          }
+        }
+
+        @Override
+        public AnnotationVisitor visitAnnotation(String name, String desc) {
+          addType(Type.getType(desc));
+          return this;
+        }
+
+        @Override
+        public AnnotationVisitor visitArray(String name) {
+          return this;
+        }
+
+        @Override
+        public void visitEnum(String name, String desc, String value) {
+          addType(Type.getType(desc));
+        }
+      };
+    }
   }
 
   private Dependencies() {

http://git-wip-us.apache.org/repos/asf/incubator-twill/blob/ae24a049/twill-java8-test/pom.xml
----------------------------------------------------------------------
diff --git a/twill-java8-test/pom.xml b/twill-java8-test/pom.xml
new file mode 100644
index 0000000..9f020f8
--- /dev/null
+++ b/twill-java8-test/pom.xml
@@ -0,0 +1,81 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one
+  or more contributor license agreements.  See the NOTICE file
+  distributed with this work for additional information
+  regarding copyright ownership.  The ASF licenses this file
+  to you under the Apache License, Version 2.0 (the
+  "License"); you may not use this file except in compliance
+  with the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+  -->
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <groupId>org.apache.twill</groupId>
+        <artifactId>twill-parent</artifactId>
+        <version>0.6.0-incubating-SNAPSHOT</version>
+    </parent>
+
+    <artifactId>twill-java8-test</artifactId>
+    <packaging>jar</packaging>
+    <name>Apache Twill Test Module for Java8</name>
+
+    <dependencies>
+        <dependency>
+            <groupId>${project.groupId}</groupId>
+            <artifactId>twill-yarn</artifactId>
+            <version>${project.version}</version>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>${project.groupId}</groupId>
+            <artifactId>twill-yarn</artifactId>
+            <version>${project.version}</version>
+            <type>test-jar</type>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.hadoop</groupId>
+            <artifactId>hadoop-minicluster</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>junit</groupId>
+            <artifactId>junit</artifactId>
+        </dependency>
+    </dependencies>
+
+    <build>
+        <plugins>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-compiler-plugin</artifactId>
+                <version>3.1</version>
+                <configuration>
+                    <source>1.8</source>
+                    <target>1.8</target>
+                </configuration>
+            </plugin>
+
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-deploy-plugin</artifactId>
+                <version>2.8.1</version>
+                <configuration>
+                    <skip>true</skip>
+                </configuration>
+            </plugin>
+        </plugins>
+    </build>
+
+</project>

http://git-wip-us.apache.org/repos/asf/incubator-twill/blob/ae24a049/twill-java8-test/src/test/java/org/apache/twill/test/Java8Test.java
----------------------------------------------------------------------
diff --git a/twill-java8-test/src/test/java/org/apache/twill/test/Java8Test.java b/twill-java8-test/src/test/java/org/apache/twill/test/Java8Test.java
new file mode 100644
index 0000000..1eca0dc
--- /dev/null
+++ b/twill-java8-test/src/test/java/org/apache/twill/test/Java8Test.java
@@ -0,0 +1,91 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.twill.test;
+
+import com.google.common.util.concurrent.Uninterruptibles;
+import org.apache.twill.api.AbstractTwillRunnable;
+import org.apache.twill.api.TwillController;
+import org.apache.twill.api.TwillRunnable;
+import org.apache.twill.api.TwillRunner;
+import org.apache.twill.api.logging.PrinterLogHandler;
+import org.apache.twill.yarn.BaseYarnTest;
+import org.apache.twill.yarn.YarnTestUtils;
+import org.junit.Assert;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.PrintWriter;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Unit test for running twill under Java8.
+ */
+public class Java8Test extends BaseYarnTest {
+
+  @Test
+  public void test() throws ExecutionException, InterruptedException {
+    TwillRunner runner = YarnTestUtils.getTwillRunner();
+
+    // Start the TestRunnable and make sure it is executed with the log message emitted.
+    CountDownLatch logLatch = new CountDownLatch(1);
+    TwillController controller = runner.prepare(new TestRunnable())
+      .addLogHandler(new PrinterLogHandler(new PrintWriter(System.out, true)))
+      .addLogHandler(logEntry -> {
+        if ("Hello World".equals(logEntry.getMessage())) {
+          logLatch.countDown();
+        }
+      })
+      .start();
+
+    Assert.assertTrue(logLatch.await(120, TimeUnit.SECONDS));
+    controller.stopAndWait();
+  }
+
+  /**
+   * A {@link TwillRunnable} that emits a log message and wait to be stopped.
+   */
+  public static final class TestRunnable extends AbstractTwillRunnable {
+
+    private static final Logger LOG = LoggerFactory.getLogger(TestRunnable.class);
+
+    private final CountDownLatch stopLatch = new CountDownLatch(1);
+
+    @Override
+    public void run() {
+      LOG.info(Compute.getMessage());
+      Uninterruptibles.awaitUninterruptibly(stopLatch);
+    }
+
+    @Override
+    public void stop() {
+      stopLatch.countDown();
+    }
+  }
+
+  /**
+   * An interface with static method, which is a Java 8 feature.
+   */
+  public interface Compute {
+    static String getMessage() {
+      return "Hello World";
+    }
+  }
+}

http://git-wip-us.apache.org/repos/asf/incubator-twill/blob/ae24a049/twill-yarn/pom.xml
----------------------------------------------------------------------
diff --git a/twill-yarn/pom.xml b/twill-yarn/pom.xml
index f1f7c97..4c6abff 100644
--- a/twill-yarn/pom.xml
+++ b/twill-yarn/pom.xml
@@ -88,6 +88,21 @@
 
     <build>
         <outputDirectory>${output.dir}</outputDirectory>
+        <plugins>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-jar-plugin</artifactId>
+                <version>2.4</version>
+                <executions>
+                    <execution>
+                        <id>test-jar</id>
+                        <goals>
+                            <goal>test-jar</goal>
+                        </goals>
+                    </execution>
+                </executions>
+            </plugin>
+        </plugins>
     </build>
 
     <profiles>

http://git-wip-us.apache.org/repos/asf/incubator-twill/blob/ae24a049/twill-yarn/src/main/java/org/apache/twill/internal/yarn/AbstractYarnAMClient.java
----------------------------------------------------------------------
diff --git a/twill-yarn/src/main/java/org/apache/twill/internal/yarn/AbstractYarnAMClient.java b/twill-yarn/src/main/java/org/apache/twill/internal/yarn/AbstractYarnAMClient.java
index cb6a58e..d28f810 100644
--- a/twill-yarn/src/main/java/org/apache/twill/internal/yarn/AbstractYarnAMClient.java
+++ b/twill-yarn/src/main/java/org/apache/twill/internal/yarn/AbstractYarnAMClient.java
@@ -139,7 +139,8 @@ public abstract class AbstractYarnAMClient<T> extends AbstractIdleService implem
         RunnableProcessLauncher launcher = (RunnableProcessLauncher) l;
         if (!launcher.isLaunched()) {
           YarnContainerInfo containerInfo = launcher.getContainerInfo();
-          LOG.info("Nothing to run in container, releasing it: {}", containerInfo.getContainer());
+          // Casting is needed in Java 8, otherwise it complains about ambiguous method over the info(String, Throwable)
+          LOG.info("Nothing to run in container, releasing it: {}", (Object) containerInfo.getContainer());
           releaseAssignedContainer(containerInfo);
         }
       }