Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compile task modifies RbConfig values #202

Closed
bjfish opened this issue Feb 2, 2022 · 8 comments · Fixed by #222
Closed

Compile task modifies RbConfig values #202

bjfish opened this issue Feb 2, 2022 · 8 comments · Fixed by #222

Comments

@bjfish
Copy link

bjfish commented Feb 2, 2022

The compile task currently modifies RbConfig values which should be avoided since these are intended to be read only and can cause issues for other users:

%w[sitearchdir sitelibdir].each do |dir|
siteconf.puts "RbConfig::MAKEFILE_CONFIG['#{dir}'] = dest_path"
siteconf.puts "RbConfig::CONFIG['#{dir}'] = dest_path"
end

@eregon
Copy link

eregon commented Feb 2, 2022

For context, this caused a fairly complex issue in TruffleRuby: oracle/truffleruby#2586
RubyGems uses RbConfig::CONFIG['sitelibdir'] to find out the initial load path (https://github.com/rubygems/rubygems/blob/c98464e8e859157375177d2b88d9ee01c1355d3f/lib/rubygems.rb#L573-L581), and because that path is changed by rake compiler it doesn't find it anymore, and insert incorrectly in the $LOAD_PATH, loading incorrect gem versions.
This could happen on CRuby too with --disable-gems but luckily it does not thanks to CRuby setting the @gem_prelude_index, a pretty well hidden "hack" which saves CRuby here.
TruffleRuby will do the same for compatibility, but I believe no gem should mutate RbConfig as it can cause issue by anything else using it.

@eregon
Copy link

eregon commented Feb 2, 2022

This issue was introduced in f48d669 / #191, cc @larskanis

@eregon
Copy link

eregon commented Feb 2, 2022

In particular this breaks using Bundler (except the Bundler shipped with TruffleRuby, that works fine because there is then a single Bundler version) + the latest TruffleRuby release (22.0), and that's only fixable here.

@kou
Copy link
Member

kou commented Feb 20, 2022

@larskanis Can we use make install sitearchdir=... instead of rewriting RbConfig::CONFIG["sitearchdir"]?

@larskanis
Copy link
Member

As written in #191, rubygems is using the same mechanism of changing RbConfig values when installing gems. That's why I didn't had a bad conscience proposing this.

Can we use make install sitearchdir=... instead of rewriting RbConfig::CONFIG["sitearchdir"]?

I think so. mkmf.rb seems to not use the variables for anything but passing it into the Makefile. This could make the siteconf file obsolete.

@eregon
Copy link

eregon commented Feb 21, 2022

Right, it is unfortunate RubyGems uses that, and it seems from a long time ago.
The only "advantage" is RubyGems is fully loaded by the time it uses that code so it doesn't trigger the issue of RubyGems not finding where to insert in $LOAD_PATH.
But of course it could break anything else depending on RbConfig::CONFIG values being correct.
I'll file an issue to RubyGems about that.

I think so. mkmf.rb seems to not use the variables for anything but passing it into the Makefile. This could make the siteconf file obsolete.

That would be great :)

@kou
Copy link
Member

kou commented Feb 22, 2022

Could @bjfish , @eregon or someone try implementing #202 (comment) approach and confirm whether the approach works with TruffleRuby?

@eregon
Copy link

eregon commented Jun 15, 2022

PR to fix it in RubyGems: rubygems/rubygems#5626
@larskanis Could you do the same here?

@kou kou closed this as completed in #222 Aug 1, 2023
kou added a commit that referenced this issue Aug 1, 2023
closes: GH-202

Instead of creating `.rake-compiler-siteconf.rb` to override
`RbConfig::MAKEFILE_CONFIG` and `RbConfig::CONFIG`, set environment
variables to `make install` command. Since `RbConfig` is suppose to be
readonly, we shouldn't overwrite its values.

Additionally, this update addresses a bug caused by
`.rake-compiler-siteconf.rb`. The file's hardcoded destination path
causes `rake-compiler` to consistently create files in that location,
even if the project is renamed or moved. Consequently, changes to the
gem project's path do not trigger the file task to update the file. This
fix resolves this issue.

---------

Co-authored-by: Sutou Kouhei <kou@clear-code.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants