Feature #13300: Strip chroot path from $LOADED_FEATURES when calling Dir.chroot

• Status: Rejected
• Assignee: -
• Target version: -

[jeremyevans0 (Jeremy Evans)]

Currently, Dir.chroot doesn't modify $LOADED_FEATURES, leading
to a situation where Kernel#require will attempt to load the same
file twice, or a different file not at all because it thinks it
is already loaded.

With this example code:

require 'fileutils'
File.write('baz.rb', 'A = 1')
require './baz'
pwd = Dir.pwd
Dir.chroot(pwd)
require './baz'
FileUtils.mkdir_p(pwd)
File.write(File.join(pwd, 'baz.rb'), '$a = 2')
require "#{pwd}/baz"
warn "$a => #{$a.inspect}" unless $a == 2

Previous output on stderr:

/baz.rb:1: warning: already initialized constant A
/home/billg/baz.rb:1: warning: previous definition of A was here
$a => nil

With this patch, no output on stderr.

[nobu (Nobuyoshi Nakada)]

Should not use features_index_add.
By replacing $LOADED_FEATURES, the internal snapshot will be invalidated.
And doesn't $LOAD_PATH need to be stripped?

diff --git a/dir.c b/dir.c
index 365f059b0f..3b55e4f4a0 100644
--- a/dir.c
+++ b/dir.c
@@ -1119,10 +1119,42 @@ check_dirname(VALUE dir)
 static VALUE
 dir_s_chroot(VALUE dir, VALUE path)
 {
+    VALUE features = rb_gv_get("$LOADED_FEATURES");
+    VALUE chroot_to;
+
     path = check_dirname(path);
+    chroot_to = rb_file_s_expand_path(1, &path);
     if (chroot(RSTRING_PTR(path)) == -1)
 	rb_sys_fail_path(path);
 
+    if (rb_type(features) == RUBY_T_ARRAY &&
+        rb_type(chroot_to) == RUBY_T_STRING) {
+	long i, features_len, chroot_len, feature_min_len;
+	VALUE feature, old_features = 0;
+	char * chroot_str = RSTRING_PTR(chroot_to);
+
+	features_len = RARRAY_LEN(features);
+	chroot_len = RSTRING_LEN(chroot_to);
+	feature_min_len = chroot_len + 1;
+
+	for (i=0; i < features_len; i++) {
+	    feature = RARRAY_AREF(features, i);
+	    if ((rb_type(feature) == RUBY_T_STRING) &&
+		RSTRING_LEN(feature) > feature_min_len &&
+	        strncmp(chroot_str, RSTRING_PTR(feature), chroot_len) == 0) {
+		if (!old_features) {
+		    features = rb_ary_dup(old_features = features);
+		}
+		feature = rb_str_substr(feature, chroot_len,
+		                        RSTRING_LEN(feature));
+		RARRAY_ASET(features, i, feature);
+	    }
+	}
+	if (old_features) {
+	    rb_ary_replace(old_features, features);
+	}
+    }
+
     return INT2FIX(0);
 }
 #else

[jeremyevans0 (Jeremy Evans)]

Thanks nobu, your patch is definitely better.

I thought about stripping $LOAD_PATH, but I believe it would break backwards compatibility slightly, since $LOAD_PATH stores where to look for future loads/requires, and is not concerned about code previously required before Dir.chroot. For example, the behavior of the following code:

require 'fileutils'
pwd = Dir.pwd
$LOAD_PATH << File.join(pwd)
Dir.chroot(pwd)
File.write('foo.rb', '$a = 1')
FileUtils.mkdir_p(pwd)
File.write(File.join(pwd, 'foo.rb'), '$a = 2')
require 'foo'
p $a

would change from outputting 2 to outputting 1 if you started stripping $LOAD_PATH.

That being said, I'm not opposed to stripping $LOAD_PATH as well as $LOADED_FEATURES, as I doubt there is significant amount of code that relies on the current behavior.

[shyouhei (Shyouhei Urabe)]

We discussesed this issue at the developer meeting yesterday.

It seems practically not possible to provide a solution that works for everyone; for instance autoloads might break because the then-specified autoload library might become out of sight. You might have placed everything inside of your chroot tree but that's not what the process can make sure.

Also, we could not be confident that LOADED_FEATURES and LOAD_PATH are the only thing that has to be tweaked before/after chroot.

Luckily you can modify LOADED_FEATURES from ruby scripts. I advice you to do what you want manually for now.

[jeremyevans0 (Jeremy Evans)]

• Status changed from Open to Rejected

shyouhei (Shyouhei Urabe) wrote:

We discussesed this issue at the developer meeting yesterday.

It seems practically not possible to provide a solution that works for everyone; for instance autoloads might break because the then-specified autoload library might become out of sight. You might have placed everything inside of your chroot tree but that's not what the process can make sure.

Correct. Actually when using chroot in real applications, autoload is the most likely thing to cause breakage. Hopefully autoload can be removed in Ruby 3.

Also, we could not be confident that LOADED_FEATURES and LOAD_PATH are the only thing that has to be tweaked before/after chroot.

Luckily you can modify LOADED_FEATURES from ruby scripts. I advice you to do what you want manually for now.

OK, that seems reasonable. I've already been using a working manual solution for many months, so I will continue to use that. Thanks for taking the time to consider this proposal, I will close this now.