Project

General

Profile

Bug #16896

MakeMakefile methods should be private

Added by Eregon (Benoit Daloze) 3 months ago. Updated about 1 month ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
-
ruby -v:
ruby 2.7.1p83 (2020-03-31 revision a0c7c23c9c) [x86_64-linux]
[ruby-core:98391]

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

Updated by hsbt (Hiroshi SHIBATA) 3 months ago

  • Description updated (diff)

Updated by larskanis (Lars Kanis) about 1 month 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:

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) about 1 month ago

  • Status changed from Open to Feedback

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

Updated by nobu (Nobuyoshi Nakada) about 1 month ago

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

Updated by shyouhei (Shyouhei Urabe) about 1 month ago

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

Updated by nobu (Nobuyoshi Nakada) about 1 month 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

#7

Updated by Eregon (Benoit Daloze) about 1 month ago

  • Description updated (diff)

Updated by Eregon (Benoit Daloze) about 1 month 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) about 1 month 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) about 1 month ago

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.

#12

Updated by nobu (Nobuyoshi Nakada) about 1 month ago

  • Status changed from Feedback to Closed

Applied in changeset git|c2a6295ec04a191c689d22254ac1ad5d665e27ad.


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

#13

Updated by nobu (Nobuyoshi Nakada) about 1 month ago

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

Also available in: Atom PDF