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

Bundled cli-ruby used by default #1346

Merged
merged 2 commits into from
Feb 10, 2023
Merged

Conversation

alvaro-shopify
Copy link
Contributor

@alvaro-shopify alvaro-shopify commented Feb 10, 2023

WHY are these changes introduced?

Fixes #1306

Some problems have appeared using the embedded version of cli-ruby to run themes commands or dev command with theme-app-extensions
Those problems seem to be related with the method used to install Ruby requirements in the local environment. There are also some problems with theme-app-extensions in windows as well

WHAT is this pull request doing?

  • Every command related with themes and theme-app-extension will use the bundled version of cli-ruby
  • Add a the new env variable SHOPIFY_CLI_EMBEDDED_THEME_CLI to run the command using the embedded version.

How to test your changes?

  • Run any theme command with --verbose flag and you should see an output similar to this:
Running system process:
  · Command: bundle exec shopify theme check /Users/alvarogutierrez/cli-themes/first-theme --output text
  · Working directory: /Users/alvarogutierrez/cli-themes/first-theme 

Post-release steps

Measuring impact

How do we know this change was effective? Please choose one:

  • n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix
  • Existing analytics will cater for this addition
  • PR includes analytics changes to measure impact

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes
  • I've made sure that any changes to dev or deploy have been reflected in the internal flowchart.

@github-actions
Copy link
Contributor

Thanks for your contribution!

Depending on what you are working on, you may want to request a review from a Shopify team:

  • Themes: @shopify/theme-developer-tools
  • UI extensions: @shopify/ui-extensions-cli
    • Checkout UI extensions: @shopify/checkout-ui-extensions-api-stewardship
  • Hydrogen: @shopify/hydrogen
  • Other: @shopify/cli-foundations

@github-actions
Copy link
Contributor

github-actions bot commented Feb 10, 2023

Benchmark report

The following table contains a summary of the startup time for all commands.

Status Command Baseline (avg) Current (avg) Diff
🟢 app build 935.67 ms 936 ms 0.04 %
🟢 app deploy 1132.33 ms 1144.33 ms 1.06 %
🟢 app dev 1139 ms 1140.33 ms 0.12 %
🟢 app env pull 1027.67 ms 1036.33 ms 0.84 %
🟢 app env show 1031 ms 1029.33 ms -0.16 %
🟢 app function build 926.33 ms 926.33 ms 0 %
🟢 app function run 919.67 ms 927.33 ms 0.83 %
🟢 app function schema 1064 ms 1066.67 ms 0.25 %
🟢 app function typegen 921 ms 922.33 ms 0.14 %
🟢 app generate extension 1103.33 ms 1109.67 ms 0.57 %
🟢 app generate schema 1060.33 ms 1067.67 ms 0.69 %
🟢 app info 1046 ms 1036.67 ms -0.89 %
🟢 app scaffold extension 1105 ms 1098 ms -0.63 %
🟢 app update-url 1008.33 ms 1009 ms 0.07 %
🟢 theme check 878.33 ms 875 ms -0.38 %
🟢 theme delete 997.33 ms 988.67 ms -0.87 %
🟢 theme dev 1001.33 ms 1001.33 ms 0 %
🟢 theme help-old 867 ms 879.67 ms 1.46 %
🟢 theme info 927.67 ms 921.33 ms -0.68 %
🟢 theme init 885.67 ms 889.67 ms 0.45 %
🟢 theme language-server 877.33 ms 863.67 ms -1.56 %
🟢 theme list 990.33 ms 995 ms 0.47 %
🟢 theme open 990.67 ms 998 ms 0.74 %
🟢 theme package 923 ms 919 ms -0.43 %
🟢 theme publish 999.67 ms 987.67 ms -1.2 %
🟢 theme pull 987 ms 989 ms 0.2 %
🟢 theme push 990 ms 991.33 ms 0.13 %
🟢 theme serve 992.33 ms 993.33 ms 0.1 %
🟢 theme share 980 ms 985.33 ms 0.54 %
🟢 webhook trigger 1012.33 ms 1007 ms -0.53 %

@github-actions

This comment has been minimized.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 10, 2023

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements 73.67% 4156/5641
🟡 Branches 70.58% 1871/2651
🟡 Functions 71.66% 1085/1514
🟡 Lines 75.02% 3968/5289

Test suite run success

1045 tests passing in 533 suites.

Report generated by 🧪jest coverage report action from b6f93cf

@alvaro-shopify alvaro-shopify merged commit 1fe444f into main Feb 10, 2023
@alvaro-shopify alvaro-shopify deleted the bundled-cli-ruby-by-default branch February 10, 2023 14:53
@shopify-shipit shopify-shipit bot temporarily deployed to production February 10, 2023 16:55 Inactive
@shopify-shipit shopify-shipit bot temporarily deployed to nightly February 11, 2023 02:26 Inactive
@Michael-Gibbons
Copy link

@alvaro-shopify I believe I am having similar issues relating to this, I would appreciate any insight you can give. When attempting to deploy an app to production (hosted on digital ocean if that matters), shopify app deploy is run. This command also bundles and pushes theme extensions. Part of which is running theme check on the extension code. By default the server is running on 2.5 whereas shopify needs 2.7. Upgrading the ruby version server side should fix probably but this seems a bit clunky no?

image

@gonzaloriestra
Copy link
Contributor

@Michael-Gibbons Ruby 2.5 reached end of life last year. We can't commit to support all older versions, and it wouldn't be a good practice security-wise. So please upgrade your Ruby version and let us know if you find any issue. Thanks!

@Michael-Gibbons
Copy link

Michael-Gibbons commented Feb 14, 2023

@gonzaloriestra I totally agree, the implication wasn't that all versions of ruby be supported. Rather I'm pointing out a circular dependency issue by introducing a local ruby dependency in a node package. Digital Ocean and heroku use heroku's nodejs buildpack which needs ruby 2.5 but shopify requires a local version of at least 2.7.

Unless I do some major fiddling by ssh'ing into my remote server and sudo updating ruby which may break the buildpack I cannot build an app with extensions since part of shopify app deploy is running theme check on the extension code, which requires that local ruby version.

An alternative solution would be to disable theme check in production.

@gonzaloriestra
Copy link
Contributor

@Michael-Gibbons we know depending on Ruby is annoying and we are working on that (it will take some time). But Buildpack should work with recent Ruby versions, no?

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 this pull request may close these issues.

[Bug]: Newest version does not work from homebrew
3 participants