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

Improve Windows NSIS installer script (setup.nsi) #831

Merged
merged 3 commits into from
Mar 25, 2024

Conversation

Pierre4012
Copy link
Contributor

Two improvements of installer script :

  1. avoid error message when Unbound is running,
  2. add "DisplayVersion" in registry thus Windows package manager (Winget) can handle Unbound.

1. Avoid error message :

When Unbound is already installed and running, when the installer is used to update Unbund, it displays an error message :

error

In order to avoid this message, I suggest a modification to stop Unbound service if already installed.

2. Adding "DisplayVersion" in registry :

Winget use uninstall parameters in Windows registry to get program versions to be able to compare installed version and last available version to propose update. Without this parameter, Winget can't handle Unbound updates :

version

This parameter is also used to show program versions in the Windows remove program menu.

I suggest adding this parameter in the installer script.

Two improvements of installer script :
- avoid error message when Unbound is running,
- add "DisplayVersion" in registry thus Windows package manager (Winget) can handle Unbound.

Avoid error message :
When Unbound is already installed and running, when the installer is used to update Unbund, it displays an error message  "Error opening file for writing".
In order to avoid this message, I suggest a modification to stop Unbound service if already installed.

Adding "DisplayVersion" in registry :
Winget use uninstall parameters in Windows registry to get program versions to be able to compare installed version and last available version to propose update. Without this parameter, Winget can't handle Unbound updates. This parameter is also used to show program versions in the Windows remove program menu.
I suggest adding this parameter in the installer script.
@gthess
Copy link
Member

gthess commented Mar 20, 2024

Hi, I like change no2 but not no1 :)
I would like for any termination of the already running service to be explicit and not because of the installer. Actually I would move unbound.exe up as to be the first file to be created, to avoid LICENSE and README.txt to be copied.

@gthess gthess self-assigned this Mar 20, 2024
Ask user to stop Unbound to permit the update
@Pierre4012
Copy link
Contributor Author

Pierre4012 commented Mar 25, 2024

Hi, I like change no2 but not no1 :) I would like for any termination of the already running service to be explicit and not because of the installer. Actually I would move unbound.exe up as to be the first file to be created, to avoid LICENSE and README.txt to be copied.

For no1 I can suggest adding a messagebox to ask user :

Pierre4012@c66659f

@gthess
Copy link
Member

gthess commented Mar 25, 2024

I like it more, but maybe instead of Abort just Quit?
Now the installer window will just hang there with the aborted message and the only action is to "cancel" the installation?
Then maybe the message could be something like "Unbound is already installed! Would you like to stop the service to continue with the installation?"

@Pierre4012
Copy link
Contributor Author

Yes, simply Quit is much more adapted, have modified it.
I added your message proposal for MessageBox specifying "update".

Pierre4012@a16aca1

Copy link
Member

@gthess gthess left a comment

Choose a reason for hiding this comment

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

Thanks for this!

@gthess gthess merged commit ef60dca into NLnetLabs:master Mar 25, 2024
1 check passed
gthess added a commit that referenced this pull request Mar 25, 2024
- Merge #831 from Pierre4012: Improve Windows NSIS installer
  script (setup.nsi).
@Pierre4012 Pierre4012 deleted the patch-2 branch March 25, 2024 20:08
gthess added a commit that referenced this pull request Mar 25, 2024
jedisct1 added a commit to jedisct1/unbound that referenced this pull request Apr 4, 2024
* nlnet/master: (24 commits)
  - Fix NLnetLabs#369: dnstap showing extra responses; for client responses   right from the cache when replying with expired data or   prefetching.
  - Fix NLnetLabs#1035: Potential Bug while parsing port from the "stub-host"   string; also affected forward-zones and remote-control host   directives.
  - For NLnetLabs#1040: adjust error text and disallow negative ports in other   parts of cfg_mark_ports.
  Changelog note for NLnetLabs#1040 - Fix NLnetLabs#1040: fix heap-buffer-overflow issue in function cfg_mark_ports   of file util/config_file.c.
  fix heap-buffer-overflow issue in function cfg_mark_ports of file util/config_file.c
  - Fix for crypto related failures to have a better error string.
  - Fix NLnetLabs#1034: DoT forward-zone via unbound-control.
  - Fix that the server does not chown the pidfile.
  - Fix that when the server truncates the pidfile, it does not follow   symbolic links.
  - Fix to add unit test for lruhash space that exercises the routines.
  - Fix comment in lruhash space function.
  - Fix for NLnetLabs#1032, add safeguard to make table space positive.
  - Fix NLnetLabs#1032: The size of subnet_msg_cache calculation mistake cause   memory usage increased beyond expectations.
  - Fix name of unit test for subnet cache response.
  - For NLnetLabs#831: Format text, use exclamation icon and explicit label   names.
  Changelog entry for NLnetLabs#831 - Merge NLnetLabs#831 from Pierre4012: Improve Windows NSIS installer   script (setup.nsi).
  Improve Windows NSIS installer script (setup.nsi) (NLnetLabs#831)
  - Fix localdata and rpz localdata to match CNAME only if no direct   type match is available.
  - Fix rpz so that rpz CNAME can apply after rpz CNAME. And fix that   clientip and nsip can give a CNAME.
  - Fix rpz for qtype CNAME after nameserver trigger.
  ...
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.

None yet

2 participants