From fb29cffab05cb5446c1e6cd68035c39143b02763 Mon Sep 17 00:00:00 2001
From: k0kubun <k0kubun@b2dd03c8-39d4-4d8f-98ff-823fe69b080e>
Date: Wed, 24 Jan 2018 14:11:25 +0000
Subject: [PATCH] process.c: add :exception option to Kernel.#system

to raise error when it fails.

[Feature 14386] [GH-1795]

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@62025 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
---
 ext/pty/pty.c            |  2 +-
 internal.h               |  5 +++--
 io.c                     |  6 ++---
 process.c                | 47 ++++++++++++++++++++++++++++++----------
 test/ruby/test_system.rb | 19 ++++++++++++++++
 5 files changed, 62 insertions(+), 17 deletions(-)

diff --git a/ext/pty/pty.c b/ext/pty/pty.c
index fb9ff19a94..1fdb220612 100644
--- a/ext/pty/pty.c
+++ b/ext/pty/pty.c
@@ -182,7 +182,7 @@ establishShell(int argc, VALUE *argv, struct pty_info *info,
 	argv = &v;
     }
 
-    carg.execarg_obj = rb_execarg_new(argc, argv, 1);
+    carg.execarg_obj = rb_execarg_new(argc, argv, 1, 0);
     carg.eargp = rb_execarg_get(carg.execarg_obj);
     rb_execarg_parent_start(carg.execarg_obj);
 
diff --git a/internal.h b/internal.h
index 20c1f9750c..4715638d5c 100644
--- a/internal.h
+++ b/internal.h
@@ -1631,6 +1631,7 @@ struct rb_execarg {
     unsigned new_pgroup_flag : 1;
     unsigned uid_given : 1;
     unsigned gid_given : 1;
+    unsigned exception : 1;
     rb_pid_t pgroup_pgid; /* asis(-1), new pgroup(0), specified pgroup (0<V). */
     VALUE rlimit_limits; /* Qfalse or [[rtype, softlim, hardlim], ...] */
     mode_t umask_mask;
@@ -1968,9 +1969,9 @@ VALUE rb_int_positive_pow(long x, unsigned long y);
 /* process.c (export) */
 int rb_exec_async_signal_safe(const struct rb_execarg *e, char *errmsg, size_t errmsg_buflen);
 rb_pid_t rb_fork_async_signal_safe(int *status, int (*chfunc)(void*, char *, size_t), void *charg, VALUE fds, char *errmsg, size_t errmsg_buflen);
-VALUE rb_execarg_new(int argc, const VALUE *argv, int accept_shell);
+VALUE rb_execarg_new(int argc, const VALUE *argv, int accept_shell, int allow_exc_opt);
 struct rb_execarg *rb_execarg_get(VALUE execarg_obj); /* dangerous.  needs GC guard. */
-VALUE rb_execarg_init(int argc, const VALUE *argv, int accept_shell, VALUE execarg_obj);
+VALUE rb_execarg_init(int argc, const VALUE *argv, int accept_shell, VALUE execarg_obj, int allow_exc_opt);
 int rb_execarg_addopt(VALUE execarg_obj, VALUE key, VALUE val);
 void rb_execarg_parent_start(VALUE execarg_obj);
 void rb_execarg_parent_end(VALUE execarg_obj);
diff --git a/io.c b/io.c
index 501d14e9c9..dde980880c 100644
--- a/io.c
+++ b/io.c
@@ -6633,7 +6633,7 @@ pipe_open_s(VALUE prog, const char *modestr, int fmode,
     VALUE execarg_obj = Qnil;
 
     if (!is_popen_fork(prog))
-	execarg_obj = rb_execarg_new(argc, argv, TRUE);
+        execarg_obj = rb_execarg_new(argc, argv, TRUE, FALSE);
     return pipe_open(execarg_obj, modestr, fmode, convconfig);
 }
 
@@ -6766,14 +6766,14 @@ rb_io_s_popen(int argc, VALUE *argv, VALUE klass)
 	    rb_raise(rb_eArgError, "too many arguments");
 	}
 #endif
