Implement installers via electron-builder #138

razzeee · 2016-05-13T19:59:01Z

Hey,

this might be totally wrong, as I'm a fist time node user. And so far I could only test win.
The installation of the exe seems to work fine, but there is no shortcut anywhere to be found. Uninstall also seems to work fine.

Would be very cool, if other people could build it for osx/linux and let the maintainer know if it works.

Cheers

razzeee · 2016-05-13T19:59:50Z

package.json

-    "linux": {
-      "synopsis": "Mattermost Desktop"
-    }
+  "app-bundle-id": "org.mattermost.desktop",


@yuya-oc this might be something you want me to change

Please keep indent.

@it33 Mattermost has two kind of domains. Which is better, .org or .com?

.com please since desktop apps will eventually be hosted at releases.mattermost.com, sorry for late response for some reason I didn't see this mention

Thank you. Changed.

yuya-oc · 2016-05-14T06:12:26Z

package.json

  "engines": {
    "node": ">=4.2.0"
  },
  "scripts": {
    "install": "cd src && npm install",
-    "postinstall": "npm run build",
+    "postinstall": "install-app-deps",


This one is not necessary because dependencies are prepared by npm run build.

yuya-oc · 2016-05-14T06:58:23Z

The builder seems to work with my Ubuntu 14.04. But it doesn't meet with current CI process (You don't have to take care yet). I saw following behavior about installation.

The installer launches the app.
There are no shortcuts as you said.
The uninstaller also launches the app.

I'm also beginner for electron-builder, but it seems that some codes are necessary to handle shortcuts. So could you check below?

https://github.com/electron-userland/electron-builder#auto-update
Where is the actual app binary installed into?

Then, if you want to continue this PR, please check Mattermost CLA. http://www.mattermost.org/mattermost-contributor-agreement/

razzeee · 2016-05-14T08:34:53Z

Sorry, I forgot to add the build folder. We would also need a background.png for osx I think? Not sure what to do there.

Yes, I'm aware of it not taking into account your ci setup right now.

3.) Yes uninstaller also opens the app, takes a few seconds then it closes and is uninstalled. That's whats happening with windows.

I'll try to look into the shortcuts again.
On windows it's installed to the userspace so C:\Users\razze\AppData\Local\mattermost for example.

CLA signed, thanks for pointing me towards it.

Edit: Here is what we have to do for the shortcuts, but it's a typescript example unfortunatly.
https://github.com/develar/onshape-desktop-shell/blob/master/src/WinSquirrelStartupEventHandler.ts

razzeee · 2016-05-14T08:37:38Z

The problematic part about me doing the autoupdate stuff is, that I don't know if or how you will handle the server part. If I can assume, that we're using https://github.com/GitbookIO/nuts and heroku, I can probably come up with something.

razzeee · 2016-05-14T08:42:10Z

Well it seems like you will need to do the server stuff. I don't have these data :)

yuya-oc · 2016-05-14T10:41:30Z

Thanks. For now, we don't have to think about the update server. I think that the point of handling Squirrel event is create/remove shortcuts on the event. If there is no server, the updater merely fails. So I think that's not important matter. Of course, we will have to handle updater and server to provide auto-update.

razzeee · 2016-05-19T21:26:49Z

script/installer.js

