diff --git a/library/Cargo.toml b/library/Cargo.toml index 32757e8..cd77b5c 100644 --- a/library/Cargo.toml +++ b/library/Cargo.toml @@ -70,6 +70,6 @@ simple-logging = "2.0.2" serial_test = "2.0.0" tempdir = "0.3.7" -# https://github.com/eqrion/cbindgen/blob/master/docs.md#buildrs +# [build-dependencies] cbindgen = "0.24.0" diff --git a/library/build.rs b/library/build.rs index 5cada53..6596e07 100644 --- a/library/build.rs +++ b/library/build.rs @@ -3,9 +3,9 @@ extern crate cbindgen; use std::env; // See: -// https://github.com/eqrion/cbindgen/blob/master/docs.md#buildrs -// https://doc.rust-lang.org/cargo/reference/build-scripts.html -// https://doc.rust-lang.org/cargo/reference/build-script-examples.html +// +// +// fn main() { let crate_dir = env::var("CARGO_MANIFEST_DIR").unwrap(); @@ -16,7 +16,7 @@ fn main() { contents.write_to_file("include/updater.h"); } Err(e) => { - println!("cargo:warning=Error generating bindings: {}", e); + println!("cargo:warning=Error generating bindings: {e}"); // If we were to exit 1 here we would stop local rust // analysis from working. So we just print the error // and continue. diff --git a/library/include/updater.h b/library/include/updater.h index aad4bf8..7d5af64 100644 --- a/library/include/updater.h +++ b/library/include/updater.h @@ -104,11 +104,11 @@ SHOREBIRD_EXPORT void shorebird_start_update_thread(void); /** * Tell the updater that we're launching from what it told us was the - * next patch to boot from. This will copy the next_boot patch to be the - * current_boot patch. + * next patch to boot from. This will copy the next boot patch to be the + * `current_boot` patch. * * It is required to call this function before calling - * shorebird_report_launch_success or shorebird_report_launch_failure. + * `shorebird_report_launch_success` or `shorebird_report_launch_failure`. */ SHOREBIRD_EXPORT void shorebird_report_launch_start(void); diff --git a/library/src/android.rs b/library/src/android.rs index 6b23ff4..9a5dea4 100644 --- a/library/src/android.rs +++ b/library/src/android.rs @@ -3,7 +3,7 @@ use std::fs; use std::io::{Cursor, Read}; use std::path::{Path, PathBuf}; -// https://stackoverflow.com/questions/67087597/is-it-possible-to-use-rusts-log-info-for-tests +// #[cfg(test)] use std::println as debug; // Workaround to use println! for logs. diff --git a/library/src/c_api.rs b/library/src/c_api.rs index e088ac9..9b7f7d8 100644 --- a/library/src/c_api.rs +++ b/library/src/c_api.rs @@ -2,21 +2,21 @@ // Currently manually prefixing all functions with "shorebird_" to avoid // name collisions with other libraries. -// cbindgen:prefix-with-name could do this for us. +// `cbindgen:prefix-with-name` could do this for us. /// This file contains the C API for the updater library. /// It is intended to be used by language bindings, and is not intended to be /// used directly by Rust code. /// The C API is not stable and may change at any time. /// You can see usage of this API in Shorebird's Flutter engine: -/// https://github.com/shorebirdtech/engine/blob/shorebird/dev/shell/common/shorebird.cc +/// use std::ffi::{CStr, CString}; use std::os::raw::c_char; use std::path::PathBuf; use crate::updater; -// https://stackoverflow.com/questions/67087597/is-it-possible-to-use-rusts-log-info-for-tests +// #[cfg(test)] use std::{println as info, println as error}; // Workaround to use println! for logs. @@ -130,11 +130,7 @@ pub extern "C" fn shorebird_should_auto_update() -> bool { #[no_mangle] pub extern "C" fn shorebird_current_boot_patch_number() -> usize { log_on_error( - || { - Ok(updater::current_boot_patch()? - .map(|p| p.number) - .unwrap_or(0)) - }, + || Ok(updater::current_boot_patch()?.map_or(0, |p| p.number)), "fetching next_boot_patch_number", 0, ) @@ -145,7 +141,7 @@ pub extern "C" fn shorebird_current_boot_patch_number() -> usize { #[no_mangle] pub extern "C" fn shorebird_next_boot_patch_number() -> usize { log_on_error( - || Ok(updater::next_boot_patch()?.map(|p| p.number).unwrap_or(0)), + || Ok(updater::next_boot_patch()?.map_or(0, |p| p.number)), "fetching next_boot_patch_number", 0, ) @@ -210,11 +206,11 @@ pub extern "C" fn shorebird_start_update_thread() { } /// Tell the updater that we're launching from what it told us was the -/// next patch to boot from. This will copy the next_boot patch to be the -/// current_boot patch. +/// next patch to boot from. This will copy the next boot patch to be the +/// `current_boot` patch. /// /// It is required to call this function before calling -/// shorebird_report_launch_success or shorebird_report_launch_failure. +/// `shorebird_report_launch_success` or `shorebird_report_launch_failure`. #[no_mangle] pub extern "C" fn shorebird_report_launch_start() { log_on_error(updater::report_launch_start, "reporting launch start", ()); diff --git a/library/src/cache.rs b/library/src/cache.rs index 889462f..4edec01 100644 --- a/library/src/cache.rs +++ b/library/src/cache.rs @@ -81,7 +81,7 @@ fn generate_client_id() -> String { } impl UpdaterState { - /// Creates a new UpdaterState. If client_id is None, a new one will be generated. + /// Creates a new `UpdaterState`. If `client_id` is None, a new one will be generated. fn new(cache_dir: PathBuf, release_version: String, client_id: Option) -> Self { Self { cache_dir, @@ -97,7 +97,7 @@ impl UpdaterState { } pub fn client_id_or_default(&self) -> String { - self.client_id.clone().unwrap_or("".to_string()) + self.client_id.clone().unwrap_or(String::new()) } pub fn is_known_good_patch(&self, patch_number: usize) -> bool { @@ -349,7 +349,7 @@ impl UpdaterState { self.slots.resize(index + 1, Slot::default()); } // Set the given slot to the given version. - self.slots[index] = slot + self.slots[index] = slot; } fn patch_path_for_index(&self, index: usize) -> PathBuf { @@ -357,10 +357,10 @@ impl UpdaterState { } fn slot_dir_for_index(&self, index: usize) -> PathBuf { - Path::new(&self.cache_dir).join(format!("slot_{}", index)) + Path::new(&self.cache_dir).join(format!("slot_{index}")) } - pub fn install_patch(&mut self, patch: PatchInfo) -> anyhow::Result<()> { + pub fn install_patch(&mut self, patch: &PatchInfo) -> anyhow::Result<()> { let slot_index = self.available_slot(); let slot_dir_string = self.slot_dir_for_index(slot_index); let slot_dir = PathBuf::from(&slot_dir_string); @@ -373,7 +373,7 @@ impl UpdaterState { if self.is_known_bad_patch(patch.number) { return Err(UpdateError::InvalidArgument( "patch".to_owned(), - format!("Refusing to install known bad patch: {:?}", patch), + format!("Refusing to install known bad patch: {patch:?}"), ) .into()); } @@ -394,27 +394,27 @@ impl UpdaterState { if let Some(latest) = self.latest_patch_number() { if patch.number < latest { warn!( - "Installed patch {} but latest downloaded patch is {:?}", - patch.number, latest + "Installed patch {} but latest downloaded patch is {latest:?}", + patch.number ); } } self.save()?; let path = self.patch_path_for_index(slot_index); - if !path.exists() { + if path.exists() { + debug!("Patch {} installed to {:?}", patch.number, path); + } else { warn!( "Patch {} installed but does not exist {:?}", patch.number, path ); - } else { - debug!("Patch {} installed to {:?}", patch.number, path); } Ok(()) } - /// Sets the current_boot slot to the next_boot slot. + /// Sets the `current_boot` slot to the `next_boot` slot. pub fn activate_current_patch(&mut self) -> Result<(), UpdateError> { if self.next_boot_slot_index.is_none() { return Err(UpdateError::InvalidState( @@ -501,11 +501,11 @@ mod tests { let tmp_dir = TempDir::new("example").unwrap(); let mut state = test_state(&tmp_dir); assert_eq!(state.latest_patch_number(), None); - state.install_patch(fake_patch(&tmp_dir, 1)).unwrap(); + state.install_patch(&fake_patch(&tmp_dir, 1)).unwrap(); assert_eq!(state.latest_patch_number(), Some(1)); - state.install_patch(fake_patch(&tmp_dir, 2)).unwrap(); + state.install_patch(&fake_patch(&tmp_dir, 2)).unwrap(); assert_eq!(state.latest_patch_number(), Some(2)); - state.install_patch(fake_patch(&tmp_dir, 1)).unwrap(); + state.install_patch(&fake_patch(&tmp_dir, 1)).unwrap(); // This probably should be Some(2) assuming we didn't write // over the top of patch 2 when re-installing patch 1. // I expect if we support rollbacks we might be more explicit @@ -520,7 +520,7 @@ mod tests { let bad_patch = fake_patch(&tmp_dir, 1); state.mark_patch_as_bad(bad_patch.number).unwrap(); let number = bad_patch.number; - assert!(state.install_patch(bad_patch).is_err()); + assert!(state.install_patch(&bad_patch).is_err()); // Calling a second time should not error. state.mark_patch_as_bad(number).unwrap(); @@ -567,8 +567,7 @@ mod tests { let tmp_dir = TempDir::new("example").unwrap(); let state = UpdaterState::load_or_new_on_error(tmp_dir.path(), "1.0.0+1"); assert!(state.client_id.is_some()); - let saved_state = - UpdaterState::load_or_new_on_error(tmp_dir.path(), "1.0.0+1"); + let saved_state = UpdaterState::load_or_new_on_error(tmp_dir.path(), "1.0.0+1"); assert_eq!(state.client_id, saved_state.client_id); } diff --git a/library/src/config.rs b/library/src/config.rs index c426211..2fe7013 100644 --- a/library/src/config.rs +++ b/library/src/config.rs @@ -85,7 +85,7 @@ pub struct UpdateConfig { pub fn set_config( app_config: AppConfig, libapp_path: PathBuf, - yaml: YamlConfig, + yaml: &YamlConfig, network_hooks: NetworkHooks, ) -> anyhow::Result<()> { with_config_mut(|config| { diff --git a/library/src/events.rs b/library/src/events.rs index d539fef..a52c747 100644 --- a/library/src/events.rs +++ b/library/src/events.rs @@ -30,16 +30,13 @@ impl<'de> Deserialize<'de> for EventType { match s.as_str() { "__patch_install__" => Ok(EventType::PatchInstallSuccess), "__patch_install_failure__" => Ok(EventType::PatchInstallFailure), - _ => Err(serde::de::Error::custom(format!( - "Unknown event type: {}", - s - ))), + _ => Err(serde::de::Error::custom(format!("Unknown event type: {s}"))), } } } /// Any edits to this struct should be made carefully and in accordance /// with our privacy policy: -/// https://docs.shorebird.dev/privacy +/// /// An event that is sent to the server when a patch is successfully installed. #[derive(Debug, Serialize, Deserialize, Clone)] pub struct PatchEvent { diff --git a/library/src/network.rs b/library/src/network.rs index 94c771e..7411a71 100644 --- a/library/src/network.rs +++ b/library/src/network.rs @@ -17,11 +17,11 @@ use crate::events::PatchEvent; use std::{println as info, println as debug}; // Workaround to use println! for logs. fn patches_check_url(base_url: &str) -> String { - format!("{}/api/v1/patches/check", base_url) + format!("{base_url}/api/v1/patches/check") } fn patches_events_url(base_url: &str) -> String { - format!("{}/api/v1/patches/events", base_url) + format!("{base_url}/api/v1/patches/events") } pub type PatchCheckRequestFn = fn(&str, PatchCheckRequest) -> anyhow::Result; @@ -186,7 +186,7 @@ pub struct Patch { /// Any edits to this struct should be made carefully and in accordance /// with our privacy policy: -/// https://docs.shorebird.dev/privacy +/// /// The request body for the patch check endpoint. #[derive(Debug, Serialize)] pub struct PatchCheckRequest { @@ -194,7 +194,7 @@ pub struct PatchCheckRequest { /// app_ids are unique to each app and are used to identify the app /// within Shorebird's system (similar to a bundle identifier). They /// are not secret and are safe to share publicly. - /// https://docs.shorebird.dev/concepts + /// pub app_id: String, /// The Shorebird channel built into the shorebird.yaml in the app. /// This is not currently used, but intended for future use to allow @@ -218,7 +218,7 @@ pub struct PatchCheckRequest { /// The request body for the create patch install event endpoint. /// /// We may want to consider making this more generic if/when we add more events -/// using something like https://github.com/dtolnay/typetag. +/// using something like . #[derive(Debug, Serialize)] pub struct CreatePatchEventRequest { event: PatchEvent, diff --git a/library/src/updater.rs b/library/src/updater.rs index b0946aa..af9e1cd 100644 --- a/library/src/updater.rs +++ b/library/src/updater.rs @@ -61,9 +61,9 @@ impl Display for UpdateError { fn fmt(&self, f: &mut Formatter) -> std::fmt::Result { match self { UpdateError::InvalidArgument(name, value) => { - write!(f, "Invalid Argument: {} -> {}", name, value) + write!(f, "Invalid Argument: {name} -> {value}") } - UpdateError::InvalidState(msg) => write!(f, "Invalid State: {}", msg), + UpdateError::InvalidState(msg) => write!(f, "Invalid State: {msg}"), UpdateError::FailedToSaveState => write!(f, "Failed to save state"), UpdateError::BadServerResponse => write!(f, "Bad server response"), UpdateError::ConfigNotInitialized => write!(f, "Config not initialized"), @@ -74,9 +74,9 @@ impl Display for UpdateError { } } -// AppConfig is the rust API. ResolvedConfig is the internal storage. -// However rusty api would probably used &str instead of String, -// but making &str from CStr* is a bit of a pain. +// `AppConfig` is the rust API. `ResolvedConfig` is the internal storage. +// However rusty api would probably used `&str` instead of `String`, +// but making `&str` from `CStr*` is a bit of a pain. pub struct AppConfig { pub cache_dir: String, pub release_version: String, @@ -98,9 +98,9 @@ fn libapp_path_from_settings(original_libapp_paths: &[String]) -> Result Result<(), UpdateError> { #[cfg(any(target_os = "android", test))] @@ -112,7 +112,7 @@ pub fn init(app_config: AppConfig, yaml: &str) -> Result<(), UpdateError> { let libapp_path = libapp_path_from_settings(&app_config.original_libapp_paths)?; debug!("libapp_path: {:?}", libapp_path); - set_config(app_config, libapp_path, config, NetworkHooks::default()) + set_config(app_config, libapp_path, &config, NetworkHooks::default()) .map_err(|err| UpdateError::InvalidState(err.to_string())) } @@ -135,12 +135,12 @@ pub fn check_for_update() -> anyhow::Result { } fn check_hash(path: &Path, expected_string: &str) -> anyhow::Result<()> { + use sha2::{Digest, Sha256}; // `Digest` is needed for `Sha256::new()`; + let expected = hex::decode(expected_string).context("Invalid hash string from server.")?; - use sha2::{Digest, Sha256}; // Digest is needed for Sha256::new(); - // Based on guidance from: - // https://github.com/RustCrypto/hashes#hashing-readable-objects + // let mut file = fs::File::open(path)?; let mut hasher = Sha256::new(); @@ -149,7 +149,7 @@ fn check_hash(path: &Path, expected_string: &str) -> anyhow::Result<()> { let hash = hasher.finalize(); let hash_matches = hash.as_slice() == expected; // This is a common error for developers. We could avoid it entirely - // by sending the hash of libapp.so to the server and having the + // by sending the hash of `libapp.so` to the server and having the // server only send updates when the hash matches. // https://github.com/shorebirdtech/updater/issues/56 if !hash_matches { @@ -162,9 +162,8 @@ fn check_hash(path: &Path, expected_string: &str) -> anyhow::Result<()> { expected_string, hex::encode(hash) ); - } else { - debug!("Hash match: {:?}", path); } + debug!("Hash match: {:?}", path); Ok(()) } @@ -176,9 +175,9 @@ fn prepare_for_install( download_path: &Path, output_path: &Path, ) -> anyhow::Result<()> { - // We abuse libapp_path to actually be the path to the data dir for now. - // This is an abuse because the variable name is libapp_path, but - // we're making it point to a the app_data directory instead. + // We abuse `libapp_path` to actually be the path to the data dir for now. + // This is an abuse because the variable name is `libapp_path`, but + // we're making it point to a the `app_data` directory instead. let app_dir = &config.libapp_path; debug!("app_dir: {:?}", app_dir); let base_r = crate::android::open_base_lib(app_dir, "libapp.so")?; @@ -285,7 +284,7 @@ fn update_internal(_: &UpdaterLockState) -> anyhow::Result { let mut state = UpdaterState::load_or_new_on_error(&config.cache_dir, &config.release_version); // Move/state update should be "atomic" (it isn't today). - state.install_patch(patch_info)?; + state.install_patch(&patch_info)?; info!("Patch {} successfully installed.", patch.number); // Should set some state to say the status is "update required" and that // we now have a different "next" version of the app from the current @@ -348,7 +347,7 @@ where /// The patch which will be run on next boot (which may still be the same /// as the current boot). -/// This may be changed any time update() or start_update_thread() are called. +/// This may be changed any time `update()` or `start_update_thread()` are called. pub fn next_boot_patch() -> anyhow::Result> { with_config(|config| { let state = UpdaterState::load_or_new_on_error(&config.cache_dir, &config.release_version); @@ -356,9 +355,9 @@ pub fn next_boot_patch() -> anyhow::Result> { }) } -/// The patch which is currently booted. This is None until -/// report_launch_start() is called at which point it is copied from -/// next_boot_patch. +/// The patch which is currently booted. This is `None` until +/// `report_launch_start()` is called at which point it is copied from +/// `next_boot_patch`. pub fn current_boot_patch() -> anyhow::Result> { with_config(|config| { let state = UpdaterState::load_or_new_on_error(&config.cache_dir, &config.release_version); @@ -514,7 +513,7 @@ mod tests { let mut state = UpdaterState::load_or_new_on_error(&config.cache_dir, &config.release_version); state - .install_patch(PatchInfo { + .install_patch(&PatchInfo { path: artifact_path, number: 1, }) @@ -627,7 +626,7 @@ mod tests { let mut state = UpdaterState::load_or_new_on_error(&config.cache_dir, &config.release_version); state - .install_patch(PatchInfo { + .install_patch(&PatchInfo { path: artifact_path, number: 1, }) @@ -678,7 +677,7 @@ mod tests { let mut state = UpdaterState::load_or_new_on_error(&config.cache_dir, &config.release_version); state - .install_patch(PatchInfo { + .install_patch(&PatchInfo { path: artifact_path, number: 1, }) diff --git a/library/src/updater_lock.rs b/library/src/updater_lock.rs index 35a989c..27692fc 100644 --- a/library/src/updater_lock.rs +++ b/library/src/updater_lock.rs @@ -39,7 +39,9 @@ where // This should never happen. Poisoning only happens if a thread panics // while holding the lock, and we never allow the updater thread to // panic. - Err(std::sync::TryLockError::Poisoned(e)) => panic!("Updater lock poisoned: {:?}", e), + Err(std::sync::TryLockError::Poisoned(e)) => { + panic!("Updater lock poisoned: {e:?}") + } } } diff --git a/patch/src/bin/string_patch.rs b/patch/src/bin/string_patch.rs index f681bf5..ec978a5 100644 --- a/patch/src/bin/string_patch.rs +++ b/patch/src/bin/string_patch.rs @@ -1,6 +1,8 @@ // This might combine with patch/main.rs. Just starting with a copy for ease. fn main() { + use sha2::{Digest, Sha256}; // Digest is needed for Sha256::new(); + let mut args = std::env::args(); args.next(); // skip program name let older = args.next().expect("base string"); @@ -14,13 +16,12 @@ fn main() { let patch = patch.into_inner(); - use sha2::{Digest, Sha256}; // Digest is needed for Sha256::new(); let mut hasher = Sha256::new(); hasher.update(&newer); let hash = hasher.finalize(); - println!("Base: {}", older); - println!("New: {}", newer); - println!("Patch: {:?}", patch); + println!("Base: {older}"); + println!("New: {newer}"); + println!("Patch: {patch:?}"); println!("Hash (new): {}", hex::encode(hash)); }