[pve-devel] [PATCH-SERIES proxmox-apt/pve-rs] better detection of standard repositories

[pve-devel] [PATCH-SERIES proxmox-apt/pve-rs] better detection of standard repositories

[Fabian Ebner]

by also requiring the suite to match. Let the caller pass along the
current suite instead of auto-detecting, and have the backend only
handle static stuff (avoids the need for Result<>).

Also refactor check_suites in a similar fashion, so it's more uniform
and so that get_current_release_codename only needs to be called once,
by the library user.

The pve-rs patch #1 is required after proxmox-apt patch #1 is applied.
The pve-rs patch #2 is required after proxmox-apt patches #3 and #5
are applied.

I'll send the corresponding patches for pmg-rs and proxmox-backup
separately.


proxmox-apt:

Fabian Ebner (5):
  standard repos: add suite parameter for stricter detection
  repo: make suite_variant helper more general
  check repos: have caller specify the current suite
  repo: remove has_suite_variant helper
  add type DebianCodename

 src/repositories/file.rs       |  84 ++++++++++++++-------------
 src/repositories/mod.rs        |  31 +++++-----
 src/repositories/release.rs    | 103 ++++++++++++++++++++++++++-------
 src/repositories/repository.rs |  33 ++++-------
 tests/repositories.rs          |  27 ++++++---
 5 files changed, 171 insertions(+), 107 deletions(-)


pve-rs:

Fabian Ebner (2):
  apt: repos: adapt to back-end changes
  apt: repos: adapt to further back-end changes

 src/apt/repositories.rs | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

-- 
2.30.2

[pve-devel] [PATCH proxmox-apt 1/5] standard repos: add suite parameter for stricter detection

[Fabian Ebner]

Require that the suite matches too when detecting standard
repositories, since no or invalid updates will be obtained when the
suite is wrong. Thus, it should not be considered to be the same
repository.

Add the parameter for get_standard_repository too, so that the two
related calls have more similar parameters, and the detection of the
current release code name can be done once in the caller once.

This also will fix an issue with the front-end, where adding a
standard repository would end up just enabling an already present
repository with the wrong suite.

Reported-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 src/repositories/mod.rs        | 20 ++++++++++----------
 src/repositories/repository.rs | 13 +++++++++++--
 tests/repositories.rs          | 21 +++++++++++++++------
 3 files changed, 36 insertions(+), 18 deletions(-)

diff --git a/src/repositories/mod.rs b/src/repositories/mod.rs
index 7bac333..8637851 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;
-use release::get_current_release_codename;
+pub use release::get_current_release_codename;
 
 mod standard;
 pub use standard::{APTRepositoryHandle, APTStandardRepository};
@@ -60,24 +60,24 @@ pub fn check_repositories(files: &[APTRepositoryFile]) -> Result<Vec<APTReposito
     Ok(infos)
 }
 
-/// Get the repository associated to the handle and the path where its usually configured.
+/// Get the repository associated to the handle and the path where it is usually configured.
 pub fn get_standard_repository(
     handle: APTRepositoryHandle,
     product: &str,
-) -> Result<(APTRepository, String), Error> {
-    let suite = get_current_release_codename()?;
-
+    suite: &str,
+) -> (APTRepository, String) {
     let repo = handle.to_repository(product, &suite);
     let path = handle.path(product);
 
-    Ok((repo, path))
+    (repo, path)
 }
 
