-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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 support for differential zip updates on macOS #7709
feat: add support for differential zip updates on macOS #7709
Conversation
🦋 Changeset detectedLatest commit: cb40ebe The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Thanks for contributing! Please see build error:
|
Fixed |
@mmaietta I will fix these unit tests, but it might take some time as I've been quite busy lately. |
Sounds great @beyondkmp! Really appreciate you picking this up! |
d1ff531
to
bd97680
Compare
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
Alright, I got the differential test suite fixed. It was an issue with circular references/imports and then I had to fix the windows+linux tests themselves w/ a small refactor. Just updated your PR with latest |
Tests are passing but there's an error when I tested it out using a localhost server
|
I have moved CURRENT_MAC_APP_ZIP_FILE_NAME into MacUpdater.ts to fix it. You can try it in latest commit. |
Testing this further, I can't get the differential download to work correctly, I keep receiving a checksum mismatch for some reason.
Are you able to get the differential download working? You can use a |
@mmaietta I can. Here's logs
The error you encountered is due to updater.zip not being the current version. The reason for this is as follows: the initial download for the update was successful, and it was copied to the cache directory. However, if you did not exit the installation of the new version, and the server subsequently updated to another version, attempting to download the incremental update would result in this error because the original updater.zip no longer matches. |
…t, in which case differential download cannot proceed, so we avoid a confusing error message with this check.
Nice work! I've added an additional check for when the update file hasn't existed yet due to app's first install/update. It also includes the override
|
Tests are passing. @beyondkmp I think all necessary steps for implementation are complete and ready for merge. Can you please confirm when you have a chance? |
Thanks for fixing the UT, I have no issues now. You can help merge it. |
Thanks for your contribution! |
@mmaietta @beyondkmp I was testing this within my application this morning - seems as if the first time it can't differentially download it never actually falls back to download the full bundle which is a little concerning... Happy to help diagnose over discord if needed (thatkawaiisam) |
That's seriously concerning. Will revert this PR and debug after |
Took another go and it and this should fix it: #8093 |
Fix #7547