Repository: datalad/datalad

Issue:
`datalad run` no longer relays exit code
This used to be a thing, and the handbook also documents it still. Somewhere in the 0.16 series it got lost.

```
❯ chronic pip install datalad==0.15.6
❯ dl run "bash -c 'exit 3'"
[INFO   ] == Command start (output follows) ===== 
[INFO   ] == Command exit (modification check follows) ===== 
[INFO   ] The command had a non-zero exit code. If this is expected, you can save the changes with 'datalad save -d . -r -F .git/COMMIT_EDITMSG' 
CommandError: 'bash -c 'exit 3'' failed with exitcode 3 under /tmp/some
❯ echo $?
3

❯ chronic pip install datalad==0.16.4
❯ dl run "bash -c 'exit 3'"
[WARNING] Requested extension 'next' is not available 
[INFO   ] == Command start (output follows) ===== 
[INFO   ] == Command exit (modification check follows) ===== 
[INFO   ] The command had a non-zero exit code. If this is expected, you can save the changes with 'datalad save -d . -r -F .git/COMMIT_EDITMSG' 
run(error): /tmp/some (dataset) [bash -c 'exit 3']
❯ echo $?
1
```

I think this was a useful feature, and we should restore it.

I believe this is caused by a change in the handling. Now we get an error result, rather than an exception handler at the very top.

That being said, the result itself contains everything needed to set a matching exit code:

```
❯ dl -f json_pp run "bash -c 'exit 3'"
[WARNING] Requested extension 'next' is not available 
[INFO   ] == Command start (output follows) ===== 
[INFO   ] == Command exit (modification check follows) ===== 
{
  "action": "run",
  "exception": "CommandError: 'bash -c 'exit 3'' failed with exitcode 3 under /tmp/some",
  "exception_traceback": "[run.py:_execute_command:701,runner.py:run:242]",
  "exit_code": 3,
  "message": "bash -c 'exit 3'",
  "msg_path": "/tmp/some/.git/COMMIT_EDITMSG",
  "path": "/tmp/some",
  "run_info": {
    "chain": [],
    "cmd": "bash -c 'exit 3'",
    "dsid": "9fdb9a12-d3f5-4b19-910d-ad1b617a912b",
    "exit": 3,
    "extra_inputs": [],
    "inputs": [],
    "outputs": [],
    "pwd": "."
  },
  "status": "error",
  "type": "dataset"
}
```

Discussion:

Patch:
diff --git a/changelog.d/pr-7641.md b/changelog.d/pr-7641.md
new file mode 100644
index 000000000..92a05d128
--- /dev/null
+++ b/changelog.d/pr-7641.md
@@ -0,0 +1,3 @@
+### 🐛 Bug Fixes
+
+- Exit with the original non-0 exit code if underlying functionality, in particular "datalad run", returned incomplete result record with a non-0 exit code.  Fixes [#7504](https://github.com/datalad/datalad/issues/7504) via [PR #7641](https://github.com/datalad/datalad/pull/7641) (by [@yarikoptic](https://github.com/yarikoptic))
diff --git a/datalad/cli/main.py b/datalad/cli/main.py
index bb68e46c8..61d3293ad 100644
--- a/datalad/cli/main.py
+++ b/datalad/cli/main.py
@@ -208,6 +208,21 @@ def _run_with_exception_handler(cmdlineargs):
             # in general we do not want to see the error again, but
             # present in debug output
             lgr.debug('could not perform all requested actions: %s', ce)
+            # fish for an exit code. If any of the "failed" results
+            # is caused by a command error, it will come with an `exit_code`
+            # property.
+            exit_codes = [
+                r['exit_code'] for r in exc.failed if 'exit_code' in r]
+            if exit_codes:
+                # we have exit code(s), take the first non-0 one
+                non0_codes = [e for e in exit_codes if e]
+                if len(non0_codes) != len(exit_codes):
+                    lgr.debug(
+                        "Among %d incomplete results with exit codes %d had exit code 0",
+                        len(exit_codes),
+                        len(exit_codes) - len(non0_codes))
+                if non0_codes:
+                    exit_code = non0_codes[0]
         elif isinstance(exc, CommandError):
             exit_code = _communicate_commanderror(exc) or exit_code
         elif isinstance(exc, KeyboardInterrupt):


Test patch:
diff --git a/datalad/cli/tests/test_main.py b/datalad/cli/tests/test_main.py
index 18254166e..3a569b18a 100644
--- a/datalad/cli/tests/test_main.py
+++ b/datalad/cli/tests/test_main.py
@@ -211,6 +211,18 @@ def test_combined_short_option():
     assert_in("too few arguments", stderr)
 
 
+# https://github.com/datalad/datalad/issues/7504
+@with_tempfile(mkdir=True)
+def test_run_exit_code(tempdir=None):
+    # datalad run is just an internal command which can readily be used to trigger
+    # desired situation
+    create(dataset=tempdir, annex=False)
+    with chpwd(tempdir):  # can't just use -C tempdir since we do "in process" run_main
+        stdout, stderr = run_main(['run', '--explicit', 'exit 3'], exit_code=3) #, expect_stderr=True)
+    print(stdout)
+    print(stderr)
+
+
 # https://github.com/datalad/datalad/issues/6814
 @with_tempfile(mkdir=True)
 def test_conflicting_short_option(tempdir=None):