唯物是真 @Scaled_Wurm

プログラミング(主にPython2.7)とか機械学習とか

nodenvに小さなバグを見つけてプルリクエストを送ってマージされた

仕事で使っていたコードのCIのスクリプトがある日突然動かなくなって、調べたらデバッグ出力用にnodenv versionsのコマンドを実行していたのが原因だったのがわかりました
少しの修正で直りそうな問題だったので休日に直してプルリクエストを送りました
英文を書くのは久々だったので苦労しましたが、プルリクエストをしたらすぐにマージしていただいたのでありがたかったです

github.com

f:id:sucrose:20190128001517p:plain

バグの原因が、あまりBashに慣れていない自分が把握してなかった仕様の話でおもしろかったのでメモ代わりに記事にしておきます

詳細

`nodenv versions` exit(1) unexpectedly when no "system" Node is installed · Issue #131 · nodenv/nodenv · GitHub

問題

nodenvはプロジェクトごとなどで様々なnodeのバージョンを切り替えながら使いたいときに使うソフトウェアでnodenv versionsを実行するとインストールされているnodeのバージョン一覧が出力されます
このときnodenv用にインストールしたnodeとシステムに元から入っているnodeが出力されるのですが、あるコミットの後からnodenv用にインストールしたnodeはあるけど、システムのnodeがない場合エラーを出して終了するようになっていました

if [ "$num_versions" -eq 0 ] && [ -n "$include_system" ]; then
  echo "Warning: no Node detected on the system" >&2
  exit 1
fi

$num_versionsにはインストールされているnodeの個数が入っていて、本来1つ以上nodeがインストールされていれば上のif文の中身は実行されないはずなのですが、何故かこの分岐に入るようになっていました

原因

最近のコミットを見ると$num_versionsは以前はforループの中で計算されていたのが、以下のようなパイプでwhileに渡してループするというコードに修正がされていました(print_versionの中でカウントされています)

{
  shopt -s nullglob
  for path in "$versions_dir"/*; do
    if [ -d "$path" ]; then
      if [ -n "$skip_aliases" ] && [ -L "$path" ]; then
        target="$(realpath "$path")"
        [ "${target%/*}" != "$versions_dir" ] || continue
      fi
      echo "${path##*/}"
    fi
  done
  shopt -u nullglob
} \
| sort_versions \
| while read -r version; do print_version "$version"; done

一瞬見ただけだと正しそうに見えるのですがこれがバグの原因でした

パイプで渡すとコマンドがサブシェルで実行されるので、| while read -r version; do 何らかの処理; doneの中で変数を操作しても外側の同名の変数には影響しません

count=0
seq 1 10 | while read -r x; do count=$(($count+1)); done
echo $count # $count は上のwhileループに影響されずに0のまま

www.atmarkit.co.jp

修正

というわけで以下のようなパイプを使わない形に修正して無事解決しました

count=0
while read -r x; do
  count=$(($count+1))
done < <(seq 1 10)
echo $count # $count は10になる

雑学

ちなみにzshだとまた動作が違ってパイプで繋がれた最後のコマンドの場合同じスコープで計算されるようです。bashでも shopt -s lastpipeすると同様になるらしい(シェルを対話的に動かしてるとlastpipeしてもダメだった)
パイプラインとサブシェルの問題はシェル依存 - 拡張 POSIX シェルスクリプト Advent Calendar 2013 - ダメ出し Blog