-	execarg_obj = rb_execarg_new((int)len, RARRAY_CONST_PTR(tmp), FALSE);
+        execarg_obj = rb_execarg_new((int)len, RARRAY_CONST_PTR(tmp), FALSE, FALSE);
 	RB_GC_GUARD(tmp);
     }
     else {
 	SafeStringValue(pname);
 	execarg_obj = Qnil;
 	if (!is_popen_fork(pname))
-	    execarg_obj = rb_execarg_new(1, &pname, TRUE);
+            execarg_obj = rb_execarg_new(1, &pname, TRUE, FALSE);
     }
     if (!NIL_P(execarg_obj)) {
 	if (!NIL_P(opt))
diff --git a/process.c b/process.c
index a983340a89..7df4d3bffe 100644
--- a/process.c
+++ b/process.c
@@ -249,7 +249,7 @@ typedef unsigned LONG_LONG unsigned_clock_t;
 typedef void (*sig_t) (int);
 #endif
 
-static ID id_in, id_out, id_err, id_pid, id_uid, id_gid;
+static ID id_in, id_out, id_err, id_pid, id_uid, id_gid, id_exception;
 static ID id_close, id_child;
 #ifdef HAVE_SETPGID
 static ID id_pgroup;
@@ -2253,12 +2253,12 @@ rb_exec_fillarg(VALUE prog, int argc, VALUE *argv, VALUE env, VALUE opthash, VAL
 }
 
 VALUE
-rb_execarg_new(int argc, const VALUE *argv, int accept_shell)
+rb_execarg_new(int argc, const VALUE *argv, int accept_shell, int allow_exc_opt)
 {
     VALUE execarg_obj;
     struct rb_execarg *eargp;
     execarg_obj = TypedData_Make_Struct(0, struct rb_execarg, &exec_arg_data_type, eargp);
-    rb_execarg_init(argc, argv, accept_shell, execarg_obj);
+    rb_execarg_init(argc, argv, accept_shell, execarg_obj, allow_exc_opt);
     return execarg_obj;
 }
 
@@ -2271,16 +2271,23 @@ rb_execarg_get(VALUE execarg_obj)
 }
 
 VALUE
