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

fix windows autoupdate #744

Merged
merged 9 commits into from
May 5, 2020
Merged

fix windows autoupdate #744

merged 9 commits into from
May 5, 2020

Conversation

mason-fish
Copy link
Contributor

@mason-fish mason-fish commented May 5, 2020

fixes #552

After digging through a fair amount of (un)documentation, the fix here has a few pieces but they are all small:

  • First off, there is a bizarre and inconsistent issue where if we try to run the updater immediately on startup then there can be an issue with multiple processes racing for resources. I only saw this once but since I added the 30s delay I never saw it again.

  • Second, the path.join works great for cross platform filepaths, but is not designed for url construction.

  • Third, in addition to the *.exe file, we also need to begin publishing the RELEASES and *.nupkg files which are already being generated by winstaller.

@mason-fish mason-fish requested review from jameskerr and a team May 5, 2020 16:11
Copy link
Contributor

@alfred-landrum alfred-landrum left a comment

Choose a reason for hiding this comment

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

🎉 . @mason-fish : can you clarify how we should describe this for release notes? Am I correct to say that once a user installs a release with this change (which will likley be v0.9.0), then auto-update should work going forward, correct? But users that have an already released Brim will need to install v0.9.0 manually.

fyi @philrz

@mason-fish
Copy link
Contributor Author

mason-fish commented May 5, 2020

@alfred-landrum Yes that's correct. So we can expect the first wave of windows autoupdates to trigger when we release v0.10.0, assuming this PR goes into v0.9.0.

@jameskerr
Copy link
Member

If you rebase you won't run into that format error anymore.

Mason Fish added 9 commits May 5, 2020 12:00
Signed-off-by: Mason Fish <mason@looky.cloud>
Signed-off-by: Mason Fish <mason@looky.cloud>
Signed-off-by: Mason Fish <mason@looky.cloud>
Signed-off-by: Mason Fish <mason@looky.cloud>
Signed-off-by: Mason Fish <mason@looky.cloud>
Signed-off-by: Mason Fish <mason@looky.cloud>
Signed-off-by: Mason Fish <mason@looky.cloud>
Signed-off-by: Mason Fish <mason@looky.cloud>
Signed-off-by: Mason Fish <mason@looky.cloud>
@mason-fish mason-fish force-pushed the 552-fix_windows_autoupdate branch from 30f5119 to 5ccf17d Compare May 5, 2020 19:00
@mason-fish mason-fish merged commit d586fc0 into master May 5, 2020
@mason-fish mason-fish deleted the 552-fix_windows_autoupdate branch May 5, 2020 19:16
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.

enable auto-update for windows
3 participants