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

feat: add preInstall macro for customise NSIS script #5728

Closed
wants to merge 1 commit into from

Conversation

altjz
Copy link

@altjz altjz commented Mar 23, 2021

For now, Custom NSIS install Scripts include customHeaderpreInit, customInit, customInstall.

Some tasks must be executed before install application files.

For example, delete some files those existed in the $INSTDIR. ( Sometime the uninstaller doesn't work if between Applications)

!macro preInstall
  MessageBox MB_OK "preInstall"
  RMDir /r "$INSTDIR\resources\"
  RMDir /r "$INSTDIR\locales\"
  Delete "$INSTDIR\logo.ico"
  Delete "$INSTDIR\uninst.exe"
!macroend

customInstall is too late for the clean job. That would be nice if we could provide the preInstall macro.

@mmaietta
Copy link
Collaborator

Hi @altjz, thanks for the PR!

For example, delete some files those existed in the $INSTDIR. ( Sometime the uninstaller doesn't work if between Applications)

Just curious, Is this the only use-case/bug we're solving for or are there additional cases that you will be using this for?

@altjz
Copy link
Author

altjz commented Mar 25, 2021

Hi @altjz, thanks for the PR!

For example, delete some files those existed in the $INSTDIR. ( Sometime the uninstaller doesn't work if between Applications)

Just curious, Is this the only use-case/bug we're solving for or are there additional cases that you will be using this for?

Many thanks for your reply, I am glad to help.

Some cases I may use it in my application.

  1. Check the files or subdirectory of $INSTDIR, (many primary users will install the application in desktop or some danger area)
  2. Check the running processes, I know electron-builder integrate one macro something like this, but that macro is only for the same Application, If we have some background services, we still need to stop them before the installation.
  3. Check the Registry before the installation, It's useful in some special cases, such as checking the if the program was not uninstalled completely last time. electron builder is able to do this but only for current Application, same GUID, same ProductName.

There are still many cases, The most important function of preInstall macro, I think is that Ensure everything is working fine before the installation, under preInit, customInit section, user still can cancel the installation(OneClick option is false). So I think the best time for these clean job is User clicks the Next button, before copy application files

Copy link
Collaborator

@mmaietta mmaietta 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 the details. Seems quite reasonable to me for this to be added.

Approved. Will leave this Open for a few days to see if any others have feedback to provide.

Thanks for your contribution!

@mmaietta
Copy link
Collaborator

@altjz , just curious, is this PR also solving the same issue for you?
#5712
Seems to solve items 2 and 3 since any ongoing background services or previous files would have required an installation to already exist, potentially due to a failed uninstall/update.

I'm not sure if we need both these PRs if we're solving (one of) the same problems. What are your thoughts?

@stale
Copy link

stale bot commented Jun 4, 2021

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the backlog label Jun 4, 2021
@stale stale bot closed this Jun 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants