From 8afdef61c18d0b9ed6387424d690bcf38a4fc25b Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Thu, 21 Nov 2024 13:01:12 -0500 Subject: [PATCH 01/11] Add the `basilisp.process` namespace --- CHANGELOG.md | 1 + docs/api/process.rst | 10 +++ src/basilisp/process.lpy | 139 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 150 insertions(+) create mode 100644 docs/api/process.rst create mode 100644 src/basilisp/process.lpy diff --git a/CHANGELOG.md b/CHANGELOG.md index e77261f4..d3335721 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Added * Added support for a subset of qualified method syntax introduced in Clojure 1.12 (#1109) + * Added the `basilisp.process` namespace (#1108) ### Changed * The Custom Data Readers Loader will only now examine the top directory and up to its immediate subdirectories of each `sys.path` entry, instead of recursive descending into every subdirectory, improving start up performance (#1135) diff --git a/docs/api/process.rst b/docs/api/process.rst new file mode 100644 index 00000000..db05619f --- /dev/null +++ b/docs/api/process.rst @@ -0,0 +1,10 @@ +basilisp.process +================ + +.. toctree:: + :maxdepth: 2 + :caption: Contents: + +.. autonamespace:: basilisp.process + :members: + :undoc-members: \ No newline at end of file diff --git a/src/basilisp/process.lpy b/src/basilisp/process.lpy new file mode 100644 index 00000000..47818b95 --- /dev/null +++ b/src/basilisp/process.lpy @@ -0,0 +1,139 @@ +(ns basilisp.process + (:import os + pathlib + subprocess)) + +(defn start + "Start an external command as ``args``. + + If ``opts`` are specified, they should be provided as a map in the first argument + position. + + The following options are available: + + :keyword ``:in``: an existing file object or file handle, ``:pipe`` to generate a + new stream, or ``:inherit`` to use the current process stdin; if not specified + ``:pipe`` will be used + :keyword ``:out``: an existing file object or file handle, ``:pipe`` to generate a + new stream, ``:discard`` to ignore stdout, or ``:inherit`` to use the current + process stdout; if not specified, ``:pipe`` will be used + :keyword ``:err``: an existing file object or file handle, ``:pipe`` to generate a + new stream, ``:discard`` to ignore stderr, ``:stdout`` to merge stdout and + stderr, or ``:inherit`` to use the current process stderr; if not specified, + ``:pipe`` will be used + :keyword ``:dir``: current directory when the process runs; on POSIX systems, if + executable is a relative path, it will be resolved relative to this value; + if not specified, the current directory will be used + :keyword ``:clear-env``: boolean which if ``true`` will prevent inheriting the + environment variables from the current process + :keyword ``:env``: a mapping of string values to string values which are added to + the subprocess environment; if ``:clear-env``, these are the only environment + variables provided to the subprocess + :keyword ``:encoding``: the string name of an encoding to use for input and output + streams; if not specified, streams are treated as bytes + + Returns :external:py:class:`subprocess.Popen` instance." + [& opts+args] + (let [[opts command] (if (map? (first opts+args)) + [(first opts+args) (rest opts+args)] + [nil opts+args]) + + {:keys [in out err dir encoding] + :or {in :pipe out :pipe err :pipe dir "."}} opts + + stdin (condp = in + :pipe subprocess/PIPE + :inherit nil + in) + stdout (condp = out + :pipe subprocess/PIPE + :inherit nil + :discard subprocess/DEVNULL + out) + stderr (condp = err + :pipe subprocess/PIPE + :discard subprocess/DEVNULL + :stdout subprocess/STDOUT + :inherit nil + err) + env (if (:clear-env opts) + (:env opts) + (into (py->lisp os/environ) (:env opts)))] + (subprocess/Popen (python/list command) + ** + :encoding encoding + :stdin stdin + :stdout stdout + :stderr stderr + :cwd (-> (pathlib/Path dir) (.resolve)) + :env (lisp->py env)))) + +(defn exec + "Execute a command as by :lpy:fn:`start` and, upon successful return, return the + captured value of the process ``stdout`` (which may be a string or bytes depending + on whether the process was opened in text or binary mode). + + 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`. + + If the return code is non-zero, throw + :external:py:exc:`subprocess.CalledProcessError`." + [& opts+args] + (let [process (apply start opts+args) + retcode (.wait process)] + (if (zero? retcode) + (slurp (.-stdout process)) + (throw + (subprocess/CalledProcessError retcode + (.-args process) + (.-stdout process) + (.-stderr process)))))) + +(defn stderr + "Return the ``stderr`` stream from the external process. + + .. warning:: + + Communication directly with the process streams introduces the possibility of + deadlocks. Users may use :lpy:fn:`communicate` as a safe alternative." + [process] + (.-stderr process)) + +(defn stdin + "Return the ``stdin`` stream from the external process. + + .. warning:: + + Communication directly with the process streams introduces the possibility of + deadlocks. Users may use :lpy:fn:`communicate` as a safe alternative." + [process] + (.-stdin process)) + +(defn stdout + "Return the ``stdout`` stream from the external process. + + .. warning:: + + Communication directly with the process streams introduces the possibility of + deadlocks. Users may use :lpy:fn:`communicate` as a safe alternative." + [process] + (.-stdout process)) + +(defn communicate + "Communicate with a subprocess, optionally sending data to the process stdin stream + and reading any data in the process stderr and stdout streams, returning them as + a string or bytes object (depending on whether the process was opened in text or + binary mode). + + This function is preferred over the use of :lpy:fn:`stderr`, :lpy:fn:`stdin`, and + :lpy:fn:`stdout` to avoid potential deadlocks. + + The following keyword/value arguments are optional: + + :keyword ``:input``: a string or bytes object (depending on whether the process + was opened in text or bniary mode); if omitted, do not send anything + :keyword ``timeout``: an optional timeout" + [process & kwargs] + (let [kwargs (apply hash-map kwargs)] + (vec + (.communicate process ** :input (:input kwargs) :timeout (:timeout kwargs))))) From 97cc6f15a582785f3c428c6a25c888dbe7c14cb7 Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Fri, 22 Nov 2024 13:10:07 -0500 Subject: [PATCH 02/11] More stuff --- src/basilisp/process.lpy | 166 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 156 insertions(+), 10 deletions(-) diff --git a/src/basilisp/process.lpy b/src/basilisp/process.lpy index 47818b95..74bf4892 100644 --- a/src/basilisp/process.lpy +++ b/src/basilisp/process.lpy @@ -1,8 +1,138 @@ (ns basilisp.process - (:import os + (:require + [basilisp.io :as bio] + [basilisp.string :as str]) + (:import contextlib + os pathlib subprocess)) +(defprotocol SubprocessRedirectable + (is-file-like? [f] + "Return true if ``f`` is a file-like object: either an integer (assumed to be a + file handle or a file-like object with a ``.fileno()`` method). + + This function says nothing about whether or not it is a valid file object or handle.") + (is-path-like? [o] + "Return true if ``o`` is a path-like object: either a string, + :external:py:class:`pathlib.Path`, or byte string.")) + +(extend-protocol SubprocessRedirectable + python/object + (is-file-like? [f] + (and (python/hasattr f "fileno") + (python/callable (.-fileno f)))) + (is-path-like? [o] + false) + + python/int + (is-file-like? [_] + true) + (is-path-like? [_] + false) + + python/str + (is-file-like? [_] + false) + (is-path-like? [_] + true) + + python/bytes + (is-file-like? [_] + false) + (is-path-like? [_] + true) + + pathlib/Path + (is-file-like? [_] + false) + (is-path-like? [_] + true)) + +(defrecord FileWrapper [path opts] + SubprocessRedirectable + (is-file-like? [self] + false) + (is-path-like? [self] + true)) + +(defn from-file + "Return a file object suitable for use as the ``:in`` option for :lpy:fn:`start`. + + Callers can specify additional ``opts``. Anything supported by + :lpy:fn:`basilisp.io/writer` and :lpy:fn:`basilisp.io/output-stream` is generally + supported here. + + .. warning:: + + ``opts`` may only be specified for path-like values. Providing options for + existing file objects and file handles will raise an exception." + [f & {:as opts}] + (cond + (is-file-like? f) (if opts + (throw + (ex-info "Cannot specify options for an open file" + {:file f :opts opts})) + f) + (is-path-like? f) (do + (when (str/includes? (:mode opts "") "r") + (throw + (ex-info "Cannot specify :in file in read mode" + {:file f :opts opts}))) + (->FileWrapper f opts)) + :else (throw + (ex-info "Expected a file-like or path-like object" + {:file f :opts opts})))) + +(defn to-file + "Return a file object suitable for use as the ``:err`` and ``:out`` options for + :lpy:fn:`start`. + + Callers can specify additional ``opts``. Anything supported by + :lpy:fn:`basilisp.io/reader` and :lpy:fn:`basilisp.io/input-stream` is generally + supported here. + + .. warning:: + + ``opts`` may only be specified for path-like values. Providing options for + existing file objects and file handles will raise an exception." + [f & {:as opts}] + (cond + (is-file-like? f) (if opts + (throw + (ex-info "Cannot specify options for an open file" + {:file f :opts opts})) + f) + (is-path-like? f) (do + (when (str/includes? (:mode opts "") "w") + (throw + (ex-info "Cannot specify :out or :err file in write mode" + {:file f :opts opts}))) + (->FileWrapper f opts)) + :else (throw + (ex-info "Expected a file-like or path-like object" + {:file f :opts opts})))) + +(defn ^:private wrapped-file-context-manager + "Wrap a potential file in a context manager for ``start``. + + Existing file-objects we just pass through using a null context manager, but + path-likes need to be opened with a context manager." + [f is-writer?] + (if (is-path-like? f) + (let [path (:path f f) + opts (:opts f) + is-binary? (str/includes? (:mode opts "") "b") + io-fn (if is-binary? + (if is-writer? + bio/output-stream + bio/input-stream) + (if is-writer? + bio/writer + bio/reader))] + (apply io-fn path opts)) + (contextlib/nullcontext f))) + (defn start "Start an external command as ``args``. @@ -29,8 +159,18 @@ :keyword ``:env``: a mapping of string values to string values which are added to the subprocess environment; if ``:clear-env``, these are the only environment variables provided to the subprocess + + The following options affect the pipe streams created by + :external:py:class:`subprocess.Popen` (if ``:pipe`` is selected), but do not apply + to any files wrapped by :lpy:fn:`from-file` or :lpy:fn:`to-file` (in which case the + options provided for those files take precedence) or if ``:inherit`` is specified + (in which case the options for the corresponding stream in the current process is + used). These options are specific to Basilisp. + :keyword ``:encoding``: the string name of an encoding to use for input and output - streams; if not specified, streams are treated as bytes + streams when ``:pipe`` is specified for any of the standard streams; this option + does not apply to any files wrapped by :lpy:fn:`from-file` or :lpy:fn:`to-file` + or if ``:inherit`` is specified; if not specified, streams are treated as bytes Returns :external:py:class:`subprocess.Popen` instance." [& opts+args] @@ -59,14 +199,20 @@ env (if (:clear-env opts) (:env opts) (into (py->lisp os/environ) (:env opts)))] - (subprocess/Popen (python/list command) - ** - :encoding encoding - :stdin stdin - :stdout stdout - :stderr stderr - :cwd (-> (pathlib/Path dir) (.resolve)) - :env (lisp->py env)))) + ;; Conditionally open files here if we're given a path-like since Python does + ;; not offer a `File` or `ProcessBuilder.Redirect` like object to handle this + ;; logic. + (with [stdin (wrapped-file-context-manager stdin true) + stdout (wrapped-file-context-manager stdout false) + stderr (wrapped-file-context-manager stderr false)] + (subprocess/Popen (python/list command) + ** + :encoding encoding + :stdin stdin + :stdout stdout + :stderr stderr + :cwd (-> (pathlib/Path dir) (.resolve)) + :env (lisp->py env))))) (defn exec "Execute a command as by :lpy:fn:`start` and, upon successful return, return the From 7de262c1745c49aec0b7ad8669521209440a2c77 Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Fri, 22 Nov 2024 14:04:23 -0500 Subject: [PATCH 03/11] exit-ref --- src/basilisp/process.lpy | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/basilisp/process.lpy b/src/basilisp/process.lpy index 74bf4892..d27f7403 100644 --- a/src/basilisp/process.lpy +++ b/src/basilisp/process.lpy @@ -113,6 +113,22 @@ (ex-info "Expected a file-like or path-like object" {:file f :opts opts})))) +(defn exit-ref + "Given a :external:py:class:`subprocess.Popen` (such as the one returned by + :lpy:fn:`start`), return a reference which can be used to wait (optionally with + timeout) for the completion of the process." + [process] + (reify + basilisp.lang.interfaces/IBlockingDeref + (deref [_ & args] + (let [[timeout-ms timeout-val] args] + (if timeout-ms + (try + (.wait process ** :timeout (/ timeout-ms 1000)) + (catch subprocess/TimeoutExpired _ + timeout-val)) + (.wait process)))))) + (defn ^:private wrapped-file-context-manager "Wrap a potential file in a context manager for ``start``. From a1822c16184cc8c0053b44a72c4da2be5d46087e Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Mon, 25 Nov 2024 11:03:04 -0500 Subject: [PATCH 04/11] Light testing --- docs/api/process.rst | 3 ++- src/basilisp/process.lpy | 5 ++--- tests/basilisp/test_process.lpy | 37 +++++++++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 4 deletions(-) create mode 100644 tests/basilisp/test_process.lpy diff --git a/docs/api/process.rst b/docs/api/process.rst index db05619f..7efb2611 100644 --- a/docs/api/process.rst +++ b/docs/api/process.rst @@ -7,4 +7,5 @@ basilisp.process .. autonamespace:: basilisp.process :members: - :undoc-members: \ No newline at end of file + :undoc-members: + :exclude-members: FileWrapper, SubprocessRedirectable, ->FileWrapper, is-file-like?, is-path-like? \ No newline at end of file diff --git a/src/basilisp/process.lpy b/src/basilisp/process.lpy index d27f7403..d6ff724a 100644 --- a/src/basilisp/process.lpy +++ b/src/basilisp/process.lpy @@ -232,8 +232,7 @@ (defn exec "Execute a command as by :lpy:fn:`start` and, upon successful return, return the - captured value of the process ``stdout`` (which may be a string or bytes depending - on whether the process was opened in text or binary mode). + captured value of the process ``stdout`` as 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`. @@ -293,7 +292,7 @@ The following keyword/value arguments are optional: :keyword ``:input``: a string or bytes object (depending on whether the process - was opened in text or bniary mode); if omitted, do not send anything + was opened in text or binary mode); if omitted, do not send anything :keyword ``timeout``: an optional timeout" [process & kwargs] (let [kwargs (apply hash-map kwargs)] diff --git a/tests/basilisp/test_process.lpy b/tests/basilisp/test_process.lpy new file mode 100644 index 00000000..c947bd21 --- /dev/null +++ b/tests/basilisp/test_process.lpy @@ -0,0 +1,37 @@ +(ns tests.basilisp.test-process + (:import pathlib) + (:require + [basilisp.process :as process] + [basilisp.test :as test :refer [deftest is are testing]] + [basilisp.test.fixtures :as fixtures :refer [*tempdir*]])) + +(test/use-fixtures :each fixtures/tempdir) + +(deftest is-file-like?-test + (are [v] (true? (process/is-file-like? v)) + -1 + 0 + 1) + + (with-open [f (python/open (pathlib/Path *tempdir* "is-file-like.txt") ** :mode "w")] + (is (true? (process/is-file-like? f)))) + + (are [v] (false? (process/is-file-like? v)) + "i'm a path-like" + #b "i'm a path-like" + (pathlib/Path "/home/chris"))) + +(deftest is-path-like?-test + (are [v] (true? (process/is-path-like? v)) + "i'm a path-like" + #b "i'm a path-like" + (pathlib/Path "/home/chris")) + + (with-open [f (python/open (pathlib/Path *tempdir* "is-path-like.txt") ** :mode "w")] + (is (false? (process/is-path-like? f)))) + + (are [v] (false? (process/is-path-like? v)) + nil + 32 + 32.1 + (python/object))) From 86c9b9c7f42e2ea4fc0df8a9d068f57c6e3a520e Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Mon, 25 Nov 2024 11:40:21 -0500 Subject: [PATCH 05/11] more tests --- tests/basilisp/test_process.lpy | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/basilisp/test_process.lpy b/tests/basilisp/test_process.lpy index c947bd21..de5821f5 100644 --- a/tests/basilisp/test_process.lpy +++ b/tests/basilisp/test_process.lpy @@ -35,3 +35,33 @@ 32 32.1 (python/object))) + +(deftest from-file-test + (is (= 3 (process/from-file 3))) + (is (thrown? basilisp.lang.exception/ExceptionInfo + (process/from-file 3 :encoding "utf-8"))) + + (is (= (process/->FileWrapper "/home/chris" {:encoding "utf-8"}) + (process/from-file "/home/chris" :encoding "utf-8"))) + (is (= (process/->FileWrapper "/home/chris" {:mode "w"}) + (process/from-file "/home/chris" :mode "w"))) + (is (thrown? basilisp.lang.exception/ExceptionInfo + (process/from-file "/home/chris" :mode "r"))) + + (is (thrown? basilisp.lang.exception/ExceptionInfo + (process/from-file nil)))) + +(deftest to-file-test + (is (= 3 (process/to-file 3))) + (is (thrown? basilisp.lang.exception/ExceptionInfo + (process/to-file 3 :encoding "utf-8"))) + + (is (= (process/->FileWrapper "/home/chris" {:encoding "utf-8"}) + (process/to-file "/home/chris" :encoding "utf-8"))) + (is (= (process/->FileWrapper "/home/chris" {:mode "r"}) + (process/to-file "/home/chris" :mode "r"))) + (is (thrown? basilisp.lang.exception/ExceptionInfo + (process/to-file "/home/chris" :mode "w"))) + + (is (thrown? basilisp.lang.exception/ExceptionInfo + (process/to-file nil)))) From 207450a4058d62611722317ba5e56f73554d2287 Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Mon, 25 Nov 2024 12:11:06 -0500 Subject: [PATCH 06/11] exit-ref test --- src/basilisp/process.lpy | 13 ++++++++----- tests/basilisp/test_process.lpy | 7 +++++++ 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/basilisp/process.lpy b/src/basilisp/process.lpy index d6ff724a..51c742a3 100644 --- a/src/basilisp/process.lpy +++ b/src/basilisp/process.lpy @@ -61,7 +61,8 @@ Callers can specify additional ``opts``. Anything supported by :lpy:fn:`basilisp.io/writer` and :lpy:fn:`basilisp.io/output-stream` is generally - supported here. + supported here. The values of individual ``opts`` are not validated until a call to + :lpy:fn:`start` or :lpy:fn:`exec`. .. warning:: @@ -90,7 +91,8 @@ Callers can specify additional ``opts``. Anything supported by :lpy:fn:`basilisp.io/reader` and :lpy:fn:`basilisp.io/input-stream` is generally - supported here. + supported here. The values of individual ``opts`` are not validated until a call + to :lpy:fn:`start` or :lpy:fn:`exec`. .. warning:: @@ -121,10 +123,11 @@ (reify basilisp.lang.interfaces/IBlockingDeref (deref [_ & args] - (let [[timeout-ms timeout-val] args] - (if timeout-ms + ;; basilisp.lang.runtime.deref converts Clojure's ms into seconds for Python + (let [[timeout-s timeout-val] args] + (if timeout-s (try - (.wait process ** :timeout (/ timeout-ms 1000)) + (.wait process ** :timeout timeout-s) (catch subprocess/TimeoutExpired _ timeout-val)) (.wait process)))))) diff --git a/tests/basilisp/test_process.lpy b/tests/basilisp/test_process.lpy index de5821f5..f6a7f110 100644 --- a/tests/basilisp/test_process.lpy +++ b/tests/basilisp/test_process.lpy @@ -65,3 +65,10 @@ (is (thrown? basilisp.lang.exception/ExceptionInfo (process/to-file nil)))) + +#?(:windows nil + :default + (deftest exit-ref-test + (= 0 @(process/exit-ref (process/start "sleep" "1"))) + (= 0 (deref (process/exit-ref (process/start "sleep" "1")) 5000 :timed-out)) + (= :timed-out (deref (process/exit-ref (process/start "sleep" "10")) 1000 :timed-out)))) From 4b145a7db9303d0dd8bf83d0d4fbe92ccadb05d5 Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Mon, 25 Nov 2024 13:50:11 -0500 Subject: [PATCH 07/11] Safe subprocess --- tests/basilisp/test_process.lpy | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/tests/basilisp/test_process.lpy b/tests/basilisp/test_process.lpy index f6a7f110..a096af6e 100644 --- a/tests/basilisp/test_process.lpy +++ b/tests/basilisp/test_process.lpy @@ -1,5 +1,7 @@ (ns tests.basilisp.test-process - (:import pathlib) + (:import pathlib + subprocess + sys) (:require [basilisp.process :as process] [basilisp.test :as test :refer [deftest is are testing]] @@ -66,9 +68,17 @@ (is (thrown? basilisp.lang.exception/ExceptionInfo (process/to-file nil)))) -#?(:windows nil - :default - (deftest exit-ref-test - (= 0 @(process/exit-ref (process/start "sleep" "1"))) - (= 0 (deref (process/exit-ref (process/start "sleep" "1")) 5000 :timed-out)) - (= :timed-out (deref (process/exit-ref (process/start "sleep" "10")) 1000 :timed-out)))) +(deftest exit-ref-test + (= 0 @(process/exit-ref (process/start sys/executable "-c" "import time; time.sleep(1)"))) + (= 0 (-> (process/start sys/executable "-c" "import time; time.sleep(1)") + (process/exit-ref ) + (deref 5000 :timed-out))) + (= :timed-out (-> (process/start sys/executable "-c" "import time; time.sleep(10)") + (process/exit-ref ) + (deref 1000 :timed-out)))) + +(deftest exec-test + (is (= "" (process/exec sys/executable "-c" "pass"))) + (is (= "hi\n" (process/exec sys/executable "-c" "print(\"hi\")"))) + (is (thrown? subprocess/CalledProcessError + (process/exec sys/executable "-c" "import sys; sys.exit(2)")))) From 440d2ec2a3f6fdac44826de105a3a9be7f9a7812 Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Mon, 25 Nov 2024 14:31:42 -0500 Subject: [PATCH 08/11] Fixes --- src/basilisp/process.lpy | 9 ++++++--- tests/basilisp/test_process.lpy | 6 +++++- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/basilisp/process.lpy b/src/basilisp/process.lpy index 51c742a3..369d8dc7 100644 --- a/src/basilisp/process.lpy +++ b/src/basilisp/process.lpy @@ -149,7 +149,8 @@ (if is-writer? bio/writer bio/reader))] - (apply io-fn path opts)) + (->> (mapcat identity opts) + (apply io-fn path))) (contextlib/nullcontext f))) (defn start @@ -216,8 +217,10 @@ :inherit nil err) env (if (:clear-env opts) - (:env opts) - (into (py->lisp os/environ) (:env opts)))] + (:env opts {}) + (-> (python/dict os/environ) + (py->lisp {:keywordize-keys false}) + (into (:env opts))))] ;; Conditionally open files here if we're given a path-like since Python does ;; not offer a `File` or `ProcessBuilder.Redirect` like object to handle this ;; logic. diff --git a/tests/basilisp/test_process.lpy b/tests/basilisp/test_process.lpy index a096af6e..4450436a 100644 --- a/tests/basilisp/test_process.lpy +++ b/tests/basilisp/test_process.lpy @@ -81,4 +81,8 @@ (is (= "" (process/exec sys/executable "-c" "pass"))) (is (= "hi\n" (process/exec sys/executable "-c" "print(\"hi\")"))) (is (thrown? subprocess/CalledProcessError - (process/exec sys/executable "-c" "import sys; sys.exit(2)")))) + (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\"])")))) From 661b36ec2ee925c07db9415a0ba668b6f631e9a9 Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Tue, 26 Nov 2024 09:41:02 -0500 Subject: [PATCH 09/11] More changes --- src/basilisp/process.lpy | 33 ++++++++++++++++++++++++++++++--- tests/basilisp/test_process.lpy | 18 ++++++++++++++++++ 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/src/basilisp/process.lpy b/src/basilisp/process.lpy index 369d8dc7..241e4174 100644 --- a/src/basilisp/process.lpy +++ b/src/basilisp/process.lpy @@ -1,4 +1,31 @@ (ns basilisp.process + "An API for starting subprocesses which wraps Python's :external:py:mod:`subprocess` + module. + + The primary API function is :lpy:fn:`start` which starts a subprocess and returns + the :external:py:class:`subprocess.Popen` instance. You can wait for the process + to terminate using :lpy:fn:`exit-ref` or you can manipulate it directly. You can + fetch the streams attached to the process using :lpy:fn:`stdin`, :lpy:fn:`stdout`, + and :lpy:fn:`stderr`. This namespace also includes an extension function + :lpy:fn:`communicate` which wraps :external:py:meth:`subprocess.Popen.communicate`. + + This namespace also includes :lpy:fn:`exec` for starting a subprocess, waiting for + its completion, and returning the stdout as a string. + + .. note:: + + There are some minor differences between the Basilisp implementation and the source + Clojure implementation. Because Python does not have the concept of an unopened + ``File`` object as Java does, standard streams can only be passed existing open + file handles or paths. If a path is given, the Basilisp :lpy:fn:`from-file` and + :lpy:fn:`to-file` functions allow specifying the options that the file at that path + will be opened with (including encoding, binary mode, etc.). + + In Clojure, as in the underlying ``java.lang.Process`` object, streams are always + opened in binary mode and it is the responsibility of callers to encode and decode + into strings as necessary. Python's ``subprocesss`` offers some flexibility to + callers to specify string encoding for pipes by default, saving them the effort of + manually encoding and decoding and this namespace extends the same convenience." (:require [basilisp.io :as bio] [basilisp.string :as str]) @@ -224,9 +251,9 @@ ;; Conditionally open files here if we're given a path-like since Python does ;; not offer a `File` or `ProcessBuilder.Redirect` like object to handle this ;; logic. - (with [stdin (wrapped-file-context-manager stdin true) - stdout (wrapped-file-context-manager stdout false) - stderr (wrapped-file-context-manager stderr false)] + (with [stdin (wrapped-file-context-manager stdin false) + stdout (wrapped-file-context-manager stdout true) + stderr (wrapped-file-context-manager stderr true)] (subprocess/Popen (python/list command) ** :encoding encoding diff --git a/tests/basilisp/test_process.lpy b/tests/basilisp/test_process.lpy index 4450436a..9886fc68 100644 --- a/tests/basilisp/test_process.lpy +++ b/tests/basilisp/test_process.lpy @@ -77,8 +77,26 @@ (process/exit-ref ) (deref 1000 :timed-out)))) +(deftest start-test + (testing "stdin" + (testing "with file path" + (let [p (doto (pathlib/Path *tempdir* "path-script.py") + (.write-text "print(\"hi from a path\")")) + proc (process/start {:in (process/from-file p :encoding "utf-8")} sys/executable "-")] + (is (= 0 @(process/exit-ref proc))) + (is (= "hi from a path\n" (.decode (.read (process/stdout proc)) "utf-8"))))) + + (testing "with file handle" + (let [p (doto (pathlib/Path *tempdir* "file-script.py") + (.write-text "print(\"hi from a file\")"))] + (with-open [f (python/open p ** :mode "r")] + (let [proc (process/start {:in (process/from-file f)} sys/executable "-")] + (is (= 0 @(process/exit-ref proc))) + (is (= "hi from a file\n" (.decode (.read (process/stdout proc)) "utf-8"))))))))) + (deftest exec-test (is (= "" (process/exec sys/executable "-c" "pass"))) + (is (= "" (process/exec sys/executable "-c" "import sys; print(\"hi\", file=sys.stderr)"))) (is (= "hi\n" (process/exec sys/executable "-c" "print(\"hi\")"))) (is (thrown? subprocess/CalledProcessError (process/exec sys/executable "-c" "import sys; sys.exit(2)"))) From 1d7cb22230681c89196ae76c3f6cdcc65922f0de Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Tue, 26 Nov 2024 09:42:18 -0500 Subject: [PATCH 10/11] Lol --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index b9ecc06d..383c7944 100644 --- a/Makefile +++ b/Makefile @@ -3,7 +3,7 @@ DOCBUILDDIR = "./docs/_build" .PHONY: clean-docs clean-docs: - @rm -rf ./docs/build + @rm -rf ./docs/_build .PHONY: docs docs: From 10dfe67c02ab7b4c9dffb558b06e7f27828585c1 Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Tue, 26 Nov 2024 10:03:52 -0500 Subject: [PATCH 11/11] stdout/stderr tests and trim newlines --- tests/basilisp/test_process.lpy | 55 +++++++++++++++++++++++++++++---- 1 file changed, 49 insertions(+), 6 deletions(-) diff --git a/tests/basilisp/test_process.lpy b/tests/basilisp/test_process.lpy index 9886fc68..aa998b7c 100644 --- a/tests/basilisp/test_process.lpy +++ b/tests/basilisp/test_process.lpy @@ -4,6 +4,7 @@ sys) (:require [basilisp.process :as process] + [basilisp.string :as str] [basilisp.test :as test :refer [deftest is are testing]] [basilisp.test.fixtures :as fixtures :refer [*tempdir*]])) @@ -71,10 +72,10 @@ (deftest exit-ref-test (= 0 @(process/exit-ref (process/start sys/executable "-c" "import time; time.sleep(1)"))) (= 0 (-> (process/start sys/executable "-c" "import time; time.sleep(1)") - (process/exit-ref ) + (process/exit-ref) (deref 5000 :timed-out))) (= :timed-out (-> (process/start sys/executable "-c" "import time; time.sleep(10)") - (process/exit-ref ) + (process/exit-ref) (deref 1000 :timed-out)))) (deftest start-test @@ -84,15 +85,57 @@ (.write-text "print(\"hi from a path\")")) proc (process/start {:in (process/from-file p :encoding "utf-8")} sys/executable "-")] (is (= 0 @(process/exit-ref proc))) - (is (= "hi from a path\n" (.decode (.read (process/stdout proc)) "utf-8"))))) + (is (= "hi from a path" (str/trim (.decode (.read (process/stdout proc)) "utf-8")))))) (testing "with file handle" - (let [p (doto (pathlib/Path *tempdir* "file-script.py") - (.write-text "print(\"hi from a file\")"))] + (let [p (doto (pathlib/Path *tempdir* "file-script.py") + (.write-text "print(\"hi from a file\")"))] (with-open [f (python/open p ** :mode "r")] (let [proc (process/start {:in (process/from-file f)} sys/executable "-")] (is (= 0 @(process/exit-ref proc))) - (is (= "hi from a file\n" (.decode (.read (process/stdout proc)) "utf-8"))))))))) + (is (= "hi from a file" (str/trim (.decode (.read (process/stdout proc)) "utf-8"))))))))) + + (testing "stdout" + (testing "with file path" + (let [p (doto (pathlib/Path *tempdir* "path-output.txt") + (.touch)) + proc (process/start {:out (process/from-file p :encoding "utf-8")} + sys/executable + "-c" + "print(\"output to path\")")] + (is (= 0 @(process/exit-ref proc))) + (is (= "output to path" (str/trim (.read-text p)))))) + + (testing "with file handle" + (let [p (pathlib/Path *tempdir* "file-output.txt")] + (with-open [f (python/open p ** :mode "w")] + (let [proc (process/start {:out (process/from-file f)} + sys/executable + "-c" + "print(\"output to file\")")] + (is (= 0 @(process/exit-ref proc))))) + (is (= "output to file" (str/trim (.read-text p))))))) + + (testing "stderr" + (testing "with file path" + (let [p (doto (pathlib/Path *tempdir* "path-error.txt") + (.touch)) + proc (process/start {:err (process/from-file p :encoding "utf-8")} + sys/executable + "-c" + "import sys; print(\"error to path\", file=sys.stderr)")] + (is (= 0 @(process/exit-ref proc))) + (is (= "error to path" (str/trim (.read-text p)))))) + + (testing "with file handle" + (let [p (pathlib/Path *tempdir* "file-error.txt")] + (with-open [f (python/open p ** :mode "w")] + (let [proc (process/start {:err (process/from-file f)} + sys/executable + "-c" + "import sys; print(\"error to file\", file=sys.stderr)")] + (is (= 0 @(process/exit-ref proc))))) + (is (= "error to file" (str/trim (.read-text p)))))))) (deftest exec-test (is (= "" (process/exec sys/executable "-c" "pass")))