Project

General

Profile

Actions

Bug #19289

closed

RbConfig::CONFIG["STRIP"] should keep `ruby_abi_version` and `ruby_abi_version` should always be part of Ruby

Added by Eregon (Benoit Daloze) over 1 year ago. Updated about 1 year ago.

Status:
Closed
Assignee:
-
Target version:
-
[ruby-core:111529]

Description

From https://github.com/grpc/grpc/pull/31970 and https://github.com/redis-rb/redis-client/issues/58

First, I think we could add -K rb_abi_version to RbConfig::CONFIG["STRIP"] so it's automatically kept if RbConfig::CONFIG["STRIP"] is used (and that should be used if one strips any native extension).

Second, I think it would be much better if the symbol is kept also for releases.
The check could be kept too for safety (e.g., it can detect Ruby 3.3.0 gems used by Ruby 3.2.0), the value of rb_abi_version would just be the same as RbConfig::CONFIG["ruby_version"], i.e., 3.2.0 for Ruby 3.2.x.

Any difference between dev and release builds is a risk of not properly testing the release, and there is proof here that removing the symbol in releases causes troubles.

Doing both of these would avoid complex and brittle logic upstream as in grpc and redis-client to deal with the new symbol.

cc @nobu (Nobuyoshi Nakada) @peterzhu2118 (Peter Zhu)

Updated by Eregon (Benoit Daloze) over 1 year ago

Ah, because the export list file is not given to strip but directly to CC/CXX:

Keeping the symbol in RbConfig::CONFIG["STRIP"] won't help.
So that first suggestion is not going to help in practice.

Instead I think we could add a clean way to detect if there is an ABI version symbol which should be preserved.
For instance, setting RbConfig::CONFIG["ruby_abi_symbol"] to either "rb_abi_version" or nil.

Updated by Eregon (Benoit Daloze) over 1 year ago

Another possibility would be to have some key in RbConfig::CONFIG which contains -Wl,-exported_symbol,_ruby_abi_version or nil,
like https://github.com/redis-rb/redis-client/blob/809bbab767f8e514ccd0d6008116c360435f3026/hiredis-client/ext/redis_client/hiredis/extconf.rb#L61
I think RbConfig::CONFIG["ruby_abi_symbol"] is cleaner and simpler though.

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

Eregon (Benoit Daloze) wrote:

the value of rb_abi_version would just be the same as RbConfig::CONFIG["ruby_version"]

Just FTR, don't forget that ruby_version can be arbitrary string:

https://github.com/ruby/ruby/blob/90a80eb076429978e720e11fb17a3cbb96de3454/configure.ac#L4177

Updated by Eregon (Benoit Daloze) over 1 year ago

vo.x (Vit Ondruch) wrote in #note-3:

Just FTR, don't forget that ruby_version can be arbitrary string:
https://github.com/ruby/ruby/blob/90a80eb076429978e720e11fb17a3cbb96de3454/configure.ac#L4177

Interesting, I think we should define RbConfig::CONFIG["abi_version"] (or "ruby_abi_version") if RbConfig::CONFIG["ruby_version"] it not always a proper ABI version.
Many places already assume RbConfig::CONFIG["ruby_version"] is the ABI version though, so that won't help much.

That said this kind of configuration is IMHO silly, changing it is basically guaranteed to break things, so a better way would be remove such configuration since it doesn't work properly (e.g. can reuse gems of another Ruby version).

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

Eregon (Benoit Daloze) wrote in #note-4:

That said this kind of configuration is IMHO silly, changing it is basically guaranteed to break things, so a better way would be remove such configuration

I think it was always meant to allow to install multiple versions of the same version of Ruby, with possibly different configuration and what not. Unfortunately, the name is not always not expressive enough and not all Ruby maintainers know/remember why this variable is there and therefore it might be misunderstood/misused.

BTW I historically proposed to allow the ruby_version to not be used at all, e.g. to be an empty string. This unfortunately have not worked out. Instead, the ruby_version was made mandatory :/

And we still carry this patch in Fedora:

https://src.fedoraproject.org/rpms/ruby/blob/rawhide/f/ruby-2.3.0-ruby_version.patch

and here is relevant PR:

https://github.com/ruby/ruby/pull/2392

Updated by peterzhu2118 (Peter Zhu) over 1 year ago

I'm not sure why ABI version checking was removed for release versions of Ruby. In the development meeting for the feature, it was decided that we'll keep the same behaviour for development and release versions.

ko1: Will the ABI version be included in the release version? Or it is only for master branch?
peter: Curently, the check is done only for development branch
mame: I don't like a check only for development branch. When we try to create a tarball for release, something weird may happen.
ko1: If so, we can leave the ABI version for release branch.

Source

Actions #7

Updated by nobu (Nobuyoshi Nakada) over 1 year ago

  • Subject changed from RbConfig::CONFIG["STRIP"] should keep `rb_abi_version` and `rb_abi_version` should always be part of Ruby to RbConfig::CONFIG["STRIP"] should keep `ruby_abi_version` and `ruby_abi_version` should always be part of Ruby
Actions #8

Updated by nobu (Nobuyoshi Nakada) over 1 year ago

  • Backport changed from 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN to 2.7: DONTNEED, 3.0: DONTNEED, 3.1: DONTNEED, 3.2: REQUIRED
Actions #9

Updated by nobu (Nobuyoshi Nakada) over 1 year ago

  • Status changed from Open to Closed

Applied in changeset git|6f3aff3961a4c5ce87e05096a1a9dcf1055b7647.


[Bug #19289] Retain ruby_abi_version function

A few extension libraries, to hide all symbols except for necessary to
load, hardcode the symbols to be exported in symbol list files for
linker without even checking by have_func. As a workaround for such
libraries, retain ruby_abi_version symbol always even in released
versions for now.

Updated by naruse (Yui NARUSE) about 1 year ago

  • Backport changed from 2.7: DONTNEED, 3.0: DONTNEED, 3.1: DONTNEED, 3.2: REQUIRED to 2.7: DONTNEED, 3.0: DONTNEED, 3.1: DONTNEED, 3.2: DONE

ruby_3_2 4110137fcfd805de03a8f5569c3d6926959b9363 merged revision(s) 6f3aff3961a4c5ce87e05096a1a9dcf1055b7647.

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0