Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

electron-updater を追加し、自動更新を行えるように #543

Closed
wants to merge 12 commits into from

Conversation

raa0121
Copy link
Contributor

@raa0121 raa0121 commented Dec 2, 2021

内容

electron-updater を追加し、自動更新を行えるようにしました。

npm run electron:build_pnever

dist_electron/nsis-web/latest.yml が生成されることを確認しました。
publich すると github に一緒に upload されるかはちょっと分からなかったです

関連 Issue

close #361 #235

Copy link
Member

@y-chan y-chan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

私なりに軽くコードを見たり、調べてみたりしました。すでにvue.config.jsにpublishの項目を設定していることもあり、electron-updaterを利用する方法は非常に良いと思いました!

ただ、せっかくなので、アップデート内容を表示してあげると、ユーザーにとってアップデートがどのような利益をもたらすのかを告知できるので、良いのかなと思いました(必須ではないので、難しそうであれば・もしくはデザインがおかしくなりそうであれば、再レビュー依頼をしていただければ、approveしたいと思います)。

また、アップデート通知をonにするか、offにするかの機能が少しほしいなと思いました。
background.tsで処理しており、electron-storeの情報が利用できると思うので、SettingDialog.vueの中に、その設定を盛り込んで、利用してもいいかなと思いました。
ただ、これはあくまで拡張的な内容なので、別PRに切り分けた方が良さそうです。
もしよろしければ、またPRしていただければ幸いです。

publich すると github に一緒に upload されるかはちょっと分からなかったです

これに関しては、Releaseに関するGitHub Actionsの編集で対応できると思います...!
また、現状はテストビルドやdevビルドも普通にリリースに上げてしまっていますが、それらをpre-releaseとして公開するような変更も必要かと思います(でなければdev版を最新版として拾ってしまう可能性があるからです)。
もしよろしければ、こちらも対応していただき、PRをいただければ幸いです...!

@Hiroshiba
Copy link
Member

僕も @y-chan さんとほぼ同意見です!
一緒にuploadされるかは、実際にreleaseを作ってたら試せるかもしれません。

@Hiroshiba
Copy link
Member

あ、ちなみにですが、自動アップデートをONにしていると2.4GBの7zファイルも全部かってにダウンロードする感じでしょうか。
だとしたらデフォルトで自動更新offのほうが良さそうだなと感じた次第です。

@raa0121
Copy link
Contributor Author

raa0121 commented Dec 5, 2021

理想の動きを plantuml でシーケン図を書いてみました。
これとは別に、メニューバーから、自動アップデートチェックを行えるようにしたいと思いますが、いかがでしょうか。

@y-chan
Copy link
Member

y-chan commented Dec 5, 2021

ちょっと処理が複雑化しそうな気がしますが、ユーザーフレンドリーにはなると思いました。

メニューバーから、自動アップデートチェック

こちらも、対応できるのであれば、ぜひお願いしたいです...!

@raa0121
Copy link
Contributor Author

raa0121 commented Dec 19, 2021

@y-chan 起動時自動アップデートチェック(ダウンロードはしない)、メニューからのアップデートチェック を実装しました。
レビューよろしくおねがいします。

Copy link
Member

@y-chan y-chan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

いろいろ試していただきありがとうございます!

とりあえず気になった部分を指摘です。
実際に機能するか、仮のリリースを作って試してみるので、本レビューはもう少しお待ちください...

src/background.ts Outdated Show resolved Hide resolved
@y-chan
Copy link
Member

y-chan commented Dec 20, 2021

とりあえずいろいろ調べてみたところ、Windowsであれば仰る通りnsis-web下にlatest.ymlが出来、Linuxだとdist_electron直下にlatest-linux.ymlが出来ます。
ところで、Windowsであれば、7z系統の追加ファイル群は勝手にダウンロードされないので良いですが、LinuxだとそもそもAppImage1つにまとめられており、かつアップロード時は分割されているので、以下のようなlatest-linux.ymlが出来ても、ダウンロードに失敗します。
尚且つLinuxだとシェルスクリプトによるインストールが必要となっており、Electron内部で、分割されたAppImageのダウンロードが出来たところで結合までは出来ないので、自動更新は達成できません...
image

@.raa0121 さんにいろいろお願いしているところ申し訳ないのですが、いろいろ考えた結果、自動更新というものが自体が適切ではなく、例えば更新を検知すればホームページに誘導するといった形が最適解に感じます。
@Hiroshiba どうでしょうか...

@Hiroshiba
Copy link
Member

なるほどです。
よくよく考えたら今はzip版も配布していて、zip版だとどういう挙動になるのか・どういう挙動が良いのか調べないとですね・・・

