From d70d3ba717849673ada75874ff4c32cd224ab939 Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Wed, 12 Feb 2025 21:41:06 -0500 Subject: [PATCH 1/4] Fix deadlock on large output from `basilisp.process/exec` --- CHANGELOG.md | 4 ++-- src/basilisp/process.lpy | 9 +++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d8b3d86c..37be0c5c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,9 +5,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] - ### Fixed - * Fixed a regression introduced in #1176 where the testrunner couldn't handle relative paths in `sys.path`, causing `basilisp test` to fail when no arugments were provided (#1204) + * Fix a regression introduced in #1176 where the testrunner couldn't handle relative paths in `sys.path`, causing `basilisp test` to fail when no arugments were provided (#1204) + * Fix a bug where `basilisp.process/exec` could deadlock reading process output if that output exceeded the buffer size (#1202) ## [v0.3.6] ### Added diff --git a/src/basilisp/process.lpy b/src/basilisp/process.lpy index 35a75cfd..53ed40ff 100644 --- a/src/basilisp/process.lpy +++ b/src/basilisp/process.lpy @@ -273,11 +273,12 @@ If the return code is non-zero, throw :external:py:exc:`subprocess.CalledProcessError`." [& opts+args] - (let [process (apply start opts+args) - retcode (.wait process)] + (let [process (apply start opts+args) + [stdout _] (.communicate process) + retcode (.-returncode process)] (if (zero? retcode) - (if-let [out (.-stdout process)] - (slurp out) + (if stdout + stdout "") (throw (subprocess/CalledProcessError retcode From 738657370870016a2ad78edfbce993cd7465cdf2 Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Wed, 12 Feb 2025 21:47:36 -0500 Subject: [PATCH 2/4] Convert to string in all cases --- src/basilisp/process.lpy | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/basilisp/process.lpy b/src/basilisp/process.lpy index 53ed40ff..90142246 100644 --- a/src/basilisp/process.lpy +++ b/src/basilisp/process.lpy @@ -265,7 +265,7 @@ (defn exec "Execute a command as by :lpy:fn:`start` and, upon successful return, return the - captured value of the process ``stdout`` as by :lpy:fn:`basilisp.core/slurp`. + captured value of the process ``stdout`` as if by :lpy:fn:`basilisp.core/slurp`. If ``opts`` are specified, they should be provided as a map in the first argument position. ``opts`` are exactly the same as those in :lpy:fn:`start`. @@ -277,9 +277,10 @@ [stdout _] (.communicate process) retcode (.-returncode process)] (if (zero? retcode) - (if stdout - stdout - "") + (cond + (byte-string? stdout) (.decode stdout "utf-8") + stdout stdout + :else "") (throw (subprocess/CalledProcessError retcode (.-args process) From 905ed5359b8d36eb10eec61080fe97b52e2ec4f6 Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Thu, 13 Feb 2025 17:41:28 -0500 Subject: [PATCH 3/4] Bunch of CI things --- src/basilisp/cli.py | 2 +- src/basilisp/lang/compiler/__init__.py | 5 ++++- src/basilisp/lang/compiler/analyzer.py | 16 ++++++++++++-- src/basilisp/lang/compiler/generator.py | 28 ++++++++++++++++++------- src/basilisp/lang/runtime.py | 2 +- 5 files changed, 41 insertions(+), 12 deletions(-) diff --git a/src/basilisp/cli.py b/src/basilisp/cli.py index 7015567e..b25e9bc7 100644 --- a/src/basilisp/cli.py +++ b/src/basilisp/cli.py @@ -398,7 +398,7 @@ def _subcommand( Callable[["argparse._SubParsersAction"], None], ]: def _wrap_add_subcommand( - f: Callable[[argparse.ArgumentParser], None] + f: Callable[[argparse.ArgumentParser], None], ) -> Callable[["argparse._SubParsersAction"], None]: def _wrapped_subcommand(subparsers: "argparse._SubParsersAction"): parser = subparsers.add_parser( diff --git a/src/basilisp/lang/compiler/__init__.py b/src/basilisp/lang/compiler/__init__.py index 7c5fcc96..1ca6615a 100644 --- a/src/basilisp/lang/compiler/__init__.py +++ b/src/basilisp/lang/compiler/__init__.py @@ -33,7 +33,10 @@ GeneratorContext, ) from basilisp.lang.compiler.generator import expressionize as _expressionize # noqa -from basilisp.lang.compiler.generator import gen_py_ast, py_module_preamble +from basilisp.lang.compiler.generator import ( + gen_py_ast, + py_module_preamble, +) from basilisp.lang.compiler.generator import statementize as _statementize from basilisp.lang.compiler.optimizer import PythonASTOptimizer from basilisp.lang.interfaces import ISeq diff --git a/src/basilisp/lang/compiler/analyzer.py b/src/basilisp/lang/compiler/analyzer.py index 1ff3a86f..956df14c 100644 --- a/src/basilisp/lang/compiler/analyzer.py +++ b/src/basilisp/lang/compiler/analyzer.py @@ -117,9 +117,21 @@ PyTuple, ) from basilisp.lang.compiler.nodes import Queue as QueueNode -from basilisp.lang.compiler.nodes import Quote, Recur, Reify, Require, RequireAlias +from basilisp.lang.compiler.nodes import ( + Quote, + Recur, + Reify, + Require, + RequireAlias, +) from basilisp.lang.compiler.nodes import Set as SetNode -from basilisp.lang.compiler.nodes import SetBang, SpecialFormNode, Throw, Try, VarRef +from basilisp.lang.compiler.nodes import ( + SetBang, + SpecialFormNode, + Throw, + Try, + VarRef, +) from basilisp.lang.compiler.nodes import Vector as VectorNode from basilisp.lang.compiler.nodes import ( WithMeta, diff --git a/src/basilisp/lang/compiler/generator.py b/src/basilisp/lang/compiler/generator.py index 26b128b3..87a4a184 100644 --- a/src/basilisp/lang/compiler/generator.py +++ b/src/basilisp/lang/compiler/generator.py @@ -86,11 +86,25 @@ PyTuple, ) from basilisp.lang.compiler.nodes import Queue as QueueNode -from basilisp.lang.compiler.nodes import Quote, Recur, Reify, Require +from basilisp.lang.compiler.nodes import ( + Quote, + Recur, + Reify, + Require, +) from basilisp.lang.compiler.nodes import Set as SetNode -from basilisp.lang.compiler.nodes import SetBang, T_withmeta, Throw, Try, VarRef +from basilisp.lang.compiler.nodes import ( + SetBang, + T_withmeta, + Throw, + Try, + VarRef, +) from basilisp.lang.compiler.nodes import Vector as VectorNode -from basilisp.lang.compiler.nodes import WithMeta, Yield +from basilisp.lang.compiler.nodes import ( + WithMeta, + Yield, +) from basilisp.lang.compiler.utils import ( ast_AsyncFunctionDef, ast_ClassDef, @@ -392,7 +406,7 @@ def attr_node(node, idx): def _simple_ast_generator( - gen_ast: Callable[P_simplegen, T_pynode] + gen_ast: Callable[P_simplegen, T_pynode], ) -> Callable[P_simplegen, GeneratedPyAST[T_pynode]]: """Wrap simpler AST generators to return a GeneratedPyAST.""" @@ -1737,7 +1751,7 @@ def __fn_meta( def __kwargs_support_decorator( - node: Union[Fn, DefTypeMethodArity, DefTypeClassMethod, DefTypeStaticMethod] + node: Union[Fn, DefTypeMethodArity, DefTypeClassMethod, DefTypeStaticMethod], ) -> Iterable[ast.expr]: if node.kwarg_support is None: return @@ -3533,8 +3547,8 @@ def _py_tuple_to_py_ast( def _with_meta_to_py_ast( ctx: GeneratorContext, node: WithMeta[T_withmeta], - *args: P_generator.args, - **kwargs: P_generator.kwargs, + *args, + **kwargs, ) -> GeneratedPyAST[ast.expr]: """Generate a Python AST node for Python interop method calls.""" assert node.op == NodeOp.WITH_META diff --git a/src/basilisp/lang/runtime.py b/src/basilisp/lang/runtime.py index b8526b9a..189513b9 100644 --- a/src/basilisp/lang/runtime.py +++ b/src/basilisp/lang/runtime.py @@ -2098,7 +2098,7 @@ def wrapped_f(*args, **kwargs): def _basilisp_fn( - arities: tuple[Union[int, kw.Keyword], ...] + arities: tuple[Union[int, kw.Keyword], ...], ) -> Callable[..., BasilispFunction]: """Create a Basilisp function, setting meta and supplying a with_meta method implementation.""" From 96046fe21476524a5cf83bff1254de97b46d066f Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Fri, 14 Feb 2025 08:38:35 -0500 Subject: [PATCH 4/4] Linesep --- tests/basilisp/test_process.lpy | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/tests/basilisp/test_process.lpy b/tests/basilisp/test_process.lpy index 70584358..0ab52d6e 100644 --- a/tests/basilisp/test_process.lpy +++ b/tests/basilisp/test_process.lpy @@ -1,5 +1,6 @@ (ns tests.basilisp.test-process - (:import pathlib + (:import os + pathlib subprocess sys) (:require @@ -141,10 +142,12 @@ (is (= "" (process/exec sys/executable "-c" "pass"))) (is (= "" (process/exec {:out :inherit} sys/executable "-c" "print(\"hi\")"))) (is (= "" (process/exec sys/executable "-c" "import sys; print(\"hi\", file=sys.stderr)"))) - (is (= "hi\n" (process/exec sys/executable "-c" "print(\"hi\")"))) + (is (= (str "hi" os/linesep) + (process/exec sys/executable "-c" "print(\"hi\")"))) (is (thrown? subprocess/CalledProcessError (process/exec sys/executable "-c" "import sys; sys.exit(2)"))) - (is (= "BASILISP\n" (process/exec {:env {"PYTHON_HOSTED_LANG" "BASILISP"}} - sys/executable - "-c" - "import os; print(os.environ[\"PYTHON_HOSTED_LANG\"])")))) + (is (= (str "BASILISP" os/linesep) + (process/exec {:env {"PYTHON_HOSTED_LANG" "BASILISP"}} + sys/executable + "-c" + "import os; print(os.environ[\"PYTHON_HOSTED_LANG\"])"))))