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

Enable post-ruby-install hook #239

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ashmaroli
Copy link

@ashmaroli ashmaroli commented Dec 6, 2021

Motivation

Currently, if one is using this action to set up Ruby and manage caching of their gemset, there is no way to *micro-manage * the environment before Bundler gets installed and consequently executing bundle install.

With this hook, the end-user will be able to:

  • change the Rubygems version if needed, or
  • install the rake gem manually and invoke Rake task(s) that sets up the project's Gemfile / Gemspec for consequent consumption by Bundler.

P.S. I did not generate the dist/index.js because I'm on Windows and the generation process is altering the line-endings.

@eregon
Copy link
Member

eregon commented Dec 7, 2021

Yes, this is very tempting but there are many problems with this approach.
What if users want to run multiple commands, what if they expect a specific shell, what if they want use | or &&, what should be exit status for multiple commands, etc?
Basically you would need to reimplement the - run: action manually, and that's a bad idea because it would be endless work.

change the Rubygems version if needed, or

Any reason you crossed that? It doesn't work maybe?

install the rake gem manually and invoke Rake task(s) that sets up the project's Gemfile / Gemspec for consequent consumption by Bundler.

IMHO not having the Gemfile/gemspec in the repo is a big hurdle for contributors and gem standards. If you generate those, then generate them whenever something changes, and have a check in CI they are up to date (by generating them and checking there is no diff).

@ashmaroli
Copy link
Author

What if users want to run multiple commands, what if they expect a specific shell, what if they want use | or &&, what should be exit status for multiple commands, etc?

I was hoping to document this in the README, if the proposal was merge-worthy. The idea was to instruct users to write a shell script to run multiple commands and execute the shell script via the provided hook..

steps:
  - uses: ruby/setup-ruby@v1
    with:
      post-ruby-install: bash .github/workflows/scripts/my_script

change the Rubygems version if needed, or

Any reason you crossed that? It doesn't work maybe?

The hook was implemented to run only if Bundler was going to be consequently installed.
However, if gem update --system x.y.z was ran as the hook, the executable of the default Bundler that gets installed along with rubygems-x.y.z conflicts with the subsequent installBundler step..

IMHO not having the Gemfile/gemspec in the repo is a big hurdle for contributors and gem standards.

I agree with you. This was actually for a repository that was started in the Ruby 1.8 era (and still under maintenance), that I contribute to. The repo had used TravisCI in the past relying on the CI's before_install hook.
I did not want to disrupt the maintainer's established workflow when proposing to migrate to use GH Actions..


@eregon Thank you for taking a look at my proposal.
I understand if you feel this is not worth the added complexity to maintainability and therefore close this.

@eregon
Copy link
Member

eregon commented Dec 7, 2021

I forgot to mention in my first reply. A general solution for this would be to extract a separate action to do the bundler caching.
And then one can do anything they want in between, and also potentially cache multiple Gemfile installs in the same CI job.
It's a significant amount of work though.

I was hoping to document this in the README, if the proposal was merge-worthy. The idea was to instruct users to write a shell script to run multiple commands and execute the shell script via the provided hook..

That's a good idea, just accept a single command and tell users to use a shell script for anything more complex.
Might still be inconvenient though to have to add a shell script in the repo just for CI reasons.

However, if gem update --system x.y.z was ran as the hook, the executable of the default Bundler that gets installed along with rubygems-x.y.z conflicts with the subsequent installBundler step..

Oh right, I hate this behavior, the command is asking to update RubyGems, not Bundler, Bundler shouldn't get installed (especially in a weird place where it cannot be uninstalled).

I'll think a bit more about this.

@ashmaroli
Copy link
Author

to extract a separate action to do the bundler caching... It's a significant amount of work though.

I did consider this option though but given that even actions/cache recommends using ruby/setup-ruby to manage bundle caching, I decided to add the proposed hook here to get the best of two routes.

I'll think a bit more about this.

Thank you very much :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants