all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Fabian Ebner <f.ebner@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH proxmox-apt 5/5] add type DebianCodename
Date: Thu, 29 Jul 2021 14:25:52 +0200	[thread overview]
Message-ID: <20210729122554.148980-6-f.ebner@proxmox.com> (raw)
In-Reply-To: <20210729122554.148980-1-f.ebner@proxmox.com>

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@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





  parent reply	other threads:[~2021-07-29 12:26 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-29 12:25 [pve-devel] [PATCH-SERIES proxmox-apt/pve-rs] better detection of standard repositories Fabian Ebner
2021-07-29 12:25 ` [pve-devel] [PATCH proxmox-apt 1/5] standard repos: add suite parameter for stricter detection Fabian Ebner
2021-07-29 12:25 ` [pve-devel] [PATCH proxmox-apt 2/5] repo: make suite_variant helper more general Fabian Ebner
2021-07-29 12:25 ` [pve-devel] [PATCH proxmox-apt 3/5] check repos: have caller specify the current suite Fabian Ebner
2021-07-29 12:25 ` [pve-devel] [PATCH proxmox-apt 4/5] repo: remove has_suite_variant helper Fabian Ebner
2021-07-29 12:25 ` Fabian Ebner [this message]
2021-07-29 12:25 ` [pve-devel] [PATCH pve-rs 1/2] apt: repos: adapt to back-end changes Fabian Ebner
2021-07-29 12:25 ` [pve-devel] [PATCH pve-rs 2/2] apt: repos: adapt to further " Fabian Ebner
2021-07-30  8:47 ` [pve-devel] applied-series: Re: [PATCH-SERIES proxmox-apt/pve-rs] better detection of standard repositories Thomas Lamprecht

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210729122554.148980-6-f.ebner@proxmox.com \
    --to=f.ebner@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal