-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
[fastlane] drop Ruby 2.0, 2.1, 2.2 and 2.3 from gemspec #16408
Conversation
@joshdholtz Found annoying bug here - Let me know if you are okay with this PR for drop, or if not then what do you think. 😊 |
@Kaspik Good call on bumping up that docker image version 👌 Looks like Ubuntu tests are failing due to not being able to unzip an IPA in |
I wonder if it has something to do with the new docker file, otherwise there was no change. It uses system unzip, right? - Honestly I have no idea how to debug this. |
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 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 ℹ️ Googlers: Go here for more info. |
@Kaspik I ended up reruning the CircleCI build with an SSH session. I was then able to run the exact unzip command that it was failing in on got this. I’m going to fix by trying to unzip a different incorrectly signed IPA that won’t zip bomb circleci@2c6ee3bb767d:~/project$ unzip fastlane_core/spec/fixtures/ipas/ContainsWatchApp.ipa -d /tmp/123 -x '__MA
COSX/*' '*.DS_Store'
Archive: fastlane_core/spec/fixtures/ipas/ContainsWatchApp.ipa
inflating: /tmp/123/ContainsWatchApp/Payload/Sample.app/Watch/Sample WatchKit App.app/Info.plist
error: invalid zip file with overlapped components (possible zip bomb) |
@googlebot I consent. |
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
Wohou, nice! Thanks for this, if all good now, oky to merge from my side. :) |
@Kaspik Looks like its good! I’ll wait for the other tests to pass. Also... TIL about zip bombs lol. I had no idea that was a thing. |
Yeah that’s an interesting one. 😂 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We’re good now! Let’s drop these 💪
Hey @Kaspik 👋 Thank you for your contribution to fastlane and congrats on getting this pull request merged 🎉 Please let us know if this change requires an immediate release by adding a comment here 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Congratulations! 🎉 This was released as part of fastlane 2.147.0 🚀
Was added in fastlane#10168 because newer versions of public_suffix didn't support all Ruby versions that fastlane did. However fastlane now requires Ruby 2.4 or above while public_suffix supports 2.3 and higher. fastlane#16408 https://github.com/weppos/publicsuffix-ruby/blob/v4.0.5/CHANGELOG.md#400
Was added in #10168 because newer versions of public_suffix didn't support all Ruby versions that fastlane did. However fastlane now requires Ruby 2.4 or above while public_suffix supports 2.3 and higher. #16408 https://github.com/weppos/publicsuffix-ruby/blob/v4.0.5/CHANGELOG.md#400
Checklist
bundle exec rspec
from the root directory to see all new and existing tests passbundle exec rubocop -a
to ensure the code style is validMotivation and Context
Circle.ci officially stopped supporting Ruby 2.3, we have no test coverage for that and they all are at EOL. There is no reason to keep supporting this without being sure it works via tests.