Install shopify-extensions during gem installation #1496

t6d · 2021-09-01T18:54:43Z

WHY are these changes introduced?

This incorporates the new extension server that will be responsible for powering

shopify extension create
shopify extension build
shopify extension serve

WHAT is this pull request doing?

It introduces a new InstallExtensionServer operation that is run as part of ext/shopify-cli/extconf.rb. The task is responsible for determining the appropriate binary version of shopify-extensions, download it, uncompress it and place it next to the shopify bin stub.

Tasks

Use single release endpoint instead of the release index when fetching information about a release
Ensure that program exists in a controlled manner if either the requested release or one of its assets can't be found
Investigate installation issue on Windows ✅
Decompression and unpacking with Ruby appears to yield a different result than using tar xf. The MD5 checksum differs. It's crucial to set the b flag when writing binary content to a file on Windows. The issue was therefore not related to TAR or GZip.
Determine whether Github's release API is paginated
↪ Yes, it is and it would therefore be easier to use the endpoint for fetching a single release: https://api.github.com/repos/Shopify/shopify-cli-extensions/releases/tags/v0.1.0
Determine platform and architecture programatically
Switch from zip files to tar.gz files for windows releases

Update checklist

~~I've added a CHANGELOG entry for this PR (if the change is public-facing)~~ The changes introduced in this PR are not yet Partner 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).

🎩

rake extensions:install
Verify that ext/shopify-extensions/shopify-extensions version yields v0.1.0
Verify rake build && gem install pkg/shopify-cli-2.3.0.gem (optionally verify that the installed version of the gem does not include the binary by using gem open -e code shopify-cli and double checking that the file ext/shopify-extensions/shopify-extensions does not exist)

ext/shopify-extensions/extconf.rb

t6d · 2021-09-15T16:26:15Z

Windows 🎩 ✅

t6d · 2021-09-15T16:34:24Z

ext/shopify-extensions/extconf.rb

+  STDERR.puts(error.message)
+rescue => error
+  STDERR.puts("Unable to install shopify-extensions: #{error}")
+end


The rescue statements allow the installation of the binary to fail silently. This should eventually be changed to fail the installation and notify the user to file a bug report.

vividviolet

Amazing job 👏 I'll top hat after lunch

.gitignore

ext/shopify-extensions/shopify_extensions.rb

test/ext/shopify-extensions/shopify_extensions_test.rb

pepicrft

As mentioned in one of the comments, I'd avoid the interactions with GitHub API and build upon their conventional URLs instead.

Rakefile

pepicrft · 2021-09-17T10:45:15Z

ext/shopify-extensions/extconf.rb

