fix(NSIS): prevent partial overwrites #6547
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
`Nsis7z::Extract` ignores the errors when copying the files and thus can leave us with the app that has old asar and bindings, but new assets. In addition to that, the app will firmly believe that it is still running an old version and would attempt to repeatedly auto-update until fixed, leading to excessive bandwidth use and very unhappy customers. This change extracts the contents of 7z archive into a separate directory before attempting to copy them with `CopyFiles` function that (unlike `Nsis7z::Extract`) does detect and report failures. To make our lives easier the `CopyFiles` will also erase all files on a failure so after retrying a few times we will ultimately have to fallback to old 7z extraction directly into output folder.
🦋 Changeset detectedLatest commit: d7c4edd The changes in this PR will be included in the next version bump. This PR includes changesets to release 8 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
✔️ Deploy Preview for car-park-attendant-cleat-11576 ready! 🔨 Explore the source changes: d7c4edd 🔍 Inspect the deploy log: https://app.netlify.com/sites/car-park-attendant-cleat-11576/deploys/61df2fe2760c6b0008426cfd 😎 Browse the preview: https://deploy-preview-6547--car-park-attendant-cleat-11576.netlify.app |
indutny-signal
commented
Jan 12, 2022
| @@ -90,5 +90,45 @@ | |||
| !macroend | |||
|
|
|||
| !macro extractUsing7za FILE | |||
| Nsis7z::Extract "${FILE}" | |||
| Push $OUTDIR | |||
| CreateDirectory "$PLUGINSDIR\7z-out" | |||
There was a problem hiding this comment.
Note: NSIS promises that $PLUGINSDIR is a temporary folder that is removed after the installation.
|
Yay. Thanks for merging it so quickly! |
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Nsis7z::Extractignores the errors when copying the files and thus canleave us with the app that has old asar and bindings, but new assets. In
addition to that, the app will firmly believe that it is still running
an old version and would attempt to repeatedly auto-update until fixed,
leading to excessive bandwidth use and very unhappy customers.
This change extracts the contents of 7z archive into a separate
directory before attempting to copy them with
CopyFilesfunction that(unlike
Nsis7z::Extract) does detect and report failures. To make ourlives easier the
CopyFileswill also erase all files on a failure soafter retrying a few times we will ultimately have to fallback to old 7z
extraction directly into output folder.
Testing
The easiest way to trigger the failure is to:
tmp.exetmp.exe(thus locking the asar, bindings, and dlls)