Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Infer main class if it isn't configured anywhere #278

Merged
merged 12 commits into from
May 17, 2018
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
/*
* 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.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.util.ArrayList;
import java.util.List;
import javax.annotation.Nullable;

/** Infers the main class in an application. */
public class MainClassFinder {

/** Helper for loading a .class file. */
private static class ClassFileLoader extends ClassLoader {

private final Path classFile;

private ClassFileLoader(Path classFile) {
this.classFile = classFile;
}

@Nullable
@Override
public Class findClass(@Nullable 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;
}
}
}

/**
* 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<String> findMainClasses(Path rootDirectory) throws IOException {
List<String> classNames = new ArrayList<>();

// Make sure rootDirectory is valid
if (!Files.exists(rootDirectory) || !Files.isDirectory(rootDirectory)) {
return classNames;
}

// Get all .class files
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/Get/Searches (or Looks at)

new DirectoryWalker(rootDirectory)
.filter(Files::isRegularFile)
.filter(path -> path.toString().endsWith(".class"))
.walk(
classFile -> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be good to put this in a separate method since it is getting to almost 6 levels of tabbing around line 91.

Class<?> fileClass = new ClassFileLoader(classFile).findClass(null);
if (fileClass == null) {
return;
}
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());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe you would want getName since the canonical name would replace a subclass like package.Hello$Main with package.Hello.Main

https://stackoverflow.com/questions/15202997/what-is-the-difference-between-canonical-name-simple-name-and-class-name-in-jav

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

K, wasn't sure which one we wanted.

}
} catch (NoSuchMethodException ignored) {
// main method not found
}
});

return classNames;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should have a private constructor


private MainClassFinder() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
* 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.io.Resources;
import java.io.IOException;
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;

/** Test for MainClassFinder. */
public class MainClassFinderTest {

@Test
public void testFindMainClass_simple() throws URISyntaxException, IOException {
Path rootDirectory = Paths.get(Resources.getResource("class-finder-tests/simple").toURI());
List<String> mainClasses = MainClassFinder.findMainClasses(rootDirectory);
Assert.assertEquals(1, mainClasses.size());
Assert.assertTrue(mainClasses.contains("HelloWorld"));
}

@Test
public void testFindMainClass_subdirectories() throws URISyntaxException, IOException {
Path rootDirectory =
Paths.get(Resources.getResource("class-finder-tests/subdirectories").toURI());
List<String> mainClasses = MainClassFinder.findMainClasses(rootDirectory);
Assert.assertEquals(1, mainClasses.size());
Assert.assertTrue(mainClasses.contains("multi.layered.HelloWorld"));
}

@Test
public void testFindMainClass_noClass() throws URISyntaxException, IOException {
Path rootDirectory = Paths.get(Resources.getResource("class-finder-tests/no-main").toURI());
List<String> 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<String> mainClasses = MainClassFinder.findMainClasses(rootDirectory);
Assert.assertEquals(2, mainClasses.size());
Assert.assertTrue(mainClasses.contains("multi.layered.HelloMoon"));
Assert.assertTrue(mainClasses.contains("HelloWorld"));
}
}
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Hi there.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ jib {
image = 'gcr.io/jib-integration-testing/emptyimage:gradle'
credHelper = 'gcr'
}
mainClass = 'com.test.Empty'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😃

// Does not have tests use user-level cache for base image layers.
useOnlyProjectCache = true
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,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.common.annotations.VisibleForTesting;
import java.io.IOException;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;
import javax.annotation.Nullable;
Expand All @@ -30,45 +33,87 @@
/** 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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

This should prob go in a static method like ProjectProperties#getForProject since constructors generally should be lightweight.

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:
*
* <ul>
* <li>1. Looks in a {@code jar} task.
* <li>2. Searches for a class defined with a main method.
* </ul>
*
* <p>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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the logic is getting a bit more complex, it'd be good to add a description to the Javadoc like:

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.

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"));
gradleBuildLogger.debug(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was talking to @patflynn and Patrick mentioned that it might be better to log something like Searching for main class... Add a 'mainClass' configuration to 'jib' to improve build speed. at the info level to help users in case they have a ton of classes to search.

"Could not find main class specified in a 'jar' task; attempting to "
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add a todo to consolidate this code with the Maven one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already added it to #262, but I can add a TODO in the code as well if you want.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay that's fine then.

+ "infer main class.");
try {
// Adds each file in each classes output directory to the classes files list.
List<String> 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")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since these messages are the same as in Maven, they can be added to HelpfulSuggestions.

.forMainClassNotFound(PLUGIN_NAME));
} else {
throw new GradleException(
HelpfulSuggestionsProvider.get(
"Multiple valid main classes were found: " + String.join(", ", mainClasses))
.forMainClassNotFound(PLUGIN_NAME));
}
} catch (IOException ex) {
throw new GradleException(
HelpfulSuggestionsProvider.get("Failed to get main class")
.forMainClassNotFound(PLUGIN_NAME),
ex);
}
}
}
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;
}

GradleBuildLogger getLogger() {
return gradleBuildLogger;
}

/** @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. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,11 @@

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.internal.file.FileResolver;
Expand All @@ -42,16 +46,20 @@ public class ProjectPropertiesTest {
@Mock private Jar mockJar;
@Mock private Project mockProject;
@Mock private GradleBuildLogger mockGradleBuildLogger;
@Mock private SourceFilesConfiguration mockSourceFilesConfiguration;

private Manifest fakeManifest;
private ProjectProperties testProjectProperties;
private final List<Path> 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);

testProjectProperties = new ProjectProperties(mockProject, mockGradleBuildLogger);
testProjectProperties =
new ProjectProperties(mockProject, mockGradleBuildLogger, mockSourceFilesConfiguration);
}

@Test
Expand Down Expand Up @@ -95,9 +103,9 @@ 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(mockGradleBuildLogger)
.debug(
"Could not find main class specified in a 'jar' task; attempting to infer main class.");
Assert.assertThat(
ex.getMessage(), CoreMatchers.containsString("add a `mainClass` configuration to jib"));
}
Expand Down
Loading