appimageの方は、onnx化ができたら1ファイルになるので、7z化を外せばなんとかなりそうです。
が、時間も掛かりそうですね・・・

処理がややこしくなってしまいそうですが、自動更新からlinuxを省いておき、意図をコメントしておくということはできそうでしょうか?
(macができるかどうかはマージ後に確認で良いかなと考えています)

@Hiroshiba
Copy link
Member

あ、あとreleaseの試作をぜひお願いしたいです。 @raa0121
たぶんraa0121さんのvoicevoxリポジトリで、プルリクエストを出されているbranchを指してreleaseタグをつければ自動的にビルドされると思います。

@PickledChair
Copy link
Member

PickledChair commented Dec 20, 2021

(macができるかどうかはマージ後に確認で良いかなと考えています)

横から失礼します……!
自分も Mac 版で electron-updater が機能するかどうか調べていたのですが、アプリの署名が必須のようです。

参考: https://blog.katsubemakito.net/nodejs/electron/autoupdater

なので、自動更新は現状 Windows だけで有効化すると良いと思います。

@raa0121
Copy link
Contributor Author

raa0121 commented Jan 15, 2022

止まっててすいません。
Mac と Linux で自動アップデートが難しいということだと、Windows でも自動更新せずに、公式サイトか Github Releases のページをブラウザで開くようにするくらいのほうが良いのかもしれないですね。

enableAutoUpdateCheck から isAutoUpdateCheck に
アップデート確認後、ダウンロードではなく、公式サイトに遷移するように
@Hiroshiba
Copy link
Member

アップデートの簡略化はおそらくかなり需要がありそうで、多くの方から望まれていそうです。
https://twitter.com/dj_tatibana/status/1505328843464224770

こちらのプルリクエストの現状はどんな感じでしょうか👀 @raa0121
自動アップデートが難しい場合は、クリックしたら差分アップデートするとかでも良いのかなと感じました!!

@raa0121
Copy link
Contributor Author

raa0121 commented Mar 22, 2022

@Hiroshiba すいません、ちょっと忙しくて手を付けられてませんでした。
現状、 #690 とほぼ同じ機能の実装のみで、テストも未実施です。
インストーラー周りは詳しいわけではないので、差分アップデート等は私には難しそうです。

Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@raa0121
なるほどです、最初にダイアログが出て案内する形なのですね! 良い機能だと感じました!!
どうでしょう、マージできる形に修正していってみませんか👀

コンフリクトを修正し、何らかの形で試してみることでマージできるのではと感じました!

@Hiroshiba
Copy link
Member

@raa0121 すみません、コンフリクト解消してくださったことに気づきませんでした🙇
良い機能だと思うので、ぜひもう一度解消をお願いしたいです!
次は解消後に一言コメント頂けると・・・!

@raa0121
Copy link
Contributor Author

raa0121 commented Jun 9, 2022

@Hiroshiba 大変遅くなりました。
コンフリクト解消しおておきました。

@raa0121
Copy link
Contributor Author

raa0121 commented Jun 9, 2022

テスト落ちたので直しますね

@raa0121 raa0121 force-pushed the electron-updater branch from 87d5f28 to 71ad3cc Compare June 9, 2022 12:41
@Hiroshiba
Copy link
Member

コンフリクト解消ありがとうございます!!
マージしてから、実際にreleaseを作ってみたいと思います。

そういえばこちらはプレリリースがあった場合などはどのようになりそうでしょうか 👀
プレリリースのときに通知してしまうとややこしいので、ユーザーのことを思うと避けた法が良いのかなと思っています。

@raa0121
Copy link
Contributor Author

raa0121 commented Jun 9, 2022

確かに、プレリリースのことまでは考えてなかったですね…
https://www.electron.build/auto-update.html#githuboptions-publishconfiguration
GithubOption に release のタイプを設定できるっぽいですが、微妙にドキュメントがわかりにくいですね…

@Hiroshiba
Copy link
Member

なんかドキュメント壊れてそうですね…!
デフォルトはdraftっぽいですが、これをreleaseにしたらダイアログが出るのか、draftでも出るのかが気になっています。

マージするには、とりあえずauto updateできそうか検証できると良いのかなと感じました。
可能であれば、 @raa0121 さんのリポジトリで試作してみて頂けるととても参考になりそうなのですが、いかがでしょう…👀

(自動更新や更新通知はビルドからリリース、コードまでと総合格闘技なのですが、その分ユーザーからの期待は大きいと思います。
リリース試作などわからない点があればサポートできると思いますので、もしよければぜひ…!)

@raa0121
Copy link
Contributor Author

raa0121 commented Feb 21, 2023

electron-updater を諦め、github api から更新情報を取ろうと思います。
一旦このPRはクローズします。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

自動アップデート機能をつける
4 participants