ruby-changes:24178
From: nobu <ko1@a...>
Date: Wed, 27 Jun 2012 09:16:01 +0900 (JST)
Subject: [ruby-changes:24178] nobu:r36229 (trunk): popen: shell commands with envvars and execopts
nobu 2012-06-27 09:15:51 +0900 (Wed, 27 Jun 2012) New Revision: 36229 http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=rev&revision=36229 Log: popen: shell commands with envvars and execopts * io.c (is_popen_fork): check if fork and raise NotImplementedError if unavailable. * io.c (rb_io_s_popen): allow environment variables hash and exec options as flat parameters, not in an array arguments. [Feature#6651] [EXPERIMENTAL] * process.c (rb_execarg_extract_options): extract exec options, but no exceptions on non-exec options and returns them as a Hash. * process.c (rb_execarg_setenv): set environment variables. Modified files: trunk/ChangeLog trunk/internal.h trunk/io.c trunk/process.c trunk/test/ruby/test_process.rb Index: ChangeLog =================================================================== --- ChangeLog (revision 36228) +++ ChangeLog (revision 36229) @@ -1,3 +1,17 @@ +Wed Jun 27 09:15:46 2012 Nobuyoshi Nakada <nobu@r...> + + * io.c (is_popen_fork): check if fork and raise NotImplementedError if + unavailable. + + * io.c (rb_io_s_popen): allow environment variables hash and exec + options as flat parameters, not in an array arguments. + [Feature#6651] [EXPERIMENTAL] + + * process.c (rb_execarg_extract_options): extract exec options, but no + exceptions on non-exec options and returns them as a Hash. + + * process.c (rb_execarg_setenv): set environment variables. + Tue Jun 26 16:57:14 2012 Koichi Sasada <ko1@a...> * thread_pthread.c (register_cached_thread_and_wait): Index: io.c =================================================================== --- io.c (revision 36228) +++ io.c (revision 36229) @@ -5692,41 +5692,36 @@ return port; } -static VALUE -pipe_open_v(int argc, VALUE *argv, const char *modestr, int fmode, convconfig_t *convconfig) +static int +is_popen_fork(VALUE prog) { - VALUE execarg_obj, ret; - execarg_obj = rb_execarg_new(argc, argv, FALSE); - ret = pipe_open(execarg_obj, modestr, fmode, convconfig); - return ret; + if (RSTRING_LEN(prog) == 1 && RSTRING_PTR(prog)[0] == '-') { +#if !defined(HAVE_FORK) + rb_raise(rb_eNotImpError, + "fork() function is unimplemented on this machine"); +#else + return TRUE; +#endif + } + return FALSE; } static VALUE pipe_open_s(VALUE prog, const char *modestr, int fmode, convconfig_t *convconfig) { - const char *cmd = RSTRING_PTR(prog); int argc = 1; VALUE *argv = &prog; - VALUE execarg_obj, ret; + VALUE execarg_obj = Qnil; - if (RSTRING_LEN(prog) == 1 && cmd[0] == '-') { -#if !defined(HAVE_FORK) - rb_raise(rb_eNotImpError, - "fork() function is unimplemented on this machine"); -#else - return pipe_open(Qnil, modestr, fmode, convconfig); -#endif - } - - execarg_obj = rb_execarg_new(argc, argv, TRUE); - ret = pipe_open(execarg_obj, modestr, fmode, convconfig); - return ret; + if (!is_popen_fork(prog)) + execarg_obj = rb_execarg_new(argc, argv, TRUE); + return pipe_open(execarg_obj, modestr, fmode, convconfig); } /* * call-seq: - * IO.popen(cmd, mode="r" [, opt]) -> io - * IO.popen(cmd, mode="r" [, opt]) {|io| block } -> obj + * IO.popen([env,] cmd, mode="r" [, opt]) -> io + * IO.popen([env,] cmd, mode="r" [, opt]) {|io| block } -> obj * * Runs the specified command as a subprocess; the subprocess's * standard input and output will be connected to the returned @@ -5766,6 +5761,11 @@ * ls_result_with_error = ls_io.read * } * + * # spawn options can be mixed with IO options + * IO.popen(["ls", "/"], :err=>[:child, :out]) {|ls_io| + * ls_result_with_error = ls_io.read + * } + * * Raises exceptions which <code>IO.pipe</code> and * <code>Kernel.spawn</code> raise. * @@ -5810,15 +5810,25 @@ rb_io_s_popen(int argc, VALUE *argv, VALUE klass) { const char *modestr; - VALUE pname, pmode, port, tmp, opt; + VALUE pname, pmode = Qnil, port, tmp, opt = Qnil, env = Qnil, execarg_obj = Qnil; int oflags, fmode; convconfig_t convconfig; - argc = rb_scan_args(argc, argv, "11:", &pname, &pmode, &opt); + if (argc > 1 && !NIL_P(opt = rb_check_hash_type(argv[argc-1]))) --argc; + if (argc > 1 && !NIL_P(env = rb_check_hash_type(argv[0]))) --argc, ++argv; + switch (argc) { + case 2: + pmode = argv[1]; + case 1: + pname = argv[0]; + break; + default: + { + int ex = !NIL_P(opt); + rb_error_arity(argc + ex, 1 + ex, 2 + ex); + } + } - rb_io_extract_modeenc(&pmode, 0, opt, &oflags, &fmode, &convconfig); - modestr = rb_io_oflags_modestr(oflags); - tmp = rb_check_array_type(pname); if (!NIL_P(tmp)) { long len = RARRAY_LEN(tmp); @@ -5829,13 +5839,25 @@ #endif tmp = rb_ary_dup(tmp); RBASIC(tmp)->klass = 0; - port = pipe_open_v((int)len, RARRAY_PTR(tmp), modestr, fmode, &convconfig); + execarg_obj = rb_execarg_new((int)len, RARRAY_PTR(tmp), FALSE); rb_ary_clear(tmp); } else { SafeStringValue(pname); - port = pipe_open_s(pname, modestr, fmode, &convconfig); + execarg_obj = Qnil; + if (!is_popen_fork(pname)) + execarg_obj = rb_execarg_new(1, &pname, TRUE); } + if (!NIL_P(execarg_obj)) { + if (!NIL_P(opt)) + opt = rb_execarg_extract_options(execarg_obj, opt); + if (!NIL_P(env)) + rb_execarg_setenv(execarg_obj, env); + } + rb_io_extract_modeenc(&pmode, 0, opt, &oflags, &fmode, &convconfig); + modestr = rb_io_oflags_modestr(oflags); + + port = pipe_open(execarg_obj, modestr, fmode, &convconfig); if (NIL_P(port)) { /* child */ if (rb_block_given_p()) { Index: process.c =================================================================== --- process.c (revision 36228) +++ process.c (revision 36229) @@ -1654,8 +1654,7 @@ goto redirect; } else { - rb_raise(rb_eArgError, "wrong exec option symbol: %s", - rb_id2name(id)); + return ST_STOP; } break; @@ -1667,7 +1666,7 @@ break; default: - rb_raise(rb_eArgError, "wrong exec option"); + return ST_STOP; } RB_GC_GUARD(execarg_obj); @@ -1686,10 +1685,31 @@ VALUE key = (VALUE)st_key; VALUE val = (VALUE)st_val; VALUE execarg_obj = (VALUE)arg; - return rb_execarg_addopt(execarg_obj, key, val); + if (rb_execarg_addopt(execarg_obj, key, val) != ST_CONTINUE) { + if (SYMBOL_P(key)) + rb_raise(rb_eArgError, "wrong exec option symbol: %"PRIsVALUE, + key); + rb_raise(rb_eArgError, "wrong exec option"); + } + return ST_CONTINUE; } static int +check_exec_options_i_extract(st_data_t st_key, st_data_t st_val, st_data_t arg) +{ + VALUE key = (VALUE)st_key; + VALUE val = (VALUE)st_val; + VALUE *args = (VALUE *)arg; + VALUE execarg_obj = args[0]; + if (rb_execarg_addopt(execarg_obj, key, val) != ST_CONTINUE) { + VALUE nonopts = args[1]; + if (NIL_P(nonopts)) args[1] = nonopts = rb_hash_new(); + rb_hash_aset(nonopts, key, val); + } + return ST_CONTINUE; +} + +static int check_exec_fds_1(struct rb_execarg *eargp, VALUE h, int maxhint, VALUE ary) { long i; @@ -1775,6 +1795,18 @@ st_foreach(RHASH_TBL(opthash), check_exec_options_i, (st_data_t)execarg_obj); } +VALUE +rb_execarg_extract_options(VALUE execarg_obj, VALUE opthash) +{ + VALUE args[2]; + if (RHASH_EMPTY_P(opthash)) + return Qnil; + args[0] = execarg_obj; + args[1] = Qnil; + st_foreach(RHASH_TBL(opthash), check_exec_options_i_extract, (st_data_t)args); + return args[1]; +} + static int check_exec_env_i(st_data_t st_key, st_data_t st_val, st_data_t arg) { @@ -2093,6 +2125,14 @@ return rb_execarg_init(argc, argv, accept_shell, e->execarg_obj); } +void +rb_execarg_setenv(VALUE execarg_obj, VALUE env) +{ + struct rb_execarg *eargp = rb_execarg_get(execarg_obj); + env = !NIL_P(env) ? rb_check_exec_env(env) : Qfalse; + eargp->env_modification = env; +} + static int fill_envp_buf_i(st_data_t st_key, st_data_t st_val, st_data_t arg) { Index: internal.h =================================================================== --- internal.h (revision 36228) +++ internal.h (revision 36229) @@ -309,6 +309,8 @@ int rb_execarg_addopt(VALUE execarg_obj, VALUE key, VALUE val); void rb_execarg_fixup(VALUE execarg_obj); int rb_execarg_run_options(const struct rb_execarg *e, struct rb_execarg *s, char* errmsg, size_t errmsg_buflen); +VALUE rb_execarg_extract_options(VALUE execarg_obj, VALUE opthash); +void rb_execarg_setenv(VALUE execarg_obj, VALUE env); #if defined __GNUC__ && __GNUC__ >= 4 #pragma GCC visibility pop Index: test/ruby/test_process.rb =================================================================== --- test/ruby/test_process.rb (revision 36228) +++ test/ruby/test_process.rb (revision 36229) @@ -303,6 +303,47 @@ end end + def _test_execopts_env_popen(cmd) + message = cmd.inspect + IO.popen({"FOO"=>"BAR"}, cmd) {|io| + assert_equal('FOO=BAR', io.read[/^FOO=.*/], message) + } + + old = ENV["hmm"] + begin + ENV["hmm"] = "fufu" + IO.popen(cmd) {|io| assert_match(/^hmm=fufu$/, io.read, message)} + IO.popen({"hmm"=>""}, cmd) {|io| assert_match(/^hmm=$/, io.read, message)} + IO.popen({"hmm"=>nil}, cmd) {|io| assert_not_match(/^hmm=/, io.read, message)} + ENV["hmm"] = "" + IO.popen(cmd) {|io| assert_match(/^hmm=$/, io.read, message)} + IO.popen({"hmm"=>""}, cmd) {|io| assert_match(/^hmm=$/, io.read, message)} + IO.popen({"hmm"=>nil}, cmd) {|io| assert_not_match(/^hmm=/, io.read, message)} + ENV["hmm"] = nil + IO.popen(cmd) {|io| assert_not_match(/^hmm=/, io.read, message)} + IO.popen({"hmm"=>""}, cmd) {|io| assert_match(/^hmm=$/, io.read, message)} + IO.popen({"hmm"=>nil}, cmd) {|io| assert_not_match(/^hmm=/, io.read, message)} + ensure + ENV["hmm"] = old + end + end + + def test_execopts_env_popen_vector + _test_execopts_env_popen(ENVCOMMAND) + end + + def test_execopts_env_popen_string + with_tmpchdir do |d| + open('test-script', 'w') do |f| + ENVCOMMAND.each_with_index do |cmd, i| + next if i.zero? or cmd == "-e" + f.puts cmd + end + end + _test_execopts_env_popen("#{RUBY} test-script") + end + end + def test_execopts_preserve_env_on_exec_failure with_tmpchdir {|d| write_file 's', <<-"End" -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/