Revert "feat: treat iOS patches as diffs instead of full artifacts (#88)" (#90)

This reverts commit f4db98b759.

.vscode/settings.json

@@ -15,5 +15,5 @@
         "reqwest",
         "tempdir",
         "vmcode"
-    ],
+    ]
 }

library/src/android.rs

@@ -184,9 +184,6 @@ pub(crate) fn open_base_lib(apks_dir: &Path, lib_name: &str) -> anyhow::Result<C
     Ok(Cursor::new(buffer))
 }
 
-// On Android we don't use a direct path to libapp.so, but rather a data dir
-// and a hard-coded name for the libapp file which we look up in the
-// split APKs in that datadir. On other platforms we just use a path.
 pub fn libapp_path_from_settings(original_libapp_paths: &[String]) -> Result<PathBuf, UpdateError> {
     // FIXME: This makes the assumption that the last path provided is the full
     // path to the libapp.so file.  This is true for the current engine, but

library/src/c_api.rs

@@ -255,7 +255,6 @@ pub extern "C" fn shorebird_report_launch_success() {
 mod test {
     use super::*;
     use crate::network::{testing_set_network_hooks, PatchCheckResponse};
-    use crate::platform;
     use anyhow::Ok;
     use serial_test::serial;
     use tempdir::TempDir;
@@ -410,7 +409,7 @@ mod test {
         let options = zip::write::FileOptions::default()
             .compression_method(zip::CompressionMethod::Stored)
             .unix_permissions(0o755);
-        let app_path = platform::get_relative_lib_path("libapp.so");
+        let app_path = crate::android::get_relative_lib_path("libapp.so");
         zip.start_file(app_path.to_str().unwrap(), options).unwrap();
         zip.write_all(libapp_contents).unwrap();
         zip.finish().unwrap();

library/src/lib.rs

@@ -11,11 +11,13 @@ mod config;
 mod events;
 mod logging;
 mod network;
-mod platform;
 mod updater;
 mod updater_lock;
 mod yaml;
 
+#[cfg(any(target_os = "android", test))]
+mod android;
+
 // Take all public items from the updater namespace and make them public.
 pub use self::updater::*;
 

library/src/platform/ios.rs

@@ -1,85 +0,0 @@
-use std::{
-    fs::File,
-    io::{Cursor, Read},
-    path::{Path, PathBuf},
-};
-
-use anyhow::Context;
-
-use crate::UpdateError;
-
-/// lib name is unused on iOS, it exists as a parameter here to match the signature of the
-/// function on Android.
-pub(crate) fn open_base_lib(app_dir: &Path, _lib_name: &str) -> anyhow::Result<Cursor<Vec<u8>>> {
-    let mut file =
-        File::open(app_dir).with_context(|| format!("Failed to open iOS app_dir {:?}", app_dir))?;
-    let mut buffer = Vec::new();
-    file.read_to_end(&mut buffer)
-        .with_context(|| format!("Failed to read iOS app_dir {:?}", app_dir))?;
-    Ok(Cursor::new(buffer))
-}
-
-pub fn libapp_path_from_settings(original_libapp_paths: &[String]) -> Result<PathBuf, UpdateError> {
-    let first = original_libapp_paths
-        .first()
-        .ok_or(UpdateError::InvalidArgument(
-            "original_libapp_paths".to_string(),
-            "empty".to_string(),
-        ));
-    first.map(PathBuf::from)
-}
-
-#[cfg(test)]
-mod tests {
-    use std::{fs::File, path::PathBuf};
-
-    use tempdir::TempDir;
-
-    use crate::UpdateError;
-
-    use super::{libapp_path_from_settings, open_base_lib};
-
-    #[test]
-    fn opens_and_reads_app() {
-        let tmp_dir = TempDir::new("test").unwrap();
-        let path = tmp_dir.path().join("foo.txt");
-        File::create(&path).unwrap();
-        let result = open_base_lib(&path.to_path_buf(), "");
-        assert!(result.is_ok());
-    }
-
-    #[test]
-    fn returns_error_if_app_fails_to_open() {
-        let tmp_dir = TempDir::new("test").unwrap();
-        let path = tmp_dir.path().join("foo.txt");
-        let result = open_base_lib(&path.to_path_buf(), "");
-        assert!(result.is_err());
-        assert_eq!(
-            format!("{}", result.unwrap_err()),
-            format!("Failed to open iOS app_dir \"{}\"", path.to_str().unwrap()),
-        );
-    }
-
-    // TODO(bryanoltman): we don't currently test read_to_end returning an Err
-    // result. We should do that, but I'm not sure how.
-
-    #[test]
-    fn libapp_path_from_settings_returns_first_path() {
-        let path1 = "some/path/1".to_string();
-        let path2 = "some/path/2".to_string();
-        let path3 = "some/path/3".to_string();
-        let result = libapp_path_from_settings(&[path1.clone(), path2.clone(), path3.clone()]);
-        assert!(result.is_ok());
-        assert_eq!(result.unwrap(), PathBuf::from(path1));
-    }
-
-    #[test]
-    fn libapp_path_from_settings_returns_err_when_provided_slice_is_empty() {
-        let result = libapp_path_from_settings(&[]);
-        assert!(result.is_err());
-        assert_eq!(
-            result.unwrap_err(),
-            UpdateError::InvalidArgument("original_libapp_paths".to_string(), "empty".to_string(),)
-        );
-    }
-}

library/src/platform/mod.rs

@@ -1,13 +0,0 @@
-#[cfg(any(target_os = "android", test))]
-pub mod android;
-#[cfg(any(target_os = "ios", test))]
-pub mod ios;
-#[cfg(not(any(target_os = "android", target_os = "ios", test)))]
-pub mod unknown;
-
-#[cfg(any(target_os = "android", test))]
-pub use android::*;
-#[cfg(target_os = "ios")]
-pub use ios::*;
-#[cfg(not(any(target_os = "android", target_os = "ios", test)))]
-pub use unknown::*;

library/src/platform/unknown.rs

@@ -1,20 +0,0 @@
-use crate::UpdateError;
-use anyhow::bail;
-use std::{
-    io::Cursor,
-    path::{Path, PathBuf},
-};
-
-const UNKNOWN_PLATFORM_ERR_MSG: &str = "Unknown platform";
-
-pub fn open_base_lib(_app_dir: &Path, _lib_name: &str) -> anyhow::Result<Cursor<Vec<u8>>> {
-    bail!(UNKNOWN_PLATFORM_ERR_MSG)
-}
-
-pub fn libapp_path_from_settings(
-    _original_libapp_paths: &[String],
-) -> Result<PathBuf, UpdateError> {
-    Err(UpdateError::InvalidState(
-        UNKNOWN_PLATFORM_ERR_MSG.to_string(),
-    ))
-}

library/src/updater.rs

@@ -1,7 +1,8 @@
 // This file's job is to be the Rust API for the updater.
 
 use std::fmt::{Display, Formatter};
-use std::fs::File;
+use std::fs;
+#[cfg(any(target_os = "android", test))]
 use std::io::{Read, Seek};
 use std::path::{Path, PathBuf};
 
@@ -15,7 +16,6 @@ use crate::logging::init_logging;
 use crate::network::{
     download_to_path, send_patch_check_request, NetworkHooks, PatchCheckResponse,
 };
-use crate::platform::{libapp_path_from_settings, open_base_lib};
 use crate::updater_lock::{with_updater_thread_lock, UpdaterLockState};
 use crate::yaml::YamlConfig;
 
@@ -84,12 +84,29 @@ pub struct AppConfig {
     pub original_libapp_paths: Vec<String>,
 }
 
+// On Android we don't use a direct path to libapp.so, but rather a data dir
+// and a hard-coded name for the libapp file which we look up in the
+// split APKs in that datadir. On other platforms we just use a path.
+#[cfg(not(any(target_os = "android", test)))]
+fn libapp_path_from_settings(original_libapp_paths: &[String]) -> Result<PathBuf, UpdateError> {
+    let first = original_libapp_paths
+        .first()
+        .ok_or(UpdateError::InvalidArgument(
+            "original_libapp_paths".to_string(),
+            "empty".to_string(),
+        ));
+    first.map(PathBuf::from)
+}
+
 /// Initialize the updater library.
 /// Takes a `AppConfig` struct and a yaml string.
 /// The yaml string is the contents of the `shorebird.yaml` file.
 /// The `AppConfig` struct is information about the running app and where
 /// the updater should keep its cache.
 pub fn init(app_config: AppConfig, yaml: &str) -> Result<(), UpdateError> {
+    #[cfg(any(target_os = "android", test))]
+    use crate::android::libapp_path_from_settings;
+
     init_logging();
     let config = YamlConfig::from_yaml(yaml)
         .map_err(|err| UpdateError::InvalidArgument("yaml".to_string(), err.to_string()))?;
@@ -127,7 +144,7 @@ fn check_hash(path: &Path, expected_string: &str) -> anyhow::Result<()> {
     // Based on guidance from:
     // <https://github.com/RustCrypto/hashes#hashing-readable-objects>
 
-    let mut file = File::open(path)?;
+    let mut file = fs::File::open(path)?;
     let mut hasher = Sha256::new();
     std::io::copy(&mut file, &mut hasher)?;
     // Check that the length from copy is the same as the file size?
@@ -152,17 +169,32 @@ fn check_hash(path: &Path, expected_string: &str) -> anyhow::Result<()> {
     Ok(())
 }
 
+// This is just a place to put our terrible android hacks.
+// And also avoid (for now) dealing with inflating patches on iOS.
+#[cfg(any(target_os = "android", test))]
 fn prepare_for_install(
-    app_dir: &PathBuf,
+    config: &UpdateConfig,
     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.
+    let app_dir = &config.libapp_path;
     debug!("app_dir: {:?}", app_dir);
-    let base_release_artifact = open_base_lib(app_dir, "libapp.so")?;
+    let base_r = crate::android::open_base_lib(app_dir, "libapp.so")?;
-    inflate(download_path, base_release_artifact, output_path)
+    inflate(download_path, base_r, output_path)
+}
+
+#[cfg(not(any(target_os = "android", test)))]
+fn prepare_for_install(
+    _config: &UpdateConfig,
+    download_path: &Path,
+    output_path: &Path,
+) -> anyhow::Result<()> {
+    // On iOS we don't yet support compressed patches, just copy the file.
+    fs::copy(download_path, output_path)?;
+    Ok(())
 }
 
 fn copy_update_config() -> anyhow::Result<UpdateConfig> {
@@ -233,7 +265,8 @@ fn update_internal(_: &UpdaterLockState) -> anyhow::Result<UpdateStatus> {
     download_to_path(&config.network_hooks, &patch.download_url, &download_path)?;
 
     let output_path = download_dir.join(format!("{}.full", patch.number));
-    prepare_for_install(&config.libapp_path, &download_path, &output_path)?;
+    // Should not pass config, rather should read necessary information earlier.
+    prepare_for_install(&config, &download_path, &output_path)?;
 
     // Check the hash before moving into place.
     check_hash(&output_path, &patch.hash).context(format!(
@@ -269,6 +302,7 @@ pub fn update() -> anyhow::Result<UpdateStatus> {
 
 /// Given a path to a patch file, and a base file, apply the patch to the base
 /// and write the result to the output path.
+#[cfg(any(target_os = "android", test))]
 fn inflate<RS>(patch_path: &Path, base_r: RS, output_path: &Path) -> anyhow::Result<()>
 where
     RS: Read + Seek,
@@ -282,9 +316,10 @@ where
     // PipeReader/Writer errors instead of file open errors.
     debug!("Reading patch file: {:?}", patch_path);
     let compressed_patch_r = BufReader::new(
-        File::open(patch_path).context(format!("Failed to open patch file: {:?}", patch_path))?,
+        fs::File::open(patch_path)
+            .context(format!("Failed to open patch file: {:?}", patch_path))?,
     );
-    let output_file_w = File::create(output_path)?;
+    let output_file_w = fs::File::create(output_path)?;
 
     // Set up a pipe to connect the writing from the decompression thread
     // to the reading of the decompressed patch data on this thread.