Project

General

Profile

Bug #15425

Store MJIT header into Ruby versioned directory.

Added by vo.x (Vit Ondruch) over 1 year ago. Updated 4 months ago.

Status:
Open
Priority:
Normal
Target version:
-
ruby -v:
ruby 2.6.0rc2 (2018-12-15 trunk 66408) [x86_64-linux]
[ruby-core:90585]
Tags:

Description

This is a followup of #15391 which fixes JIT to respect the configuration options. However, I still wonder, why the file is versioned and why it is not stored in the versioned directory alongside all other internal Ruby headers. I believe, that while it now respects the header configuration flags, it still does not respect options such as "--with-ruby-version"


Files


Related issues

Related to Ruby master - Bug #16416: Suspicious include directories.OpenActions

Updated by nobu (Nobuyoshi Nakada) over 1 year ago

  • Status changed from Open to Feedback

It should be placed under the directory which has the version and architecture name.
How did you configure it?

Updated by vo.x (Vit Ondruch) over 1 year ago

I am using --with-rubyhdrdir=/usr/include --with-rubyarchhdrdir=/usr/include --enable-multiarch configuration options. This is the output:

$ tree
.
`-- ruby-2.6.0
    |-- ruby
    |   |-- backward
    |   |   |-- classext.h
    |   |   |-- rubyio.h
    |   |   |-- rubysig.h
    |   |   |-- st.h
    |   |   `-- util.h
    |   |-- backward.h
    |   |-- debug.h
    |   |-- defines.h
    |   |-- digest.h
    |   |-- encoding.h
    |   |-- intern.h
    |   |-- io.h
    |   |-- missing.h
    |   |-- onigmo.h
    |   |-- oniguruma.h
    |   |-- re.h
    |   |-- regex.h
    |   |-- ruby.h
    |   |-- st.h
    |   |-- subst.h
    |   |-- thread.h
    |   |-- thread_native.h
    |   |-- util.h
    |   |-- version.h
    |   `-- vm.h
    |-- ruby-2.6.0
    |   `-- x86_64-linux
    |-- ruby.h
    `-- x86_64-linux
        |-- rb_mjit_min_header-2.6.0.h
        `-- ruby
            `-- config.h

7 directories, 28 files

Different headers respect the configuration differently. The header should be probably placed on the same location as "config.h" for consistency. The ruby-2.6.0/ruby-2.6.0/x86_64-linux should not be created at all.

#3

Updated by vo.x (Vit Ondruch) about 1 year ago

  • Status changed from Feedback to Open

Updated by mame (Yusuke Endoh) 4 months ago

  • Assignee set to k0kubun (Takashi Kokubun)

Updated by nobu (Nobuyoshi Nakada) 4 months ago

It seems stored in the rubyarchhdrdir already.

$ LC_ALL=C tree usr/include/
usr/include/
|-- rb_mjit_min_header-2.7.0.h
|-- ruby
|   |-- assert.h
|   |-- backward
|   |   |-- classext.h
|   |   |-- cxxanyargs.hpp
|   |   |-- rubyio.h
|   |   |-- rubysig.h
|   |   |-- st.h
|   |   `-- util.h
|   |-- backward.h
|   |-- config.h
|   |-- debug.h
|   |-- defines.h
|   |-- digest.h
|   |-- encoding.h
|   |-- intern.h
|   |-- io.h
|   |-- missing.h
|   |-- onigmo.h
|   |-- oniguruma.h
|   |-- re.h
|   |-- regex.h
|   |-- ruby.h
|   |-- st.h
|   |-- subst.h
|   |-- thread.h
|   |-- thread_native.h
|   |-- util.h
|   |-- version.h
|   `-- vm.h
`-- ruby.h

2 directories, 30 files

Updated by k0kubun (Takashi Kokubun) 4 months ago

why the file is versioned

Basically rubyarchhdrdir directory should protect the header from different Ruby versions, but you almost proved that it's necessary when you passed --with-rubyhdrdir=/usr/include --with-rubyarchhdrdir=/usr/include. At least 2.6's header can't be used from 2.7.

why it is not stored in the versioned directory alongside all other internal Ruby headers

IIRC, nobu chose the directory to support multi-arch (or universal?) build, and it seems to follow his intention as he quoted.

The header should be probably placed on the same location as "config.h" for consistency. The ruby-2.6.0/ruby-2.6.0/x86_64-linux should not be created at all.

