-
-
Notifications
You must be signed in to change notification settings - Fork 248
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
Update Windows installation script #503
Conversation
|
||
svc.on('alreadyuninstalled', () => { | ||
svc.install(); | ||
}); | ||
|
||
svc.on('uninstall', () => { | ||
svc.install(); | ||
}); | ||
|
||
svc.on('error', () => { | ||
svc.install(); | ||
}); | ||
|
||
svc.on('install', () => { | ||
svc.start(); | ||
console.log("Service installed"); | ||
}); | ||
|
||
svc.uninstall(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found this quite redundant so simplified, not sure if there was a reason for all the other event listeners.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code would trigger an install after uninstalling it, or attempting, first. The intention is to make sure the service script can update itself. I think this portion should be reverted as it allows service changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah ok. There seems to be a bug or something with node-windows
uninstall at the moment that causes an exception coreybutler/node-windows#324 even though the service is successfully uninstalled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah figured out a workaround.
I’m not familiar with chocolates, but seems fine as long as that dependency is soft installed. |
Yeah chocolatey and winget are very similar, chocolatey has a bit more mature. |
Change ready to go then? |
Yep I've tested the latest script using a new Windows Sandbox and it's one click to get the server running now 😇 |
I updated the uninstallation instructions. It doesn't need a full path to the scrypted.exe? |
No because Even though we specify the
|
* Update Windows installation script * Fix sc.exe * Workaround node-windows quirks with uninstall and start
winget
since the Node.js additional tools already use Chocolatey)windows-build-tools
and install the Node.js additional tools for Windows to compile native modules as recommended by https://github.com/felixrieseberg/windows-build-tools using the official script https://github.com/nodejs/node/blob/main/tools/msvs/install_tools/install_tools.bat#L55 fixes Windows installation script windows-build-tools no longer required? #494node-windows
service start not working by adding delay between install and start Service fails to start first time on Windows Server 2016 coreybutler/node-windows#318node-windows
svc.uninstall()
throws exception by adding more delay between uninstall and file delete EPERM error when uninstalling the service coreybutler/node-windows#324After merging, can simplify the installation instructions at https://github.com/koush/scrypted/wiki/Installation:-Windows to remove the dependency on
winget
We can also simplify the uninstallation instructions at https://github.com/koush/scrypted/wiki/Uninstallation#windows to use
sc.exe
directly which avoids the dependency onnode-windows