From fd3f80b30e114f88a2719a009751d7664873bd03 Mon Sep 17 00:00:00 2001
From: iamdanfox <iamdanfox@gmail.com>
Date: Wed, 13 Feb 2019 17:13:08 +0000
Subject: [PATCH] [improvement] javaHome accepts $JAVA_11_HOME style values
 (#76)

## Before this PR

We want to allow people to use a java installation whose location is supplied by another environment variable, e.g. $JAVA_11_HOME but this indirection is currently impossible. When choosing your desired java home, you had to specify either:

- a literal path on disk
- an empty string, which results in the value of $JAVA_HOME

## After this PR

You can now specify the following and everything should Just Work:

```yaml
javaHome: $JAVA_11_HOME
```
---
 README.md                  |  4 ++--
 launchlib/launcher.go      | 20 ++++++++++++++++----
 launchlib/launcher_test.go | 13 ++++++++++++-
 launchlib/monitor.go       |  4 ++--
 4 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/README.md b/README.md
index f0c87371..e6f62ef4 100644
--- a/README.md
+++ b/README.md
@@ -17,8 +17,8 @@ configType: java
 configVersion: 1
 # REQUIRED - The main class to be run
 mainClass: my.package.Main
-# OPTIONAL - Path to the JRE, defaults to the JAVA_HOME environment variable if unset
-javaHome: javaHome
+# OPTIONAL - Path to the JRE or environment variable name (e.g. $JAVA_11_HOME). Defaults to the JAVA_HOME environment variable if unset
+javaHome: /opt/palantir/jdk8/Contents/Home
 # REQUIRED - The classpath entries; the final classpath is the ':'-concatenated list in the given order
 classpath:
   - ./foo.jar
diff --git a/launchlib/launcher.go b/launchlib/launcher.go
index 9b42a98f..9ad9b3a0 100644
--- a/launchlib/launcher.go
+++ b/launchlib/launcher.go
@@ -153,13 +153,25 @@ func verifyPathIsSafeForExec(execPath string) (string, error) {
 // Returns explicitJavaHome if it is not the empty string, or the value of the JAVA_HOME environment variable otherwise.
 // Panics if neither of them is set.
 func getJavaHome(explicitJavaHome string) (string, error) {
-	if explicitJavaHome != "" {
-		return explicitJavaHome, nil
+	if explicitJavaHome == "" {
+		return loadEnvVar("JAVA_HOME")
 	}
 
-	javaHome := os.Getenv("JAVA_HOME")
+	if explicitJavaHome[0] == '$' {
+		if len(explicitJavaHome) == 1 {
+			return "", fmt.Errorf("javaHome set to just '$' is not allowed, please use a path or an env var name like $JAVA_11_HOME")
+		}
+
+		return loadEnvVar(explicitJavaHome[1:])
+	}
+
+	return explicitJavaHome, nil
+}
+
+func loadEnvVar(envVar string) (string, error) {
+	javaHome := os.Getenv(envVar)
 	if len(javaHome) == 0 {
-		return "", fmt.Errorf("JAVA_HOME environment variable not set")
+		return "", fmt.Errorf("%s environment variable not set", envVar)
 	}
 	return javaHome, nil
 }
diff --git a/launchlib/launcher_test.go b/launchlib/launcher_test.go
index 91da88fc..23bb5132 100644
--- a/launchlib/launcher_test.go
+++ b/launchlib/launcher_test.go
@@ -24,7 +24,7 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func TestGetJavaHome(t *testing.T) {
+func TestGetJavaHome_usesJAVA_HOMEbydefault(t *testing.T) {
 	originalJavaHome := os.Getenv("JAVA_HOME")
 	require.NoError(t, os.Setenv("JAVA_HOME", "foo"))
 
@@ -38,6 +38,17 @@ func TestGetJavaHome(t *testing.T) {
 	require.NoError(t, os.Setenv("JAVA_HOME", originalJavaHome))
 }
 
+func TestGetJavaHome_allowsReadingOtherEnvVar(t *testing.T) {
+	original := os.Getenv("SOME_VAR")
+	defer func() { require.NoError(t, os.Setenv("SOME_VAR", original)) }()
+
+	require.NoError(t, os.Setenv("SOME_VAR", "foo"))
+
+	javaHome, javaHomeErr := getJavaHome("$SOME_VAR")
+	assert.NoError(t, javaHomeErr)
+	assert.Equal(t, "foo", javaHome)
+}
+
 func TestSetCustomEnvironment(t *testing.T) {
 	originalEnv := make(map[string]string)
 	customEnv := map[string]string{
diff --git a/launchlib/monitor.go b/launchlib/monitor.go
index 73d3aba7..aadcebcc 100755
--- a/launchlib/monitor.go
+++ b/launchlib/monitor.go
@@ -89,14 +89,14 @@ func (m *ProcessMonitor) SignalSubProcesses(sign os.Signal) error {
 	}
 
 	if len(errPids) > 0 {
-		return errors.Errorf("unable to kill sub-processes for pids %s", errPids)
+		return errors.Errorf("unable to kill sub-processes for pids %v", errPids)
 	}
 	return nil
 }
 
 func (m *ProcessMonitor) verify() error {
 	if os.Getppid() != m.PrimaryPID {
-		return errors.Errorf("ProcessMonitor is a sub-process of '%s' not service primary process '%s'. "+
+		return errors.Errorf("ProcessMonitor is a sub-process of '%d' not service primary process '%d'. "+
 			"ProcessMonitor is expected to only be used by the go-java-launcher itself, under the same process as the"+
 			" service", os.Getppid(), m.PrimaryPID)
 	}