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

[TASK] Rewrite Windows installer from scratch, fixes #1840 #1880

Merged
merged 1 commit into from Nov 5, 2019

Conversation

gilbertsoft
Copy link
Member

@gilbertsoft gilbertsoft commented Oct 10, 2019

The Problem/Issue/Bug:

How this PR Solves The Problem:

[FEATURE] Rewrite Windows installer from scratch

This patch introduces the following new features:

  • include plugin EnVar for better path handling
  • create unicode installer by default
  • use SOLID lzma compression
  • introduce installation types Full, Simple and Minimal
  • use of DDEV branding images
  • show license pages of selected components only
  • use start menu selection page
  • provide option to run mkcert -install for attended install
  • allow downgrade of DDEV
  • copy license files for all installed components
  • power off DDEV before update
  • clean up multiple path entries in the registry
  • copy icons for nice start menu links
  • write various link helpers for start menu short cuts to Links directory
  • create various start menu short cuts
  • introduce section groups for a better overview
  • create mkcert install and uninstall links and start menu short cuts
  • remember last install directory for updates
  • clean up mistakenly created keys in the registry
  • add links and icon to the uninstaller entry
  • log external calls to the install files window
  • improve 64 bit OS support and abort on 32 bit
  • provide option to download and install Docker Desktop
  • provide option to open the Docker Toolbox download page
  • provide option to run mkcert -uninstall in uninstaller
  • cleanly remove installed files
  • license pages will be shown on first install only

And some other goodies simplifying development on the installer:

  • allow manual compilation e.g. from an IDE
  • short description of how to do common changes to the installer
  • inline documentation
  • test project to test libraries
  • docker support can be disabled by option /DDOCKER_EXCLUDE

The new graphics can be found in winpkg/graphics and their sources (psd) in winpkg/graphics/sources.

Additional plugins are stored in winpkg/plugins.

Manual Testing Instructions:

  • multiple path entries in current user are removed after install
  • install directory is added to system path
  • installation type Full selects all components
  • installation type Simple selects all components except WinNFSd group
  • installation type Minimal selects DDEV components only
  • DDEV branding images are shown on each page
  • license page of sudo is shown only if sudo is selected
  • license page of mkcert is shown only if mkcert is selected
  • license page of WinNFSd is shown only if WinNFSd is selected
  • start menu folder can be changed
  • start menu creation can be disabled
  • selecting mkcert -install for attended install sets up mkcert correctly
  • allow downgrade of DDEV
  • license files for all installed components are present
  • power off DDEV after install
  • icons for start menu are shown
  • various link helpers for start menu short cuts are written to Links directory
  • start menu short cuts are created in start menu
  • mkcert install and uninstall links and start menu short cuts are created
  • use last install directory for updates
  • clean up mistakenly created keys in the registry
  • HKLM "SOFTWARE\NSIS_ddev" is deleted after install
  • HKLM "Software\Microsoft\Windows\CurrentVersion\Uninstall\ddev" is deleted after install
  • uninstaller is shown in control panel app with icon and links
  • abort on 32 bit OS
  • option to download Docker Desktop is shown on start if not found
  • option to go to the download page of Docker Toolbox is shown on start if not found and Docker Desktop not supported
  • option to run mkcert -uninstall is shown in uninstaller and executed if mkcert is present
  • all files and DDEV folder is removed after uninstall

Automated Testing Overview:

Related Issue Link(s):

#1840

Release/Deployment notes:

@rfay
Copy link
Member

rfay commented Oct 10, 2019

Thanks! I'll wait until you say to try it out. However, an official installer will be built in the artifacts build, I'll try to post a link to that when it runs.

@rfay
Copy link
Member

rfay commented Oct 10, 2019

@rfay
Copy link
Member

rfay commented Oct 10, 2019

Oh, I wanted to make sure that the full wish list for the installer is visible, please edit with your wishlist:

@gilbertsoft
Copy link
Member Author

Build failure is at https://circleci.com/gh/drud/ddev/18463?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-checks-link

Yeah, has to fail currently because it's WIP. First running build without the Add to Path will be available soon....

@gilbertsoft
Copy link
Member Author

gilbertsoft commented Oct 11, 2019

Currently planed links with names:

!define PRODUCT_NAME "DDEV"
!define PRODUCT_NAME_FULL "${PRODUCT_NAME} Local"
!define PRODUCT_VERSION "${VERSION}"

