From 5446b34b11bffdc9e3e461f42a00dcdf9c617765 Mon Sep 17 00:00:00 2001
From: Brian de Alwis <bdealwis@google.com>
Date: Mon, 26 Apr 2021 15:46:03 -0400
Subject: [PATCH 1/9] Ensure working JVM before enabling Jib actions to avoid
 hangs

---
 pkg/skaffold/build/jib/init.go     |  4 +++
 pkg/skaffold/build/jib/jib.go      |  4 +++
 pkg/skaffold/build/jib/jvm.go      | 47 ++++++++++++++++++++++++++++++
 pkg/skaffold/build/jib/jvm_test.go | 44 ++++++++++++++++++++++++++++
 4 files changed, 99 insertions(+)
 create mode 100644 pkg/skaffold/build/jib/jvm.go
 create mode 100644 pkg/skaffold/build/jib/jvm_test.go

diff --git a/pkg/skaffold/build/jib/init.go b/pkg/skaffold/build/jib/init.go
index ebdf7a26b46..8bbbb16122a 100644
--- a/pkg/skaffold/build/jib/init.go
+++ b/pkg/skaffold/build/jib/init.go
@@ -85,6 +85,10 @@ type jibJSON struct {
 
 // validate checks if a file is a valid Jib configuration. Returns the list of Config objects corresponding to each Jib project built by the file, or nil if Jib is not configured.
 func validate(path string, enableGradleAnalysis bool) []ArtifactConfig {
+	if !JVMFound {
+		logrus.Debugf("Skipping Jib for init for %q: no functioning Java VM", path)
+		return nil
+	}
 	// Determine whether maven or gradle
 	var builderType PluginType
 	var executable, wrapper, taskName, searchString, consoleFlag string
diff --git a/pkg/skaffold/build/jib/jib.go b/pkg/skaffold/build/jib/jib.go
index 30e17618137..c29ff0b6164 100644
--- a/pkg/skaffold/build/jib/jib.go
+++ b/pkg/skaffold/build/jib/jib.go
@@ -117,6 +117,10 @@ func GetDependencies(ctx context.Context, workspace string, artifact *latest.Jib
 
 // DeterminePluginType tries to determine the Jib plugin type for the given artifact.
 func DeterminePluginType(workspace string, artifact *latest.JibArtifact) (PluginType, error) {
+	if !JVMFound {
+		return "", errors.New("no working JVM available")
+	}
+
 	// check if explicitly specified
 	if artifact != nil {
 		if t := PluginType(artifact.Type); t.IsKnown() {
diff --git a/pkg/skaffold/build/jib/jvm.go b/pkg/skaffold/build/jib/jvm.go
new file mode 100644
index 00000000000..cf56639770a
--- /dev/null
+++ b/pkg/skaffold/build/jib/jvm.go
@@ -0,0 +1,47 @@
+/*
+Copyright 2021 The Skaffold Authors
+
+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 jib
+
+import (
+	"os/exec"
+
+	"github.com/sirupsen/logrus"
+
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
+)
+
+var (
+	// JVMFound is true if a Java VM was found and works.
+	JVMFound bool
+)
+
+func init() {
+	JVMFound = resolveJVM()
+}
+
+// resolveJVMForInit returns true if a Java VM was found and works.  It is intended for
+// `skaffold init` on macOS where calling out to the Maven Wrapper script (mvnw) can
+// hang if there is no installed Java VM found.
+func resolveJVM() bool {
+	// TODO: should we have an override for testing?
+	cmd := exec.Command("java", "-version")
+	err := util.RunCmd(cmd)
+	if err != nil {
+		logrus.Warnf("Skipping Jib init: could not resolve JVM: %v", err)
+	}
+	return err == nil
+}
diff --git a/pkg/skaffold/build/jib/jvm_test.go b/pkg/skaffold/build/jib/jvm_test.go
new file mode 100644
index 00000000000..60641caefb9
--- /dev/null
+++ b/pkg/skaffold/build/jib/jvm_test.go
@@ -0,0 +1,44 @@
+/*
+Copyright 2021 The Skaffold Authors
+
+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 jib
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
+	"github.com/GoogleContainerTools/skaffold/testutil"
+)
+
+func TestResolveJVM(t *testing.T) {
+	tests := []struct {
+		name string
+		cmd      *testutil.FakeCmd
+		expected bool
+	}{
+		{name: "found", cmd: testutil.CmdRun("java -version"), expected: true},
+		{name: "not found", cmd: testutil.CmdRunErr("java -version", errors.New("not found")), expected: false},
+	}
+	for _, test := range tests {
+		testutil.Run(t, test.name, func(t *testutil.T) {
+			t.Override(&util.DefaultExecCommand, test.cmd)
+
+			result := resolveJVM()
+			t.CheckDeepEqual(test.expected, result)
+		})
+	}
+}

From 9509b1c9787e1aadb318aa4f066f73ff8f20a95a Mon Sep 17 00:00:00 2001
From: Brian de Alwis <bdealwis@google.com>
Date: Mon, 26 Apr 2021 23:24:12 -0400
Subject: [PATCH 2/9] Use sync.Once as package is initialized very early

---
 pkg/skaffold/build/jib/init.go |  2 +-
 pkg/skaffold/build/jib/jib.go  |  2 +-
 pkg/skaffold/build/jib/jvm.go  | 14 ++++++++++----
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/pkg/skaffold/build/jib/init.go b/pkg/skaffold/build/jib/init.go
index 8bbbb16122a..30fa4d43efb 100644
--- a/pkg/skaffold/build/jib/init.go
+++ b/pkg/skaffold/build/jib/init.go
@@ -85,7 +85,7 @@ type jibJSON struct {
 
 // validate checks if a file is a valid Jib configuration. Returns the list of Config objects corresponding to each Jib project built by the file, or nil if Jib is not configured.
 func validate(path string, enableGradleAnalysis bool) []ArtifactConfig {
-	if !JVMFound {
+	if !JVMFound() {
 		logrus.Debugf("Skipping Jib for init for %q: no functioning Java VM", path)
 		return nil
 	}
diff --git a/pkg/skaffold/build/jib/jib.go b/pkg/skaffold/build/jib/jib.go
index c29ff0b6164..bd5ee26da33 100644
--- a/pkg/skaffold/build/jib/jib.go
+++ b/pkg/skaffold/build/jib/jib.go
@@ -117,7 +117,7 @@ func GetDependencies(ctx context.Context, workspace string, artifact *latest.Jib
 
 // DeterminePluginType tries to determine the Jib plugin type for the given artifact.
 func DeterminePluginType(workspace string, artifact *latest.JibArtifact) (PluginType, error) {
-	if !JVMFound {
+	if !JVMFound() {
 		return "", errors.New("no working JVM available")
 	}
 
diff --git a/pkg/skaffold/build/jib/jvm.go b/pkg/skaffold/build/jib/jvm.go
index cf56639770a..c6a9c940947 100644
--- a/pkg/skaffold/build/jib/jvm.go
+++ b/pkg/skaffold/build/jib/jvm.go
@@ -18,6 +18,7 @@ package jib
 
 import (
 	"os/exec"
+	"sync"
 
 	"github.com/sirupsen/logrus"
 
@@ -26,11 +27,16 @@ import (
 
 var (
 	// JVMFound is true if a Java VM was found and works.
-	JVMFound bool
+	resolveJVMOnce sync.Once
+	jvmPresent bool
 )
 
-func init() {
-	JVMFound = resolveJVM()
+// JVMFound returns true if a Java VM was found and works.
+func JVMFound() bool {
+	resolveJVMOnce.Do(func() {
+		jvmPresent = resolveJVM()
+	})
+	return jvmPresent
 }
 
 // resolveJVMForInit returns true if a Java VM was found and works.  It is intended for
@@ -41,7 +47,7 @@ func resolveJVM() bool {
 	cmd := exec.Command("java", "-version")
 	err := util.RunCmd(cmd)
 	if err != nil {
-		logrus.Warnf("Skipping Jib init: could not resolve JVM: %v", err)
+		logrus.Warnf("Skipping Jib: no JVM: %v failed: %v", cmd.Args, err)
 	}
 	return err == nil
 }

From ae5461cdcbfeaec873534bad9a356a8ae9d5dc9a Mon Sep 17 00:00:00 2001
From: Brian de Alwis <bdealwis@google.com>
Date: Tue, 27 Apr 2021 13:24:18 -0400
Subject: [PATCH 3/9] Sharp eyes

Co-authored-by: Marlon Gamez <marlongamez@google.com>
---
 pkg/skaffold/build/jib/jvm.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pkg/skaffold/build/jib/jvm.go b/pkg/skaffold/build/jib/jvm.go
index c6a9c940947..f05fcbd8c30 100644
--- a/pkg/skaffold/build/jib/jvm.go
+++ b/pkg/skaffold/build/jib/jvm.go
@@ -39,7 +39,7 @@ func JVMFound() bool {
 	return jvmPresent
 }
 
-// resolveJVMForInit returns true if a Java VM was found and works.  It is intended for
+// resolveJVM returns true if a Java VM was found and works.  It is intended for
 // `skaffold init` on macOS where calling out to the Maven Wrapper script (mvnw) can
 // hang if there is no installed Java VM found.
 func resolveJVM() bool {

From bd4c395b9f906501c914a68878cdf5203f92be73 Mon Sep 17 00:00:00 2001
From: Brian de Alwis <bdealwis@google.com>
Date: Tue, 27 Apr 2021 13:36:53 -0400
Subject: [PATCH 4/9] goimports

---
 pkg/skaffold/build/jib/jvm.go      | 2 +-
 pkg/skaffold/build/jib/jvm_test.go | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/pkg/skaffold/build/jib/jvm.go b/pkg/skaffold/build/jib/jvm.go
index f05fcbd8c30..03232de2192 100644
--- a/pkg/skaffold/build/jib/jvm.go
+++ b/pkg/skaffold/build/jib/jvm.go
@@ -28,7 +28,7 @@ import (
 var (
 	// JVMFound is true if a Java VM was found and works.
 	resolveJVMOnce sync.Once
-	jvmPresent bool
+	jvmPresent     bool
 )
 
 // JVMFound returns true if a Java VM was found and works.
diff --git a/pkg/skaffold/build/jib/jvm_test.go b/pkg/skaffold/build/jib/jvm_test.go
index 60641caefb9..35c5163ddbb 100644
--- a/pkg/skaffold/build/jib/jvm_test.go
+++ b/pkg/skaffold/build/jib/jvm_test.go
@@ -26,7 +26,7 @@ import (
 
 func TestResolveJVM(t *testing.T) {
 	tests := []struct {
-		name string
+		name     string
 		cmd      *testutil.FakeCmd
 		expected bool
 	}{

From b99a9c74a933d8f4611b462f8beb56cacaaa5daa Mon Sep 17 00:00:00 2001
From: Brian de Alwis <bdealwis@google.com>
Date: Tue, 27 Apr 2021 15:01:26 -0400
Subject: [PATCH 5/9] Make jib tests predictable

---
 pkg/skaffold/build/jib/jvm_test.go | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/pkg/skaffold/build/jib/jvm_test.go b/pkg/skaffold/build/jib/jvm_test.go
index 35c5163ddbb..c9af4553fd9 100644
--- a/pkg/skaffold/build/jib/jvm_test.go
+++ b/pkg/skaffold/build/jib/jvm_test.go
@@ -18,12 +18,19 @@ package jib
 
 import (
 	"errors"
+	"os"
 	"testing"
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
 	"github.com/GoogleContainerTools/skaffold/testutil"
 )
 
+func TestMain(m *testing.M) {
+	JVMFound()	// prime the pump
+	jvmPresent = true
+	os.Exit(m.Run())
+}
+
 func TestResolveJVM(t *testing.T) {
 	tests := []struct {
 		name     string

From a3c3da9d099ca1db7360a80c5c75d942339bb815 Mon Sep 17 00:00:00 2001
From: Brian de Alwis <bdealwis@google.com>
Date: Tue, 27 Apr 2021 15:33:00 -0400
Subject: [PATCH 6/9] goimports

---
 pkg/skaffold/build/jib/jvm_test.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pkg/skaffold/build/jib/jvm_test.go b/pkg/skaffold/build/jib/jvm_test.go
index c9af4553fd9..c2d9183204d 100644
--- a/pkg/skaffold/build/jib/jvm_test.go
+++ b/pkg/skaffold/build/jib/jvm_test.go
@@ -26,7 +26,7 @@ import (
 )
 
 func TestMain(m *testing.M) {
-	JVMFound()	// prime the pump
+	JVMFound() // prime the pump
 	jvmPresent = true
 	os.Exit(m.Run())
 }

From 8b7fcc11a0b48c1b1d0345bfa642a57619c2abbf Mon Sep 17 00:00:00 2001
From: Brian de Alwis <bdealwis@google.com>
Date: Tue, 27 Apr 2021 15:58:06 -0400
Subject: [PATCH 7/9] Ensure TestExpectedBuildFailures fails without a JVM

---
 integration/build_test.go | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/integration/build_test.go b/integration/build_test.go
index 83c3b5b6a0e..1bb2f802263 100644
--- a/integration/build_test.go
+++ b/integration/build_test.go
@@ -28,6 +28,7 @@ import (
 	"github.com/sirupsen/logrus"
 
 	"github.com/GoogleContainerTools/skaffold/integration/skaffold"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/jib"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
 	kubectx "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/context"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext"
@@ -112,6 +113,9 @@ func TestBuild(t *testing.T) {
 // TestExpectedBuildFailures verifies that `skaffold build` fails in expected ways
 func TestExpectedBuildFailures(t *testing.T) {
 	MarkIntegrationTest(t, CanRunWithoutGcp)
+	if !jib.JVMFound() {
+		t.Fatal("test requires Java VM")
+	}
 
 	tests := []struct {
 		description string

From 8fc744f16fd3a9bb08113177d823b9ffe78fa47e Mon Sep 17 00:00:00 2001
From: Brian de Alwis <bdealwis@google.com>
Date: Mon, 3 May 2021 09:52:33 -0400
Subject: [PATCH 8/9] Make jib.JVMFound() replaceable for tests

Add TestMain around jib unit tests to force JVMFound to true.
---
 pkg/skaffold/build/gcb/jib_test.go |  7 +++++++
 pkg/skaffold/build/jib/jvm.go      | 18 +++++++++++++-----
 pkg/skaffold/build/jib/jvm_test.go |  4 ++--
 3 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/pkg/skaffold/build/gcb/jib_test.go b/pkg/skaffold/build/gcb/jib_test.go
index cfc80c60a77..e8d0d965291 100644
--- a/pkg/skaffold/build/gcb/jib_test.go
+++ b/pkg/skaffold/build/gcb/jib_test.go
@@ -17,6 +17,7 @@ limitations under the License.
 package gcb
 
 import (
+	"os"
 	"path/filepath"
 	"testing"
 
@@ -27,6 +28,12 @@ import (
 	"github.com/GoogleContainerTools/skaffold/testutil"
 )
 
+func TestMain(m *testing.M) {
+	// these tests don't actually require a JVM
+	jib.JVMFound = func() bool { return true }
+	os.Exit(m.Run())
+}
+
 func TestJibMavenBuildSpec(t *testing.T) {
 	tests := []struct {
 		description  string
diff --git a/pkg/skaffold/build/jib/jvm.go b/pkg/skaffold/build/jib/jvm.go
index 03232de2192..9faec2e3b6d 100644
--- a/pkg/skaffold/build/jib/jvm.go
+++ b/pkg/skaffold/build/jib/jvm.go
@@ -26,13 +26,19 @@ import (
 )
 
 var (
-	// JVMFound is true if a Java VM was found and works.
+	// replaceable for testing
+	JVMFound = jvmFound
+
+	// JVMFound() returns true if a Java VM was found and works.
 	resolveJVMOnce sync.Once
 	jvmPresent     bool
+
 )
 
-// JVMFound returns true if a Java VM was found and works.
-func JVMFound() bool {
+// jvmFound returns true if a Java VM was found and works.
+func jvmFound() bool {
+	// Check on demand: performing the check in an init() causes the
+	// check to be run even when no jib functionality was used.
 	resolveJVMOnce.Do(func() {
 		jvmPresent = resolveJVM()
 	})
@@ -41,9 +47,11 @@ func JVMFound() bool {
 
 // resolveJVM returns true if a Java VM was found and works.  It is intended for
 // `skaffold init` on macOS where calling out to the Maven Wrapper script (mvnw) can
-// hang if there is no installed Java VM found.
+// hang if there is no installed Java VM found. 
 func resolveJVM() bool {
-	// TODO: should we have an override for testing?
+	// Note that just checking for the existence of `java` is insufficient
+	// as macOS ships with /usr/bin/java that tries to hand off to a JVM
+	// installed in /Library/Java/JavaVirtualMachines
 	cmd := exec.Command("java", "-version")
 	err := util.RunCmd(cmd)
 	if err != nil {
diff --git a/pkg/skaffold/build/jib/jvm_test.go b/pkg/skaffold/build/jib/jvm_test.go
index c2d9183204d..3b2379ae2a8 100644
--- a/pkg/skaffold/build/jib/jvm_test.go
+++ b/pkg/skaffold/build/jib/jvm_test.go
@@ -26,8 +26,8 @@ import (
 )
 
 func TestMain(m *testing.M) {
-	JVMFound() // prime the pump
-	jvmPresent = true
+	// these tests don't actually require a JVM
+	JVMFound = func() bool { return true }
 	os.Exit(m.Run())
 }
 

From 36bfafa1f57f955cc5ba45383b2cf170c486b78c Mon Sep 17 00:00:00 2001
From: Brian de Alwis <bdealwis@google.com>
Date: Mon, 3 May 2021 10:27:58 -0400
Subject: [PATCH 9/9] pacify golangci-lint

---
 pkg/skaffold/build/jib/jvm.go | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/pkg/skaffold/build/jib/jvm.go b/pkg/skaffold/build/jib/jvm.go
index 9faec2e3b6d..67528946e69 100644
--- a/pkg/skaffold/build/jib/jvm.go
+++ b/pkg/skaffold/build/jib/jvm.go
@@ -26,13 +26,12 @@ import (
 )
 
 var (
-	// replaceable for testing
+	// JVMFound is replaceable for testing
 	JVMFound = jvmFound
 
 	// JVMFound() returns true if a Java VM was found and works.
 	resolveJVMOnce sync.Once
 	jvmPresent     bool
-
 )
 
 // jvmFound returns true if a Java VM was found and works.
@@ -47,7 +46,7 @@ func jvmFound() bool {
 
 // resolveJVM returns true if a Java VM was found and works.  It is intended for
 // `skaffold init` on macOS where calling out to the Maven Wrapper script (mvnw) can
-// hang if there is no installed Java VM found. 
+// hang if there is no installed Java VM found.
 func resolveJVM() bool {
 	// Note that just checking for the existence of `java` is insufficient
 	// as macOS ships with /usr/bin/java that tries to hand off to a JVM