Make the setup script generated by bundle install --standalone multiplatform

[gustavothecoder]

What was the end-user or developer problem that led to this PR?

As discussed in #3186, removing the hardcoded platform from setup.rb allows --standalone mode to support native extensions for multiple platforms, which is an improvement to the feature.

What is your fix for the problem, implemented in this PR?

I created a path helper that formats RbConfig::CONFIG['arch'] to be Rubygems standard. The helper I created is basically a copy of Gem::Platform#initialize.

My first thought was to copy Gem::Platform#initialize at runtime so that the arch's formatting logic remains centralized, but I haven't been able to figure out a simple way to do this.

Make sure the following tasks are checked

• Describe the problem / feature
• Write tests for features and bug fixes
• Write code to solve the problem
• Make sure you follow the current code style and write meaningful commit messages without tags

[gustavothecoder]

I'm wrong? Is it better to write tests for each platform?

[gustavothecoder]

I'll work on the CI failures.

[deivid-rodriguez]

I created a path helper that formats RbConfig::CONFIG['arch'] to be Rubygems standard. The helper I created is basically a copy of Gem::Platform#initialize.

Is it an exact copy or do you need any changes? Indeed it's quite unfortunate, but I don't have any better ideas? I guess we should at least add a note to Gem::Platform#initialize to note that if the method is changed, the copy needs to be changed too.

Not sure if anyone has any better ideas.

[gustavothecoder]

I created a path helper that formats RbConfig::CONFIG['arch'] to be Rubygems standard. The helper I created is basically a copy of Gem::Platform#initialize.

Is it an exact copy or do you need any changes? Indeed it's quite unfortunate, but I don't have any better ideas? I guess we should at least add a note to Gem::Platform#initialize to note that if the method is changed, the copy needs to be changed too.

Not sure if anyone has any better ideas.

I only copied this case. I agree to the addition of a note.

[deivid-rodriguez]

I see! But it also has some logic from Gem::Platform.local, right? Do you think it'd be possible toc refactor things a bit, so that the exact logic that needs to be kept duplicated can at least live on a separate method? 🤔

[gustavothecoder]

I see! But it also has some logic from Gem::Platform.local, right? Do you think it'd be possible toc refactor things a bit, so that the exact logic that needs to be kept duplicated can at least live on a separate method? thinking

Right! I forgot this code from Gem::Platform.local.

I think we can move arch = "#{arch}_60" if arch =~ /mswin(?:32|64)$/ from Gem::Platform.local to the case I copied. What do you think?

[deivid-rodriguez]

I think that makes sense!

[gustavothecoder]

@deivid-rodriguez I think this one is ready to go 😁

[deivid-rodriguez]

@gustavothecoder It looks good but looking at the changed resolver specs, this now sounds backwards incompatible? 😬

[gustavothecoder]

@gustavothecoder It looks good but looking at the changed resolver specs, this now sounds backwards incompatible? grimacing

It seems it is 😞

I think it's possible to do something like this:

  def self.local
    arch = RbConfig::CONFIG["arch"]
    force_mswin_version = true
    @local ||= new(arch, force_mswin_version)
  end

  def initialize(arch, force_mswin_version = false)
    case arch
    when Array then
      @cpu, @os, @version = arch
    when String then # Update Bundler::Standalone#add_arch_formatter whenever this case is changed.
      arch = "#{arch}_60" if force_mswin_version && /mswin(?:32|64)$/.match?(arch)
      # ...
    end
  end

Personally, I don't really like this solution. Maybe we shouldn't refactor Gem::Platform for now 🤔

[deivid-rodriguez]

You're probably right. Let's let this settle for a few days in case we come up with better ideas or other maintainers chime in.

[Bo98]

Doesn't necessarily have to be this PR, but another case to consider is prebuilt native gems such as nokogiri and sorbet-static. They install with a suffix to the version:

$:.unshift File.expand_path("#{__dir__}/../#{RUBY_ENGINE}/#{Gem.ruby_api_version}/gems/nokogiri-1.15.5-x86_64-darwin/lib")

The exact formatting of the suffix varies from gem to gem unfortunately. Older sorbet-static for example use: sorbet-static-0.5.10461-universal-darwin-22.

However, when generating the setup script in Bundler, we do know the list of possible suffixes given we store all of them in the lockfile so it's possibly a case of writing that list into the setup file and making it select the most appropriate one for the running platform.

[Bo98]

Is it an exact copy or do you need any changes? Indeed it's quite unfortunate, but I don't have any better ideas? I guess we should at least add a note to Gem::Platform#initialize to note that if the method is changed, the copy needs to be changed too.

Not sure if anyone has any better ideas.

Just to brainstorm an idea here while I'm passing: perhaps we could have the shared code in its own standalone (no requires) file in RubyGems, which is required & called within RubyGems where it's usually needed and then for the setup script we File.read that to copy into setup.rb (probably stripping the frozen_string_literal line if it's there). Edit: actually RubyGems and Bundler aren't necessarily synced so that wouldn't work as is but the idea could maybe work if you copy the file into both RubyGems and Bundler and have maybe a GitHub Action to ensure they are synced. A bit odd I guess but I'm just throwing things out there.

---

Anyway, as for the mswin problem: I guess what we could do is have the shared code like:

def platform_for_arch(arch) # or whatever you want to call it
  arch = arch.split "-"
  # as usual...
end

then in gem/platform we have:

def initialize(arch)
    case arch
    when Array then
      @cpu, @os, @version = arch
    when String then
      @cpu, @os, @version = platform_for_arch(arch)

and in setup.rb we have:

def local_platform
  arch = RbConfig::CONFIG["arch"]
  arch = "#{arch}_60" if arch =~ /mswin(?:32|64)$/
  platform_for_arch(arch).compact.join "-"
end

if that makes things any easier and/or clearer?

[gustavothecoder]

Thank for contributing to the discussion @Bo98. I tried to implement your approach, and with some tweaks, I think it works. I already updated this PR with it, so I'll be grateful if you can give me feedback 😄

About the prebuilt native gems, certainly we need to handle this case to truly have a multiplatform setup script, but I can't solve this problem right now, so I would like not to increase the scope of this PR.