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

package-fastlane 2.0 that installs and builds Ruby 2.4.5 on users machine on brew cask install fastlane #99

Closed
wants to merge 25 commits into from

Conversation

joshdholtz
Copy link
Member

@joshdholtz joshdholtz commented Nov 8, 2019

Motivation

Fixes fastlane/fastlane#15496

Problem

  • Packaged fastlane used Ruby 2.2 which is too old for some fastlane dependencies and causes installation issues leading to packaged fastlane installed version 2.28.3
  • Packaged fastlane could also only be built on OSX 10.1

Goal

  • Support Ruby 2.4.5
    • A lot of Ruby dependencies are getting bumped to a Ruby 2.4 minimum due to security things
  • Can be built on Catalina
  • Easier to maintain

Description

  • Completely removed previous method of packaging a portal version of Ruby
    • Ruby isn't really meant to do that and is tougher to do on modern macOS version and with modern Ruby versions
  • Ruby is now compiled on the users machine using ruby-build
    • Smaller package
    • Easier to update Ruby versions in the future
    • Easier to test on local machine (clone repo and run ./uninstall and ./install)
  • Handles smooth upgrading from previous packaged version to this version
    • This new version writes two text files with Ruby version and packaged fastlane version that allows for upgrades/migration
    • If the package fastlane version text file isn't found, the whole ~/.fastlane/bin directory gets removed
  • The ./install script will only install/upgrade ruby-build and openssl only if needed
  • The ./install script will only build Ruby if the version declared is not already built

How To review

  • ALL of the logic is in uninstall and install
  • Everything else has pretty much been removed

How to test

  1. Clone the repo
  2. Run ./uninstall
  3. Run ./install

@@ -11,3 +11,18 @@ cask 'fastlane' do

Copy link
Collaborator

Choose a reason for hiding this comment

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

Line 5 and 6 (GitHub won't let me comment on the line) might need to be changed to point to the new install location.

Copy link
Member Author

Choose a reason for hiding this comment

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

@taquitos I JUST updated this file to what is actually in cask as you were commenting probably 🙃

@taquitos
Copy link
Collaborator

taquitos commented Nov 8, 2019

Do you have an idea for a good test matrix? Like what states should our mac be in when we test this update? Mojave with previous cask, without, catalina, ect... There's a decent amount of logic here I want to make sure we're covering our bases in a test matrix

Copy link
Member Author

@joshdholtz joshdholtz left a comment

Choose a reason for hiding this comment

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

@taquitos Ahhh, I went over a bunch of tests with @snatchev during a pair session but probably good to create a test matrix. Maybe I'll make it in a PR template and then also drop it into this PR description. Thoughts?

@taquitos
Copy link
Collaborator

taquitos commented Nov 8, 2019

Oh, if you wrote any of that down, maybe just copypasta into a comment, otherwise maybe later on a formal matrix would be good. I really wanted to make sure we're covering all our bases but it sounds like you and @snatchev did 👍
The PR looks good to me but I'd want to make sure I can find time to test it out (which I might not have until Tuesday)

@joshdholtz
Copy link
Member Author

@taquitos I'll def write it out! And whenever you get to it is 👍 I've been finding other people to test it out. It worked on @ohayon's machine this morning but would definitely like for you to break it too 😇

# Check if old version and clean it up
if [ ! -f $PACKAGED_FASTLANE_VERSION_FILE ]; then
echoc "Found old version of brew packaged fastlane. Cleaning up..." yellow
"$CURRENT_DIR/uninstall" -y
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be calling the uninstall script of the version in ~/.fastlane/bin?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmmm, that is a good question actually 🤔 That isn't how it the previous one worked. The current version of packaged fastlane didn't actually copy the uninstall script that wouldn't work but I think it makes sense to put it in there. So I think I'll make changes to copy this uninstall script there after install. But for the sake of this part of the code, I'll check if there is an uninstall script in ~/.fastlane/bin and if so I'll run that otherwise I'll run the new one 😬

@milch
Copy link
Collaborator

milch commented Nov 8, 2019

Re: testing, how about running install/uninstall on Circle/Travis/GitHub Actions?

@joshdholtz
Copy link
Member Author

Ahhhh yeah yeah! I'll set it up on Circle 😊 I like it I like it

Copy link
Contributor

@ohayon ohayon left a comment

Choose a reason for hiding this comment

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

This looks great. Still pretty amazed you got this modernized and working. 💯

@KrauseFx
Copy link
Member

Whoah, what is this? A reunion? 🕺 🚀 🎉

$FASTLANE_DIR/bundle/bin/bundle-env gem install fastlane --no-document --env-shebang

# Copy the fastlane executable to run fastlane in the bundled environment
cp "$CURRENT_DIR/fastlane_shim" "$FASTLANE_DIR/fastlane"
Copy link
Contributor

@Larusso Larusso Nov 14, 2019

Choose a reason for hiding this comment

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

You deleted this file.
What I mean is there is no fastlane_shim in the root.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ahhh, that was supposed to be committed! Thanks 😊

while getopts ":pub" opt; do
case $opt in
p ) SKIP_SETTING_PATH=1;;
u ) DO_UNINSTALL=1;;
Copy link
Contributor

