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

App Updater #546

Closed
wants to merge 16 commits into from
Closed

App Updater #546

wants to merge 16 commits into from

Conversation

halgari
Copy link
Collaborator

@halgari halgari commented Aug 10, 2023

First steps of the app updater, disabled in the code, but the main parts can be run from the CLI and the unit tests can be run. Description of the service can be found here: https://github.com/Nexus-Mods/NexusMods.App/pull/546/files?short_path=d82896c#diff-d82896c18eb5f8f050e974d17f1209f8e859c7b6a069ab38b0e9ee1513b88ba7

After this is in, I'll release a v0.2.1 version of the app with this included, and then we can test the end-to-end process of updating.

halgari and others added 11 commits August 8, 2023 06:27
* Cleanup FOMOD

* Add dummy implementations

* Cleanup
)

* Fix cli verbs ignoring the passed modName.
Fix NxmDownloadProtocolHandler ignoring passed modName.

# Conflicts:
#	src/NexusMods.CLI/Verbs/DownloadAndInstallMod.cs

* Add final check for ModManagementVerb tests

* Make AVerbTest public so it can be used in other tests

* Add SkyrimLegendary edition end to end tests for milestone 1.
- Installs SKSE, Skyui, USLEP and check if everything went well.

* Add milestone one tests for SkrimSpecialEdition
- Install SKSE64, Skyui, USSEP and check if everything went well
- Some code formatting

* Fix build

* Use const
@erri120 erri120 marked this pull request as draft August 10, 2023 17:12
@github-actions
Copy link
Contributor

This PR conflicts with main. You need to rebase the PR before it can be merged.

@github-actions
Copy link
Contributor

This PR doesn't conflict with main anymore. It can be merged after all status checks have passed and it has been reviewed.

@halgari halgari requested a review from a team August 15, 2023 18:54
@halgari halgari self-assigned this Aug 15, 2023
@halgari halgari added this to the v0.2 milestone Aug 15, 2023
@erri120
Copy link
Member

erri120 commented Aug 16, 2023

Some things to note: the updater should fetch the correct release asset, depending on the InstallationMethod. The update process will also be different depending on the method and the OS. There should also be an "opt-out" compile constant that completely disables the automatic update checking, since we might publish the App on package systems that do updating for us (winget, choco, AUR).

@github-actions
Copy link
Contributor

This PR conflicts with main. You need to rebase the PR before it can be merged.

@Sewer56
Copy link
Member

Sewer56 commented Aug 17, 2023

I had a look over the code; I have nothing else to add other than what @erri120 already said above.
i.e. In some situations, the App might end up shipping in external package systems.

Edit:

More specifically: Some packaging systems for applications (e.g. AppImage) treat the application itself as read-only, and would therefore not be able to utilise the updater.

@halgari
Copy link
Collaborator Author

halgari commented Aug 21, 2023

Some things to note: the updater should fetch the correct release asset, depending on the InstallationMethod.

Why would we want inno setup involved on updates just because the original version was installed that way? I don't know enough about Flatpack to comment on that, but with Windows I don't think we want to download and run inno setup for updates.

@halgari
Copy link
Collaborator Author

halgari commented Sep 7, 2023

After reviewing this with the team, we're going to go down a different route of showing a dialog and instructing users where to go to update the app themselves

@halgari halgari closed this Sep 7, 2023
@halgari halgari deleted the app-updater-2 branch February 26, 2024 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants