Feature #15425
closedStore MJIT header into Ruby versioned directory.
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
Updated by nobu (Nobuyoshi Nakada) over 5 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 5 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.
Updated by vo.x (Vit Ondruch) about 5 years ago
- Status changed from Feedback to Open
Updated by mame (Yusuke Endoh) over 4 years ago
- Assignee set to k0kubun (Takashi Kokubun)
Updated by nobu (Nobuyoshi Nakada) over 4 years 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 4 years 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 4 years 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
Updated by vo.x (Vit Ondruch) over 4 years ago
- Related to Bug #16415: MJIT is using/creating some suspicious include directories added
Updated by vo.x (Vit Ondruch) over 4 years ago
- Related to deleted (Bug #16415: MJIT is using/creating some suspicious include directories)
Updated by vo.x (Vit Ondruch) over 4 years ago
- Related to Bug #16416: Suspicious include directories. added
Updated by vo.x (Vit Ondruch) over 4 years 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 4 years ago
- File 0001-Store-MJIT-header-into-Ruby-versioned-directory-1.patch 0001-Store-MJIT-header-into-Ruby-versioned-directory-1.patch added
- File 0001-Store-MJIT-header-into-Ruby-versioned-directory-2.patch 0001-Store-MJIT-header-into-Ruby-versioned-directory-2.patch added
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 4 years 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)
Updated by k0kubun (Takashi Kokubun) almost 3 years ago
- Status changed from Open to Feedback
Updated by jeremyevans0 (Jeremy Evans) over 2 years ago
- Tracker changed from Bug to Feature
- ruby -v deleted (
ruby 2.6.0rc2 (2018-12-15 trunk 66408) [x86_64-linux]) - Backport deleted (
2.4: UNKNOWN, 2.5: UNKNOWN)
Updated by vo.x (Vit Ondruch) 6 months ago
This is probably no issue for Ruby 3.3. The header in question was dropped by git|31f4b2d86bfbc753cec9be376719acc4b120e944. So this can be closed (I used to have powers to close the tickets myself, how have I lost them? 🤔)
Updated by Eregon (Benoit Daloze) 6 months ago
- Status changed from Feedback to Closed