!define PRODUCT_WEB_SITE "${PRODUCT_NAME} Website"
!define PRODUCT_WEB_SITE_URL "https://www.ddev.com"
!define PRODUCT_DOCUMENTATION "${PRODUCT_NAME} Documentation"
!define PRODUCT_DOCUMENTATION_URL "https://ddev.readthedocs.io"
!define PRODUCT_RELEASE_NOTES "${PRODUCT_NAME} Release Notes"
!define PRODUCT_RELEASE_NOTES_URL "https://github.com/drud/ddev/releases/tag/${PRODUCT_VERSION}"
!define PRODUCT_ISSUES "${PRODUCT_NAME} Issues"
!define PRODUCT_ISSUES_URL "https://github.com/drud/ddev/issues"
!define PRODUCT_CODE "${PRODUCT_NAME} Source Code"
!define PRODUCT_CODE_URL "https://github.com/drud/ddev"
!define PRODUCT_CONTRIBUTING "Contributing to ${PRODUCT_NAME_FULL}"
!define PRODUCT_CONTRIBUTING_URL "https://github.com/drud/ddev/blob/master/CONTRIBUTING.md#contributing"
!define PRODUCT_AWESOME_DDEV "${PRODUCT_NAME} Tips & Tricks"
!define PRODUCT_AWESOME_DDEV_URL "https://github.com/drud/awesome-ddev"
!define PRODUCT_DDEV_CONTRIB "${PRODUCT_NAME} Services & Snippets"
!define PRODUCT_DDEV_CONTRIB_URL "https://github.com/drud/ddev-contrib"
!define PRODUCT_COMMUNITY "${PRODUCT_NAME} Community"
!define PRODUCT_COMMUNITY_URL "https://github.com/drud/community"

@gilbertsoft
Copy link
Member Author

Current build should success and is a preview of the start menu link creation. Will continue later...

@gilbertsoft
Copy link
Member Author

@rfay I added a task list to the initial description. Feel free to extend (don't know if this is possible, else post me a comment)

@gilbertsoft
Copy link
Member Author

@rfay do you know why this fails?
Error while loading icon from "/usr/share/nsis\Contrib\Graphics\Icons\nsis3-install.ico": can't open file
Do I have to use slahes instead of back slashes or isn't the file really available? On Windows it exists and I was able to compile without errors and warnings :(

@rfay
Copy link
Member

rfay commented Oct 11, 2019

IIRC nsis wants all backslash-delimited paths. Even on a linux build. And even though Windows code is happy to accept forward slashes.

@gilbertsoft gilbertsoft force-pushed the new-win-installer branch 2 times, most recently from 6748463 to 85d7c59 Compare October 11, 2019 23:05
@rfay rfay changed the title [TASK] Rewrite Windows installer from the scratch [TASK] Rewrite Windows installer from scratch Oct 12, 2019
@rfay
Copy link
Member

rfay commented Oct 12, 2019

Wow, is that awesome or what?

Annotation 2019-10-11 185155-3
Annotation 2019-10-11 185155-2
Annotation 2019-10-11 185155

@rfay
Copy link
Member

rfay commented Oct 12, 2019

I have only the most minor of nits to report. (I haven't actually tested the results, just ran through the installer)

  • It worries me a little bit changing from C:\Program Files\ddev to C:\Program Files\DDEV. I know I haven't ever seen a case-sensitive Windows machine, but as an old Linuxer it scares me. (DDEV-Local is in fact the official branding, as I'm sure you know).
  • The automatic mkcert at the end... Will that break choco? It often requires privileges. Of course choco is run with privs... But when run manually people might be surprised by an escalation.

@gilbertsoft
Copy link
Member Author

Yeah, it will become great imho :) I'm still optimizing, next build includes the logic to show only licenses for tools which are selected for install. I will tell you when everything should work but currently there are still some things to do... Will continue later...

@gilbertsoft
Copy link
Member Author

gilbertsoft commented Oct 12, 2019

I have only the most minor of nits to report. (I haven't actually tested the results, just ran through the installer)

Yeah, was a branding preview, not fully functional

It worries me a little bit changing from C:\Program Files\ddev to C:\Program Files\DDEV. I know I haven't ever seen a case-sensitive Windows machine, but as an old Linuxer it scares me. (DDEV-Local is in fact the official branding, as I'm sure you know).

imho this should work fine on Windows but will think about it. I tried to streamline everything and also using the same official naming ofc... Chaning the install folder to DDEV-Local is not possible because it will break the updates

The automatic mkcert at the end... Will that break choco? It often requires privileges. Of course choco is run with privs... But when run manually people might be surprised by an escalation.

I thought about this too but have to test. If it's breaking will come up with an alternative solution ofc

@rfay
Copy link
Member

rfay commented Oct 27, 2019

I took the liberty of pushing the (successful) experiment of shortening the submenu in 6ac96e5

I was just going to give a diff, but figured it was better to just push it. If you don't like it you can revert/remove/whatever.

smaller_menu

