-
-
Notifications
You must be signed in to change notification settings - Fork 261
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
Unable to bundle install latest psych 5.0.0 on github actions #409
Comments
A good place to check are the Actions workflows for the repo, in this case, ruby/psych. It looks like In general, when building Ruby or any extension gems (like psych), there are often build dependencies. They may not be included with the 'base' Actions image. |
As it says:
So you'll need to install libyaml-dev or so, or use |
Shouldn't installing Pysch 5.0.0 on Ruby 2.7.6 just work? Thanks to ruby/psych#550 or am I missing something? |
Psych version 4 has yaml bundled. Psych version 5 does not? See https://github.com/ruby/psych/tree/4-0-stable/ext/psych/yaml vs https://github.com/ruby/psych/tree/v5.0.0/ext/psych/yaml |
Oh I thought Ruby itself bundled libyaml |
3.1 does, master does not |
but Pysch 5.0.0 can't use it? |
I suppose it could, never tried. Vendoring c source for dependencies has pros and cons, and other default extension gems do not vendor, like Ruby's OpenSSL. |
It was just psych itself vendoring libyaml. |
👋 Can we install I'm not sure how handle the additional packages like |
Yes. Similar code is in setup-ruby-pkgs, but the packages are user defined. Windows builds already have all packages installed by setup-ruby. By packages, I mean Ruby dependencies, but you knew that. |
Briefly looked at ruby/ruby CI, libncurses5-dev & libyaml-dev are needed with Ubuntu, but not macOS? |
I don't think setup-ruby should install any system package, for various reasons including time (it's a non-trivial overhead and in many cases not needed) and confusing side effects of setup-ruby (i.e. some extra system packages installed after the action). Just like one needs to install a system package when using mysql2 or pg gem, now one needs to |
I want to strongly encourage this group to please push this new library requirement down the stack (to runner-images if that's the right place) and not up the stack to the individuals trying to maintain CI. Asking users of psych to add a step to every pipeline (or alternatively to use setup-ruby-pkgs) is a considerable cost in terms of human time, and a collectively large amount of clock time and cpu time to install it. Especially for indirect users of psych (e.g., a project requires rdoc which then transitively requires psych). I'm not complaining, but as a data point from someone who is familiar with the problem and the workarounds, I've already spent a few hours trying to get just one project's pipelines green again. |
Is it common to have an unbounded dependency on psych though? Because otherwise it's not a problem. I think it's worth a try to create an issue at https://github.com/actions/runner-images to ask to preinstall |
See 'Ubuntu : Update the libyaml version to 0.2.5 (latest)' in actions/runner-images |
Because of this change, the https://github.com/ruby/logger This probably won't be the only case. |
We fixed it by removing rdoc from the development dependency list. |
I'll be devil's advocate here, undecided whether setup-ruby should handle this. This type of issue may happen more in the future.
Off-topic: Puma has a bit of 'app reload' logic, and upgrading bundled extension gems in the main process causes issues. Our yaml needs were trivial, so we replaced Psych with pure Ruby code. Maybe rdoc can do the same? |
FWIW I measured how long Also from @MSP-Greg's last reply it comforts me in my position that we certainly don't want additional complexity and risks here (using a package from another Ubuntu version feels very risky, especially if the existing /usr/lib/libyaml.so is of a different version). I understand it's annoying and possibly more frequent but this is not the place to solve it, this action sets up Ruby it doesn't guess system dependencies or do anything about them. As I said in #409 (comment), I think the only way to make it nicer is to ask to add |
Oh and it looks like I was very lucky with thet 5-6 second run, because in 3 other runs it's more like 15 seconds: (FWIW that's a big part of why I don't like |
Just to clarify, if we used kinetic, we'd need to install both libyaml & libyaml-dev... I suspect runner-images may not want to add libyaml-dev, not sure. I'll see... |
Help - can anyone verify whether libyaml & libyaml-dev are a normal part of a macOS install? Looking at the macOS workflow in ruby/ruby, can't see any mention of it? Maybe I need more coffee... Or, one can use |
Actually runner images provide some versions of Ruby, so that guarantees |
@MSP-Greg |
On my dev machine:
So this is the equivalent of both |
I suspected as much, thanks for verifying. |
I don't see a libyaml installation on the core macOS system (but I may not be looking in all the places):
so I'm guessing it's a brew-installed package. |
Interesting. You mentioned some CI that failed. Did it fail on Ubuntu and pass on macOS? I assume GitHub Actions? I opened an issue, actions/runner-images#6725, and I hope 'parity' between Ubuntu & macOS is a valid reason... |
@MSP-Greg Correct, the failures have only been on Ubuntu images. |
Closing the loop here: libyaml-dev should be on runner images sometime this week: |
Needed to be able to install psych >=5. Need to do it before setup-ruby runs as that runs `bundle install`. Related to: - ruby/psych#541 - ruby/setup-ruby#409 - actions/runner-images#6725 Yes, libyaml-dev will be added to GitHub runner images (this week they say) but opening this PR just in case anyone encounters failing builds. I don't think it hurts having this in the repo even after libyaml-dev have been added to the images. Added some blank lines to make the workflow easier to read.
Needed to be able to install psych >=5. Need to do it before setup-ruby runs as that runs `bundle install`. Related to: - ruby/psych#541 - ruby/setup-ruby#409 - actions/runner-images#6725 Yes, libyaml-dev will be added to GitHub runner images (this week they say) but opening this PR just in case anyone encounters failing builds. I don't think it hurts having this in the repo even after libyaml-dev have been added to the images. Added some blank lines to make the workflow easier to read.
Ensure the following before filing this issue
I verified it reproduces with the latest version with
- uses: ruby/setup-ruby@v1
(see Versioning policy)I tried to reproduce the issue locally by following the workflow steps (including all commands done by
ruby/setup-ruby
, except forDownloading Ruby
&Extracting Ruby
),and it did not reproduce locally (if it does reproduce locally, it's not a ruby/setup-ruby issue)
Are you running on a GitHub-hosted runner or a self-hosted runner?
GitHub-hosted runner
The workflow code or a link to the workflow file
Link to the log of a failed workflow job, or to a gist with the output
https://github.com/danielwestendorf/gh-actions-ruby-psych-5/actions/runs/3622724366/jobs/6107778167#step:3:44
The command and output of the failing step
Any other notes?
No response
The text was updated successfully, but these errors were encountered: