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

brew formula using ruby@2.5 dependency #101

Merged
merged 46 commits into from
Dec 10, 2019
Merged

brew formula using ruby@2.5 dependency #101

merged 46 commits into from
Dec 10, 2019

Conversation

joshdholtz
Copy link
Member

@joshdholtz joshdholtz commented Dec 8, 2019

Motivation

Fixes fastlane/fastlane#15496
Closes #99
Closes #95

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 or 2.5
    • A lot of Ruby dependencies are getting bumped to a Ruby 2.4 minimum due to security things
  • Can be built on any macoS
  • Easier to maintain

Description

  • Doesn't use brew cask at all anymore
    • Now uses brew formula
    • Install will now be brew install fastlane (instead of brew cask install fastlane)
  • Uses brew's ruby@2.5 dependency which installs really quickly without compiling
  • install copies bundle-env, fastlane into the brew fastlane directory

Brew formula

class Fastlane < Formula
  desc "Easiest way to build and release mobile apps"
  homepage "https://fastlane.tools"
  url "https://github.com/fastlane/packaged-fastlane/archive/2.0.0-beta-8.tar.gz"
  version "2.135.0"
  sha256 "6ec37c33a4021464f3f0dfe4aee6cc2aa65a46084f32885908838cb37f7636be"
  head "https://github.com/fastlane/packaged-fastlane.git"

  depends_on "ruby@2.5"

  def install
    system "./install", version.to_s, prefix
  end

  def fastlane_dir_bin
    File.join(prefix, "bin")
  end

  def fastlane_executable
    File.join(fastlane_dir_bin, "fastlane")
  end

  test do
    assert_true File.exist?(fastlane_executable)
    assert_true File.exist?(File.join(prefix, "bundle-env"))
  end
end

@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.

@joshdholtz joshdholtz marked this pull request as ready for review December 9, 2019 14:31
@milch
Copy link
Collaborator

milch commented Dec 9, 2019

The changes here aside, the reason we originally used brew cask instead of brew was that fastlane didn’t fit the acceptable formulae guidelines.

Did we reevaluate this / ask homebrew maintainers / plan to publish our own tap?

@joshdholtz
Copy link
Member Author

@milch I had some tweets with Mike McQuaid this weekend and this should be good and is preferred 😊

https://twitter.com/MikeMcQuaid/status/1204014091662241794

@milch
Copy link
Collaborator

milch commented Dec 9, 2019

Interesting, though he may not be aware that fastlane can update itself, which seems to be the only remaining blocker: https://github.com/fastlane/fastlane/blob/e81ae36abb59dd105f3be3105016e796733499fb/fastlane/lib/fastlane/actions/update_fastlane.rb

This part would also need to be updated to instruct users to update using brew instead: https://github.com/fastlane/fastlane/blob/51c167c392fed17239cd0f1e00fd00e015c381a7/fastlane_core/lib/fastlane_core/update_checker/update_checker.rb#L78-L79

IIRC at the time we had multiple blockers to being a regular homebrew formula, the self updating thing seems to be the only thing that could be against the guidelines at the moment.

Let’s ask @MikeMcQuaid directly, though. Maybe it doesn’t matter much at all

@joshdholtz
Copy link
Member Author

@milch Good call on the self updating bit! I will ask and report back 👌

@MikeMcQuaid
Copy link

This part would also need to be updated to instruct users to update using brew instead: https://github.com/fastlane/fastlane/blob/51c167c392fed17239cd0f1e00fd00e015c381a7/fastlane_core/lib/fastlane_core/update_checker/update_checker.rb#L78-L79

Yes, it would be good to address that upstream first if possible. If not, the formula will need to patch the relevant functionality in until the upstream PR is merged.

  def caveats; <<~EOS
    To run fastlane:
      $ #{fastlane_executable} <arguments>

    If you need to have fastlane in your PATH run:
      #{prepend_path_in_profile(fastlane_dir_bin)}

    After doing so close the terminal session and restart it to start using fastlane  🚀
  EOS
  end

Just to save you time on the PR: these are not Homebrew-specific caveats but more general usage instructions so can be omitted on the relevant PR (and the functions called accordingly, too).

