[前][次][番号順一覧][スレッド一覧]

ruby-changes:24167

From: nobu <ko1@a...>
Date: Tue, 26 Jun 2012 10:02:11 +0900 (JST)
Subject: [ruby-changes:24167] nobu:r36218 (trunk): Revert r36213 "popen: shell commands with envvar"

nobu	2012-06-26 10:01:59 +0900 (Tue, 26 Jun 2012)

  New Revision: 36218

  http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=rev&revision=36218

  Log:
    Revert r36213 "popen: shell commands with envvar"
    
    * io.c (rb_io_s_popen): revert r36213 "popen: shell commands with
      envvar" because it disabled to let single command bypass shell.

  Modified files:
    trunk/ChangeLog
    trunk/io.c
    trunk/test/ruby/test_process.rb

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 36217)
+++ ChangeLog	(revision 36218)
@@ -1,3 +1,8 @@
+Tue Jun 26 10:01:56 2012  Nobuyoshi Nakada  <nobu@r...>
+
+	* io.c (rb_io_s_popen): revert r36213 "popen: shell commands with
+	  envvar" because it disabled to let single command bypass shell.
+
 Mon Jun 25 17:49:28 2012  Nobuyoshi Nakada  <nobu@r...>
 
 	* class.c (rb_mix_module): revert Module#mix.
@@ -7,14 +12,6 @@
 	* proc.c (rb_mod_define_method): allow method transplanting from a
 	  module to either class or module.  [ruby-core:34267][Feature #4254]
 
-Mon Jun 25 15:42:00 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 shell commands with modified environment
-	  variables.
-
 Mon Jun 25 11:34:45 2012  NAKAMURA Usaku  <usa@r...>
 
 	* internal.h: use rb_pid_t instead of pid_t because of there is no
Index: io.c
===================================================================
--- io.c	(revision 36217)
+++ io.c	(revision 36218)
@@ -5692,30 +5692,35 @@
     return port;
 }
 
-static int
-is_popen_fork(VALUE prog)
+static VALUE
+pipe_open_v(int argc, VALUE *argv, const char *modestr, int fmode, convconfig_t *convconfig)
 {
-    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;
+    VALUE execarg_obj, ret;
+    execarg_obj = rb_execarg_new(argc, argv, FALSE);
+    ret = pipe_open(execarg_obj, modestr, fmode, convconfig);
+    return ret;
 }
 
 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 = Qnil;
+    VALUE execarg_obj, ret;
 
-    if (!is_popen_fork(prog))
-	execarg_obj = rb_execarg_new(argc, argv, TRUE);
-    return pipe_open(execarg_obj, modestr, fmode, convconfig);
+    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;
 }
 
 /*
@@ -5805,7 +5810,7 @@
 rb_io_s_popen(int argc, VALUE *argv, VALUE klass)
 {
     const char *modestr;
-    VALUE pname, pmode, port, tmp, opt, execarg_obj;
+    VALUE pname, pmode, port, tmp, opt;
     int oflags, fmode;
     convconfig_t convconfig;
 
@@ -5824,16 +5829,13 @@
 #endif
 	tmp = rb_ary_dup(tmp);
 	RBASIC(tmp)->klass = 0;
-	execarg_obj = rb_execarg_new((int)len, RARRAY_PTR(tmp), TRUE);
+	port = pipe_open_v((int)len, RARRAY_PTR(tmp), modestr, fmode, &convconfig);
 	rb_ary_clear(tmp);
     }
     else {
 	SafeStringValue(pname);
-	execarg_obj = Qnil;
-	if (!is_popen_fork(pname))
-	    execarg_obj = rb_execarg_new(1, &pname, TRUE);
+	port = pipe_open_s(pname, modestr, fmode, &convconfig);
     }
-    port = pipe_open(execarg_obj, modestr, fmode, &convconfig);
     if (NIL_P(port)) {
 	/* child */
 	if (rb_block_given_p()) {
Index: test/ruby/test_process.rb
===================================================================
--- test/ruby/test_process.rb	(revision 36217)
+++ test/ruby/test_process.rb	(revision 36218)
@@ -275,53 +275,34 @@
       assert_equal("PATH\n", io.read)
     }
 
+    IO.popen([{"FOO"=>"BAR"}, *ENVCOMMAND]) {|io|
+      assert_match(/^FOO=BAR$/, io.read)
+    }
+
     with_tmpchdir {|d|
       system({"fofo"=>"haha"}, *ENVCOMMAND, STDOUT=>"out")
-      assert_match(/^fofo=haha$/, File.read("out").chomp, message)
+      assert_match(/^fofo=haha$/, File.read("out").chomp)
     }
-  end
 
-  def _test_execopts_env_popen(cmd)
-    message = cmd.inspect
-    IO.popen([{"FOO"=>"BAR"}, *cmd]) {|io|
-      assert_match(/^FOO=BAR$/, io.read, 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)}
+      IO.popen(ENVCOMMAND) {|io| assert_match(/^hmm=fufu$/, io.read) }
+      IO.popen([{"hmm"=>""}, *ENVCOMMAND]) {|io| assert_match(/^hmm=$/, io.read) }
+      IO.popen([{"hmm"=>nil}, *ENVCOMMAND]) {|io| assert_not_match(/^hmm=/, io.read) }
       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)}
+      IO.popen(ENVCOMMAND) {|io| assert_match(/^hmm=$/, io.read) }
+      IO.popen([{"hmm"=>""}, *ENVCOMMAND]) {|io| assert_match(/^hmm=$/, io.read) }
+      IO.popen([{"hmm"=>nil}, *ENVCOMMAND]) {|io| assert_not_match(/^hmm=/, io.read) }
       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)}
+      IO.popen(ENVCOMMAND) {|io| assert_not_match(/^hmm=/, io.read) }
+      IO.popen([{"hmm"=>""}, *ENVCOMMAND]) {|io| assert_match(/^hmm=$/, io.read) }
+      IO.popen([{"hmm"=>nil}, *ENVCOMMAND]) {|io| assert_not_match(/^hmm=/, io.read) }
     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"
@@ -622,7 +603,7 @@
   def test_execopts_popen
     with_tmpchdir {|d|
       IO.popen("#{RUBY} -e 'puts :foo'") {|io| assert_equal("foo\n", io.read) }
-      IO.popen(["echo bar"]) {|io| assert_equal("bar\n", io.read) }
+      assert_raise(Errno::ENOENT) { IO.popen(["echo bar"]) {} } # assuming "echo bar" command not exist.
       IO.popen(ECHO["baz"]) {|io| assert_equal("baz\n", io.read) }
       assert_raise(ArgumentError) {
         IO.popen([*ECHO["qux"], STDOUT=>STDOUT]) {|io| }

--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/

[前][次][番号順一覧][スレッド一覧]