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

Run CI on GitHub Actions #397

Merged
merged 5 commits into from
Feb 18, 2021
Merged

Run CI on GitHub Actions #397

merged 5 commits into from
Feb 18, 2021

Conversation

dsander
Copy link
Contributor

@dsander dsander commented Jan 30, 2021

Ruby 2.3 has some strange openssl version mismatch issue on GitHub
Actions. The rugged gem itself installs fine but when it loads it's C
extensions it somehow loads the wrong openssl version. Running the specs
interactively (even on GitHub Actions) works without issues.

Sorry for the commit mess, getting this to work(ish) was harder than expected. I think the Actions are not run in the PR because I am not a maintainer, The output can be checked here: https://github.com/dsander/pronto/actions/runs/523525169

Sadly I have no clue how we can get the tests on Ruby 2.3 to work.

Rubocop comments are working: dsander#1

Labeled as draft because the switch might mean to drop CI for Ruby 2.3.

@dsander dsander force-pushed the github-actions branch 11 times, most recently from 17fc577 to c0c94c8 Compare January 30, 2021 14:21
@dsander dsander changed the title Use GitHub Action for CI Run CI on GitHub Actions Jan 30, 2021
@dsander dsander force-pushed the github-actions branch 4 times, most recently from 4f32cec to a61d6f9 Compare January 30, 2021 15:02
@dsander dsander mentioned this pull request Jan 30, 2021
Copy link
Member

@ashkulz ashkulz left a comment

Choose a reason for hiding this comment

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

Did you try running without bundle exec? Also, what's the version of bundler on the Ruby 2.3 run -- maybe you specifically need to use a version to enable support for such an old version?

pronto.gemspec Outdated Show resolved Hide resolved
@ashkulz
Copy link
Member

ashkulz commented Jan 31, 2021

@dsander I'm open to dropping Ruby 2.3 support -- I think Ruby 2.5 is going to be EOL in a few months, so makes sense to drop something which can't run in CI. What do you think? I don't want to keep Travis CI just for the 2.3 support.

@dsander
Copy link
Contributor Author

dsander commented Jan 31, 2021

Did you try running without bundle exec? Also, what's the version of bundler on the Ruby 2.3 run -- maybe you specifically need to use a version to enable support for such an old version?

I don't think bundler is at fault here, the gems load without issues. I think what happens is that rugged builds it's bundled libgit against the correct - bundled with ruby 2.3 - openssl 1.0 library. Something,, for some reason then loads system installed openssl 1.1 library at runtime and segfaults. This issue seems to describe the same problem libgit2/rugged#718.

I don't even know where to point the finger, is it the github action stack, the ruby/setup-ruby action, rugged, the way rugged compiles libgit, how ruby builds C extensions or how ruby/C loads libraries?

What makes things worse, I have no clue is why everything works normally when the specs are run interactively "inside" the action when connecting to it via ssh.

@dsander I'm open to dropping Ruby 2.3 support -- I think Ruby 2.5 is going to be EOL in a few months, so makes sense to drop something which can't run in CI. What do you think? I don't want to keep Travis CI just for the 2.3 support.

More than happy to drop it, 2.3 is EOL for almost two years, so hopefully not many people are still using it 😄

@dsander
Copy link
Contributor Author

dsander commented Jan 31, 2021

If we drop Ruby 2.3 from CI, the rubocop version is also bumped (or we need to enforce the older version in the gemspec. It not complains about new cops not being enabled: https://github.com/dsander/pronto/runs/1800709555?check_suite_focus=true#step:5:11

I don't think enabling all new cops is an option because everyone runs a different rubocop version locally

pronto.gemspec Outdated Show resolved Hide resolved
@ashkulz
Copy link
Member

ashkulz commented Feb 1, 2021

I think we should leave Ruby 2.3 in, but explicitly exclude it like done for pronto-haml, so that it's present in the definition and is documented why it fails but doesn't actually get run -- using continue-on-error isn't great, see actions/runner#2347

@dsander
Copy link
Contributor Author

dsander commented Feb 2, 2021

I think we should leave Ruby 2.3 in, but explicitly exclude it like done for pronto-haml, so that it's present in the definition and is documented why it fails but doesn't actually get run

Sure, that works as well. I thought you were implying dropping Ruby 2.3 support all together.

Copy link
Member

@ashkulz ashkulz left a comment

Choose a reason for hiding this comment

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

Thanks for your patience, @dsander! Sorry if I wasn't clear above, I definitely didn't mean to drop Ruby 2.3 support immediately -- that would mean the next release would've to be 0.12.0 (as it's a breaking change).

.github/workflows/action.yml Outdated Show resolved Hide resolved
pronto.gemspec Outdated Show resolved Hide resolved
.github/workflows/action.yml Outdated Show resolved Hide resolved
@ashkulz
Copy link
Member

ashkulz commented Feb 3, 2021

Can you also remove .travis.yml as part of this PR? I think we're good to go, otherwise 👍

@dsander dsander marked this pull request as ready for review February 6, 2021 13:24
@dsander dsander requested a review from a team as a code owner February 6, 2021 13:24
Ruby 2.3 has some strange openssl version mismatch issue on GitHub
Actions. The `rugged` gem itself installs fine but when it loads it's C
extensions it somehow loads the wrong openssl version. Running the specs
interactively (even on GitHub Actions) works without issues.
For this reason Ruby 2.3 is excluded from version the matrix.
.github/workflows/action.yml Show resolved Hide resolved
pronto.gemspec Show resolved Hide resolved
@ashkulz ashkulz merged commit a213079 into prontolabs:master Feb 18, 2021
@ashkulz
Copy link
Member

ashkulz commented Feb 18, 2021

Thanks, @dsander! Sorry about not merging this earlier 🙁

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.

2 participants