Skip to content

Commit

Permalink
Move the CLI into its own subproject (#27114)
Browse files Browse the repository at this point in the history
Projects the depend on the CLI currently depend on core. This should not
always be the case. The EnvironmentAwareCommand will remain in :core,
but the rest of the CLI components have been moved into their own
subproject of :core, :core:cli.
  • Loading branch information
hub-cap committed Nov 19, 2017
1 parent 03a501d commit f2fef68
Show file tree
Hide file tree
Showing 17 changed files with 121 additions and 29 deletions.
1 change: 1 addition & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ subprojects {
"org.elasticsearch.gradle:build-tools:${version}": ':build-tools',
"org.elasticsearch:rest-api-spec:${version}": ':rest-api-spec',
"org.elasticsearch:elasticsearch:${version}": ':core',
"org.elasticsearch:elasticsearch-cli:${version}": ':core:cli',
"org.elasticsearch.client:elasticsearch-rest-client:${version}": ':client:rest',
"org.elasticsearch.client:elasticsearch-rest-client-sniffer:${version}": ':client:sniffer',
"org.elasticsearch.client:elasticsearch-rest-high-level-client:${version}": ':client:rest-high-level',
Expand Down
8 changes: 7 additions & 1 deletion core/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ dependencies {
compile 'org.elasticsearch:securesm:1.1'

// utilities
compile 'net.sf.jopt-simple:jopt-simple:5.0.2'
compile "org.elasticsearch:elasticsearch-cli:${version}"
compile 'com.carrotsearch:hppc:0.7.1'

// time handling, remove with java 8 time
Expand Down Expand Up @@ -265,6 +265,12 @@ if (JavaVersion.current() > JavaVersion.VERSION_1_8) {
dependencyLicenses {
mapping from: /lucene-.*/, to: 'lucene'
mapping from: /jackson-.*/, to: 'jackson'
dependencies = project.configurations.runtime.fileCollection {
it.group.startsWith('org.elasticsearch') == false ||
// keep the following org.elasticsearch jars in
(it.name == 'jna' ||
it.name == 'securesm')
}
}

if (isEclipse == false || project.path == ":core-tests") {
Expand Down
36 changes: 36 additions & 0 deletions core/cli/build.gradle
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import org.elasticsearch.gradle.precommit.PrecommitTasks

apply plugin: 'elasticsearch.build'

archivesBaseName = 'elasticsearch-cli'

dependencies {
compile 'net.sf.jopt-simple:jopt-simple:5.0.2'
}

test.enabled = false
// Since CLI does not depend on :core, it cannot run the jarHell task
jarHell.enabled = false

forbiddenApisMain {
signaturesURLs = [PrecommitTasks.getResource('/forbidden/jdk-signatures.txt')]
}
File renamed without changes.
File renamed without changes.
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,6 @@
import joptsimple.OptionParser;
import joptsimple.OptionSet;
import joptsimple.OptionSpec;
import org.apache.logging.log4j.Level;
import org.apache.lucene.util.SetOnce;
import org.elasticsearch.common.SuppressForbidden;
import org.elasticsearch.common.logging.LogConfigurator;
import org.elasticsearch.common.settings.Settings;

import java.io.Closeable;
import java.io.IOException;
Expand Down Expand Up @@ -55,12 +50,13 @@ public Command(String description) {
this.description = description;
}

final SetOnce<Thread> shutdownHookThread = new SetOnce<>();
private Thread shutdownHookThread;

/** Parses options for this command from args and executes it. */
public final int main(String[] args, Terminal terminal) throws Exception {
if (addShutdownHook()) {
shutdownHookThread.set(new Thread(() -> {

shutdownHookThread = new Thread(() -> {
try {
this.close();
} catch (final IOException e) {
Expand All @@ -75,16 +71,11 @@ public final int main(String[] args, Terminal terminal) throws Exception {
throw new AssertionError(impossible);
}
}
}));
Runtime.getRuntime().addShutdownHook(shutdownHookThread.get());
});
Runtime.getRuntime().addShutdownHook(shutdownHookThread);
}

if (shouldConfigureLoggingWithoutConfig()) {
// initialize default for es.logger.level because we will not read the log4j2.properties
final String loggerLevel = System.getProperty("es.logger.level", Level.INFO.name());
final Settings settings = Settings.builder().put("logger.level", loggerLevel).build();
LogConfigurator.configureWithoutConfig(settings);
}
beforeExecute();

try {
mainWithoutErrorHandling(args, terminal);
Expand All @@ -103,14 +94,10 @@ public final int main(String[] args, Terminal terminal) throws Exception {
}

/**
* Indicate whether or not logging should be configured without reading a log4j2.properties. Most commands should do this because we do
* not configure logging for CLI tools. Only commands that configure logging on their own should not do this.
*
* @return true if logging should be configured without reading a log4j2.properties file
* Setup method to be executed before parsing or execution of the command being run. Any exceptions thrown by the
* method will not be cleanly caught by the parser.
*/
protected boolean shouldConfigureLoggingWithoutConfig() {
return true;
}
protected void beforeExecute() {}

/**
* Executes the command, but all errors are thrown.
Expand Down Expand Up @@ -166,6 +153,11 @@ protected boolean addShutdownHook() {
return true;
}

/** Gets the shutdown hook thread if it exists **/
Thread getShutdownHookThread() {
return shutdownHookThread;
}

@Override
public void close() throws IOException {

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.elasticsearch.cli;

import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

/**
* Annotation to suppress forbidden-apis errors inside a whole class, a method, or a field.
*/
@Retention(RetentionPolicy.CLASS)
@Target({ ElementType.CONSTRUCTOR, ElementType.FIELD, ElementType.METHOD, ElementType.TYPE })
public @interface SuppressForbidden {
String reason();
}

Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@

package org.elasticsearch.cli;

import org.elasticsearch.common.SuppressForbidden;

import java.io.BufferedReader;
import java.io.Console;
import java.io.IOException;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@
import joptsimple.OptionSet;
import joptsimple.OptionSpec;
import joptsimple.util.KeyValuePair;
import org.apache.logging.log4j.Level;
import org.elasticsearch.common.SuppressForbidden;
import org.elasticsearch.common.logging.LogConfigurator;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
import org.elasticsearch.node.InternalSettingsPreparer;
Expand Down Expand Up @@ -102,6 +104,26 @@ private static void putSystemPropertyIfSettingIsMissing(final Map<String, String
}
}

@Override
protected final void beforeExecute() {
if (shouldConfigureLoggingWithoutConfig()) {
// initialize default for es.logger.level because we will not read the log4j2.properties
final String loggerLevel = System.getProperty("es.logger.level", Level.INFO.name());
final Settings settings = Settings.builder().put("logger.level", loggerLevel).build();
LogConfigurator.configureWithoutConfig(settings);
}
}

/**
* Indicate whether or not logging should be configured without reading a log4j2.properties. Most commands should do this because we do
* not configure logging for CLI tools. Only commands that configure logging on their own should not do this.
*
* @return true if logging should be configured without reading a log4j2.properties file
*/
protected boolean shouldConfigureLoggingWithoutConfig() {
return true;
}

/** Execute the command with the initialized {@link Environment}. */
protected abstract void execute(Terminal terminal, OptionSet options, Environment env) throws Exception;

Expand Down
1 change: 1 addition & 0 deletions distribution/tools/plugin-cli/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ apply plugin: 'elasticsearch.build'

dependencies {
provided "org.elasticsearch:elasticsearch:${version}"
provided "org.elasticsearch:elasticsearch-cli:${version}"
testCompile "org.elasticsearch.test:framework:${version}"
testCompile 'com.google.jimfs:jimfs:1.1'
testCompile 'com.google.guava:guava:18.0'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,11 @@ public void close() throws IOException {
};
final MockTerminal terminal = new MockTerminal();
command.main(new String[0], terminal);
assertNotNull(command.shutdownHookThread.get());
assertNotNull(command.getShutdownHookThread());
// successful removal here asserts that the runtime hook was installed in Command#main
assertTrue(Runtime.getRuntime().removeShutdownHook(command.shutdownHookThread.get()));
command.shutdownHookThread.get().run();
command.shutdownHookThread.get().join();
assertTrue(Runtime.getRuntime().removeShutdownHook(command.getShutdownHookThread()));
command.getShutdownHookThread().run();
command.getShutdownHookThread().join();
assertTrue(closed.get());
final String output = terminal.getOutput();
if (shouldThrow) {
Expand Down
1 change: 1 addition & 0 deletions settings.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ List projects = [
'build-tools',
'rest-api-spec',
'core',
'core:cli',
'docs',
'client:rest',
'client:rest-high-level',
Expand Down
1 change: 1 addition & 0 deletions test/framework/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import org.elasticsearch.gradle.precommit.PrecommitTasks;
dependencies {
compile "org.elasticsearch.client:elasticsearch-rest-client:${version}"
compile "org.elasticsearch:elasticsearch:${version}"
compile "org.elasticsearch:elasticsearch-cli:${version}"
compile "com.carrotsearch.randomizedtesting:randomizedtesting-runner:${versions.randomizedrunner}"
compile "junit:junit:${versions.junit}"
compile "org.hamcrest:hamcrest-all:${versions.hamcrest}"
Expand Down

0 comments on commit f2fef68

Please sign in to comment.