Project

General

Profile

Actions

Bug #15425

open

Store MJIT header into Ruby versioned directory.

Added by vo.x (Vit Ondruch) over 2 years ago. Updated over 1 year 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.ClosedActions

Updated by nobu (Nobuyoshi Nakada) over 2 years 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 2 years 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.

Actions #3

Updated by vo.x (Vit Ondruch) over 2 years ago

  • Status changed from Feedback to Open

Updated by mame (Yusuke Endoh) over 1 year ago

  • Assignee set to k0kubun (Takashi Kokubun)

Updated by nobu (Nobuyoshi Nakada) over 1 year 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) over 1 year 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) over 1 year 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
Actions #8

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

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

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

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

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

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

Updated by vo.x (Vit Ondruch) over 1 year 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) over 1 year 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) over 1 year 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)
Actions

Also available in: Atom PDF