From b0276dfd74472df67c8e9d521d61fab4e47d26ad Mon Sep 17 00:00:00 2001 From: a-kenji Date: Thu, 17 Mar 2022 11:40:09 +0100 Subject: [PATCH] fix(feat): `disable_automatic_asset_installation` (#1226) * fix(feat): `disable_automatic_asset_installation` This fixes a regression in the feature system: The asset installation didn't get turned off by the feature. Add error logging to the install functions. Properly show features in setup disable `mkdir` in `wasm_vm` on `feature-disable-asset-installation` Alternative: Is this even needed? We make sure the directory is there upon the normal asset installation. fixes #1130 --- Cargo.toml | 2 +- src/commands.rs | 1 - src/install.rs | 23 ++++++++++++++++++----- zellij-server/src/wasm_vm.rs | 12 ++++++++++-- zellij-utils/Cargo.toml | 2 ++ 5 files changed, 31 insertions(+), 9 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 30030d61..67896386 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -64,4 +64,4 @@ bin-dir = "{ bin }{ binary-ext }" pkg-fmt = "tgz" [features] -disable_automatic_asset_installation = [] +disable_automatic_asset_installation = [ "zellij-utils/disable_automatic_asset_installation" ] diff --git a/src/commands.rs b/src/commands.rs index 54274134..cfbcb8f0 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -89,7 +89,6 @@ fn create_new_client() -> ClientInfo { fn install_default_assets(opts: &CliArgs) { let data_dir = opts.data_dir.clone().unwrap_or_else(get_default_data_dir); - #[cfg(not(disable_automatic_asset_installation))] populate_data_dir(&data_dir); } diff --git a/src/install.rs b/src/install.rs index e7778dde..047c2370 100644 --- a/src/install.rs +++ b/src/install.rs @@ -1,7 +1,10 @@ +#[cfg(not(feature = "disable_automatic_asset_installation"))] use std::fs; use std::path::Path; +#[cfg(not(feature = "disable_automatic_asset_installation"))] use zellij_utils::{consts::VERSION, shared::set_permissions}; +#[cfg(not(feature = "disable_automatic_asset_installation"))] macro_rules! asset_map { ($($src:literal => $dst:literal),+ $(,)?) => { { @@ -14,6 +17,7 @@ macro_rules! asset_map { } } +#[cfg(not(feature = "disable_automatic_asset_installation"))] pub(crate) fn populate_data_dir(data_dir: &Path) { // First run installation of default plugins & layouts let mut assets = asset_map! { @@ -28,11 +32,20 @@ pub(crate) fn populate_data_dir(data_dir: &Path) { for (path, bytes) in assets { let path = data_dir.join(path); - let parent_path = path.parent().unwrap(); - fs::create_dir_all(parent_path).unwrap(); - set_permissions(parent_path).unwrap(); - if out_of_date || !path.exists() { - fs::write(path, bytes).expect("Failed to install default assets!"); + // TODO: Is the [path.parent()] really necessary here? + // We already have the path and the parent through `data_dir` + if let Some(parent_path) = path.parent() { + fs::create_dir_all(parent_path).unwrap_or_else(|e| log::error!("{:?}", e)); + set_permissions(parent_path).unwrap_or_else(|e| log::error!("{:?}", e)); + if out_of_date || !path.exists() { + fs::write(path, bytes) + .unwrap_or_else(|e| log::error!("Failed to install default assets! {:?}", e)); + } + } else { + log::error!("The path {:?} has no parent directory", path); } } } + +#[cfg(feature = "disable_automatic_asset_installation")] +pub(crate) fn populate_data_dir(_data_dir: &Path) {} diff --git a/zellij-server/src/wasm_vm.rs b/zellij-server/src/wasm_vm.rs index ebea0b8c..4e29d323 100644 --- a/zellij-server/src/wasm_vm.rs +++ b/zellij-server/src/wasm_vm.rs @@ -92,7 +92,9 @@ pub(crate) fn wasm_thread_main( let mut connected_clients: Vec = vec![]; let plugin_dir = data_dir.join("plugins/"); let plugin_global_data_dir = plugin_dir.join("data"); - fs::create_dir_all(&plugin_global_data_dir).unwrap(); + + #[cfg(not(feature = "disable_automatic_asset_installation"))] + fs::create_dir_all(&plugin_global_data_dir).unwrap_or_else(|e| log::error!("{:?}", e)); loop { let (event, mut err_ctx) = bus.recv().expect("failed to receive event on channel"); @@ -270,7 +272,13 @@ fn start_plugin( let input = Pipe::new(); let stderr = LoggingPipe::new(&plugin.location.to_string(), plugin_id); let plugin_own_data_dir = plugin_global_data_dir.join(Url::from(&plugin.location).to_string()); - fs::create_dir_all(&plugin_own_data_dir).unwrap(); + fs::create_dir_all(&plugin_own_data_dir).unwrap_or_else(|e| { + log::error!( + "Could not create plugin_own_data_dir in {:?} \n Error: {:?}", + &plugin_own_data_dir, + e + ) + }); let mut wasi_env = WasiState::new("Zellij") .env("CLICOLOR_FORCE", "1") diff --git a/zellij-utils/Cargo.toml b/zellij-utils/Cargo.toml index cb4042f2..39d8bf71 100644 --- a/zellij-utils/Cargo.toml +++ b/zellij-utils/Cargo.toml @@ -46,3 +46,5 @@ features = ["unstable"] [dev-dependencies] tempfile = "3.2.0" +[features] +disable_automatic_asset_installation = [ ]