From 7616546a6161061832f718bc4bc8e72e30f0a2f7 Mon Sep 17 00:00:00 2001 From: Jiahao XU <Jiahao_XU@outlook.com> Date: Thu, 23 Jun 2022 21:27:15 +1000 Subject: [PATCH 1/7] Impl new fn `atomic_install` for atommically installing a file Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com> --- src/helpers.rs | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/src/helpers.rs b/src/helpers.rs index 03bf97c9..a13574c0 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -1,4 +1,6 @@ use std::fmt::Debug; +use std::fs; +use std::io; use std::path::{Path, PathBuf}; use bytes::Bytes; @@ -8,6 +10,7 @@ use log::debug; use once_cell::sync::OnceCell; use reqwest::{Client, ClientBuilder, Method, Response}; use serde::Serialize; +use tempfile::NamedTempFile; use tinytemplate::TinyTemplate; use url::Url; @@ -179,6 +182,48 @@ pub fn get_install_path<P: AsRef<Path>>(install_path: Option<P>) -> Option<PathB dir } +/// Atomically install a file. +/// +/// This is a blocking function, must be called in `block_in_place` mode. +pub fn atomic_install(src: &Path, dst: &Path) -> io::Result<()> { + debug!( + "Attempting to atomically rename from '{}' to '{}'", + src.display(), + dst.display() + ); + + if fs::rename(src, dst).is_err() { + debug!("Attempting at atomically failed, fallback to creating tempfile."); + // src and dst is not on the same filesystem/mountpoint. + // Fallback to creating NamedTempFile on the parent dir of + // dst. + + let mut src_file = fs::File::open(src)?; + + let parent = dst.parent().unwrap(); + debug!("Creating named tempfile at '{}'", parent.display()); + let mut tempfile = NamedTempFile::new_in(parent)?; + + debug!( + "Copying from '{}' to '{}'", + src.display(), + tempfile.path().display() + ); + io::copy(&mut src_file, tempfile.as_file_mut())?; + + debug!( + "Persisting '{}' to '{}'", + tempfile.path().display(), + dst.display() + ); + tempfile.persist(dst).map_err(io::Error::from)?; + } else { + debug!("Attempting at atomically succeeded."); + } + + Ok(()) +} + pub trait Template: Serialize { fn render(&self, template: &str) -> Result<String, BinstallError> where From 97bfeb7bd8093b42d4cd745f9f45af0c48a70098 Mon Sep 17 00:00:00 2001 From: Jiahao XU <Jiahao_XU@outlook.com> Date: Thu, 23 Jun 2022 21:27:30 +1000 Subject: [PATCH 2/7] Use `atomic_install` in `install_bin` Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com> --- src/bins.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/bins.rs b/src/bins.rs index 76dfc9fc..18134f70 100644 --- a/src/bins.rs +++ b/src/bins.rs @@ -4,7 +4,7 @@ use cargo_toml::Product; use log::debug; use serde::Serialize; -use crate::{BinstallError, PkgFmt, PkgMeta, Template}; +use crate::{atomic_install, BinstallError, PkgFmt, PkgMeta, Template}; pub struct BinFile { pub base_name: String, @@ -80,11 +80,11 @@ impl BinFile { pub fn install_bin(&self) -> Result<(), BinstallError> { // TODO: check if file already exists debug!( - "Copy file from '{}' to '{}'", + "Atomically install file from '{}' to '{}'", self.source.display(), self.dest.display() ); - std::fs::copy(&self.source, &self.dest)?; + atomic_install(&self.source, &self.dest)?; #[cfg(target_family = "unix")] { From d4105585db3cf3d90f30b4fb10cedadfc4546f7c Mon Sep 17 00:00:00 2001 From: Jiahao XU <Jiahao_XU@outlook.com> Date: Thu, 23 Jun 2022 22:50:30 +1000 Subject: [PATCH 3/7] Fix `atomic_install`: Copy permissions of src Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com> --- src/helpers.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/helpers.rs b/src/helpers.rs index a13574c0..361dc60e 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -211,6 +211,15 @@ pub fn atomic_install(src: &Path, dst: &Path) -> io::Result<()> { ); io::copy(&mut src_file, tempfile.as_file_mut())?; + debug!("Retrieving permissions of '{}'", src.display()); + let permissions = src_file.metadata()?.permissions(); + + debug!( + "Setting permissions of '{}' to '{permissions:#?}'", + tempfile.path().display() + ); + tempfile.as_file().set_permissions(permissions)?; + debug!( "Persisting '{}' to '{}'", tempfile.path().display(), From e9c86dfad4713da5de25b9296fa0a7cc774f7570 Mon Sep 17 00:00:00 2001 From: Jiahao XU <Jiahao_XU@outlook.com> Date: Thu, 23 Jun 2022 22:51:50 +1000 Subject: [PATCH 4/7] Rm `set_permissions` in `BinFile::install_bin` Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com> --- src/bins.rs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/bins.rs b/src/bins.rs index 18134f70..56ff66e5 100644 --- a/src/bins.rs +++ b/src/bins.rs @@ -86,13 +86,6 @@ impl BinFile { ); atomic_install(&self.source, &self.dest)?; - #[cfg(target_family = "unix")] - { - use std::os::unix::fs::PermissionsExt; - debug!("Set permissions 755 on '{}'", self.dest.display()); - std::fs::set_permissions(&self.dest, std::fs::Permissions::from_mode(0o755))?; - } - Ok(()) } From 77ce57815ce2674e7fcb3d0be7352de90e33578a Mon Sep 17 00:00:00 2001 From: Jiahao XU <Jiahao_XU@outlook.com> Date: Thu, 23 Jun 2022 23:26:30 +1000 Subject: [PATCH 5/7] Impl new fn `helpers::symlink_file` Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com> --- src/helpers.rs | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/src/helpers.rs b/src/helpers.rs index 361dc60e..0fbe2256 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -233,6 +233,40 @@ pub fn atomic_install(src: &Path, dst: &Path) -> io::Result<()> { Ok(()) } +fn symlink_file<P: AsRef<Path>, Q: AsRef<Path>>(original: P, link: Q) -> io::Result<()> { + #[cfg(target_family = "unix")] + let f = std::os::unix::fs::symlink; + #[cfg(target_family = "windows")] + let f = std::os::windows::fs::symlink_file; + + f(original, link) +} + +/// Atomically install symlink "link" to a file "dst". +/// +/// This is a blocking function, must be called in `block_in_place` mode. +pub fn atomic_symlink_file(dest: &Path, link: &Path) -> io::Result<()> { + let parent = link.parent().unwrap(); + + debug!("Creating tempPath at '{}'", parent.display()); + let temp_path = NamedTempFile::new_in(parent)?.into_temp_path(); + fs::remove_file(&temp_path)?; + + debug!( + "Creating symlink '{}' to file '{}'", + temp_path.display(), + dest.display() + ); + symlink_file(dest, &temp_path)?; + + debug!( + "Persisting '{}' to '{}'", + temp_path.display(), + link.display() + ); + temp_path.persist(link).map_err(io::Error::from) +} + pub trait Template: Serialize { fn render(&self, template: &str) -> Result<String, BinstallError> where From 43238e39a30c836bef02a6b57dd2444af9b25218 Mon Sep 17 00:00:00 2001 From: Jiahao XU <Jiahao_XU@outlook.com> Date: Thu, 23 Jun 2022 23:26:46 +1000 Subject: [PATCH 6/7] Use `atomic_symlink_file` in `install_link` Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com> --- src/bins.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/bins.rs b/src/bins.rs index 56ff66e5..ce427dd1 100644 --- a/src/bins.rs +++ b/src/bins.rs @@ -4,7 +4,7 @@ use cargo_toml::Product; use log::debug; use serde::Serialize; -use crate::{atomic_install, BinstallError, PkgFmt, PkgMeta, Template}; +use crate::{atomic_install, atomic_symlink_file, BinstallError, PkgFmt, PkgMeta, Template}; pub struct BinFile { pub base_name: String, @@ -103,10 +103,7 @@ impl BinFile { self.link.display(), dest.display() ); - #[cfg(target_family = "unix")] - std::os::unix::fs::symlink(dest, &self.link)?; - #[cfg(target_family = "windows")] - std::os::windows::fs::symlink_file(dest, &self.link)?; + atomic_symlink_file(dest, &self.link)?; Ok(()) } From 14c606d72b5e3c6b10d4143f2293befd22a8f367 Mon Sep 17 00:00:00 2001 From: Jiahao XU <Jiahao_XU@outlook.com> Date: Thu, 23 Jun 2022 23:27:16 +1000 Subject: [PATCH 7/7] Simplify `BinFile::link_dest` Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com> --- src/bins.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/bins.rs b/src/bins.rs index ce427dd1..d08cf15a 100644 --- a/src/bins.rs +++ b/src/bins.rs @@ -109,12 +109,9 @@ impl BinFile { } fn link_dest(&self) -> &Path { - #[cfg(target_family = "unix")] - { + if cfg!(target_family = "unix") { Path::new(self.dest.file_name().unwrap()) - } - #[cfg(target_family = "windows")] - { + } else { &self.dest } }