fix: Add /FORCEREGISTRY flag to windows installer#5517
Conversation
|
I don't have much experience with nsis scripts and maybe there is a better way to do this |
|
💻 Deploy preview deleted (fix: Add /FORCEREGISTRY flag to windows installer). |
Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>
|
Doc changes look OK to me. |
ptodev
left a comment
There was a problem hiding this comment.
This feels like a workaround. Can we just recommend to users to run the uninstaller if they want to reset the installation?
If the uninstaller isn't an option because certain things need to be preserved, then could we just add an installer option to delete all Alloy registry keys prior to the installation?
Yes that's the point. No if we uninstall the data directory will be removed so we lose WAL, position files etc.
Sure that could work but then the updating alloy is two steps no? One to prune registry and then one to update it again or did you have something else in mind? |
|
Would it help to add a little clarification around upgrade behavior? Maybe something like this (just below the install options and above the note)? |
ptodev
left a comment
There was a problem hiding this comment.
we lose WAL, position files etc
By default the "data" files are in a different directory from the one the Alloy executable resides in. I don't know if the uninstaller will remove it. If it doesn't then this could be a problem in itself, since people would expect the uninstaller to remove that too.
Sure that could work but then the updating alloy is two steps no? One to prune registry and then one to update it again or did you have something else in mind?
No, what I mean is that if FORCEREGISTRY is set, then install_script.nsis can delete all the Alloy registry keys at the start of the installation. Then the rest of the installation can proceed as normal. It'd make the script cleaner and easier to understand. I'd also clean up the registry from keys that are old or unused.
|
Sure I guess that could work |
|
@ptodev I change to the suggestion you made, much simpler |
|
TruffleHog Scan Results Summary: Found 11 potential secrets (0 verified, 11 unverified)
Review: Check if unverified secrets are false positives. |
### Pull Request Details When using the windows installer to upgrade alloy it's not possible to update arguments, environment etc if it was previously written. I added a flag that will force set these in windows registry. ### Issue(s) fixed by this Pull Request <!-- Fixes #issue_id --> ### Notes to the Reviewer <!-- Add any relevant notes for the reviewers and testers of this PR. --> ### PR Checklist <!-- Remove items that do not apply. For completed items, change [ ] to [x]. --> - [x] Documentation added - [ ] Tests updated - [ ] Config converters updated --------- Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>
Pull Request Details
When using the windows installer to upgrade alloy it's not possible to update arguments, environment etc if it was previously written. I added a flag that will force set these in windows registry.
Issue(s) fixed by this Pull Request
Notes to the Reviewer
PR Checklist