[jsk_robot_startup][jsk_fetch_startup] Add dry run option of update_workspace.sh

[tkmtnt7000]

This PR added dry run option of update_workspace.sh.

Now fetch and pr2 workspaces forcefully update at 3:00, but this function forcibly erases changes in the robot body, which was troublesome when, for example, we wanted to prepare a demonstration the day before a tour.
This PR avoids that situation.

豊島岡の見学会準備のフィードバックを受けての変更です．
毎朝3時にロボット体内の変更をすべて消し去るのはよくないのではないか，というところを反映しています．
今回の変更を適用するとFetchのwstool updateは手動で行うのがデフォルトになります．

[mqcmd196]

I'm sorry, I overlooked your fetch's PRs for some github notification fault...

[tkmtnt7000]

このwstool diffをメール本文に追加できると良いが，diffの中身に含まれているシングルクオートやダブルクオートが後のrostopic pubする部分で悪さをしてうまくいかない

[sktometometo]

sedでエスケープシーケンスつけたりして回避できないかな

[tkmtnt7000]

ありがとうございます。試してみます。

[tkmtnt7000]

8a73f09 でできるようになった気がしています．

[mqcmd196]

I agree with adding options for dryrun, but I don't agree with making dryrun default.

for example, we wanted to prepare a demonstration the day before a tour

If you want to do this, I think the correct way is sending app's PR to develop/fetch then merge. If you find some trouble with the app, you create PR additionally.

[tkmtnt7000]

If you want to do this, I think the correct way is sending app's PR to develop/fetch then merge. If you find some trouble with the app, you create PR additionally.

Yes, so far I have agreed with that view, but the problem might be that the workspace is cleaned up to default branch without any notice for first-time users. If there are any changes left in the robot's body, it would be more helpful to notify the user by leaving the changes in robots' workspace instead of deleting them.

毎回手動でワークスペースを更新させるこのPRは更新忘れなどを誘発するのでやりすぎな気もしますが，体内diffが残っていたらその旨を通知してdiffを残しておくくらいにしても良いかもしれないと思ってのPRでした．

610と73B2のFetch, PR2ユーザーあたりが関係していそうなのですこし意見を伺いたい気持ちもあります．（すでにPR1012はworkspace updateを停止しているらしいので）

FYI:自分も耳が痛い話ではありますが最近githubが活発でない（結果として他人の研究内容・実装領域がわかりづらい）ことの弊害もあるのではないかとの意見も少しもらったので

[sktometometo]

1. デモの前に変更をリセットしてほしくない話
2. デフォルトの挙動を変える話と
3. diff送りたい話

は多分それぞれ別の話で、1. はそのときcron書き換えて無効化するなり-nオプションつけるなりしたらいいかなと思うし、3は普通にやったら良さそう
2については

[mqcmd196]

個人的にはgit dirtyな状態をfetchユーザのworkspaceでは放置してほしくないです．そのあとに使う人が，いつもと違う挙動だった時にcommitで何が違うのかを遡れない，dirtyな状態にした人の変更をわざわざ見ないといけない，dirtyな状態にした人に連絡を取ってどうすればいいか聞かないといけない，というのを毎回やるのは結構つらいです．

基本は自分のユーザ作ってfetch:/home/fetch/melodic/を上流とするworkspaceを作る，それで難しい開発はfetchユーザで試して，自分の実験が終わったらそれをコミットして必要に応じてPRに出す，でいいと思っています．で develop/fetch は master ではないので，アグレッシブにコミットしていったらいいと思います．

FYI:自分も耳が痛い話ではありますが最近githubが活発でない（結果として他人の研究内容・実装領域がわかりづらい）ことの弊害もあるのではないかとの意見も少しもらったので

これは完全に申し訳なくて，このリポジトリがwatchされていると勘違いして，イベントが何も見れていませんでした．気を付けます．

ちなみに git reset --hard がやりすぎ，ということであれば git stash するのも手だと思うのですがどうでしょう

[nakane11]

610のPR2でworkspace updateを停止した理由としては、diffとは別にbuildに失敗して後から手動でやり直す必要があると、翌朝デモなどですぐに使いたいときに困るというのと、サーボ落ち問題などのデバッグのための一時的な変更が消されてしまうというのがあります

pr1012のユーザが少ないからできるのかもしれませんが、修理用のdiffが共有されている状態だと、ブランチを切るだけでなくリモートにpushする必要があるのは少し手間でした

[mqcmd196]

diffとは別にbuildに失敗して後から手動でやり直す必要がある

これはどうして失敗するのですか？

サーボ落ち問題などのデバッグのための一時的な変更が消されてしまう

なるほど，これはcronを一時的に切るという理由にはなりそうです．

[nakane11]

これはどうして失敗するのですか？

buildの順序の問題みたいです（cmakeを直せば解決する）

[tkmtnt7000]

