From 1aa6cdb7d31599a7f78516a1c6207a2dd08421c9 Mon Sep 17 00:00:00 2001 From: Tad Cordle Date: Tue, 15 May 2018 16:36:11 -0400 Subject: [PATCH 1/9] Add core functionality --- .../tools/jib/frontend/MainClassFinder.java | 120 ++++++++++++++++++ .../jib/frontend/MainClassFinderTest.java | 75 +++++++++++ .../tools/jib/gradle/ProjectProperties.java | 24 +++- .../tools/jib/maven/ProjectProperties.java | 25 +++- 4 files changed, 237 insertions(+), 7 deletions(-) create mode 100644 jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MainClassFinder.java create mode 100644 jib-core/src/test/java/com/google/cloud/tools/jib/frontend/MainClassFinderTest.java diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MainClassFinder.java b/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MainClassFinder.java new file mode 100644 index 0000000000..de310a2d82 --- /dev/null +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MainClassFinder.java @@ -0,0 +1,120 @@ +/* + * Copyright 2018 Google LLC. All Rights Reserved. + * + * Licensed 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 com.google.cloud.tools.jib.frontend; + +import com.google.common.base.Strings; +import java.io.IOException; +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import javax.annotation.Nullable; +import jdk.internal.org.objectweb.asm.ClassReader; +import jdk.internal.org.objectweb.asm.ClassVisitor; +import jdk.internal.org.objectweb.asm.MethodVisitor; +import jdk.internal.org.objectweb.asm.Opcodes; +import jdk.internal.org.objectweb.asm.Type; + +/** Class used for inferring the main class in an application. */ +public class MainClassFinder { + + /** Exception thrown when multiple main class candidates are found. */ + public static class MultipleClassesFoundException extends Exception { + MultipleClassesFoundException() { + super("Multiple classes found while trying to infer main class"); + } + } + + /** ClassVisitor used to search for main method within a class file. */ + private static class ClassDescriptor extends ClassVisitor { + private boolean foundMainMethod; + + private ClassDescriptor() { + super(Opcodes.ASM5); + } + + /** Builds a ClassDescriptor from an input stream. */ + static ClassDescriptor build(InputStream inputStream) throws IOException { + ClassReader classReader = new ClassReader(inputStream); + ClassDescriptor classDescriptor = new ClassDescriptor(); + classReader.accept(classDescriptor, ClassReader.SKIP_CODE); + return classDescriptor; + } + + @Nullable + @Override + public MethodVisitor visitMethod( + int access, String name, String desc, String signature, String[] exceptions) { + Type methodType = Type.getMethodType(Type.VOID_TYPE, Type.getType(String[].class)); + if ((access & Opcodes.ACC_PUBLIC) != 0 + && (access & Opcodes.ACC_STATIC) != 0 + && name.equals("main") + && desc.equals(methodType.getDescriptor())) { + this.foundMainMethod = true; + } + return null; + } + + boolean isMainMethodFound() { + return foundMainMethod; + } + } + + /** + * Searches for a class containing a main method in a list of files. + * + * @return the name of the class if one is found, null if no class is found. + * @throws MultipleClassesFoundException if more than one valid main class is found. + */ + @Nullable + public static String findMainClass(List classFiles, String rootDirectory) + throws MultipleClassesFoundException { + String className = null; + for (Path classFile : classFiles) { + // Skip non-class files + if (!classFile.toString().endsWith(".class")) { + continue; + } + + try (InputStream inputStream = Files.newInputStream(classFile)) { + ClassDescriptor classDescriptor = ClassDescriptor.build(inputStream); + if (!classDescriptor.isMainMethodFound()) { + // Valid class, but has no main method + continue; + } + } catch (IOException | ArrayIndexOutOfBoundsException ex) { + // Not a valid class file + continue; + } + + if (className == null) { + // Found a valid main class, save it and continue + className = classFile.toAbsolutePath().toString(); + if (!Strings.isNullOrEmpty(rootDirectory)) { + className = className.substring(rootDirectory.length() + 1); + } + className = className.replace('/', '.').replace('\\', '.'); + className = className.substring(0, className.length() - ".class".length()); + } else { + // Found more than one valid main class, error out + throw new MultipleClassesFoundException(); + } + } + + return className; + } +} diff --git a/jib-core/src/test/java/com/google/cloud/tools/jib/frontend/MainClassFinderTest.java b/jib-core/src/test/java/com/google/cloud/tools/jib/frontend/MainClassFinderTest.java new file mode 100644 index 0000000000..0729957843 --- /dev/null +++ b/jib-core/src/test/java/com/google/cloud/tools/jib/frontend/MainClassFinderTest.java @@ -0,0 +1,75 @@ +/* + * Copyright 2018 Google LLC. All Rights Reserved. + * + * Licensed 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 com.google.cloud.tools.jib.frontend; + +import com.google.cloud.tools.jib.frontend.MainClassFinder.MultipleClassesFoundException; +import com.google.common.io.Resources; +import java.io.IOException; +import java.net.URISyntaxException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.List; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import org.junit.Assert; +import org.junit.Test; + +/** Test for MainClassFinder. */ +public class MainClassFinderTest { + + @Test + public void testFindMainClass_simple() + throws URISyntaxException, MultipleClassesFoundException, IOException { + Path rootDirectory = Paths.get(Resources.getResource("application/classes").toURI()); + try (Stream classStream = Files.walk(rootDirectory)) { + List classFiles = classStream.collect(Collectors.toList()); + + String mainClass = MainClassFinder.findMainClass(classFiles, rootDirectory.toString()); + + Assert.assertEquals("HelloWorld", mainClass); + } + } + + @Test + public void testFindMainClass_none() throws URISyntaxException, MultipleClassesFoundException { + Path rootDirectory = Paths.get(Resources.getResource("application/classes").toURI()); + List classFiles = new ArrayList<>(); + + String mainClass = MainClassFinder.findMainClass(classFiles, rootDirectory.toString()); + + Assert.assertEquals(null, mainClass); + } + + @Test + public void testFindMainClass_multiple() throws URISyntaxException, IOException { + Path rootDirectory = Paths.get(Resources.getResource("application/classes").toURI()); + try (Stream classStream = Files.walk(rootDirectory)) { + List classFiles = classStream.collect(Collectors.toList()); + classFiles.add( + Paths.get(Resources.getResource("application/classes/HelloWorld.class").toURI())); + try { + MainClassFinder.findMainClass(classFiles, rootDirectory.toString()); + Assert.fail(); + } catch (MultipleClassesFoundException ex) { + Assert.assertEquals( + "Multiple classes found while trying to infer main class", ex.getMessage()); + } + } + } +} diff --git a/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ProjectProperties.java b/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ProjectProperties.java index 619bf457fa..153f4265a0 100644 --- a/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ProjectProperties.java +++ b/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ProjectProperties.java @@ -18,6 +18,8 @@ import com.google.cloud.tools.jib.builder.BuildConfiguration; import com.google.cloud.tools.jib.builder.SourceFilesConfiguration; +import com.google.cloud.tools.jib.frontend.MainClassFinder; +import com.google.cloud.tools.jib.frontend.MainClassFinder.MultipleClassesFoundException; import java.io.IOException; import java.util.ArrayList; import java.util.List; @@ -47,9 +49,25 @@ String getMainClass(@Nullable String mainClass) { if (mainClass == null) { mainClass = getMainClassFromJarTask(); if (mainClass == null) { - throw new GradleException( - HelpfulSuggestionsProvider.get("Could not find main class specified in a 'jar' task") - .suggest("add a `mainClass` configuration to jib")); + getLogger() + .info( + "Could not find main class specified in a 'jar' task; attempting to " + + "infer main class."); + try { + mainClass = + MainClassFinder.findMainClass( + getSourceFilesConfiguration().getClassesFiles(), + project.getBuildDir().getAbsolutePath()); + } catch (MultipleClassesFoundException ex) { + throw new GradleException( + HelpfulSuggestionsProvider.get("Failed to get main class: " + ex.getMessage()) + .suggest("add a `mainClass` configuration to jib-maven-plugin")); + } + if (mainClass == null) { + throw new GradleException( + HelpfulSuggestionsProvider.get("Could not infer main class") + .suggest("add a `mainClass` configuration to jib-maven-plugin")); + } } } if (!BuildConfiguration.isValidJavaClass(mainClass)) { diff --git a/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/ProjectProperties.java b/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/ProjectProperties.java index 89cc81c58e..bf0fbe804b 100644 --- a/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/ProjectProperties.java +++ b/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/ProjectProperties.java @@ -18,6 +18,8 @@ import com.google.cloud.tools.jib.builder.BuildConfiguration; import com.google.cloud.tools.jib.builder.SourceFilesConfiguration; +import com.google.cloud.tools.jib.frontend.MainClassFinder; +import com.google.cloud.tools.jib.frontend.MainClassFinder.MultipleClassesFoundException; import com.google.common.base.Preconditions; import java.io.IOException; import javax.annotation.Nullable; @@ -85,10 +87,25 @@ String getMainClass(@Nullable String mainClass) throws MojoExecutionException { if (mainClass == null) { mainClass = getMainClassFromMavenJarPlugin(); if (mainClass == null) { - throw new MojoExecutionException( - HelpfulSuggestionsProvider.get( - "Could not find main class specified in maven-jar-plugin") - .suggest("add a `mainClass` configuration to jib-maven-plugin")); + getLog() + .info( + "Could not find main class specified in maven-jar-plugin; attempting to " + + "infer main class."); + try { + mainClass = + MainClassFinder.findMainClass( + getSourceFilesConfiguration().getClassesFiles(), + project.getBuild().getOutputDirectory()); + } catch (MultipleClassesFoundException ex) { + throw new MojoExecutionException( + HelpfulSuggestionsProvider.get("Failed to get main class: " + ex.getMessage()) + .suggest("add a `mainClass` configuration to jib-maven-plugin")); + } + if (mainClass == null) { + throw new MojoExecutionException( + HelpfulSuggestionsProvider.get("Could not infer main class") + .suggest("add a `mainClass` configuration to jib-maven-plugin")); + } } } Preconditions.checkNotNull(mainClass); From 8ac2af50cf467c14bf01e6d91f0196fa53b8c515 Mon Sep 17 00:00:00 2001 From: Tad Cordle Date: Wed, 16 May 2018 11:05:38 -0400 Subject: [PATCH 2/9] Add recursive search, add tests --- .../tools/jib/frontend/MainClassFinder.java | 64 +++++++++--------- .../MultipleClassesFoundException.java | 27 ++++++++ .../jib/frontend/MainClassFinderTest.java | 55 +++++++-------- .../multiple/HelloWorld.class | Bin 0 -> 534 bytes .../class-finder-tests/multiple/NotMain.class | Bin 0 -> 448 bytes .../multiple/multi/Layer1Class.class | Bin 0 -> 337 bytes .../multiple/multi/Layer2Class.class | Bin 0 -> 270 bytes .../multiple/multi/layered/HelloMoon.class | Bin 0 -> 558 bytes .../class-finder-tests/no-main/NotMain.class | Bin 0 -> 448 bytes .../no-main/multi/Layer1Class.class | Bin 0 -> 337 bytes .../no-main/multi/Layer2Class.class | Bin 0 -> 270 bytes .../simple/HelloWorld.class | Bin 0 -> 618 bytes .../simple/NotAClassButMightBe.class | 0 .../simple/NotEvenAClass.txt | 1 + .../class-finder-tests/simple/NotMain.class | Bin 0 -> 448 bytes .../subdirectories/multi/Layer1Class.class | Bin 0 -> 337 bytes .../multi/layered/HelloWorld.class | Bin 0 -> 562 bytes .../multi/layered/Layer2Class.class | Bin 0 -> 286 bytes .../tools/jib/gradle/ProjectProperties.java | 8 +-- .../tools/jib/maven/ProjectProperties.java | 8 +-- 20 files changed, 90 insertions(+), 73 deletions(-) create mode 100644 jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MultipleClassesFoundException.java create mode 100644 jib-core/src/test/resources/class-finder-tests/multiple/HelloWorld.class create mode 100644 jib-core/src/test/resources/class-finder-tests/multiple/NotMain.class create mode 100644 jib-core/src/test/resources/class-finder-tests/multiple/multi/Layer1Class.class create mode 100644 jib-core/src/test/resources/class-finder-tests/multiple/multi/Layer2Class.class create mode 100644 jib-core/src/test/resources/class-finder-tests/multiple/multi/layered/HelloMoon.class create mode 100644 jib-core/src/test/resources/class-finder-tests/no-main/NotMain.class create mode 100644 jib-core/src/test/resources/class-finder-tests/no-main/multi/Layer1Class.class create mode 100644 jib-core/src/test/resources/class-finder-tests/no-main/multi/Layer2Class.class create mode 100644 jib-core/src/test/resources/class-finder-tests/simple/HelloWorld.class create mode 100644 jib-core/src/test/resources/class-finder-tests/simple/NotAClassButMightBe.class create mode 100644 jib-core/src/test/resources/class-finder-tests/simple/NotEvenAClass.txt create mode 100644 jib-core/src/test/resources/class-finder-tests/simple/NotMain.class create mode 100644 jib-core/src/test/resources/class-finder-tests/subdirectories/multi/Layer1Class.class create mode 100644 jib-core/src/test/resources/class-finder-tests/subdirectories/multi/layered/HelloWorld.class create mode 100644 jib-core/src/test/resources/class-finder-tests/subdirectories/multi/layered/Layer2Class.class diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MainClassFinder.java b/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MainClassFinder.java index de310a2d82..a532ceb22c 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MainClassFinder.java +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MainClassFinder.java @@ -21,7 +21,10 @@ import java.io.InputStream; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.Paths; import java.util.List; +import java.util.stream.Collectors; +import java.util.stream.Stream; import javax.annotation.Nullable; import jdk.internal.org.objectweb.asm.ClassReader; import jdk.internal.org.objectweb.asm.ClassVisitor; @@ -32,13 +35,6 @@ /** Class used for inferring the main class in an application. */ public class MainClassFinder { - /** Exception thrown when multiple main class candidates are found. */ - public static class MultipleClassesFoundException extends Exception { - MultipleClassesFoundException() { - super("Multiple classes found while trying to infer main class"); - } - } - /** ClassVisitor used to search for main method within a class file. */ private static class ClassDescriptor extends ClassVisitor { private boolean foundMainMethod; @@ -75,43 +71,47 @@ boolean isMainMethodFound() { } /** - * Searches for a class containing a main method in a list of files. + * Searches for a class containing a main method given a root directory. * * @return the name of the class if one is found, null if no class is found. * @throws MultipleClassesFoundException if more than one valid main class is found. */ @Nullable - public static String findMainClass(List classFiles, String rootDirectory) - throws MultipleClassesFoundException { + public static String findMainClass(String rootDirectory) + throws MultipleClassesFoundException, IOException { String className = null; - for (Path classFile : classFiles) { - // Skip non-class files - if (!classFile.toString().endsWith(".class")) { - continue; - } + try (Stream pathStream = Files.walk(Paths.get(rootDirectory))) { + List classFiles = pathStream.filter(Files::isRegularFile).collect(Collectors.toList()); + for (Path classFile : classFiles) { + // Skip non-class files + if (!classFile.toString().endsWith(".class")) { + continue; + } - try (InputStream inputStream = Files.newInputStream(classFile)) { - ClassDescriptor classDescriptor = ClassDescriptor.build(inputStream); - if (!classDescriptor.isMainMethodFound()) { - // Valid class, but has no main method + try (InputStream inputStream = Files.newInputStream(classFile)) { + ClassDescriptor classDescriptor = ClassDescriptor.build(inputStream); + if (!classDescriptor.isMainMethodFound()) { + // Valid class, but has no main method + continue; + } + } catch (IOException | ArrayIndexOutOfBoundsException ex) { + // Not a valid class file continue; } - } catch (IOException | ArrayIndexOutOfBoundsException ex) { - // Not a valid class file - continue; - } - if (className == null) { - // Found a valid main class, save it and continue - className = classFile.toAbsolutePath().toString(); + String name = classFile.toAbsolutePath().toString(); if (!Strings.isNullOrEmpty(rootDirectory)) { - className = className.substring(rootDirectory.length() + 1); + name = name.substring(rootDirectory.length() + 1); + } + name = name.replace('/', '.').replace('\\', '.'); + name = name.substring(0, name.length() - ".class".length()); + if (className == null) { + // Found a valid main class, save it and continue + className = name; + } else { + // Found more than one valid main class, error out + throw new MultipleClassesFoundException(className, name); } - className = className.replace('/', '.').replace('\\', '.'); - className = className.substring(0, className.length() - ".class".length()); - } else { - // Found more than one valid main class, error out - throw new MultipleClassesFoundException(); } } diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MultipleClassesFoundException.java b/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MultipleClassesFoundException.java new file mode 100644 index 0000000000..70d91ee4f2 --- /dev/null +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MultipleClassesFoundException.java @@ -0,0 +1,27 @@ +/* + * Copyright 2018 Google LLC. All Rights Reserved. + * + * Licensed 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 com.google.cloud.tools.jib.frontend; + +public class MultipleClassesFoundException extends Exception { + MultipleClassesFoundException(String className1, String className2) { + super( + "Multiple classes found while trying to infer main class: " + + className1 + + ", " + + className2); + } +} diff --git a/jib-core/src/test/java/com/google/cloud/tools/jib/frontend/MainClassFinderTest.java b/jib-core/src/test/java/com/google/cloud/tools/jib/frontend/MainClassFinderTest.java index 0729957843..55da5aa458 100644 --- a/jib-core/src/test/java/com/google/cloud/tools/jib/frontend/MainClassFinderTest.java +++ b/jib-core/src/test/java/com/google/cloud/tools/jib/frontend/MainClassFinderTest.java @@ -16,17 +16,11 @@ package com.google.cloud.tools.jib.frontend; -import com.google.cloud.tools.jib.frontend.MainClassFinder.MultipleClassesFoundException; import com.google.common.io.Resources; import java.io.IOException; import java.net.URISyntaxException; -import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; -import java.util.ArrayList; -import java.util.List; -import java.util.stream.Collectors; -import java.util.stream.Stream; import org.junit.Assert; import org.junit.Test; @@ -36,40 +30,39 @@ public class MainClassFinderTest { @Test public void testFindMainClass_simple() throws URISyntaxException, MultipleClassesFoundException, IOException { - Path rootDirectory = Paths.get(Resources.getResource("application/classes").toURI()); - try (Stream classStream = Files.walk(rootDirectory)) { - List classFiles = classStream.collect(Collectors.toList()); - - String mainClass = MainClassFinder.findMainClass(classFiles, rootDirectory.toString()); - - Assert.assertEquals("HelloWorld", mainClass); - } + Path rootDirectory = Paths.get(Resources.getResource("class-finder-tests/simple").toURI()); + String mainClass = MainClassFinder.findMainClass(rootDirectory.toString()); + Assert.assertEquals("HelloWorld", mainClass); } @Test - public void testFindMainClass_none() throws URISyntaxException, MultipleClassesFoundException { - Path rootDirectory = Paths.get(Resources.getResource("application/classes").toURI()); - List classFiles = new ArrayList<>(); - - String mainClass = MainClassFinder.findMainClass(classFiles, rootDirectory.toString()); + public void testFindMainClass_subdirectories() + throws URISyntaxException, MultipleClassesFoundException, IOException { + Path rootDirectory = + Paths.get(Resources.getResource("class-finder-tests/subdirectories").toURI()); + String mainClass = MainClassFinder.findMainClass(rootDirectory.toString()); + Assert.assertEquals("multi.layered.HelloWorld", mainClass); + } + @Test + public void testFindMainClass_noClass() + throws URISyntaxException, MultipleClassesFoundException, IOException { + Path rootDirectory = Paths.get(Resources.getResource("class-finder-tests/no-main").toURI()); + String mainClass = MainClassFinder.findMainClass(rootDirectory.toString()); Assert.assertEquals(null, mainClass); } @Test public void testFindMainClass_multiple() throws URISyntaxException, IOException { - Path rootDirectory = Paths.get(Resources.getResource("application/classes").toURI()); - try (Stream classStream = Files.walk(rootDirectory)) { - List classFiles = classStream.collect(Collectors.toList()); - classFiles.add( - Paths.get(Resources.getResource("application/classes/HelloWorld.class").toURI())); - try { - MainClassFinder.findMainClass(classFiles, rootDirectory.toString()); - Assert.fail(); - } catch (MultipleClassesFoundException ex) { - Assert.assertEquals( - "Multiple classes found while trying to infer main class", ex.getMessage()); - } + Path rootDirectory = Paths.get(Resources.getResource("class-finder-tests/multiple").toURI()); + try { + MainClassFinder.findMainClass(rootDirectory.toString()); + Assert.fail(); + } catch (MultipleClassesFoundException ex) { + Assert.assertTrue( + ex.getMessage().contains("Multiple classes found while trying to infer main class: ")); + Assert.assertTrue(ex.getMessage().contains("multi.layered.HelloMoon")); + Assert.assertTrue(ex.getMessage().contains("HelloWorld")); } } } diff --git a/jib-core/src/test/resources/class-finder-tests/multiple/HelloWorld.class b/jib-core/src/test/resources/class-finder-tests/multiple/HelloWorld.class new file mode 100644 index 0000000000000000000000000000000000000000..d47291bbb363b84d0f0beeff35270eb955a69ffe GIT binary patch literal 534 zcmZuu%TB^j5IvVyX|3`o;9J2Jy09A)7Z^7t#zmth7+kozzztk-+iObE_*uHp#DyQ= zM;WKZq>|9ZOy``Lb7tD_pU*D z7kVfdN}deF^~~>!@Q(KtAyrQgxa#px#`PbVi^g)wV0%|WDSfX)HSCav&t<^SXg_!p z{={9ygOS^fLK%!Yi3blyH03Oz8LY0Jg#&RZW7ATqdmKyIFi;&Wf}>ej@L6H>hpc2c5i9fh*lQdr-PhCV-aRB9^)oO zm)6_M4n+%j`t0$O0kxw@(WJ_y%qZuNKEPfR+M-xUzzSCA-JUm4UZu*!8cKx3!>5TD fVQDOLMq%8jp|XB4EA`MR}(HX z@^cbVO|~8T!_j{|omsvvVsR^#vD;|R)Nn4^uo+J0@3dAIdjf1$J6ftz#F6XgHU^VUJ=YmvX+E)pkg^I_MuFdBGjz( zf+P}pB~n%Li)iLsl@a>EzR@D}c7V*ZPhdUYNc!a^*WXZU9BBPyAu1T^X#1s aXuxGgXkNp^inDef9NYxy8uZw1p!ElXKRYh~ literal 0 HcmV?d00001 diff --git a/jib-core/src/test/resources/class-finder-tests/multiple/multi/Layer2Class.class b/jib-core/src/test/resources/class-finder-tests/multiple/multi/Layer2Class.class new file mode 100644 index 0000000000000000000000000000000000000000..daf0a2e46ec0c22862c641d1fc0d6fbfb21d6f40 GIT binary patch literal 270 zcmZXO&1%9>5QWd=*QU{EeS+@Pg#qcV#ZACf(1peQO}yw0*97t-^s%}Uy6^#fsNzIi z2+qLAoHKJ^F8_aL03YakFwu3fX4qExNEn^wJ76=4`38eOX}7C*9_MOswVC&616O_|DtYUBNf-82(N;lqMW7(6BU zw%vzX#MiOmIlsF{L%(vvi?}3qQb* zGTs&=lHe@v?LFuA%(-(vzurFp9An=^0qX{;sOhMi*g!+WriLws!l?{objFZvAM_Y< z=V~YzO0EpVb?o;=c*px5Ar)5*xYy&MOzIyn7d^-cL&NoBFOrVOry>+X=Tdl{x>73W zkf6_Hz))-7yJP;usV5jY-6)j7sFPmE!x2f?OK1kOtKx7VE@a|h^;h;NQL>;x*RXA2 z2fGZ#lx_NSYfHnPg(g}I^?!^pl;>`4`eQMO7NG8Q5(%FkMa4u^XV#^1Zpn8DOLMq%8jp|XB4EA`MR}(HX z@^cbVO|~8T!_j{|omsvvVsR^#vD;|R)Nn4^uo+J0@3dAIdjf1$J6ftz#F6XgHU^VUJ=YmvX+E)pkg^I_MuFdBGjz( zf+P}pB~n%Li)iLsl@a>EzR@D}c7V*ZPhdUYNc!a^*WXZU9BBPyAu1T^X#1s aXuxGgXkNp^inDef9NYxy8uZw1p!ElXKRYh~ literal 0 HcmV?d00001 diff --git a/jib-core/src/test/resources/class-finder-tests/no-main/multi/Layer2Class.class b/jib-core/src/test/resources/class-finder-tests/no-main/multi/Layer2Class.class new file mode 100644 index 0000000000000000000000000000000000000000..daf0a2e46ec0c22862c641d1fc0d6fbfb21d6f40 GIT binary patch literal 270 zcmZXO&1%9>5QWd=*QU{EeS+@Pg#qcV#ZACf(1peQO}yw0*97t-^s%}Uy6^#fsNzIi z2+qLAoHKJ^F8_aL03YakFwu3fX4qExNEn^wJ76=4`38eOX}7C*9_MOswVC&616O_|DtYUBNf-82(N;lqMW7(6BU zw%vzXH<8154il(mF==87(*|Ztq%j_aSp#zh<^?ia+SB2- zK+>+C3Z!=YOC^wRYEK>ZyKNPm$hJ$Q*z`NnJ(You>TfU=Ug@5I)!b9A>!107d$~!C zuGF5un0?W_mbbFuO3!Ju!a#e@W;{a%4o{W-p#{vA-w!%!S4Tth1KssV%7Td@12qc^ zSmfN8p1+Ic)(k9JSjLKgap;E!T*1OBY$^^!1V#rN9ks7jC;SDq?t7u?@>}?Q5|yv@ zwcj}A?jap0*`>cytnnM>)SIZurSXdWzi-n~;eqP)q(f_EyZ*NeHDvkQq9*}vjv+>i zRfAbDr`Mh#9%6c!F%yFYjPTw1YCs;Nta2!zNF+MEk7UeMACV{~pO9jHMf&_1nI{;m b2ZAGP1xnna%$rmBAzUR|CP#aW35DOLMq%8jp|XB4EA`MR}(HX z@^cbVO|~8T!_j{|omsvvVsR^#vD;|R)Nn4^uo+J0@3dAIdjf1$J6ftz#F6XgHU^VUJ=YmvX+E)pkg^I_MuFdBGjz( zf+P}pB~n%Li)iLsl@a>EzR@D}c7V*ZPhdUYNc!a^*WXZU9BBPyAu1T^X#1s aXuxGgXkNp^inDef9NYxy8uZw1p!ElXKRYh~ literal 0 HcmV?d00001 diff --git a/jib-core/src/test/resources/class-finder-tests/subdirectories/multi/layered/HelloWorld.class b/jib-core/src/test/resources/class-finder-tests/subdirectories/multi/layered/HelloWorld.class new file mode 100644 index 0000000000000000000000000000000000000000..e0a3bb54d2b8a7956a34064421da28cda88aafe6 GIT binary patch literal 562 zcmZ`$O;5r=5Pi#6KdgdO5b+B#)m^ zy9fq?q3B9qT*qEdgm=8B2r0WkpQ|npWm5lwxo9LO40YFwRU{q7ry>*s=SnCQ+y|i= zv`Nw9(r2i)9^5g1;?(31olX==f7niM?JgV*$LvXFD_+bp*Yt)PNXbo&^7E> z*u@@$mC}vp!JxmZ2QJJ}yfpbgNBN{+( zkH)i8jYXJ2drX=X&0T3N8yijJX(h?YfZCx`G^lbYGs^j+cd(a~HYpZT(8daVTeAkr rt5g|SLy?f==hIY-uvcWt**E0o0wz(Q{p+ay5iwB0I^ncliW%5Ht#5g= literal 0 HcmV?d00001 diff --git a/jib-core/src/test/resources/class-finder-tests/subdirectories/multi/layered/Layer2Class.class b/jib-core/src/test/resources/class-finder-tests/subdirectories/multi/layered/Layer2Class.class new file mode 100644 index 0000000000000000000000000000000000000000..994c983fb49f78608ce76665393139908cbccbab GIT binary patch literal 286 zcmZ`!%WA?<5Iqy~s4;ECuEdo)bzwldQ@Rnl3c66--^5FMkm~CMse2YT18rX$xZi>g`@{9VzMseDt$rhhk^pTwsK zV8bB{|8?LSv&ZK7d$dhXGOaO&#Xp!JPBvh4nMExJoY@cPn*|FVqsubzj0eV`?HzRC KGb8jEEd;+kLOo3Y literal 0 HcmV?d00001 diff --git a/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ProjectProperties.java b/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ProjectProperties.java index 153f4265a0..297d96de60 100644 --- a/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ProjectProperties.java +++ b/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ProjectProperties.java @@ -19,7 +19,7 @@ import com.google.cloud.tools.jib.builder.BuildConfiguration; import com.google.cloud.tools.jib.builder.SourceFilesConfiguration; import com.google.cloud.tools.jib.frontend.MainClassFinder; -import com.google.cloud.tools.jib.frontend.MainClassFinder.MultipleClassesFoundException; +import com.google.cloud.tools.jib.frontend.MultipleClassesFoundException; import java.io.IOException; import java.util.ArrayList; import java.util.List; @@ -55,10 +55,8 @@ String getMainClass(@Nullable String mainClass) { + "infer main class."); try { mainClass = - MainClassFinder.findMainClass( - getSourceFilesConfiguration().getClassesFiles(), - project.getBuildDir().getAbsolutePath()); - } catch (MultipleClassesFoundException ex) { + MainClassFinder.findMainClass(project.getBuildDir().getAbsolutePath()); + } catch (MultipleClassesFoundException | IOException ex) { throw new GradleException( HelpfulSuggestionsProvider.get("Failed to get main class: " + ex.getMessage()) .suggest("add a `mainClass` configuration to jib-maven-plugin")); diff --git a/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/ProjectProperties.java b/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/ProjectProperties.java index bf0fbe804b..7ebae0a40b 100644 --- a/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/ProjectProperties.java +++ b/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/ProjectProperties.java @@ -19,7 +19,7 @@ import com.google.cloud.tools.jib.builder.BuildConfiguration; import com.google.cloud.tools.jib.builder.SourceFilesConfiguration; import com.google.cloud.tools.jib.frontend.MainClassFinder; -import com.google.cloud.tools.jib.frontend.MainClassFinder.MultipleClassesFoundException; +import com.google.cloud.tools.jib.frontend.MultipleClassesFoundException; import com.google.common.base.Preconditions; import java.io.IOException; import javax.annotation.Nullable; @@ -93,10 +93,8 @@ String getMainClass(@Nullable String mainClass) throws MojoExecutionException { + "infer main class."); try { mainClass = - MainClassFinder.findMainClass( - getSourceFilesConfiguration().getClassesFiles(), - project.getBuild().getOutputDirectory()); - } catch (MultipleClassesFoundException ex) { + MainClassFinder.findMainClass(project.getBuild().getOutputDirectory()); + } catch (MultipleClassesFoundException | IOException ex) { throw new MojoExecutionException( HelpfulSuggestionsProvider.get("Failed to get main class: " + ex.getMessage()) .suggest("add a `mainClass` configuration to jib-maven-plugin")); From b05a9a92054e82534d7a1ae634ad8e1ab2813082 Mon Sep 17 00:00:00 2001 From: Tad Cordle Date: Wed, 16 May 2018 13:12:36 -0400 Subject: [PATCH 3/9] Fix ProjectProperties unit tests --- .../tools/jib/frontend/MainClassFinder.java | 21 ++++++++++++------- .../tools/jib/gradle/ProjectProperties.java | 7 +++---- .../jib/gradle/ProjectPropertiesTest.java | 18 ++++++++++++---- .../jib/maven/ProjectPropertiesTest.java | 9 +++++++- 4 files changed, 38 insertions(+), 17 deletions(-) diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MainClassFinder.java b/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MainClassFinder.java index a532ceb22c..921e15ff11 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MainClassFinder.java +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MainClassFinder.java @@ -79,15 +79,20 @@ boolean isMainMethodFound() { @Nullable public static String findMainClass(String rootDirectory) throws MultipleClassesFoundException, IOException { + // Make sure rootDirectory is valid + if (!Files.exists(Paths.get(rootDirectory)) || !Files.isDirectory(Paths.get(rootDirectory))) { + return null; + } + String className = null; try (Stream pathStream = Files.walk(Paths.get(rootDirectory))) { - List classFiles = pathStream.filter(Files::isRegularFile).collect(Collectors.toList()); - for (Path classFile : classFiles) { - // Skip non-class files - if (!classFile.toString().endsWith(".class")) { - continue; - } + List classFiles = + pathStream + .filter(Files::isRegularFile) + .filter(path -> path.toString().endsWith(".class")) + .collect(Collectors.toList()); + for (Path classFile : classFiles) { try (InputStream inputStream = Files.newInputStream(classFile)) { ClassDescriptor classDescriptor = ClassDescriptor.build(inputStream); if (!classDescriptor.isMainMethodFound()) { @@ -99,17 +104,17 @@ public static String findMainClass(String rootDirectory) continue; } + // Convert filename to class name String name = classFile.toAbsolutePath().toString(); if (!Strings.isNullOrEmpty(rootDirectory)) { name = name.substring(rootDirectory.length() + 1); } name = name.replace('/', '.').replace('\\', '.'); name = name.substring(0, name.length() - ".class".length()); + if (className == null) { - // Found a valid main class, save it and continue className = name; } else { - // Found more than one valid main class, error out throw new MultipleClassesFoundException(className, name); } } diff --git a/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ProjectProperties.java b/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ProjectProperties.java index 297d96de60..19fcf8d6ac 100644 --- a/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ProjectProperties.java +++ b/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ProjectProperties.java @@ -54,17 +54,16 @@ String getMainClass(@Nullable String mainClass) { "Could not find main class specified in a 'jar' task; attempting to " + "infer main class."); try { - mainClass = - MainClassFinder.findMainClass(project.getBuildDir().getAbsolutePath()); + mainClass = MainClassFinder.findMainClass(project.getBuildDir().getAbsolutePath()); } catch (MultipleClassesFoundException | IOException ex) { throw new GradleException( HelpfulSuggestionsProvider.get("Failed to get main class: " + ex.getMessage()) - .suggest("add a `mainClass` configuration to jib-maven-plugin")); + .suggest("add a `mainClass` configuration to jib")); } if (mainClass == null) { throw new GradleException( HelpfulSuggestionsProvider.get("Could not infer main class") - .suggest("add a `mainClass` configuration to jib-maven-plugin")); + .suggest("add a `mainClass` configuration to jib")); } } } diff --git a/jib-gradle-plugin/src/test/java/com/google/cloud/tools/jib/gradle/ProjectPropertiesTest.java b/jib-gradle-plugin/src/test/java/com/google/cloud/tools/jib/gradle/ProjectPropertiesTest.java index 08caf62537..e3aef546c5 100644 --- a/jib-gradle-plugin/src/test/java/com/google/cloud/tools/jib/gradle/ProjectPropertiesTest.java +++ b/jib-gradle-plugin/src/test/java/com/google/cloud/tools/jib/gradle/ProjectPropertiesTest.java @@ -16,6 +16,8 @@ package com.google.cloud.tools.jib.gradle; +import java.io.File; +import java.io.IOException; import java.util.Collections; import org.gradle.api.GradleException; import org.gradle.api.Project; @@ -29,7 +31,9 @@ import org.hamcrest.CoreMatchers; import org.junit.Assert; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.TemporaryFolder; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.Mockito; @@ -39,6 +43,8 @@ @RunWith(MockitoJUnitRunner.class) public class ProjectPropertiesTest { + @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); + @Mock private FileResolver mockFileResolver; @Mock private Jar mockJar; @Mock private Project mockProject; @@ -48,10 +54,13 @@ public class ProjectPropertiesTest { private ProjectProperties testProjectProperties; @Before - public void setUp() { + public void setUp() throws IOException { fakeManifest = new DefaultManifest(mockFileResolver); Mockito.when(mockJar.getManifest()).thenReturn(fakeManifest); + File tempFolder = temporaryFolder.newFolder(); + Mockito.when(mockProject.getBuildDir()).thenReturn(tempFolder); + testProjectProperties = new ProjectProperties(mockProject, mockLogger); } @@ -95,9 +104,10 @@ private void assertGetMainClassFails() { Assert.fail("Main class not expected"); } catch (GradleException ex) { - Assert.assertThat( - ex.getMessage(), - CoreMatchers.containsString("Could not find main class specified in a 'jar' task")); + Mockito.verify(mockLogger) + .info( + "Could not find main class specified in a 'jar' task; attempting to infer main class."); + Assert.assertThat(ex.getMessage(), CoreMatchers.containsString("Could not infer main class")); Assert.assertThat( ex.getMessage(), CoreMatchers.containsString("add a `mainClass` configuration to jib")); } diff --git a/jib-maven-plugin/src/test/java/com/google/cloud/tools/jib/maven/ProjectPropertiesTest.java b/jib-maven-plugin/src/test/java/com/google/cloud/tools/jib/maven/ProjectPropertiesTest.java index 37f00d851b..65a01c734a 100644 --- a/jib-maven-plugin/src/test/java/com/google/cloud/tools/jib/maven/ProjectPropertiesTest.java +++ b/jib-maven-plugin/src/test/java/com/google/cloud/tools/jib/maven/ProjectPropertiesTest.java @@ -16,6 +16,7 @@ package com.google.cloud.tools.jib.maven; +import org.apache.maven.model.Build; import org.apache.maven.model.Plugin; import org.apache.maven.plugin.MojoExecutionException; import org.apache.maven.plugin.logging.Log; @@ -38,6 +39,8 @@ public class ProjectPropertiesTest { @Mock private Log mockLog; @Mock private Plugin mockJarPlugin; + @Mock private Build mockBuild; + private final Xpp3Dom fakeJarPluginConfiguration = new Xpp3Dom(""); private final Xpp3Dom jarPluginMainClass = new Xpp3Dom("mainClass"); @@ -53,6 +56,9 @@ public void setUp() { Mockito.when(mockJarPlugin.getConfiguration()).thenReturn(fakeJarPluginConfiguration); + Mockito.when(mockMavenProject.getBuild()).thenReturn(mockBuild); + Mockito.when(mockBuild.getOutputDirectory()).thenReturn("fake/dir"); + testProjectProperties = new ProjectProperties(mockMavenProject, mockLog); } @@ -97,9 +103,10 @@ private void assertGetMainClassFails() { Assert.fail("Main class not expected"); } catch (MojoExecutionException ex) { + Mockito.verify(mockLog).info("Could not find main class specified in maven-jar-plugin; attempting to infer main class."); Assert.assertThat( ex.getMessage(), - CoreMatchers.containsString("Could not find main class specified in maven-jar-plugin")); + CoreMatchers.containsString("Could not infer main class")); Assert.assertThat( ex.getMessage(), CoreMatchers.containsString("add a `mainClass` configuration to jib-maven-plugin")); From 49b9e892b1351a03bfd440e24bc333876bf10e98 Mon Sep 17 00:00:00 2001 From: Tad Cordle Date: Wed, 16 May 2018 14:51:27 -0400 Subject: [PATCH 4/9] Use ClassLoader instead of ClassVisitory; use class inference in integration tests --- .../tools/jib/frontend/MainClassFinder.java | 85 ++++++++----------- .../resources/projects/empty/build.gradle | 1 - .../resources/projects/simple/build.gradle | 1 - .../tools/jib/gradle/ProjectProperties.java | 11 ++- .../tools/jib/maven/ProjectProperties.java | 3 +- .../jib/maven/ProjectPropertiesTest.java | 8 +- .../src/test/resources/projects/empty/pom.xml | 1 - .../test/resources/projects/simple/pom.xml | 1 - 8 files changed, 52 insertions(+), 59 deletions(-) diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MainClassFinder.java b/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MainClassFinder.java index 921e15ff11..d556e38ebb 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MainClassFinder.java +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MainClassFinder.java @@ -16,9 +16,9 @@ package com.google.cloud.tools.jib.frontend; -import com.google.common.base.Strings; import java.io.IOException; -import java.io.InputStream; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -26,47 +26,29 @@ import java.util.stream.Collectors; import java.util.stream.Stream; import javax.annotation.Nullable; -import jdk.internal.org.objectweb.asm.ClassReader; -import jdk.internal.org.objectweb.asm.ClassVisitor; -import jdk.internal.org.objectweb.asm.MethodVisitor; -import jdk.internal.org.objectweb.asm.Opcodes; -import jdk.internal.org.objectweb.asm.Type; /** Class used for inferring the main class in an application. */ public class MainClassFinder { - /** ClassVisitor used to search for main method within a class file. */ - private static class ClassDescriptor extends ClassVisitor { - private boolean foundMainMethod; + /** Helper class for loading a .class file. */ + private static class ClassFileLoader extends ClassLoader { - private ClassDescriptor() { - super(Opcodes.ASM5); - } + private Path classFile; - /** Builds a ClassDescriptor from an input stream. */ - static ClassDescriptor build(InputStream inputStream) throws IOException { - ClassReader classReader = new ClassReader(inputStream); - ClassDescriptor classDescriptor = new ClassDescriptor(); - classReader.accept(classDescriptor, ClassReader.SKIP_CODE); - return classDescriptor; + ClassFileLoader(Path classFile) { + this.classFile = classFile; } @Nullable @Override - public MethodVisitor visitMethod( - int access, String name, String desc, String signature, String[] exceptions) { - Type methodType = Type.getMethodType(Type.VOID_TYPE, Type.getType(String[].class)); - if ((access & Opcodes.ACC_PUBLIC) != 0 - && (access & Opcodes.ACC_STATIC) != 0 - && name.equals("main") - && desc.equals(methodType.getDescriptor())) { - this.foundMainMethod = true; + public Class findClass(String name) { + try { + byte[] bytes = Files.readAllBytes(classFile); + return defineClass(name, bytes, 0, bytes.length); + } catch (IOException | ClassFormatError ignored) { + // Not a valid class file + return null; } - return null; - } - - boolean isMainMethodFound() { - return foundMainMethod; } } @@ -93,29 +75,36 @@ public static String findMainClass(String rootDirectory) .collect(Collectors.toList()); for (Path classFile : classFiles) { - try (InputStream inputStream = Files.newInputStream(classFile)) { - ClassDescriptor classDescriptor = ClassDescriptor.build(inputStream); - if (!classDescriptor.isMainMethodFound()) { - // Valid class, but has no main method - continue; - } - } catch (IOException | ArrayIndexOutOfBoundsException ex) { - // Not a valid class file - continue; - } - // Convert filename to class name String name = classFile.toAbsolutePath().toString(); - if (!Strings.isNullOrEmpty(rootDirectory)) { + if (!rootDirectory.isEmpty()) { name = name.substring(rootDirectory.length() + 1); } name = name.replace('/', '.').replace('\\', '.'); name = name.substring(0, name.length() - ".class".length()); - if (className == null) { - className = name; - } else { - throw new MultipleClassesFoundException(className, name); + // Load class from file + Class fileClass = new ClassFileLoader(classFile).findClass(name); + if (fileClass == null) { + continue; + } + + // Check if class contains a public static void main(String[] args) + try { + Method main = fileClass.getMethod("main", String[].class); + if (main != null + && main.getReturnType() == void.class + && Modifier.isStatic(main.getModifiers()) + && Modifier.isPublic(main.getModifiers())) { + if (className == null) { + className = name; + } else { + throw new MultipleClassesFoundException(className, name); + } + } + + } catch (NoSuchMethodException ignored) { + // main method not found } } } diff --git a/jib-gradle-plugin/src/integration-test/resources/projects/empty/build.gradle b/jib-gradle-plugin/src/integration-test/resources/projects/empty/build.gradle index f801759553..d6c59b7ae9 100644 --- a/jib-gradle-plugin/src/integration-test/resources/projects/empty/build.gradle +++ b/jib-gradle-plugin/src/integration-test/resources/projects/empty/build.gradle @@ -15,7 +15,6 @@ jib { image = 'gcr.io/jib-integration-testing/emptyimage:gradle' credHelper = 'gcr' } - mainClass = 'com.test.Empty' // Does not have tests use user-level cache for base image layers. useOnlyProjectCache = true } diff --git a/jib-gradle-plugin/src/integration-test/resources/projects/simple/build.gradle b/jib-gradle-plugin/src/integration-test/resources/projects/simple/build.gradle index 5024f49272..763ba021b9 100644 --- a/jib-gradle-plugin/src/integration-test/resources/projects/simple/build.gradle +++ b/jib-gradle-plugin/src/integration-test/resources/projects/simple/build.gradle @@ -19,7 +19,6 @@ jib { image = 'gcr.io/jib-integration-testing/simpleimage:gradle' credHelper = 'gcr' } - mainClass = 'com.test.HelloWorld' // Does not have tests use user-level cache for base image layers. useOnlyProjectCache = true } diff --git a/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ProjectProperties.java b/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ProjectProperties.java index 19fcf8d6ac..f0b88d02a4 100644 --- a/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ProjectProperties.java +++ b/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ProjectProperties.java @@ -54,7 +54,16 @@ String getMainClass(@Nullable String mainClass) { "Could not find main class specified in a 'jar' task; attempting to " + "infer main class."); try { - mainClass = MainClassFinder.findMainClass(project.getBuildDir().getAbsolutePath()); + mainClass = + MainClassFinder.findMainClass( + project + .getBuildDir() + .toPath() + .resolve("classes") + .resolve("java") + .resolve("main") + .toFile() + .getAbsolutePath()); } catch (MultipleClassesFoundException | IOException ex) { throw new GradleException( HelpfulSuggestionsProvider.get("Failed to get main class: " + ex.getMessage()) diff --git a/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/ProjectProperties.java b/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/ProjectProperties.java index 7ebae0a40b..fd252e5852 100644 --- a/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/ProjectProperties.java +++ b/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/ProjectProperties.java @@ -92,8 +92,7 @@ String getMainClass(@Nullable String mainClass) throws MojoExecutionException { "Could not find main class specified in maven-jar-plugin; attempting to " + "infer main class."); try { - mainClass = - MainClassFinder.findMainClass(project.getBuild().getOutputDirectory()); + mainClass = MainClassFinder.findMainClass(project.getBuild().getOutputDirectory()); } catch (MultipleClassesFoundException | IOException ex) { throw new MojoExecutionException( HelpfulSuggestionsProvider.get("Failed to get main class: " + ex.getMessage()) diff --git a/jib-maven-plugin/src/test/java/com/google/cloud/tools/jib/maven/ProjectPropertiesTest.java b/jib-maven-plugin/src/test/java/com/google/cloud/tools/jib/maven/ProjectPropertiesTest.java index 65a01c734a..cc4cd933b1 100644 --- a/jib-maven-plugin/src/test/java/com/google/cloud/tools/jib/maven/ProjectPropertiesTest.java +++ b/jib-maven-plugin/src/test/java/com/google/cloud/tools/jib/maven/ProjectPropertiesTest.java @@ -103,10 +103,10 @@ private void assertGetMainClassFails() { Assert.fail("Main class not expected"); } catch (MojoExecutionException ex) { - Mockito.verify(mockLog).info("Could not find main class specified in maven-jar-plugin; attempting to infer main class."); - Assert.assertThat( - ex.getMessage(), - CoreMatchers.containsString("Could not infer main class")); + Mockito.verify(mockLog) + .info( + "Could not find main class specified in maven-jar-plugin; attempting to infer main class."); + Assert.assertThat(ex.getMessage(), CoreMatchers.containsString("Could not infer main class")); Assert.assertThat( ex.getMessage(), CoreMatchers.containsString("add a `mainClass` configuration to jib-maven-plugin")); diff --git a/jib-maven-plugin/src/test/resources/projects/empty/pom.xml b/jib-maven-plugin/src/test/resources/projects/empty/pom.xml index 275cf5c7c8..3e301cbb2e 100644 --- a/jib-maven-plugin/src/test/resources/projects/empty/pom.xml +++ b/jib-maven-plugin/src/test/resources/projects/empty/pom.xml @@ -31,7 +31,6 @@ gcr.io/jib-integration-testing/emptyimage:maven - com.test.Empty true diff --git a/jib-maven-plugin/src/test/resources/projects/simple/pom.xml b/jib-maven-plugin/src/test/resources/projects/simple/pom.xml index ac581903a0..34f54f7261 100644 --- a/jib-maven-plugin/src/test/resources/projects/simple/pom.xml +++ b/jib-maven-plugin/src/test/resources/projects/simple/pom.xml @@ -41,7 +41,6 @@ gcr.io/jib-integration-testing/simpleimage:maven - com.test.HelloWorld true From 0389ae2f0007365f03b140a4dde9bca58f00440f Mon Sep 17 00:00:00 2001 From: Tad Cordle Date: Wed, 16 May 2018 15:31:26 -0400 Subject: [PATCH 5/9] Comments and cleanup --- .../tools/jib/frontend/MainClassFinder.java | 19 ++++++++++--------- .../tools/jib/gradle/ProjectProperties.java | 6 +++--- .../tools/jib/maven/ProjectProperties.java | 2 +- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MainClassFinder.java b/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MainClassFinder.java index d556e38ebb..bc1a51ca1e 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MainClassFinder.java +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MainClassFinder.java @@ -35,7 +35,7 @@ private static class ClassFileLoader extends ClassLoader { private Path classFile; - ClassFileLoader(Path classFile) { + private ClassFileLoader(Path classFile) { this.classFile = classFile; } @@ -53,9 +53,10 @@ public Class findClass(String name) { } /** - * Searches for a class containing a main method given a root directory. + * Searches for a class containing a main method given an absolute root directory. * * @return the name of the class if one is found, null if no class is found. + * @throws IOException if searching/reading files fails. * @throws MultipleClassesFoundException if more than one valid main class is found. */ @Nullable @@ -68,6 +69,7 @@ public static String findMainClass(String rootDirectory) String className = null; try (Stream pathStream = Files.walk(Paths.get(rootDirectory))) { + // Get all .class files List classFiles = pathStream .filter(Files::isRegularFile) @@ -75,34 +77,33 @@ public static String findMainClass(String rootDirectory) .collect(Collectors.toList()); for (Path classFile : classFiles) { - // Convert filename to class name + // Convert filename (rootDir/path/to/ClassName.class) to class name (path.to.ClassName) String name = classFile.toAbsolutePath().toString(); - if (!rootDirectory.isEmpty()) { - name = name.substring(rootDirectory.length() + 1); - } + name = name.substring(rootDirectory.length() + 1); name = name.replace('/', '.').replace('\\', '.'); name = name.substring(0, name.length() - ".class".length()); - // Load class from file Class fileClass = new ClassFileLoader(classFile).findClass(name); if (fileClass == null) { + // Got an invalid class file, keep searching continue; } - // Check if class contains a public static void main(String[] args) try { + // Check if class contains a public static void main(String[] args) Method main = fileClass.getMethod("main", String[].class); if (main != null && main.getReturnType() == void.class && Modifier.isStatic(main.getModifiers()) && Modifier.isPublic(main.getModifiers())) { if (className == null) { + // Main method found; save it and continue searching for duplicates className = name; } else { + // Found more than one main method throw new MultipleClassesFoundException(className, name); } } - } catch (NoSuchMethodException ignored) { // main method not found } diff --git a/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ProjectProperties.java b/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ProjectProperties.java index f0b88d02a4..f2883a7ff8 100644 --- a/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ProjectProperties.java +++ b/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ProjectProperties.java @@ -62,8 +62,8 @@ String getMainClass(@Nullable String mainClass) { .resolve("classes") .resolve("java") .resolve("main") - .toFile() - .getAbsolutePath()); + .toAbsolutePath() + .toString()); } catch (MultipleClassesFoundException | IOException ex) { throw new GradleException( HelpfulSuggestionsProvider.get("Failed to get main class: " + ex.getMessage()) @@ -82,7 +82,7 @@ String getMainClass(@Nullable String mainClass) { return mainClass; } - Logger getLogger() { + private Logger getLogger() { return logger; } diff --git a/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/ProjectProperties.java b/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/ProjectProperties.java index fd252e5852..e1ace26d0e 100644 --- a/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/ProjectProperties.java +++ b/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/ProjectProperties.java @@ -150,7 +150,7 @@ private String getMainClassFromMavenJarPlugin(Plugin mavenJarPlugin) { } /** Returns the Maven logger. */ - Log getLog() { + private Log getLog() { return log; } } From 60f3fc561d9d013e4db80d9e83becc3679791917 Mon Sep 17 00:00:00 2001 From: Tad Cordle Date: Wed, 16 May 2018 16:49:06 -0400 Subject: [PATCH 6/9] Remove getLogger() from gradle ProjectProperties --- .../cloud/tools/jib/gradle/ProjectProperties.java | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ProjectProperties.java b/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ProjectProperties.java index f3e6db42f4..c056db364d 100644 --- a/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ProjectProperties.java +++ b/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ProjectProperties.java @@ -48,10 +48,9 @@ String getMainClass(@Nullable String mainClass) { if (mainClass == null) { mainClass = getMainClassFromJarTask(); if (mainClass == null) { - getLogger() - .info( - "Could not find main class specified in a 'jar' task; attempting to " - + "infer main class."); + gradleBuildLogger.info( + "Could not find main class specified in a 'jar' task; attempting to " + + "infer main class."); try { mainClass = MainClassFinder.findMainClass( @@ -76,15 +75,11 @@ String getMainClass(@Nullable String mainClass) { } } if (!BuildConfiguration.isValidJavaClass(mainClass)) { - getLogger().warn("'mainClass' is not a valid Java class : " + mainClass); + gradleBuildLogger.warn("'mainClass' is not a valid Java class : " + mainClass); } return mainClass; } - private GradleBuildLogger getLogger() { - return gradleBuildLogger; - } - /** @return the {@link SourceFilesConfiguration} based on the current project */ SourceFilesConfiguration getSourceFilesConfiguration() { try { From eeeed9acb759ad94d81c0aae116ede8af9e5cccd Mon Sep 17 00:00:00 2001 From: Tad Cordle Date: Thu, 17 May 2018 10:43:45 -0400 Subject: [PATCH 7/9] Feedback --- .../tools/jib/frontend/MainClassFinder.java | 101 ++++++++---------- .../MultipleClassesFoundException.java | 27 ----- .../jib/frontend/MainClassFinderTest.java | 37 +++---- .../tools/jib/gradle/ProjectProperties.java | 50 +++++---- .../jib/gradle/ProjectPropertiesTest.java | 34 ++++-- .../tools/jib/maven/ProjectProperties.java | 34 ++++-- .../jib/maven/ProjectPropertiesTest.java | 3 +- 7 files changed, 139 insertions(+), 147 deletions(-) delete mode 100644 jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MultipleClassesFoundException.java diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MainClassFinder.java b/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MainClassFinder.java index bc1a51ca1e..e6f5cd9607 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MainClassFinder.java +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MainClassFinder.java @@ -16,24 +16,24 @@ package com.google.cloud.tools.jib.frontend; +import com.google.cloud.tools.jib.filesystem.DirectoryWalker; import java.io.IOException; import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.Paths; +import java.util.ArrayList; import java.util.List; -import java.util.stream.Collectors; -import java.util.stream.Stream; +import java.util.StringJoiner; import javax.annotation.Nullable; -/** Class used for inferring the main class in an application. */ +/** Infers the main class in an application. */ public class MainClassFinder { - /** Helper class for loading a .class file. */ + /** Helper for loading a .class file. */ private static class ClassFileLoader extends ClassLoader { - private Path classFile; + private final Path classFile; private ClassFileLoader(Path classFile) { this.classFile = classFile; @@ -53,63 +53,54 @@ public Class findClass(String name) { } /** - * Searches for a class containing a main method given an absolute root directory. + * Searches for a .class file containing a main method given an absolute root directory. * * @return the name of the class if one is found, null if no class is found. * @throws IOException if searching/reading files fails. - * @throws MultipleClassesFoundException if more than one valid main class is found. */ - @Nullable - public static String findMainClass(String rootDirectory) - throws MultipleClassesFoundException, IOException { + public static List findMainClass(Path rootDirectory) throws IOException { + List results = new ArrayList<>(); + // Make sure rootDirectory is valid - if (!Files.exists(Paths.get(rootDirectory)) || !Files.isDirectory(Paths.get(rootDirectory))) { - return null; + if (!Files.exists(rootDirectory) || !Files.isDirectory(rootDirectory)) { + return results; } - String className = null; - try (Stream pathStream = Files.walk(Paths.get(rootDirectory))) { - // Get all .class files - List classFiles = - pathStream - .filter(Files::isRegularFile) - .filter(path -> path.toString().endsWith(".class")) - .collect(Collectors.toList()); - - for (Path classFile : classFiles) { - // Convert filename (rootDir/path/to/ClassName.class) to class name (path.to.ClassName) - String name = classFile.toAbsolutePath().toString(); - name = name.substring(rootDirectory.length() + 1); - name = name.replace('/', '.').replace('\\', '.'); - name = name.substring(0, name.length() - ".class".length()); - - Class fileClass = new ClassFileLoader(classFile).findClass(name); - if (fileClass == null) { - // Got an invalid class file, keep searching - continue; - } + // Get all .class files + new DirectoryWalker(rootDirectory) + .filter(Files::isRegularFile) + .filter(path -> path.toString().endsWith(".class")) + .walk( + classFile -> { + // Convert filename (rootDir/path/to/ClassName.class) to class name + // (path.to.ClassName) + Path relativized = rootDirectory.relativize(classFile); + StringJoiner stringJoiner = new StringJoiner("."); + for (Path path : relativized) { + stringJoiner.add(path.toString()); + } + String name = stringJoiner.toString(); + name = name.substring(0, name.length() - ".class".length()); - try { - // Check if class contains a public static void main(String[] args) - Method main = fileClass.getMethod("main", String[].class); - if (main != null - && main.getReturnType() == void.class - && Modifier.isStatic(main.getModifiers()) - && Modifier.isPublic(main.getModifiers())) { - if (className == null) { - // Main method found; save it and continue searching for duplicates - className = name; - } else { - // Found more than one main method - throw new MultipleClassesFoundException(className, name); - } - } - } catch (NoSuchMethodException ignored) { - // main method not found - } - } - } + Class fileClass = new ClassFileLoader(classFile).findClass(name); + if (fileClass != null) { + try { + // Check if class contains {@code public static void main(String[] args)} + Method main = fileClass.getMethod("main", String[].class); + if (main != null + && main.getReturnType() == void.class + && Modifier.isStatic(main.getModifiers()) + && Modifier.isPublic(main.getModifiers())) { + results.add(name); + } + } catch (NoSuchMethodException ignored) { + // main method not found + } + } + }); - return className; + return results; } + + private MainClassFinder() {} } diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MultipleClassesFoundException.java b/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MultipleClassesFoundException.java deleted file mode 100644 index 70d91ee4f2..0000000000 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MultipleClassesFoundException.java +++ /dev/null @@ -1,27 +0,0 @@ -/* - * Copyright 2018 Google LLC. All Rights Reserved. - * - * Licensed 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 com.google.cloud.tools.jib.frontend; - -public class MultipleClassesFoundException extends Exception { - MultipleClassesFoundException(String className1, String className2) { - super( - "Multiple classes found while trying to infer main class: " - + className1 - + ", " - + className2); - } -} diff --git a/jib-core/src/test/java/com/google/cloud/tools/jib/frontend/MainClassFinderTest.java b/jib-core/src/test/java/com/google/cloud/tools/jib/frontend/MainClassFinderTest.java index 55da5aa458..0358823c1b 100644 --- a/jib-core/src/test/java/com/google/cloud/tools/jib/frontend/MainClassFinderTest.java +++ b/jib-core/src/test/java/com/google/cloud/tools/jib/frontend/MainClassFinderTest.java @@ -21,6 +21,7 @@ import java.net.URISyntaxException; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.List; import org.junit.Assert; import org.junit.Test; @@ -28,41 +29,35 @@ public class MainClassFinderTest { @Test - public void testFindMainClass_simple() - throws URISyntaxException, MultipleClassesFoundException, IOException { + public void testFindMainClass_simple() throws URISyntaxException, IOException { Path rootDirectory = Paths.get(Resources.getResource("class-finder-tests/simple").toURI()); - String mainClass = MainClassFinder.findMainClass(rootDirectory.toString()); - Assert.assertEquals("HelloWorld", mainClass); + List mainClasses = MainClassFinder.findMainClass(rootDirectory); + Assert.assertEquals(1, mainClasses.size()); + Assert.assertTrue(mainClasses.contains("HelloWorld")); } @Test - public void testFindMainClass_subdirectories() - throws URISyntaxException, MultipleClassesFoundException, IOException { + public void testFindMainClass_subdirectories() throws URISyntaxException, IOException { Path rootDirectory = Paths.get(Resources.getResource("class-finder-tests/subdirectories").toURI()); - String mainClass = MainClassFinder.findMainClass(rootDirectory.toString()); - Assert.assertEquals("multi.layered.HelloWorld", mainClass); + List mainClasses = MainClassFinder.findMainClass(rootDirectory); + Assert.assertEquals(1, mainClasses.size()); + Assert.assertTrue(mainClasses.contains("multi.layered.HelloWorld")); } @Test - public void testFindMainClass_noClass() - throws URISyntaxException, MultipleClassesFoundException, IOException { + public void testFindMainClass_noClass() throws URISyntaxException, IOException { Path rootDirectory = Paths.get(Resources.getResource("class-finder-tests/no-main").toURI()); - String mainClass = MainClassFinder.findMainClass(rootDirectory.toString()); - Assert.assertEquals(null, mainClass); + List mainClasses = MainClassFinder.findMainClass(rootDirectory); + Assert.assertTrue(mainClasses.isEmpty()); } @Test public void testFindMainClass_multiple() throws URISyntaxException, IOException { Path rootDirectory = Paths.get(Resources.getResource("class-finder-tests/multiple").toURI()); - try { - MainClassFinder.findMainClass(rootDirectory.toString()); - Assert.fail(); - } catch (MultipleClassesFoundException ex) { - Assert.assertTrue( - ex.getMessage().contains("Multiple classes found while trying to infer main class: ")); - Assert.assertTrue(ex.getMessage().contains("multi.layered.HelloMoon")); - Assert.assertTrue(ex.getMessage().contains("HelloWorld")); - } + List mainClasses = MainClassFinder.findMainClass(rootDirectory); + Assert.assertEquals(2, mainClasses.size()); + Assert.assertTrue(mainClasses.contains("multi.layered.HelloMoon")); + Assert.assertTrue(mainClasses.contains("HelloWorld")); } } diff --git a/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ProjectProperties.java b/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ProjectProperties.java index c056db364d..670629662c 100644 --- a/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ProjectProperties.java +++ b/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ProjectProperties.java @@ -19,14 +19,17 @@ import com.google.cloud.tools.jib.builder.BuildConfiguration; import com.google.cloud.tools.jib.builder.SourceFilesConfiguration; import com.google.cloud.tools.jib.frontend.MainClassFinder; -import com.google.cloud.tools.jib.frontend.MultipleClassesFoundException; import java.io.IOException; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.ArrayList; import java.util.List; import javax.annotation.Nullable; import org.gradle.api.GradleException; import org.gradle.api.Project; import org.gradle.api.Task; +import org.gradle.api.plugins.JavaPluginConvention; +import org.gradle.api.tasks.SourceSet; import org.gradle.jvm.tasks.Jar; /** Obtains information about a Gradle {@link Project} that uses Jib. */ @@ -48,29 +51,36 @@ String getMainClass(@Nullable String mainClass) { if (mainClass == null) { mainClass = getMainClassFromJarTask(); if (mainClass == null) { - gradleBuildLogger.info( + gradleBuildLogger.debug( "Could not find main class specified in a 'jar' task; attempting to " + "infer main class."); + + final String mainClassSuggestion = "add a `mainClass` configuration to jib"; try { - mainClass = - MainClassFinder.findMainClass( - project - .getBuildDir() - .toPath() - .resolve("classes") - .resolve("java") - .resolve("main") - .toAbsolutePath() - .toString()); - } catch (MultipleClassesFoundException | IOException ex) { - throw new GradleException( - HelpfulSuggestionsProvider.get("Failed to get main class: " + ex.getMessage()) - .suggest("add a `mainClass` configuration to jib")); - } - if (mainClass == null) { + // Adds each file in each classes output directory to the classes files list. + JavaPluginConvention javaPluginConvention = + project.getConvention().getPlugin(JavaPluginConvention.class); + SourceSet mainSourceSet = javaPluginConvention.getSourceSets().getByName("main"); + Path classesDirs = Paths.get(mainSourceSet.getOutput().getClassesDirs().getAsPath()); + List mainClasses = new ArrayList<>(MainClassFinder.findMainClass(classesDirs)); + + if (mainClasses.size() == 1) { + mainClass = mainClasses.get(0); + } else if (mainClasses.size() == 0) { + throw new GradleException( + HelpfulSuggestionsProvider.get("Main class was not found") + .suggest(mainClassSuggestion)); + } else { + throw new GradleException( + HelpfulSuggestionsProvider.get( + "Multiple valid main classes were found: " + String.join(", ", mainClasses)) + .suggest(mainClassSuggestion)); + } + } catch (IOException ex) { throw new GradleException( - HelpfulSuggestionsProvider.get("Could not infer main class") - .suggest("add a `mainClass` configuration to jib")); + HelpfulSuggestionsProvider.get("Failed to get main class") + .suggest(mainClassSuggestion), + ex); } } } diff --git a/jib-gradle-plugin/src/test/java/com/google/cloud/tools/jib/gradle/ProjectPropertiesTest.java b/jib-gradle-plugin/src/test/java/com/google/cloud/tools/jib/gradle/ProjectPropertiesTest.java index 16fcca2338..9f97633c49 100644 --- a/jib-gradle-plugin/src/test/java/com/google/cloud/tools/jib/gradle/ProjectPropertiesTest.java +++ b/jib-gradle-plugin/src/test/java/com/google/cloud/tools/jib/gradle/ProjectPropertiesTest.java @@ -16,23 +16,25 @@ package com.google.cloud.tools.jib.gradle; -import java.io.File; -import java.io.IOException; import java.util.Collections; import org.gradle.api.GradleException; import org.gradle.api.Project; +import org.gradle.api.file.FileCollection; import org.gradle.api.internal.file.FileResolver; import org.gradle.api.java.archives.Manifest; import org.gradle.api.java.archives.internal.DefaultManifest; +import org.gradle.api.plugins.Convention; +import org.gradle.api.plugins.JavaPluginConvention; +import org.gradle.api.tasks.SourceSet; +import org.gradle.api.tasks.SourceSetContainer; +import org.gradle.api.tasks.SourceSetOutput; import org.gradle.internal.impldep.com.google.common.collect.ImmutableMap; import org.gradle.internal.impldep.com.google.common.collect.ImmutableSet; import org.gradle.jvm.tasks.Jar; import org.hamcrest.CoreMatchers; import org.junit.Assert; import org.junit.Before; -import org.junit.Rule; import org.junit.Test; -import org.junit.rules.TemporaryFolder; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.Mockito; @@ -42,23 +44,34 @@ @RunWith(MockitoJUnitRunner.class) public class ProjectPropertiesTest { - @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); - @Mock private FileResolver mockFileResolver; @Mock private Jar mockJar; @Mock private Project mockProject; @Mock private GradleBuildLogger mockGradleBuildLogger; + @Mock private Convention mockConvention; + @Mock private JavaPluginConvention mockPluginConvention; + @Mock private SourceSetContainer mockSourceSetContainer; + @Mock private SourceSet mockSourceSet; + @Mock private SourceSetOutput mockSourceSetOutput; + @Mock private FileCollection mockFileCollection; + private Manifest fakeManifest; private ProjectProperties testProjectProperties; @Before - public void setUp() throws IOException { + public void setUp() { fakeManifest = new DefaultManifest(mockFileResolver); Mockito.when(mockJar.getManifest()).thenReturn(fakeManifest); - File tempFolder = temporaryFolder.newFolder(); - Mockito.when(mockProject.getBuildDir()).thenReturn(tempFolder); + Mockito.when(mockProject.getConvention()).thenReturn(mockConvention); + Mockito.when(mockConvention.getPlugin(JavaPluginConvention.class)) + .thenReturn(mockPluginConvention); + Mockito.when(mockPluginConvention.getSourceSets()).thenReturn(mockSourceSetContainer); + Mockito.when(mockSourceSetContainer.getByName("main")).thenReturn(mockSourceSet); + Mockito.when(mockSourceSet.getOutput()).thenReturn(mockSourceSetOutput); + Mockito.when(mockSourceSetOutput.getClassesDirs()).thenReturn(mockFileCollection); + Mockito.when(mockFileCollection.getAsPath()).thenReturn("a/b/c"); testProjectProperties = new ProjectProperties(mockProject, mockGradleBuildLogger); } @@ -105,9 +118,8 @@ private void assertGetMainClassFails() { } catch (GradleException ex) { Mockito.verify(mockGradleBuildLogger) - .info( + .debug( "Could not find main class specified in a 'jar' task; attempting to infer main class."); - Assert.assertThat(ex.getMessage(), CoreMatchers.containsString("Could not infer main class")); Assert.assertThat( ex.getMessage(), CoreMatchers.containsString("add a `mainClass` configuration to jib")); } diff --git a/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/ProjectProperties.java b/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/ProjectProperties.java index cae50f5488..f0a1f282ad 100644 --- a/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/ProjectProperties.java +++ b/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/ProjectProperties.java @@ -19,9 +19,10 @@ import com.google.cloud.tools.jib.builder.BuildConfiguration; import com.google.cloud.tools.jib.builder.SourceFilesConfiguration; import com.google.cloud.tools.jib.frontend.MainClassFinder; -import com.google.cloud.tools.jib.frontend.MultipleClassesFoundException; import com.google.common.base.Preconditions; import java.io.IOException; +import java.nio.file.Paths; +import java.util.List; import javax.annotation.Nullable; import org.apache.maven.model.Plugin; import org.apache.maven.plugin.MojoExecutionException; @@ -59,20 +60,31 @@ String getMainClass(@Nullable String mainClass) throws MojoExecutionException { if (mainClass == null) { mainClass = getMainClassFromMavenJarPlugin(); if (mainClass == null) { - log.info( + log.debug( "Could not find main class specified in maven-jar-plugin; attempting to infer main " + "class."); + + final String mainClassSuggestion = "add a `mainClass` configuration to jib-maven-plugin"; try { - mainClass = MainClassFinder.findMainClass(project.getBuild().getOutputDirectory()); - } catch (MultipleClassesFoundException | IOException ex) { - throw new MojoExecutionException( - HelpfulSuggestionsProvider.get("Failed to get main class: " + ex.getMessage()) - .suggest("add a `mainClass` configuration to jib-maven-plugin")); - } - if (mainClass == null) { + List mainClasses = + MainClassFinder.findMainClass(Paths.get(project.getBuild().getOutputDirectory())); + if (mainClasses.size() == 1) { + mainClass = mainClasses.get(0); + } else if (mainClasses.size() == 0) { + throw new MojoExecutionException( + HelpfulSuggestionsProvider.get("Main class was not found") + .suggest(mainClassSuggestion)); + } else { + throw new MojoExecutionException( + HelpfulSuggestionsProvider.get( + "Multiple valid main classes were found: " + String.join(", ", mainClasses)) + .suggest(mainClassSuggestion)); + } + } catch (IOException ex) { throw new MojoExecutionException( - HelpfulSuggestionsProvider.get("Could not infer main class") - .suggest("add a `mainClass` configuration to jib-maven-plugin")); + HelpfulSuggestionsProvider.get("Failed to get main class") + .suggest(mainClassSuggestion), + ex); } } } diff --git a/jib-maven-plugin/src/test/java/com/google/cloud/tools/jib/maven/ProjectPropertiesTest.java b/jib-maven-plugin/src/test/java/com/google/cloud/tools/jib/maven/ProjectPropertiesTest.java index cc4cd933b1..a607a465aa 100644 --- a/jib-maven-plugin/src/test/java/com/google/cloud/tools/jib/maven/ProjectPropertiesTest.java +++ b/jib-maven-plugin/src/test/java/com/google/cloud/tools/jib/maven/ProjectPropertiesTest.java @@ -104,9 +104,8 @@ private void assertGetMainClassFails() { } catch (MojoExecutionException ex) { Mockito.verify(mockLog) - .info( + .debug( "Could not find main class specified in maven-jar-plugin; attempting to infer main class."); - Assert.assertThat(ex.getMessage(), CoreMatchers.containsString("Could not infer main class")); Assert.assertThat( ex.getMessage(), CoreMatchers.containsString("add a `mainClass` configuration to jib-maven-plugin")); From bca760bb1a73b07add4cdacd11aaac85517b1041 Mon Sep 17 00:00:00 2001 From: Tad Cordle Date: Thu, 17 May 2018 12:39:23 -0400 Subject: [PATCH 8/9] Feedback --- .../jib/frontend/HelpfulSuggestions.java | 4 ++ .../tools/jib/frontend/MainClassFinder.java | 48 ++++++--------- .../jib/frontend/HelpfulSuggestionsTest.java | 3 + .../jib/frontend/MainClassFinderTest.java | 8 +-- .../tools/jib/gradle/ProjectProperties.java | 58 ++++++++++++------- .../jib/gradle/ProjectPropertiesTest.java | 32 +++------- .../tools/jib/maven/ProjectProperties.java | 24 ++++++-- 7 files changed, 94 insertions(+), 83 deletions(-) diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/HelpfulSuggestions.java b/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/HelpfulSuggestions.java index adb82264f1..b80b4f0f3e 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/HelpfulSuggestions.java +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/HelpfulSuggestions.java @@ -100,6 +100,10 @@ public String forDockerContextInsecureRecursiveDelete(String directory) { return suggest("clear " + directory + " manually before creating the Docker context"); } + public String forMainClassNotFound(String pluginName) { + return suggest("add a `mainClass` configuration to " + pluginName); + } + public String none() { return messagePrefix; } diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MainClassFinder.java b/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MainClassFinder.java index e6f5cd9607..45f9bb2ba9 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MainClassFinder.java +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MainClassFinder.java @@ -24,7 +24,6 @@ import java.nio.file.Path; import java.util.ArrayList; import java.util.List; -import java.util.StringJoiner; import javax.annotation.Nullable; /** Infers the main class in an application. */ @@ -41,7 +40,7 @@ private ClassFileLoader(Path classFile) { @Nullable @Override - public Class findClass(String name) { + public Class findClass(@Nullable String name) { try { byte[] bytes = Files.readAllBytes(classFile); return defineClass(name, bytes, 0, bytes.length); @@ -53,17 +52,17 @@ public Class findClass(String name) { } /** - * Searches for a .class file containing a main method given an absolute root directory. + * Searches for a .class file containing a main method in a root directory. * * @return the name of the class if one is found, null if no class is found. * @throws IOException if searching/reading files fails. */ - public static List findMainClass(Path rootDirectory) throws IOException { - List results = new ArrayList<>(); + public static List findMainClasses(Path rootDirectory) throws IOException { + List classNames = new ArrayList<>(); // Make sure rootDirectory is valid if (!Files.exists(rootDirectory) || !Files.isDirectory(rootDirectory)) { - return results; + return classNames; } // Get all .class files @@ -72,34 +71,25 @@ public static List findMainClass(Path rootDirectory) throws IOException .filter(path -> path.toString().endsWith(".class")) .walk( classFile -> { - // Convert filename (rootDir/path/to/ClassName.class) to class name - // (path.to.ClassName) - Path relativized = rootDirectory.relativize(classFile); - StringJoiner stringJoiner = new StringJoiner("."); - for (Path path : relativized) { - stringJoiner.add(path.toString()); + Class fileClass = new ClassFileLoader(classFile).findClass(null); + if (fileClass == null) { + return; } - String name = stringJoiner.toString(); - name = name.substring(0, name.length() - ".class".length()); - - Class fileClass = new ClassFileLoader(classFile).findClass(name); - if (fileClass != null) { - try { - // Check if class contains {@code public static void main(String[] args)} - Method main = fileClass.getMethod("main", String[].class); - if (main != null - && main.getReturnType() == void.class - && Modifier.isStatic(main.getModifiers()) - && Modifier.isPublic(main.getModifiers())) { - results.add(name); - } - } catch (NoSuchMethodException ignored) { - // main method not found + try { + // Check if class contains {@code public static void main(String[] args)} + Method main = fileClass.getMethod("main", String[].class); + if (main != null + && main.getReturnType() == void.class + && Modifier.isStatic(main.getModifiers()) + && Modifier.isPublic(main.getModifiers())) { + classNames.add(fileClass.getCanonicalName()); } + } catch (NoSuchMethodException ignored) { + // main method not found } }); - return results; + return classNames; } private MainClassFinder() {} diff --git a/jib-core/src/test/java/com/google/cloud/tools/jib/frontend/HelpfulSuggestionsTest.java b/jib-core/src/test/java/com/google/cloud/tools/jib/frontend/HelpfulSuggestionsTest.java index 8d156053bb..efcf1addc2 100644 --- a/jib-core/src/test/java/com/google/cloud/tools/jib/frontend/HelpfulSuggestionsTest.java +++ b/jib-core/src/test/java/com/google/cloud/tools/jib/frontend/HelpfulSuggestionsTest.java @@ -61,6 +61,9 @@ public void testSuggestions_smoke() { Assert.assertEquals( "messagePrefix, perhaps you should clear directory manually before creating the Docker context", TEST_HELPFUL_SUGGESTIONS.forDockerContextInsecureRecursiveDelete("directory")); + Assert.assertEquals( + "messagePrefix, perhaps you should add a `mainClass` configuration to plugin", + TEST_HELPFUL_SUGGESTIONS.forMainClassNotFound("plugin")); Assert.assertEquals("messagePrefix", TEST_HELPFUL_SUGGESTIONS.none()); } } diff --git a/jib-core/src/test/java/com/google/cloud/tools/jib/frontend/MainClassFinderTest.java b/jib-core/src/test/java/com/google/cloud/tools/jib/frontend/MainClassFinderTest.java index 0358823c1b..f09a8b0020 100644 --- a/jib-core/src/test/java/com/google/cloud/tools/jib/frontend/MainClassFinderTest.java +++ b/jib-core/src/test/java/com/google/cloud/tools/jib/frontend/MainClassFinderTest.java @@ -31,7 +31,7 @@ public class MainClassFinderTest { @Test public void testFindMainClass_simple() throws URISyntaxException, IOException { Path rootDirectory = Paths.get(Resources.getResource("class-finder-tests/simple").toURI()); - List mainClasses = MainClassFinder.findMainClass(rootDirectory); + List mainClasses = MainClassFinder.findMainClasses(rootDirectory); Assert.assertEquals(1, mainClasses.size()); Assert.assertTrue(mainClasses.contains("HelloWorld")); } @@ -40,7 +40,7 @@ public void testFindMainClass_simple() throws URISyntaxException, IOException { public void testFindMainClass_subdirectories() throws URISyntaxException, IOException { Path rootDirectory = Paths.get(Resources.getResource("class-finder-tests/subdirectories").toURI()); - List mainClasses = MainClassFinder.findMainClass(rootDirectory); + List mainClasses = MainClassFinder.findMainClasses(rootDirectory); Assert.assertEquals(1, mainClasses.size()); Assert.assertTrue(mainClasses.contains("multi.layered.HelloWorld")); } @@ -48,14 +48,14 @@ public void testFindMainClass_subdirectories() throws URISyntaxException, IOExce @Test public void testFindMainClass_noClass() throws URISyntaxException, IOException { Path rootDirectory = Paths.get(Resources.getResource("class-finder-tests/no-main").toURI()); - List mainClasses = MainClassFinder.findMainClass(rootDirectory); + List mainClasses = MainClassFinder.findMainClasses(rootDirectory); Assert.assertTrue(mainClasses.isEmpty()); } @Test public void testFindMainClass_multiple() throws URISyntaxException, IOException { Path rootDirectory = Paths.get(Resources.getResource("class-finder-tests/multiple").toURI()); - List mainClasses = MainClassFinder.findMainClass(rootDirectory); + List mainClasses = MainClassFinder.findMainClasses(rootDirectory); Assert.assertEquals(2, mainClasses.size()); Assert.assertTrue(mainClasses.contains("multi.layered.HelloMoon")); Assert.assertTrue(mainClasses.contains("HelloWorld")); diff --git a/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ProjectProperties.java b/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ProjectProperties.java index 670629662c..3c18eae5c1 100644 --- a/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ProjectProperties.java +++ b/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ProjectProperties.java @@ -19,33 +19,57 @@ import com.google.cloud.tools.jib.builder.BuildConfiguration; import com.google.cloud.tools.jib.builder.SourceFilesConfiguration; import com.google.cloud.tools.jib.frontend.MainClassFinder; +import com.google.common.annotations.VisibleForTesting; import java.io.IOException; import java.nio.file.Path; -import java.nio.file.Paths; import java.util.ArrayList; import java.util.List; import javax.annotation.Nullable; import org.gradle.api.GradleException; import org.gradle.api.Project; import org.gradle.api.Task; -import org.gradle.api.plugins.JavaPluginConvention; -import org.gradle.api.tasks.SourceSet; import org.gradle.jvm.tasks.Jar; /** Obtains information about a Gradle {@link Project} that uses Jib. */ class ProjectProperties { + private static final String PLUGIN_NAME = "jib"; + private final Project project; private final GradleBuildLogger gradleBuildLogger; + private final SourceFilesConfiguration sourceFilesConfiguration; ProjectProperties(Project project, GradleBuildLogger gradleBuildLogger) { this.project = project; this.gradleBuildLogger = gradleBuildLogger; + try { + sourceFilesConfiguration = GradleSourceFilesConfiguration.getForProject(project); + } catch (IOException ex) { + throw new GradleException("Obtaining project build output files failed", ex); + } + } + + @VisibleForTesting + ProjectProperties( + Project project, + GradleBuildLogger gradleBuildLogger, + SourceFilesConfiguration sourceFilesConfiguration) { + this.project = project; + this.gradleBuildLogger = gradleBuildLogger; + this.sourceFilesConfiguration = sourceFilesConfiguration; } /** - * @param mainClass the configured main class - * @return the main class to use for the container entrypoint. + * If {@code mainClass} is {@code null}, tries to infer main class in this order: + * + *
    + *
  • 1. Looks in a {@code jar} task. + *
  • 2. Searches for a class defined with a main method. + *
+ * + *

Warns if main class is not valid. + * + * @throws GradleException if no valid main class is not found. */ String getMainClass(@Nullable String mainClass) { if (mainClass == null) { @@ -54,32 +78,29 @@ String getMainClass(@Nullable String mainClass) { gradleBuildLogger.debug( "Could not find main class specified in a 'jar' task; attempting to " + "infer main class."); - - final String mainClassSuggestion = "add a `mainClass` configuration to jib"; try { // Adds each file in each classes output directory to the classes files list. - JavaPluginConvention javaPluginConvention = - project.getConvention().getPlugin(JavaPluginConvention.class); - SourceSet mainSourceSet = javaPluginConvention.getSourceSets().getByName("main"); - Path classesDirs = Paths.get(mainSourceSet.getOutput().getClassesDirs().getAsPath()); - List mainClasses = new ArrayList<>(MainClassFinder.findMainClass(classesDirs)); + List mainClasses = new ArrayList<>(); + for (Path classPath : sourceFilesConfiguration.getClassesFiles()) { + mainClasses.addAll(MainClassFinder.findMainClasses(classPath)); + } if (mainClasses.size() == 1) { mainClass = mainClasses.get(0); } else if (mainClasses.size() == 0) { throw new GradleException( HelpfulSuggestionsProvider.get("Main class was not found") - .suggest(mainClassSuggestion)); + .forMainClassNotFound(PLUGIN_NAME)); } else { throw new GradleException( HelpfulSuggestionsProvider.get( "Multiple valid main classes were found: " + String.join(", ", mainClasses)) - .suggest(mainClassSuggestion)); + .forMainClassNotFound(PLUGIN_NAME)); } } catch (IOException ex) { throw new GradleException( HelpfulSuggestionsProvider.get("Failed to get main class") - .suggest(mainClassSuggestion), + .forMainClassNotFound(PLUGIN_NAME), ex); } } @@ -92,12 +113,7 @@ String getMainClass(@Nullable String mainClass) { /** @return the {@link SourceFilesConfiguration} based on the current project */ SourceFilesConfiguration getSourceFilesConfiguration() { - try { - return GradleSourceFilesConfiguration.getForProject(project); - - } catch (IOException ex) { - throw new GradleException("Obtaining project build output files failed", ex); - } + return sourceFilesConfiguration; } /** Extracts main class from 'jar' task, if available. */ diff --git a/jib-gradle-plugin/src/test/java/com/google/cloud/tools/jib/gradle/ProjectPropertiesTest.java b/jib-gradle-plugin/src/test/java/com/google/cloud/tools/jib/gradle/ProjectPropertiesTest.java index 9f97633c49..737c49975b 100644 --- a/jib-gradle-plugin/src/test/java/com/google/cloud/tools/jib/gradle/ProjectPropertiesTest.java +++ b/jib-gradle-plugin/src/test/java/com/google/cloud/tools/jib/gradle/ProjectPropertiesTest.java @@ -16,18 +16,16 @@ package com.google.cloud.tools.jib.gradle; +import com.google.cloud.tools.jib.builder.SourceFilesConfiguration; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.Collections; +import java.util.List; import org.gradle.api.GradleException; import org.gradle.api.Project; -import org.gradle.api.file.FileCollection; import org.gradle.api.internal.file.FileResolver; import org.gradle.api.java.archives.Manifest; import org.gradle.api.java.archives.internal.DefaultManifest; -import org.gradle.api.plugins.Convention; -import org.gradle.api.plugins.JavaPluginConvention; -import org.gradle.api.tasks.SourceSet; -import org.gradle.api.tasks.SourceSetContainer; -import org.gradle.api.tasks.SourceSetOutput; import org.gradle.internal.impldep.com.google.common.collect.ImmutableMap; import org.gradle.internal.impldep.com.google.common.collect.ImmutableSet; import org.gradle.jvm.tasks.Jar; @@ -48,32 +46,20 @@ public class ProjectPropertiesTest { @Mock private Jar mockJar; @Mock private Project mockProject; @Mock private GradleBuildLogger mockGradleBuildLogger; - - @Mock private Convention mockConvention; - @Mock private JavaPluginConvention mockPluginConvention; - @Mock private SourceSetContainer mockSourceSetContainer; - @Mock private SourceSet mockSourceSet; - @Mock private SourceSetOutput mockSourceSetOutput; - @Mock private FileCollection mockFileCollection; + @Mock private SourceFilesConfiguration mockSourceFilesConfiguration; private Manifest fakeManifest; private ProjectProperties testProjectProperties; + private final List classesPath = Collections.singletonList(Paths.get("a/b/c")); @Before public void setUp() { fakeManifest = new DefaultManifest(mockFileResolver); Mockito.when(mockJar.getManifest()).thenReturn(fakeManifest); + Mockito.when(mockSourceFilesConfiguration.getClassesFiles()).thenReturn(classesPath); - Mockito.when(mockProject.getConvention()).thenReturn(mockConvention); - Mockito.when(mockConvention.getPlugin(JavaPluginConvention.class)) - .thenReturn(mockPluginConvention); - Mockito.when(mockPluginConvention.getSourceSets()).thenReturn(mockSourceSetContainer); - Mockito.when(mockSourceSetContainer.getByName("main")).thenReturn(mockSourceSet); - Mockito.when(mockSourceSet.getOutput()).thenReturn(mockSourceSetOutput); - Mockito.when(mockSourceSetOutput.getClassesDirs()).thenReturn(mockFileCollection); - Mockito.when(mockFileCollection.getAsPath()).thenReturn("a/b/c"); - - testProjectProperties = new ProjectProperties(mockProject, mockGradleBuildLogger); + testProjectProperties = + new ProjectProperties(mockProject, mockGradleBuildLogger, mockSourceFilesConfiguration); } @Test diff --git a/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/ProjectProperties.java b/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/ProjectProperties.java index f0a1f282ad..34948b507d 100644 --- a/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/ProjectProperties.java +++ b/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/ProjectProperties.java @@ -33,6 +33,8 @@ /** Obtains information about a {@link MavenProject}. */ class ProjectProperties { + private static final String PLUGIN_NAME = "jib-maven-plugin"; + private final MavenProject project; private final Log log; @@ -55,7 +57,18 @@ SourceFilesConfiguration getSourceFilesConfiguration() throws MojoExecutionExcep } } - /** @return the main class to use in the container entrypoint */ + /** + * If {@code mainClass} is {@code null}, tries to infer main class in this order: + * + *

    + *
  • 1. Looks in a {@code jar} task. + *
  • 2. Searches for a class defined with a main method. + *
+ * + *

Warns if main class is not valid. + * + * @throws MojoExecutionException if no valid main class is not found. + */ String getMainClass(@Nullable String mainClass) throws MojoExecutionException { if (mainClass == null) { mainClass = getMainClassFromMavenJarPlugin(); @@ -64,26 +77,25 @@ String getMainClass(@Nullable String mainClass) throws MojoExecutionException { "Could not find main class specified in maven-jar-plugin; attempting to infer main " + "class."); - final String mainClassSuggestion = "add a `mainClass` configuration to jib-maven-plugin"; try { List mainClasses = - MainClassFinder.findMainClass(Paths.get(project.getBuild().getOutputDirectory())); + MainClassFinder.findMainClasses(Paths.get(project.getBuild().getOutputDirectory())); if (mainClasses.size() == 1) { mainClass = mainClasses.get(0); } else if (mainClasses.size() == 0) { throw new MojoExecutionException( HelpfulSuggestionsProvider.get("Main class was not found") - .suggest(mainClassSuggestion)); + .forMainClassNotFound(PLUGIN_NAME)); } else { throw new MojoExecutionException( HelpfulSuggestionsProvider.get( "Multiple valid main classes were found: " + String.join(", ", mainClasses)) - .suggest(mainClassSuggestion)); + .forMainClassNotFound(PLUGIN_NAME)); } } catch (IOException ex) { throw new MojoExecutionException( HelpfulSuggestionsProvider.get("Failed to get main class") - .suggest(mainClassSuggestion), + .forMainClassNotFound(PLUGIN_NAME), ex); } } From 9d1c3f6db7bdd12b003c24d0eb40a383da14926b Mon Sep 17 00:00:00 2001 From: Tad Cordle Date: Thu, 17 May 2018 14:09:14 -0400 Subject: [PATCH 9/9] Feedback --- .../tools/jib/frontend/MainClassFinder.java | 2 +- .../tools/jib/gradle/BuildDockerTask.java | 3 +- .../tools/jib/gradle/BuildImageTask.java | 3 +- .../tools/jib/gradle/DockerContextTask.java | 3 +- .../tools/jib/gradle/ProjectProperties.java | 12 ++++-- .../jib/gradle/ProjectPropertiesTest.java | 2 +- .../tools/jib/maven/BuildDockerMojo.java | 2 +- .../cloud/tools/jib/maven/BuildImageMojo.java | 2 +- .../tools/jib/maven/DockerContextMojo.java | 2 +- .../tools/jib/maven/ProjectProperties.java | 43 +++++++++++++------ .../jib/maven/ProjectPropertiesTest.java | 14 ++++-- 11 files changed, 60 insertions(+), 28 deletions(-) diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MainClassFinder.java b/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MainClassFinder.java index 45f9bb2ba9..3cff4957b3 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MainClassFinder.java +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/frontend/MainClassFinder.java @@ -82,7 +82,7 @@ public static List findMainClasses(Path rootDirectory) throws IOExceptio && main.getReturnType() == void.class && Modifier.isStatic(main.getModifiers()) && Modifier.isPublic(main.getModifiers())) { - classNames.add(fileClass.getCanonicalName()); + classNames.add(fileClass.getName()); } } catch (NoSuchMethodException ignored) { // main method not found diff --git a/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/BuildDockerTask.java b/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/BuildDockerTask.java index c2e2bdf641..38fbcb9c5d 100644 --- a/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/BuildDockerTask.java +++ b/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/BuildDockerTask.java @@ -89,7 +89,8 @@ public void buildDocker() throws InvalidImageReferenceException { + "' does not use a specific image digest - build may not be reproducible"); } - ProjectProperties projectProperties = new ProjectProperties(getProject(), gradleBuildLogger); + ProjectProperties projectProperties = + ProjectProperties.getForProject(getProject(), gradleBuildLogger); String mainClass = projectProperties.getMainClass(jibExtension.getMainClass()); RegistryCredentials knownBaseRegistryCredentials = null; diff --git a/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/BuildImageTask.java b/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/BuildImageTask.java index abdcc0f905..b694ea1f5a 100644 --- a/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/BuildImageTask.java +++ b/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/BuildImageTask.java @@ -95,7 +95,8 @@ public void buildImage() throws InvalidImageReferenceException { + "' does not use a specific image digest - build may not be reproducible"); } - ProjectProperties projectProperties = new ProjectProperties(getProject(), gradleBuildLogger); + ProjectProperties projectProperties = + ProjectProperties.getForProject(getProject(), gradleBuildLogger); String mainClass = projectProperties.getMainClass(jibExtension.getMainClass()); RegistryCredentials knownBaseRegistryCredentials = null; diff --git a/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/DockerContextTask.java b/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/DockerContextTask.java index ce295c055f..8a09731492 100644 --- a/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/DockerContextTask.java +++ b/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/DockerContextTask.java @@ -65,7 +65,8 @@ public void generateDockerContext() { GradleBuildLogger gradleBuildLogger = new GradleBuildLogger(getLogger()); - ProjectProperties projectProperties = new ProjectProperties(getProject(), gradleBuildLogger); + ProjectProperties projectProperties = + ProjectProperties.getForProject(getProject(), gradleBuildLogger); String mainClass = projectProperties.getMainClass(jibExtension.getMainClass()); String targetDir = getTargetDir(); diff --git a/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ProjectProperties.java b/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ProjectProperties.java index 3c18eae5c1..5b0233ae71 100644 --- a/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ProjectProperties.java +++ b/jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ProjectProperties.java @@ -39,11 +39,11 @@ class ProjectProperties { private final GradleBuildLogger gradleBuildLogger; private final SourceFilesConfiguration sourceFilesConfiguration; - ProjectProperties(Project project, GradleBuildLogger gradleBuildLogger) { - this.project = project; - this.gradleBuildLogger = gradleBuildLogger; + /** @return a ProjectProperties from the given project and logger. */ + static ProjectProperties getForProject(Project project, GradleBuildLogger gradleBuildLogger) { try { - sourceFilesConfiguration = GradleSourceFilesConfiguration.getForProject(project); + return new ProjectProperties( + project, gradleBuildLogger, GradleSourceFilesConfiguration.getForProject(project)); } catch (IOException ex) { throw new GradleException("Obtaining project build output files failed", ex); } @@ -73,6 +73,10 @@ class ProjectProperties { */ String getMainClass(@Nullable String mainClass) { if (mainClass == null) { + gradleBuildLogger.info( + "Searching for main class... Add a 'mainClass' configuration to '" + + PLUGIN_NAME + + "' to improve build speed."); mainClass = getMainClassFromJarTask(); if (mainClass == null) { gradleBuildLogger.debug( diff --git a/jib-gradle-plugin/src/test/java/com/google/cloud/tools/jib/gradle/ProjectPropertiesTest.java b/jib-gradle-plugin/src/test/java/com/google/cloud/tools/jib/gradle/ProjectPropertiesTest.java index 737c49975b..2685238919 100644 --- a/jib-gradle-plugin/src/test/java/com/google/cloud/tools/jib/gradle/ProjectPropertiesTest.java +++ b/jib-gradle-plugin/src/test/java/com/google/cloud/tools/jib/gradle/ProjectPropertiesTest.java @@ -48,9 +48,9 @@ public class ProjectPropertiesTest { @Mock private GradleBuildLogger mockGradleBuildLogger; @Mock private SourceFilesConfiguration mockSourceFilesConfiguration; + private final List classesPath = Collections.singletonList(Paths.get("a/b/c")); private Manifest fakeManifest; private ProjectProperties testProjectProperties; - private final List classesPath = Collections.singletonList(Paths.get("a/b/c")); @Before public void setUp() { diff --git a/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/BuildDockerMojo.java b/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/BuildDockerMojo.java index 2762cd63a2..fb97638c23 100644 --- a/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/BuildDockerMojo.java +++ b/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/BuildDockerMojo.java @@ -57,7 +57,7 @@ public class BuildDockerMojo extends JibPluginConfiguration { /** TODO: Consolidate with BuildImageMojo. */ @Override public void execute() throws MojoExecutionException, MojoFailureException { - ProjectProperties projectProperties = new ProjectProperties(getProject(), getLog()); + ProjectProperties projectProperties = ProjectProperties.getForProject(getProject(), getLog()); String inferredMainClass = projectProperties.getMainClass(getMainClass()); SourceFilesConfiguration sourceFilesConfiguration = diff --git a/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/BuildImageMojo.java b/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/BuildImageMojo.java index c4ca4a5914..e951b617fe 100644 --- a/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/BuildImageMojo.java +++ b/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/BuildImageMojo.java @@ -77,7 +77,7 @@ private Class getManifestTemplateClass() { public void execute() throws MojoExecutionException, MojoFailureException { validateParameters(); - ProjectProperties projectProperties = new ProjectProperties(getProject(), getLog()); + ProjectProperties projectProperties = ProjectProperties.getForProject(getProject(), getLog()); String inferredMainClass = projectProperties.getMainClass(getMainClass()); SourceFilesConfiguration sourceFilesConfiguration = diff --git a/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/DockerContextMojo.java b/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/DockerContextMojo.java index a132ea0198..c7a1727e8b 100644 --- a/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/DockerContextMojo.java +++ b/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/DockerContextMojo.java @@ -43,7 +43,7 @@ public class DockerContextMojo extends JibPluginConfiguration { public void execute() throws MojoExecutionException { Preconditions.checkNotNull(targetDir); - ProjectProperties projectProperties = new ProjectProperties(getProject(), getLog()); + ProjectProperties projectProperties = ProjectProperties.getForProject(getProject(), getLog()); String inferredMainClass = projectProperties.getMainClass(getMainClass()); try { diff --git a/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/ProjectProperties.java b/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/ProjectProperties.java index 34948b507d..69b920670a 100644 --- a/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/ProjectProperties.java +++ b/jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/ProjectProperties.java @@ -19,9 +19,11 @@ import com.google.cloud.tools.jib.builder.BuildConfiguration; import com.google.cloud.tools.jib.builder.SourceFilesConfiguration; import com.google.cloud.tools.jib.frontend.MainClassFinder; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import java.io.IOException; -import java.nio.file.Paths; +import java.nio.file.Path; +import java.util.ArrayList; import java.util.List; import javax.annotation.Nullable; import org.apache.maven.model.Plugin; @@ -37,17 +39,14 @@ class ProjectProperties { private final MavenProject project; private final Log log; + private final SourceFilesConfiguration sourceFilesConfiguration; - ProjectProperties(MavenProject project, Log log) { - this.project = project; - this.log = log; - } - - /** @return the {@link SourceFilesConfiguration} based on the current project */ - SourceFilesConfiguration getSourceFilesConfiguration() throws MojoExecutionException { + /** @return a ProjectProperties from the given project and logger. */ + static ProjectProperties getForProject(MavenProject project, Log log) + throws MojoExecutionException { try { - return MavenSourceFilesConfiguration.getForProject(project); - + return new ProjectProperties( + project, log, MavenSourceFilesConfiguration.getForProject(project)); } catch (IOException ex) { throw new MojoExecutionException( "Obtaining project build output files failed; make sure you have compiled your project " @@ -57,6 +56,19 @@ SourceFilesConfiguration getSourceFilesConfiguration() throws MojoExecutionExcep } } + @VisibleForTesting + ProjectProperties( + MavenProject project, Log log, SourceFilesConfiguration sourceFilesConfiguration) { + this.project = project; + this.log = log; + this.sourceFilesConfiguration = sourceFilesConfiguration; + } + + /** @return the {@link SourceFilesConfiguration} based on the current project */ + SourceFilesConfiguration getSourceFilesConfiguration() { + return sourceFilesConfiguration; + } + /** * If {@code mainClass} is {@code null}, tries to infer main class in this order: * @@ -71,6 +83,10 @@ SourceFilesConfiguration getSourceFilesConfiguration() throws MojoExecutionExcep */ String getMainClass(@Nullable String mainClass) throws MojoExecutionException { if (mainClass == null) { + log.info( + "Searching for main class... Add a 'mainClass' configuration to '" + + PLUGIN_NAME + + "' to improve build speed."); mainClass = getMainClassFromMavenJarPlugin(); if (mainClass == null) { log.debug( @@ -78,8 +94,11 @@ String getMainClass(@Nullable String mainClass) throws MojoExecutionException { + "class."); try { - List mainClasses = - MainClassFinder.findMainClasses(Paths.get(project.getBuild().getOutputDirectory())); + List mainClasses = new ArrayList<>(); + for (Path classPath : sourceFilesConfiguration.getClassesFiles()) { + mainClasses.addAll(MainClassFinder.findMainClasses(classPath)); + } + if (mainClasses.size() == 1) { mainClass = mainClasses.get(0); } else if (mainClasses.size() == 0) { diff --git a/jib-maven-plugin/src/test/java/com/google/cloud/tools/jib/maven/ProjectPropertiesTest.java b/jib-maven-plugin/src/test/java/com/google/cloud/tools/jib/maven/ProjectPropertiesTest.java index a607a465aa..94ae59cd4c 100644 --- a/jib-maven-plugin/src/test/java/com/google/cloud/tools/jib/maven/ProjectPropertiesTest.java +++ b/jib-maven-plugin/src/test/java/com/google/cloud/tools/jib/maven/ProjectPropertiesTest.java @@ -16,6 +16,11 @@ package com.google.cloud.tools.jib.maven; +import com.google.cloud.tools.jib.builder.SourceFilesConfiguration; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Collections; +import java.util.List; import org.apache.maven.model.Build; import org.apache.maven.model.Plugin; import org.apache.maven.plugin.MojoExecutionException; @@ -38,11 +43,13 @@ public class ProjectPropertiesTest { @Mock private MavenProject mockMavenProject; @Mock private Log mockLog; @Mock private Plugin mockJarPlugin; + @Mock private SourceFilesConfiguration mockSourceFilesConfiguration; @Mock private Build mockBuild; private final Xpp3Dom fakeJarPluginConfiguration = new Xpp3Dom(""); private final Xpp3Dom jarPluginMainClass = new Xpp3Dom("mainClass"); + private final List classesPath = Collections.singletonList(Paths.get("a/b/c")); private ProjectProperties testProjectProperties; @@ -55,11 +62,10 @@ public void setUp() { manifest.addChild(jarPluginMainClass); Mockito.when(mockJarPlugin.getConfiguration()).thenReturn(fakeJarPluginConfiguration); + Mockito.when(mockSourceFilesConfiguration.getClassesFiles()).thenReturn(classesPath); - Mockito.when(mockMavenProject.getBuild()).thenReturn(mockBuild); - Mockito.when(mockBuild.getOutputDirectory()).thenReturn("fake/dir"); - - testProjectProperties = new ProjectProperties(mockMavenProject, mockLog); + testProjectProperties = + new ProjectProperties(mockMavenProject, mockLog, mockSourceFilesConfiguration); } @Test