From e5ec44451ae60ac66d070fdc2c0b752836570149 Mon Sep 17 00:00:00 2001 From: Nathaniel Graham Date: Fri, 14 Oct 2022 14:22:41 -0600 Subject: [PATCH 01/12] Workaround for Large File Corruption During Save --- helix-view/src/document.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 0daa983f65ca..27bc4f029759 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -560,8 +560,15 @@ impl Document { } } - let mut file = File::create(path).await?; - to_writer(&mut file, encoding, &text).await?; + // TODO Temporary file creation is a workaround to solve large file corruption + // that occurs when crashing during a file save + let mut tmp_path = path.clone(); + tmp_path.set_file_name(format!(".~{}", path.file_name().unwrap().to_str().unwrap())); + let mut tmp_file = File::create(&tmp_path).await?; + to_writer(&mut tmp_file, encoding, &text).await?; + + tokio::fs::copy(&tmp_path, path).await?; + tokio::fs::remove_file(tmp_path).await?; if let Some(language_server) = language_server { if !language_server.is_initialized() { From 4bf41bae55287fa86edce28fa7617f15e1a4e3f9 Mon Sep 17 00:00:00 2001 From: Nathaniel Graham Date: Fri, 14 Oct 2022 19:47:15 -0600 Subject: [PATCH 02/12] Cache file in ~/.cache/helix --- helix-view/src/document.rs | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 27bc4f029759..1bf543d92039 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -562,13 +562,26 @@ impl Document { // TODO Temporary file creation is a workaround to solve large file corruption // that occurs when crashing during a file save - let mut tmp_path = path.clone(); - tmp_path.set_file_name(format!(".~{}", path.file_name().unwrap().to_str().unwrap())); + let tmp_path = PathBuf::from(std::env::var("HOME").unwrap()) + .join(".cache/") + .join("helix/") + .join(format!("~{}", path.file_name().unwrap().to_str().unwrap())); + + if let Some(parent) = tmp_path.parent() { + // TODO: display a prompt asking the user if the directories should be created + if !parent.exists() { + if force { + std::fs::DirBuilder::new().recursive(true).create(parent)?; + } else { + bail!("can't save file, parent directory does not exist"); + } + } + } + let mut tmp_file = File::create(&tmp_path).await?; to_writer(&mut tmp_file, encoding, &text).await?; - tokio::fs::copy(&tmp_path, path).await?; - tokio::fs::remove_file(tmp_path).await?; + tokio::fs::rename(&tmp_path, path).await?; if let Some(language_server) = language_server { if !language_server.is_initialized() { From ee222fc3b6d388a5e8820477d5c2eaed9bee1262 Mon Sep 17 00:00:00 2001 From: Nathaniel Graham Date: Fri, 14 Oct 2022 20:09:26 -0600 Subject: [PATCH 03/12] Shorter cache folder retrieval --- helix-view/src/document.rs | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 1bf543d92039..031906947784 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -3,6 +3,7 @@ use futures_util::future::BoxFuture; use futures_util::FutureExt; use helix_core::auto_pairs::AutoPairs; use helix_core::Range; +use helix_loader::cache_dir; use serde::de::{self, Deserialize, Deserializer}; use serde::Serialize; use std::borrow::Cow; @@ -562,21 +563,8 @@ impl Document { // TODO Temporary file creation is a workaround to solve large file corruption // that occurs when crashing during a file save - let tmp_path = PathBuf::from(std::env::var("HOME").unwrap()) - .join(".cache/") - .join("helix/") - .join(format!("~{}", path.file_name().unwrap().to_str().unwrap())); - - if let Some(parent) = tmp_path.parent() { - // TODO: display a prompt asking the user if the directories should be created - if !parent.exists() { - if force { - std::fs::DirBuilder::new().recursive(true).create(parent)?; - } else { - bail!("can't save file, parent directory does not exist"); - } - } - } + let tmp_path = + cache_dir().join(format!("~{}", path.file_name().unwrap().to_str().unwrap())); let mut tmp_file = File::create(&tmp_path).await?; to_writer(&mut tmp_file, encoding, &text).await?; From 946e73b2abeabec02add9c076a7ea9a497c8bb6b Mon Sep 17 00:00:00 2001 From: Nathaniel Graham Date: Fri, 14 Oct 2022 20:14:27 -0600 Subject: [PATCH 04/12] Removed to_str() for non-utf8 unix filenames --- helix-view/src/document.rs | 3 +-- temp.txt | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) create mode 100644 temp.txt diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 031906947784..0707f986052d 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -563,8 +563,7 @@ impl Document { // TODO Temporary file creation is a workaround to solve large file corruption // that occurs when crashing during a file save - let tmp_path = - cache_dir().join(format!("~{}", path.file_name().unwrap().to_str().unwrap())); + let tmp_path = cache_dir().join(format!("~{:?}", path.file_name().unwrap())); let mut tmp_file = File::create(&tmp_path).await?; to_writer(&mut tmp_file, encoding, &text).await?; diff --git a/temp.txt b/temp.txt new file mode 100644 index 000000000000..67b3a2ad5114 --- /dev/null +++ b/temp.txt @@ -0,0 +1 @@ +asdffdsa From 96d001863482beff816ee1f21fc81615945f5732 Mon Sep 17 00:00:00 2001 From: Nathaniel Graham Date: Fri, 14 Oct 2022 20:16:35 -0600 Subject: [PATCH 05/12] Delete temp.txt *facepalm* --- temp.txt | 1 - 1 file changed, 1 deletion(-) delete mode 100644 temp.txt diff --git a/temp.txt b/temp.txt deleted file mode 100644 index 67b3a2ad5114..000000000000 --- a/temp.txt +++ /dev/null @@ -1 +0,0 @@ -asdffdsa From 7bb56dc634e0c7c56643efc9733f471fe2c1ffe8 Mon Sep 17 00:00:00 2001 From: Nathaniel Graham Date: Fri, 14 Oct 2022 20:48:47 -0600 Subject: [PATCH 06/12] Changed rename to copy and delete --- helix-view/src/document.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 0707f986052d..a4c9b57bbdcf 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -568,7 +568,8 @@ impl Document { let mut tmp_file = File::create(&tmp_path).await?; to_writer(&mut tmp_file, encoding, &text).await?; - tokio::fs::rename(&tmp_path, path).await?; + tokio::fs::copy(&tmp_path, path).await?; + tokio::fs::remove_file(tmp_path).await?; if let Some(language_server) = language_server { if !language_server.is_initialized() { From 60ffe22947c5b55dc2448e6662192828187d4ec0 Mon Sep 17 00:00:00 2001 From: Nathaniel Graham Date: Fri, 14 Oct 2022 21:04:04 -0600 Subject: [PATCH 07/12] Changed HELIX_LOG_LEVEL to debug --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index ef47a277b1b9..a7791564bf96 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -40,7 +40,7 @@ jobs: runs-on: ${{ matrix.os }} env: RUST_BACKTRACE: 1 - HELIX_LOG_LEVEL: info + HELIX_LOG_LEVEL: debug steps: - name: Checkout sources uses: actions/checkout@v3 From 801beb718fa782fd627948c70ae2c3d03e28f9fa Mon Sep 17 00:00:00 2001 From: Nathaniel Graham Date: Fri, 14 Oct 2022 22:22:16 -0600 Subject: [PATCH 08/12] Save file creates cache if not exist --- helix-view/src/document.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index a4c9b57bbdcf..99233aad1215 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -565,12 +565,27 @@ impl Document { // that occurs when crashing during a file save let tmp_path = cache_dir().join(format!("~{:?}", path.file_name().unwrap())); + if let Some(parent) = tmp_path.parent() { + // TODO Temporary fix. Normally, this directory will exist by this point, + // but not during the integration tests. + if !parent.exists() { + if force { + std::fs::DirBuilder::new().recursive(true).create(parent)?; + } else { + bail!("can't save file, parent directory does not exist"); + } + } + } + let mut tmp_file = File::create(&tmp_path).await?; to_writer(&mut tmp_file, encoding, &text).await?; tokio::fs::copy(&tmp_path, path).await?; tokio::fs::remove_file(tmp_path).await?; + // TODO figure out why rename makes the tests break + //tokio::fs::rename(tmp_path, path).await?; + if let Some(language_server) = language_server { if !language_server.is_initialized() { return Ok(()); From 73e80d3a9d52a48b077448bf4d23dbdf54f224a4 Mon Sep 17 00:00:00 2001 From: Nathaniel Graham Date: Sat, 15 Oct 2022 11:45:06 -0600 Subject: [PATCH 09/12] Moved cachefile back to local directory to use tokio::rename --- helix-view/src/document.rs | 23 ++++------------------- 1 file changed, 4 insertions(+), 19 deletions(-) diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 99233aad1215..bf10ffeb75a5 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -3,7 +3,6 @@ use futures_util::future::BoxFuture; use futures_util::FutureExt; use helix_core::auto_pairs::AutoPairs; use helix_core::Range; -use helix_loader::cache_dir; use serde::de::{self, Deserialize, Deserializer}; use serde::Serialize; use std::borrow::Cow; @@ -563,28 +562,14 @@ impl Document { // TODO Temporary file creation is a workaround to solve large file corruption // that occurs when crashing during a file save - let tmp_path = cache_dir().join(format!("~{:?}", path.file_name().unwrap())); + let mut tmp_filename = std::ffi::OsString::from("~"); + tmp_filename.push(path.file_name().unwrap().to_os_string()); - if let Some(parent) = tmp_path.parent() { - // TODO Temporary fix. Normally, this directory will exist by this point, - // but not during the integration tests. - if !parent.exists() { - if force { - std::fs::DirBuilder::new().recursive(true).create(parent)?; - } else { - bail!("can't save file, parent directory does not exist"); - } - } - } + let tmp_path = path.parent().unwrap().join(tmp_filename); let mut tmp_file = File::create(&tmp_path).await?; to_writer(&mut tmp_file, encoding, &text).await?; - - tokio::fs::copy(&tmp_path, path).await?; - tokio::fs::remove_file(tmp_path).await?; - - // TODO figure out why rename makes the tests break - //tokio::fs::rename(tmp_path, path).await?; + tokio::fs::rename(tmp_path, path).await?; if let Some(language_server) = language_server { if !language_server.is_initialized() { From 6d02594d1cb274caec9c36945aaa40f032ff890d Mon Sep 17 00:00:00 2001 From: Nathaniel Graham Date: Sat, 15 Oct 2022 11:46:33 -0600 Subject: [PATCH 10/12] Removed manual OsString coersion --- helix-view/src/document.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index bf10ffeb75a5..d8505dc12e86 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -563,7 +563,7 @@ impl Document { // TODO Temporary file creation is a workaround to solve large file corruption // that occurs when crashing during a file save let mut tmp_filename = std::ffi::OsString::from("~"); - tmp_filename.push(path.file_name().unwrap().to_os_string()); + tmp_filename.push(path.file_name().unwrap()); let tmp_path = path.parent().unwrap().join(tmp_filename); From 3dcd52553e5f1c1b3835d0a4e70b4db7f2bf5e0a Mon Sep 17 00:00:00 2001 From: Nathaniel Graham Date: Sat, 15 Oct 2022 23:56:37 -0600 Subject: [PATCH 11/12] Logging way too much, but gained knowledge --- helix-view/src/document.rs | 40 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index d8505dc12e86..da7807420dac 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -13,6 +13,7 @@ use std::future::Future; use std::path::{Path, PathBuf}; use std::str::FromStr; use std::sync::Arc; +use tokio::io::AsyncReadExt; use helix_core::{ encoding, @@ -562,14 +563,53 @@ impl Document { // TODO Temporary file creation is a workaround to solve large file corruption // that occurs when crashing during a file save + + // running 5 tests + // test test::write::test_write_concurrent ... ignored + // test test::write::test_write_fail_mod_flag ... ignored + // test test::write::test_write_fail_new_path ... ignored + // 2022-10-15T23:53:23.412 helix_view::clipboard::provider [WARN] No clipboard provider found! Yanking and pasting will be internal to Helix + // 2022-10-15T23:53:23.412 helix_view::clipboard::provider [WARN] No clipboard provider found! Yanking and pasting will be internal to Helix + // 2022-10-15T23:53:23.673 helix_view::document [INFO] logs are getting printed + // 2022-10-15T23:53:23.673 helix_view::document [INFO] logs are still getting printed + // 2022-10-15T23:53:23.674 helix_view::document [INFO] logs are still yet getting printed + // 2022-10-15T23:53:23.674 helix_view::document [INFO] and yet they still be printed + // 2022-10-15T23:53:23.674 helix_view::document [INFO] I can't belive i'm doing this + // 2022-10-15T23:53:23.680 helix_view::document [INFO] logs are getting printed + // 2022-10-15T23:53:23.680 helix_view::document [INFO] logs are still getting printed + // 2022-10-15T23:53:23.680 helix_view::document [INFO] logs are still yet getting printed + // 2022-10-15T23:53:23.680 helix_view::document [INFO] and yet they still be printed + // 2022-10-15T23:53:23.680 helix_view::document [INFO] I can't belive i'm doing this + // test test::write::test_write_quit ... FAILED + // test test::write::test_write ... FAILED let mut tmp_filename = std::ffi::OsString::from("~"); tmp_filename.push(path.file_name().unwrap()); let tmp_path = path.parent().unwrap().join(tmp_filename); let mut tmp_file = File::create(&tmp_path).await?; + log::info!("logs are getting printed"); + to_writer(&mut tmp_file, encoding, &text).await?; + log::info!("logs are still getting printed"); + + let mut tmp_file_content = String::new(); + log::info!("logs are still yet getting printed"); + + let mut actual_file_content = String::new(); + log::info!("and yet they still be printed"); + + let mut actual_file = File::create(&path).await?; + log::info!("I can't belive i'm doing this"); + + tmp_file.read_to_string(&mut tmp_file_content).await?; + log::info!("tmp file contents: {}", tmp_file_content); + tokio::fs::rename(tmp_path, path).await?; + log::info!("I renamed the file"); + + actual_file.read_to_string(&mut actual_file_content).await?; + log::info!("actual file contents after rename: {}", actual_file_content); if let Some(language_server) = language_server { if !language_server.is_initialized() { From a51f5b3ba716bcb975bed965fffebe2ec2a5f9ba Mon Sep 17 00:00:00 2001 From: Nathaniel Graham Date: Sat, 15 Oct 2022 23:58:24 -0600 Subject: [PATCH 12/12] added filenames to output --- helix-view/src/document.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index da7807420dac..4cb80bb24b83 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -589,7 +589,8 @@ impl Document { let mut tmp_file = File::create(&tmp_path).await?; log::info!("logs are getting printed"); - + log::info!("temp file location: {:?}", tmp_path); + log::info!("actual file location: {:?}", path); to_writer(&mut tmp_file, encoding, &text).await?; log::info!("logs are still getting printed");