Choose a reason for hiding this comment

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

The help text and the cask file still contain the -u option flag.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops! Thanks for catching that

{
PACKAGE=$1

read -p "The installer needs to install/upgrade $PACKAGE with brew. Is that okay? (y/N) " -n 1 choice
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be very helpful to skip the prompts with a the help of a command line option. Automating the installation a little bit easier.

@Larusso
Copy link
Contributor

Larusso commented Nov 14, 2019

So I played around a bit this afternoon because I need to roll out the newest version of fastlane to my buildmachines. As a disclaimer. I already used my own way of installing the fastlane.zip file as the defaults you set are not how I want to roll the installation process.
I used a custom cask definition which constructed a more controlled installation result for me without the need to adjust any PATH variable or that fastlane ends up in the user home directory.

version :latest
  version :latest
  sha256 :no_check

  url 'https://fastlane.tools/fastlane.zip'
  name 'fastlane'
  homepage 'https://fastlane.tools/'


  binary "#{staged_path}/fastlane_lib/snapshot"
  binary "#{staged_path}/fastlane_lib/sigh"
  binary "#{staged_path}/fastlane_lib/scan"
  binary "#{staged_path}/fastlane_lib/produce"
  binary "#{staged_path}/fastlane_lib/pem"
  binary "#{staged_path}/fastlane_lib/match"
  binary "#{staged_path}/fastlane_lib/gym"
  binary "#{staged_path}/fastlane_lib/frameit"
  binary "#{staged_path}/fastlane_lib/fastlane"
  binary "#{staged_path}/fastlane_lib/deliver"
  binary "#{staged_path}/fastlane_lib/cert"
  binary "#{staged_path}/fastlane_lib/bundle"
  binary "#{staged_path}/fastlane_lib/bundle_update_checker.rb"
  binary "#{staged_path}/fastlane_lib/parse_env.rb"
  binary "#{staged_path}/fastlane_lib/VERSION"


  preflight do
    system_command "sed", args: ['-i', '', "s/{{IS_INSTALLED_VIA_HOMEBREW}}/$INSTALLED_VIA_HOMEBREW/g", "#{staged_path}/fastlane_lib/bundle/bin/bundle-env"]
    system_command "#{staged_path}/fastlane_lib/fastlane", args: ["update_fastlane"]
  end
end

Granted this is a hard hack but worked for me so far.
To be able to roll out today I took your installer branch and adjusted a few things so it works for me again.
wooga@29b7075

I had to remove the promts, and bend the destination URL to a sub directory. This way fastlane stays in the homebew staged_path. It's not nice but get's the job done. I would love to use the default installer from you guys but without more control where stuff gets installed I can't.

@Larusso
Copy link
Contributor

Larusso commented Nov 15, 2019

Ok I played around with it some more. I created a fork yesterday and adjusted it a little bit to be able to roll out. But I ran into OpenSSL error messages when using sigh for instance (SSL_connect returned=1 errno=0 state=error: certificate verify failed). I used rbenv and installed fastlane via gem to test the behavior there and have no issues. I updated brew and tried a fresh install again to no avail.

@Larusso
Copy link
Contributor

Larusso commented Nov 15, 2019

Ok removing this line from bundle-env seems to fix the issue:

export SSL_CERT_FILE=$PREFIX/share/cacert.pem

Larusso and others added 2 commits November 28, 2019 16:55
Hey I made some adjustments to your changes for the new installer
package.

* Add missing `fastlane_shim` file
* remove ssl certificate environment line from `bundle-env`
* added option switch `-u` to skip prompts for autoupdate/install
…n-brew-install

Fix 2.0 installer script
@googlebot
Copy link

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: no and removed cla: yes labels Dec 4, 2019
@joshdholtz joshdholtz changed the title package-fastlane 2.0 that installs and builds Ruby 2.4.5 on users machine on brew cask install fastlane package-fastlane 2.0 that installs and builds Ruby 2.4.5 on users machine on brew cask install fastlane Dec 7, 2019
@janpio janpio deleted the 2.0-build-and-install-ruby-on-brew-install branch December 11, 2019 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When installing via homebrew it downloads version 2.28.3 instead of the latest version
8 participants