-rb_execarg_init(int argc, const VALUE *orig_argv, int accept_shell, VALUE execarg_obj)
+rb_execarg_init(int argc, const VALUE *orig_argv, int accept_shell, VALUE execarg_obj, int allow_exc_opt)
 {
     struct rb_execarg *eargp = rb_execarg_get(execarg_obj);
-    VALUE prog, ret;
+    VALUE prog, ret, exception = Qnil;
     VALUE env = Qnil, opthash = Qnil;
     VALUE argv_buf;
     VALUE *argv = ALLOCV_N(VALUE, argv_buf, argc);
     MEMCPY(argv, orig_argv, VALUE, argc);
     prog = rb_exec_getargs(&argc, &argv, accept_shell, &env, &opthash);
+    if (allow_exc_opt && !NIL_P(opthash) && rb_hash_has_key(opthash, ID2SYM(id_exception))) {
+        opthash = rb_hash_dup(opthash);
+        exception = rb_hash_delete(opthash, ID2SYM(id_exception));
+    }
     rb_exec_fillarg(prog, argc, argv, env, opthash, execarg_obj);
+    if (RTEST(exception)) {
+        eargp->exception = 1;
+    }
     ALLOCV_END(argv_buf);
     ret = eargp->use_shell ? eargp->invoke.sh.shell_script : eargp->invoke.cmd.command_name;
     RB_GC_GUARD(execarg_obj);
@@ -2599,7 +2606,7 @@ rb_f_exec(int argc, const VALUE *argv)
     char errmsg[CHILD_ERRMSG_BUFLEN] = { '\0' };
     int err;
 
-    execarg_obj = rb_execarg_new(argc, argv, TRUE);
+    execarg_obj = rb_execarg_new(argc, argv, TRUE, FALSE);
     eargp = rb_execarg_get(execarg_obj);
     before_exec(); /* stop timer thread before redirects */
     rb_execarg_parent_start(execarg_obj);
@@ -3989,7 +3996,7 @@ rb_spawn_internal(int argc, const VALUE *argv, char *errmsg, size_t errmsg_bufle
 {
     VALUE execarg_obj;
 
-    execarg_obj = rb_execarg_new(argc, argv, TRUE);
+    execarg_obj = rb_execarg_new(argc, argv, TRUE, FALSE);
     return rb_execarg_spawn(execarg_obj, errmsg, errmsg_buflen);
 }
 
@@ -4043,6 +4050,8 @@ rb_f_system(int argc, VALUE *argv)
 {
     rb_pid_t pid;
     int status;
+    VALUE execarg_obj;
+    struct rb_execarg *eargp;
 
 #if defined(SIGCLD) && !defined(SIGCHLD)
 # define SIGCHLD SIGCLD
@@ -4054,7 +4063,8 @@ rb_f_system(int argc, VALUE *argv)
     rb_last_status_clear();
     chfunc = signal(SIGCHLD, SIG_DFL);
 #endif
-    pid = rb_spawn_internal(argc, argv, NULL, 0);
+    execarg_obj = rb_execarg_new(argc, argv, TRUE, TRUE);
+    pid = rb_execarg_spawn(execarg_obj, NULL, 0);
 #if defined(HAVE_WORKING_FORK) || defined(HAVE_SPAWNV)
     if (pid > 0) {
         int ret, status;
@@ -4066,12 +4076,26 @@ rb_f_system(int argc, VALUE *argv)
 #ifdef SIGCHLD
     signal(SIGCHLD, chfunc);
 #endif
+    TypedData_Get_Struct(execarg_obj, struct rb_execarg, &exec_arg_data_type, eargp);
     if (pid < 0) {
-	return Qnil;
+        if (eargp->exception) {
+            int err = errno;
+            rb_syserr_fail_str(err, eargp->invoke.sh.shell_script);
+            RB_GC_GUARD(execarg_obj);
+        }
+        else {
+            return Qnil;
+        }
     }
     status = PST2INT(rb_last_status_get());
     if (status == EXIT_SUCCESS) return Qtrue;
-    return Qfalse;
+    if (eargp->exception) {
+        rb_raise(rb_eRuntimeError, "Command failed with status (%d): %s",
+                 WEXITSTATUS(status), RSTRING_PTR(eargp->invoke.sh.shell_script));
+    }
+    else {
+        return Qfalse;
+    }
 }
 
 /*
@@ -4350,7 +4374,7 @@ rb_f_spawn(int argc, VALUE *argv)
     VALUE execarg_obj, fail_str;
     struct rb_execarg *eargp;
 
-    execarg_obj = rb_execarg_new(argc, argv, TRUE);
+    execarg_obj = rb_execarg_new(argc, argv, TRUE, FALSE);
     eargp = rb_execarg_get(execarg_obj);
     fail_str = eargp->use_shell ? eargp->invoke.sh.shell_script : eargp->invoke.cmd.command_name;
 
@@ -8038,6 +8062,7 @@ Init_process(void)
     id_MACH_ABSOLUTE_TIME_BASED_CLOCK_MONOTONIC = rb_intern("MACH_ABSOLUTE_TIME_BASED_CLOCK_MONOTONIC");
 #endif
     id_hertz = rb_intern("hertz");
+    id_exception = rb_intern("exception");
 
     InitVM(process);
 }
diff --git a/test/ruby/test_system.rb b/test/ruby/test_system.rb
index 60037ab044..0c0ad33fb6 100644
--- a/test/ruby/test_system.rb
+++ b/test/ruby/test_system.rb
@@ -160,4 +160,23 @@ class TestSystem < Test::Unit::TestCase
       assert_equal(true, system(tmpfilename), '[ruby-core:32745]')
     }
   end if File.executable?("/bin/sh")
+
+  def test_system_exception
+    ruby = EnvUtil.rubybin
+    assert_nothing_raised do
+      system('feature_14235', exception: false)
+      system("'#{ruby}' -e 'exit 1'", exception: false)
+    end
+    assert_raise(Errno::ENOENT) do
+      system('feature_14235', exception: true)
+    end
+    assert_raise(RuntimeError) do
+      system("'#{ruby}' -e 'exit 1'", exception: true)
+    end
+    begin
+      system("'#{ruby}' -e 'exit 1'", exception: true)
+    rescue RuntimeError => e
+      assert_equal true, e.message.include?('status (1)')
+    end
+  end
 end