From 9e65dd0c4d07d4daa27dcf2568a9ecb5c5796225 Mon Sep 17 00:00:00 2001 From: Brian de Alwis Date: Thu, 6 May 2021 17:47:44 -0400 Subject: [PATCH 1/5] Add support for 'pydevd --continue' --- python/helper-image/Dockerfile | 12 +++++++++ python/helper-image/pydevd.patch | 46 ++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+) create mode 100644 python/helper-image/pydevd.patch diff --git a/python/helper-image/Dockerfile b/python/helper-image/Dockerfile index ce689b18..ab170f58 100644 --- a/python/helper-image/Dockerfile +++ b/python/helper-image/Dockerfile @@ -30,31 +30,43 @@ FROM python:2.7 as python27 RUN PYTHONUSERBASE=/dbgpy pip install --user ptvsd debugpy RUN PYTHONUSERBASE=/dbgpy/pydevd/python2.7 pip install --user pydevd --no-warn-script-location +COPY pydevd.patch . +RUN patch -p0 -d /dbgpy/pydevd/python2.7/lib/python2.7/site-packages < pydevd.patch RUN PYTHONUSERBASE=/dbgpy/pydevd-pycharm/python2.7 pip install --user pydevd-pycharm --no-warn-script-location FROM python:3.5 as python35 RUN PYTHONUSERBASE=/dbgpy pip install --user ptvsd debugpy RUN PYTHONUSERBASE=/dbgpy/pydevd/python3.5 pip install --user pydevd --no-warn-script-location +COPY pydevd.patch . +RUN patch -p0 -d /dbgpy/pydevd/python3.5/lib/python3.5/site-packages < pydevd.patch RUN PYTHONUSERBASE=/dbgpy/pydevd-pycharm/python3.5 pip install --user pydevd-pycharm --no-warn-script-location FROM python:3.6 as python36 RUN PYTHONUSERBASE=/dbgpy pip install --user ptvsd debugpy RUN PYTHONUSERBASE=/dbgpy/pydevd/python3.6 pip install --user pydevd --no-warn-script-location +COPY pydevd.patch . +RUN patch -p0 -d /dbgpy/pydevd/python3.6/lib/python3.6/site-packages < pydevd.patch RUN PYTHONUSERBASE=/dbgpy/pydevd-pycharm/python3.6 pip install --user pydevd-pycharm --no-warn-script-location FROM python:3.7 as python37 RUN PYTHONUSERBASE=/dbgpy pip install --user ptvsd debugpy RUN PYTHONUSERBASE=/dbgpy/pydevd/python3.7 pip install --user pydevd --no-warn-script-location +COPY pydevd.patch . +RUN patch -p0 -d /dbgpy/pydevd/python3.7/lib/python3.7/site-packages < pydevd.patch RUN PYTHONUSERBASE=/dbgpy/pydevd-pycharm/python3.7 pip install --user pydevd-pycharm --no-warn-script-location FROM python:3.8 as python38 RUN PYTHONUSERBASE=/dbgpy pip install --user ptvsd debugpy RUN PYTHONUSERBASE=/dbgpy/pydevd/python3.8 pip install --user pydevd --no-warn-script-location +COPY pydevd.patch . +RUN patch -p0 -d /dbgpy/pydevd/python3.8/lib/python3.8/site-packages < pydevd.patch RUN PYTHONUSERBASE=/dbgpy/pydevd-pycharm/python3.8 pip install --user pydevd-pycharm --no-warn-script-location FROM python:3.9 as python39 RUN PYTHONUSERBASE=/dbgpy pip install --user ptvsd debugpy RUN PYTHONUSERBASE=/dbgpy/pydevd/python3.9 pip install --user pydevd --no-warn-script-location +COPY pydevd.patch . +RUN patch -p0 -d /dbgpy/pydevd/python3.9/lib/python3.9/site-packages < pydevd.patch RUN PYTHONUSERBASE=/dbgpy/pydevd-pycharm/python3.9 pip install --user pydevd-pycharm --no-warn-script-location FROM golang:1.14.1 as build diff --git a/python/helper-image/pydevd.patch b/python/helper-image/pydevd.patch new file mode 100644 index 00000000..ee6dc8ff --- /dev/null +++ b/python/helper-image/pydevd.patch @@ -0,0 +1,46 @@ +commit e6df155b16ee470200b3cda0c99a95e34a4a1d1b +Author: Brian de Alwis +Date: Thu May 6 12:23:05 2021 -0400 + + Initial support for a `--continue` flag for `--server` + +diff --git _pydevd_bundle/pydevd_command_line_handling.py _pydevd_bundle/pydevd_command_line_handling.py +index 2afae09..2985a35 100644 +--- _pydevd_bundle/pydevd_command_line_handling.py ++++ _pydevd_bundle/pydevd_command_line_handling.py +@@ -69,6 +69,7 @@ ACCEPTED_ARG_HANDLERS = [ + ArgHandlerWithParam('client-access-token'), + + ArgHandlerBool('server'), ++ ArgHandlerBool('continue'), + ArgHandlerBool('DEBUG_RECORD_SOCKET_READS'), + ArgHandlerBool('multiproc'), # Used by PyCharm (reuses connection: ssh tunneling) + ArgHandlerBool('multiprocess'), # Used by PyDev (creates new connection to ide) +diff --git pydevd.py pydevd.py +index 4639778..9745add 100644 +--- pydevd.py ++++ pydevd.py +@@ -3276,14 +3276,21 @@ def main(): + + apply_debugger_options(setup) + ++ wait = True ++ if setup['continue']: ++ wait = False ++ + try: +- debugger.connect(host, port) ++ if wait: ++ debugger.connect(host, port) ++ else: ++ debugger.create_wait_for_connection_thread() + except: + sys.stderr.write("Could not connect to %s: %s\n" % (host, port)) + pydev_log.exception() + sys.exit(1) + +- globals = debugger.run(setup['file'], None, None, is_module) ++ globals = debugger.run(setup['file'], None, None, is_module, set_trace=wait) + + if setup['cmd-line']: + debugger.wait_for_commands(globals) From 81fe612cd4f3f5a8bdfff7a97cbad35902a520fd Mon Sep 17 00:00:00 2001 From: Brian de Alwis Date: Wed, 12 May 2021 11:30:00 -0400 Subject: [PATCH 2/5] add support for --continue --- python/helper-image/launcher/launcher.go | 5 ++- python/helper-image/launcher/launcher_test.go | 11 ++++- python/helper-image/pydevd.patch | 40 ++++++++++++++----- 3 files changed, 44 insertions(+), 12 deletions(-) diff --git a/python/helper-image/launcher/launcher.go b/python/helper-image/launcher/launcher.go index 1986faea..0f3b5a29 100644 --- a/python/helper-image/launcher/launcher.go +++ b/python/helper-image/launcher/launcher.go @@ -308,7 +308,7 @@ func (pc *pythonContext) updateCommandLine(ctx context.Context) error { // Appropriate location to resolve pydevd is set in updateEnv // TODO: check for modules (and fail?) cmdline = append(cmdline, pc.args[0]) - cmdline = append(cmdline, "-m", "pydevd", "--port", strconv.Itoa(int(pc.port)), "--server") + cmdline = append(cmdline, "-m", "pydevd", "--server", "--port", strconv.Itoa(int(pc.port))) if pc.env["WRAPPER_VERBOSE"] != "" { cmdline = append(cmdline, "--DEBUG") } @@ -316,6 +316,9 @@ func (pc *pythonContext) updateCommandLine(ctx context.Context) error { // From the pydevd source, PyCharm wants multiproc cmdline = append(cmdline, "--multiproc") } + if !pc.wait { + cmdline = append(cmdline, "--continue") + } cmdline = append(cmdline, "--file") // --file is expected as last argument cmdline = append(cmdline, pc.args[1:]...) if pc.wait { diff --git a/python/helper-image/launcher/launcher_test.go b/python/helper-image/launcher/launcher_test.go index a1ddb472..7803b527 100644 --- a/python/helper-image/launcher/launcher_test.go +++ b/python/helper-image/launcher/launcher_test.go @@ -275,8 +275,15 @@ func TestLaunch(t *testing.T) { description: "pydevd", pc: pythonContext{debugMode: "pydevd", port: 2345, wait: false, args: []string{"python", "app.py"}, env: nil}, commands: RunCmdOut([]string{"python", "-V"}, "Python 3.7.4\n"). - AndRunCmd([]string{"python", "-m", "pydevd", "--port", "2345", "--server", "--file", "app.py"}), - expected: pythonContext{debugMode: "pydevd", port: 2345, wait: false, args: []string{"python", "-m", "pydevd", "--port", "2345", "--server", "--file", "app.py"}, env: env{"PYTHONPATH": dbgRoot + "/python/pydevd/python3.7/lib/python3.7/site-packages"}}, + AndRunCmd([]string{"python", "-m", "pydevd", "--server", "--port", "2345", "--continue", "--file", "app.py"}), + expected: pythonContext{debugMode: "pydevd", port: 2345, wait: false, args: []string{"python", "-m", "pydevd", "--server", "--port", "2345", "--continue", "--file", "app.py"}, env: env{"PYTHONPATH": dbgRoot + "/python/pydevd/python3.7/lib/python3.7/site-packages"}}, + }, + { + description: "pydevd with wait", + pc: pythonContext{debugMode: "pydevd", port: 2345, wait: true, args: []string{"python", "app.py"}, env: nil}, + commands: RunCmdOut([]string{"python", "-V"}, "Python 3.7.4\n"). + AndRunCmd([]string{"python", "-m", "pydevd", "--server", "--port", "2345", "--file", "app.py"}), + expected: pythonContext{debugMode: "pydevd", port: 2345, wait: true, args: []string{"python", "-m", "pydevd", "--server", "--port", "2345", "--file", "app.py"}, env: env{"PYTHONPATH": dbgRoot + "/python/pydevd/python3.7/lib/python3.7/site-packages"}}, }, } diff --git a/python/helper-image/pydevd.patch b/python/helper-image/pydevd.patch index ee6dc8ff..ed09f5ef 100644 --- a/python/helper-image/pydevd.patch +++ b/python/helper-image/pydevd.patch @@ -1,9 +1,3 @@ -commit e6df155b16ee470200b3cda0c99a95e34a4a1d1b -Author: Brian de Alwis -Date: Thu May 6 12:23:05 2021 -0400 - - Initial support for a `--continue` flag for `--server` - diff --git _pydevd_bundle/pydevd_command_line_handling.py _pydevd_bundle/pydevd_command_line_handling.py index 2afae09..2985a35 100644 --- _pydevd_bundle/pydevd_command_line_handling.py @@ -17,10 +11,38 @@ index 2afae09..2985a35 100644 ArgHandlerBool('multiproc'), # Used by PyCharm (reuses connection: ssh tunneling) ArgHandlerBool('multiprocess'), # Used by PyDev (creates new connection to ide) diff --git pydevd.py pydevd.py -index 4639778..9745add 100644 +index 4639778..0da7e27 100644 --- pydevd.py +++ pydevd.py -@@ -3276,14 +3276,21 @@ def main(): +@@ -1376,6 +1376,8 @@ class PyDB(object): + + def run(self): + host = SetupHolder.setup['client'] ++ if host is None: ++ host = 'localhost' + port = SetupHolder.setup['port'] + + self._server_socket = create_server_socket(host=host, port=port) +@@ -2240,7 +2242,7 @@ class PyDB(object): + from _pydev_bundle.pydev_monkey import patch_thread_modules + patch_thread_modules() + +- def run(self, file, globals=None, locals=None, is_module=False, set_trace=True): ++ def run(self, file, globals=None, locals=None, is_module=False, set_trace=True, wait=True): + module_name = None + entry_point_fn = '' + if is_module: +@@ -2322,7 +2324,8 @@ class PyDB(object): + sys.path.insert(0, os.path.split(os_path_abspath(file))[0]) + + if set_trace: +- self.wait_for_ready_to_run() ++ if wait: ++ self.wait_for_ready_to_run() + + # call prepare_to_run when we already have all information about breakpoints + self.prepare_to_run() +@@ -3276,14 +3279,21 @@ def main(): apply_debugger_options(setup) @@ -40,7 +62,7 @@ index 4639778..9745add 100644 sys.exit(1) - globals = debugger.run(setup['file'], None, None, is_module) -+ globals = debugger.run(setup['file'], None, None, is_module, set_trace=wait) ++ globals = debugger.run(setup['file'], None, None, is_module, wait=wait) if setup['cmd-line']: debugger.wait_for_commands(globals) From 8632338253fc24aa2152dbaed3ec54b25a23eb1d Mon Sep 17 00:00:00 2001 From: Brian de Alwis Date: Fri, 14 May 2021 14:27:42 -0400 Subject: [PATCH 3/5] update pydevd patch --- python/helper-image/pydevd.patch | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/helper-image/pydevd.patch b/python/helper-image/pydevd.patch index ed09f5ef..989fd3ea 100644 --- a/python/helper-image/pydevd.patch +++ b/python/helper-image/pydevd.patch @@ -11,7 +11,7 @@ index 2afae09..2985a35 100644 ArgHandlerBool('multiproc'), # Used by PyCharm (reuses connection: ssh tunneling) ArgHandlerBool('multiprocess'), # Used by PyDev (creates new connection to ide) diff --git pydevd.py pydevd.py -index 4639778..0da7e27 100644 +index 4639778..9ecfec0 100644 --- pydevd.py +++ pydevd.py @@ -1376,6 +1376,8 @@ class PyDB(object): @@ -19,7 +19,7 @@ index 4639778..0da7e27 100644 def run(self): host = SetupHolder.setup['client'] + if host is None: -+ host = 'localhost' ++ host = '' port = SetupHolder.setup['port'] self._server_socket = create_server_socket(host=host, port=port) From ef3a44a2cb83131a99cb6f3cb85cbe326351da73 Mon Sep 17 00:00:00 2001 From: Brian de Alwis Date: Mon, 17 May 2021 09:59:49 -0400 Subject: [PATCH 4/5] Add support for launching modules with pydev --- python/helper-image/launcher/launcher.go | 58 +++++++++++-- python/helper-image/launcher/launcher_test.go | 84 +++++++++++++++++++ 2 files changed, 136 insertions(+), 6 deletions(-) diff --git a/python/helper-image/launcher/launcher.go b/python/helper-image/launcher/launcher.go index 0f3b5a29..a8a5abe1 100644 --- a/python/helper-image/launcher/launcher.go +++ b/python/helper-image/launcher/launcher.go @@ -52,6 +52,7 @@ import ( "flag" "fmt" "io" + "io/ioutil" "os" "os/exec" "path/filepath" @@ -312,15 +313,19 @@ func (pc *pythonContext) updateCommandLine(ctx context.Context) error { if pc.env["WRAPPER_VERBOSE"] != "" { cmdline = append(cmdline, "--DEBUG") } - if pc.debugMode == ModePydevdPycharm { - // From the pydevd source, PyCharm wants multiproc - cmdline = append(cmdline, "--multiproc") - } if !pc.wait { cmdline = append(cmdline, "--continue") } - cmdline = append(cmdline, "--file") // --file is expected as last argument - cmdline = append(cmdline, pc.args[1:]...) + + // --file is expected as last pydev argument, but it must be a file, and so launching with + // a module requires some special handling. + cmdline = append(cmdline, "--file") + file, args, err := handlePydevModule(pc.args[1:]) + if err != nil { + return err + } + cmdline = append(cmdline, file) + cmdline = append(cmdline, args...) if pc.wait { logrus.Warn("pydevd does not support wait-for-client") } @@ -356,6 +361,47 @@ func determinePythonMajorMinor(ctx context.Context, launcherBin string, env env) return } +// handlePydevModule applies special pydevd handling for a python module. When a module is +// found, we write out a python script that uses runpy to invoke the module. +func handlePydevModule(args []string) (string, []string, error) { + switch { + case len(args) == 0: + return "", nil, fmt.Errorf("no python command-line specified") // shouldn't happen + case !strings.HasPrefix(args[0], "-"): + // this is a file + return args[0], args[1:], nil + case !strings.HasPrefix(args[0], "-m"): + // this is some other command-line flag + return "", nil, fmt.Errorf("expected python module: %q", args) + } + module := args[0][2:] + remaining := args[1:] + if module == "" { + if len(args) == 1 { + return "", nil, fmt.Errorf("missing python module: %q", args) + } + module = args[1] + remaining = args[2:] + } + + snippet := strings.ReplaceAll(`import sys +import runpy +runpy.run_module('{module}', run_name="__main__",alter_sys=True) +`, `{module}`, module) + + // write out the temp location as other locations may not be writable + d, err := ioutil.TempDir("", "pydevd*") + if err != nil { + return "", nil, err + } + // use a skaffold-specific file name to ensure no possibility of it matching a user import + f := filepath.Join(d, "skaffold_pydevd_launch.py") + if err := ioutil.WriteFile(f, []byte(snippet), 0755); err != nil { + return "", nil, err + } + return f, remaining, nil +} + func isEnabled(env env) bool { v, found := env["WRAPPER_ENABLED"] return !found || (v != "0" && v != "false" && v != "no") diff --git a/python/helper-image/launcher/launcher_test.go b/python/helper-image/launcher/launcher_test.go index 7803b527..2f72ef43 100644 --- a/python/helper-image/launcher/launcher_test.go +++ b/python/helper-image/launcher/launcher_test.go @@ -20,10 +20,12 @@ import ( "context" "fmt" "io/ioutil" + "os" "path/filepath" "testing" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" ) func TestValidateDebugMode(t *testing.T) { @@ -312,3 +314,85 @@ func TestPathExists(t *testing.T) { t.Error("pathExists failed on real path") } } + +func TestHandlePydevModule(t *testing.T) { + tmp := os.TempDir() + + tests := []struct { + description string + args []string + shouldErr bool + module string + file string + remaining []string + }{ + { + description: "plain file", + args: []string{"app.py"}, + file: "app.py", + }, + { + description: "-mmodule", + args: []string{"-mmodule"}, + file: filepath.Join(tmp, "*", "skaffold_pydevd_launch.py"), + }, + { + description: "-m module", + args: []string{"-m", "module"}, + file: filepath.Join(tmp, "*", "skaffold_pydevd_launch.py"), + }, + { + description: "- should error", + args: []string{"-", "module"}, + shouldErr: true, + }, + { + description: "-x should error", + args: []string{"-x", "module"}, + shouldErr: true, + }, + { + description: "lone -m should error", + args: []string{"-m"}, + shouldErr: true, + }, + { + description: "no args should error", + shouldErr: true, + }, + } + for _, test := range tests { + t.Run(test.description, func(t *testing.T) { + file, args, err := handlePydevModule(test.args) + if test.shouldErr { + if err == nil { + t.Error("Expected an error") + } + } else { + if !fileMatch(t, test.file, file) { + t.Errorf("Wanted %q but got %q", test.file, file) + } + if diff := cmp.Diff(args, test.remaining, cmpopts.EquateEmpty()); diff != "" { + t.Errorf("remaining args %T differ (-got, +want): %s", test.remaining, diff) + } + } + }) + } +} + +func fileMatch(t *testing.T, glob, file string) bool { + if file == glob { + return true + } + matches, err := filepath.Glob(glob) + if err != nil { + t.Errorf("Failed to expand globe %q: %v", glob, err) + return false + } + for _, m := range matches { + if file == m { + return true + } + } + return false +} From c7dff011069922a4d28900739bb5f463beccc0ad Mon Sep 17 00:00:00 2001 From: Brian de Alwis Date: Tue, 18 May 2021 12:44:36 -0400 Subject: [PATCH 5/5] add explanation of pydevd module handling process --- python/helper-image/launcher/launcher.go | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/python/helper-image/launcher/launcher.go b/python/helper-image/launcher/launcher.go index a8a5abe1..dfe4c70a 100644 --- a/python/helper-image/launcher/launcher.go +++ b/python/helper-image/launcher/launcher.go @@ -45,6 +45,25 @@ limitations under the License. // This launcher will determine the python executable based on the `original-command-line`. // The launcher will configure the PYTHONPATH to point to the appropriate installation // of pydevd/debugpy/ptvsd for the corresponding python binary. +// +// debugpy and ptvsd are pretty straightforward translations of the +// launcher command-line `python -m debugpy`. +// +// pydevd is more involved as pydevd does not support loading modules +// from the command-line (e.g., `python -m flask`). This launcher +// instead creates a small module-loader script using runpy. +// So `launcher --mode pydevd --port 5678 -- python -m flask app.py` +// will create a temp file named `skaffold_pydevd_launch.py`: +// ``` +// import sys +// import runpy +// runpy.run_module('flask', run_name="__main__",alter_sys=True) +// ``` +// and will then invoke: +// ``` +// python -m pydevd --server --port 5678 --DEBUG --continue \ +// --file /tmp/pydevd716531212/skaffold_pydevd_launch.py +// ``` package main import ( @@ -307,7 +326,6 @@ func (pc *pythonContext) updateCommandLine(ctx context.Context) error { case ModePydevd, ModePydevdPycharm: // Appropriate location to resolve pydevd is set in updateEnv - // TODO: check for modules (and fail?) cmdline = append(cmdline, pc.args[0]) cmdline = append(cmdline, "-m", "pydevd", "--server", "--port", strconv.Itoa(int(pc.port))) if pc.env["WRAPPER_VERBOSE"] != "" {