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

Change to 7-zip self extracting archive #103

Closed
wants to merge 2 commits into from
Closed

Change to 7-zip self extracting archive #103

wants to merge 2 commits into from

Conversation

bepvte
Copy link

@bepvte bepvte commented Apr 16, 2021

This pull request changes the windows self extraction method from NSIS to 7-zip self extracting archive.
It adds some additional external dependencies to the build process, but they are very lightweight.
I also updated dependencies to the latest security patch version.

@bepvte bepvte changed the title This pull request changes the windows self extraction method from NSIS to 7-zip self extracting archive. Change to 7-zip self extracting archive Apr 16, 2021
@zerebos
Copy link
Member

zerebos commented Apr 16, 2021

7-zip version can be done automatically by setting this line to false https://github.com/BetterDiscord/Installer/blob/main/package.json#L55 however as per my conversation with Microsoft, that version is more likely to trigger virus flags for whatever reason.

I've uploaded a version of the 7z build for you to checkout on my server https://bd.zerebos.com/BetterDiscord-Windows.exe

@bepvte
Copy link
Author

bepvte commented Apr 16, 2021

I forced pushed this before I saw your comment, just updating the readme a bit better. Its unfortunate that this version would trigger antivirus more frequently apparently. The one you provided still uses the NSIS install framework, which is what I suspect was causing the issues. My version uses only 7-zip and no NSIS script or anything like that.

@bepvte
Copy link
Author

bepvte commented Apr 16, 2021

The main reason I use 7-zip is to get a self extracting portable archive that runs a file from inside it easily, because some users have issues with the NSIS based self extracting archive. I didn't choose 7-zip because of better compression or anything like that. I'm not sure how to fix the antivirus issue, but if the EXE changes anyway between installer releases, I feel like the antivirus problem would be the same for both NSIS and 7-Zip.

@zerebos
Copy link
Member

zerebos commented Apr 16, 2021

The one you provided still uses the NSIS install framework, which is what I suspect was causing the issues.

Well I'd want more than a suspicion before I consider a change like this 😓

@zerebos
Copy link
Member

zerebos commented Apr 16, 2021

It is also of general note that this change would also seem to introduce:

  • Requiring administrative permissions (prompted by system)
  • Preload screen showing Extracting... which can be unclear and confusing from a UX perspective

@bepvte
Copy link
Author

bepvte commented Apr 16, 2021

Well, if it helps, providing people who cannot open the installer my version of the .exe seems to fix their issues. I'm not exactly sure what happens with the current installer to prevent it from working correctly, but I strongly suspect its to do with self extraction, as my version (which works for them) is basically identical except for the self-extraction process.

I think a much easier and less intrusive change to consider would be upgrading electron-builder. Its several versions out of date, and the updates change the NSIS version. I will test if this solves the extraction issue for users.

@bepvte
Copy link
Author

bepvte commented Apr 16, 2021

It seems that upgrading electron-builder does not fix the issue for those affected. If you'd like, I can try adding some commands that will prevent the installer from seeking administrator, or we can just continue with my somewhat fixed version being in support channel pins for people who the released version doesn't work for+close this pull request.

@ObserverOfTime
Copy link

ObserverOfTime commented Apr 23, 2021

Instead of complicating the build process by running a batch file which relies on user-installed files that may not even be in the locations it expects, you could use @erhhung/node-resource-hacker & 7zip-bin. By moving all the code to JavaScript, the contents of 7z_config.txt can also be set as a string to avoid an extra read.

@bepvte
Copy link
Author

bepvte commented Apr 23, 2021

These are very useful, but I don't think we are moving forward with switching to another packaging method anyway. Additionally, I would still need the LZMA SDK in the correct location for it to work with those.

@bepvte bepvte closed this Apr 23, 2021
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.

3 participants