directmlのダウンロードをwindowsでのみ有効にするように修正

[qwerty2501]

VOICEVOX/voicevox_core#210 (comment) でテストに失敗するため、Windows以外ではdirectmlを指定してあっても無視をして通常のダウンロードを行うように修正した

[qwerty2501]

build.rs上でtarget_os="windows"で判定しようとするとtargetがwindowsでも実行環境のOSで判定されてしまうため環境変数から判定されたTRIPLETでOSを判定するようにしている

[qwerty2501]

再度レビューとなってしまい申し訳ありませんがよろしくおねがいします

[Hiroshiba]

LGTM！！

なのですが、仮にcargoビルド時のコマンドでfeatureを指定する方法があれば、コマンド引数で制御しちゃう手もありそうだなと思いました。

それが不可能なら、ビルド前にtomlを書き換える手もありそうですが…それは流石に強引すぎそう。

[qwerty2501]

なのですが、仮にcargoビルド時のコマンドでfeatureを指定する方法があれば、コマンド引数で制御しちゃう手もありそうだなと思いました。

もちろんできますが、テスト時には --all-featuresにして全てのfeatureが有効になった状態でテストされるのをできるだけ維持したほうがよいかなと思ったのでこういう対応になってます

[Hiroshiba]

なるほどです。
ちゃんと影響を把握していないのですが、テストコードやらビルドコードがごちゃごちゃするよりは、-rs側に統一的なロジックがあるのも見通しが良いのかなと感じました！

[PickledChair]

@qwerty2501 @Hiroshiba

どちらの意見も納得できる部分があり、それぞれの案にトレードオフがあるので、どちらを採用すべきかちょっと迷うところがありますね……。ただ私の意見としては

cargoビルド時のコマンドでfeatureを指定する方法があれば、コマンド引数で制御しちゃう手もありそうだなと思いました。

という Hiroshiba さんの意見寄りです。

テスト時には --all-featuresにして全てのfeatureが有効になった状態でテストされるのをできるだけ維持したほうがよいかなと思ったのでこういう対応になってます

という qwerty さんの意見にも賛成できる点はあります。現状、voicevox_core の特別な feature は directml だけだと思いますが、今後何か feature が増えた場合も、--all-features オプションの指定があることにより、テストの設定を変えずに漏れなく全ての feature のコードをテストできて便利、という側面はあると思います。

ただ、Linux と macOS でも directml feature を設定してよいという状態なのは、実際にはビルド結果が directml feature を設定しない場合と同様になるという直感的でないものになり、個人的にはちょっと違和感があります（それよりはビルドが落ちてほしい）。

また、directml feature が設定されていると #[cfg(not(feature="directml"))] の条件でコンパイルされる部分のテストが行えないことになります。default のコンパイル条件のコードがテストされないことがある点で --all-features の指定は問題があるかもしれません（むしろ default のコードの方が一般的なケースなので優先的にテストされてほしい）。

以上より、voicevox_core 側のテストにおける --all-features の指定をやめて、特にテストしたい feature がある場合は明示的に指定して追加のテストをするという方が好ましい感じがしたのですが、いかがでしょうか……？

[qwerty2501]

たしかに--all-featuresだと not(feature="directml") のようなコードが増えた場合その部分がテストできなくなりますね。
そういう意味だとテスト実行時に必要なfeatureを指定できるようにするのが正のように思えてきました
このPRについては一度closeし、voicevox_coreのPRに対して修正をかけるようにします