Bug #16896
MakeMakefile methods should be private
Description
Right now they are public, and since mkmf.rb does include MakeMakefile
it defines a lot of public methods on all objects.
$ ruby -e 'a=1.public_methods; require "mkmf"; b=1.public_methods; puts (b-a).sort' append_cflags append_cppflags append_ldflags append_library arg_config cc_command cc_config check_signedness check_sizeof checking_for checking_message configuration conftest_source convertible_int cpp_command cpp_include create_header create_makefile create_tmpsrc depend_rules dir_config dummy_makefile each_compile_rules egrep_cpp enable_config find_executable find_executable0 find_header find_library find_type have_const have_devel? have_framework have_func have_header have_library have_macro have_struct_member have_type have_typeof? have_var init_mkmf install_dirs install_files install_rb libpath_env libpathflag link_command link_config log_src macro_defined? map_dir merge_libs message mkintpath mkmf_failed modified? pkg_config relative_from scalar_ptr_type? scalar_type? split_libs timestamp_file try_cflags try_compile try_const try_constant try_cpp try_cppflags try_do try_func try_header try_ldflags try_link try_link0 try_run try_signedness try_static_assert try_type try_var typedef_expr what_type? winsep with_cflags with_config with_cppflags with_destdir with_ldflags with_werror xpopen xsystem
Since I expect all those methods are called without a receiver, I think they could be private.
Thoughts?
The current situation makes extconf.rb brittle to method name conflicts.
One real bug caused by this is FFI doesn't work when mkmf is required (filed as https://github.com/ffi/ffi/issues/776 ):
$ ruby -rmkmf -rffi -e 'module A; class A < FFI::Struct; layout :a, :int; end; end' |& cat /home/eregon/.rubies/ruby-2.7.1/lib/ruby/2.7.0/mkmf.rb:1270:in `find_type': wrong number of arguments (given 1, expected 2+) (ArgumentError) from /home/eregon/.rubies/ruby-2.7.1/lib/ruby/gems/2.7.0/gems/ffi-1.12.2/lib/ffi/struct.rb:271:in `find_type' from /home/eregon/.rubies/ruby-2.7.1/lib/ruby/gems/2.7.0/gems/ffi-1.12.2/lib/ffi/struct.rb:265:in `find_field_type' from /home/eregon/.rubies/ruby-2.7.1/lib/ruby/gems/2.7.0/gems/ffi-1.12.2/lib/ffi/struct.rb:305:in `array_layout' from /home/eregon/.rubies/ruby-2.7.1/lib/ruby/gems/2.7.0/gems/ffi-1.12.2/lib/ffi/struct.rb:217:in `layout' from -e:1:in `<class:A>' from -e:1:in `<module:A>' from -e:1:in `<main>'
Updated by larskanis (Lars Kanis) 7 months ago
I agree with Eregon (Benoit Daloze) that making the above methods private would be desirable. However I searched on github for code examples of public methods of MakeMakefile
and found at least these two:
- https://github.com/cloudavail/snippets/blob/8f78d6a20f77e7d682b8901192b1bc8f288b779c/ruby/executable_exists_cross_platform/executable_exists_cross_platform.rb
- https://github.com/razor-x/config_curator/blob/b0c0742ba0c36acf66de3eafd23a7d17b210036b/lib/config_curator/utils.rb
I didn't find any gems that use these public functions, but the proposed change seems at least to break some other code. Not sure if this is valuable enough to block the proposed change.
Updated by nobu (Nobuyoshi Nakada) 7 months ago
- Status changed from Open to Feedback
Why do you need to use mkmf outside extconf.rb?
Updated by nobu (Nobuyoshi Nakada) 7 months ago
I found that ext/-test-/cxxanyargs/extconf.rb
calls try_link
with a receiver.
Updated by shyouhei (Shyouhei Urabe) 7 months ago
I'd be happy to rewrite cxxanyargs' extconf.rb if there is an alternative way without a receiver.
Updated by nobu (Nobuyoshi Nakada) 7 months ago
shyouhei (Shyouhei Urabe) wrote in #note-5:
I'd be happy to rewrite cxxanyargs' extconf.rb if there is an alternative way without a receiver.
I don't think you have to rewrite it, it feels better way for the future.
https://github.com/ruby/ruby/pull/3311
Updated by Eregon (Benoit Daloze) 7 months ago
I think MakeMakefile.foo
usages are fine, so how about using module_function
, so on include
they are private, and when used on the singleton class (MakeMakefile.foo
) they are public?
Updated by Eregon (Benoit Daloze) 7 months ago
nobu (Nobuyoshi Nakada) wrote in #note-3:
Why do you need to use mkmf outside extconf.rb?
Actually the issue above happened inside extconf.rb
.
But extconf.rb
might require anything, including FFI
, so there can conflicts in extconf.rb
itself.
See https://github.com/oracle/truffleruby/issues/1896#issuecomment-629438770 for more context, where this caused a bug on TruffleRuby (because there the socket
stdlib uses ffi
).
Updated by nobu (Nobuyoshi Nakada) 7 months ago
Eregon (Benoit Daloze) wrote in #note-8:
I think
MakeMakefile.foo
usages are fine, so how about usingmodule_function
, so oninclude
they are private, and when used on the singleton class (MakeMakefile.foo
) they are public?
I have tried module_function
first, and found cxx.try_link
.
As I prefer per-instance configuration over process-global, don't want to disallow this usage.
Updated by Eregon (Benoit Daloze) 7 months ago
Right, it's used here: https://github.com/ruby/ruby/blob/5d02c1dd14648d95178ac5ec756f5b03fe00d549/ext/-test-/cxxanyargs/extconf.rb#L12
The PR sounds good then, thanks!
Updated by nobu (Nobuyoshi Nakada) 7 months ago
- Status changed from Feedback to Closed
Applied in changeset git|c2a6295ec04a191c689d22254ac1ad5d665e27ad.
Make the mkmf methods private in the global [Bug #16896]
Updated by nobu (Nobuyoshi Nakada) 7 months ago
- Backport changed from 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN to 2.5: UNKNOWN, 2.6: REQUIRED, 2.7: REQUIRED