パッと見た感じだとjsk_rviz_pluginsのビルドに失敗している
https://gist.github.com/tkmtnt7000/e22df9aaec497fc1fda7b1acd7a2318a#file-update_workspace_pr1012-txt-L9399

[sktometometo]

途中まで書いてて放置してごめん、デフォルトの挙動をどうするかの話は、nightly build + 常時動かす実験としてのキッチンデモのあるFetchとないPR2で違う事情があると思っています。
PR2のupdate_workspace.sh の事情把握してないんだけど、Fetchと同様に動作させる形になっているとしたら多分ブランチの意味合いとか、実験・開発のフローも少し変わるはずなので、ロボットごとにデフォルトの挙動は変えて然るべきかなと思います。
(PR2に移植されたときにキチンと議論してなくてすいません)

PR2は常時ロボットを動かす実験というものが無い(Tweetとかはあるけど)しnightly buildも無いので、毎日環境をリセットする必然性は薄いかなと思います。ただ、新入生がつかおうとして安定して動作させられる環境(ラップトップ側だけでプログラムを書いて実験出来る環境)はメンテナンスした方がいいと思うので、各々のブランチとは別に共通して安定して動作する環境は合ったほうがいいと思う。PR2のdevelop環境はこの立ち位置が結構強いかなって思ってます。
そうすると開発フローとしてはなにかロボット体内のプログラムを帰るときにdevelop環境にPRを送るみたいなことする必要はなくて、ここの環境でしばらく動作させて安定したらdevelop環境にマージするくらいでもいいのかなって思います。

そうなると今の update_workspace.sh を動作させるのはたまに安定版であるdevelop環境がきちんと動作することを確認するために使うという意味くらいになりそうだから、cron切っておくのはいいんじゃないかな。
使っている人がいない個体については有効化しておいても良さそうだけど。

もしPR2で毎日update_workspace.sh的なものを動作させるとしたら、体内環境をdevelop環境へリセットするというより、develop環境とは違う状態であることをnotifyするくらいでいいかもですが、これは使う人たちが今こういう状態ですってコミュニケーションしていれば十分な気もする。あとupdate_workspace.shにその機能はないです。

Fetchのupdate_workspace.shはもともとキッチンデモをdevelop環境のnightly buildとして動作させる意味合いで今の形にしましたが、今キッチンデモはdevelop/fetch環境のnightly buildとしての用途と、常時行われる実験としての用途の二種類あると思っていてこっちもちょっと分けたほうがいい話だなと思っています。
nightly buildの話はPR2と同じで共通安定版であるdevelop環境がbuildからデモの実行まで正常に動作して、安定版を使いたい新入生とかがすぐにFetchを使えるように毎日テストしておくという意味合いで、もともと update_workspace.sh はこの用途で作ってました。これは放置しているだけでFetchが使える状態にあるかどうかが確認できるので皆知っての通り便利に使っています。ただ、ちょっと不完全な部分があると思っていて、今, update_workspace.sh とリブートとキッチンデモは別々に走っているのでその場でジョブを走らせるのが大変だなと思っています。昼間走らせると誰か使っているかもだし。
nightly buildの話は共通安定版の話というよりは、develop環境がbuildからデモの実行まで正常に動作して安定状態であると言えるかどうかを毎日テストしておくという意味合いで、もともと update_workspace.sh はこの用途で作ってました。これは放置しているだけでFetchが使える状態にあるかどうかが確認できるので皆知っての通り便利に使っています。ただ、ちょっと不完全な部分があると思っていて、今, update_workspace.sh とリブートとキッチンデモは別々に走っているのでその場でジョブを走らせるのが大変だなと思っています。昼間走らせると誰か使っているかもだし。

常時行われる実験という意味合いは上の基盤の上に乗っかっている形になると思うんだけど、キッチンデモを安定動作する実験であることを前提にしていると思います。ただこれはdevelop環境を不安定にする実験とか開発とちょっと相性が悪くて、不安定期間が長くなると実験できなくなるんだよね。それでmasterに送れないようなPRをdevelop環境に送るなって話が出てた。
こういう話があったので、develop環境の動作を不安定にさせるような体内の環境を大幅に切り替えるような実験・開発をするときは体内の自分のユーザー以下にワークスペースを作ってsupervisorとは完全に別に動作させるという話になってたと思うんだけど、前述のnightly buildの全プロセスを一度に走らせられない話や、develop/fetch環境が定期的にmasterにrebaseされる話と一緒になってこういう系の実験・開発はdevelop環境へのマージがすごく大変になります(大変でした、諦めた変更もある)。

こういう事情で、develop環境にすぐにマージするという話はdevelop環境を共通安定版として使うという話とある程度干渉する話だと思ってて、今のFetchの使い方だとこういう話起きるかなと思ってます。順当に解決するには不安定版(develop)と安定版(stable)両方を用意してそれぞれでテストを走らせて、安定版に乗っかった実験をする人は安定版に乗っかって実験するとかになると思うんだけど、2つの環境を自動で切り替えるような仕組みをまた作る必要があるし、ブランチ管理がさらに大変になるし....
今こういう部分に取り組める人はいないと思うので、masterに送るかわからないようなある程度ずっと動かしておきたい変更があるときに、周知した上でPR2と同様に一時的にupdate_workspace.shを切って動作させるのはいいんじゃないかなと思います。