Copy link
Member

@rfay rfay left a comment

Choose a reason for hiding this comment

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

I read through the changes, and of course I'm not very good with nsis, but it seemed like massive improvement. I made some minor comments that might be worth checking on.

@@ -0,0 +1,84 @@
Name "DDEV Lib Test"
Copy link
Member

Choose a reason for hiding this comment

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

This seems like a nice thing.

I do suspect that this will be bundled into the release build by accident.

Is there a way that we can extend or use this as part of "make test"?

Copy link
Member Author

Choose a reason for hiding this comment

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

I created this to test the two libs and thought I will include it for later manual tests after changes in the libs. For the normal build process this it not needed ofc.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's currently not possible to use this test automated, you have to compile the installer and run it. Then verify the output of the installer...

Copy link
Member

Choose a reason for hiding this comment

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

We could do that on the Windows test runners. It has potential for the future.

winpkg/ddev.nsi Show resolved Hide resolved
winpkg/include/ddev.nsh Show resolved Hide resolved
winpkg/include/ddev.nsh Show resolved Hide resolved
@rfay
Copy link
Member

rfay commented Oct 28, 2019

I had to rebase this to fix timezone-related test failure. So you'll need to reset your fork.

@rfay
Copy link
Member

rfay commented Oct 28, 2019

Copy link
Member Author

@gilbertsoft gilbertsoft left a comment

Choose a reason for hiding this comment

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

I had to rebase this to fix timezone-related test failure. So you'll need to reset your fork.

Didn't see that, sorry. Pushed some changes again, possible issues on updates are solved now, means the install directory page and start menu page are not shown in update mode. If the user likes to change e.g. the install directory he has to uninstall first and run then the installer

I also didn't see your changes before, have to merge them and push it again...

@gilbertsoft
Copy link
Member Author

I happen to notice that the uninstaller doesn't seem to be signed. Sad. I'll take a look.

Added a new issue for that, we don't need to worry about it here: #1898

Will have a look at this
https://nsis.sourceforge.io/Signing_an_Uninstaller

@gilbertsoft
Copy link
Member Author

I happened to notice that these links got added to the "Recently added" section, which probably isn't the absolutely best-case scenario. However, I'm OK with it if you can live with it:

Annotation 2019-10-25 165659

I don't think we can change this behavior, this does Windows automatically

@gilbertsoft
Copy link
Member Author

I've seen this scenario a number of times, probably including with old installer, but this just happened to me getting ready to test, doing the uninstall. The directory is empty though, so all the things we needed to uninstall are gone. Edit: It's because nfsd is still running. Not sure if you want to fiddle with that or not. I'm ok with this result if you're OK with it.

Annotation 2019-10-25 165659

Yes, if something is still running it will fail to remove the directory. We can also omit the message but I think it's important for the user to see something went wrong.

@gilbertsoft
Copy link
Member Author

I'm fine with reducing the number of items in that folder. But I experimented with it (manually) and it didn't make any difference.

That's probably because you didn't uninstall before?

@gilbertsoft
Copy link
Member Author

I tested on both Win10 Home/Docker Toolbox and Win10 Pro/Docker Desktop, and both seemed quite robust and well done.

I do think we have to show them the uninstaller, so that's the only blocker from my end.

Ofc this is possible but I don't think this is needed. Nowadays uninstaller links aren't created in the start menu. You can right click to the start menu Windows icon on the bottom left and choose Apps and Features on the top to see all installed products and their uninstaller

@gilbertsoft
Copy link
Member Author

gilbertsoft commented Oct 30, 2019

Should be updated now and you can rebase it again @rfay and we can test this version again before releasing an alpha...

@rfay
Copy link
Member

rfay commented Oct 31, 2019

Please rebase against upstream/master so it can pass tests, thanks.

@gilbertsoft
Copy link
Member Author

Please rebase against upstream/master so it can pass tests, thanks.

Done

@gilbertsoft
Copy link
Member Author

