ヒカカク！をRails5.2.1化したら、なぜかコントリビュータになった話
kojiです。みなさんバグを踏むのは好きですか？
僕は正常にうごくプログラムが大好きな一般のエンジニアですが、歩いていればバグに当たるタイプというか、周囲のみんなが快調に実装を進めている中、ひとりレアアイテムを引き当てて半日を棒に振る程度には彼らに愛されています。
ジラフの開発チームでは「Ruby、Railsなどの技術スタックは常に最新バージョンを使うようにしよう」という方針があるのですが、CTOいわく
「だって、その方が新しいバグを拾えるでしょ」
ということだったので、デキるエンジニアはバグが好きなんだな…と思うことにして毎日自分をなぐさめています。
そんな僕が、先日ヒカカク！をRails5.2化する作業をしていたら、bootsnapのバグを踏み抜いてしまい、幸運にもコントリビュータになれたという話を書いてみたいと思います。
（注：Railsではなくbootsnapのコントリビュータです。釣りっぽいタイトルでごめんなさい）
あと、これからアップグレードする方のためになればと思い、ハマりどころについても最後に記しておきます。
ヒカカク！をRails5.2.1にアップグレードする
さて8/7に発表されたRails5.2.1、さっそくアップグレードされた方もいるのではないでしょうか！
ActiveRecord周りなど、マイナーバージョン１つとは思えないたくさんの修正がつまっていて頭がさがる思いです。
ヒカカク！は最近5.0化したばかりでしたが、それ以来も最新版にキャッチアップする作業を続けていて、ついにRails5.1→5.2へのアップグレード作業を行うことになりました。
幸運にも、デプロイ前日というタイミングでRails5.2.1が発表されたため、いち早く最新バージョンに移行することができました 🎉
困ったこと
移行中にCIを回していたところ、YAML.load_fileメソッドがコケるという現象に遭遇しました。
結論から言うと、Rails5.2から導入されたbootsnapがYAML.load_fileをオーバーライドしていたのが原因でした。
これを素のRubyと同じように、どちらの書き方でも通るように改修しました。（2018/8/10現在 最新masterブランチのみ反映）
※ ここから下は改修アプローチの解説になるので、Rails5.2化のハマりどころについて知りたい方は、最後まで読み飛ばしてください。
改修のきっかけ
ひとまず作業を優先してワークアラウンドでしのいだのですが、「まあ、後世のためにissueでも立てておくか」というつもりでソースコードを読んでみたところ・・
YAMLに関する記述は１ファイルに集約されており、問題の箇所は簡単にみつかりました。
まずいのは引数を無条件に to_sしているところ。下記のように、Fileオブジェクトをto_sした場合、ファイル名ではなく、Fileインスタンスの情報を返します。
これは・・to_sをto_pathに変えるだけの美味しいお仕事や！
リポジトリは活発にメンテされているので、この程度の改修ならさくっと取り込んでもらえるかも？（勝利のファンファーレ）
PR戦術を練る
ということでPRに挑戦してみます。
改修自体は簡単そうですが、どうやって変更を説明するか？というのが悩みどころです。
影響範囲が比較的大きい低レイヤーメソッドなので、改修する必要性を明確に説明できなければなりません。「実は意図があってあえてto_sしてました」という話がないとは言えないし。
1. 「bootsnapが上書いているYAML.load_fileの挙動はRubyと共通であるべき」という説得が一番通りやすそう
2. つまり、Rubyでの挙動がどうなっているかを正確に説明できなければならない
3. CRubyのコードで該当箇所を探しあてる必要がある
これ、がっつりCを読まないといけないヤツや・・（遠ざかるファンファーレ）
Rubyコードでいいの？
また実装面でも「Rubyレベルで修正するのが本当にベストな修正なのか？」という疑問もありました。
というのもRubyコードだけでこの改修を終わらせる場合、YAML.load_fileが呼ばれる際に毎回respond_to?などを利用して「to_pathするかto_sするかを判断する」という分岐を挟むことになります。
パフォーマンス向上のためのgemなので、呼び出しのオーバーヘッドが重くなるようだったら改修する意味が薄くなりそう。
検討してみると、どうやらC拡張関数の改修が必要らしいということがわかりました。
なお僕のCに関するスペックはというと・・
修論でちょっとしたシミュレーションプログラムを書いたくらいで、社会人になってからは１行も書いていません。K&Rも読んでませんしポインタは人並み程度に嫌いだしコンパイル言語恐怖症でさらには末尾の；をよく忘れます。
つまりあんまり書けません。しょんぼり。
C言語と向き合う
ま、見るだけ見てみようということでgithub上でRubyのコードを追いかけてみました。
やっかいだったのは、RubyのYAMLがpsychという別のモジュールで処理されていること。該当箇所が見当たらずすぐに行き詰まったのですが、過去のイシューを読んでいるとヒントらしきことが書かれていました。
ファイルシステムを扱う全てのメソッドは、いったん#to_pathを呼んでから#to_sにフォールバックするようになっている
どうやらRubyではYAMLモジュールだけでなく、全てのファイルIOに関して引数がto_pathできるかをチェックしているらしい。ま、そりゃそうだわ。
というわけで、今度はfile.cモジュールを中心に捜索していったところ、おそらくこれ、という箇所を発見！（戻ってくる音楽隊）
プルリクエスト
やっと正確な実装を理解できたところで、コードを修正して手元で確認。
おお、社会人になってから初めてCコンパイルをしたよ（感動）
震えながらPRを投げてみたところ・・
翌朝にはレビューがついてました。
こ、これは、世界のrafael franca大先生だ・・ Σ（・□・；）
「GCを考慮してこのマクロ使った方がいいよ」というアドバイスでした。さっそく修正PRを出したところあっさりマージ。
内容としてはこんなことになりました。
タッタッタタラー♪
・・・どこがC言語やねん。
ありがとうShopify。ありがとうRuby。ありがとうプリプロセッサ。マクロを１コ足した程度でコントリビュータと言ってしまっていいのでしょうか。
そのほか移行にあたっての問題
Rails5.2化へ向けて問題になった箇所を、いつか誰かの役に立つかもしれないということで書いておきます。
webpackerがdevDependenciesを参照しない
rake yarn:install時に、webpackerがpackage.jsonの中にあるdevDependenciesを参照してくれず、CIが死んでしまうようになりました。
該当イシューがこちらに立っています。
幸い依存パッケージが少数だったので、泣く泣くDependenciesに入れることでいったん仮の対処としました。
ActiveRecord::QueryMethods#joins がコケる（※5.2.1以降）
Rails5.2.1からActiveRecord内部の一部のインターフェースが変わっている関係で、joinsが走る場所でテストがコケるようになってしまいました。
Ransackが依存しているpolyamorous gemが怪しいと思い、イシューを立てて質問したところ、実はy-yagiさんがRansack本体に光の速さでパッチを当ててくださっていたことが判明。ありがたや〜
すでにRansack2.0として公表されているため、 bundle update ransackしておけば問題なく動くと思われます。
cache_keyの仕様が変わる
ActiveRecord::Integration#cache_keyがレコードごとに一意になり updated_atの情報を持たなくなります。
今まで通り、更新日時を含めたキーとして取得したい場合は、 cache_key_with_versionというメソッドを使うことができます。
application.config.active_record.chache_versioninの設定で、しばらく旧仕様を引っ張ることもできますね。
ActiveRecord#Dirtyの仕様変更
これは前からわかっていた話なのですが、Rails4系からバージョンを上げていったため改修が追いついておらず、モデル内に散らばっていた attribute_wasを放置していました。この機会にようやく根絶しました。
破壊的な変更の割に、正確なドキュメントが見当たらないのが頭痛の種ですが、この辺りを読むとざっくりつかめるかと思います（完全ではない印象）。
ややこしいことにActiveModel#Dirtyのattribute_wasはそのまま生きているたりするので、知らずに使ったら通ってしまうように思うのですが、これはどういうことなのでしょうか・・
また、ここのQiita記事が素晴らしくポイントを押さえているので、「えーっと、あれどうだっけ・・」となった時に何度も参照させていただきました。ありがとうございます。
YAML.load_fileが失敗する
はい。現時点だとgithubからbootsnapの最新マスターを引くか、もしくは引数に渡すファイルオブジェクトをto_pathすることで対応可能です。
まあ最初からFile.openなどせず、パスを文字列で渡す手もありますね。かんたん。
あと荒技として compile_cache_yamlオプションを切っておくという手もあります。
まとめ
というわけで、ヒカカク！のアップグレード時になぜかC言語と格闘した話でした。
スママやpeingといったプロダクトはすでにRails5.2.0だったため、5.1系のヒカカク！チームとしては若干の負け感があったのですが、今は兄弟たちより0.01m高い場所からほんのり見下すような穏やかな気持ちです。
（といってもスママの5.2化を担当したのも自分・・誰と戦っているのか）
バグを踏みやすいエンジニアってどうやねんと思いますが、おかげで他プロダクトのエラーを未然に防げたかと思うと悪くない気がします。世界中で発生するデバッグ作業を救えたかもしれないなんて、エンジニア冥利につきますね。
・・という、真っ赤なお鼻のトナカイさんみたいなオチでした。
最後に宣伝です。
ジラフでは最新バージョンのRubyやRailsを使って、ヒカカク！やスママ、peingといったプロダクトを一緒に開発する仲間を募集しています。
社内にはC++使いや、scala使い、PHP使いなどもいて、goやsolidityの勉強会も活発に行われていますが、仕事でC言語を書かされることはありません。
あくまで開発はRuby on Rails＋フロントjavascriptなのでご安心ください！