@joshdholtz
Copy link
Member Author

joshdholtz commented Dec 9, 2019

@MikeMcQuaid Thanks for jumping in here! I'm a homebrew user but pretty new to distributing something through it 😇

  1. So just to confirm... fastlane is not allowed to update itself if its installed as a formula, correct?

  2. If caveats isn't the best place to put instructions for the user to run something like echo 'export PATH="/usr/local/opt/fastlane/bin:$PATH"' >> ~/.zshrc, where would that best go? I want it to make as easy on users as possible to know where to add it to their path.

@MikeMcQuaid
Copy link

  • So just to confirm... fastlane is not allowed to update itself if its installed as a formula, correct?

Correct.

2. If caveats isn't the best place to put instructions for the user to run something like echo 'export PATH="/usr/local/opt/fastlane/bin:$PATH"' >> ~/.zshrc, where would that best go? I want it to make as easy on users as possible to know where to add it to their path.

It'll be in the PATH by default and beyond that is the same for every Homebrew package so can be omitted.

@joshdholtz
Copy link
Member Author

It'll be in the PATH by default and beyond that is the same for every Homebrew package so can be omitted.

😱 OMG that makes so much sense. I was thinking waaaaayyyy too hard about that.

Updating fastlane options

1. Manually update the fastlane.rb formula to pin to the latest after every release

This approach I guess is what this current PR is aimed at but thankfully @milch pointed out that we can't self update which seems like this would not be the best approach since it would require a PR into the homebrew/core every release (which sometimes happens more than once a weekl. This could probably be automated to some degree but I think still requires someone from the homebrew team to manually merge (?????) and I don't want to create more work on them 🙃 Thoughts?

2. Create a tap for _fastlane _ where after every release we can create a new formula to pin to each specific version

We could create a custom tap and have new formulas for each version that get released that get automatically added in our fastlane release/deploy script. This would allow users to pin to specific versions of they wanted but also could probably add a "latest fastlane" that (similar to one that would be in homebrew/core) but it wouldn't depend on the homebrew team to review/merge.

3. Keep our current cask that self updates but use ruby@2.5 instead of using ruby-build

We could keep our current cask implementation that self updates but instead of using ruby-build we use brew's ruby@2.5.

@MikeMcQuaid I tried looking and couldn't find anything for having a cask depend on a formula so would we just need to run brew install ruby@2.5 from within out cask install script to use that dependency?


@milch @snatchev I think I summarized up all of our options here 😇 I would personally prefer to have fastlane as a formula and not as a cask. I have heard some complaints about the auto updating thing being a a bit confusing to users which is what the current cask implementation does.

I think my personal preference would be to be OPTION 1 since brew install fastlane and brew update fastlane seems like the most straight forward things for users to do. We could also then maybe do a OPTION 2 to have versioned versions (if we needed).

I think my biggest concern/worries are:

  1. Opening up a new PR into homebrew/core for each release of fastlane (mostly weekly and sometimes more) cc: @MikeMcQuaid
  2. Teaching users that they would need to use brew upgrade fastlane but this could be mentioned within fastlane and might be what users are expecting already.

This is a lot of words here 🙃 My apologies! Appreciate all your time to read and think about this ❤️

@milch
Copy link
Collaborator

milch commented Dec 9, 2019

I think option 1/2 would definitely align best with user expectations.

Between those options, on one hand, it’d be nice if install instructions were just brew install fastlane instead of brew tap ... && brew install fastlane since the former lends itself more to self-discovery by users. On the other hand because there will be a delay between pushing new versions and them being available in homebrew unless we control the tap, there might be cases where users have to wait (or switch to a different installation method) to get a critical bugfix. The update checker would also need to be updated to account for the lag between the homebrew version and the gem version.

Out of these two issues I think the latter is more important, so I’d tend to gravitate to the second option. Maybe a mixed approach could also work, where we publish versions to homebrew in a cadence that they can handle (say, semi-monthly) but control our own tap for the latest versions and nightlies (assuming users can seamlessly change versions between formulas that are contained in different taps)

