Warn when Python debugging library path is not found (#71)

* buildx now requires a context
* Append any library paths to PYTHONPATH to allow users to
  manually override file locations if required.

Author: briandealwis

buildx.sh

@@ -7,7 +7,9 @@ PLATFORMS=linux/amd64,linux/arm64
 
 if ! docker buildx inspect skaffold-builder >/dev/null 2>&1; then
   echo ">> creating "docker buildx" builder 'skaffold-builder'"
-  docker buildx create --name skaffold-builder --platform $PLATFORMS
+  # Docker 3.3.0 require creating a builder within a context
+  docker context create skaffold
+  docker buildx create --name skaffold-builder --platform $PLATFORMS skaffold
 fi
 
 loadOrPush=$(if [ "$PUSH_IMAGE" = true ]; then echo --platform $PLATFORMS --push; else echo --load; fi)

python/helper-image/launcher/env.go

@@ -44,11 +44,11 @@ func (e env) AsPairs() []string {
 	return m
 }
 
-// PrependFilepath prepands a path to a environment variable.
-func (e env) PrependFilepath(key string, path string) {
+// AppendFilepath appands a path to a environment variable.
+func (e env) AppendFilepath(key string, path string) {
 	v := e[key]
 	if v != "" {
-		v = path + string(filepath.ListSeparator) + v
+		v = v + string(filepath.ListSeparator) + path
 	} else {
 		v = path
 	}

python/helper-image/launcher/env_test.go

@@ -80,7 +80,7 @@ func TestEnvFromPairs(t *testing.T) {
 	}
 }
 
-func TestEnvPrependFilepath(t *testing.T) {
+func TestEnvAppendFilepath(t *testing.T) {
 	tests := []struct {
 		description string
 		env         env
@@ -89,15 +89,15 @@ func TestEnvPrependFilepath(t *testing.T) {
 		expected    map[string]string
 	}{
 		{"empty", env{}, "PATH", "value", env{"PATH": "value"}},
-		{"existing value", env{"PATH": "other"}, "PATH", "value", env{"PATH": "value" + string(filepath.ListSeparator) + "other"}},
+		{"existing value", env{"PATH": "other"}, "PATH", "value", env{"PATH": "other" + string(filepath.ListSeparator) + "value"}},
 		{"other value unchanged", env{"PYTHONPATH": "other"}, "PATH", "value", env{"PATH": "value", "PYTHONPATH": "other"}},
-		{"existing value with other value unchanged", env{"PATH": "other", "PYTHONPATH": "other"}, "PATH", "value", env{"PATH": "value" + string(filepath.ListSeparator) + "other", "PYTHONPATH": "other"}},
+		{"existing value with other value unchanged", env{"PATH": "other", "PYTHONPATH": "other"}, "PATH", "value", env{"PATH": "other" + string(filepath.ListSeparator) + "value", "PYTHONPATH": "other"}},
 	}
 
 	for _, test := range tests {
 		t.Run(test.description, func(t *testing.T) {
 			result := test.env // not a copy but that's ok for this test
-			result.PrependFilepath(test.key, test.value)
+			result.AppendFilepath(test.key, test.value)
 			if len(result) != len(test.expected) {
 				t.Errorf("expected %v but got %v", test.expected, result)
 			} else {

python/helper-image/launcher/launcher.go

@@ -257,19 +257,25 @@ func (pc *pythonContext) updateEnv(ctx context.Context) error {
 	}
 	// The skaffold-debug-python helper image places pydevd and debugpy in /dbg/python/lib/pythonM.N,
 	// but separates pydevd and pydevd-pycharm in separate directories to avoid possible leakage.
+	var libraryPath string
 	switch pc.debugMode {
 	case ModePtvsd, ModeDebugpy:
-		libraryPath := fmt.Sprintf(dbgRoot+"/python/lib/python%d.%d/site-packages", major, minor)
-		pc.env.PrependFilepath("PYTHONPATH", libraryPath)
+		libraryPath = fmt.Sprintf(dbgRoot+"/python/lib/python%d.%d/site-packages", major, minor)
 
 	case ModePydevd:
-		libraryPath := fmt.Sprintf(dbgRoot+"/python/pydevd/python%d.%d/lib/python%d.%d/site-packages", major, minor, major, minor)
-		pc.env.PrependFilepath("PYTHONPATH", libraryPath)
+		libraryPath = fmt.Sprintf(dbgRoot+"/python/pydevd/python%d.%d/lib/python%d.%d/site-packages", major, minor, major, minor)
+
 	case ModePydevdPycharm:
-		libraryPath := fmt.Sprintf(dbgRoot+"/python/pydevd-pycharm/python%d.%d/lib/python%d.%d/site-packages", major, minor, major, minor)
-		pc.env.PrependFilepath("PYTHONPATH", libraryPath)
+		libraryPath = fmt.Sprintf(dbgRoot+"/python/pydevd-pycharm/python%d.%d/lib/python%d.%d/site-packages", major, minor, major, minor)
+	}
+	if libraryPath != "" {
+		if !pathExists(libraryPath) {
+			// Warn as the user may have installed debugpy themselves
+			logrus.Warnf("Debugging support for Python %d.%d not found: may require manually installing %q", major, minor, pc.debugMode)
+		}
+		// Append to ensure user-configured values are found first.
+		pc.env.AppendFilepath("PYTHONPATH", libraryPath)
 	}
-
 	return nil
 }
 
@@ -358,3 +364,11 @@ func logrusLevel(env env) logrus.Level {
 	}
 	return logrus.WarnLevel
 }
+
+func pathExists(path string) bool {
+	_, err := os.Stat(path)
+	if err == nil || !os.IsNotExist(err) {
+		return true
+	}
+	return false
+}

python/helper-image/launcher/launcher_test.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"fmt"
 	"io/ioutil"
+	"path/filepath"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -293,3 +294,12 @@ func TestLaunch(t *testing.T) {
 		})
 	}
 }
+
+func TestPathExists(t *testing.T) {
+	if pathExists(filepath.Join("this", "should", "not", "exist")) {
+		t.Error("pathExists should have failed on non-existent path")
+	}
+	if !pathExists(t.TempDir()) {
+		t.Error("pathExists failed on real path")
+	}
+}