[pbs-devel] [PATCH proxmox 6/6] apt: avoid global apt config

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

From: Dietmar Maurer <dietmar@proxmox.com>

Because it was only used for the test setup. Instead, we simply
add an apt_lists_dir parameter where we need it.

Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
---
 proxmox-apt/src/api.rs                     |  5 +++-
 proxmox-apt/src/config.rs                  | 35 ----------------------
 proxmox-apt/src/lib.rs                     |  2 +-
 proxmox-apt/src/repositories/file.rs       |  6 ++--
 proxmox-apt/src/repositories/mod.rs        |  5 ++--
 proxmox-apt/src/repositories/repository.rs | 15 +++++-----
 proxmox-apt/tests/repositories.rs          | 24 +++++----------
 7 files changed, 25 insertions(+), 67 deletions(-)
 delete mode 100644 proxmox-apt/src/config.rs

diff --git a/proxmox-apt/src/api.rs b/proxmox-apt/src/api.rs
index af01048e..14dfb53b 100644
--- a/proxmox-apt/src/api.rs
+++ b/proxmox-apt/src/api.rs
@@ -1,4 +1,5 @@
 // API function that work without feature "cache"
+use std::path::Path;
 
 use anyhow::{bail, Error};
 
@@ -27,11 +28,13 @@ pub fn get_changelog(options: &APTGetChangelogOptions) -> Result<String, Error>
 
 /// Get APT repository information.
 pub fn list_repositories(product: &str) -> Result<APTRepositoriesResult, Error> {
+    let apt_lists_dir = Path::new("/var/lib/apt/lists");
+
     let (files, errors, digest) = crate::repositories::repositories()?;
 
     let suite = crate::repositories::get_current_release_codename()?;
 
-    let infos = crate::repositories::check_repositories(&files, suite);
+    let infos = crate::repositories::check_repositories(&files, suite, apt_lists_dir);
     let standard_repos = crate::repositories::standard_repositories(&files, product, suite);
 
     Ok(APTRepositoriesResult {
diff --git a/proxmox-apt/src/config.rs b/proxmox-apt/src/config.rs
deleted file mode 100644
index fcb66cbb..00000000
--- a/proxmox-apt/src/config.rs
+++ /dev/null
@@ -1,35 +0,0 @@
-use once_cell::sync::OnceCell;
-
-static GLOBAL_CONFIG: OnceCell<APTConfig> = OnceCell::new();
-
-/// APT configuration variables.
-pub struct APTConfig {
-    /// Dir::State
-    pub dir_state: String,
-    /// Dir::State::Lists
-    pub dir_state_lists: String,
-}
-
-impl APTConfig {
-    /// Create a new configuration overriding the provided values.
-    pub fn new(dir_state: Option<&str>, dir_state_lists: Option<&str>) -> Self {
-        Self {
-            dir_state: dir_state.unwrap_or("/var/lib/apt/").to_string(),
-            dir_state_lists: dir_state_lists.unwrap_or("lists/").to_string(),
-        }
-    }
-}
-
-/// Get the configuration.
-///
-/// Initializes with default values if init() wasn't called before.
-pub fn get() -> &'static APTConfig {
-    GLOBAL_CONFIG.get_or_init(|| APTConfig::new(None, None))
-}
-
-/// Initialize the configuration.
-///
-/// Only has an effect if no init() or get() has been called yet.
-pub fn init(config: APTConfig) -> &'static APTConfig {
-    GLOBAL_CONFIG.get_or_init(|| config)
-}
diff --git a/proxmox-apt/src/lib.rs b/proxmox-apt/src/lib.rs
index f25ac90b..cd504739 100644
--- a/proxmox-apt/src/lib.rs
+++ b/proxmox-apt/src/lib.rs
@@ -9,6 +9,6 @@ pub mod cache;
 mod cache_api;
 #[cfg(feature = "cache")]
 pub use cache_api::{get_package_versions, list_available_apt_update, update_database};
-pub mod config;
+
 pub mod deb822;
 pub mod repositories;
diff --git a/proxmox-apt/src/repositories/file.rs b/proxmox-apt/src/repositories/file.rs
index 21f612ef..078f6407 100644
--- a/proxmox-apt/src/repositories/file.rs
+++ b/proxmox-apt/src/repositories/file.rs
@@ -59,7 +59,7 @@ pub trait APTRepositoryFileImpl {
     fn check_suites(&self, current_codename: DebianCodename) -> Vec<APTRepositoryInfo>;
 
     /// Checks for official URIs.
-    fn check_uris(&self) -> Vec<APTRepositoryInfo>;
+    fn check_uris(&self, apt_lists_dir: &Path) -> Vec<APTRepositoryInfo>;
 }
 
 impl APTRepositoryFileImpl for APTRepositoryFile {
@@ -357,7 +357,7 @@ impl APTRepositoryFileImpl for APTRepositoryFile {
         infos
     }
 
-    fn check_uris(&self) -> Vec<APTRepositoryInfo> {
+    fn check_uris(&self, apt_lists_dir: &Path) -> Vec<APTRepositoryInfo> {
         let mut infos = vec![];
 
         let path = match &self.path {
@@ -366,7 +366,7 @@ impl APTRepositoryFileImpl for APTRepositoryFile {
         };
 
         for (n, repo) in self.repositories.iter().enumerate() {
-            let mut origin = match repo.get_cached_origin() {
+            let mut origin = match repo.get_cached_origin(apt_lists_dir) {
                 Ok(option) => option,
                 Err(_) => None,
             };
diff --git a/proxmox-apt/src/repositories/mod.rs b/proxmox-apt/src/repositories/mod.rs
index 7768a47a..451b356b 100644
--- a/proxmox-apt/src/repositories/mod.rs
+++ b/proxmox-apt/src/repositories/mod.rs
@@ -1,5 +1,5 @@
 use std::collections::BTreeMap;
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 
 use anyhow::{bail, Error};
 
@@ -56,12 +56,13 @@ fn common_digest(files: &[APTRepositoryFile]) -> ConfigDigest {
 pub fn check_repositories(
     files: &[APTRepositoryFile],
     current_suite: DebianCodename,
+    apt_lists_dir: &Path,
 ) -> Vec<APTRepositoryInfo> {
     let mut infos = vec![];
 
     for file in files.iter() {
         infos.append(&mut file.check_suites(current_suite));
-        infos.append(&mut file.check_uris());
+        infos.append(&mut file.check_uris(apt_lists_dir));
     }
 
     infos
diff --git a/proxmox-apt/src/repositories/repository.rs b/proxmox-apt/src/repositories/repository.rs
index 596c6385..62ad49d8 100644
--- a/proxmox-apt/src/repositories/repository.rs
+++ b/proxmox-apt/src/repositories/repository.rs
@@ -1,5 +1,5 @@
 use std::io::{BufRead, BufReader, Write};
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 
 use anyhow::{bail, format_err, Error};
 
@@ -33,7 +33,7 @@ pub trait APTRepositoryImpl {
     fn origin_from_uris(&self) -> Option<String>;
 
     /// Get the `Origin:` value from a cached InRelease file.
-    fn get_cached_origin(&self) -> Result<Option<String>, Error>;
+    fn get_cached_origin(&self, apt_lists_dir: &Path) -> Result<Option<String>, Error>;
 
     /// Writes a repository in the corresponding format followed by a blank.
     ///
@@ -163,13 +163,13 @@ impl APTRepositoryImpl for APTRepository {
         None
     }
 
-    fn get_cached_origin(&self) -> Result<Option<String>, Error> {
+    fn get_cached_origin(&self, apt_lists_dir: &Path) -> Result<Option<String>, Error> {
         for uri in self.uris.iter() {
             for suite in self.suites.iter() {
-                let mut file = release_filename(uri, suite, false);
+                let mut file = release_filename(apt_lists_dir, uri, suite, false);
 
                 if !file.exists() {
-                    file = release_filename(uri, suite, true);
+                    file = release_filename(apt_lists_dir, uri, suite, true);
                     if !file.exists() {
                         continue;
                     }
@@ -206,9 +206,8 @@ impl APTRepositoryImpl for APTRepository {
 }
 
 /// Get the path to the cached (In)Release file.
-fn release_filename(uri: &str, suite: &str, detached: bool) -> PathBuf {
-    let mut path = PathBuf::from(&crate::config::get().dir_state);
-    path.push(&crate::config::get().dir_state_lists);
+fn release_filename(apt_lists_dir: &Path, uri: &str, suite: &str, detached: bool) -> PathBuf {
+    let mut path = PathBuf::from(apt_lists_dir);
 
     let encoded_uri = uri_to_filename(uri);
     let filename = if detached { "Release" } else { "InRelease" };
diff --git a/proxmox-apt/tests/repositories.rs b/proxmox-apt/tests/repositories.rs
index e4a94525..e4efcab6 100644
--- a/proxmox-apt/tests/repositories.rs
+++ b/proxmox-apt/tests/repositories.rs
@@ -2,8 +2,6 @@ use std::path::PathBuf;
 
 use anyhow::{bail, format_err, Error};
 
-use proxmox_apt::config::APTConfig;
-
 use proxmox_apt::repositories::{
     check_repositories, get_current_release_codename, standard_repositories, DebianCodename,
 };
@@ -185,17 +183,13 @@ fn test_empty_write() -> Result<(), Error> {
 fn test_check_repositories() -> Result<(), Error> {
     let test_dir = std::env::current_dir()?.join("tests");
     let read_dir = test_dir.join("sources.list.d");
-
-    proxmox_apt::config::init(APTConfig::new(
-        Some(&test_dir.into_os_string().into_string().unwrap()),
-        None,
-    ));
+    let apt_lists_dir: PathBuf = test_dir.join("lists");
 
     let absolute_suite_list = read_dir.join("absolute_suite.list");
     let mut file = APTRepositoryFile::new(absolute_suite_list)?.unwrap();
     file.parse()?;
 
-    let infos = check_repositories(&[file], DebianCodename::Bullseye);
+    let infos = check_repositories(&[file], DebianCodename::Bullseye, &apt_lists_dir);
 
     assert!(infos.is_empty());
     let pve_list = read_dir.join("pve.list");
@@ -220,7 +214,7 @@ fn test_check_repositories() -> Result<(), Error> {
     }
     expected_infos.sort();
 
-    let mut infos = check_repositories(&[file], DebianCodename::Bullseye);
+    let mut infos = check_repositories(&[file], DebianCodename::Bullseye, &apt_lists_dir);
     infos.sort();
 
     assert_eq!(infos, expected_infos);
@@ -286,7 +280,7 @@ fn test_check_repositories() -> Result<(), Error> {
     }
     expected_infos.sort();
 
-    let mut infos = check_repositories(&[file], DebianCodename::Bullseye);
+    let mut infos = check_repositories(&[file], DebianCodename::Bullseye, &apt_lists_dir);
     infos.sort();
 
     assert_eq!(infos, expected_infos);
@@ -318,7 +312,7 @@ fn test_check_repositories() -> Result<(), Error> {
     }
     expected_infos.sort();
 
-    let mut infos = check_repositories(&[file], DebianCodename::Bullseye);
+    let mut infos = check_repositories(&[file], DebianCodename::Bullseye, &apt_lists_dir);
     infos.sort();
 
     assert_eq!(infos, expected_infos);
@@ -329,11 +323,7 @@ fn test_check_repositories() -> Result<(), Error> {
 fn test_get_cached_origin() -> Result<(), Error> {
     let test_dir = std::env::current_dir()?.join("tests");
     let read_dir = test_dir.join("sources.list.d");
-
-    proxmox_apt::config::init(APTConfig::new(
-        Some(&test_dir.into_os_string().into_string().unwrap()),
-        None,
-    ));
+    let apt_lists_dir: PathBuf = test_dir.clone().join("lists");
 
     let pve_list = read_dir.join("pve.list");
     let mut file = APTRepositoryFile::new(pve_list)?.unwrap();
@@ -351,7 +341,7 @@ fn test_get_cached_origin() -> Result<(), Error> {
     assert_eq!(file.repositories.len(), origins.len());
 
     for (n, repo) in file.repositories.iter().enumerate() {
-        assert_eq!(repo.get_cached_origin()?, origins[n]);
+        assert_eq!(repo.get_cached_origin(&apt_lists_dir)?, origins[n]);
     }
 
     Ok(())
-- 
2.39.2



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