[pve-devel] [PATCH proxmox-apt 5/5] add type DebianCodename

Fabian Ebner f.ebner at proxmox.com
Thu Jul 29 14:25:52 CEST 2021


which allows to get rid of an possible error with check_suites, and
easily detect unexpected values with get_current_release_codename.

The check_repos function needs to be adapted, since the type does
not include suite names like oldstable,experimental,etc.

Signed-off-by: Fabian Ebner <f.ebner at proxmox.com>
---
 src/repositories/file.rs    |  73 ++++++++++++-------------
 src/repositories/mod.rs     |  18 +++----
 src/repositories/release.rs | 103 ++++++++++++++++++++++++++++--------
 tests/repositories.rs       |  18 +++----
 4 files changed, 133 insertions(+), 79 deletions(-)

diff --git a/src/repositories/file.rs b/src/repositories/file.rs
index fe994f7..3df59db 100644
--- a/src/repositories/file.rs
+++ b/src/repositories/file.rs
@@ -1,11 +1,11 @@
-use std::convert::TryFrom;
+use std::convert::{TryFrom, TryInto};
 use std::fmt::Display;
 use std::path::{Path, PathBuf};
 
-use anyhow::{bail, format_err, Error};
+use anyhow::{format_err, Error};
 use serde::{Deserialize, Serialize};
 
-use crate::repositories::release::DEBIAN_SUITES;
+use crate::repositories::release::DebianCodename;
 use crate::repositories::repository::{
     APTRepository, APTRepositoryFileType, APTRepositoryPackageType,
 };