+MAKEFILE
+
+begin
+  ShopifyExtensions.install(


I might have missed the discussion, but what's the motivation behind pulling the binary at installation time as opposed to vendoring it as part of releasing the package? Is it because we don't want to include the binaries of architectures other than ours?

Have we considered Nokogiri's approach where multiple native gems are generated containing the right binary for each architecture?

Is it because we don't want to include the binaries of architectures other than ours?

That is correct. We've considered this option but dismissed it due to binary size. The binaries are quite largue as each one includes the Go runtime: ~ 10 MB per binary (5 MB compressed)

Have we considered Nokogiri's approach where multiple native gems are generated containing the right binary for each architecture?

We have not. I wasn't aware that this option exists. From the link you provided, it's not immediately evident to me how to actually package and publish OS specific versions of Ruby gems. Do you have any experience with this yourself and could elaborate a bit?

The approach we took follows the one that ESBuild uses to obtain its binary when you install the corresponding node package. Can you expand a bit on the concerns you have with this approach?

@pepibumur, do you see this as a blocker or something that we could improve incrementally. Since we're not releasing to partners before my intermission, I'd be ok to wait with integrating that, but it makes testing this a little more manual.

After some discussion, we decided that this can be an incremental improvement and is no reason to hold back on this PR. Especially now that the installation of shopify-extensions isn't done during gem installation for now. I'll investigate whether I want to pivot the approach when I'm back from intermission.

That said, now that we're no longer making API calls to GitHub but directly construct the download URL, I'm inclined to stick to the current approach.

ext/shopify-extensions/shopify_extensions.rb

Adds a new Ruby extension called `shopify-extensions` to download the development server during installation.

* Supports updating of shopify-extensions through shopify:extensions:update VERSION=vx.y.Z * Supports symlinking a local development build through shopify:extensions:symlink

Removes API calls to Github in favor of constructing the download URL manually.

The version is now stored in a plain text file instead of being part of the Ruby code. This makes updating the contents a little easier.

t6d · 2021-09-17T20:01:28Z

I removed running this code during gem installation for now. The code in this PR is therefore only affecting people who work on the CLI and only if they choose to run any of the extensions rake tasks.

vividviolet

Tophat'd 👍

t6d · 2021-09-17T20:21:42Z

🎩'ed Windows one more time. rake install:extensions does the right thing and fetches the shopify-extensions.exe.

t6d self-assigned this Sep 1, 2021

t6d requested review from cecyc and vividviolet September 1, 2021 18:54

t6d force-pushed the feature/download-shopify-cli-extensions-during-installation branch 6 times, most recently from c6c0d00 to 5399ac6 Compare September 10, 2021 19:09

t6d commented Sep 10, 2021

View reviewed changes

ext/shopify-extensions/extconf.rb Outdated Show resolved Hide resolved

t6d force-pushed the feature/download-shopify-cli-extensions-during-installation branch 5 times, most recently from f7c83d6 to 55ccc64 Compare September 14, 2021 16:41

t6d marked this pull request as ready for review September 15, 2021 16:10

t6d force-pushed the feature/download-shopify-cli-extensions-during-installation branch from 472bd42 to 9002909 Compare September 15, 2021 16:12

t6d commented Sep 15, 2021

View reviewed changes

vividviolet reviewed Sep 16, 2021

View reviewed changes

.gitignore Outdated Show resolved Hide resolved

ext/shopify-extensions/shopify_extensions.rb Outdated Show resolved Hide resolved

test/ext/shopify-extensions/shopify_extensions_test.rb Show resolved Hide resolved

test/ext/shopify-extensions/shopify_extensions_test.rb Show resolved Hide resolved

t6d force-pushed the feature/download-shopify-cli-extensions-during-installation branch from fb7e815 to 1c41fd7 Compare September 16, 2021 17:08

pepicrft reviewed Sep 17, 2021

View reviewed changes

pepicrft mentioned this pull request Sep 17, 2021

Add an acceptance test that ensures that the installation of extensio… #1545

Merged

3 tasks

t6d added 2 commits September 17, 2021 11:14

Install extension server

f7c5190

Adds a new Ruby extension called `shopify-extensions` to download the development server during installation.

Rake tasks for managing shopify-extensions

abd7361

* Supports updating of shopify-extensions through shopify:extensions:update VERSION=vx.y.Z * Supports symlinking a local development build through shopify:extensions:symlink

t6d force-pushed the feature/download-shopify-cli-extensions-during-installation branch from 4a1c5bc to abd7361 Compare September 17, 2021 15:14

t6d added 2 commits September 17, 2021 13:14

Remove API calls to Github

ddfd179

Removes API calls to Github in favor of constructing the download URL manually.

Remove unnecessary dependencies

e3890eb

t6d force-pushed the feature/download-shopify-cli-extensions-during-installation branch from a8d12cb to 7f224a5 Compare September 17, 2021 19:35

rake extensions:install & disable auto install

0f98a82

t6d force-pushed the feature/download-shopify-cli-extensions-during-installation branch from 7f224a5 to 0f98a82 Compare September 17, 2021 19:43

shopify-extensions version

646eb64

The version is now stored in a plain text file instead of being part of the Ruby code. This makes updating the contents a little easier.

vividviolet approved these changes Sep 17, 2021

View reviewed changes

t6d merged commit 5673c91 into main Sep 17, 2021

t6d deleted the feature/download-shopify-cli-extensions-during-installation branch September 17, 2021 20:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Install shopify-extensions during gem installation #1496

Install shopify-extensions during gem installation #1496

t6d commented Sep 1, 2021 •

edited

t6d commented Sep 15, 2021 •

edited

t6d Sep 15, 2021

vividviolet left a comment

pepicrft left a comment

pepicrft Sep 17, 2021

pepicrft Sep 17, 2021

t6d Sep 17, 2021 •

edited

t6d Sep 17, 2021

t6d Sep 17, 2021

t6d Sep 17, 2021

t6d commented Sep 17, 2021

vividviolet left a comment

t6d commented Sep 17, 2021

Install shopify-extensions during gem installation #1496

Install shopify-extensions during gem installation #1496

Conversation

t6d commented Sep 1, 2021 • edited

WHY are these changes introduced?

WHAT is this pull request doing?

Tasks

Update checklist

🎩

t6d commented Sep 15, 2021 • edited

t6d Sep 15, 2021

Choose a reason for hiding this comment

vividviolet left a comment

Choose a reason for hiding this comment

pepicrft left a comment

Choose a reason for hiding this comment

pepicrft Sep 17, 2021

Choose a reason for hiding this comment

pepicrft Sep 17, 2021

Choose a reason for hiding this comment

t6d Sep 17, 2021 • edited

Choose a reason for hiding this comment

t6d Sep 17, 2021

Choose a reason for hiding this comment

t6d Sep 17, 2021

Choose a reason for hiding this comment

t6d Sep 17, 2021

Choose a reason for hiding this comment

t6d commented Sep 17, 2021

vividviolet left a comment

Choose a reason for hiding this comment

t6d commented Sep 17, 2021

t6d commented Sep 1, 2021 •

edited

t6d commented Sep 15, 2021 •

edited

t6d Sep 17, 2021 •

edited