Project

General

Profile

Bug #15425

Store MJIT header into Ruby versioned directory.

Added by vo.x (Vit Ondruch) 12 months ago. Updated 1 day ago.

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

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"


Related issues

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

History

Updated by nobu (Nobuyoshi Nakada) 12 months 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) 12 months 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) 11 months ago

  • Status changed from Feedback to Open

Updated by mame (Yusuke Endoh) 2 days ago

  • Assignee set to k0kubun (Takashi Kokubun)

Updated by nobu (Nobuyoshi Nakada) 2 days 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) 2 days 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) 1 day 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) 1 day ago

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

Updated by vo.x (Vit Ondruch) 1 day ago

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

Updated by vo.x (Vit Ondruch) 1 day ago

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

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

Also available in: Atom PDF