@@ -300,7 +300,7 @@ impl APTRepositoryFile {
 
     /// Checks if old or unstable suites are configured and also that the
     /// `stable` keyword is not used.
-    pub fn check_suites(&self, current_suite: &str) -> Result<Vec<APTRepositoryInfo>, Error> {
+    pub fn check_suites(&self, current_codename: DebianCodename) -> Vec<APTRepositoryInfo> {
         let mut infos = vec![];
 
         for (n, repo) in self.repositories.iter().enumerate() {
@@ -308,60 +308,55 @@ impl APTRepositoryFile {
                 continue;
             }
 
-            let mut add_info = |kind, message| {
+            let mut add_info = |kind: &str, message| {
                 infos.push(APTRepositoryInfo {
                     path: self.path.clone(),
                     index: n,
                     property: Some("Suites".to_string()),
-                    kind,
+                    kind: kind.to_string(),
                     message,
                 })
             };
 
-            let current_index = match DEBIAN_SUITES
-                .iter()
-                .position(|&suite| suite == current_suite)
-            {
-                Some(index) => index,
-                None => bail!("unknown release {}", current_suite),
-            };
+            let message_old = |suite| format!("old suite '{}' configured!", suite);
+            let message_new =
+                |suite| format!("suite '{}' should not be used in production!", suite);
+            let message_stable = "use the name of the stable distribution instead of 'stable'!";
 
             for suite in repo.suites.iter() {
                 let base_suite = suite_variant(suite).0;
 
-                if base_suite == "stable" {
-                    add_info(
-                        "warning".to_string(),
-                        "use the name of the stable distribution instead of 'stable'!".to_string(),
-                    );
-                }
-
-                if let Some(n) = DEBIAN_SUITES.iter().position(|&suite| suite == base_suite) {
-                    if n < current_index {
-                        add_info(
-                            "warning".to_string(),
-                            format!("old suite '{}' configured!", base_suite),
-                        );
+                match base_suite {
+                    "oldoldstable" | "oldstable" => {
+                        add_info("warning", message_old(base_suite));
                     }
-
-                    if n == current_index + 1 {
-                        add_info(
-                            "ignore-pre-upgrade-warning".to_string(),
-                            format!("suite '{}' should not be used in production!", base_suite),
-                        );
+                    "testing" | "unstable" | "experimental" | "sid" => {
+                        add_info("warning", message_new(base_suite));
                     }
-
-                    if n > current_index + 1 {
-                        add_info(
-                            "warning".to_string(),
-                            format!("suite '{}' should not be used in production!", base_suite),
-                        );
+                    "stable" => {
+                        add_info("warning", message_stable.to_string());
                     }
+                    _ => (),
+                };
+
+                let codename: DebianCodename = match base_suite.try_into() {
+                    Ok(codename) => codename,
+                    Err(_) => continue,
+                };
+
+                if codename < current_codename {
+                    add_info("warning", message_old(base_suite));
+                }
+
+                if Some(codename) == current_codename.next() {
+                    add_info("ignore-pre-upgrade-warning", message_new(base_suite));
+                } else if codename > current_codename {
+                    add_info("warning", message_new(base_suite));
                 }
             }
         }
 
-        Ok(infos)
+        infos
     }
 
     /// Checks for official URIs.
diff --git a/src/repositories/mod.rs b/src/repositories/mod.rs
index 8f5500e..88b515d 100644
--- a/src/repositories/mod.rs
+++ b/src/repositories/mod.rs
@@ -12,7 +12,7 @@ mod file;
 pub use file::{APTRepositoryFile, APTRepositoryFileError, APTRepositoryInfo};
 
 mod release;
-pub use release::get_current_release_codename;
+pub use release::{get_current_release_codename, DebianCodename};
 
 mod standard;
 pub use standard::{APTRepositoryHandle, APTStandardRepository};
@@ -51,25 +51,25 @@ fn common_digest(files: &[APTRepositoryFile]) -> [u8; 32] {
 /// `badge` for official URIs.
 pub fn check_repositories(
     files: &[APTRepositoryFile],
-    current_suite: &str,
-) -> Result<Vec<APTRepositoryInfo>, Error> {
+    current_suite: DebianCodename,
+) -> Vec<APTRepositoryInfo> {
     let mut infos = vec![];
 
     for file in files.iter() {
-        infos.append(&mut file.check_suites(current_suite)?);
+        infos.append(&mut file.check_suites(current_suite));
         infos.append(&mut file.check_uris());
     }
 
-    Ok(infos)
+    infos
 }
 
 /// Get the repository associated to the handle and the path where it is usually configured.
 pub fn get_standard_repository(
     handle: APTRepositoryHandle,
     product: &str,
-    suite: &str,
+    suite: DebianCodename,
 ) -> (APTRepository, String) {
-    let repo = handle.to_repository(product, &suite);
+    let repo = handle.to_repository(product, &suite.to_string());
     let path = handle.path(product);
 
     (repo, path)
@@ -80,7 +80,7 @@ pub fn get_standard_repository(
 pub fn standard_repositories(
     files: &[APTRepositoryFile],
     product: &str,
-    suite: &str,
+    suite: DebianCodename,
 ) -> Vec<APTStandardRepository> {
     let mut result = vec![
         APTStandardRepository::from(APTRepositoryHandle::Enterprise),
@@ -104,7 +104,7 @@ pub fn standard_repositories(
                     continue;
                 }
 
-                if repo.is_referenced_repository(entry.handle, product, suite) {
+                if repo.is_referenced_repository(entry.handle, product, &suite.to_string()) {
                     entry.status = Some(repo.enabled);
                 }
             }
diff --git a/src/repositories/release.rs b/src/repositories/release.rs
index 688f038..dbbc699 100644
--- a/src/repositories/release.rs
+++ b/src/repositories/release.rs
@@ -1,29 +1,88 @@
+use std::convert::{TryFrom, TryInto};
+use std::fmt::Display;
+use std::io::{BufRead, BufReader};
+
 use anyhow::{bail, format_err, Error};
 
-use std::io::{BufRead, BufReader};
+/// The code names of Debian releases. Does not include `sid`.
+#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
+pub enum DebianCodename {
+    Lenny = 5,
+    Squeeze,
+    Wheezy,
+    Jessie,
+    Stretch,
+    Buster,
+    Bullseye,
+    Bookworm,
+    Trixie,
+}
 
-/// The suites of Debian releases, ordered chronologically, with variable releases
-/// like 'oldstable' and 'testing' ordered at the extremes. Does not include 'stable'.
-pub const DEBIAN_SUITES: [&str; 15] = [
-    "oldoldstable",
-    "oldstable",
-    "lenny",
-    "squeeze",
-    "wheezy",
-    "jessie",
-    "stretch",
-    "buster",
-    "bullseye",
-    "bookworm",
-    "trixie",
-    "sid",
-    "testing",
-    "unstable",
-    "experimental",
-];
+impl DebianCodename {
+    pub fn next(&self) -> Option<Self> {
+        match (*self as u8 + 1).try_into() {
+            Ok(codename) => Some(codename),
+            Err(_) => None,
+        }
+    }
+}
+
+impl TryFrom<&str> for DebianCodename {
+    type Error = Error;
+
+    fn try_from(string: &str) -> Result<Self, Error> {
+        match string {
+            "lenny" => Ok(DebianCodename::Lenny),
+            "squeeze" => Ok(DebianCodename::Squeeze),
+            "wheezy" => Ok(DebianCodename::Wheezy),
+            "jessie" => Ok(DebianCodename::Jessie),
+            "stretch" => Ok(DebianCodename::Stretch),
+            "buster" => Ok(DebianCodename::Buster),
+            "bullseye" => Ok(DebianCodename::Bullseye),
+            "bookworm" => Ok(DebianCodename::Bookworm),
+            "trixie" => Ok(DebianCodename::Trixie),
+            _ => bail!("unknown Debian code name '{}'", string),
+        }
+    }
+}
+
+impl TryFrom<u8> for DebianCodename {
+    type Error = Error;
+
+    fn try_from(number: u8) -> Result<Self, Error> {
+        match number {
+            5 => Ok(DebianCodename::Lenny),
+            6 => Ok(DebianCodename::Squeeze),
+            7 => Ok(DebianCodename::Wheezy),
+            8 => Ok(DebianCodename::Jessie),
+            9 => Ok(DebianCodename::Stretch),
+            10 => Ok(DebianCodename::Buster),
+            11 => Ok(DebianCodename::Bullseye),
+            12 => Ok(DebianCodename::Bookworm),
+            13 => Ok(DebianCodename::Trixie),
+            _ => bail!("unknown Debian release number '{}'", number),
+        }
+    }
+}
+
+impl Display for DebianCodename {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            DebianCodename::Lenny => write!(f, "lenny"),
+            DebianCodename::Squeeze => write!(f, "squeeze"),
+            DebianCodename::Wheezy => write!(f, "wheezy"),
+            DebianCodename::Jessie => write!(f, "jessie"),
+            DebianCodename::Stretch => write!(f, "stretch"),
+            DebianCodename::Buster => write!(f, "buster"),
+            DebianCodename::Bullseye => write!(f, "bullseye"),
+            DebianCodename::Bookworm => write!(f, "bookworm"),
+            DebianCodename::Trixie => write!(f, "trixie"),
+        }
+    }
+}
 
 /// Read the `VERSION_CODENAME` from `/etc/os-release`.
-pub fn get_current_release_codename() -> Result<String, Error> {
+pub fn get_current_release_codename() -> Result<DebianCodename, Error> {
     let raw = std::fs::read("/etc/os-release")
         .map_err(|err| format_err!("unable to read '/etc/os-release' - {}", err))?;
 
@@ -34,7 +93,7 @@ pub fn get_current_release_codename() -> Result<String, Error> {
 
         if let Some(codename) = line.strip_prefix("VERSION_CODENAME=") {
             let codename = codename.trim_matches(&['"', '\''][..]);
-            return Ok(codename.to_string());
+            return codename.try_into();
         }
     }
 
diff --git a/tests/repositories.rs b/tests/repositories.rs
index a9a7b96..d79ea72 100644
--- a/tests/repositories.rs
+++ b/tests/repositories.rs
@@ -6,7 +6,7 @@ use proxmox_apt::config::APTConfig;
 
 use proxmox_apt::repositories::{
     check_repositories, get_current_release_codename, standard_repositories, APTRepositoryFile,
-    APTRepositoryHandle, APTRepositoryInfo, APTStandardRepository,
+    APTRepositoryHandle, APTRepositoryInfo, APTStandardRepository, DebianCodename,
 };
 
 #[test]
@@ -187,7 +187,7 @@ fn test_check_repositories() -> Result<(), Error> {
     let mut file = APTRepositoryFile::new(&absolute_suite_list)?.unwrap();
     file.parse()?;
 
-    let infos = check_repositories(&vec![file], "bullseye")?;
+    let infos = check_repositories(&vec![file], DebianCodename::Bullseye);
 
     assert_eq!(infos.is_empty(), true);
     let pve_list = read_dir.join("pve.list");
@@ -212,7 +212,7 @@ fn test_check_repositories() -> Result<(), Error> {
     }
     expected_infos.sort();
 
-    let mut infos = check_repositories(&vec![file], "bullseye")?;
+    let mut infos = check_repositories(&vec![file], DebianCodename::Bullseye);
     infos.sort();
 
     assert_eq!(infos, expected_infos);
@@ -278,7 +278,7 @@ fn test_check_repositories() -> Result<(), Error> {
     }
     expected_infos.sort();
 
-    let mut infos = check_repositories(&vec![file], "bullseye")?;
+    let mut infos = check_repositories(&vec![file], DebianCodename::Bullseye);
     infos.sort();
 
     assert_eq!(infos, expected_infos);
@@ -337,7 +337,7 @@ fn test_standard_repositories() -> Result<(), Error> {
     let mut file = APTRepositoryFile::new(&absolute_suite_list)?.unwrap();
     file.parse()?;
 
-    let std_repos = standard_repositories(&vec![file], "pve", "bullseye");
+    let std_repos = standard_repositories(&vec![file], "pve", DebianCodename::Bullseye);
 
     assert_eq!(std_repos, expected);
 
@@ -347,14 +347,14 @@ fn test_standard_repositories() -> Result<(), Error> {
 
     let file_vec = vec![file];
 
-    let std_repos = standard_repositories(&file_vec, "pbs", "bullseye");
+    let std_repos = standard_repositories(&file_vec, "pbs", DebianCodename::Bullseye);
 
     assert_eq!(&std_repos, &expected[0..=2]);
 
     expected[0].status = Some(false);
     expected[1].status = Some(true);
 
-    let std_repos = standard_repositories(&file_vec, "pve", "bullseye");
+    let std_repos = standard_repositories(&file_vec, "pve", DebianCodename::Bullseye);
 
     assert_eq!(std_repos, expected);
 
@@ -368,7 +368,7 @@ fn test_standard_repositories() -> Result<(), Error> {
     expected[1].status = Some(true);
     expected[2].status = Some(false);
 
-    let std_repos = standard_repositories(&file_vec, "pve", "bullseye");
+    let std_repos = standard_repositories(&file_vec, "pve", DebianCodename::Bullseye);
 
     assert_eq!(std_repos, expected);
 
@@ -379,7 +379,7 @@ fn test_standard_repositories() -> Result<(), Error> {
 fn test_get_current_release_codename() -> Result<(), Error> {
     let codename = get_current_release_codename()?;
 
-    assert_eq!(&codename, "bullseye");
+    assert!(codename == DebianCodename::Bullseye);
 
     Ok(())
 }
-- 
2.30.2






More information about the pve-devel mailing list