Bug #16896: MakeMakefile methods should be private

• Status: Closed
• Assignee: -
• Target version: -
• ruby -v: ruby 2.7.1p83 (2020-03-31 revision a0c7c23c9c) [x86_64-linux]
• Backport: 2.5: UNKNOWN, 2.6: REQUIRED, 2.7: REQUIRED

[Eregon (Benoit Daloze)]

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>'

[hsbt (Hiroshi SHIBATA)]

• Description updated (diff)

[larskanis (Lars Kanis)]

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.

[nobu (Nobuyoshi Nakada)]

• Status changed from Open to Feedback

Why do you need to use mkmf outside extconf.rb?

[nobu (Nobuyoshi Nakada)]

I found that ext/-test-/cxxanyargs/extconf.rb calls try_link with a receiver.

[shyouhei (Shyouhei Urabe)]

I'd be happy to rewrite cxxanyargs' extconf.rb if there is an alternative way without a receiver.

[nobu (Nobuyoshi Nakada)]

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

[Eregon (Benoit Daloze)]

• Description updated (diff)

[Eregon (Benoit Daloze)]

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?

[Eregon (Benoit Daloze)]

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).

[nobu (Nobuyoshi Nakada)]

Eregon (Benoit Daloze) wrote in #note-8:

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?

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.

[Eregon (Benoit Daloze)]

Right, it's used here: https://github.com/ruby/ruby/blob/5d02c1dd14648d95178ac5ec756f5b03fe00d549/ext/-test-/cxxanyargs/extconf.rb#L12
The PR sounds good then, thanks!

[nobu (Nobuyoshi Nakada)]

• Status changed from Feedback to Closed

Applied in changeset git|c2a6295ec04a191c689d22254ac1ad5d665e27ad.

---

Make the mkmf methods private in the global [Bug #16896]

[nobu (Nobuyoshi Nakada)]

• Backport changed from 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN to 2.5: UNKNOWN, 2.6: REQUIRED, 2.7: REQUIRED