You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@reef.apache.org by we...@apache.org on 2016/11/10 22:32:57 UTC
reef git commit: [REEF-1665] Better default value for Java runtime
path
Repository: reef
Updated Branches:
refs/heads/master a5395f875 -> b7a98d795
[REEF-1665] Better default value for Java runtime path
* Fix the default path to java binary use `{{JAVA_HOME}}` syntax for all OS
* Implement `{{...}}` variable expansion for REEF local runtime
* Write unit tests for various cases of the variable expansion
* Minor code refactoring for better readability
This fix allows to run `runtests.ps1 -Yarn ...` again.
JIRA:
[REEF-1665](https://issues.apache.org/jira/browse/REEF-1665)
Pull Request:
This closes #1179
Project: http://git-wip-us.apache.org/repos/asf/reef/repo
Commit: http://git-wip-us.apache.org/repos/asf/reef/commit/b7a98d79
Tree: http://git-wip-us.apache.org/repos/asf/reef/tree/b7a98d79
Diff: http://git-wip-us.apache.org/repos/asf/reef/diff/b7a98d79
Branch: refs/heads/master
Commit: b7a98d795ca2fa1714da21640f3f1fc19d4c0a4d
Parents: a5395f8
Author: Sergiy Matusevych <mo...@apache.org>
Authored: Mon Nov 7 15:56:13 2016 -0800
Committer: Markus Weimer <we...@apache.org>
Committed: Thu Nov 10 14:31:50 2016 -0800
----------------------------------------------------------------------
.../common/launch/JavaLaunchCommandBuilder.java | 11 ++-
.../runtime/local/process/RunnableProcess.java | 48 ++++++++++-
.../RunnableProcessExpandVariablesTest.java | 83 ++++++++++++++++++++
.../runtime/local/process/package-info.java | 22 ++++++
4 files changed, 156 insertions(+), 8 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/reef/blob/b7a98d79/lang/java/reef-common/src/main/java/org/apache/reef/runtime/common/launch/JavaLaunchCommandBuilder.java
----------------------------------------------------------------------
diff --git a/lang/java/reef-common/src/main/java/org/apache/reef/runtime/common/launch/JavaLaunchCommandBuilder.java b/lang/java/reef-common/src/main/java/org/apache/reef/runtime/common/launch/JavaLaunchCommandBuilder.java
index bf77ee4..5f81a5d 100644
--- a/lang/java/reef-common/src/main/java/org/apache/reef/runtime/common/launch/JavaLaunchCommandBuilder.java
+++ b/lang/java/reef-common/src/main/java/org/apache/reef/runtime/common/launch/JavaLaunchCommandBuilder.java
@@ -37,19 +37,22 @@ import java.util.regex.Pattern;
* Build the launch command for Java REEF processes.
*/
public final class JavaLaunchCommandBuilder implements LaunchCommandBuilder {
+
private static final Logger LOG = Logger.getLogger(JavaLaunchCommandBuilder.class.getName());
- private static final String DEFAULT_JAVA_PATH = System.getenv("JAVA_HOME") + "/bin/" + "java";
+ private static final String DEFAULT_JAVA_PATH = "{{JAVA_HOME}}/bin/java";
private static final String[] DEFAULT_OPTIONS = {"-XX:PermSize=128m", "-XX:MaxPermSize=128m"};
+
+ private final Map<String, JVMOption> options = new HashMap<>();
+ private final List<String> commandPrefixList;
+ private final Class launcherClass;
+
private String stderrPath = null;
private String stdoutPath = null;
private Optional<List<String>> evaluatorConfigurationPaths = Optional.empty();
private String javaPath = null;
private String classPath = null;
private Boolean assertionsEnabled = null;
- private final Map<String, JVMOption> options = new HashMap<>();
- private final List<String> commandPrefixList;
- private final Class launcherClass;
/**
* Constructor that populates default options, using the default Launcher
http://git-wip-us.apache.org/repos/asf/reef/blob/b7a98d79/lang/java/reef-runtime-local/src/main/java/org/apache/reef/runtime/local/process/RunnableProcess.java
----------------------------------------------------------------------
diff --git a/lang/java/reef-runtime-local/src/main/java/org/apache/reef/runtime/local/process/RunnableProcess.java b/lang/java/reef-runtime-local/src/main/java/org/apache/reef/runtime/local/process/RunnableProcess.java
index d545b3f..46b7d9f 100644
--- a/lang/java/reef-runtime-local/src/main/java/org/apache/reef/runtime/local/process/RunnableProcess.java
+++ b/lang/java/reef-runtime-local/src/main/java/org/apache/reef/runtime/local/process/RunnableProcess.java
@@ -23,6 +23,7 @@ import org.apache.reef.util.OSUtils;
import java.io.*;
import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.concurrent.TimeUnit;
@@ -31,6 +32,8 @@ import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
import java.util.logging.Level;
import java.util.logging.Logger;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
/**
* A runnable class that encapsulates a process.
@@ -105,7 +108,7 @@ public final class RunnableProcess implements Runnable {
final String standardErrorFileName) {
this.processObserver = processObserver;
- this.command = Collections.unmodifiableList(command);
+ this.command = Collections.unmodifiableList(expandEnvironmentVariables(command));
this.id = id;
this.folder = folder;
@@ -120,6 +123,45 @@ public final class RunnableProcess implements Runnable {
LOG.log(Level.FINEST, "RunnableProcess ready.");
}
+ private static final Pattern ENV_REGEX = Pattern.compile("\\{\\{(\\w+)}}");
+
+ /**
+ * Replace {{ENV_VAR}} placeholders with the values of the corresponding environment variables.
+ * @param command An input string with {{ENV_VAR}} placeholders
+ * to be replaced with the values of the corresponding environment variables.
+ * Replace unknown/unset variables with an empty string.
+ * @return A new string with all the placeholders expanded.
+ */
+ public static String expandEnvironmentVariables(final String command) {
+
+ final Matcher match = ENV_REGEX.matcher(command);
+ final StringBuilder res = new StringBuilder(command.length());
+
+ int i = 0;
+ while (match.find()) {
+ final String var = System.getenv(match.group(1));
+ res.append(command.substring(i, match.start())).append(var == null ? "" : var);
+ i = match.end();
+ }
+
+ return res.append(command.substring(i, command.length())).toString();
+ }
+
+ /**
+ * Replace {{ENV_VAR}} placeholders with the values of the corresponding environment variables.
+ * @param command An input list of strings with {{ENV_VAR}} placeholders
+ * to be replaced with the values of the corresponding environment variables.
+ * Replace unknown/unset variables with an empty string.
+ * @return A new list of strings with all the placeholders expanded.
+ */
+ public static List<String> expandEnvironmentVariables(final List<String> command) {
+ final ArrayList<String> res = new ArrayList<>(command.size());
+ for (final String cmd : command) {
+ res.add(expandEnvironmentVariables(cmd));
+ }
+ return res;
+ }
+
/**
* Runs the configured process.
* @throws IllegalStateException if the process is already running or has been running before.
@@ -157,9 +199,7 @@ public final class RunnableProcess implements Runnable {
this.processObserver.onProcessStarted(this.id);
} catch (final IOException ex) {
- LOG.log(Level.SEVERE,
- "Unable to spawn process \"{0}\" wth command {1}\n Exception:{2}",
- new Object[] {this.id, this.command, ex});
+ LOG.log(Level.SEVERE, "Unable to spawn process " + this.id + " with command " + this.command, ex);
}
} finally {
http://git-wip-us.apache.org/repos/asf/reef/blob/b7a98d79/lang/java/reef-runtime-local/src/test/java/org/apache/reef/runtime/local/process/RunnableProcessExpandVariablesTest.java
----------------------------------------------------------------------
diff --git a/lang/java/reef-runtime-local/src/test/java/org/apache/reef/runtime/local/process/RunnableProcessExpandVariablesTest.java b/lang/java/reef-runtime-local/src/test/java/org/apache/reef/runtime/local/process/RunnableProcessExpandVariablesTest.java
new file mode 100644
index 0000000..169ff6e
--- /dev/null
+++ b/lang/java/reef-runtime-local/src/test/java/org/apache/reef/runtime/local/process/RunnableProcessExpandVariablesTest.java
@@ -0,0 +1,83 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.reef.runtime.local.process;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.Arrays;
+import java.util.List;
+
+/**
+ * Unit tests for the RunnableProcess utility methods.
+ */
+public final class RunnableProcessExpandVariablesTest {
+
+ @Test
+ public void testExpandEnvironmentVariablesEmpty() {
+ final String res = RunnableProcess.expandEnvironmentVariables("");
+ Assert.assertEquals("", res);
+ }
+
+ @Test
+ public void testExpandEnvironmentVariablesNoVars() {
+ final String res = RunnableProcess.expandEnvironmentVariables("foo*bar");
+ Assert.assertEquals("foo*bar", res);
+ }
+
+ @Test
+ public void testExpandEnvironmentVariablesPartial1() {
+ final String res = RunnableProcess.expandEnvironmentVariables("foo{{bar");
+ Assert.assertEquals("foo{{bar", res);
+ }
+
+ @Test
+ public void testExpandEnvironmentVariablesPartial2() {
+ final String res = RunnableProcess.expandEnvironmentVariables("foo{{}}bar");
+ Assert.assertEquals("foo{{}}bar", res);
+ }
+
+ @Test
+ public void testExpandEnvironmentVariablesNonMatching() {
+ final String res = RunnableProcess.expandEnvironmentVariables("foo*{{SOME_RANDOM_VAR}}*bar");
+ Assert.assertEquals("foo**bar", res);
+ }
+
+ @Test
+ public void testExpandEnvironmentVariablesMatching() {
+ final String path = System.getenv("PATH");
+ final String res = RunnableProcess.expandEnvironmentVariables("foo*{{PATH}}*bar");
+ Assert.assertEquals("foo*" + path + "*bar", res);
+ }
+
+ @Test
+ public void testExpandEnvironmentVariablesMatchingMany() {
+ final String path = System.getenv("PATH");
+ final String res = RunnableProcess.expandEnvironmentVariables("foo*{{PATH}}{{PATH}}*bar");
+ Assert.assertEquals("foo*" + path + path + "*bar", res);
+ }
+
+ @Test
+ public void testExpandEnvironmentVariablesList() {
+ final String path = System.getenv("PATH");
+ final List<String> res = RunnableProcess.expandEnvironmentVariables(
+ Arrays.asList("foo*{{PATH}}", "{{PATH}}*bar", "", "etc"));
+ Assert.assertArrayEquals(new String[] {"foo*" + path, path + "*bar", "", "etc"}, res.toArray());
+ }
+}
http://git-wip-us.apache.org/repos/asf/reef/blob/b7a98d79/lang/java/reef-runtime-local/src/test/java/org/apache/reef/runtime/local/process/package-info.java
----------------------------------------------------------------------
diff --git a/lang/java/reef-runtime-local/src/test/java/org/apache/reef/runtime/local/process/package-info.java b/lang/java/reef-runtime-local/src/test/java/org/apache/reef/runtime/local/process/package-info.java
new file mode 100644
index 0000000..8c12f40
--- /dev/null
+++ b/lang/java/reef-runtime-local/src/test/java/org/apache/reef/runtime/local/process/package-info.java
@@ -0,0 +1,22 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+/**
+ * Tests for the local runtime runnable process.
+ */
+package org.apache.reef.runtime.local.process;