-/// Return handles for standard Proxmox repositories and whether their status, where
-/// None means not configured, and Some(bool) indicates enabled or disabled
+/// Return handles for standard Proxmox repositories and their status, where
+/// `None` means not configured, and `Some(bool)` indicates enabled or disabled.
 pub fn standard_repositories(
-    product: &str,
     files: &[APTRepositoryFile],
+    product: &str,
+    suite: &str,
 ) -> Vec<APTStandardRepository> {
     let mut result = vec![
         APTStandardRepository::from(APTRepositoryHandle::Enterprise),
@@ -101,7 +101,7 @@ pub fn standard_repositories(
                     continue;
                 }
 
-                if repo.is_referenced_repository(entry.handle, product) {
+                if repo.is_referenced_repository(entry.handle, product, suite) {
                     entry.status = Some(repo.enabled);
                 }
             }
diff --git a/src/repositories/repository.rs b/src/repositories/repository.rs
index b56ec47..fc16327 100644
--- a/src/repositories/repository.rs
+++ b/src/repositories/repository.rs
@@ -270,7 +270,12 @@ impl APTRepository {
     }
 
     /// Checks if the repository is the one referenced by the handle.
-    pub fn is_referenced_repository(&self, handle: APTRepositoryHandle, product: &str) -> bool {
+    pub fn is_referenced_repository(
+        &self,
+        handle: APTRepositoryHandle,
+        product: &str,
+        suite: &str,
+    ) -> bool {
         let (package_type, handle_uris, component) = handle.info(product);
 
         let mut found_uri = false;
@@ -281,7 +286,11 @@ impl APTRepository {
             found_uri = found_uri || handle_uris.iter().any(|handle_uri| handle_uri == uri);
         }
 
-        self.types.contains(&package_type) && found_uri && self.components.contains(&component)
+        self.types.contains(&package_type)
+            && found_uri
+            // using contains would require a &String
+            && self.suites.iter().any(|self_suite| self_suite == suite)
+            && self.components.contains(&component)
     }
 
     /// Check if a variant of the given suite is configured in this repository
diff --git a/tests/repositories.rs b/tests/repositories.rs
index 6435671..4cbde60 100644
--- a/tests/repositories.rs
+++ b/tests/repositories.rs
@@ -5,8 +5,8 @@ use anyhow::{bail, format_err, Error};
 use proxmox_apt::config::APTConfig;
 
 use proxmox_apt::repositories::{
-    check_repositories, standard_repositories, APTRepositoryFile, APTRepositoryHandle,
-    APTRepositoryInfo, APTStandardRepository,
+    check_repositories, get_current_release_codename, standard_repositories, APTRepositoryFile,
+    APTRepositoryHandle, APTRepositoryInfo, APTStandardRepository,
 };
 
 #[test]
@@ -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("pve", &vec![file]);
+    let std_repos = standard_repositories(&vec![file], "pve", "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("pbs", &file_vec);
+    let std_repos = standard_repositories(&file_vec, "pbs", "bullseye");
 
     assert_eq!(&std_repos, &expected[0..=2]);
 
     expected[0].status = Some(false);
     expected[1].status = Some(true);
 
-    let std_repos = standard_repositories("pve", &file_vec);
+    let std_repos = standard_repositories(&file_vec, "pve", "bullseye");
 
     assert_eq!(std_repos, expected);
 
@@ -368,9 +368,18 @@ fn test_standard_repositories() -> Result<(), Error> {
     expected[1].status = Some(true);
     expected[2].status = Some(false);
 
-    let std_repos = standard_repositories("pve", &file_vec);
+    let std_repos = standard_repositories(&file_vec, "pve", "bullseye");
 
     assert_eq!(std_repos, expected);
 
     Ok(())
 }
+
+#[test]
+fn test_get_current_release_codename() -> Result<(), Error> {
+    let codename = get_current_release_codename()?;
+
+    assert_eq!(&codename, "bullseye");
+
+    Ok(())
+}
-- 
2.30.2

[pve-devel] [PATCH proxmox-apt 2/5] repo: make suite_variant helper more general

[Fabian Ebner]

use the first appearance of '-' or '/' to detect the variant instead
of keeping a list of possible variants, which would need to include
things like "-proposed-updates-debug".

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 src/repositories/repository.rs | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/src/repositories/repository.rs b/src/repositories/repository.rs
index fc16327..d85f063 100644
--- a/src/repositories/repository.rs
+++ b/src/repositories/repository.rs
@@ -433,16 +433,12 @@ fn host_from_uri(uri: &str) -> Option<&str> {
 }
 
 /// Splits the suite into its base part and variant.
+/// Does not expect the base part to contain either `-` or `/`.
 fn suite_variant(suite: &str) -> (&str, &str) {
-    let variants = ["-backports-sloppy", "-backports", "-updates", "/updates"];
-
-    for variant in variants.iter() {
-        if let Some(base) = suite.strip_suffix(variant) {
-            return (base, variant);
-        }
+    match suite.find(&['-', '/'][..]) {
+        Some(n) => (&suite[0..n], &suite[n..]),
+        None => (suite, ""),
     }
-
-    (suite, "")
 }
 
 /// Strips existing double quotes from the string first, and then adds double quotes at
-- 
2.30.2

[pve-devel] [PATCH proxmox-apt 3/5] check repos: have caller specify the current suite

[Fabian Ebner]

Like that, a potential error is further up the stack, and it's more
consistent with what the standard_repository functions do.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 src/repositories/file.rs | 6 ++----
 src/repositories/mod.rs  | 7 +++++--
 tests/repositories.rs    | 6 +++---
 3 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/src/repositories/file.rs b/src/repositories/file.rs
index 49cc358..254af4d 100644
--- a/src/repositories/file.rs
+++ b/src/repositories/file.rs
@@ -5,7 +5,7 @@ use std::path::{Path, PathBuf};
 use anyhow::{bail, format_err, Error};
 use serde::{Deserialize, Serialize};
 
-use crate::repositories::release::{get_current_release_codename, DEBIAN_SUITES};
+use crate::repositories::release::DEBIAN_SUITES;
 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) -> Result<Vec<APTRepositoryInfo>, Error> {
+    pub fn check_suites(&self, current_suite: &str) -> Result<Vec<APTRepositoryInfo>, Error> {
         let mut infos = vec![];
 
         for (n, repo) in self.repositories.iter().enumerate() {
@@ -318,8 +318,6 @@ impl APTRepositoryFile {
                 })
             };
 
-            let current_suite = get_current_release_codename()?;
-
             let current_index = match DEBIAN_SUITES
                 .iter()
                 .position(|&suite| suite == current_suite)
diff --git a/src/repositories/mod.rs b/src/repositories/mod.rs
index 8637851..8f5500e 100644
--- a/src/repositories/mod.rs
+++ b/src/repositories/mod.rs
@@ -49,11 +49,14 @@ fn common_digest(files: &[APTRepositoryFile]) -> [u8; 32] {
 /// `warnings` for bad suites.
 /// `ignore-pre-upgrade-warning` when the next stable suite is configured.
 /// `badge` for official URIs.
-pub fn check_repositories(files: &[APTRepositoryFile]) -> Result<Vec<APTRepositoryInfo>, Error> {
+pub fn check_repositories(
+    files: &[APTRepositoryFile],
+    current_suite: &str,
+) -> Result<Vec<APTRepositoryInfo>, Error> {
     let mut infos = vec![];
 
     for file in files.iter() {
-        infos.append(&mut file.check_suites()?);
+        infos.append(&mut file.check_suites(current_suite)?);
         infos.append(&mut file.check_uris());
     }
 
diff --git a/tests/repositories.rs b/tests/repositories.rs
index 4cbde60..a9a7b96 100644
--- a/tests/repositories.rs
+++ b/tests/repositories.rs
@@ -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])?;
+    let infos = check_repositories(&vec![file], "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])?;
+    let mut infos = check_repositories(&vec![file], "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])?;
+    let mut infos = check_repositories(&vec![file], "bullseye")?;
     infos.sort();
 
     assert_eq!(infos, expected_infos);
-- 
2.30.2

[pve-devel] [PATCH proxmox-apt 4/5] repo: remove has_suite_variant helper

[Fabian Ebner]

by exchanging loops in the check_suites function, which was the only
user. Exchanging loops also helps for introducing a type for Debian condenames.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Breaking change, as the helper was publicly accessable via the type,
but it was not actually used outside the library by us.

 src/repositories/file.rs       | 35 ++++++++++++++++++++++------------
 src/repositories/repository.rs | 16 ----------------
 2 files changed, 23 insertions(+), 28 deletions(-)

diff --git a/src/repositories/file.rs b/src/repositories/file.rs
index 254af4d..fe994f7 100644
--- a/src/repositories/file.rs
+++ b/src/repositories/file.rs
@@ -326,37 +326,39 @@ impl APTRepositoryFile {
                 None => bail!("unknown release {}", current_suite),
             };
 
-            for (n, suite) in DEBIAN_SUITES.iter().enumerate() {
-                if repo.has_suite_variant(suite) {
+            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!", suite),
+                            format!("old suite '{}' configured!", base_suite),
                         );
                     }
 
                     if n == current_index + 1 {
                         add_info(
                             "ignore-pre-upgrade-warning".to_string(),
-                            format!("suite '{}' should not be used in production!", suite),
+                            format!("suite '{}' should not be used in production!", base_suite),
                         );
                     }
 
                     if n > current_index + 1 {
                         add_info(
                             "warning".to_string(),
-                            format!("suite '{}' should not be used in production!", suite),
+                            format!("suite '{}' should not be used in production!", base_suite),
                         );
                     }
                 }
             }
-
-            if repo.has_suite_variant("stable") {
-                add_info(
-                    "warning".to_string(),
-                    "use the name of the stable distribution instead of 'stable'!".to_string(),
-                );
-            }
         }
 
         Ok(infos)
@@ -390,3 +392,12 @@ impl APTRepositoryFile {
         infos
     }
 }
+
+/// Splits the suite into its base part and variant.
+/// Does not expect the base part to contain either `-` or `/`.
+fn suite_variant(suite: &str) -> (&str, &str) {
+    match suite.find(&['-', '/'][..]) {
+        Some(n) => (&suite[0..n], &suite[n..]),
+        None => (suite, ""),
+    }
+}
diff --git a/src/repositories/repository.rs b/src/repositories/repository.rs
index d85f063..85c8bdd 100644
--- a/src/repositories/repository.rs
+++ b/src/repositories/repository.rs
@@ -293,13 +293,6 @@ impl APTRepository {
             && self.components.contains(&component)
     }
 
-    /// Check if a variant of the given suite is configured in this repository
-    pub fn has_suite_variant(&self, base_suite: &str) -> bool {
-        self.suites
-            .iter()
-            .any(|suite| suite_variant(suite).0 == base_suite)
-    }
-
     /// Guess the origin from the repository's URIs.
     ///
     /// Intended to be used as a fallback for get_cached_origin.
@@ -432,15 +425,6 @@ fn host_from_uri(uri: &str) -> Option<&str> {
     Some(host)
 }
 
-/// Splits the suite into its base part and variant.
-/// Does not expect the base part to contain either `-` or `/`.
-fn suite_variant(suite: &str) -> (&str, &str) {
-    match suite.find(&['-', '/'][..]) {
-        Some(n) => (&suite[0..n], &suite[n..]),
-        None => (suite, ""),
-    }
-}
-
 /// Strips existing double quotes from the string first, and then adds double quotes at
 /// the beginning and end if there is an ASCII whitespace in the `string`, which is not
 /// escaped by `[]`.
-- 
2.30.2

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

[Fabian Ebner]

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

[pve-devel] [PATCH pve-rs 1/2] apt: repos: adapt to back-end changes

[Fabian Ebner]

It's up to the caller to provide the current release for standard
repository detection/addition.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Dependency bump for proxmox-apt patch #1 needed.

 src/apt/repositories.rs | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/src/apt/repositories.rs b/src/apt/repositories.rs
index 79c2334..22fc4b7 100644
--- a/src/apt/repositories.rs
+++ b/src/apt/repositories.rs
@@ -44,8 +44,11 @@ mod export {
         let (files, errors, digest) = proxmox_apt::repositories::repositories()?;
         let digest = proxmox::tools::digest_to_hex(&digest);
 
+        let suite = proxmox_apt::repositories::get_current_release_codename()?;
+
         let infos = proxmox_apt::repositories::check_repositories(&files)?;
-        let standard_repos = proxmox_apt::repositories::standard_repositories("pve", &files);
+        let standard_repos =
+            proxmox_apt::repositories::standard_repositories(&files, "pve", &suite);
 
         Ok(RepositoriesResult {
             files,
@@ -65,6 +68,7 @@ mod export {
         let (mut files, errors, current_digest) = proxmox_apt::repositories::repositories()?;
 
         let handle: APTRepositoryHandle = handle.try_into()?;
+        let suite = proxmox_apt::repositories::get_current_release_codename()?;
 
         if let Some(digest) = digest {
             let expected_digest = proxmox::tools::hex_to_digest(digest)?;
@@ -76,7 +80,7 @@ mod export {
         // check if it's already configured first
         for file in files.iter_mut() {
             for repo in file.repositories.iter_mut() {
-                if repo.is_referenced_repository(handle, "pve") {
+                if repo.is_referenced_repository(handle, "pve", &suite) {
                     if repo.enabled {
                         return Ok(());
                     }
@@ -89,7 +93,8 @@ mod export {
             }
         }
 
-        let (repo, path) = proxmox_apt::repositories::get_standard_repository(handle, "pve")?;
+        let (repo, path) =
+            proxmox_apt::repositories::get_standard_repository(handle, "pve", &suite);
 
         if let Some(error) = errors.iter().find(|error| error.path == path) {
             bail!(
-- 
2.30.2

[pve-devel] [PATCH pve-rs 2/2] apt: repos: adapt to further back-end changes

[Fabian Ebner]

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Dependency bump for proxmox-apt patches #3 and #5 needed.

Can be squashed into the previous patch if all proxmox-apt patches are
applied.

 src/apt/repositories.rs | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/src/apt/repositories.rs b/src/apt/repositories.rs
index 22fc4b7..b93719f 100644
--- a/src/apt/repositories.rs
+++ b/src/apt/repositories.rs
@@ -46,9 +46,8 @@ mod export {
 
         let suite = proxmox_apt::repositories::get_current_release_codename()?;
 
-        let infos = proxmox_apt::repositories::check_repositories(&files)?;
-        let standard_repos =
-            proxmox_apt::repositories::standard_repositories(&files, "pve", &suite);
+        let infos = proxmox_apt::repositories::check_repositories(&files, suite);
+        let standard_repos = proxmox_apt::repositories::standard_repositories(&files, "pve", suite);
 
         Ok(RepositoriesResult {
             files,
@@ -80,7 +79,7 @@ mod export {
         // check if it's already configured first
         for file in files.iter_mut() {
             for repo in file.repositories.iter_mut() {
-                if repo.is_referenced_repository(handle, "pve", &suite) {
+                if repo.is_referenced_repository(handle, "pve", &suite.to_string()) {
                     if repo.enabled {
                         return Ok(());
                     }
@@ -93,8 +92,7 @@ mod export {
             }
         }
 
-        let (repo, path) =
-            proxmox_apt::repositories::get_standard_repository(handle, "pve", &suite);
+        let (repo, path) = proxmox_apt::repositories::get_standard_repository(handle, "pve", suite);
 
         if let Some(error) = errors.iter().find(|error| error.path == path) {
             bail!(
-- 
2.30.2

[pve-devel] applied-series: Re: [PATCH-SERIES proxmox-apt/pve-rs] better detection of standard repositories

[Thomas Lamprecht]

On 29/07/2021 14:25, Fabian Ebner wrote:
> I'll send the corresponding patches for pmg-rs and proxmox-backup
> separately.

pmg-rs and pve-rs really need to merge, at least into one repo, could be
a workspace with a common, pmg and pve subcrates that produces two binary
packages, just one package and no subscrates will just bring us the linkage
hell again, so I'D like to avoid making the same mistakes we made in pbs.

> proxmox-apt:
> 
> Fabian Ebner (5):
>   standard repos: add suite parameter for stricter detection
>   repo: make suite_variant helper more general
>   check repos: have caller specify the current suite
>   repo: remove has_suite_variant helper
>   add type DebianCodename
> 
>  src/repositories/file.rs       |  84 ++++++++++++++-------------
>  src/repositories/mod.rs        |  31 +++++-----
>  src/repositories/release.rs    | 103 ++++++++++++++++++++++++++-------
>  src/repositories/repository.rs |  33 ++++-------
>  tests/repositories.rs          |  27 ++++++---
>  5 files changed, 171 insertions(+), 107 deletions(-)
> 
> 
> pve-rs:
> 
> Fabian Ebner (2):
>   apt: repos: adapt to back-end changes
>   apt: repos: adapt to further back-end changes
> 
>  src/apt/repositories.rs | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 


applied series, thanks!