Skip to content
This repository has been archived by the owner on Jun 1, 2023. It is now read-only.

[scripts] use RUBY_PLATFORM instead of RbConfig to check platform details for Javy installation #1853

Merged
merged 2 commits into from Dec 14, 2021

Conversation

jacobsteves
Copy link
Member

@jacobsteves jacobsteves commented Dec 14, 2021

WHY are these changes introduced?

Mac machines come preinstalled with a universal distribution of Ruby created by Apple that includes both arm64 and x86 binaries, and will use the appropriate one depending on the cpu of the machine. There is a bug (or a confusing feature?) in the distribution with the use of RbConfig. No matter what CPU you're on (intel or M1) the RbConfig sets the host_cpu field to x86. The current approach to installing Javy checks this variable and downloads the appropriate binary, but if you're on an M1 machine, that means we download the binary for Intel machines. When you install a new version of ruby with chruby/vrm/brew, then this problem disappears. However, Javy should work out of the box.

WHAT is this pull request doing?

Instead of using RbConfig::CONFIG, use RUBY_PLATFORM constant. This constant reports the host machine cpu, even when using a universal distribution. For arm it reports universal.arm64-darwin<version> and for x86 it reports universal.x86_64-darwin<version>. I am currently downloading linux and windows VMs to verify that it works there, but there should be no problems as there is code out there that does the same thing.

How to test your changes?

Ideally, you would test this on M1 and Intel machines. I have tested on both myself, though.

  1. Check your ruby installation
    • Open a ruby console and check RUBY_PLATFORM. It should give you the details on if you're using a universal distribution or a platform specific one.
    • You can also check which ruby in the shell. /usr/bin/ruby indicates that you're using the default system ruby install.
  2. Change which ruby version you want to use
    • To use the default system ruby (universal installation)
      • If you have not manually changed your ruby version with chruby/rvm/brew, navigate to a folder outside of any ruby dev project and follow step 1. to check your installation.
      • If that doesn't work, try running chruby_use 2.7.1.
      • If that also doesn't work and you've installed custom versions of ruby already yourself, you may have to update your $PATH to use /usr/bin/ruby.
    • To use an arch specific installation of ruby:
      • The easiest way is to go into the shopify-cli directory and run dev up. This will install ruby 2.7.5. To use this ruby version, create a temp/ folder in the repo and create your scripts there.
  3. Create a new typscript script
  4. Run npm run prebuild to compile the TS into JS
  5. Run shopify script javy -i build/index.js -o build/index.wasm
  6. Verify that a new WASM file was created at build/index.wasm

My 🎩

Using a universal installation on M1
Using an arm installation on M1

Update checklist

  • I've added a CHANGELOG entry for this PR (if the change is public-facing)
  • I've considered possible cross-platform impacts (Mac, Linux, Windows).
  • I've left the version number as is (we'll handle incrementing this when releasing).

@jacobsteves jacobsteves requested review from jbourassa, andrewhassan, a team, pepicrft and amcaplan and removed request for a team December 14, 2021 16:10
Copy link
Contributor

@andrewhassan andrewhassan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's unfortunate that RbConfig::CONFIG doesn't have the correct CPU architecture. I did a little research online and it seems like it might be due to how the universal Ruby binary is compiled. Until RbConfig is fixed (if it ever is), I think that using RUBY_PLATFORM is a good compromise. Thanks for fixing this!

Copy link
Contributor

@jbourassa jbourassa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that works, but with one caveat that JRuby will not work with this. I don't expect folks to use the CLI on JRuby (no reason to -- slower start time etc.), and it's not in our testing matrix either 👍.

@jacobsteves
Copy link
Member Author

one caveat that JRuby will not work with this. I don't expect folks to use the CLI on JRuby (no reason to -- slower start time etc.), and it's not in our testing matrix either

Yeah, that is true. I agree that most people probably wont use this on JRuby, but if we ever find out that we want to support JRuby we could add in a couple more cases to the switch statements

@jacobsteves jacobsteves merged commit 8a2f7ae into main Dec 14, 2021
@jacobsteves jacobsteves deleted the js.javy-universal-distro branch December 14, 2021 18:19
@gonzaloriestra gonzaloriestra mentioned this pull request Dec 21, 2021
@shopify-shipit shopify-shipit bot temporarily deployed to rubygems December 22, 2021 07:00 Inactive
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants