diff --git a/lib/rubygems/config_file.rb b/lib/rubygems/config_file.rb index a2bcb6dfbc24..2bbab6b2f5e4 100644 --- a/lib/rubygems/config_file.rb +++ b/lib/rubygems/config_file.rb @@ -47,8 +47,8 @@ class Gem::ConfigFile DEFAULT_CONCURRENT_DOWNLOADS = 8 DEFAULT_CERT_EXPIRATION_LENGTH_DAYS = 365 DEFAULT_IPV4_FALLBACK_ENABLED = false - # TODO: Use false as default value for this option in RubyGems 4.0 - DEFAULT_INSTALL_EXTENSION_IN_LIB = true + # Changed from true to false in RubyGems 4.0 for better gem organization + DEFAULT_INSTALL_EXTENSION_IN_LIB = false ## # For Ruby packagers to set configuration defaults. Set in @@ -229,6 +229,13 @@ def initialize(args) # TODO: We should handle concurrent_downloads same as other options @cert_expiration_length_days = @hash[:cert_expiration_length_days] if @hash.key? :cert_expiration_length_days @install_extension_in_lib = @hash[:install_extension_in_lib] if @hash.key? :install_extension_in_lib + # Issue deprecation warning if install_extension_in_lib is set to true + if @hash.key?(:install_extension_in_lib) && @hash[:install_extension_in_lib] == true + warn "Setting install_extension_in_lib to true is deprecated in RubyGems 4.0. " \ + "Native extensions should be installed in /ext directory by default for better organization. " \ + "If your gem requires /lib placement, consider updating your gem structure. " \ + "This setting will be removed in a future RubyGems version." + end @ipv4_fallback_enabled = @hash[:ipv4_fallback_enabled] if @hash.key? :ipv4_fallback_enabled @home = @hash[:gemhome] if @hash.key? :gemhome diff --git a/lib/rubygems/ext/cargo_builder.rb b/lib/rubygems/ext/cargo_builder.rb index 21b50f394d55..104d603512c8 100644 --- a/lib/rubygems/ext/cargo_builder.rb +++ b/lib/rubygems/ext/cargo_builder.rb @@ -51,6 +51,14 @@ def build(extension, dest_path, results, args = [], lib_dir = nil, cargo_dir = D nesting = extension_nesting(extension) if Gem.install_extension_in_lib && lib_dir + # Warn about using the deprecated /lib placement + gem_name = detect_gem_name_from_path(cargo_dir) + if gem_name + warn "Gem '#{gem_name}' is installing native extensions in /lib directory. " \ + "Consider moving extensions to /ext directory for better organization. " \ + "Set install_extension_in_lib: true in your .gemrc to maintain current behavior." + end + nested_lib_dir = File.join(lib_dir, nesting) FileUtils.mkdir_p nested_lib_dir FileUtils.cp_r dlext_path, nested_lib_dir, remove_destination: true @@ -347,4 +355,19 @@ def initialize(dir) MSG end end + + def self.detect_gem_name_from_path(cargo_dir) + # Try to detect gem name from the cargo directory path + # Look for patterns like /path/to/gem_name/ext/extension_name + path_parts = cargo_dir.split(File::SEPARATOR) + + # Find the gem name by looking for the parent of 'ext' directory + ext_index = path_parts.rindex('ext') + return nil unless ext_index && ext_index > 0 + + gem_name = path_parts[ext_index - 1] + return nil if gem_name.nil? || gem_name.empty? + + gem_name + end end diff --git a/lib/rubygems/ext/ext_conf_builder.rb b/lib/rubygems/ext/ext_conf_builder.rb index e652a221f83b..021ff7cca01b 100644 --- a/lib/rubygems/ext/ext_conf_builder.rb +++ b/lib/rubygems/ext/ext_conf_builder.rb @@ -49,6 +49,14 @@ def self.build(extension, dest_path, results, args=[], lib_dir=nil, extension_di # Do not copy extension libraries by default when cross-compiling # not to conflict with the one already built for the host platform. if Gem.install_extension_in_lib && lib_dir && !is_cross_compiling + # Warn about using the deprecated /lib placement + gem_name = detect_gem_name_from_path(extension_dir) + if gem_name + warn "Gem '#{gem_name}' is installing native extensions in /lib directory. " \ + "Consider moving extensions to /ext directory for better organization. " \ + "Set install_extension_in_lib: true in your .gemrc to maintain current behavior." + end + FileUtils.mkdir_p lib_dir entries = Dir.entries(full_tmp_dest) - %w[. ..] entries = entries.map {|entry| File.join full_tmp_dest, entry } @@ -74,4 +82,19 @@ def self.get_relative_path(path, base) path[0..base.length - 1] = "." if path.start_with?(base) path end + + def self.detect_gem_name_from_path(extension_dir) + # Try to detect gem name from the extension directory path + # Look for patterns like /path/to/gem_name/ext/extension_name + path_parts = extension_dir.split(File::SEPARATOR) + + # Find the gem name by looking for the parent of 'ext' directory + ext_index = path_parts.rindex('ext') + return nil unless ext_index && ext_index > 0 + + gem_name = path_parts[ext_index - 1] + return nil if gem_name.nil? || gem_name.empty? + + gem_name + end end diff --git a/test/rubygems/test_gem_config_file.rb b/test/rubygems/test_gem_config_file.rb index 4230eda4d399..bef21cd87b59 100644 --- a/test/rubygems/test_gem_config_file.rb +++ b/test/rubygems/test_gem_config_file.rb @@ -529,6 +529,28 @@ def test_load_install_extension_in_lib_from_config assert_equal(false, @cfg.install_extension_in_lib) end + def test_install_extension_in_lib_deprecation_warning + File.open @temp_conf, "w" do |fp| + fp.puts ":install_extension_in_lib: true" + end + + # Capture the warning using stderr + require "stringio" + stderr = StringIO.new + $stderr = stderr + + util_config_file + + # Restore stderr + $stderr = STDERR + + assert_equal(true, @cfg.install_extension_in_lib) + warning_output = stderr.string + assert_includes warning_output, "Setting install_extension_in_lib to true is deprecated in RubyGems 4.0" + assert_includes warning_output, "Native extensions should be installed in /ext directory by default" + assert_includes warning_output, "This setting will be removed in a future RubyGems version" + end + def test_disable_default_gem_server File.open @temp_conf, "w" do |fp| fp.puts ":disable_default_gem_server: true" diff --git a/test/rubygems/test_gem_ext_cargo_builder.rb b/test/rubygems/test_gem_ext_cargo_builder.rb index b970e442c250..bbbc2b6ff495 100644 --- a/test/rubygems/test_gem_ext_cargo_builder.rb +++ b/test/rubygems/test_gem_ext_cargo_builder.rb @@ -226,4 +226,108 @@ def replace_in_rust_file(name, from, to) content = @fixture_dir.join(name).read.gsub(from, to) File.write(File.join(@ext, name), content) end + + def test_build_with_lib_placement_warning + skip_unsupported_platforms! + setup_rust_gem "rust_ruby_example" + + # Set up lib directory for the gem + gem_dir = File.join @tempdir, "rust_ruby_example" + lib_dir = File.join gem_dir, "lib" + FileUtils.mkdir_p lib_dir + + # Mock the install_extension_in_lib setting to true + original_setting = Gem.install_extension_in_lib + Gem.configuration.install_extension_in_lib = true + + # Capture stderr for warning + require "stringio" + stderr = StringIO.new + original_stderr = $stderr + $stderr = stderr + + output = [] + + begin + Dir.chdir @ext do + ENV.update(@rust_envs) + builder = Gem::Ext::CargoBuilder.new + builder.build "Cargo.toml", @dest_path, output, [], lib_dir, @ext + end + + # Restore stderr + $stderr = original_stderr + + warning_output = stderr.string + assert_includes warning_output, "Gem 'rust_ruby_example' is installing native extensions in /lib directory" + assert_includes warning_output, "Consider moving extensions to /ext directory for better organization" + assert_includes warning_output, "Set install_extension_in_lib: true in your .gemrc to maintain current behavior" + ensure + # Restore original setting + Gem.configuration.install_extension_in_lib = original_setting + $stderr = original_stderr + end + end + + def test_build_without_lib_placement_warning_when_false + skip_unsupported_platforms! + setup_rust_gem "rust_ruby_example" + + # Set up lib directory for the gem + gem_dir = File.join @tempdir, "rust_ruby_example" + lib_dir = File.join gem_dir, "lib" + FileUtils.mkdir_p lib_dir + + # Mock the install_extension_in_lib setting to false + original_setting = Gem.install_extension_in_lib + Gem.configuration.install_extension_in_lib = false + + # Capture stderr for warning + require "stringio" + stderr = StringIO.new + original_stderr = $stderr + $stderr = stderr + + output = [] + + begin + Dir.chdir @ext do + ENV.update(@rust_envs) + builder = Gem::Ext::CargoBuilder.new + builder.build "Cargo.toml", @dest_path, output, [], lib_dir, @ext + end + + # Restore stderr + $stderr = original_stderr + + warning_output = stderr.string + refute_includes warning_output, "Gem 'rust_ruby_example' is installing native extensions in /lib directory" + ensure + # Restore original setting + Gem.configuration.install_extension_in_lib = original_setting + $stderr = original_stderr + end + end + + def test_detect_gem_name_from_path + # Test with standard gem structure + path = "/path/to/test_gem/ext/extension" + gem_name = Gem::Ext::CargoBuilder.detect_gem_name_from_path(path) + assert_equal "test_gem", gem_name + + # Test with no ext directory + path = "/path/to/test_gem/lib" + gem_name = Gem::Ext::CargoBuilder.detect_gem_name_from_path(path) + assert_nil gem_name + + # Test with ext at root path + path = "/ext/extension" + gem_name = Gem::Ext::CargoBuilder.detect_gem_name_from_path(path) + assert_nil gem_name + + # Test with empty path + path = "" + gem_name = Gem::Ext::CargoBuilder.detect_gem_name_from_path(path) + assert_nil gem_name + end end diff --git a/test/rubygems/test_gem_ext_ext_conf_builder.rb b/test/rubygems/test_gem_ext_ext_conf_builder.rb index 218c6f3d5e36..84c9737787e0 100644 --- a/test/rubygems/test_gem_ext_ext_conf_builder.rb +++ b/test/rubygems/test_gem_ext_ext_conf_builder.rb @@ -225,4 +225,129 @@ def configure_args(args = nil) RbConfig::CONFIG.delete "configure_args" end end + + def test_class_build_with_lib_placement_warning + if Gem.java_platform? + pend("failing on jruby") + end + + if vc_windows? && !nmake_found? + pend("test_class_build_with_lib_placement_warning skipped - nmake not found") + end + + # Set up a gem-like directory structure + gem_dir = File.join @tempdir, "test_gem" + ext_dir = File.join gem_dir, "ext", "extension" + lib_dir = File.join gem_dir, "lib" + + FileUtils.mkdir_p ext_dir + FileUtils.mkdir_p lib_dir + + File.open File.join(ext_dir, "extconf.rb"), "w" do |extconf| + extconf.puts "require 'mkmf'\ncreate_makefile 'foo'" + end + + # Mock the install_extension_in_lib setting to true + original_setting = Gem.install_extension_in_lib + Gem.configuration.install_extension_in_lib = true + + # Capture stderr for warning + require "stringio" + stderr = StringIO.new + original_stderr = $stderr + $stderr = stderr + + output = [] + + begin + result = Gem::Ext::ExtConfBuilder.build "extconf.rb", @dest_path, output, [], lib_dir, ext_dir + + # Restore stderr + $stderr = original_stderr + + warning_output = stderr.string + assert_includes warning_output, "Gem 'test_gem' is installing native extensions in /lib directory" + assert_includes warning_output, "Consider moving extensions to /ext directory for better organization" + assert_includes warning_output, "Set install_extension_in_lib: true in your .gemrc to maintain current behavior" + ensure + # Restore original setting + Gem.configuration.install_extension_in_lib = original_setting + $stderr = original_stderr + end + end + + def test_class_build_without_lib_placement_warning_when_false + if Gem.java_platform? + pend("failing on jruby") + end + + if vc_windows? && !nmake_found? + pend("test_class_build_without_lib_placement_warning_when_false skipped - nmake not found") + end + + # Set up a gem-like directory structure + gem_dir = File.join @tempdir, "test_gem" + ext_dir = File.join gem_dir, "ext", "extension" + lib_dir = File.join gem_dir, "lib" + + FileUtils.mkdir_p ext_dir + FileUtils.mkdir_p lib_dir + + File.open File.join(ext_dir, "extconf.rb"), "w" do |extconf| + extconf.puts "require 'mkmf'\ncreate_makefile 'foo'" + end + + # Mock the install_extension_in_lib setting to false + original_setting = Gem.install_extension_in_lib + Gem.configuration.install_extension_in_lib = false + + # Capture stderr for warning + require "stringio" + stderr = StringIO.new + original_stderr = $stderr + $stderr = stderr + + output = [] + + begin + result = Gem::Ext::ExtConfBuilder.build "extconf.rb", @dest_path, output, [], lib_dir, ext_dir + + # Restore stderr + $stderr = original_stderr + + warning_output = stderr.string + refute_includes warning_output, "Gem 'test_gem' is installing native extensions in /lib directory" + ensure + # Restore original setting + Gem.configuration.install_extension_in_lib = original_setting + $stderr = original_stderr + end + end + + def test_detect_gem_name_from_path + # Test with standard gem structure + path = "/path/to/test_gem/ext/extension" + gem_name = Gem::Ext::ExtConfBuilder.detect_gem_name_from_path(path) + assert_equal "test_gem", gem_name + + # Test with nested structure + path = "/path/to/nested/test_gem/ext/extension" + gem_name = Gem::Ext::ExtConfBuilder.detect_gem_name_from_path(path) + assert_equal "test_gem", gem_name + + # Test with no ext directory + path = "/path/to/test_gem/lib" + gem_name = Gem::Ext::ExtConfBuilder.detect_gem_name_from_path(path) + assert_nil gem_name + + # Test with ext at root + path = "/ext/extension" + gem_name = Gem::Ext::ExtConfBuilder.detect_gem_name_from_path(path) + assert_nil gem_name + + # Test with empty path + path = "" + gem_name = Gem::Ext::ExtConfBuilder.detect_gem_name_from_path(path) + assert_nil gem_name + end end diff --git a/test/rubygems/test_require.rb b/test/rubygems/test_require.rb index f63c23c3159d..e4ccf0c3d920 100644 --- a/test/rubygems/test_require.rb +++ b/test/rubygems/test_require.rb @@ -248,8 +248,8 @@ def test_activate_via_require_respects_loaded_files lib_dir = File.expand_path("../lib", __dir__) rubylibdir = File.realdirpath(RbConfig::CONFIG["rubylibdir"]) if rubylibdir == lib_dir - # testing in the ruby repository where RubyGems' lib/ == stdlib lib/ - # In that case we want to move the stdlib lib/ to still be after b-2 in $LOAD_PATH + # testing in the ruby repository where RubyGems' /lib == stdlib lib/ + # In that case we want to move the stdlib /lib to still be after b-2 in $LOAD_PATH lp = $LOAD_PATH.dup $LOAD_PATH.delete lib_dir $LOAD_PATH.push lib_dir @@ -283,7 +283,7 @@ def test_activate_via_require_respects_loaded_files assert_includes $LOAD_PATH, b2.full_require_paths[0] assert_includes $LOAD_PATH, rubylibdir message = proc { - "this test relies on the b-2 gem lib/ to be before stdlib to make sense\n" + + "this test relies on the b-2 gem /lib to be before stdlib to make sense\n" + $LOAD_PATH.pretty_inspect } assert_operator $LOAD_PATH.index(b2.full_require_paths[0]), :<, $LOAD_PATH.index(rubylibdir), message