Sidenote: I just remembered another reason for why we bundled ruby directly in the packaged fastlane, which was that at the time we were getting a lot of issues from people with problems installing/upgrading fastlane due to other gems they had installed (or incompatible OpenSSL versions Ruby was compiled with). Moving to a homebrew vended Ruby may bring that back, though I don’t know if that is still a frequently occurring issue for fastlane users

@joshdholtz
Copy link
Member Author

joshdholtz commented Dec 9, 2019

Out of these two issues I think the latter is more important, so I’d tend to gravitate to the second option. Maybe a mixed approach could also work, where we publish versions to homebrew in a cadence that they can handle (say, semi-monthly) but control our own tap for the latest versions and nightlies (assuming users can seamlessly change versions between formulas that are contained in different taps)

I think I like option 2 approach too so that we are in full control of the cadence and aren't burdening the homebrew team

Sidenote: I just remembered another reason for why we bundled ruby directly in the packaged fastlane, which was that at the time we were getting a lot of issues from people with problems installing/upgrading fastlane due to other gems they had installed (or incompatible OpenSSL versions Ruby was compiled with). Moving to a homebrew vended Ruby may bring that back, though I don’t know if that is still a frequently occurring issue for fastlane users

I definitely saw this when I was gutting the cask process. The problem is its pretty much impossible to package new versions of Ruby 😔 I spent quite a bit of time getting OpenSLL to work with the ruby-build approach but there were still probably some issues that would have been happened with that. I have seen zero issues using ruby@2.5 from homebrew since it depends on openssl from homebrew. To test this I actually full uninstalled homebrew off of my MacBook and it it worked without any problems. Using the ruby@2.5 from homebrew should be the way to go to avoid any OpenSSL issues 🤞

@MikeMcQuaid
Copy link

1. Manually update the fastlane.rb formula to pin to the latest after every release

This is the best option except you can do it automatically using something like https://github.com/marketplace/actions/bump-homebrew-formula or just using brew bump-formula-pr. The other approaches have reduced discoverability which results in fewer people using your software.

  1. Opening up a new PR into homebrew/core for each release of fastlane (mostly weekly and sometimes more)

This is fine.

2. Teaching users that they would need to use brew upgrade fastlane but this could be mentioned within fastlane and might be what users are expecting already.

Indeed.

Between those options, on one hand, it’d be nice if install instructions were just brew install fastlane instead of brew tap ... && brew install fastlane since the former lends itself more to self-discovery by users

This.

On the other hand because there will be a delay between pushing new versions and them being available in homebrew unless we control the tap, there might be cases where users have to wait (or switch to a different installation method) to get a critical bugfix.

Yes they may have to wait but generally version bumps take <24h (and often less). You can also CC me to speed things up.

aren't burdening the homebrew team

We definitely don't consider updates a burnen

@milch
Copy link
Collaborator

milch commented Dec 9, 2019

Sounds good, let’s go with the option 1 then 👍 Thanks for the help Mike!

So just to summarize the open TODOs for that approach are:

  • Change update_fastlane logic to be disabled on homebrew (+ nice user message)
  • Change messaging around how to update fastlane with homebrew (i.e. use brew upgrade)
  • Have the update checker work with homebrew versions instead of checking gem versions

That last one can probably be done after it becomes available in homebrew, but should probably be a fast follow so users don’t get confused when the update checker tells them that there is an update but brew tells them otherwise

@joshdholtz
Copy link
Member Author

Option 1 it is!

@milch Thanks for getting this convo started and for summarizing what needs to change ❤️
@MikeMcQuaid Thanks for jumping in here and on Twitter to help us out ❤️

@joshdholtz
Copy link
Member Author

@milch Got the update_fastlane stuff added to be deprecated and add a nice user message. Are you able to give this another review when you get some time? 😊🙏

Copy link
Member

@snatchev snatchev left a comment

Choose a reason for hiding this comment

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

Looks great! Thank you for the input @MikeMcQuaid!

@joshdholtz joshdholtz merged commit 7db4799 into master Dec 10, 2019
@joshdholtz joshdholtz deleted the 2.0-brew-formula branch December 10, 2019 23:47
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
6 participants