public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH proxmox v2] sys: pass create options as reference
@ 2024-05-29  9:55 Dietmar Maurer
  2024-05-29 10:22 ` Fabian Grünbichler
  0 siblings, 1 reply; 5+ messages in thread
From: Dietmar Maurer @ 2024-05-29  9:55 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
---

Changes in v2: change everywhere, so "cargo build --all --all-features" works again

 proxmox-acme-api/src/account_config.rs        |  2 +-
 proxmox-acme-api/src/config.rs                |  2 +-
 proxmox-openid/src/auth_state.rs              |  4 ++--
 .../src/filesystem_helpers.rs                 | 10 +++++-----
 proxmox-rest-server/src/api_config.rs         |  4 ++--
 proxmox-rest-server/src/daemon.rs             |  2 +-
 proxmox-rest-server/src/file_logger.rs        |  2 +-
 proxmox-rest-server/src/lib.rs                |  2 +-
 proxmox-rest-server/src/worker_task.rs        | 10 +++++-----
 proxmox-rrd/src/cache.rs                      |  4 ++--
 proxmox-rrd/src/cache/journal.rs              |  4 ++--
 proxmox-rrd/src/cache/rrd_map.rs              |  4 ++--
 proxmox-rrd/src/rrd.rs                        |  2 +-
 proxmox-subscription/src/files.rs             |  4 ++--
 proxmox-sys/src/fs/dir.rs                     | 20 +++++++++----------
 proxmox-sys/src/fs/file.rs                    |  8 ++++----
 proxmox-sys/src/logrotate.rs                  |  2 +-
 .../src/dns/resolv_conf.rs                    |  2 +-
 18 files changed, 44 insertions(+), 44 deletions(-)

diff --git a/proxmox-acme-api/src/account_config.rs b/proxmox-acme-api/src/account_config.rs
index 7c7bd8a3..fd71c285 100644
--- a/proxmox-acme-api/src/account_config.rs
+++ b/proxmox-acme-api/src/account_config.rs
@@ -223,7 +223,7 @@ pub(crate) fn save_account_config(
     replace_file(
         account_cfg_filename,
         &data,
-        CreateOptions::new()
+        &CreateOptions::new()
             .perm(nix::sys::stat::Mode::from_bits_truncate(0o600))
             .owner(nix::unistd::ROOT)
             .group(nix::unistd::Gid::from_raw(0)),
diff --git a/proxmox-acme-api/src/config.rs b/proxmox-acme-api/src/config.rs
index 02b2e68a..de5a07df 100644
--- a/proxmox-acme-api/src/config.rs
+++ b/proxmox-acme-api/src/config.rs
@@ -43,7 +43,7 @@ pub(crate) fn create_secret_subdir<P: AsRef<Path>>(dir: P) -> nix::Result<()> {
         .group(nix::unistd::Gid::from_raw(0))
         .perm(nix::sys::stat::Mode::from_bits_truncate(0o700));
 
-    match proxmox_sys::fs::create_dir(dir, root_only) {
+    match proxmox_sys::fs::create_dir(dir, &root_only) {
         Ok(()) => Ok(()),
         Err(err) if err.already_exists() => Ok(()),
         Err(err) => Err(err),
diff --git a/proxmox-openid/src/auth_state.rs b/proxmox-openid/src/auth_state.rs
index f2a299a0..72b7ce4c 100644
--- a/proxmox-openid/src/auth_state.rs
+++ b/proxmox-openid/src/auth_state.rs
@@ -20,7 +20,7 @@ fn load_auth_state_locked(
         lock_path,
         std::time::Duration::new(10, 0),
         true,
-        CreateOptions::new(),
+        &CreateOptions::new(),
     )?;
 
     let mut path = state_dir.to_owned();
@@ -50,7 +50,7 @@ fn replace_auth_state(path: &Path, data: &Vec<Value>) -> Result<(), Error> {
     let options = CreateOptions::new().perm(mode);
     let raw = serde_json::to_string_pretty(data)?;
 
-    replace_file(path, raw.as_bytes(), options, false)?;
+    replace_file(path, raw.as_bytes(), &options, false)?;
 
     Ok(())
 }
diff --git a/proxmox-product-config/src/filesystem_helpers.rs b/proxmox-product-config/src/filesystem_helpers.rs
index 2c486949..461e09bd 100644
--- a/proxmox-product-config/src/filesystem_helpers.rs
+++ b/proxmox-product-config/src/filesystem_helpers.rs
@@ -66,14 +66,14 @@ pub fn replace_privileged_config<P: AsRef<std::path::Path>>(
     data: &[u8],
 ) -> Result<(), Error> {
     let options = privileged_create_options();
-    proxmox_sys::fs::replace_file(path, data, options, true)?;
+    proxmox_sys::fs::replace_file(path, data, &options, true)?;
     Ok(())
 }
 
 /// Atomically write data to file owned by `api-user.uid:api-user.gid` with permission `0660`.
 pub fn replace_config<P: AsRef<std::path::Path>>(path: P, data: &[u8]) -> Result<(), Error> {
     let options = default_create_options();
-    proxmox_sys::fs::replace_file(path, data, options, true)?;
+    proxmox_sys::fs::replace_file(path, data, &options, true)?;
     Ok(())
 }
 
@@ -82,7 +82,7 @@ pub fn replace_config<P: AsRef<std::path::Path>>(path: P, data: &[u8]) -> Result
 /// Only the superuser can read and write those files.
 pub fn replace_secret_config<P: AsRef<std::path::Path>>(path: P, data: &[u8]) -> Result<(), Error> {
     let options = secret_create_options();
-    proxmox_sys::fs::replace_file(path, data, options, true)?;
+    proxmox_sys::fs::replace_file(path, data, &options, true)?;
     Ok(())
 }
 
@@ -92,7 +92,7 @@ pub fn replace_secret_config<P: AsRef<std::path::Path>>(path: P, data: &[u8]) ->
 /// for system configuration files inside "/etc/" (i.e. "/etc/resolv.conf").
 pub fn replace_system_config<P: AsRef<std::path::Path>>(path: P, data: &[u8]) -> Result<(), Error> {
     let options = system_config_create_options();
-    proxmox_sys::fs::replace_file(path, data, options, true)?;
+    proxmox_sys::fs::replace_file(path, data, &options, true)?;
     Ok(())
 }
 
@@ -124,6 +124,6 @@ pub fn open_api_lockfile<P: AsRef<std::path::Path>>(
 ) -> Result<ApiLockGuard, Error> {
     let options = lockfile_create_options();
     let timeout = timeout.unwrap_or(std::time::Duration::new(10, 0));
-    let file = proxmox_sys::fs::open_file_locked(&path, timeout, exclusive, options)?;
+    let file = proxmox_sys::fs::open_file_locked(&path, timeout, exclusive, &options)?;
     Ok(ApiLockGuard(Some(file)))
 }
diff --git a/proxmox-rest-server/src/api_config.rs b/proxmox-rest-server/src/api_config.rs
index 80589446..1ab2130e 100644
--- a/proxmox-rest-server/src/api_config.rs
+++ b/proxmox-rest-server/src/api_config.rs
@@ -213,7 +213,7 @@ impl ApiConfig {
         let path: PathBuf = path.into();
         if let Some(base) = path.parent() {
             if !base.exists() {
-                create_path(base, None, dir_opts).map_err(|err| format_err!("{}", err))?;
+                create_path(base, None, dir_opts.as_ref()).map_err(|err| format_err!("{}", err))?;
             }
         }
 
@@ -252,7 +252,7 @@ impl ApiConfig {
         let path: PathBuf = path.into();
         if let Some(base) = path.parent() {
             if !base.exists() {
-                create_path(base, None, dir_opts).map_err(|err| format_err!("{}", err))?;
+                create_path(base, None, dir_opts.as_ref()).map_err(|err| format_err!("{}", err))?;
             }
         }
 
diff --git a/proxmox-rest-server/src/daemon.rs b/proxmox-rest-server/src/daemon.rs
index fedbf545..098b93f0 100644
--- a/proxmox-rest-server/src/daemon.rs
+++ b/proxmox-rest-server/src/daemon.rs
@@ -186,7 +186,7 @@ impl Reloader {
                     proxmox_sys::fs::replace_file(
                         pid_fn,
                         pid_str.as_bytes(),
-                        CreateOptions::new(),
+                        &CreateOptions::new(),
                         false,
                     )?;
                 }
diff --git a/proxmox-rest-server/src/file_logger.rs b/proxmox-rest-server/src/file_logger.rs
index 2bb1fac6..50a23a02 100644
--- a/proxmox-rest-server/src/file_logger.rs
+++ b/proxmox-rest-server/src/file_logger.rs
@@ -98,7 +98,7 @@ impl FileLogger {
         }
 
         let file =
-            atomic_open_or_create_file(&file_name, flags, &[], options.file_opts.clone(), false)?;
+            atomic_open_or_create_file(&file_name, flags, &[], &options.file_opts, false)?;
 
         Ok(file)
     }
diff --git a/proxmox-rest-server/src/lib.rs b/proxmox-rest-server/src/lib.rs
index ce9e4f15..ac0cc4ce 100644
--- a/proxmox-rest-server/src/lib.rs
+++ b/proxmox-rest-server/src/lib.rs
@@ -80,7 +80,7 @@ pub(crate) fn pstart() -> u64 {
 /// Helper to write the PID into a file
 pub fn write_pid(pid_fn: &str) -> Result<(), Error> {
     let pid_str = format!("{}\n", *PID);
-    proxmox_sys::fs::replace_file(pid_fn, pid_str.as_bytes(), CreateOptions::new(), false)
+    proxmox_sys::fs::replace_file(pid_fn, pid_str.as_bytes(), &CreateOptions::new(), false)
 }
 
 /// Helper to read the PID from a file
diff --git a/proxmox-rest-server/src/worker_task.rs b/proxmox-rest-server/src/worker_task.rs
index 4cf24cc4..b63f7049 100644
--- a/proxmox-rest-server/src/worker_task.rs
+++ b/proxmox-rest-server/src/worker_task.rs
@@ -83,7 +83,7 @@ impl WorkerTaskSetup {
         let timeout = std::time::Duration::new(10, 0);
 
         let file =
-            proxmox_sys::fs::open_file_locked(&self.task_lock_fn, timeout, exclusive, options)?;
+            proxmox_sys::fs::open_file_locked(&self.task_lock_fn, timeout, exclusive, &options)?;
 
         Ok(TaskListLockGuard(file))
     }
@@ -107,7 +107,7 @@ impl WorkerTaskSetup {
             .clone()
             .perm(nix::sys::stat::Mode::from_bits_truncate(0o755));
 
-        create_path(&path, None, Some(dir_opts))?;
+        create_path(&path, None, Some(&dir_opts))?;
 
         path.push(upid.to_string());
         Ok(path)
@@ -166,7 +166,7 @@ impl WorkerTaskSetup {
             .clone()
             .perm(nix::sys::stat::Mode::from_bits_truncate(0o660));
 
-        replace_file(&self.active_tasks_fn, active_raw.as_bytes(), options, false)?;
+        replace_file(&self.active_tasks_fn, active_raw.as_bytes(), &options, false)?;
 
         finish_list.sort_unstable_by(|a, b| match (&a.state, &b.state) {
             (Some(s1), Some(s2)) => s1.cmp(s2),
@@ -185,7 +185,7 @@ impl WorkerTaskSetup {
                 &self.task_archive_fn,
                 OFlag::O_APPEND | OFlag::O_RDWR,
                 &[],
-                options,
+                &options,
                 false,
             )?;
             for info in &finish_list {
@@ -212,7 +212,7 @@ impl WorkerTaskSetup {
                 .clone()
                 .perm(nix::sys::stat::Mode::from_bits_truncate(0o755));
 
-            create_path(&self.taskdir, Some(dir_opts.clone()), Some(dir_opts))?;
+            create_path(&self.taskdir, Some(&dir_opts), Some(&dir_opts))?;
             // fixme:??? create_path(pbs_buildcfg::PROXMOX_BACKUP_RUN_DIR, None, Some(opts))?;
             Ok(())
         })
diff --git a/proxmox-rrd/src/cache.rs b/proxmox-rrd/src/cache.rs
index e3d2f8d5..683c8c4e 100644
--- a/proxmox-rrd/src/cache.rs
+++ b/proxmox-rrd/src/cache.rs
@@ -67,8 +67,8 @@ impl Cache {
 
         create_path(
             &basedir,
-            Some(dir_options.clone()),
-            Some(dir_options.clone()),
+            Some(&dir_options),
+            Some(&dir_options),
         )
         .map_err(|err: Error| format_err!("unable to create rrdb stat dir - {}", err))?;
 
diff --git a/proxmox-rrd/src/cache/journal.rs b/proxmox-rrd/src/cache/journal.rs
index c196b342..447c0690 100644
--- a/proxmox-rrd/src/cache/journal.rs
+++ b/proxmox-rrd/src/cache/journal.rs
@@ -116,7 +116,7 @@ impl JournalState {
             &journal_path,
             flags,
             &[],
-            self.config.file_options.clone(),
+            &self.config.file_options,
             false,
         )?;
         Ok(BufReader::new(journal))
@@ -131,7 +131,7 @@ impl JournalState {
             &journal_path,
             flags,
             &[],
-            config.file_options.clone(),
+            &config.file_options,
             false,
         )?;
         Ok(journal)
diff --git a/proxmox-rrd/src/cache/rrd_map.rs b/proxmox-rrd/src/cache/rrd_map.rs
index 881b3987..8c09e2af 100644
--- a/proxmox-rrd/src/cache/rrd_map.rs
+++ b/proxmox-rrd/src/cache/rrd_map.rs
@@ -46,8 +46,8 @@ impl RRDMap {
             path.push(rel_path);
             create_path(
                 path.parent().unwrap(),
-                Some(self.config.dir_options.clone()),
-                Some(self.config.dir_options.clone()),
+                Some(&self.config.dir_options),
+                Some(&self.config.dir_options),
             )?;
 
             let mut rrd = (self.load_rrd_cb)(&path, rel_path, dst);
diff --git a/proxmox-rrd/src/rrd.rs b/proxmox-rrd/src/rrd.rs
index 440abe06..2e8f0f06 100644
--- a/proxmox-rrd/src/rrd.rs
+++ b/proxmox-rrd/src/rrd.rs
@@ -424,7 +424,7 @@ impl Database {
         options: CreateOptions,
         avoid_page_cache: bool,
     ) -> Result<(), Error> {
-        let (fd, tmp_path) = make_tmp_file(path, options)?;
+        let (fd, tmp_path) = make_tmp_file(path, &options)?;
         let mut file = unsafe { std::fs::File::from_raw_fd(fd.into_raw_fd()) };
 
         let mut try_block = || -> Result<(), Error> {
diff --git a/proxmox-subscription/src/files.rs b/proxmox-subscription/src/files.rs
index 37008f4a..f0ff9bf4 100644
--- a/proxmox-subscription/src/files.rs
+++ b/proxmox-subscription/src/files.rs
@@ -127,7 +127,7 @@ pub fn write_subscription<P: AsRef<Path>>(
         format!("{}\n{}\n{}\n", info.key.as_ref().unwrap(), csum, encoded)
     };
 
-    replace_file(path, raw.as_bytes(), file_opts, true)?;
+    replace_file(path, raw.as_bytes(), &file_opts, true)?;
 
     Ok(())
 }
@@ -152,7 +152,7 @@ pub fn update_apt_auth<P: AsRef<Path>>(
             let conf = format!("machine {url}\n login {}\n password {}\n", key, password,);
 
             // we use a namespaced .conf file, so just overwrite..
-            replace_file(path, conf.as_bytes(), file_opts, true)
+            replace_file(path, conf.as_bytes(), &file_opts, true)
                 .map_err(|e| format_err!("Error saving apt auth config - {}", e))?;
         }
         _ => match std::fs::remove_file(path) {
diff --git a/proxmox-sys/src/fs/dir.rs b/proxmox-sys/src/fs/dir.rs
index c39b9fdf..8e75111a 100644
--- a/proxmox-sys/src/fs/dir.rs
+++ b/proxmox-sys/src/fs/dir.rs
@@ -14,7 +14,7 @@ use crate::fs::{fchown, CreateOptions};
 /// Creates directory at the provided path with specified ownership.
 ///
 /// Errors if the directory already exists.
-pub fn create_dir<P: AsRef<Path>>(path: P, options: CreateOptions) -> Result<(), nix::Error> {
+pub fn create_dir<P: AsRef<Path>>(path: P, options: &CreateOptions) -> Result<(), nix::Error> {
     // clippy bug?: from_bits_truncate is actually a const fn...
     #[allow(clippy::or_fun_call)]
     let mode: stat::Mode = options
@@ -87,7 +87,7 @@ pub fn ensure_dir_exists<P: AsRef<Path>>(
 /// create_path(
 ///     "/var/lib/mytool/wwwdata",
 ///     None,
-///     Some(CreateOptions::new()
+///     Some(&CreateOptions::new()
 ///         .perm(Mode::from_bits(0o777).unwrap())
 ///         .owner(Uid::from_raw(33))
 ///     ),
@@ -97,16 +97,16 @@ pub fn ensure_dir_exists<P: AsRef<Path>>(
 /// ```
 pub fn create_path<P: AsRef<Path>>(
     path: P,
-    intermediate_opts: Option<CreateOptions>,
-    final_opts: Option<CreateOptions>,
+    intermediate_opts: Option<&CreateOptions>,
+    final_opts: Option<&CreateOptions>,
 ) -> Result<bool, Error> {
     create_path_do(path.as_ref(), intermediate_opts, final_opts)
 }
 
 fn create_path_do(
     path: &Path,
-    intermediate_opts: Option<CreateOptions>,
-    final_opts: Option<CreateOptions>,
+    intermediate_opts: Option<&CreateOptions>,
+    final_opts: Option<&CreateOptions>,
 ) -> Result<bool, Error> {
     use std::path::Component;
 
@@ -146,8 +146,8 @@ fn create_path_do(
 fn create_path_at_do(
     mut at: OwnedFd,
     mut iter: std::iter::Peekable<std::path::Components>,
-    intermediate_opts: Option<CreateOptions>,
-    final_opts: Option<CreateOptions>,
+    intermediate_opts: Option<&CreateOptions>,
+    final_opts: Option<&CreateOptions>,
 ) -> Result<bool, Error> {
     let mut created = false;
     loop {
@@ -206,9 +206,9 @@ mod tests {
     fn test_create_path() {
         create_path(
             "testdir/testsub/testsub2/testfinal",
-            Some(CreateOptions::new().perm(stat::Mode::from_bits_truncate(0o755))),
+            Some(&CreateOptions::new().perm(stat::Mode::from_bits_truncate(0o755))),
             Some(
-                CreateOptions::new()
+                &CreateOptions::new()
                     .owner(nix::unistd::Uid::effective())
                     .group(nix::unistd::Gid::effective()),
             ),
diff --git a/proxmox-sys/src/fs/file.rs b/proxmox-sys/src/fs/file.rs
index ac513891..41915ebb 100644
--- a/proxmox-sys/src/fs/file.rs
+++ b/proxmox-sys/src/fs/file.rs
@@ -128,7 +128,7 @@ pub fn file_read_firstline<P: AsRef<Path>>(path: P) -> Result<String, Error> {
 /// a RawFd and PathBuf for it
 pub fn make_tmp_file<P: AsRef<Path>>(
     path: P,
-    options: CreateOptions,
+    options: &CreateOptions,
 ) -> Result<(File, PathBuf), Error> {
     let path = path.as_ref();
 
@@ -159,7 +159,7 @@ pub fn make_tmp_file<P: AsRef<Path>>(
 pub fn replace_file<P: AsRef<Path>>(
     path: P,
     data: &[u8],
-    options: CreateOptions,
+    options: &CreateOptions,
     fsync: bool,
 ) -> Result<(), Error> {
     let (fd, tmp_path) = make_tmp_file(&path, options)?;
@@ -204,7 +204,7 @@ pub fn atomic_open_or_create_file<P: AsRef<Path>>(
     path: P,
     mut oflag: OFlag,
     initial_data: &[u8],
-    options: CreateOptions,
+    options: &CreateOptions,
     fsync: bool,
 ) -> Result<File, Error> {
     let path = path.as_ref();
@@ -410,7 +410,7 @@ pub fn open_file_locked<P: AsRef<Path>>(
     path: P,
     timeout: Duration,
     exclusive: bool,
-    options: CreateOptions,
+    options: &CreateOptions,
 ) -> Result<File, Error> {
     let path = path.as_ref();
 
diff --git a/proxmox-sys/src/logrotate.rs b/proxmox-sys/src/logrotate.rs
index 704a18ce..af09723f 100644
--- a/proxmox-sys/src/logrotate.rs
+++ b/proxmox-sys/src/logrotate.rs
@@ -64,7 +64,7 @@ impl LogRotate {
         options: &CreateOptions,
     ) -> Result<(), Error> {
         let mut source = File::open(source_path)?;
-        let (fd, tmp_path) = make_tmp_file(target_path, options.clone())?;
+        let (fd, tmp_path) = make_tmp_file(target_path, options)?;
         let target = unsafe { File::from_raw_fd(fd.into_raw_fd()) };
         let mut encoder = match zstd::stream::write::Encoder::new(target, 0) {
             Ok(encoder) => encoder,
diff --git a/proxmox-system-management-api/src/dns/resolv_conf.rs b/proxmox-system-management-api/src/dns/resolv_conf.rs
index 50419c50..4016e777 100644
--- a/proxmox-system-management-api/src/dns/resolv_conf.rs
+++ b/proxmox-system-management-api/src/dns/resolv_conf.rs
@@ -138,7 +138,7 @@ pub fn update_dns(
         data.push_str(&options);
     }
 
-    replace_file(RESOLV_CONF_FN, data.as_bytes(), CreateOptions::new(), true)?;
+    replace_file(RESOLV_CONF_FN, data.as_bytes(), &CreateOptions::new(), true)?;
 
     Ok(())
 }
-- 
2.39.2


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [pbs-devel] [PATCH proxmox v2] sys: pass create options as reference
  2024-05-29  9:55 [pbs-devel] [PATCH proxmox v2] sys: pass create options as reference Dietmar Maurer
@ 2024-05-29 10:22 ` Fabian Grünbichler
  2024-05-29 10:48   ` Dietmar Maurer
  2024-05-29 12:44   ` Dietmar Maurer
  0 siblings, 2 replies; 5+ messages in thread
From: Fabian Grünbichler @ 2024-05-29 10:22 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Dietmar Maurer


> Dietmar Maurer <dietmar@proxmox.com> hat am 29.05.2024 11:55 CEST geschrieben:
> 
>  
> Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
> ---
> 
> Changes in v2: change everywhere, so "cargo build --all --all-features" works again

what about proxmox-backup (and potentially other crates using it)?

I guess we could make a note for the next proxmox-sys bump we do for other reasons, but bumping half the world just for this seems a lot of work for little gain?

> 
>  proxmox-acme-api/src/account_config.rs        |  2 +-
>  proxmox-acme-api/src/config.rs                |  2 +-
>  proxmox-openid/src/auth_state.rs              |  4 ++--
>  .../src/filesystem_helpers.rs                 | 10 +++++-----
>  proxmox-rest-server/src/api_config.rs         |  4 ++--
>  proxmox-rest-server/src/daemon.rs             |  2 +-
>  proxmox-rest-server/src/file_logger.rs        |  2 +-
>  proxmox-rest-server/src/lib.rs                |  2 +-
>  proxmox-rest-server/src/worker_task.rs        | 10 +++++-----
>  proxmox-rrd/src/cache.rs                      |  4 ++--
>  proxmox-rrd/src/cache/journal.rs              |  4 ++--
>  proxmox-rrd/src/cache/rrd_map.rs              |  4 ++--
>  proxmox-rrd/src/rrd.rs                        |  2 +-
>  proxmox-subscription/src/files.rs             |  4 ++--
>  proxmox-sys/src/fs/dir.rs                     | 20 +++++++++----------
>  proxmox-sys/src/fs/file.rs                    |  8 ++++----
>  proxmox-sys/src/logrotate.rs                  |  2 +-
>  .../src/dns/resolv_conf.rs                    |  2 +-
>  18 files changed, 44 insertions(+), 44 deletions(-)
> 

[..]

> diff --git a/proxmox-subscription/src/files.rs b/proxmox-subscription/src/files.rs
> index 37008f4a..f0ff9bf4 100644
> --- a/proxmox-subscription/src/files.rs
> +++ b/proxmox-subscription/src/files.rs
> @@ -127,7 +127,7 @@ pub fn write_subscription<P: AsRef<Path>>(
>          format!("{}\n{}\n{}\n", info.key.as_ref().unwrap(), csum, encoded)
>      };
>  
> -    replace_file(path, raw.as_bytes(), file_opts, true)?;
> +    replace_file(path, raw.as_bytes(), &file_opts, true)?;
>  
>      Ok(())
>  }
> @@ -152,7 +152,7 @@ pub fn update_apt_auth<P: AsRef<Path>>(

this at least itself takes it as non-ref, shouldn't that be updated as well then?

>              let conf = format!("machine {url}\n login {}\n password {}\n", key, password,);
>  
>              // we use a namespaced .conf file, so just overwrite..
> -            replace_file(path, conf.as_bytes(), file_opts, true)
> +            replace_file(path, conf.as_bytes(), &file_opts, true)
>                  .map_err(|e| format_err!("Error saving apt auth config - {}", e))?;
>          }
>          _ => match std::fs::remove_file(path) {


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [pbs-devel] [PATCH proxmox v2] sys: pass create options as reference
  2024-05-29 10:22 ` Fabian Grünbichler
@ 2024-05-29 10:48   ` Dietmar Maurer
  2024-05-29 12:44   ` Dietmar Maurer
  1 sibling, 0 replies; 5+ messages in thread
From: Dietmar Maurer @ 2024-05-29 10:48 UTC (permalink / raw)
  To: Fabian Grünbichler, Proxmox Backup Server development discussion

> what about proxmox-backup (and potentially other crates using it)?

step by step ...

I will not touch proxmox-backup until we agree that we want to do it this way...


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [pbs-devel] [PATCH proxmox v2] sys: pass create options as reference
  2024-05-29 10:22 ` Fabian Grünbichler
  2024-05-29 10:48   ` Dietmar Maurer
@ 2024-05-29 12:44   ` Dietmar Maurer
  2024-06-03 12:21     ` Fabian Grünbichler
  1 sibling, 1 reply; 5+ messages in thread
From: Dietmar Maurer @ 2024-05-29 12:44 UTC (permalink / raw)
  To: Fabian Grünbichler, Proxmox Backup Server development discussion

> I guess we could make a note for the next proxmox-sys bump we do for other reasons, but bumping half the world just for this seems a lot of work for little gain?

This is just a cleanup, so it is not really required. But if we delay all cleanups, cleanup will take a long time (or will never happen ...)

But yes, we want to split this crate anyways, so lets wait for that.


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [pbs-devel] [PATCH proxmox v2] sys: pass create options as reference
  2024-05-29 12:44   ` Dietmar Maurer
@ 2024-06-03 12:21     ` Fabian Grünbichler
  0 siblings, 0 replies; 5+ messages in thread
From: Fabian Grünbichler @ 2024-06-03 12:21 UTC (permalink / raw)
  To: Dietmar Maurer, Proxmox Backup Server development discussion

On May 29, 2024 2:44 pm, Dietmar Maurer wrote:
>> I guess we could make a note for the next proxmox-sys bump we do for other reasons, but bumping half the world just for this seems a lot of work for little gain?
> 
> This is just a cleanup, so it is not really required. But if we delay all cleanups, cleanup will take a long time (or will never happen ...)

we don't need to delay all cleanups (and I hope we don't :)), but for
those that contain breaking changes that cause a lot of churn, we need
to decide whether the churn *now* is worth it. and most of the time,
that decision will incorporate the benefits of the change (if it is
mostly cosmetic, then the scales will tip towards "let's put it on a
list of things to do when we have unavoidable/important breaking
changes").

improving our processes so we don't forget to incorporate such small
cleanups when the next good opportunity arrives would also be good
- because we definitely don't want to end up with them never happening!

> But yes, we want to split this crate anyways, so lets wait for that.

ack. issues like this are definitely an argument for crates with limited
scope and clear boundaries ;)


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-06-03 12:21 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-29  9:55 [pbs-devel] [PATCH proxmox v2] sys: pass create options as reference Dietmar Maurer
2024-05-29 10:22 ` Fabian Grünbichler
2024-05-29 10:48   ` Dietmar Maurer
2024-05-29 12:44   ` Dietmar Maurer
2024-06-03 12:21     ` Fabian Grünbichler

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal