1
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

More than 3 years have passed since last update.

nvmにコントリビュートするのに5年かかりました

Posted at

TL;DR

きっかけ

https://github.com/DhavalKapil/luaver
私はluaverというnvmのLua版のプロダクトに頻繁にコントリビュートしていました。luaverは当初はBashにsourceして使うプロダクトだったのですが、nvmと違いPOSIX sh互換ではありませんでした。当時luaverの作者のDhavalKapilはzshのサポートを追加したいようだったので、Shellcheck導入やBashismの排除などに関する知識の提供を勉強しながら行っていました。その中で、当時からnvmは複数のシェルをサポートしていて、かつシェルにソースして使うプロダクトとしてはとても成熟していたので、nvmのシェルスクリプトの書き方が非常に参考になっていました。例を挙げると、コマンド実行時のエラーハンドリングなどはnvmのコードから大いに学ぶことがありました。一方で、nvmですら用いられていないような高度なテクニックの開発もしていました。

問題の発見

luaverのコントリビュートの中で特に力を入れていたのが、PATH環境変数からluaverが追加したLuaのバイナリが格納されているディレクトリのコンポーネントの除去に関するコードです。nvmでもluaverでも当初はsedを使って行われていたのですが、いくらかテストしている中で、sedでは絶対にバグが発生してしまうことが発覚しました。(具体的にはディレクトリ名に正規表現のメタ文字列が含まれているとおかしくなってしまいます)なので、POSIX互換でなんとかPATHコンポーネントを除去する方法として、awkコマンドを用いてPATHコンポーネントを除去する方法を考案しました。luaverにはこのコードは入っていませんが、せっかく考えついたのでnvmにとりあえずプルリクとして送っておきました。

フィードバック

https://github.com/nvm-sh/nvm/pull/1360#issuecomment-268956543
https://github.com/nvm-sh/nvm/pull/1360#pullrequestreview-14273700
プルリクを送った直後、nvmのAuthorのljharbからフィードバックが返ってきました。内容は、このプルリクは本当に妥当なのかという疑問と、テストを追加してくれというものでした。当然そのような反応が返ってくるのは分かっていたので対応しましたが、その後、そのプルリクへの対応は5年もの間有耶無耶になってしまいました。

Changesetの説明

ここでChangesetの説明をしておきます。以下がプルリクのメインの部分のPatchです。

-  nvm_echo "${1-}" | command sed \
-    -e "s#${NVM_DIR}/[^/]*${2-}[^:]*:##g" \
-    -e "s#:${NVM_DIR}/[^/]*${2-}[^:]*##g" \
-    -e "s#${NVM_DIR}/[^/]*${2-}[^:]*##g" \
-    -e "s#${NVM_DIR}/versions/[^/]*/[^/]*${2-}[^:]*:##g" \
-    -e "s#:${NVM_DIR}/versions/[^/]*/[^/]*${2-}[^:]*##g" \
-    -e "s#${NVM_DIR}/versions/[^/]*/[^/]*${2-}[^:]*##g"
+  command printf %s "${1-}" | command awk -v NVM_DIR="${NVM_DIR}" -v RS=: '
+  index($0, NVM_DIR) == 1 {
+    path = substr($0, length(NVM_DIR) + 1)
+    if (path ~ "^(/versions/[^/]*)?/[^/]*'"${2-}"'.*$") { next }
+  }
+  { print }' | command paste -s -d: -

従来はsedを使って、${NVM_DIR}から始まる、nvmが管理しているNodeのPATHコンポーネントを除去するという動作をしていました。後方互換性のために2種類あり、PATHの一番最初にある場合と二番目以降にある場合とそれしかない場合に対応するためにそれぞれ3種類あるので、合計6種類ものsedスクリプトがありました。しかし${NVM_DIR}に正規表現メタ文字が含まれている場合は、メタ文字として動作してしまうので、想定通り動作しないという問題がありました。
新しいコードではawkを使っています。awkはマイナーですがPOSIX標準に含まれていてほとんど方言もないため、どのような環境であっても信頼性高く使うことができます。このコードのポイントは-vオプションです。sedと異なり、awkは変数として文字列を-vオプションで渡すことができるので、${NVM_DIR}に正規表現メタ文字が含まれていたとしても正常に動作します。またRSという変数はレコードの区切りを指定する変数なので、PATH環境変数に対して:を指定すると、自動的に各PATHコンポーネントに分割されてawkプログラムが適用されるようになります。
awkでは次のような形式でコードを記述します:条件 { コード }。一行目のindex($0, NVM_DIR) == 1という条件は、レコードが${NVM_DIR}と先頭一致している場合は以下のコードを実行するという式です。2行目はレコードから先頭の${NVM_DIR}を取り除いた部分文字列を取り出すというコードです。3行目は取り出した部分文字列が関数の第二引数で指定されたとパスと一致しているかを検査して、一致している場合はそのレコードの出力をスキップするというコードです。第二引数には正規表現メタ文字は混入しえないという仕様だったので、第二引数はシェルの変数展開で直接挿入しています。このawkのコードはエスケープのことを考えなくてもいいようにシングルクオートで囲っているのですが、ここだけダブルクオートで囲う必要があったので一旦シングルクオートを終了させてダブルクオートにここだけ変えています。(例えばecho 'This is '"$(pwd)"'.'と書くとThis is /Users/umireon.と出力されます)4行目は無条件で現在のレコードの値を出力するというコードですが、3行目でnextが実行された場合はここには到達しないので、条件に一致するPATHコンポーネントだけが除外されて出力されるという挙動が実現できます。

復活

6日前、突然このプルリクがnvmのAuthorのljharbによってApproveされました。5年前のプルリクだったので至る所ぶっ壊れまくっていたはずなので、いきなりApproveされて困惑しました。思わずこう質問しました:「Is this PR still valid?」。答えはこうでした:「yep! there's an unrelated test failure; i'll rebase this again after fixing it.」。その後、いくらかやりとりをして現状に即していなかったコードを修正して、無事にマージされました。

結論

忍耐も時には大事です。

1
0
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
1
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?