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

[ fix, change ] puts behavior for Ruby v3.0.x #41

Merged
merged 1 commit into from
Sep 5, 2021
Merged

[ fix, change ] puts behavior for Ruby v3.0.x #41

merged 1 commit into from
Sep 5, 2021

Conversation

gemmaro
Copy link
Contributor

@gemmaro gemmaro commented Sep 2, 2021

  • Ruby v2.7.x 以前では通っていたテストも Ruby v3.x.x から Encoding::CompatibilityError により失敗する箇所があることを確認しました。
    • Ruby v3 から Kernel#puts (もしくは IO#write )に変化があったようです(詳細は未調査です)。
  • この変更は puts に変更を加えることでこの問題を回避しようとしています。
    • この変更の仕方ではなく、別のメソッドを定義してそれを使うことで回避した方が安全かもしれません。
  • lib/i18n.rb へのテストを追加しました。
  • 関連: GitHub Acitons: support Ruby 2.7 and 3.0 #39

@gemmaro gemmaro marked this pull request as ready for review September 2, 2021 23:32
@takahashim
Copy link
Contributor

PRありがとうございます!

これなんですが、「(詳細は未調査です)」について、何か思い当たるところ等ありませんか?
基本的にはShift_JIS外(CP932にはある)文字を含んだStringをShift_JISにforce_encodingしているものを出力するのでエラーになっているかと思うのですが、それで2.xと3.xの挙動の違いがなぜ起こるのか、というところまではわかっていません。その辺が不明な状況で rescue Encoding::CompatibilityError するのはちょっと危険かな? と思っています。

とはいえRuby 3.xで使えないのは不便なので、詳細不明の場合はまず取り込んでから検証するのでもいいか…と思っています。

@gemmaro
Copy link
Contributor Author

gemmaro commented Sep 5, 2021

これなんですが、「(詳細は未調査です)」について、何か思い当たるところ等ありませんか?

今のところ思い当たるところはありません。

その辺が不明な状況で rescue Encoding::CompatibilityError するのはちょっと危険かな? と思っています。

こちらおっしゃる通りだと思います。2.7.x -> 3.x で関連するイシュー (GitHub, Redmine) が直ちに見当たらなかったため、調査を後回しにしています。

とはいえRuby 3.xで使えないのは不便なので、詳細不明の場合はまず取り込んでから検証するのでもいいか…と思っています。

おそらくRubyの実装を見ればわかりそうではあるのですが、私は調査の方針がついておらず時間が掛かりそうです。
マージ可否の判断はお任せします。

@takahashim
Copy link
Contributor

了解です、危ういのは危ういとして、それでも正常なケースでの挙動がおかしくなることはなりそう(異常ケースで誤って通るケースはあるかも)なので、このPRでマージします。ありがとうございました!

@takahashim takahashim merged commit a96b783 into aozorahack:ga-support-ruby-30 Sep 5, 2021
@gemmaro gemmaro deleted the ga-support-ruby-30 branch September 5, 2021 14:28
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