[tkmtnt7000]

皆様ありがとうございます．
議論の流れを受けて，一旦Fetchのデフォルト挙動を変えるコミットについては削除しました．
(今回の変更によってワークスペースになんらかのdiffがある場合はfetch@のメールに通知されるようにはなっているます．)

色々とお話を聞いている中で思い出しましたが，
今のupdate_workspace.shができた背景としては複数人で共用のロボットを使っている場合に，次にロボットを使う人がそれ以前に使った人のサイレント変更によって実験の前にまずデバッグから始めなければならない，という状況を避ける意味合いがあったと思います．

基本は自分のユーザ作ってfetch:/home/fetch/melodic/を上流とするworkspaceを作る，それで難しい開発はfetchユーザで試して，自分の実験が終わったらそれをコミットして必要に応じてPRに出す，でいいと思っています．

実験で使ったデモンストレーションは基本的にロボットのデフォルト挙動をcriticalに変更するものは少ないはずなので，jsk_demosすぐにPRとして出してどんどんマージしていくと，ロボットのデモがリッチになって良いと思います．他の研究室メンバーが何をしようとしているのかを知ることでその他の人の研究が進む種になるのではないか，というところが今回このPRが出ることになった理由の一つでもあります．

そうなると今の update_workspace.sh を動作させるのはたまに安定版であるdevelop環境がきちんと動作することを確認するために使うという意味くらいになりそうだから、cron切っておくのはいいんじゃないかな。
使っている人がいない個体については有効化しておいても良さそうだけど。

個人的な意見ではありますが，現在のワークスペース構成でロボット体内でビルドが通ることを確認しておくことに意義はありそうな気がしてます．毎日する必要はないので頻度に関しては柔軟に対応して良いと思います．

[k-okada]

結局どうなったのかな。

1. diff があったら伝えてほしい
2. build が通るか確認してほしい
3. 最新のbranchをmerge してほしい

という目的があるとして

1. はできるとして、diff があったときに reset hard するか stash するか 放置するか
2. は出来るとして、エラーがあったらどうするか？　→　人出で直す
3. は出来るとして、 merge 失敗してコンフリクトしたらどうするか？　→　元に戻るか

常時デモが動くのは良いことなんだけど、一方でデモは毎回進化していってほしいという事があります。ここがむつかしい。
今回困ったのは、前日にデモを確認して、当然、デモの度に進化させるのが至上命題なので、何か気が付いたことはさっとプログラムを修正するんですが、その修正しても、翌日の3時にリセットされると思うと結局いじれないのと、jsk-ros-pkg/jsk_pr2eus#495 でプルリクエストが出ているけど、出しただけでどこにも反映されていないので、なにか良い方法がないか、というのが話の始まりでした。
そう考えると午前3時に毎日リセットされてしまう、というところを何とかならないか、と思ったという事かな。cron変えればよい、という話も出ているけど、次の日に10時からデモで、夜中の11時にプログラム書いていて、いいでもできた、と思ったときに、Cron間違えたり忘れると朝の3時に全消しになるリスク、とういのは、かなりの恐怖ではないかな。

他の案で思いつくのは、

1. プルリクエスト出したら developにもマージするべきだった？
2. 自分のワークスペースで作業すればよかった？ちなみに、最初にデフォルトで上がるWorkspaceを変えて運用する方法を定着させて、~/fetch/ros (?) 出ない場合は特殊モードでWarningをメールなり起動音なりモニタなりで表現する？デモの前や集中実験期間はこちらで各自のプログラムが動く

ですが、現状でreset --hard がやりすぎなだけかと思ったのですが、意図はgit dirtyな状態を許容するのではなく

1. dirty な状況が送られてくる→誰かが頑張ってデバッグなり新機能の開発をしていることがわかる
2. dirty な状況が送られ続けてくる→良いところでPRを出せなくなっているOR放置している。→メールがき続けることで本にへのプレッシャーとなってほしい
3. よくわからないdirty が残っていて、何か行われている様子が無い。あるいは本人から連絡がない。そういう時はgit reset --hard することはOKという事でみんなで許容する。

でよいのでは？と思ったんだけど３）は発動されずに、結局diffが残り続けるのかな。あるいはreset hard した時にプログラムが動くことが保証されていない？大体OKだけど、hardして戻ったときのcommitだと、その時点の古い外部依存ライブラリでしかコンパイルしていないから、その途中で外部依存ライブラリがupdate されたらコンパイル通らない、というケースがあるかな。

[sktometometo]

@tkmtnt7000 Nice feature. thanks!

[tkmtnt7000]

ありがとうございます。一旦マージします