-
Notifications
You must be signed in to change notification settings - Fork 374
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
WIP - Add installer downloader and in-app upgrades backend #7583
base: main
Are you sure you want to change the base?
Conversation
8bdac28
to
bf3eeb4
Compare
mullvad-update/src/app.rs
Outdated
} | ||
|
||
impl<SigProgress, AppProgress> HttpAppDownloader<SigProgress, AppProgress> { | ||
const MAX_SIGNATURE_SIZE: usize = 1 * 1024; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1*
causes a clippy warning
mullvad-update/src/app.rs
Outdated
#[derive(Debug)] | ||
pub enum DownloadError { | ||
FetchSignature(anyhow::Error), | ||
FetchApp(anyhow::Error), | ||
Verification(anyhow::Error), | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These error variants are never matched on, and the enum doesn't implement the Error trait, so is there really any point to it? Why not just use anyhow
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I get this warning when running cargo check
warning: profiles for the non root package will be ignored, specify profiles at the workspace root:
package: C:\Users\Sebastian\Documents\Programming\mullvad\installer-downloader\Cargo.toml
workspace: C:\Users\Sebastian\Documents\Programming\mullvad\Cargo.toml
Is the opt level specified in installer-downloader
being overwritten by the workspace level?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess convert-assets.py
is not supposed to be tracked in the repository, since we store the converted file, right?
It isn't being correctly applied. This is noted in the toml file. I think it'll be replaced by a build script. |
add_file_server_mock(&mut server, "/my_file", file_data); | ||
|
||
// Interrupt after exactly half the file has been downloaded | ||
let mut limited_buffer = vec![0u8; file_data.len() / 2].into_boxed_slice(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.into_boxed_slice()
and the following into_vec()
aren't needed. The test compiles and passes when I remove them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it work without removing vec!
? I was sure that it would automatically resize the buffer in that case. I'll try it, but somehow I trust it more knowing it's a fixed size.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're still creating the writer from a slice into the vec, so it can't modify the capacity, right?
if active_download.is_some() { | ||
continue; | ||
} | ||
let Some(version_info) = version_info.clone() else { | ||
continue; | ||
}; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we should log some warning here?
I think it should because it makes it easier to regenerate them if the SVGs are edited. |
ec26088
to
15fb2cc
Compare
mullvad-update/src/deserializer.rs
Outdated
pub signed: Response, | ||
} | ||
|
||
/// Helper class that leaves the signed data untouched |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:man-gesturing-no:
/// Helper class that leaves the signed data untouched | |
/// Helper type that leaves the signed data untouched |
/// Beta preface text | ||
pub const BETA_PREFACE_DESC: &str = "Want to try the new Beta version? "; | ||
/// Beta link text | ||
pub const BETA_LINK_TEXT: &str = "Click here!"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the text really up to date with the design? Shouldn't it say "Want to try out new features? Try our new beta!"
Co-authored-by: Sebastian Holmin <[email protected]>
This also replaces the PGP verifier with a SHA256 checksum verifier
`format` was also updated to support signing
This brings down the binary size of installer-downloader from 2.3 M to 1.4 M with size optimizations enabled
ea86df4
to
ce4521b
Compare
Notably, this means that the loader must run as a privileged user
Co-authored-by: Joakim Hulthe <[email protected]>
ce4521b
to
a72844c
Compare
This PR aims to introduce a minimal installer as well as most of the functionality for in-app upgrades (except UI). This is still a work in progress.
The main things that are missing:
The planned native module for the Electron frontend is not in scope for this PR.
Close DES-1685
Close DES-1684
Close DES-1641
Close DES-1640
Close DES-1571
This change is