I have no idea why ruby-2.6.0/ruby-2.6.0/x86_64-linux is created, but rb_mjit_min_header-2.6.0.h is not placed in that directory and so I believe it should be a separated ticket.

it still does not respect options such as "--with-ruby-version"

So maybe respecting --with-ruby-version in the header file name (or removing the version fragment) is the only thing we need to discuss/implement here?

Updated by vo.x (Vit Ondruch) 4 months ago

So maybe respecting --with-ruby-version in the header file name (or removing the version fragment) is the only thing we need to discuss/implement here?

That would be good start. The version there is probably useless.

This is the configuration I used:

$ ./configure --with-ruby-version='foo'

And this is the result:

$ cd /usr/local/include/
$ tree
.
└── ruby-foo
    ├── ruby
    │   ├── assert.h
    │   ├── backward
    │   │   ├── classext.h
    │   │   ├── cxxanyargs.hpp
    │   │   ├── rubyio.h
    │   │   ├── rubysig.h
    │   │   ├── st.h
    │   │   └── util.h
    │   ├── backward.h
    │   ├── debug.h
    │   ├── defines.h
    │   ├── digest.h
    │   ├── encoding.h
    │   ├── intern.h
    │   ├── io.h
    │   ├── missing.h
    │   ├── onigmo.h
    │   ├── oniguruma.h
    │   ├── regex.h
    │   ├── re.h
    │   ├── ruby.h
    │   ├── st.h
    │   ├── subst.h
    │   ├── thread.h
    │   ├── thread_native.h
    │   ├── util.h
    │   ├── version.h
    │   └── vm.h
    ├── ruby-
    │   └── x86_64-linux
    ├── ruby.h
    └── x86_64-linux
        ├── rb_mjit_min_header-2.7.0.h
        └── ruby
            └── config.h

7 directories, 30 files
#8

Updated by vo.x (Vit Ondruch) 4 months ago

  • Related to Bug #16415: MJIT is using/creating some suspicious include directories added
#9

Updated by vo.x (Vit Ondruch) 4 months ago

  • Related to deleted (Bug #16415: MJIT is using/creating some suspicious include directories)
#10

Updated by vo.x (Vit Ondruch) 4 months ago

  • Related to Bug #16416: Suspicious include directories. added

Updated by vo.x (Vit Ondruch) 4 months ago

Isn't part of the problem, that the ruby_version variable is read much later than it is used for configuration of MJIT header names? These are the relevant lines:

https://github.com/ruby/ruby/blob/40026a408df5e3576380f6c1d8bf6c119fa2e32b/configure.ac#L3099
https://github.com/ruby/ruby/blob/40026a408df5e3576380f6c1d8bf6c119fa2e32b/configure.ac#L3719

Updated by vo.x (Vit Ondruch) 4 months ago

The two attached patches might improve the issue. The 0001-Store-MJIT-header-into-Ruby-versioned-directory-1.patch includes the ruby_version in the header name, while the 0001-Store-MJIT-header-into-Ruby-versioned-directory-2.patch removes the version from the header altogether, because the header lives in versioned directory anyway.

However, they still don't move the file to the same place as the config.h :/

Updated by nobu (Nobuyoshi Nakada) 4 months ago

Why is this needed?
It seems having no effects.

--- a/configure.ac
+++ b/configure.ac
@@ -3096,9 +3096,6 @@ AC_ARG_ENABLE(multiarch,
          [multiarch=], [unset multiarch])
 AS_IF([test ${multiarch+set}], [
    AC_DEFINE(ENABLE_MULTIARCH)
-   MJIT_HEADER_INSTALL_DIR=include/'${arch}/${RUBY_VERSION_NAME}'
-], [
-   MJIT_HEADER_INSTALL_DIR=include/'${RUBY_VERSION_NAME}/${arch}'
 ])

 archlibdir='${libdir}/${arch}'
@@ -3772,6 +3769,12 @@ AS_IF([test "${LOAD_RELATIVE+set}"], [
     RUBY_EXEC_PREFIX=''
 ])

+AS_IF([test ${multiarch+set}], [
+   MJIT_HEADER_INSTALL_DIR=include/'${arch}/${RUBY_VERSION_NAME}'
+], [
+   MJIT_HEADER_INSTALL_DIR=include/'${RUBY_VERSION_NAME}/${arch}'
+])
+
 AC_SUBST(RUBY_EXEC_PREFIX)

 AC_SUBST(libdirname, ${multiarch+arch}libdir)

Also available in: Atom PDF