Unfortunately, the uninstall is once again not added to the list (and doesn't show up on recently added). This one is important. I checked and the uninstall is in fact in /c/Program Files/DDEV

FYI I get the same result on two different machines, Win10 Pro and Win10 Home. My bet is the reason you don't see this behavior is maybe you have a monitor bigger than 13" ?

Is this solved now by removing the two links to GH?

@rfay
Copy link
Member

rfay commented Nov 2, 2019

Unfortunately there's a serious regression in this one. The default path for installation is no longer set.

nodefaultfolder

This patch introduces the following new features:
- include plugin EnVar for better path handling
- create unicode installer by default
- use SOLID lzma compression
- introduce installation types `Full`, `Simple` and `Minimal`
- use of DDEV branding images
- show license pages of selected components only
- use start menu selection page
- provide option to run `mkcert -install` for attended install
- allow downgrade of DDEV
- copy license files for all installed components
- power off DDEV before update
- clean up multiple path entries in the registry
- copy icons for nice start menu links
- write various link helpers for start menu short cuts to Links directory
- create various start menu short cuts
- introduce section groups for a better overview
- create mkcert install and uninstall links and start menu short cuts
- remember last install directory for updates
- clean up mistakenly created keys in the registry
- add links and icon to the uninstaller entry
- log external calls to the install files window
- improve 64 bit OS support and abort on 32 bit
- provide option to download and install Docker Desktop
- provide option to open the Docker Toolbox download page
- provide option to run `mkcert -uninstall` in uninstaller
- cleanly remove installed files
- license pages will be shown on first install only

And some other goodies simplifying development on the installer:
- allow manual compilation e.g. from an IDE
- short description of how to do common changes to the installer
- inline documentation
- test project to test libraries
- docker support can be disabled by option `/DDOCKER_EXCLUDE`

The new graphics can be found in winpkg/graphics and their sources (psd) in winpkg/graphics/sources.

Additional plugins are stored in winpkg/plugins.
@rfay
Copy link
Member

rfay commented Nov 4, 2019

I pushed a release build for final verification, think this can go in if it manually tests OK. The built release is https://19482-80669528-gh.circle-artifacts.com/0/artifacts/ddev_windows_installer.v1.11.3-alpha4.exe for a final run-through.

My own testing of that says it seems to be just fine now, ready for pull and an alpha release.

One note: It's not necessary to combine all your commits into one because the final PR merge will be a squash merge. When you keep just one commit going it's impossible to see what you might have done to fix a problem (like the one you just fixed).

Thanks for all your amazing work on this. I know it's been a long haul, and everybody in the community is going to appreciate it very much.

@rfay rfay changed the title [TASK] Rewrite Windows installer from scratch [TASK] Rewrite Windows installer from scratch, fixes #1840 Nov 5, 2019
@gilbertsoft
Copy link
Member Author

One note: It's not necessary to combine all your commits into one because the final PR merge will be a squash merge. When you keep just one commit going it's impossible to see what you might have done to fix a problem (like the one you just fixed).

You're right, for the lastest commits this would have been better

@gilbertsoft
Copy link
Member Author

I pushed a release build for final verification, think this can go in if it manually tests OK. The built release is https://19482-80669528-gh.circle-artifacts.com/0/artifacts/ddev_windows_installer.v1.11.3-alpha4.exe for a final run-through.

My own testing of that says it seems to be just fine now, ready for pull and an alpha release.

I tested again on various systems: Win 10 & 7, Win 10 32 bit (to see the error message). I did fresh installs and updates, looks everything fine so far.

@gilbertsoft
Copy link
Member Author

Thanks for all your amazing work on this. I know it's been a long haul, and everybody in the community is going to appreciate it very much.

You're welcome! And thanks for DDEV :)

@rfay rfay merged commit 4edb8d0 into ddev:master Nov 5, 2019
rfay pushed a commit to rfay/ddev that referenced this pull request Jan 15, 2020
…ev#1880)

This patch introduces the following new features:
- include plugin EnVar for better path handling
- create unicode installer by default
- use SOLID lzma compression
- introduce installation types `Full`, `Simple` and `Minimal`
- use of DDEV branding images
- show license pages of selected components only
- use start menu selection page
- provide option to run `mkcert -install` for attended install
- allow downgrade of DDEV
- copy license files for all installed components
- power off DDEV before update
- clean up multiple path entries in the registry
- copy icons for nice start menu links
- write various link helpers for start menu short cuts to Links directory
- create various start menu short cuts
- introduce section groups for a better overview
- create mkcert install and uninstall links and start menu short cuts
- remember last install directory for updates
- clean up mistakenly created keys in the registry
- add links and icon to the uninstaller entry
- log external calls to the install files window
- improve 64 bit OS support and abort on 32 bit
- provide option to download and install Docker Desktop
- provide option to open the Docker Toolbox download page
- provide option to run `mkcert -uninstall` in uninstaller
- cleanly remove installed files
- license pages will be shown on first install only

And some other goodies simplifying development on the installer:
- allow manual compilation e.g. from an IDE
- short description of how to do common changes to the installer
- inline documentation
- test project to test libraries
- docker support can be disabled by option `/DDOCKER_EXCLUDE`

The new graphics can be found in winpkg/graphics and their sources (psd) in winpkg/graphics/sources.

Additional plugins are stored in winpkg/plugins.
@gilbertsoft gilbertsoft deleted the new-win-installer branch April 17, 2020 15:33
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