From 64f34cff2789b31c3745b8c98b574f341c627ccc Mon Sep 17 00:00:00 2001 From: Tian Gao Date: Sun, 12 May 2024 16:43:08 -0700 Subject: [PATCH 1/5] Fix patching when shell=True in subprocess --- src/viztracer/patch.py | 3 +++ tests/test_multiprocess.py | 24 ++++++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/src/viztracer/patch.py b/src/viztracer/patch.py index 22835ac6..62065c86 100644 --- a/src/viztracer/patch.py +++ b/src/viztracer/patch.py @@ -70,6 +70,9 @@ def subprocess_init(self: subprocess.Popen[Any], args: Union[str, Sequence[Any], if isinstance(new_args, Sequence): if "python" in os.path.basename(new_args[0]): new_args = build_command(new_args) + if new_args is not None and kwargs.get("shell"): + # For shell=True, we have to pass a string + new_args = " ".join(new_args) else: new_args = None diff --git a/tests/test_multiprocess.py b/tests/test_multiprocess.py index b134c08b..3defde9f 100644 --- a/tests/test_multiprocess.py +++ b/tests/test_multiprocess.py @@ -51,6 +51,15 @@ def fib(n): p.wait() """ +file_subprocess_shell = """ +import subprocess +import os +with open(os.path.join(os.path.dirname(__file__), "sub.py"), "w") as f: + f.write("print('hello')") +path = os.path.join(os.path.dirname(__file__), "sub.py") +print(subprocess.call(f"python {path}", shell=True)) +""" + file_fork = """ import os import time @@ -295,6 +304,21 @@ def test_code_string(self): with open(os.path.join(tmpdir, os.listdir(tmpdir)[0])) as f: self.assertSubprocessName("python -c", json.load(f)) + def test_subprocess_shell_true(self): + def check_func(data): + pids = set() + for entry in data["traceEvents"]: + pids.add(entry["pid"]) + self.assertEqual(len(pids), 2) + + with tempfile.TemporaryDirectory() as tmpdir: + output_path = os.path.join(tmpdir, "result.json") + self.template(["viztracer", "-o", output_path, "cmdline_test.py"], + expected_output_file=output_path, + script=file_subprocess_shell, + expected_stdout=".*hello.*", + check_func=check_func) + def test_nested(self): def check_func(data): pids = set() From a7a03c8573e46d92659077bab692ac8b52acf260 Mon Sep 17 00:00:00 2001 From: Tian Gao Date: Sun, 12 May 2024 17:02:48 -0700 Subject: [PATCH 2/5] Fix windows --- src/viztracer/patch.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/viztracer/patch.py b/src/viztracer/patch.py index 62065c86..06183dd2 100644 --- a/src/viztracer/patch.py +++ b/src/viztracer/patch.py @@ -70,8 +70,10 @@ def subprocess_init(self: subprocess.Popen[Any], args: Union[str, Sequence[Any], if isinstance(new_args, Sequence): if "python" in os.path.basename(new_args[0]): new_args = build_command(new_args) - if new_args is not None and kwargs.get("shell"): - # For shell=True, we have to pass a string + if new_args is not None and kwargs.get("shell") and isinstance(args, str): + # For shell=True, we should convert the commands back to string + # if it was passed as string + # This is mostly for Unix shell new_args = " ".join(new_args) else: new_args = None From ad46bb2bb12d0171d0a620481b5c2532565b232a Mon Sep 17 00:00:00 2001 From: Tian Gao Date: Sun, 12 May 2024 17:44:08 -0700 Subject: [PATCH 3/5] Fix shlex --- src/viztracer/patch.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/viztracer/patch.py b/src/viztracer/patch.py index 06183dd2..394b1b3d 100644 --- a/src/viztracer/patch.py +++ b/src/viztracer/patch.py @@ -66,7 +66,7 @@ def build_command(args: Sequence[str]) -> list[str] | None: def subprocess_init(self: subprocess.Popen[Any], args: Union[str, Sequence[Any], Any], **kwargs: Any) -> None: new_args = args if isinstance(new_args, str): - new_args = shlex.split(new_args) + new_args = shlex.split(new_args, posix=sys.platform != "win32") if isinstance(new_args, Sequence): if "python" in os.path.basename(new_args[0]): new_args = build_command(new_args) From edbef1fd3a575738849408a0e9287dd726d9028e Mon Sep 17 00:00:00 2001 From: Tian Gao Date: Sun, 12 May 2024 22:08:13 -0700 Subject: [PATCH 4/5] Set macos deployment target --- .github/workflows/wheels.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/wheels.yml b/.github/workflows/wheels.yml index 8ba7469b..993da144 100644 --- a/.github/workflows/wheels.yml +++ b/.github/workflows/wheels.yml @@ -81,3 +81,4 @@ jobs: run: python -m cibuildwheel --output-dir wheelhouse env: CIBW_BUILD: 'cp${{ matrix.python-version }}-*' + CIBW_ENVIRONMENT_MACOS: MACOSX_DEPLOYMENT_TARGET=10.15 From 7c18f4d12cfd5fb836dfacd9a03f8d17826e0a66 Mon Sep 17 00:00:00 2001 From: Tian Gao Date: Sun, 12 May 2024 22:44:54 -0700 Subject: [PATCH 5/5] Add env for release --- .github/workflows/release.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index b89d1931..4d89d0e0 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -85,6 +85,7 @@ jobs: run: python -m cibuildwheel --output-dir wheelhouse env: CIBW_BUILD: 'cp${{ matrix.python-version }}-*' + CIBW_ENVIRONMENT_MACOS: MACOSX_DEPLOYMENT_TARGET=10.15 - name: Publish wheels to PyPI Unix if: matrix.os != 'windows-latest'