From eebfc3618ae290683456dc4e2fc7136857a95c57 Mon Sep 17 00:00:00 2001
From: Fran R <francois.regnoult@kpmg.co.uk>
Date: Mon, 22 Jan 2024 17:47:26 +0000
Subject: [PATCH] feat: Commands should fail the build if their exit code is
 not zero (#534)

---
 README.md                  |  1 +
 package.py                 | 26 ++++++++++++++++++--------
 tests/test_package_toml.py | 20 +++++++++++++++++++-
 3 files changed, 38 insertions(+), 9 deletions(-)

diff --git a/README.md b/README.md
index e34ade5a..e6619e30 100644
--- a/README.md
+++ b/README.md
@@ -439,6 +439,7 @@ source_path = [
 - If you specify a source path as a string that references a folder and the runtime begins with `python` or `nodejs`, the build process will automatically build python and nodejs dependencies if `requirements.txt` or `package.json` file will be found in the source folder. If you want to customize this behavior, please use the object notation as explained below.
 - All arguments except `path` are optional.
 - `patterns` - List of Python regex filenames should satisfy. Default value is "include everything" which is equal to `patterns = [".*"]`. This can also be specified as multiline heredoc string (no comments allowed). Some examples of valid patterns:
+- If you use the `commands` option and chain multiple commands, only the exit code of last command will be checked for success. If you prefer to fail fast, start the commands with the bash option `set -e` or powershell option `$ErrorActionPreference="Stop"`
 
 ```txt
     !.*/.*\.txt        # Filter all txt files recursively
diff --git a/package.py b/package.py
index e554b9cd..a4e6b736 100644
--- a/package.py
+++ b/package.py
@@ -914,17 +914,27 @@ def execute(self, build_plan, zip_stream, query):
                             # XXX: timestamp=0 - what actually do with it?
                             zs.write_dirs(rd, prefix=prefix, timestamp=0)
             elif cmd == "sh":
-                r, w = os.pipe()
-                side_ch = os.fdopen(r)
                 path, script = action[1:]
-                script = "{}\npwd >&{}".format(script, w)
+                p = subprocess.Popen(
+                    script,
+                    shell=True,
+                    stdout=subprocess.PIPE,
+                    stderr=subprocess.PIPE,
+                    cwd=path,
+                )
 
-                p = subprocess.Popen(script, shell=True, cwd=path, pass_fds=(w,))
-                os.close(w)
-                sh_work_dir = side_ch.read().strip()
                 p.wait()
-                log.info("WD: %s", sh_work_dir)
-                side_ch.close()
+                call_stdout, call_stderr = p.communicate()
+                exit_code = p.returncode
+                log.info("exit_code: %s", exit_code)
+                if exit_code != 0:
+                    raise RuntimeError(
+                        "Script did not run successfully, exit code {}: {} - {}".format(
+                            exit_code,
+                            call_stdout.decode("utf-8").strip(),
+                            call_stderr.decode("utf-8").strip(),
+                        )
+                    )
             elif cmd == "set:filter":
                 patterns = action[1]
                 pf = ZipContentFilter(args=self._args)
diff --git a/tests/test_package_toml.py b/tests/test_package_toml.py
index 129ac588..4861c6f4 100644
--- a/tests/test_package_toml.py
+++ b/tests/test_package_toml.py
@@ -1,4 +1,6 @@
-from package import get_build_system_from_pyproject_toml
+from package import get_build_system_from_pyproject_toml, BuildPlanManager
+from pytest import raises
+from unittest.mock import Mock
 
 
 def test_get_build_system_from_pyproject_toml_inexistent():
@@ -14,6 +16,22 @@ def test_get_build_system_from_pyproject_toml_unknown():
     )
 
 
+def test_build_manager_sucess_command():
+    bpm = BuildPlanManager(args=Mock())
+    # Should not have exception raised
+    bpm.execute(build_plan=[["sh", "/tmp", "pwd"]], zip_stream=None, query=None)
+
+
+def test_build_manager_failing_command():
+    bpm = BuildPlanManager(args=Mock())
+    with raises(Exception):
+        bpm.execute(
+            build_plan=[["sh", "/tmp", "NOTACOMMAND"]],
+            zip_stream=None,
+            query=None,
+        )
+
+
 def test_get_build_system_from_pyproject_toml_poetry():
     assert (
         get_build_system_from_pyproject_toml(