[GR-17790] Support :chdir in #spawn.

PullRequest: truffleruby/990

Author: eregon

.gitignore

@@ -31,13 +31,15 @@ bench/psd.rb/psd_native/ext/psd_native/*.su
 bench/psd.rb/psd_native/lib/psd_native/psd_native.su
 
 src/main/c/*/Makefile
+!src/main/c/spawn-helper/Makefile
 !src/main/c/truffleposix/Makefile
 src/main/c/*/*.bc
 src/main/c/*/*.su
 src/main/c/*/extconf.h
 src/main/c/*/mkmf.log
 src/main/c/cext/ruby.o
 src/main/c/etc/constdefs.h
+src/main/c/spawn-helper/spawn-helper
 src/main/c/sulongmock/sulongmock.c
 src/main/c/sulongmock/sulongmock.o
 src/main/c/truffleposix/truffleposix.o

CHANGELOG.md

@@ -8,6 +8,7 @@ Compatibility:
 
 * `GC.stat` can now take an option (#1716).
 * `Kernel#load` with `wrap` has been implemented (#1739).
+* Implemented `Kernel#spawn` with `:chdir` (#1492).
 
 # 19.2.0
 

mx.truffleruby/suite.py

@@ -268,6 +268,7 @@
             },
             "output": ".",
             "results": [
+                "src/main/c/spawn-helper/spawn-helper",
                 "src/main/c/truffleposix/<lib:truffleposix>",
                 "src/main/c/sulongmock/sulongmock.o",
                 "src/main/c/cext/ruby.o",
@@ -461,6 +462,9 @@
                     "file:lib/gems/specifications/test-unit-3.2.9.gemspec",
                     "file:lib/gems/specifications/xmlrpc-0.3.0.gemspec",
                 ],
+                "lib/truffle/": [
+                    "dependency:org.truffleruby.cext/src/main/c/spawn-helper/spawn-helper",
+                ],
                 "src/main/c/openssl/": [
                     "file:src/main/c/openssl/deprecation.rb",
                     "file:src/main/c/openssl/extconf.rb",

spec/tags/core/process/spawn_tags.txt

@@ -72,10 +72,6 @@ slow:Process.spawn with a command array raises a TypeError if an element in the
 slow:Process.spawn redirects both STDERR and STDOUT at the time to the given name
 fails:Process.spawn sets the umask if given the :umask option
 fails:Process.spawn raises an Errno::EACCES when the file does not have execute permissions
-fails:Process.spawn with multiple arguments raises an ArgumentError if an argument includes a null byte
-fails:Process.spawn with a command array raises an ArgumentError if the Strings in the Array include a null byte
-fails:Process.spawn when passed :chdir changes to the directory passed for :chdir
-fails:Process.spawn when passed :chdir calls #to_path to convert the :chdir value
 ci:Process.spawn redirects STDOUT to the given file if out: String
 ci:Process.spawn redirects STDOUT to the given file if out: [String name, String mode]
 ci:Process.spawn redirects STDERR to the given file if err: String

src/main/c/Makefile

@@ -17,6 +17,7 @@ CFLAGS := -Wall -Werror -Wno-unused-function -fPIC -std=c99
 LDFLAGS := -m64
 
 TRUFFLE_POSIX := truffleposix/libtruffleposix.$(DLEXT)
+SPAWN_HELPER := spawn-helper/spawn-helper
 SULONG_HEADERS := $(SULONG_POLYGLOT_H)
 GENERATE_SULONG_MOCK := $(ROOT)/tool/generate-sulongmock.rb
 
@@ -25,7 +26,7 @@ RBCONFIG := $(ROOT)/lib/truffle/rbconfig.rb
 RBCONFIG_MKMF := $(ROOT)/lib/truffle/rbconfig-for-mkmf.rb
 PREPROCESS := $(ROOT)/lib/cext/preprocess.rb
 MKMF := $(ROOT)/lib/mri/mkmf.rb
-BASIC_EXTCONF_DEPS := $(TRUFFLE_POSIX) $(RUBY_HEADERS) $(RBCONFIG) $(RBCONFIG_MKMF) $(PREPROCESS) $(MKMF)
+BASIC_EXTCONF_DEPS := $(SPAWN_HELPER) $(TRUFFLE_POSIX) $(RUBY_HEADERS) $(RBCONFIG) $(RBCONFIG_MKMF) $(PREPROCESS) $(MKMF)
 # These libraries are needed for try_link() in extconf.rb
 EXTCONF_DEPS := $(BASIC_EXTCONF_DEPS) sulongmock/sulongmock.o cext/ruby.o cext/ruby.su
 
@@ -51,6 +52,10 @@ clean_cexts:
 	rm -f etc/Makefile etc/*.bc etc/etc.su etc/constdefs.h
 	rm -f rbconfig-sizeof/Makefile rbconfig-sizeof/*.bc rbconfig-sizeof/sizeof.su
 
+# spawn-helper
+$(SPAWN_HELPER): spawn-helper/Makefile spawn-helper/spawn-helper.c
+	cd spawn-helper && $(MAKE)
+
 # truffleposix
 $(TRUFFLE_POSIX): truffleposix/Makefile truffleposix/truffleposix.c
 	cd truffleposix && $(MAKE)

src/main/c/spawn-helper/Makefile

@@ -0,0 +1,6 @@
+CC := cc
+CFLAGS := -Wall -Werror -std=c99
+LDFLAGS := -m64
+
+spawn-helper: spawn-helper.c
+	$(CC) $(LDFLAGS) -o $@ $<

src/main/c/spawn-helper/spawn-helper.c

@@ -0,0 +1,38 @@
+/*
+ * Copyright (c) 2019 Oracle and/or its affiliates. All rights reserved. This
+ * code is released under a tri EPL/GPL/LGPL license. You can use it,
+ * redistribute it and/or modify it under the terms of the:
+ *
+ * Eclipse Public License version 1.0, or
+ * GNU General Public License version 2, or
+ * GNU Lesser General Public License version 2.1.
+ */
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <unistd.h>
+
+int main(int argc, char* argv[]) {
+    if (argc < 4) {
+        fprintf(stderr, "Usage: %s working_directory executable argv0 args...\n", argv[0]);
+        return EXIT_FAILURE;
+    }
+
+    char* working_directory = argv[1];
+    char* executable = argv[2];
+    argv += 3;
+
+    int r = chdir(working_directory);
+    if (r != 0) {
+        perror("chdir");
+        fprintf(stderr, "working_directory=%s\n", working_directory);
+        return EXIT_FAILURE;
+    }
+
+    execvp(executable, argv);
+
+    /* execvp only returns on failure */
+    perror("execvp");
+    fprintf(stderr, "executable=%s\n", executable);
+    return EXIT_FAILURE;
+}

src/main/ruby/truffleruby/core/truffle/process_operations.rb

@@ -155,14 +155,14 @@ def parse(env_or_cmd, *args)
         else
           if cmd = Truffle::Type.try_convert(command, Array, :to_ary)
             raise ArgumentError, 'wrong first argument' unless cmd.size == 2
-            command = StringValue(cmd[0])
-            name = StringValue(cmd[1])
+            command = Truffle::Type.check_null_safe(StringValue(cmd[0]))
+            name = Truffle::Type.check_null_safe(StringValue(cmd[1]))
           else
-            name = command = StringValue(command)
+            name = command = Truffle::Type.check_null_safe(StringValue(command))
           end
 
           argv = [name]
-          args.each { |arg| argv << StringValue(arg) }
+          args.each { |arg| argv << Truffle::Type.check_null_safe(StringValue(arg)) }
 
           @command = command
           @argv = argv
@@ -452,6 +452,7 @@ def posix_spawnp(command, args, env_array, options)
         redirects = []
         pgroup = -1
         fds_to_close = []
+        chdir = nil
 
         options.each_pair do |key, value|
           case key
@@ -479,11 +480,20 @@ def posix_spawnp(command, args, env_array, options)
                 end
               end
             end
+          when :chdir
+            chdir = value
           else
             raise "Unknown spawn option: #{key}"
           end
         end
 
+        if chdir
+          # Go through spawn-helper to change the working dir and then execve()
+          spawn_helper = "#{Truffle::Boot.ruby_home}/lib/truffle/spawn-helper"
+          args = [spawn_helper, chdir, command, *args]
+          command = spawn_helper
+        end
+
         Truffle::POSIX.with_array_of_ints(redirects) do |redirects_ptr|
           Truffle::POSIX.with_array_of_strings_pointer(args) do |argv|
             Truffle::POSIX.with_array_of_strings_pointer(env_array) do |env|

test/mri/excludes/TestProcess.rb

@@ -40,8 +40,6 @@
 exclude :test_execopt_env_path, "needs investigation"
 exclude :test_execopts_chdir, "needs investigation"
 exclude :test_execopts_close_others, "needs investigation"
-exclude :test_execopts_open_chdir, "unknown spawn option: chdir"
-exclude :test_execopts_open_chdir_m17n_path, "needs investigation"
 exclude :test_execopts_rlimit, "unknown exec option: :rlimit_core"
 exclude :test_execopts_umask, "unknown spawn option: umask"
 exclude :test_popen_exit, "needs investigation"

test/mri/excludes/TestThreadQueue.rb

@@ -4,5 +4,4 @@
 exclude :test_queue_with_trap, "needs investigation"
 exclude :test_sized_queue_assign_max, "hangs"
 exclude :test_sized_queue_initialized, "NullPointerException in SizedQueueNodes$PushNode"
-exclude :test_thr_kill, "Unknown spawn option: chdir"
 exclude :test_sized_queue_one_closed_interrupt, "needs investigation"