+  return Promise.resolve({
+    appDirectory: path.join(outPath, 'Mattermost-win32-ia32'),
+    iconUrl: 'https://raw.githubusercontent.com/mattermost/desktop/master/resources/icon.ico',
+    //loadingGif: path.join(rootPath, 'assets', 'img', 'loading.gif'),


It would be nice if you could get somebody to provide a now installing splash screen. See https://github.com/electron/electron-api-demos/blob/master/assets/img/loading.gif as an example

razzeee · 2016-05-19T21:33:19Z

Is it correct that this PR is against master? Or should it be dev?

Finally got it to work on windows, haven't tested any other OS. Implementing auto update should be trivial when we get a server. This work is heavily based on: https://github.com/electron/electron-api-demos

You might want to review this with whitespaces off:
https://github.com/mattermost/desktop/pull/138/files?w=0

yuya-oc · 2016-05-20T15:49:05Z

src/main.js

+}
+
+// Handle Squirrel on Windows startup events
+switch (process.argv[1]) {


electron-winstaller recommends that the event handler should be placed at the top of main.js. https://github.com/electron/windows-installer#handling-squirrel-events

Then, please return after app.quit() to suppress the other events of app.

Did you check mongodb-js/electron-squirrel-startup? It's introduced on README.md of electron-winstaller.

Well as everything above is a function, this will be called as the first thing. But if you prefere, I can move it to the top, just let me know.

Edit: I'll have a look at mongodb-js/electron-squirrel-startup later

Please move to the top.

Sure, this part will be called at the first, but events are already registered into app module. So I'm afraid of that some issues happen by depending its timing or other reason. The top of main.js looks proper to avoid any other processing.

yuya-oc · 2016-05-20T16:06:48Z

Is it correct that this PR is against master? Or should it be dev?

Good question. master is okay.

JFYI, now there are two active branches, master and dev. They almost follows git-flow. However recently I feel that it's not simple for pull-requests. So the branching model will be a coming issue.

razzeee · 2016-05-20T16:08:40Z

@yuya-oc well you might want to update contributing.md to help with the branch choice

razzeee · 2016-05-20T21:00:27Z

Updated to use mongodb-js/electron-squirrel-startup
Fixed indent
Only build installer from 64bit
Small update for the copyright year

yuya-oc · 2016-05-21T10:39:16Z

package.json

    "electron-connect": "^0.3.7",
    "electron-packager": "^7.0.1",
    "electron-prebuilt": "0.37.8",
+    "electron-squirrel-startup": "^1.0.0",


This must be installed with src/package.json to bundle.

yuya-oc · 2016-05-21T10:43:17Z

Just curious, what does your "works fine" mean? I have never seen any shortcuts, is it correct?

Environment:
Windows 10 x64
Node v4.2.2, npm 2.14.7 (maybe this is not matter)

razzeee · 2016-05-21T12:40:33Z

Shortcuts work fine for me, but its under the Author name so in the folder Yuya Ochiai.

Enviroment is Win10 x64

yuya-oc · 2016-05-21T14:02:02Z

I had overlooked the folder, thank you!

razzeee reviewed May 13, 2016
View reviewed changes

yuya-oc added Type/Enhancement Windows labels May 14, 2016

yuya-oc added this to the v1.3.0 milestone May 14, 2016

yuya-oc reviewed May 14, 2016
View reviewed changes

razzeee force-pushed the create-installers branch from 4ad0889 to 62ec718 Compare May 14, 2016 08:29

razzeee force-pushed the create-installers branch from 62ec718 to be7cdb1 Compare May 19, 2016 21:24

razzeee reviewed May 19, 2016
View reviewed changes

razzeee force-pushed the create-installers branch 2 times, most recently from e553610 to 1829847 Compare May 19, 2016 22:58

yuya-oc reviewed May 20, 2016
View reviewed changes

razzeee added 2 commits May 20, 2016 22:58

Implement installers via electron-builder

68b54ee

Extend copyright to this year

6431423

razzeee force-pushed the create-installers branch from 1829847 to 6431423 Compare May 20, 2016 20:59

yuya-oc reviewed May 21, 2016
View reviewed changes

yuya-oc merged commit 8790dec into mattermost:master May 21, 2016

yuya-oc mentioned this pull request May 21, 2016

Introduce windows installer into CI process #146

Closed

razzeee deleted the create-installers branch May 21, 2016 14:38

razzeee added a commit to razzeee/desktop that referenced this pull request Jun 11, 2016

Change package entry for electron-builder. See mattermost#138 (comment)

f82a64f

razzeee mentioned this pull request Jun 11, 2016

Change package entry for electron-builder. #165

Merged

razzeee added a commit to razzeee/desktop that referenced this pull request Jun 12, 2016

Change package entry for electron-builder. See mattermost#138 (comment)

9428582

razzeee added a commit to razzeee/desktop that referenced this pull request Jun 12, 2016

Change package entry for electron-builder. See mattermost#138 (comment)

3ad2386

razzeee added a commit to razzeee/desktop that referenced this pull request Jun 12, 2016

Change package entry for electron-builder. See mattermost#138 (comment)

fc40305

razzeee added a commit to razzeee/desktop that referenced this pull request Jun 12, 2016

Change package entry for electron-builder. See mattermost#138 (comment)

02c2fc1

razzeee added a commit to razzeee/desktop that referenced this pull request Jun 12, 2016

Change package entry for electron-builder. See mattermost#138 (comment)

87da29d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement installers via electron-builder #138

Implement installers via electron-builder #138

razzeee commented May 13, 2016

razzeee May 13, 2016

yuya-oc May 14, 2016

it33 May 19, 2016

razzeee May 19, 2016

yuya-oc May 14, 2016

yuya-oc commented May 14, 2016 •

edited

Loading

razzeee commented May 14, 2016 •

edited

Loading

razzeee commented May 14, 2016

razzeee commented May 14, 2016

yuya-oc commented May 14, 2016 •

edited

Loading

razzeee May 19, 2016

razzeee commented May 19, 2016

yuya-oc May 20, 2016

yuya-oc May 20, 2016

razzeee May 20, 2016 •

edited

Loading

yuya-oc May 20, 2016

yuya-oc commented May 20, 2016

razzeee commented May 20, 2016

razzeee commented May 20, 2016

yuya-oc May 21, 2016

yuya-oc commented May 21, 2016

razzeee commented May 21, 2016

yuya-oc commented May 21, 2016

Implement installers via electron-builder #138

Implement installers via electron-builder #138

Conversation

razzeee commented May 13, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuya-oc commented May 14, 2016 • edited Loading

razzeee commented May 14, 2016 • edited Loading

razzeee commented May 14, 2016

razzeee commented May 14, 2016

yuya-oc commented May 14, 2016 • edited Loading

Choose a reason for hiding this comment

razzeee commented May 19, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

razzeee May 20, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuya-oc commented May 20, 2016

razzeee commented May 20, 2016

razzeee commented May 20, 2016

Choose a reason for hiding this comment

yuya-oc commented May 21, 2016

razzeee commented May 21, 2016

yuya-oc commented May 21, 2016

yuya-oc commented May 14, 2016 •

edited

Loading

razzeee commented May 14, 2016 •

edited

Loading

yuya-oc commented May 14, 2016 •

edited

Loading

razzeee May 20, 2016 •

edited

Loading