[pbs-devel] [PATCH/RFC-SERIES v2] APT repositories API/UI

public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed

* [pbs-devel] [PATCH/RFC-SERIES v2] APT repositories API/UI
@ 2021-02-26 15:09 Fabian Ebner
  2021-02-26 15:09 ` [pbs-devel] [PATCH v2 proxmox-apt 01/10] initial commit Fabian Ebner
                   ` (9 more replies)
  0 siblings, 10 replies; 13+ messages in thread
From: Fabian Ebner @ 2021-02-26 15:09 UTC (permalink / raw)
  To: pbs-devel

List the configured repositories and have some basic checks for them.

Conceptual continuation of [0]. The plan is to use perlmod to make the Rust
implementation available for PVE+PMG as well.

There's still the question if introducing a digest is worth it. At the moment,
the warnings returned by the checkrepositories call might not match up with the
repositories returned previously, but that's a rather minor issue.
Should a digest be added now to be future-proof? Should it live in the
proxmox-apt crate and be file-level, or would it be enough to hash the result
from parsing in the PBS API call and use that? The latter seems like the
more pragmatic approach and avoids cluttering the APT backend.

Changes from v1:
    * Perl -> Rust
    * PVE -> PBS
    * Don't rely on regexes for parsing.
    * Add writer and tests.
    * UI: pin warnings to the repository they're for.
    * Keep order of options consistent with configuration.
    * Smaller things noted on the individual patches.

Still missing:
    * Upgrade suite/distribuiton button to be used before major release
      upgrades (but it's really simply to add that now).
    * perlmod magic and integration in PVE and PMG.

[0]: https://lists.proxmox.com/pipermail/pve-devel/2021-January/046726.html


proxmox-apt:

Fabian Ebner (4):
  initial commit
  add files for Debian packaging
  add functions to check for Proxmox repositories
  add check_repositories function


widget-toolkit:

Fabian Ebner (2):
  add UI for APT repositories
  add warnings

 src/Makefile                |   1 +
 src/node/APTRepositories.js | 259 ++++++++++++++++++++++++++++++++++++
 2 files changed, 260 insertions(+)
 create mode 100644 src/node/APTRepositories.js


proxmox-backup:

Fabian Ebner (4):
  depend on new proxmox-apt crate
  api: apt: add repositories call
  ui: add repositories
  add check_repositories_call

 Cargo.toml                  |  1 +
 src/api2/node/apt.rs        | 78 +++++++++++++++++++++++++++++++++++++
 www/ServerAdministration.js |  7 ++++
 3 files changed, 86 insertions(+)

-- 
2.20.1





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

* [pbs-devel] [PATCH v2 proxmox-apt 01/10] initial commit
  2021-02-26 15:09 [pbs-devel] [PATCH/RFC-SERIES v2] APT repositories API/UI Fabian Ebner
@ 2021-02-26 15:09 ` Fabian Ebner
  2021-03-10 15:14   ` Wolfgang Bumiller
  2021-02-26 15:09 ` [pbs-devel] [PATCH v2 proxmox-apt 02/10] add files for Debian packaging Fabian Ebner
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 13+ messages in thread
From: Fabian Ebner @ 2021-02-26 15:09 UTC (permalink / raw)
  To: pbs-devel

This create is inteded to hold the code to interact with APT repositories and
packages.

Starting off with code for parsing and writing .list and .sources repository
configuration files. The parser is partially inspired by the network
configuration parser in proxmox-backup, but it does not use a lexer,
because there's not much variation in how things are to be parsed.

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

Changes from v1:
    * Switched from Perl to Rust.
    * Split things by whitespace instead of using regexes everywhere,
      being the cleaner approach.
    * Used PascalCase for all keys for consistency.
    * File type is now also explicitly returned.
    * Added a writer and tests.
    * The options are not a hash anymore, because it's necessary to keep
      the same order when writing them back out (i.e. Languages-Add
      Languages-Remove)

 .cargo/config                                 |   5 +
 .gitignore                                    |   3 +
 Cargo.toml                                    |  22 ++
 rustfmt.toml                                  |   1 +
 src/lib.rs                                    |   3 +
 src/repositories/check.rs                     |  47 ++++
 src/repositories/list_parser.rs               | 181 +++++++++++++
 src/repositories/mod.rs                       | 246 ++++++++++++++++++
 src/repositories/sources_parser.rs            | 217 +++++++++++++++
 src/repositories/writer.rs                    |  89 +++++++
 src/types.rs                                  | 197 ++++++++++++++
 tests/repositories.rs                         |  73 ++++++
 .../sources.list.d.expected/multiline.sources |   8 +
 .../options_comment.list                      |   3 +
 .../pbs-enterprise.list                       |   2 +
 tests/sources.list.d.expected/pve.list        |  13 +
 tests/sources.list.d.expected/standard.list   |   7 +
 .../sources.list.d.expected/standard.sources  |  11 +
 tests/sources.list.d/multiline.sources        |   9 +
 tests/sources.list.d/options_comment.list     |   2 +
 tests/sources.list.d/pbs-enterprise.list      |   1 +
 tests/sources.list.d/pve.list                 |  10 +
 tests/sources.list.d/standard.list            |   6 +
 tests/sources.list.d/standard.sources         |  10 +
 24 files changed, 1166 insertions(+)
 create mode 100644 .cargo/config
 create mode 100644 .gitignore
 create mode 100644 Cargo.toml
 create mode 100644 rustfmt.toml
 create mode 100644 src/lib.rs
 create mode 100644 src/repositories/check.rs
 create mode 100644 src/repositories/list_parser.rs
 create mode 100644 src/repositories/mod.rs
 create mode 100644 src/repositories/sources_parser.rs
 create mode 100644 src/repositories/writer.rs
 create mode 100644 src/types.rs
 create mode 100644 tests/repositories.rs
 create mode 100644 tests/sources.list.d.expected/multiline.sources
 create mode 100644 tests/sources.list.d.expected/options_comment.list
 create mode 100644 tests/sources.list.d.expected/pbs-enterprise.list
 create mode 100644 tests/sources.list.d.expected/pve.list
 create mode 100644 tests/sources.list.d.expected/standard.list
 create mode 100644 tests/sources.list.d.expected/standard.sources
 create mode 100644 tests/sources.list.d/multiline.sources
 create mode 100644 tests/sources.list.d/options_comment.list
 create mode 100644 tests/sources.list.d/pbs-enterprise.list
 create mode 100644 tests/sources.list.d/pve.list
 create mode 100644 tests/sources.list.d/standard.list
 create mode 100644 tests/sources.list.d/standard.sources

diff --git a/.cargo/config b/.cargo/config
new file mode 100644
index 0000000..3b5b6e4
--- /dev/null
+++ b/.cargo/config
@@ -0,0 +1,5 @@
+[source]
+[source.debian-packages]
+directory = "/usr/share/cargo/registry"
+[source.crates-io]
+replace-with = "debian-packages"
diff --git a/.gitignore b/.gitignore
new file mode 100644
index 0000000..0d00c41
--- /dev/null
+++ b/.gitignore
@@ -0,0 +1,3 @@
+Cargo.lock
+target/
+tests/sources.list.d.actual
diff --git a/Cargo.toml b/Cargo.toml
new file mode 100644
index 0000000..a0ecc26
--- /dev/null
+++ b/Cargo.toml
@@ -0,0 +1,22 @@
+[package]
+name = "proxmox-apt"
+version = "0.1.0"
+authors = [
+    "Fabian Ebner <f.ebner@proxmox.com>",
+    "Proxmox Support Team <support@proxmox.com>",
+]
+edition = "2018"
+license = "AGPL-3"
+description = "Proxmox library for APT"
+homepage = "https://www.proxmox.com"
+
+exclude = [ "debian" ]
+
+[lib]
+name = "proxmox_apt"
+path = "src/lib.rs"
+
+[dependencies]
+anyhow = "1.0"
+proxmox = { version = "0.11.0", features = [ "api-macro" ] }
+serde = { version = "1.0", features = ["derive"] }
diff --git a/rustfmt.toml b/rustfmt.toml
new file mode 100644
index 0000000..32a9786
--- /dev/null
+++ b/rustfmt.toml
@@ -0,0 +1 @@
+edition = "2018"
diff --git a/src/lib.rs b/src/lib.rs
new file mode 100644
index 0000000..b065c0f
--- /dev/null
+++ b/src/lib.rs
@@ -0,0 +1,3 @@
+pub mod types;
+
+pub mod repositories;
diff --git a/src/repositories/check.rs b/src/repositories/check.rs
new file mode 100644
index 0000000..87fbbac
--- /dev/null
+++ b/src/repositories/check.rs
@@ -0,0 +1,47 @@
+use anyhow::{bail, Error};
+
+use crate::types::{APTRepository, APTRepositoryFileType};
+
+impl APTRepository {
+    /// Makes sure that all basic properties of a repository are present and
+    /// not obviously invalid.
+    pub fn basic_check(&self) -> Result<(), Error> {
+        if self.types.is_empty() {
+            bail!("missing package type(s)");
+        }
+        if self.uris.is_empty() {
+            bail!("missing URI(s)");
+        }
+        if self.suites.is_empty() {
+            bail!("missing suite(s)");
+        }
+
+        for uri in self.uris.iter() {
+            if !uri.contains(':') || uri.len() < 3 {
+                bail!("invalid URI: '{}'", uri);
+            }
+        }
+
+        for suite in self.suites.iter() {
+            if !suite.ends_with('/') && self.components.is_empty() {
+                bail!("missing component(s)");
+            } else if suite.ends_with('/') && !self.components.is_empty() {
+                bail!("absolute suite '{}' does not allow component(s)", suite);
+            }
+        }
+
+        if self.file_type == APTRepositoryFileType::List {
+            if self.types.len() > 1 {
+                bail!("more than one package type");
+            }
+            if self.uris.len() > 1 {
+                bail!("more than one URI");
+            }
+            if self.suites.len() > 1 {
+                bail!("more than one suite");
+            }
+        }
+
+        Ok(())
+    }
+}
diff --git a/src/repositories/list_parser.rs b/src/repositories/list_parser.rs
new file mode 100644
index 0000000..b26fed2
--- /dev/null
+++ b/src/repositories/list_parser.rs
@@ -0,0 +1,181 @@
+use std::convert::TryInto;
+use std::io::BufRead;
+use std::iter::{Iterator, Peekable};
+use std::str::SplitAsciiWhitespace;
+
+use anyhow::{bail, Error};
+
+use super::APTRepositoryParser;
+use crate::types::{APTRepository, APTRepositoryFileType, APTRepositoryOption};
+
+pub struct APTListFileParser<R: BufRead> {
+    path: String,
+    input: R,
+    line_nr: usize,
+    comment: String,
+}
+
+impl<R: BufRead> APTListFileParser<R> {
+    pub fn new(path: String, reader: R) -> Self {
+        Self {
+            path,
+            input: reader,
+            line_nr: 0,
+            comment: String::new(),
+        }
+    }
+
+    /// Helper to parse options from the existing token stream.
+    /// Also returns `Ok(())` if there are no options.
+    /// Errors when options are invalid or not closed by `']'`.
+    fn parse_options(
+        options: &mut Vec<APTRepositoryOption>,
+        tokens: &mut Peekable<SplitAsciiWhitespace>,
+    ) -> Result<(), Error> {
+        let mut option = match tokens.peek() {
+            Some(token) => {
+                match token.strip_prefix('[') {
+                    Some(option) => option,
+                    None => return Ok(()), // doesn't look like options
+                }
+            }
+            None => return Ok(()),
+        };
+
+        tokens.next(); // avoid reading the beginning twice
+
+        let mut finished = false;
+        loop {
+            if let Some(stripped) = option.strip_suffix(']') {
+                option = stripped;
+                if option.is_empty() {
+                    break;
+                }
+                finished = true; // but still need to handle the last one
+            };
+
+            if let Some(mid) = option.find('=') {
+                let (key, mut value_str) = option.split_at(mid);
+                value_str = &value_str[1..];
+
+                if key.is_empty() {
+                    bail!("option with value '{}' has no key", value_str);
+                } else if value_str.is_empty() {
+                    bail!("option with key '{}' has no value", key);
+                }
+
+                let values: Vec<String> = value_str
+                    .split(',')
+                    .map(|value| value.to_string())
+                    .collect();
+
+                options.push(APTRepositoryOption {
+                    key: key.to_string(),
+                    values,
+                });
+            } else if !option.is_empty() {
+                bail!("got invalid option - '{}'", option);
+            }
+
+            if finished {
+                break;
+            }
+
+            option = match tokens.next() {
+                Some(option) => option,
+                None => bail!("options not closed by ']'"),
+            }
+        }
+
+        Ok(())
+    }
+
+    /// Parse a repository or comment in one-line format.
+    /// Commented out repositories are also detected and returned with the
+    /// `enabled` property set to `false`.
+    /// If the line contains a repository, `self.comment` is added to the
+    /// `comment` property.
+    /// If the line contains a comment, it is added to `self.comment`.
+    fn parse_one_line(&mut self, mut line: &str) -> Result<Option<APTRepository>, Error> {
+        line = line.trim_matches(|c| char::is_ascii_whitespace(&c));
+
+        // check for commented out repository first
+        if let Some(commented_out) = line.strip_prefix('#') {
+            if let Ok(Some(mut repo)) = self.parse_one_line(commented_out) {
+                repo.set_enabled(false);
+                return Ok(Some(repo));
+            }
+        }
+
+        let mut repo =
+            APTRepository::new(self.path.clone(), self.line_nr, APTRepositoryFileType::List);
+
+        // now handle "real" comment
+        if let Some(comment_start) = line.find('#') {
+            let (line_start, comment) = line.split_at(comment_start);
+            self.comment = format!("{}{}\n", self.comment, &comment[1..]);
+            line = line_start;
+        }
+
+        let mut tokens = line.split_ascii_whitespace().peekable();
+
+        match tokens.next() {
+            Some(package_type) => {
+                repo.types.push(package_type.try_into()?);
+            }
+            None => return Ok(None), // empty line
+        }
+
+        Self::parse_options(&mut repo.options, &mut tokens)?;
+
+        match tokens.next() {
+            Some(uri) => repo.uris.push(uri.to_string()),
+            None => bail!("got no URI"),
+        };
+
+        match tokens.next() {
+            Some(suite) => repo.suites.push(suite.to_string()),
+            None => bail!("got no suite"),
+        };
+
+        for component in tokens {
+            repo.components.push(component.to_string());
+        }
+
+        repo.comment = self.comment.clone();
+
+        Ok(Some(repo))
+    }
+}
+
+impl<R: BufRead> APTRepositoryParser for APTListFileParser<R> {
+    fn parse_repositories(&mut self) -> Result<Vec<APTRepository>, Error> {
+        let mut repositories = Vec::<APTRepository>::new();
+        let mut line = String::new();
+
+        loop {
+            self.line_nr += 1;
+            line.clear();
+
+            match self.input.read_line(&mut line) {
+                Err(err) => bail!("input error for '{}' - {}", self.path, err),
+                Ok(0) => break,
+                Ok(_) => match self.parse_one_line(&line) {
+                    Ok(Some(repo)) => {
+                        repositories.push(repo);
+                        self.comment.clear();
+                    }
+                    Ok(None) => continue,
+                    Err(err) => bail!(
+                        "malformed entry in '{}' line {} - {}",
+                        self.path,
+                        self.line_nr,
+                        err,
+                    ),
+                },
+            }
+        }
+
+        Ok(repositories)
+    }
+}
diff --git a/src/repositories/mod.rs b/src/repositories/mod.rs
new file mode 100644
index 0000000..123222a
--- /dev/null
+++ b/src/repositories/mod.rs
@@ -0,0 +1,246 @@
+use std::collections::BTreeMap;
+use std::convert::TryFrom;
+use std::ffi::OsString;
+use std::path::{Path, PathBuf};
+
+use anyhow::{bail, format_err, Error};
+
+use crate::types::{APTRepository, APTRepositoryFileType, APTRepositoryOption};
+
+mod list_parser;
+use list_parser::APTListFileParser;
+
+mod sources_parser;
+use sources_parser::APTSourcesFileParser;
+
+mod check;
+mod writer;
+
+const APT_SOURCES_LIST_FILENAME: &str = "/etc/apt/sources.list";
+const APT_SOURCES_LIST_DIRECTORY: &str = "/etc/apt/sources.list.d/";
+
+impl APTRepository {
+    /// Crates an empty repository with infomration that is known before parsing.
+    fn new(path: String, number: usize, file_type: APTRepositoryFileType) -> Self {
+        Self {
+            types: vec![],
+            uris: vec![],
+            suites: vec![],
+            components: vec![],
+            options: vec![],
+            comment: String::new(),
+            path,
+            number,
+            file_type,
+            enabled: true,
+        }
+    }
+
+    /// Changes the `enabled` flag and makes sure the `Enabled` option for
+    /// `APTRepositoryPackageType::Sources` repositories is updated too.
+    fn set_enabled(&mut self, enabled: bool) {
+        self.enabled = enabled;
+
+        if self.file_type == APTRepositoryFileType::Sources {
+            let enabled_string = match enabled {
+                true => "true".to_string(),
+                false => "false".to_string(),
+            };
+            for option in self.options.iter_mut() {
+                if option.key == "Enabled" {
+                    option.values = vec![enabled_string];
+                    return;
+                }
+            }
+            self.options.push(APTRepositoryOption {
+                key: "Enabled".to_string(),
+                values: vec![enabled_string],
+            });
+        }
+    }
+}
+
+trait APTRepositoryParser {
+    /// Parse all repositories including the disabled ones.
+    fn parse_repositories(&mut self) -> Result<Vec<APTRepository>, Error>;
+}
+
+/// Helper to decide whether a file name is considered valid by APT and to
+/// extract its file type and the path as a string.
+/// Hidden files yield `Ok(None)`, while invalid file names yield an error.
+fn check_filename<P: AsRef<Path>>(
+    path: P,
+) -> Result<Option<(APTRepositoryFileType, String)>, OsString> {
+    let path: PathBuf = path.as_ref().to_path_buf();
+    let path_string = path.clone().into_os_string().into_string()?;
+
+    let file_name = match path.file_name() {
+        Some(file_name) => file_name.to_os_string().into_string()?,
+        None => return Err(OsString::from(path_string)),
+    };
+
+    // APT silently ignores hidden files
+    if file_name.starts_with('.') {
+        return Ok(None);
+    }
+
+    let extension = match path.extension() {
+        Some(extension) => extension.to_os_string().into_string()?,
+        None => return Err(OsString::from(path_string)),
+    };
+
+    let file_type = match APTRepositoryFileType::try_from(&extension[..]) {
+        Ok(file_type) => file_type,
+        _ => return Err(OsString::from(path_string)),
+    };
+
+    // APT ignores such files but issues a warning
+    if !file_name
+        .chars()
+        .all(|x| x.is_ascii_alphanumeric() || x == '_' || x == '-' || x == '.')
+    {
+        return Err(OsString::from(path_string));
+    }
+
+    Ok(Some((file_type, path_string)))
+}
+
+/// Returns all APT repositories configured in the specified files including
+/// disabled ones.
+/// Warns about invalid file names and some format violations, while other
+/// format violations result in an error.
+pub fn repositories_from_files<P: AsRef<Path>>(paths: &[P]) -> Result<Vec<APTRepository>, Error> {
+    let mut repos = Vec::<APTRepository>::new();
+
+    for path in paths.iter() {
+        let path = path.as_ref();
+
+        if !path.is_file() {
+            eprintln!("Ignoring {:?} - not a file", path);
+            continue;
+        }
+
+        let file_type;
+        let path_string;
+
+        match check_filename(path) {
+            Ok(Some(res)) => {
+                file_type = res.0;
+                path_string = res.1;
+            }
+            Ok(None) => continue,
+            Err(path) => {
+                eprintln!("Ignoring {:?} - invalid file name", path);
+                continue;
+            }
+        }
+
+        let contents = std::fs::read(path)
+            .map_err(|err| format_err!("unable to read {:?} - {}", path, err))?;
+
+        let mut parser: Box<dyn APTRepositoryParser> = match file_type {
+            APTRepositoryFileType::List => {
+                Box::new(APTListFileParser::new(path_string, &contents[..]))
+            }
+            APTRepositoryFileType::Sources => {
+                Box::new(APTSourcesFileParser::new(path_string, &contents[..]))
+            }
+        };
+
+        repos.append(&mut parser.parse_repositories()?);
+    }
+
+    for repo in repos.iter() {
+        repo.basic_check().map_err(|err| {
+            format_err!("check for {}:{} failed - {}", repo.path, repo.number, err)
+        })?;
+    }
+
+    Ok(repos)
+}
+
+/// Returns all APT repositories configured in `/etc/apt/sources.list` and
+/// in `/etc/apt/sources.list.d` including disabled repositories.
+/// Warns about invalid file names and some format violations, while other
+/// format violations result in an error.
+pub fn repositories() -> Result<Vec<APTRepository>, Error> {
+    let mut paths = Vec::<PathBuf>::new();
+
+    let sources_list_path = PathBuf::from(APT_SOURCES_LIST_FILENAME);
+    if sources_list_path.is_file() {
+        paths.push(sources_list_path)
+    };
+
+    let sources_list_d_path = PathBuf::from(APT_SOURCES_LIST_DIRECTORY);
+    if sources_list_d_path.is_dir() {
+        for entry in std::fs::read_dir(sources_list_d_path)? {
+            paths.push(entry?.path());
+        }
+    }
+
+    let repos = repositories_from_files(&paths)?;
+
+    Ok(repos)
+}
+
+/// Write the repositories to the respective files specified by their
+/// `path` property and in the order determined by their `number` property.
+/// Does a `check::basic_check(repository)` for each repository first.
+pub fn write_repositories<A: AsRef<APTRepository>>(repos: &[A]) -> Result<(), Error> {
+    let mut repos: Vec<&APTRepository> = repos.iter().map(|repo| repo.as_ref()).collect();
+
+    // check before writing
+    for repo in repos.iter() {
+        repo.basic_check().map_err(|err| {
+            format_err!("check for {}:{} failed - {}", repo.path, repo.number, err)
+        })?;
+    }
+
+    repos.sort_by(|a, b| {
+        if a.path == b.path {
+            a.number.cmp(&b.number)
+        } else {
+            a.path.cmp(&b.path)
+        }
+    });
+
+    let mut files = BTreeMap::<String, Vec<u8>>::new();
+
+    for repo in repos.iter() {
+        let raw = match files.get_mut(&repo.path) {
+            Some(raw) => raw,
+            None => {
+                files.insert(repo.path.clone(), vec![]);
+                files.get_mut(&repo.path).unwrap()
+            }
+        };
+
+        repo.write(&mut *raw)
+            .map_err(|err| format_err!("writing {}:{} failed - {}", repo.path, repo.number, err))?;
+    }
+
+    for (path, content) in files.iter() {
+        let path = PathBuf::from(path);
+        let dir = path.parent().unwrap();
+
+        std::fs::create_dir_all(dir)
+            .map_err(|err| format_err!("unable to create dir {:?} - {}", dir, err))?;
+
+        let pid = std::process::id();
+        let mut tmp_path = path.clone();
+        tmp_path.set_extension("tmp");
+        tmp_path.set_extension(format!("{}", pid));
+
+        if let Err(err) = std::fs::write(&tmp_path, &content) {
+            let _ = std::fs::remove_file(&tmp_path);
+            bail!("write failed: {}", err);
+        }
+
+        if let Err(err) = std::fs::rename(&tmp_path, &path) {
+            let _ = std::fs::remove_file(&tmp_path);
+            bail!("rename failed for {:?} - {}", path, err);
+        }
+    }
+
+    Ok(())
+}
diff --git a/src/repositories/sources_parser.rs b/src/repositories/sources_parser.rs
new file mode 100644
index 0000000..fdfe603
--- /dev/null
+++ b/src/repositories/sources_parser.rs
@@ -0,0 +1,217 @@
+use std::convert::TryInto;
+use std::io::BufRead;
+use std::iter::Iterator;
+
+use anyhow::{bail, Error};
+
+use crate::types::{
+    APTRepository, APTRepositoryFileType, APTRepositoryOption, APTRepositoryPackageType,
+};
+
+use super::APTRepositoryParser;
+
+pub struct APTSourcesFileParser<R: BufRead> {
+    path: String,
+    input: R,
+    stanza_nr: usize,
+    comment: String,
+}
+
+/// See `man sources.list` and `man deb822` for the format specification.
+impl<R: BufRead> APTSourcesFileParser<R> {
+    pub fn new(path: String, reader: R) -> Self {
+        Self {
+            path,
+            input: reader,
+            stanza_nr: 1,
+            comment: String::new(),
+        }
+    }
+
+    /// Based on APT's `StringToBool` in `strutl.cc`
+    fn string_to_bool(string: &str, default: bool) -> bool {
+        let string = string.trim_matches(|c| char::is_ascii_whitespace(&c));
+        let string = string.to_lowercase();
+
+        match &string[..] {
+            "1" | "yes" | "true" | "with" | "on" | "enable" => true,
+            "0" | "no" | "false" | "without" | "off" | "disable" => false,
+            _ => default,
+        }
+    }
+
+    /// Checks if `key` is valid according to deb822
+    fn valid_key(key: &str) -> bool {
+        if key.starts_with('-') {
+            return false;
+        };
+        return key.chars().all(|c| matches!(c, '!'..='9' | ';'..='~'));
+    }
+
+    /// Try parsing a repository in stanza format from `lines`.
+    /// Returns `Ok(None)` when no stanza can be found.
+    /// Comments are added to `self.comments`. If a stanza can be found,
+    /// `self.comment` is added to the repository's `comment` property.
+    /// Fully commented out stanzas are treated as comments.
+    fn parse_stanza(&mut self, lines: &str) -> Result<Option<APTRepository>, Error> {
+        let mut repo = APTRepository::new(
+            self.path.clone(),
+            self.stanza_nr,
+            APTRepositoryFileType::Sources,
+        );
+
+        // Values may be folded into multiple lines.
+        // Those lines have to start with a space or a tab.
+        let lines = lines.replace("\n ", " ");
+        let lines = lines.replace("\n\t", " ");
+
+        let mut got_something = false;
+
+        for line in lines.lines() {
+            let line = line.trim_matches(|c| char::is_ascii_whitespace(&c));
+            if line.is_empty() {
+                continue;
+            }
+
+            if let Some(commented_out) = line.strip_prefix('#') {
+                self.comment = format!("{}{}\n", self.comment, commented_out);
+                continue;
+            }
+
+            if let Some(mid) = line.find(':') {
+                let (key, value_str) = line.split_at(mid);
+                let value_str = &value_str[1..];
+                let key = key.trim_matches(|c| char::is_ascii_whitespace(&c));
+
+                if key.is_empty() {
+                    bail!("option with value '{}' has no key", value_str);
+                } else if value_str.is_empty() {
+                    // ignored by APT
+                    eprintln!("option with key '{}' has no value", key);
+                    continue;
+                }
+
+                if !Self::valid_key(key) {
+                    // ignored by APT
+                    eprintln!("option with invalid key '{}'", key);
+                    continue;
+                }
+
+                let values: Vec<String> = value_str
+                    .split_ascii_whitespace()
+                    .map(|value| value.to_string())
+                    .collect();
+
+                match key {
+                    "Types" => {
+                        let mut types = Vec::<APTRepositoryPackageType>::new();
+                        for package_type in values {
+                            types.push((&package_type[..]).try_into()?);
+                        }
+                        if !repo.types.is_empty() {
+                            eprintln!("key 'Types' was defined twice");
+                        }
+                        repo.types = types;
+                    }
+                    "URIs" => {
+                        if !repo.uris.is_empty() {
+                            eprintln!("key 'URIs' was defined twice");
+                        }
+                        repo.uris = values;
+                    }
+                    "Suites" => {
+                        if !repo.suites.is_empty() {
+                            eprintln!("key 'Suites' was defined twice");
+                        }
+                        repo.suites = values;
+                    }
+                    "Components" => {
+                        if !repo.components.is_empty() {
+                            eprintln!("key 'Components' was defined twice");
+                        }
+                        repo.components = values;
+                    }
+                    "Enabled" => {
+                        repo.set_enabled(Self::string_to_bool(value_str, true));
+                    }
+                    _ => repo.options.push(APTRepositoryOption {
+                        key: key.to_string(),
+                        values,
+                    }),
+                }
+            } else {
+                bail!("got invalid line - '{:?}'", line);
+            }
+
+            got_something = true;
+        }
+
+        if !got_something {
+            return Ok(None);
+        }
+
+        repo.comment = self.comment.clone();
+
+        Ok(Some(repo))
+    }
+
+    /// Helper function for `parse_repositories`
+    fn try_parse_stanza(
+        &mut self,
+        lines: &str,
+        repos: &mut Vec<APTRepository>,
+    ) -> Result<(), Error> {
+        match self.parse_stanza(&lines[..]) {
+            Ok(Some(repo)) => {
+                repos.push(repo);
+                self.comment.clear();
+                self.stanza_nr += 1;
+            }
+            Ok(None) => (),
+            Err(err) => {
+                bail!(
+                    "malformed entry in '{}' stanza {} - {}",
+                    self.path,
+                    self.stanza_nr,
+                    err,
+                );
+            }
+        }
+
+        Ok(())
+    }
+}
+
+impl<R: BufRead> APTRepositoryParser for APTSourcesFileParser<R> {
+    fn parse_repositories(&mut self) -> Result<Vec<APTRepository>, Error> {
+        let mut repos = Vec::<APTRepository>::new();
+        let mut lines = String::new();
+        let mut offset: usize = 0;
+
+        loop {
+            match self.input.read_line(&mut lines) {
+                Err(err) => bail!("input error for '{}' - {}", self.path, err),
+                Ok(0) => {
+                    self.try_parse_stanza(&lines[..], &mut repos)?;
+                    break;
+                }
+                Ok(bytes_read) => {
+                    if (&lines[offset..])
+                        .trim_matches(|c| char::is_ascii_whitespace(&c))
+                        .is_empty()
+                    {
+                        // detected end of stanza
+
+                        self.try_parse_stanza(&lines[..], &mut repos)?;
+                        lines.clear();
+                        offset = 0;
+                    } else {
+                        offset += bytes_read;
+                    }
+                }
+            }
+        }
+
+        Ok(repos)
+    }
+}
diff --git a/src/repositories/writer.rs b/src/repositories/writer.rs
new file mode 100644
index 0000000..6feb1c7
--- /dev/null
+++ b/src/repositories/writer.rs
@@ -0,0 +1,89 @@
+use std::io::Write;
+
+use anyhow::{bail, Error};
+
+use crate::types::{APTRepository, APTRepositoryFileType};
+
+impl APTRepository {
+    /// Writes a repository in the corresponding format followed by a blank.
+    /// Expects that `basic_check()` for the repository was successful.
+    pub fn write(&self, w: &mut dyn Write) -> Result<(), Error> {
+        match self.file_type {
+            APTRepositoryFileType::List => write_one_line(self, w),
+            APTRepositoryFileType::Sources => write_stanza(self, w),
+        }
+    }
+}
+
+/// Writes a repository in one-line format followed by a blank line.
+/// Expects that `repo.file_type == APTRepositoryFileType::List`.
+/// Expects that `basic_check()` for the repository was successful.
+fn write_one_line(repo: &APTRepository, w: &mut dyn Write) -> Result<(), Error> {
+    if repo.file_type != APTRepositoryFileType::List {
+        bail!("not a .list repository");
+    }
+
+    if !repo.comment.is_empty() {
+        for line in repo.comment.lines() {
+            writeln!(w, "#{}", line)?;
+        }
+    }
+
+    if !repo.enabled {
+        write!(w, "# ")?;
+    }
+
+    write!(w, "{} ", String::from(repo.types[0]))?;
+
+    if !repo.options.is_empty() {
+        let option_string = repo
+            .options
+            .iter()
+            .map(|option| format!("{}={}", option.key, option.values.join(",")))
+            .collect::<Vec<String>>()
+            .join(" ");
+
+        write!(w, "[ {} ] ", option_string)?;
+    };
+
+    write!(w, "{} ", repo.uris[0])?;
+    write!(w, "{} ", repo.suites[0])?;
+    writeln!(w, "{}", repo.components.join(" "))?;
+
+    writeln!(w)?;
+
+    Ok(())
+}
+
+/// Writes a single stanza followed by a blank line.
+/// Expects that `repo.file_type == APTRepositoryFileType::Sources`.
+fn write_stanza(repo: &APTRepository, w: &mut dyn Write) -> Result<(), Error> {
+    if repo.file_type != APTRepositoryFileType::Sources {
+        bail!("not a .sources repository");
+    }
+
+    if !repo.comment.is_empty() {
+        for line in repo.comment.lines() {
+            writeln!(w, "#{}", line)?;
+        }
+    }
+
+    let type_strings = repo
+        .types
+        .iter()
+        .map(|package_type| String::from(*package_type))
+        .collect::<Vec<String>>();
+
+    writeln!(w, "Types: {}", type_strings.join(" "))?;
+    writeln!(w, "URIs: {}", repo.uris.join(" "))?;
+    writeln!(w, "Suites: {}", repo.suites.join(" "))?;
+    writeln!(w, "Components: {}", repo.components.join(" "))?;
+
+    for option in repo.options.iter() {
+        writeln!(w, "{}: {}", option.key, option.values.join(" "))?;
+    }
+
+    writeln!(w)?;
+
+    Ok(())
+}
diff --git a/src/types.rs b/src/types.rs
new file mode 100644
index 0000000..7a95daf
--- /dev/null
+++ b/src/types.rs
@@ -0,0 +1,197 @@
+use std::convert::{From, TryFrom};
+
+use anyhow::{bail, Error};
+use serde::{Deserialize, Serialize};
+
+use proxmox::api::api;
+
+#[api()]
+#[derive(Debug, Copy, Clone, Serialize, Deserialize, PartialEq)]
+#[serde(rename_all = "lowercase")]
+pub enum APTRepositoryFileType {
+    /// One-line-style format
+    List,
+    /// DEB822-style format
+    Sources,
+}
+
+impl TryFrom<&str> for APTRepositoryFileType {
+    type Error = Error;
+
+    fn try_from(string: &str) -> Result<Self, Error> {
+        match &string[..] {
+            "list" => Ok(APTRepositoryFileType::List),
+            "sources" => Ok(APTRepositoryFileType::Sources),
+            _ => bail!("invalid file type '{}'", string),
+        }
+    }
+}
+
+impl From<APTRepositoryFileType> for String {
+    fn from(file_type: APTRepositoryFileType) -> String {
+        match file_type {
+            APTRepositoryFileType::List => "list".to_string(),
+            APTRepositoryFileType::Sources => "sources".to_string(),
+        }
+    }
+}
+
+#[api()]
+#[derive(Debug, Copy, Clone, Serialize, Deserialize, PartialEq)]
+#[serde(rename_all = "kebab-case")]
+pub enum APTRepositoryPackageType {
+    /// Debian package
+    Deb,
+    /// Debian source package
+    DebSrc,
+}
+
+impl TryFrom<&str> for APTRepositoryPackageType {
+    type Error = Error;
+
+    fn try_from(string: &str) -> Result<Self, Error> {
+        match &string[..] {
+            "deb" => Ok(APTRepositoryPackageType::Deb),
+            "deb-src" => Ok(APTRepositoryPackageType::DebSrc),
+            _ => bail!("invalid file type '{}'", string),
+        }
+    }
+}
+
+impl From<APTRepositoryPackageType> for String {
+    fn from(package_type: APTRepositoryPackageType) -> String {
+        match package_type {
+            APTRepositoryPackageType::Deb => "deb".to_string(),
+            APTRepositoryPackageType::DebSrc => "deb-src".to_string(),
+        }
+    }
+}
+
+#[api(
+    properties: {
+        Key: {
+            description: "Option key.",
+            type: String,
+        },
+        Values: {
+            description: "Option values.",
+            type: Array,
+            items: {
+                description: "Value.",
+                type: String,
+            },
+        },
+    },
+)]
+#[derive(Debug, Clone, Serialize, Deserialize)]
+#[serde(rename_all = "PascalCase")] // for consistency
+/// Additional options for an APT repository.
+/// Used for both single- and mutli-value options.
+pub struct APTRepositoryOption {
+    /// Option key.
+    pub key: String,
+    /// Option value(s).
+    pub values: Vec<String>,
+}
+
+#[api(
+    properties: {
+        Types: {
+            description: "List of package types.",
+            type: Array,
+            items: {
+                type: APTRepositoryPackageType,
+            },
+        },
+        URIs: {
+            description: "List of repository URIs.",
+            type: Array,
+            items: {
+                description: "Repository URI.",
+                type: String,
+            },
+        },
+        Suites: {
+            description: "List of distributions.",
+            type: Array,
+            items: {
+                description: "Package distribution.",
+                type: String,
+            },
+        },
+        Components: {
+            description: "List of repository components.",
+            type: Array,
+            items: {
+                description: "Repository component.",
+                type: String,
+            },
+        },
+        Options: {
+            type: Array,
+            optional: true,
+            items: {
+                type: APTRepositoryOption,
+            },
+        },
+        Comment: {
+            description: "Associated comment.",
+            type: String,
+            optional: true,
+        },
+        Path: {
+            description: "Path to the defining file.",
+            type: String,
+        },
+        Number: {
+            description: "Line or stanza number.",
+            type: Integer,
+        },
+        FileType: {
+            type: APTRepositoryFileType,
+        },
+        Enabled: {
+            description: "Whether the repository is enabled or not.",
+            type: Boolean,
+        },
+    },
+)]
+#[derive(Debug, Clone, Serialize, Deserialize)]
+#[serde(rename_all = "PascalCase")]
+/// Describes an APT repository.
+pub struct APTRepository {
+    /// List of package types.
+    #[serde(skip_serializing_if = "Vec::is_empty")]
+    pub types: Vec<APTRepositoryPackageType>,
+    /// List of repository URIs.
+    #[serde(skip_serializing_if = "Vec::is_empty")]
+    #[serde(rename = "URIs")]
+    pub uris: Vec<String>,
+    /// List of package distributions.
+    #[serde(skip_serializing_if = "Vec::is_empty")]
+    pub suites: Vec<String>,
+    /// List of repository components.
+    #[serde(skip_serializing_if = "Vec::is_empty")]
+    pub components: Vec<String>,
+    /// Additional options.
+    #[serde(skip_serializing_if = "Vec::is_empty")]
+    pub options: Vec<APTRepositoryOption>,
+    /// Associated comment.
+    #[serde(skip_serializing_if = "String::is_empty")]
+    pub comment: String,
+    /// Path to the defining file.
+    #[serde(skip_serializing_if = "String::is_empty")]
+    pub path: String,
+    /// Line or stanza number.
+    pub number: usize,
+    /// Format of the defining file.
+    pub file_type: APTRepositoryFileType,
+    /// Whether the repository is enabled or not.
+    pub enabled: bool,
+}
+
+impl AsRef<APTRepository> for APTRepository {
+    fn as_ref(&self) -> &APTRepository {
+        &self
+    }
+}
diff --git a/tests/repositories.rs b/tests/repositories.rs
new file mode 100644
index 0000000..020e133
--- /dev/null
+++ b/tests/repositories.rs
@@ -0,0 +1,73 @@
+use std::collections::HashMap;
+use std::path::PathBuf;
+
+use anyhow::{bail, format_err, Error};
+
+use proxmox_apt::repositories::{repositories_from_files, write_repositories};
+use proxmox_apt::types::APTRepository;
+
+#[test]
+fn test_parse_write() -> Result<(), Error> {
+    let test_dir = std::env::current_dir()?.join("tests");
+    let read_dir = test_dir.join("sources.list.d");
+    let write_dir = test_dir.join("sources.list.d.actual");
+    let expected_dir = test_dir.join("sources.list.d.expected");
+
+    let mut paths = Vec::<PathBuf>::new();
+    for entry in std::fs::read_dir(read_dir)? {
+        paths.push(entry?.path());
+    }
+
+    let repos = repositories_from_files(&paths)?;
+
+    // used to mess up the order from parsing and to check that each repo has a
+    // unique path:number
+    let mut repo_hash = HashMap::<String, APTRepository>::new();
+
+    for mut repo in repos {
+        let path = PathBuf::from(repo.path);
+        let new_path = write_dir.join(path.file_name().unwrap());
+
+        repo.path = new_path.into_os_string().into_string().unwrap();
+
+        let key = format!("{}:{}", repo.path, repo.number);
+
+        if repo_hash.insert(key.clone(), repo).is_some() {
+            bail!("key '{}' is not unique!", key);
+        }
+    }
+
+    if write_dir.is_dir() {
+        std::fs::remove_dir_all(&write_dir)
+            .map_err(|err| format_err!("unable to remove dir {:?} - {}", write_dir, err))?;
+    }
+
+    std::fs::create_dir_all(&write_dir)
+        .map_err(|err| format_err!("unable to create dir {:?} - {}", write_dir, err))?;
+
+    let repos_vec: Vec<&APTRepository> = repo_hash.values().collect();
+    write_repositories(&repos_vec)?;
+
+    let mut expected_count = 0;
+
+    for entry in std::fs::read_dir(expected_dir)? {
+        expected_count += 1;
+
+        let expected_path = entry?.path();
+        let actual_path = write_dir.join(expected_path.file_name().unwrap());
+
+        let expected_contents = std::fs::read(&expected_path)
+            .map_err(|err| format_err!("unable to read {:?} - {}", expected_path, err))?;
+
+        let actual_contents = std::fs::read(&actual_path)
+            .map_err(|err| format_err!("unable to read {:?} - {}", actual_path, err))?;
+
+        assert_eq!(expected_contents, actual_contents);
+    }
+
+    let actual_count = std::fs::read_dir(write_dir)?.count();
+
+    assert_eq!(expected_count, actual_count);
+
+    Ok(())
+}
diff --git a/tests/sources.list.d.expected/multiline.sources b/tests/sources.list.d.expected/multiline.sources
new file mode 100644
index 0000000..91f53c2
--- /dev/null
+++ b/tests/sources.list.d.expected/multiline.sources
@@ -0,0 +1,8 @@
+# comment in here
+Types: deb deb-src
+URIs: http://ftp.at.debian.org/debian
+Suites: buster buster-updates
+Components: main contrib
+Languages: it de fr
+Enabled: false
+
diff --git a/tests/sources.list.d.expected/options_comment.list b/tests/sources.list.d.expected/options_comment.list
new file mode 100644
index 0000000..f0952e4
--- /dev/null
+++ b/tests/sources.list.d.expected/options_comment.list
@@ -0,0 +1,3 @@
+# comment
+deb [ lang=it,de arch=amd64 ] http://ftp.at.debian.org/debian buster main contrib
+
diff --git a/tests/sources.list.d.expected/pbs-enterprise.list b/tests/sources.list.d.expected/pbs-enterprise.list
new file mode 100644
index 0000000..acb2990
--- /dev/null
+++ b/tests/sources.list.d.expected/pbs-enterprise.list
@@ -0,0 +1,2 @@
+deb https://enterprise.proxmox.com/debian/pbs buster pbs-enterprise
+
diff --git a/tests/sources.list.d.expected/pve.list b/tests/sources.list.d.expected/pve.list
new file mode 100644
index 0000000..127a49a
--- /dev/null
+++ b/tests/sources.list.d.expected/pve.list
@@ -0,0 +1,13 @@
+deb http://ftp.debian.org/debian buster main contrib
+
+deb http://ftp.debian.org/debian buster-updates main contrib
+
+# PVE pve-no-subscription repository provided by proxmox.com,
+# NOT recommended for production use
+deb http://download.proxmox.com/debian/pve buster pve-no-subscription
+
+# deb https://enterprise.proxmox.com/debian/pve buster pve-enterprise
+
+# security updates
+deb http://security.debian.org/debian-security buster/updates main contrib
+
diff --git a/tests/sources.list.d.expected/standard.list b/tests/sources.list.d.expected/standard.list
new file mode 100644
index 0000000..63c1b60
--- /dev/null
+++ b/tests/sources.list.d.expected/standard.list
@@ -0,0 +1,7 @@
+deb http://ftp.at.debian.org/debian buster main contrib
+
+deb http://ftp.at.debian.org/debian buster-updates main contrib
+
+# security updates
+deb http://security.debian.org buster/updates main contrib
+
diff --git a/tests/sources.list.d.expected/standard.sources b/tests/sources.list.d.expected/standard.sources
new file mode 100644
index 0000000..56ce280
--- /dev/null
+++ b/tests/sources.list.d.expected/standard.sources
@@ -0,0 +1,11 @@
+Types: deb
+URIs: http://ftp.at.debian.org/debian
+Suites: buster buster-updates
+Components: main contrib
+
+# security updates
+Types: deb
+URIs: http://security.debian.org
+Suites: buster/updates
+Components: main contrib
+
diff --git a/tests/sources.list.d/multiline.sources b/tests/sources.list.d/multiline.sources
new file mode 100644
index 0000000..c3a1ff0
--- /dev/null
+++ b/tests/sources.list.d/multiline.sources
@@ -0,0 +1,9 @@
+Types: deb deb-src
+URIs: http://ftp.at.debian.org/debian
+Suites: buster buster-updates
+# comment in here
+Components: main contrib
+Languages: it
+ de
+	fr
+Enabled: off
diff --git a/tests/sources.list.d/options_comment.list b/tests/sources.list.d/options_comment.list
new file mode 100644
index 0000000..e3f4112
--- /dev/null
+++ b/tests/sources.list.d/options_comment.list
@@ -0,0 +1,2 @@
+deb [ lang=it,de arch=amd64 ] http://ftp.at.debian.org/debian buster main contrib # comment
+
diff --git a/tests/sources.list.d/pbs-enterprise.list b/tests/sources.list.d/pbs-enterprise.list
new file mode 100644
index 0000000..5f8763c
--- /dev/null
+++ b/tests/sources.list.d/pbs-enterprise.list
@@ -0,0 +1 @@
+deb https://enterprise.proxmox.com/debian/pbs buster pbs-enterprise
diff --git a/tests/sources.list.d/pve.list b/tests/sources.list.d/pve.list
new file mode 100644
index 0000000..6213f72
--- /dev/null
+++ b/tests/sources.list.d/pve.list
@@ -0,0 +1,10 @@
+deb http://ftp.debian.org/debian buster main contrib
+deb http://ftp.debian.org/debian buster-updates main contrib
+
+# PVE pve-no-subscription repository provided by proxmox.com,
+# NOT recommended for production use
+deb http://download.proxmox.com/debian/pve buster pve-no-subscription
+# deb https://enterprise.proxmox.com/debian/pve buster pve-enterprise
+
+# security updates
+deb http://security.debian.org/debian-security buster/updates main contrib
diff --git a/tests/sources.list.d/standard.list b/tests/sources.list.d/standard.list
new file mode 100644
index 0000000..26db887
--- /dev/null
+++ b/tests/sources.list.d/standard.list
@@ -0,0 +1,6 @@
+deb http://ftp.at.debian.org/debian buster main contrib
+
+deb http://ftp.at.debian.org/debian buster-updates main contrib
+
+# security updates
+deb http://security.debian.org buster/updates main contrib
diff --git a/tests/sources.list.d/standard.sources b/tests/sources.list.d/standard.sources
new file mode 100644
index 0000000..605202e
--- /dev/null
+++ b/tests/sources.list.d/standard.sources
@@ -0,0 +1,10 @@
+Types: deb
+URIs: http://ftp.at.debian.org/debian
+Suites: buster buster-updates
+Components: main contrib
+
+# security updates
+Types: deb
+URIs: http://security.debian.org
+Suites: buster/updates
+Components: main contrib
-- 
2.20.1





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

* [pbs-devel] [PATCH v2 proxmox-apt 02/10] add files for Debian packaging
  2021-02-26 15:09 [pbs-devel] [PATCH/RFC-SERIES v2] APT repositories API/UI Fabian Ebner
  2021-02-26 15:09 ` [pbs-devel] [PATCH v2 proxmox-apt 01/10] initial commit Fabian Ebner
@ 2021-02-26 15:09 ` Fabian Ebner
  2021-02-26 15:09 ` [pbs-devel] [PATCH v2 proxmox-apt 03/10] add functions to check for Proxmox repositories Fabian Ebner
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Fabian Ebner @ 2021-02-26 15:09 UTC (permalink / raw)
  To: pbs-devel

The Makefile is based on the one from Mira's conntrack series, as it already got
some review.

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

New in v2.

 .gitignore           |  5 ++++
 Makefile             | 64 ++++++++++++++++++++++++++++++++++++++++++++
 debian/changelog     |  5 ++++
 debian/copyright     | 16 +++++++++++
 debian/debcargo.toml |  7 +++++
 5 files changed, 97 insertions(+)
 create mode 100644 Makefile
 create mode 100644 debian/changelog
 create mode 100644 debian/copyright
 create mode 100644 debian/debcargo.toml

diff --git a/.gitignore b/.gitignore
index 0d00c41..6c46431 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,3 +1,8 @@
 Cargo.lock
 target/
 tests/sources.list.d.actual
+proxmox-apt-*/
+*proxmox-apt*.buildinfo
+*proxmox-apt*.tar.?z
+*proxmox-apt*.changes
+*proxmox-apt*.deb
diff --git a/Makefile b/Makefile
new file mode 100644
index 0000000..de3d620
--- /dev/null
+++ b/Makefile
@@ -0,0 +1,64 @@
+include /usr/share/dpkg/pkg-info.mk
+include /usr/share/dpkg/architecture.mk
+
+PACKAGE=proxmox-apt
+BUILDDIR ?= $(PACKAGE)-$(DEB_VERSION_UPSTREAM)
+BUILDDIR_TMP ?= $(BUILDDIR).tmp
+
+DEB=librust-$(PACKAGE)-dev_$(DEB_VERSION_UPSTREAM_REVISION)_$(DEB_BUILD_ARCH).deb
+DSC=rust-$(PACKAGE)_$(DEB_VERSION_UPSTREAM_REVISION).dsc
+
+ifeq ($(BUILD_MODE), release)
+CARGO_BUILD_ARGS += --release
+COMPILEDIR := target/release
+else
+COMPILEDIR := target/debug
+endif
+
+all: cargo-build $(SUBDIRS)
+
+.PHONY: cargo-build
+cargo-build:
+	cargo build $(CARGO_BUILD_ARGS)
+
+.PHONY: build
+build:
+	rm -rf $(BUILDDIR) $(BUILDDIR_TMP); mkdir $(BUILDDIR_TMP)
+	debcargo package \
+	--config debian/debcargo.toml \
+	--changelog-ready \
+	--no-overlay-write-back \
+	--directory $(BUILDDIR_TMP) \
+	$(PACKAGE) \
+	$(shell dpkg-parsechangelog -l debian/changelog -SVersion | sed -e 's/-.*//')
+	rm -f $(BUILDDIR_TMP)/Cargo.lock
+	find $(BUILDDIR_TMP)/debian -name "*.hint" -delete
+	mv $(BUILDDIR_TMP) $(BUILDDIR)
+
+.PHONY: deb
+deb: $(DEB)
+$(DEB): build
+	cd $(BUILDDIR); dpkg-buildpackage -b -us -uc --no-pre-clean
+	lintian $(DEB)
+
+.PHONY: dsc
+dsc: $(DSC)
+$(DSC): build
+	cd $(BUILDDIR); dpkg-buildpackage -S -us -uc -d -nc
+	lintian $(DSC)
+
+.PHONY: dinstall
+dinstall: $(DEB)
+	dpkg -i $(DEB)
+
+.PHONY: upload
+upload: $(DEB) $(DBG_DEB)
+	tar cf - $(DEB) $(DBG_DEB) | ssh -X repoman@repo.proxmox.com -- upload --product pbs,pmg,pve --dist buster --arch $(DEB_BUILD_ARCH)
+
+.PHONY: distclean
+distclean: clean
+
+.PHONY: clean
+clean:
+	rm -rf *.deb *.buildinfo *.changes *.dsc rust-$(PACKAGE)_*.tar.?z $(BUILDDIR) $(BUILDDIR_TMP)
+	find . -name '*~' -exec rm {} ';'
diff --git a/debian/changelog b/debian/changelog
new file mode 100644
index 0000000..11e26ed
--- /dev/null
+++ b/debian/changelog
@@ -0,0 +1,5 @@
+rust-proxmox-apt (0.1.0-1) unstable; urgency=medium
+
+  * Initial release.
+
+ -- Proxmox Support Team <support@proxmox.com>  Thu, 18 Feb 2021 10:20:44 +0100
diff --git a/debian/copyright b/debian/copyright
new file mode 100644
index 0000000..5661ef6
--- /dev/null
+++ b/debian/copyright
@@ -0,0 +1,16 @@
+Copyright (C) 2021 Proxmox Server Solutions GmbH
+
+This software is written by Proxmox Server Solutions GmbH <support@proxmox.com>
+
+This program is free software: you can redistribute it and/or modify
+it under the terms of the GNU Affero General Public License as published by
+the Free Software Foundation, either version 3 of the License, or
+(at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU Affero General Public License for more details.
+
+You should have received a copy of the GNU Affero General Public License
+along with this program.  If not, see <http://www.gnu.org/licenses/>.
diff --git a/debian/debcargo.toml b/debian/debcargo.toml
new file mode 100644
index 0000000..74e3854
--- /dev/null
+++ b/debian/debcargo.toml
@@ -0,0 +1,7 @@
+overlay = "."
+crate_src_path = ".."
+maintainer = "Proxmox Support Team <support@proxmox.com>"
+
+[source]
+vcs_git = "git://git.proxmox.com/git/proxmox-apt.git"
+vcs_browser = "https://git.proxmox.com/?p=proxmox-apt.git"
-- 
2.20.1





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

* [pbs-devel] [PATCH v2 proxmox-apt 03/10] add functions to check for Proxmox repositories
  2021-02-26 15:09 [pbs-devel] [PATCH/RFC-SERIES v2] APT repositories API/UI Fabian Ebner
  2021-02-26 15:09 ` [pbs-devel] [PATCH v2 proxmox-apt 01/10] initial commit Fabian Ebner
  2021-02-26 15:09 ` [pbs-devel] [PATCH v2 proxmox-apt 02/10] add files for Debian packaging Fabian Ebner
@ 2021-02-26 15:09 ` Fabian Ebner
  2021-02-26 15:09 ` [pbs-devel] [PATCH v2 proxmox-apt 04/10] add check_repositories function Fabian Ebner
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Fabian Ebner @ 2021-02-26 15:09 UTC (permalink / raw)
  To: pbs-devel

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

Changes from v1:
    * split this and checks with suites into two also API-wise
    * add tests

 src/repositories/check.rs | 42 +++++++++++++++++++++++++++++++++++++++
 src/repositories/mod.rs   | 21 ++++++++++++++++++++
 tests/repositories.rs     | 29 ++++++++++++++++++++++++++-
 3 files changed, 91 insertions(+), 1 deletion(-)

diff --git a/src/repositories/check.rs b/src/repositories/check.rs
index 87fbbac..d0656cd 100644
--- a/src/repositories/check.rs
+++ b/src/repositories/check.rs
@@ -44,4 +44,46 @@ impl APTRepository {
 
         Ok(())
     }
+
+    /// Checks if the repository is the no-subscription repository of the specified
+    /// Proxmox product.
+    pub fn is_no_subscription(&self, product: &str) -> bool {
+        let base_uri = "http://download.proxmox.com/debian";
+        let no_subscription_uri = format!("{}/{}", base_uri, product);
+        let no_subscription_component = format!("{}-no-subscription", product);
+
+        if self
+            .uris
+            .iter()
+            .any(|uri| uri.trim_end_matches('/') == no_subscription_uri)
+        {
+            return self
+                .components
+                .iter()
+                .any(|comp| *comp == no_subscription_component);
+        } else {
+            false
+        }
+    }
+
+    /// Checks if the repository is the enterprise repository of the specified
+    /// Proxmox product.
+    pub fn is_enterprise(&self, product: &str) -> bool {
+        let base_uri = "https://enterprise.proxmox.com/debian";
+        let enterprise_uri = format!("{}/{}", base_uri, product);
+        let enterprise_component = format!("{}-enterprise", product);
+
+        if self
+            .uris
+            .iter()
+            .any(|uri| uri.trim_end_matches('/') == enterprise_uri)
+        {
+            return self
+                .components
+                .iter()
+                .any(|comp| *comp == enterprise_component);
+        } else {
+            false
+        }
+    }
 }
diff --git a/src/repositories/mod.rs b/src/repositories/mod.rs
index 123222a..4951cb1 100644
--- a/src/repositories/mod.rs
+++ b/src/repositories/mod.rs
@@ -105,6 +105,27 @@ fn check_filename<P: AsRef<Path>>(
     Ok(Some((file_type, path_string)))
 }
 
+/// Checks if the enterprise repository for the specified Proxmox product is
+/// configured and enabled.
+pub fn enterprise_repository_enabled<A: AsRef<APTRepository>>(repos: &[A], product: &str) -> bool {
+    repos.iter().any(|repo| {
+        let repo = repo.as_ref();
+        repo.enabled && repo.is_enterprise(product)
+    })
+}
+
+/// Checks if the no-subscription repository for the specified Proxmox product
+/// is configured and enabled.
+pub fn no_subscription_repository_enabled<A: AsRef<APTRepository>>(
+    repos: &[A],
+    product: &str,
+) -> bool {
+    repos.iter().any(|repo| {
+        let repo = repo.as_ref();
+        repo.enabled && repo.is_no_subscription(product)
+    })
+}
+
 /// Returns all APT repositories configured in the specified files including
 /// disabled ones.
 /// Warns about invalid file names and some format violations, while other
diff --git a/tests/repositories.rs b/tests/repositories.rs
index 020e133..deb2f6a 100644
--- a/tests/repositories.rs
+++ b/tests/repositories.rs
@@ -3,7 +3,10 @@ use std::path::PathBuf;
 
 use anyhow::{bail, format_err, Error};
 
-use proxmox_apt::repositories::{repositories_from_files, write_repositories};
+use proxmox_apt::repositories::{
+    enterprise_repository_enabled, no_subscription_repository_enabled, repositories_from_files,
+    write_repositories,
+};
 use proxmox_apt::types::APTRepository;
 
 #[test]
@@ -71,3 +74,27 @@ fn test_parse_write() -> Result<(), Error> {
 
     Ok(())
 }
+
+#[test]
+fn test_proxmox_repositories() -> Result<(), Error> {
+    let test_dir = std::env::current_dir()?.join("tests");
+    let read_dir = test_dir.join("sources.list.d");
+
+    let pve_list = read_dir.join("pve.list");
+    let repos = repositories_from_files(&vec![pve_list])?;
+
+    assert_eq!(false, enterprise_repository_enabled(&repos, "pbs"));
+    assert_eq!(false, enterprise_repository_enabled(&repos, "pve"));
+    assert_eq!(false, no_subscription_repository_enabled(&repos, "pmg"));
+    assert_eq!(true, no_subscription_repository_enabled(&repos, "pve"));
+
+    let pbs_list = read_dir.join("pbs-enterprise.list");
+    let repos = repositories_from_files(&vec![pbs_list])?;
+
+    assert_eq!(true, enterprise_repository_enabled(&repos, "pbs"));
+    assert_eq!(false, enterprise_repository_enabled(&repos, "pve"));
+    assert_eq!(false, no_subscription_repository_enabled(&repos, "pmg"));
+    assert_eq!(false, no_subscription_repository_enabled(&repos, "pve"));
+
+    Ok(())
+}
-- 
2.20.1





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

* [pbs-devel] [PATCH v2 proxmox-apt 04/10] add check_repositories function
  2021-02-26 15:09 [pbs-devel] [PATCH/RFC-SERIES v2] APT repositories API/UI Fabian Ebner
                   ` (2 preceding siblings ...)
  2021-02-26 15:09 ` [pbs-devel] [PATCH v2 proxmox-apt 03/10] add functions to check for Proxmox repositories Fabian Ebner
@ 2021-02-26 15:09 ` Fabian Ebner
  2021-03-10 15:17   ` Wolfgang Bumiller
  2021-02-26 15:09 ` [pbs-devel] [PATCH v2 proxmox-backup 05/10] cargo: depend on new proxmox-apt crate Fabian Ebner
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 13+ messages in thread
From: Fabian Ebner @ 2021-02-26 15:09 UTC (permalink / raw)
  To: pbs-devel

which, for now, checks the suites.

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

Changes from v1:
    * split this and information about Proxmox repositories in two
    * add tests

 src/repositories/check.rs                 | 78 ++++++++++++++++++++++-
 src/repositories/mod.rs                   | 16 ++++-
 src/types.rs                              | 30 +++++++++
 tests/repositories.rs                     | 60 ++++++++++++++++-
 tests/sources.list.d.expected/bad.sources | 20 ++++++
 tests/sources.list.d/bad.sources          | 19 ++++++
 6 files changed, 219 insertions(+), 4 deletions(-)
 create mode 100644 tests/sources.list.d.expected/bad.sources
 create mode 100644 tests/sources.list.d/bad.sources

diff --git a/src/repositories/check.rs b/src/repositories/check.rs
index d0656cd..7726ff3 100644
--- a/src/repositories/check.rs
+++ b/src/repositories/check.rs
@@ -1,6 +1,23 @@
 use anyhow::{bail, Error};
 
-use crate::types::{APTRepository, APTRepositoryFileType};
+use crate::types::{
+    APTRepository, APTRepositoryFileType, APTRepositoryPackageType, APTRepositoryWarning,
+};
+
+/// Checks if `suite` is some variant of `base_suite`, e.g. `buster-backports`
+/// is a variant of `buster`.
+#[allow(clippy::useless_format)]
+fn suite_is_variant(suite: &str, base_suite: &str) -> bool {
+    let variants = vec![
+        format!("{}", base_suite),
+        format!("{}-backports", base_suite),
+        format!("{}-backports-sloppy", base_suite),
+        format!("{}-updates", base_suite),
+        format!("{}/updates", base_suite),
+    ];
+
+    variants.iter().any(|variant| *variant == suite)
+}
 
 impl APTRepository {
     /// Makes sure that all basic properties of a repository are present and
@@ -45,6 +62,65 @@ impl APTRepository {
         Ok(())
     }
 
+    /// Checks if old or unstable suites are configured and also that the `stable`
+    /// keyword is not used.
+    pub fn check_suites(&self) -> Vec<APTRepositoryWarning> {
+        let old_suites = vec![
+            "lenny",
+            "squeeze",
+            "wheezy",
+            "jessie",
+            "stretch",
+            "oldoldstable",
+            "oldstable",
+        ];
+
+        let new_suites = vec!["unstable", "sid", "experimental"];
+
+        let mut warnings = vec![];
+
+        let mut add_warning = |warning| {
+            warnings.push(APTRepositoryWarning {
+                path: self.path.clone(),
+                number: self.number,
+                warning,
+            });
+        };
+
+        if self
+            .types
+            .iter()
+            .any(|package_type| *package_type == APTRepositoryPackageType::Deb)
+        {
+            for suite in self.suites.iter() {
+                if old_suites
+                    .iter()
+                    .any(|base_suite| suite_is_variant(suite, base_suite))
+                {
+                    add_warning(format!("old suite '{}' configured!", suite));
+                }
+
+                if new_suites
+                    .iter()
+                    .any(|base_suite| suite_is_variant(suite, base_suite))
+                {
+                    add_warning(format!(
+                        "suite '{}' should not be used in production!",
+                        suite,
+                    ));
+                }
+
+                if suite_is_variant(suite, "stable") {
+                    add_warning(
+                        "use the name of the stable distribution instead of 'stable'!".to_string(),
+                    );
+                }
+            }
+        }
+
+        warnings
+    }
+
     /// Checks if the repository is the no-subscription repository of the specified
     /// Proxmox product.
     pub fn is_no_subscription(&self, product: &str) -> bool {
diff --git a/src/repositories/mod.rs b/src/repositories/mod.rs
index 4951cb1..38933ef 100644
--- a/src/repositories/mod.rs
+++ b/src/repositories/mod.rs
@@ -5,7 +5,9 @@ use std::path::{Path, PathBuf};
 
 use anyhow::{bail, format_err, Error};
 
-use crate::types::{APTRepository, APTRepositoryFileType, APTRepositoryOption};
+use crate::types::{
+    APTRepository, APTRepositoryFileType, APTRepositoryOption, APTRepositoryWarning,
+};
 
 mod list_parser;
 use list_parser::APTListFileParser;
@@ -105,6 +107,18 @@ fn check_filename<P: AsRef<Path>>(
     Ok(Some((file_type, path_string)))
 }
 
+/// Produces warnings if there are problems withe the repositories.
+/// Currently only checks the suites.
+pub fn check_repositories<A: AsRef<APTRepository>>(repos: &[A]) -> Vec<APTRepositoryWarning> {
+    let mut warnings = vec![];
+
+    for repo in repos.iter() {
+        warnings.append(&mut repo.as_ref().check_suites());
+    }
+
+    warnings
+}
+
 /// Checks if the enterprise repository for the specified Proxmox product is
 /// configured and enabled.
 pub fn enterprise_repository_enabled<A: AsRef<APTRepository>>(repos: &[A], product: &str) -> bool {
diff --git a/src/types.rs b/src/types.rs
index 7a95daf..1a4a50e 100644
--- a/src/types.rs
+++ b/src/types.rs
@@ -195,3 +195,33 @@ impl AsRef<APTRepository> for APTRepository {
         &self
     }
 }
+
+#[api(
+    properties: {
+        Path: {
+            description: "Path to the defining file.",
+            type: String,
+        },
+        Number: {
+            description: "Line or stanza number.",
+            type: Integer,
+        },
+        Warning: {
+            description: "Warning message.",
+            type: String,
+        },
+    },
+)]
+#[derive(Debug, Clone, Serialize, Deserialize)]
+#[serde(rename_all = "PascalCase")]
+/// Warning for an APT repository.
+pub struct APTRepositoryWarning {
+    /// Path to the defining file.
+    #[serde(skip_serializing_if = "String::is_empty")]
+    pub path: String,
+    /// Line or stanza number.
+    pub number: usize,
+    /// Warning
+    #[serde(skip_serializing_if = "String::is_empty")]
+    pub warning: String,
+}
diff --git a/tests/repositories.rs b/tests/repositories.rs
index deb2f6a..1b548bd 100644
--- a/tests/repositories.rs
+++ b/tests/repositories.rs
@@ -4,8 +4,8 @@ use std::path::PathBuf;
 use anyhow::{bail, format_err, Error};
 
 use proxmox_apt::repositories::{
-    enterprise_repository_enabled, no_subscription_repository_enabled, repositories_from_files,
-    write_repositories,
+    check_repositories, enterprise_repository_enabled, no_subscription_repository_enabled,
+    repositories_from_files, write_repositories,
 };
 use proxmox_apt::types::APTRepository;
 
@@ -98,3 +98,59 @@ fn test_proxmox_repositories() -> Result<(), Error> {
 
     Ok(())
 }
+
+#[test]
+fn test_check_repositories() -> Result<(), Error> {
+    let test_dir = std::env::current_dir()?.join("tests");
+    let read_dir = test_dir.join("sources.list.d");
+
+    let pve_list = read_dir.join("pve.list");
+    let repos = repositories_from_files(&vec![pve_list])?;
+
+    let warnings = check_repositories(&repos);
+
+    assert_eq!(warnings.is_empty(), true);
+
+    let bad_sources = read_dir.join("bad.sources");
+    let repos = repositories_from_files(&vec![bad_sources])?;
+
+    let mut expected_warnings = HashMap::<String, String>::new();
+    expected_warnings.insert(
+        "bad.sources:1".to_string(),
+        "suite 'sid' should not be used in production!".to_string(),
+    );
+    expected_warnings.insert(
+        "bad.sources:2".to_string(),
+        "old suite 'lenny-backports' configured!".to_string(),
+    );
+    expected_warnings.insert(
+        "bad.sources:3".to_string(),
+        "old suite 'stretch/updates' configured!".to_string(),
+    );
+    expected_warnings.insert(
+        "bad.sources:4".to_string(),
+        "use the name of the stable distribution instead of 'stable'!".to_string(),
+    );
+
+    let warnings = check_repositories(&repos);
+
+    assert_eq!(warnings.len(), expected_warnings.len());
+
+    for warning in warnings.iter() {
+        let path = PathBuf::from(warning.path.clone());
+        let file_name = path
+            .file_name()
+            .unwrap()
+            .to_os_string()
+            .into_string()
+            .unwrap();
+
+        let key = format!("{}:{}", file_name, warning.number);
+        let message = expected_warnings.get(&key);
+
+        assert_eq!(message.is_some(), true);
+        assert_eq!(*message.unwrap(), warning.warning);
+    }
+
+    Ok(())
+}
diff --git a/tests/sources.list.d.expected/bad.sources b/tests/sources.list.d.expected/bad.sources
new file mode 100644
index 0000000..7601263
--- /dev/null
+++ b/tests/sources.list.d.expected/bad.sources
@@ -0,0 +1,20 @@
+Types: deb
+URIs: http://ftp.at.debian.org/debian
+Suites: sid
+Components: main contrib
+
+Types: deb
+URIs: http://ftp.at.debian.org/debian
+Suites: lenny-backports
+Components: contrib
+
+Types: deb
+URIs: http://security.debian.org
+Suites: stretch/updates
+Components: main contrib
+
+Types: deb
+URIs: http://ftp.at.debian.org/debian
+Suites: stable
+Components: main
+
diff --git a/tests/sources.list.d/bad.sources b/tests/sources.list.d/bad.sources
new file mode 100644
index 0000000..963f527
--- /dev/null
+++ b/tests/sources.list.d/bad.sources
@@ -0,0 +1,19 @@
+Types: deb
+URIs: http://ftp.at.debian.org/debian
+Suites: sid
+Components: main contrib
+
+Types: deb
+URIs: http://ftp.at.debian.org/debian
+Suites: lenny-backports
+Components: contrib
+
+Types: deb
+URIs: http://security.debian.org
+Suites: stretch/updates
+Components: main contrib
+
+Suites: stable
+URIs: http://ftp.at.debian.org/debian
+Components: main
+Types: deb
-- 
2.20.1





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

* [pbs-devel] [PATCH v2 proxmox-backup 05/10] cargo: depend on new proxmox-apt crate
  2021-02-26 15:09 [pbs-devel] [PATCH/RFC-SERIES v2] APT repositories API/UI Fabian Ebner
                   ` (3 preceding siblings ...)
  2021-02-26 15:09 ` [pbs-devel] [PATCH v2 proxmox-apt 04/10] add check_repositories function Fabian Ebner
@ 2021-02-26 15:09 ` Fabian Ebner
  2021-02-26 15:09 ` [pbs-devel] [PATCH v2 proxmox-backup 06/10] api: apt: add repositories call Fabian Ebner
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Fabian Ebner @ 2021-02-26 15:09 UTC (permalink / raw)
  To: pbs-devel

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

New in v2.

 Cargo.toml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Cargo.toml b/Cargo.toml
index cc821691..5d34e4b6 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -51,6 +51,7 @@ pathpatterns = "0.1.2"
 proxmox = { version = "0.11.0", features = [ "sortable-macro", "api-macro", "websocket" ] }
 #proxmox = { git = "git://git.proxmox.com/git/proxmox", version = "0.1.2", features = [ "sortable-macro", "api-macro" ] }
 #proxmox = { path = "../proxmox/proxmox", features = [ "sortable-macro", "api-macro", "websocket" ] }
+proxmox-apt = "0.1.0"
 proxmox-fuse = "0.1.1"
 pxar = { version = "0.9.0", features = [ "tokio-io" ] }
 #pxar = { path = "../pxar", features = [ "tokio-io" ] }
-- 
2.20.1





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

* [pbs-devel] [PATCH v2 proxmox-backup 06/10] api: apt: add repositories call
  2021-02-26 15:09 [pbs-devel] [PATCH/RFC-SERIES v2] APT repositories API/UI Fabian Ebner
                   ` (4 preceding siblings ...)
  2021-02-26 15:09 ` [pbs-devel] [PATCH v2 proxmox-backup 05/10] cargo: depend on new proxmox-apt crate Fabian Ebner
@ 2021-02-26 15:09 ` Fabian Ebner
  2021-02-26 15:09 ` [pbs-devel] [PATCH v2 widget-toolkit 07/10] add UI for APT repositories Fabian Ebner
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Fabian Ebner @ 2021-02-26 15:09 UTC (permalink / raw)
  To: pbs-devel

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

New in v2.

 src/api2/node/apt.rs | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/src/api2/node/apt.rs b/src/api2/node/apt.rs
index 9d723f17..5584972b 100644
--- a/src/api2/node/apt.rs
+++ b/src/api2/node/apt.rs
@@ -6,6 +6,8 @@ use proxmox::list_subdirs_api_method;
 use proxmox::api::{api, RpcEnvironment, RpcEnvironmentType, Permission};
 use proxmox::api::router::{Router, SubdirMap};
 
+use proxmox_apt::types::APTRepository;
+
 use crate::server::WorkerTask;
 use crate::tools::{apt, http, subscription};
 
@@ -350,8 +352,33 @@ pub fn get_versions() -> Result<Vec<APTUpdateInfo>, Error> {
     Ok(packages)
 }
 
+#[api(
+    input: {
+        properties: {
+            node: {
+                schema: NODE_SCHEMA,
+            },
+        },
+    },
+    returns: {
+        description: "List of configured APT repositories",
+        type: Array,
+        items: {
+            type: APTRepository,
+        },
+    },
+    access: {
+        permission: &Permission::Privilege(&[], PRIV_SYS_AUDIT, false),
+    },
+)]
+/// Get APT repository information
+pub fn get_repositories() -> Result<Vec<APTRepository>, Error> {
+    proxmox_apt::repositories::repositories()
+}
+
 const SUBDIRS: SubdirMap = &[
     ("changelog", &Router::new().get(&API_METHOD_APT_GET_CHANGELOG)),
+    ("repositories", &Router::new().get(&API_METHOD_GET_REPOSITORIES)),
     ("update", &Router::new()
         .get(&API_METHOD_APT_UPDATE_AVAILABLE)
         .post(&API_METHOD_APT_UPDATE_DATABASE)
-- 
2.20.1





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

* [pbs-devel] [PATCH v2 widget-toolkit 07/10] add UI for APT repositories
  2021-02-26 15:09 [pbs-devel] [PATCH/RFC-SERIES v2] APT repositories API/UI Fabian Ebner
                   ` (5 preceding siblings ...)
  2021-02-26 15:09 ` [pbs-devel] [PATCH v2 proxmox-backup 06/10] api: apt: add repositories call Fabian Ebner
@ 2021-02-26 15:09 ` Fabian Ebner
  2021-02-26 15:09 ` [pbs-devel] [PATCH v2 proxmox-backup 08/10] ui: add grid " Fabian Ebner
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Fabian Ebner @ 2021-02-26 15:09 UTC (permalink / raw)
  To: pbs-devel

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

Changes from v1:
    * Make sure grid reloads when the tab is switched back and to the window,
      which is relevant for PBS.
    * Avoid extracting file type from path, now it's returned by the API.
    * Adapt to options being an array of key+values pairs.

 src/Makefile                |   1 +
 src/node/APTRepositories.js | 145 ++++++++++++++++++++++++++++++++++++
 2 files changed, 146 insertions(+)
 create mode 100644 src/node/APTRepositories.js

diff --git a/src/Makefile b/src/Makefile
index 46b90ae..f172d55 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -54,6 +54,7 @@ JSSRC=					\
 	window/DiskSmart.js		\
 	window/ZFSDetail.js		\
 	node/APT.js			\
+	node/APTRepositories.js		\
 	node/NetworkEdit.js		\
 	node/NetworkView.js		\
 	node/DNSEdit.js			\
diff --git a/src/node/APTRepositories.js b/src/node/APTRepositories.js
new file mode 100644
index 0000000..0b21a8a
--- /dev/null
+++ b/src/node/APTRepositories.js
@@ -0,0 +1,145 @@
+Ext.define('apt-repolist', {
+    extend: 'Ext.data.Model',
+    fields: [
+	'Path',
+	'Number',
+	'FileType',
+	'Enabled',
+	'Comment',
+	'Types',
+	'URIs',
+	'Suites',
+	'Components',
+	'Options',
+    ],
+});
+
+Ext.define('Proxmox.node.APTRepositories', {
+    extend: 'Ext.grid.GridPanel',
+
+    xtype: 'proxmoxNodeAPTRepositories',
+
+    sortableColumns: false,
+
+    columns: [
+	{
+	    header: gettext('Enabled'),
+	    dataIndex: 'Enabled',
+	    renderer: Proxmox.Utils.format_enabled_toggle,
+	    width: 90,
+	},
+	{
+	    header: gettext('Types'),
+	    dataIndex: 'Types',
+	    renderer: function(options, cell, record) {
+		return record.data.Types.join(' ');
+	    },
+	    width: 100,
+	},
+	{
+	    header: gettext('URIs'),
+	    dataIndex: 'URIs',
+	    renderer: function(options, cell, record) {
+		return record.data.URIs.join(' ');
+	    },
+	    width: 350,
+	},
+	{
+	    header: gettext('Suites'),
+	    dataIndex: 'Suites',
+	    renderer: function(options, cell, record) {
+		return record.data.Suites.join(' ');
+	    },
+	    width: 130,
+	},
+	{
+	    header: gettext('Components'),
+	    dataIndex: 'Components',
+	    renderer: function(options, cell, record) {
+		return record.data.Components.join(' ');
+	    },
+	    width: 170,
+	},
+	{
+	    header: gettext('Options'),
+	    dataIndex: 'Options',
+	    renderer: function(options, cell, record) {
+		if (!options) {
+		    return '';
+		}
+
+		let filetype = record.data.FileType;
+		let text = '';
+
+		options.forEach(function(option) {
+		    let key = option.Key;
+		    if (filetype === 'list') {
+			let values = option.Values.join(',');
+			text += `${key}=${values} `;
+		    } else if (filetype === 'sources') {
+			let values = option.Values.join(' ');
+			text += `${key}: ${values}\n`;
+		    } else {
+			throw "unkown file type";
+		    }
+		});
+		return text;
+	    },
+	    flex: 1,
+	},
+	{
+	    header: gettext('Comment'),
+	    dataIndex: 'Comment',
+	    flex: 1,
+	},
+    ],
+
+    initComponent: function() {
+	let me = this;
+
+	if (!me.nodename) {
+	    throw "no node name specified";
+	}
+
+	let store = Ext.create('Ext.data.Store', {
+	    model: 'apt-repolist',
+	    groupField: 'Path',
+	    proxy: {
+		type: 'proxmox',
+		url: `/api2/json/nodes/${me.nodename}/apt/repositories`,
+	    },
+	    sorters: [
+		{
+		    property: 'Number',
+		    direction: 'ASC',
+		},
+	    ],
+	});
+
+	let groupingFeature = Ext.create('Ext.grid.feature.Grouping', {
+	    groupHeaderTpl: '{[ "File: " + values.name ]} ({rows.length} ' +
+		'repositor{[values.rows.length > 1 ? "ies" : "y"]})',
+	    enableGroupingMenu: false,
+	});
+
+	let reload = function() {
+	    store.load();
+	};
+
+	let sm = Ext.create('Ext.selection.RowModel', {});
+
+	Ext.apply(me, {
+	    store: store,
+	    selModel: sm,
+	    features: [groupingFeature],
+	    listeners: {
+		activate: reload,
+	    },
+	});
+
+	Proxmox.Utils.monStoreErrors(me, store, true);
+	reload();
+
+	me.callParent();
+    },
+});
-- 
2.20.1





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

* [pbs-devel] [PATCH v2 proxmox-backup 08/10] ui: add grid for APT repositories
  2021-02-26 15:09 [pbs-devel] [PATCH/RFC-SERIES v2] APT repositories API/UI Fabian Ebner
                   ` (6 preceding siblings ...)
  2021-02-26 15:09 ` [pbs-devel] [PATCH v2 widget-toolkit 07/10] add UI for APT repositories Fabian Ebner
@ 2021-02-26 15:09 ` Fabian Ebner
  2021-02-26 15:09 ` [pbs-devel] [RFC v2 proxmox-backup 09/10] api: apt: add check_repositories_call Fabian Ebner
  2021-02-26 15:09 ` [pbs-devel] [RFC v2 widget-toolkit 10/10] APT: show warnings for repositories Fabian Ebner
  9 siblings, 0 replies; 13+ messages in thread
From: Fabian Ebner @ 2021-02-26 15:09 UTC (permalink / raw)
  To: pbs-devel

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

Changes from v1: 
    * PBS instead of PVE

 www/ServerAdministration.js | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/www/ServerAdministration.js b/www/ServerAdministration.js
index 0d803ac4..027a13b9 100644
--- a/www/ServerAdministration.js
+++ b/www/ServerAdministration.js
@@ -53,6 +53,13 @@ Ext.define('PBS.ServerAdministration', {
 	    itemId: 'updates',
 	    nodename: 'localhost',
 	},
+	{
+	    xtype: 'proxmoxNodeAPTRepositories',
+	    title: gettext('Repositories'),
+	    iconCls: 'fa fa-files-o',
+	    itemId: 'aptrepositories',
+	    nodename: 'localhost',
+	},
 	{
 	    xtype: 'proxmoxJournalView',
 	    itemId: 'logs',
-- 
2.20.1





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

* [pbs-devel] [RFC v2 proxmox-backup 09/10] api: apt: add check_repositories_call
  2021-02-26 15:09 [pbs-devel] [PATCH/RFC-SERIES v2] APT repositories API/UI Fabian Ebner
                   ` (7 preceding siblings ...)
  2021-02-26 15:09 ` [pbs-devel] [PATCH v2 proxmox-backup 08/10] ui: add grid " Fabian Ebner
@ 2021-02-26 15:09 ` Fabian Ebner
  2021-02-26 15:09 ` [pbs-devel] [RFC v2 widget-toolkit 10/10] APT: show warnings for repositories Fabian Ebner
  9 siblings, 0 replies; 13+ messages in thread
From: Fabian Ebner @ 2021-02-26 15:09 UTC (permalink / raw)
  To: pbs-devel

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

New in v2.

Sent as RFC, because maybe that's not the API we want. But it seems usable
enough.

 src/api2/node/apt.rs | 53 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 52 insertions(+), 1 deletion(-)

diff --git a/src/api2/node/apt.rs b/src/api2/node/apt.rs
index 5584972b..617d1920 100644
--- a/src/api2/node/apt.rs
+++ b/src/api2/node/apt.rs
@@ -6,7 +6,7 @@ use proxmox::list_subdirs_api_method;
 use proxmox::api::{api, RpcEnvironment, RpcEnvironmentType, Permission};
 use proxmox::api::router::{Router, SubdirMap};
 
-use proxmox_apt::types::APTRepository;
+use proxmox_apt::types::{APTRepository, APTRepositoryWarning};
 
 use crate::server::WorkerTask;
 use crate::tools::{apt, http, subscription};
@@ -352,6 +352,56 @@ pub fn get_versions() -> Result<Vec<APTUpdateInfo>, Error> {
     Ok(packages)
 }
 
+#[api(
+    input: {
+        properties: {
+            node: {
+                schema: NODE_SCHEMA,
+            },
+        },
+    },
+    returns: {
+        description: "Warnings for APT repositories and status of Proxmox repositories.",
+        properties: {
+            warnings: {
+                description: "Warnings for APT repositories.",
+                type: Array,
+                items: {
+                    type: APTRepositoryWarning,
+                },
+            },
+            enterprise: {
+                description: "Whether the enterprise repository is enabled.",
+                type: Boolean,
+            },
+            nosubscription: {
+                description: "Whether the no-subscription repository is enabled.",
+                type: Boolean,
+            },
+        },
+    },
+    access: {
+        permission: &Permission::Privilege(&[], PRIV_SYS_AUDIT, false),
+    },
+)]
+/// Check APT repository configuration.
+pub fn check_repositories() -> Result<Value, Error> {
+    let repos = proxmox_apt::repositories::repositories()?;
+
+    let warnings = proxmox_apt::repositories::check_repositories(&repos);
+
+    let enterprise_enabled =
+        proxmox_apt::repositories::enterprise_repository_enabled(&repos, "pbs");
+    let no_subscription_enabled =
+        proxmox_apt::repositories::no_subscription_repository_enabled(&repos, "pbs");
+
+    Ok(json!({
+        "warnings": warnings,
+        "enterprise": enterprise_enabled,
+        "nosubscription": no_subscription_enabled
+    }))
+}
+
 #[api(
     input: {
         properties: {
@@ -378,6 +428,7 @@ pub fn get_repositories() -> Result<Vec<APTRepository>, Error> {
 
 const SUBDIRS: SubdirMap = &[
     ("changelog", &Router::new().get(&API_METHOD_APT_GET_CHANGELOG)),
+    ("checkrepositories", &Router::new().get(&API_METHOD_CHECK_REPOSITORIES)),
     ("repositories", &Router::new().get(&API_METHOD_GET_REPOSITORIES)),
     ("update", &Router::new()
         .get(&API_METHOD_APT_UPDATE_AVAILABLE)
-- 
2.20.1





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

* [pbs-devel] [RFC v2 widget-toolkit 10/10] APT: show warnings for repositories
  2021-02-26 15:09 [pbs-devel] [PATCH/RFC-SERIES v2] APT repositories API/UI Fabian Ebner
                   ` (8 preceding siblings ...)
  2021-02-26 15:09 ` [pbs-devel] [RFC v2 proxmox-backup 09/10] api: apt: add check_repositories_call Fabian Ebner
@ 2021-02-26 15:09 ` Fabian Ebner
  9 siblings, 0 replies; 13+ messages in thread
From: Fabian Ebner @ 2021-02-26 15:09 UTC (permalink / raw)
  To: pbs-devel

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

Changes from v1:
    * Use the row body feature to pin warnings to the repository they are for
      and make warnings more prominent.
    * Also check the subscription status.

Sent as an RFC, because there's probably a better way than using the
title bar for the global warning.

 src/node/APTRepositories.js | 116 +++++++++++++++++++++++++++++++++++-
 1 file changed, 115 insertions(+), 1 deletion(-)

diff --git a/src/node/APTRepositories.js b/src/node/APTRepositories.js
index f4a3a43..c8033df 100644
--- a/src/node/APTRepositories.js
+++ b/src/node/APTRepositories.js
@@ -21,6 +21,51 @@ Ext.define('Proxmox.node.APTRepositories', {
 
     sortableColumns: false,
 
+    viewModel: {
+	data: {
+	    subscriptionActive: '',
+	    noSubscriptionRepo: '',
+	    enterpriseRepo: '',
+	},
+	formulas: {
+	    globalWarning: function(get) {
+		if (get('subscriptionActive') === '' ||
+		    get('noSubscriptionRepo') === '' ||
+		    get('enterpriseRepo') === '') {
+		    return '';
+		}
+
+		if (!get('subscriptionActive') && get('enterpriseRepo')) {
+		    return "WARNING: The enterprise repository is configured " +
+			"but there is no active subscription!";
+		}
+
+		if (get('noSubscriptionRepo')) {
+		    return "WARNING: The no-subscription repository is not " +
+			"recommended for production use!";
+		}
+
+		if (!get('enterpriseRepo') && !get('noSubscriptionRepo')) {
+		    return "WARNING: Neither the enterprise nor the " +
+			"no-subscription repository is active!";
+		}
+
+		return '';
+	    },
+	},
+    },
+
+    tbar: [
+	{
+	    xtype: 'label',
+	    text: 'default',
+	    style: 'color: red;',
+	    bind: {
+		text: '{globalWarning}',
+	    },
+	},
+    ],
+
     columns: [
 	{
 	    header: gettext('Enabled'),
@@ -94,6 +139,71 @@ Ext.define('Proxmox.node.APTRepositories', {
 	},
     ],
 
+    check_repositories: function(rowBodyFeature) {
+	let me = this;
+	let vm = me.getViewModel();
+
+	Proxmox.Utils.API2Request({
+	    url: `/nodes/${me.nodename}/apt/checkrepositories`,
+	    method: 'GET',
+	    failure: function(response, opts) {
+		Ext.Msg.alert(gettext('Error'), response.htmlStatus);
+	    },
+	    success: function(response, opts) {
+		const data = response.result.data;
+
+		vm.set('enterpriseRepo', data.enterprise);
+		vm.set('noSubscriptionRepo', data.nosubscription);
+
+		let warnings = {};
+		for (const warning of data.warnings) {
+		    const key = `${warning.Path}:${warning.Number}`;
+		    if (warnings[key]) {
+			warnings[key] += "\n";
+			warnings[key] += "Warning: " + warning.Warning;
+		    } else {
+			warnings[key] = "Warning: " + warning.Warning;
+		    }
+		}
+
+		rowBodyFeature.getAdditionalData = function(innerData, rowIndex, record, orig) {
+		    let headerCt = this.view.headerCt;
+		    let colspan = headerCt.getColumnCount();
+
+		    const key = `${innerData.Path}:${innerData.Number}`;
+		    const warning_text = warnings[key];
+
+		    return {
+			rowBody: '<div style="color: red; white-space: pre-line">' +
+			    Ext.String.htmlEncode(warning_text) +
+			    '</div>',
+			rowBodyCls: warning_text ? '' : Ext.baseCSSPrefix + 'grid-row-body-hidden',
+			rowBodyColspan: colspan,
+		    };
+		};
+	    },
+	});
+    },
+
+    check_subscription: function() {
+	let me = this;
+	let vm = me.getViewModel();
+
+	Proxmox.Utils.API2Request({
+	    url: `/nodes/${me.nodename}/subscription`,
+	    method: 'GET',
+	    failure: function(response, opts) {
+		Ext.Msg.alert(gettext('Error'), response.htmlStatus);
+	    },
+	    success: function(response, opts) {
+		const res = response.result;
+		const subscription = !(res === null || res === undefined ||
+		    !res || res.data.status.toLowerCase() !== 'active');
+		vm.set('subscriptionActive', subscription);
+	    },
+	});
+    },
+
     initComponent: function() {
 	let me = this;
 
@@ -116,6 +226,8 @@ Ext.define('Proxmox.node.APTRepositories', {
 	    ],
 	});
 
+	let rowBodyFeature = Ext.create('Ext.grid.feature.RowBody', {});
+
 	let groupingFeature = Ext.create('Ext.grid.feature.Grouping', {
 	    groupHeaderTpl: '{[ "File: " + values.name ]} ({rows.length} ' +
 		'repositor{[values.rows.length > 1 ? "ies" : "y"]})',
@@ -124,6 +236,8 @@ Ext.define('Proxmox.node.APTRepositories', {
 
 	let reload = function() {
 	    store.load();
+	    me.check_repositories(rowBodyFeature);
+	    me.check_subscription();
 	};
 
 	let sm = Ext.create('Ext.selection.RowModel', {});
@@ -131,7 +245,7 @@ Ext.define('Proxmox.node.APTRepositories', {
 	Ext.apply(me, {
 	    store: store,
 	    selModel: sm,
-	    features: [groupingFeature],
+	    features: [groupingFeature, rowBodyFeature],
 	    listeners: {
 		activate: reload,
 	    },
-- 
2.20.1





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

* Re: [pbs-devel] [PATCH v2 proxmox-apt 01/10] initial commit
  2021-02-26 15:09 ` [pbs-devel] [PATCH v2 proxmox-apt 01/10] initial commit Fabian Ebner
@ 2021-03-10 15:14   ` Wolfgang Bumiller
  0 siblings, 0 replies; 13+ messages in thread
From: Wolfgang Bumiller @ 2021-03-10 15:14 UTC (permalink / raw)
  To: Fabian Ebner; +Cc: pbs-devel

comments inline

On Fri, Feb 26, 2021 at 04:09:50PM +0100, Fabian Ebner wrote:
> This create is inteded to hold the code to interact with APT repositories and
> packages.
> 
> Starting off with code for parsing and writing .list and .sources repository
> configuration files. The parser is partially inspired by the network
> configuration parser in proxmox-backup, but it does not use a lexer,
> because there's not much variation in how things are to be parsed.
> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
> 
> Changes from v1:
>     * Switched from Perl to Rust.
>     * Split things by whitespace instead of using regexes everywhere,
>       being the cleaner approach.
>     * Used PascalCase for all keys for consistency.
>     * File type is now also explicitly returned.
>     * Added a writer and tests.
>     * The options are not a hash anymore, because it's necessary to keep
>       the same order when writing them back out (i.e. Languages-Add
>       Languages-Remove)
> 
>  .cargo/config                                 |   5 +
>  .gitignore                                    |   3 +
>  Cargo.toml                                    |  22 ++
>  rustfmt.toml                                  |   1 +
>  src/lib.rs                                    |   3 +
>  src/repositories/check.rs                     |  47 ++++
>  src/repositories/list_parser.rs               | 181 +++++++++++++
>  src/repositories/mod.rs                       | 246 ++++++++++++++++++
>  src/repositories/sources_parser.rs            | 217 +++++++++++++++
>  src/repositories/writer.rs                    |  89 +++++++
>  src/types.rs                                  | 197 ++++++++++++++
>  tests/repositories.rs                         |  73 ++++++
>  .../sources.list.d.expected/multiline.sources |   8 +
>  .../options_comment.list                      |   3 +
>  .../pbs-enterprise.list                       |   2 +
>  tests/sources.list.d.expected/pve.list        |  13 +
>  tests/sources.list.d.expected/standard.list   |   7 +
>  .../sources.list.d.expected/standard.sources  |  11 +
>  tests/sources.list.d/multiline.sources        |   9 +
>  tests/sources.list.d/options_comment.list     |   2 +
>  tests/sources.list.d/pbs-enterprise.list      |   1 +
>  tests/sources.list.d/pve.list                 |  10 +
>  tests/sources.list.d/standard.list            |   6 +
>  tests/sources.list.d/standard.sources         |  10 +
>  24 files changed, 1166 insertions(+)
>  create mode 100644 .cargo/config
>  create mode 100644 .gitignore
>  create mode 100644 Cargo.toml
>  create mode 100644 rustfmt.toml
>  create mode 100644 src/lib.rs
>  create mode 100644 src/repositories/check.rs
>  create mode 100644 src/repositories/list_parser.rs
>  create mode 100644 src/repositories/mod.rs
>  create mode 100644 src/repositories/sources_parser.rs
>  create mode 100644 src/repositories/writer.rs
>  create mode 100644 src/types.rs
>  create mode 100644 tests/repositories.rs
>  create mode 100644 tests/sources.list.d.expected/multiline.sources
>  create mode 100644 tests/sources.list.d.expected/options_comment.list
>  create mode 100644 tests/sources.list.d.expected/pbs-enterprise.list
>  create mode 100644 tests/sources.list.d.expected/pve.list
>  create mode 100644 tests/sources.list.d.expected/standard.list
>  create mode 100644 tests/sources.list.d.expected/standard.sources
>  create mode 100644 tests/sources.list.d/multiline.sources
>  create mode 100644 tests/sources.list.d/options_comment.list
>  create mode 100644 tests/sources.list.d/pbs-enterprise.list
>  create mode 100644 tests/sources.list.d/pve.list
>  create mode 100644 tests/sources.list.d/standard.list
>  create mode 100644 tests/sources.list.d/standard.sources
> 
> diff --git a/.cargo/config b/.cargo/config
> new file mode 100644
> index 0000000..3b5b6e4
> --- /dev/null
> +++ b/.cargo/config
> @@ -0,0 +1,5 @@
> +[source]
> +[source.debian-packages]
> +directory = "/usr/share/cargo/registry"
> +[source.crates-io]
> +replace-with = "debian-packages"
> diff --git a/.gitignore b/.gitignore
> new file mode 100644
> index 0000000..0d00c41
> --- /dev/null
> +++ b/.gitignore
> @@ -0,0 +1,3 @@
> +Cargo.lock
> +target/
> +tests/sources.list.d.actual
> diff --git a/Cargo.toml b/Cargo.toml
> new file mode 100644
> index 0000000..a0ecc26
> --- /dev/null
> +++ b/Cargo.toml
> @@ -0,0 +1,22 @@
> +[package]
> +name = "proxmox-apt"
> +version = "0.1.0"
> +authors = [
> +    "Fabian Ebner <f.ebner@proxmox.com>",
> +    "Proxmox Support Team <support@proxmox.com>",
> +]
> +edition = "2018"
> +license = "AGPL-3"
> +description = "Proxmox library for APT"
> +homepage = "https://www.proxmox.com"
> +
> +exclude = [ "debian" ]
> +
> +[lib]
> +name = "proxmox_apt"
> +path = "src/lib.rs"
> +
> +[dependencies]
> +anyhow = "1.0"
> +proxmox = { version = "0.11.0", features = [ "api-macro" ] }
> +serde = { version = "1.0", features = ["derive"] }
> diff --git a/rustfmt.toml b/rustfmt.toml
> new file mode 100644
> index 0000000..32a9786
> --- /dev/null
> +++ b/rustfmt.toml
> @@ -0,0 +1 @@
> +edition = "2018"
> diff --git a/src/lib.rs b/src/lib.rs
> new file mode 100644
> index 0000000..b065c0f
> --- /dev/null
> +++ b/src/lib.rs
> @@ -0,0 +1,3 @@
> +pub mod types;
> +
> +pub mod repositories;
> diff --git a/src/repositories/check.rs b/src/repositories/check.rs
> new file mode 100644
> index 0000000..87fbbac
> --- /dev/null
> +++ b/src/repositories/check.rs
> @@ -0,0 +1,47 @@
> +use anyhow::{bail, Error};
> +
> +use crate::types::{APTRepository, APTRepositoryFileType};
> +
> +impl APTRepository {
> +    /// Makes sure that all basic properties of a repository are present and
> +    /// not obviously invalid.
> +    pub fn basic_check(&self) -> Result<(), Error> {
> +        if self.types.is_empty() {
> +            bail!("missing package type(s)");
> +        }
> +        if self.uris.is_empty() {
> +            bail!("missing URI(s)");
> +        }
> +        if self.suites.is_empty() {
> +            bail!("missing suite(s)");
> +        }
> +
> +        for uri in self.uris.iter() {
> +            if !uri.contains(':') || uri.len() < 3 {
> +                bail!("invalid URI: '{}'", uri);
> +            }
> +        }
> +
> +        for suite in self.suites.iter() {
> +            if !suite.ends_with('/') && self.components.is_empty() {
> +                bail!("missing component(s)");
> +            } else if suite.ends_with('/') && !self.components.is_empty() {
> +                bail!("absolute suite '{}' does not allow component(s)", suite);
> +            }
> +        }
> +
> +        if self.file_type == APTRepositoryFileType::List {
> +            if self.types.len() > 1 {
> +                bail!("more than one package type");
> +            }
> +            if self.uris.len() > 1 {
> +                bail!("more than one URI");
> +            }
> +            if self.suites.len() > 1 {
> +                bail!("more than one suite");
> +            }
> +        }
> +
> +        Ok(())
> +    }
> +}
> diff --git a/src/repositories/list_parser.rs b/src/repositories/list_parser.rs
> new file mode 100644
> index 0000000..b26fed2
> --- /dev/null
> +++ b/src/repositories/list_parser.rs
> @@ -0,0 +1,181 @@
> +use std::convert::TryInto;
> +use std::io::BufRead;
> +use std::iter::{Iterator, Peekable};
> +use std::str::SplitAsciiWhitespace;
> +
> +use anyhow::{bail, Error};
> +
> +use super::APTRepositoryParser;
> +use crate::types::{APTRepository, APTRepositoryFileType, APTRepositoryOption};
> +
> +pub struct APTListFileParser<R: BufRead> {
> +    path: String,
> +    input: R,
> +    line_nr: usize,
> +    comment: String,
> +}
> +
> +impl<R: BufRead> APTListFileParser<R> {
> +    pub fn new(path: String, reader: R) -> Self {
> +        Self {
> +            path,
> +            input: reader,
> +            line_nr: 0,
> +            comment: String::new(),
> +        }
> +    }
> +
> +    /// Helper to parse options from the existing token stream.
> +    /// Also returns `Ok(())` if there are no options.
> +    /// Errors when options are invalid or not closed by `']'`.
> +    fn parse_options(
> +        options: &mut Vec<APTRepositoryOption>,
> +        tokens: &mut Peekable<SplitAsciiWhitespace>,
> +    ) -> Result<(), Error> {
> +        let mut option = match tokens.peek() {
> +            Some(token) => {
> +                match token.strip_prefix('[') {
> +                    Some(option) => option,
> +                    None => return Ok(()), // doesn't look like options
> +                }
> +            }
> +            None => return Ok(()),
> +        };
> +
> +        tokens.next(); // avoid reading the beginning twice
> +
> +        let mut finished = false;
> +        loop {
> +            if let Some(stripped) = option.strip_suffix(']') {
> +                option = stripped;
> +                if option.is_empty() {
> +                    break;
> +                }
> +                finished = true; // but still need to handle the last one
> +            };
> +
> +            if let Some(mid) = option.find('=') {
> +                let (key, mut value_str) = option.split_at(mid);
> +                value_str = &value_str[1..];
> +
> +                if key.is_empty() {
> +                    bail!("option with value '{}' has no key", value_str);

Seems weird to be using `value_str` here right before checking whether
it is empty. Maybe use `("option has no key: '{}'", option)`?

> +                } else if value_str.is_empty() {

(And is there a reason you wrote this as an `else if` rather than its
own condition?)

> +                    bail!("option with key '{}' has no value", key);
> +                }
> +
> +                let values: Vec<String> = value_str
> +                    .split(',')
> +                    .map(|value| value.to_string())
> +                    .collect();
> +
> +                options.push(APTRepositoryOption {
> +                    key: key.to_string(),
> +                    values,
> +                });
> +            } else if !option.is_empty() {
> +                bail!("got invalid option - '{}'", option);
> +            }
> +
> +            if finished {
> +                break;
> +            }
> +
> +            option = match tokens.next() {
> +                Some(option) => option,
> +                None => bail!("options not closed by ']'"),
> +            }
> +        }
> +
> +        Ok(())
> +    }
> +
> +    /// Parse a repository or comment in one-line format.
> +    /// Commented out repositories are also detected and returned with the
> +    /// `enabled` property set to `false`.
> +    /// If the line contains a repository, `self.comment` is added to the
> +    /// `comment` property.
> +    /// If the line contains a comment, it is added to `self.comment`.
> +    fn parse_one_line(&mut self, mut line: &str) -> Result<Option<APTRepository>, Error> {
> +        line = line.trim_matches(|c| char::is_ascii_whitespace(&c));
> +
> +        // check for commented out repository first
> +        if let Some(commented_out) = line.strip_prefix('#') {
> +            if let Ok(Some(mut repo)) = self.parse_one_line(commented_out) {
> +                repo.set_enabled(false);
> +                return Ok(Some(repo));
> +            }
> +        }
> +
> +        let mut repo =
> +            APTRepository::new(self.path.clone(), self.line_nr, APTRepositoryFileType::List);
> +
> +        // now handle "real" comment
> +        if let Some(comment_start) = line.find('#') {
> +            let (line_start, comment) = line.split_at(comment_start);
> +            self.comment = format!("{}{}\n", self.comment, &comment[1..]);
> +            line = line_start;
> +        }
> +
> +        let mut tokens = line.split_ascii_whitespace().peekable();
> +
> +        match tokens.next() {
> +            Some(package_type) => {
> +                repo.types.push(package_type.try_into()?);
> +            }
> +            None => return Ok(None), // empty line
> +        }
> +
> +        Self::parse_options(&mut repo.options, &mut tokens)?;
> +
> +        match tokens.next() {
> +            Some(uri) => repo.uris.push(uri.to_string()),
> +            None => bail!("got no URI"),
> +        };
> +
> +        match tokens.next() {
> +            Some(suite) => repo.suites.push(suite.to_string()),
> +            None => bail!("got no suite"),
> +        };
> +
> +        for component in tokens {
> +            repo.components.push(component.to_string());
> +        }

Nit: not sure if this matters but for "required" next iterator items
like this IMO the surrounding 'match' makes a bit of noise, and the
final loop is basically an `.extend()`. Luckily *all* of these want to
convert the token to a String, so *personally* I would prefer this:

        // the rest of the line is just '<uri> <suite> [<components>...]'
        let mut tokens = tokens.map(str::to_string);
        repo.uris
            .push(tokens.next().ok_or_else(|| format_err!("missing URI"))?;
        repo.suites
            .push(tokens.next().ok_or_else(|| format_err!("missing suite"))?;
        repo.components.extend(tokens);

(untested (only compile-tested))

> +
> +        repo.comment = self.comment.clone();

String implements Default, so you could use

    repo.comment = std::mem::take(&mut self.comment);

instead, since when you return `Some` here you `.clear()` the comment
anyway.

> +
> +        Ok(Some(repo))
> +    }
> +}
> +
> +impl<R: BufRead> APTRepositoryParser for APTListFileParser<R> {
> +    fn parse_repositories(&mut self) -> Result<Vec<APTRepository>, Error> {
> +        let mut repositories = Vec::<APTRepository>::new();
> +        let mut line = String::new();
> +
> +        loop {
> +            self.line_nr += 1;
> +            line.clear();
> +
> +            match self.input.read_line(&mut line) {
> +                Err(err) => bail!("input error for '{}' - {}", self.path, err),
> +                Ok(0) => break,
> +                Ok(_) => match self.parse_one_line(&line) {
> +                    Ok(Some(repo)) => {
> +                        repositories.push(repo);
> +                        self.comment.clear();
> +                    }
> +                    Ok(None) => continue,
> +                    Err(err) => bail!(
> +                        "malformed entry in '{}' line {} - {}",
> +                        self.path,
> +                        self.line_nr,
> +                        err,
> +                    ),
> +                },
> +            }
> +        }
> +
> +        Ok(repositories)
> +    }
> +}
> diff --git a/src/repositories/mod.rs b/src/repositories/mod.rs
> new file mode 100644
> index 0000000..123222a
> --- /dev/null
> +++ b/src/repositories/mod.rs
> @@ -0,0 +1,246 @@
> +use std::collections::BTreeMap;
> +use std::convert::TryFrom;
> +use std::ffi::OsString;
> +use std::path::{Path, PathBuf};
> +
> +use anyhow::{bail, format_err, Error};
> +
> +use crate::types::{APTRepository, APTRepositoryFileType, APTRepositoryOption};
> +
> +mod list_parser;
> +use list_parser::APTListFileParser;
> +
> +mod sources_parser;
> +use sources_parser::APTSourcesFileParser;
> +
> +mod check;
> +mod writer;
> +
> +const APT_SOURCES_LIST_FILENAME: &str = "/etc/apt/sources.list";
> +const APT_SOURCES_LIST_DIRECTORY: &str = "/etc/apt/sources.list.d/";
> +
> +impl APTRepository {
> +    /// Crates an empty repository with infomration that is known before parsing.
> +    fn new(path: String, number: usize, file_type: APTRepositoryFileType) -> Self {
> +        Self {
> +            types: vec![],
> +            uris: vec![],
> +            suites: vec![],
> +            components: vec![],
> +            options: vec![],
> +            comment: String::new(),
> +            path,
> +            number,
> +            file_type,
> +            enabled: true,
> +        }
> +    }
> +
> +    /// Changes the `enabled` flag and makes sure the `Enabled` option for
> +    /// `APTRepositoryPackageType::Sources` repositories is updated too.
> +    fn set_enabled(&mut self, enabled: bool) {
> +        self.enabled = enabled;
> +
> +        if self.file_type == APTRepositoryFileType::Sources {
> +            let enabled_string = match enabled {
> +                true => "true".to_string(),
> +                false => "false".to_string(),
> +            };
> +            for option in self.options.iter_mut() {
> +                if option.key == "Enabled" {
> +                    option.values = vec![enabled_string];
> +                    return;
> +                }
> +            }
> +            self.options.push(APTRepositoryOption {
> +                key: "Enabled".to_string(),
> +                values: vec![enabled_string],
> +            });
> +        }
> +    }
> +}
> +
> +trait APTRepositoryParser {
> +    /// Parse all repositories including the disabled ones.
> +    fn parse_repositories(&mut self) -> Result<Vec<APTRepository>, Error>;
> +}
> +
> +/// Helper to decide whether a file name is considered valid by APT and to
> +/// extract its file type and the path as a string.
> +/// Hidden files yield `Ok(None)`, while invalid file names yield an error.
> +fn check_filename<P: AsRef<Path>>(
> +    path: P,
> +) -> Result<Option<(APTRepositoryFileType, String)>, OsString> {
> +    let path: PathBuf = path.as_ref().to_path_buf();
> +    let path_string = path.clone().into_os_string().into_string()?;
> +
> +    let file_name = match path.file_name() {
> +        Some(file_name) => file_name.to_os_string().into_string()?,
> +        None => return Err(OsString::from(path_string)),
> +    };
> +
> +    // APT silently ignores hidden files
> +    if file_name.starts_with('.') {
> +        return Ok(None);
> +    }
> +
> +    let extension = match path.extension() {
> +        Some(extension) => extension.to_os_string().into_string()?,
> +        None => return Err(OsString::from(path_string)),
> +    };
> +
> +    let file_type = match APTRepositoryFileType::try_from(&extension[..]) {
> +        Ok(file_type) => file_type,
> +        _ => return Err(OsString::from(path_string)),
> +    };
> +
> +    // APT ignores such files but issues a warning
> +    if !file_name
> +        .chars()
> +        .all(|x| x.is_ascii_alphanumeric() || x == '_' || x == '-' || x == '.')
> +    {
> +        return Err(OsString::from(path_string));
> +    }
> +
> +    Ok(Some((file_type, path_string)))
> +}
> +
> +/// Returns all APT repositories configured in the specified files including
> +/// disabled ones.
> +/// Warns about invalid file names and some format violations, while other
> +/// format violations result in an error.
> +pub fn repositories_from_files<P: AsRef<Path>>(paths: &[P]) -> Result<Vec<APTRepository>, Error> {
> +    let mut repos = Vec::<APTRepository>::new();
> +
> +    for path in paths.iter() {

Nit: no need for the .iter() here (and a few other places)

> +        let path = path.as_ref();

Nit: given this is a generic but wants to work with &Path, I'd love for
the loop's body here to be its own function to monomorphize the bigger
parts of the code.

> +
> +        if !path.is_file() {
> +            eprintln!("Ignoring {:?} - not a file", path);
> +            continue;
> +        }
> +
> +        let file_type;
> +        let path_string;
> +
> +        match check_filename(path) {
> +            Ok(Some(res)) => {
> +                file_type = res.0;
> +                path_string = res.1;
> +            }
> +            Ok(None) => continue,
> +            Err(path) => {
> +                eprintln!("Ignoring {:?} - invalid file name", path);
> +                continue;
> +            }
> +        }
> +
> +        let contents = std::fs::read(path)
> +            .map_err(|err| format_err!("unable to read {:?} - {}", path, err))?;
> +
> +        let mut parser: Box<dyn APTRepositoryParser> = match file_type {
> +            APTRepositoryFileType::List => {
> +                Box::new(APTListFileParser::new(path_string, &contents[..]))
> +            }
> +            APTRepositoryFileType::Sources => {
> +                Box::new(APTSourcesFileParser::new(path_string, &contents[..]))
> +            }
> +        };
> +
> +        repos.append(&mut parser.parse_repositories()?);

Given that `parse_repositories()` comes from a private trait, I think it
would make sense to let it take an `&mut Vec<>` and pass `repos` here
rather than creating a new `Vec` and using `append`.

> +    }
> +
> +    for repo in repos.iter() {
> +        repo.basic_check().map_err(|err| {
> +            format_err!("check for {}:{} failed - {}", repo.path, repo.number, err)
> +        })?;
> +    }
> +
> +    Ok(repos)
> +}
> +
> +/// Returns all APT repositories configured in `/etc/apt/sources.list` and
> +/// in `/etc/apt/sources.list.d` including disabled repositories.
> +/// Warns about invalid file names and some format violations, while other
> +/// format violations result in an error.
> +pub fn repositories() -> Result<Vec<APTRepository>, Error> {
> +    let mut paths = Vec::<PathBuf>::new();
> +
> +    let sources_list_path = PathBuf::from(APT_SOURCES_LIST_FILENAME);
> +    if sources_list_path.is_file() {
> +        paths.push(sources_list_path)
> +    };
> +
> +    let sources_list_d_path = PathBuf::from(APT_SOURCES_LIST_DIRECTORY);
> +    if sources_list_d_path.is_dir() {
> +        for entry in std::fs::read_dir(sources_list_d_path)? {
> +            paths.push(entry?.path());
> +        }
> +    }
> +
> +    let repos = repositories_from_files(&paths)?;
> +
> +    Ok(repos)
> +}
> +
> +/// Write the repositories to the respective files specified by their
> +/// `path` property and in the order determined by their `number` property.
> +/// Does a `check::basic_check(repository)` for each repository first.
> +pub fn write_repositories<A: AsRef<APTRepository>>(repos: &[A]) -> Result<(), Error> {
> +    let mut repos: Vec<&APTRepository> = repos.iter().map(|repo| repo.as_ref()).collect();

Nit: the rest could be its own non-generic function for
monomorphization. Fewer instances of the exact same code ;-)

> +
> +    // check before writing
> +    for repo in repos.iter() {
> +        repo.basic_check().map_err(|err| {
> +            format_err!("check for {}:{} failed - {}", repo.path, repo.number, err)
> +        })?;
> +    }
> +
> +    repos.sort_by(|a, b| {
> +        if a.path == b.path {

Rather than doing `==` plus `.cmp` I think it may be nicer to write this
as a `match` on `a.path.cmp(&b.path)` and with
`Ordering::Equal => a.number.cmp(&b.number)`...

> +            a.number.cmp(&b.number)
> +        } else {
> +            a.path.cmp(&b.path)
> +        }
> +    });
> +
> +    let mut files = BTreeMap::<String, Vec<u8>>::new();
> +
> +    for repo in repos.iter() {
> +        let raw = match files.get_mut(&repo.path) {
> +            Some(raw) => raw,
> +            None => {
> +                files.insert(repo.path.clone(), vec![]);
> +                files.get_mut(&repo.path).unwrap()
> +            }
> +        };

^ This makes me cringe but I just have to accept it... :-(
That is until they fix the `Entry` API to work with borrowed keys >.>
(This REALLY should just be writable as
`files.entry(&repo.path).or_default()`, but for now this would require
cloning the path -_-)

> +
> +        repo.write(&mut *raw)
> +            .map_err(|err| format_err!("writing {}:{} failed - {}", repo.path, repo.number, err))?;
> +    }
> +
> +    for (path, content) in files.iter() {
> +        let path = PathBuf::from(path);
> +        let dir = path.parent().unwrap();
> +
> +        std::fs::create_dir_all(dir)
> +            .map_err(|err| format_err!("unable to create dir {:?} - {}", dir, err))?;
> +
> +        let pid = std::process::id();
> +        let mut tmp_path = path.clone();
> +        tmp_path.set_extension("tmp");
> +        tmp_path.set_extension(format!("{}", pid));
> +
> +        if let Err(err) = std::fs::write(&tmp_path, &content) {
> +            let _ = std::fs::remove_file(&tmp_path);
> +            bail!("write failed: {}", err);
> +        }
> +
> +        if let Err(err) = std::fs::rename(&tmp_path, &path) {
> +            let _ = std::fs::remove_file(&tmp_path);
> +            bail!("rename failed for {:?} - {}", path, err);
> +        }
> +    }
> +
> +    Ok(())
> +}
> diff --git a/src/repositories/sources_parser.rs b/src/repositories/sources_parser.rs
> new file mode 100644
> index 0000000..fdfe603
> --- /dev/null
> +++ b/src/repositories/sources_parser.rs
> @@ -0,0 +1,217 @@
> +use std::convert::TryInto;
> +use std::io::BufRead;
> +use std::iter::Iterator;
> +
> +use anyhow::{bail, Error};
> +
> +use crate::types::{
> +    APTRepository, APTRepositoryFileType, APTRepositoryOption, APTRepositoryPackageType,
> +};
> +
> +use super::APTRepositoryParser;
> +
> +pub struct APTSourcesFileParser<R: BufRead> {
> +    path: String,
> +    input: R,
> +    stanza_nr: usize,
> +    comment: String,
> +}
> +
> +/// See `man sources.list` and `man deb822` for the format specification.
> +impl<R: BufRead> APTSourcesFileParser<R> {
> +    pub fn new(path: String, reader: R) -> Self {
> +        Self {
> +            path,
> +            input: reader,
> +            stanza_nr: 1,
> +            comment: String::new(),
> +        }
> +    }
> +
> +    /// Based on APT's `StringToBool` in `strutl.cc`
> +    fn string_to_bool(string: &str, default: bool) -> bool {
> +        let string = string.trim_matches(|c| char::is_ascii_whitespace(&c));
> +        let string = string.to_lowercase();
> +
> +        match &string[..] {
> +            "1" | "yes" | "true" | "with" | "on" | "enable" => true,
> +            "0" | "no" | "false" | "without" | "off" | "disable" => false,
> +            _ => default,
> +        }
> +    }
> +
> +    /// Checks if `key` is valid according to deb822
> +    fn valid_key(key: &str) -> bool {
> +        if key.starts_with('-') {
> +            return false;
> +        };
> +        return key.chars().all(|c| matches!(c, '!'..='9' | ';'..='~'));
> +    }
> +
> +    /// Try parsing a repository in stanza format from `lines`.
> +    /// Returns `Ok(None)` when no stanza can be found.
> +    /// Comments are added to `self.comments`. If a stanza can be found,
> +    /// `self.comment` is added to the repository's `comment` property.
> +    /// Fully commented out stanzas are treated as comments.
> +    fn parse_stanza(&mut self, lines: &str) -> Result<Option<APTRepository>, Error> {
> +        let mut repo = APTRepository::new(
> +            self.path.clone(),
> +            self.stanza_nr,
> +            APTRepositoryFileType::Sources,
> +        );
> +
> +        // Values may be folded into multiple lines.
> +        // Those lines have to start with a space or a tab.
> +        let lines = lines.replace("\n ", " ");
> +        let lines = lines.replace("\n\t", " ");
> +
> +        let mut got_something = false;
> +
> +        for line in lines.lines() {
> +            let line = line.trim_matches(|c| char::is_ascii_whitespace(&c));
> +            if line.is_empty() {
> +                continue;
> +            }
> +
> +            if let Some(commented_out) = line.strip_prefix('#') {
> +                self.comment = format!("{}{}\n", self.comment, commented_out);
> +                continue;
> +            }
> +
> +            if let Some(mid) = line.find(':') {
> +                let (key, value_str) = line.split_at(mid);
> +                let value_str = &value_str[1..];
> +                let key = key.trim_matches(|c| char::is_ascii_whitespace(&c));
> +
> +                if key.is_empty() {
> +                    bail!("option with value '{}' has no key", value_str);
> +                } else if value_str.is_empty() {
> +                    // ignored by APT
> +                    eprintln!("option with key '{}' has no value", key);
> +                    continue;
> +                }
> +
> +                if !Self::valid_key(key) {
> +                    // ignored by APT
> +                    eprintln!("option with invalid key '{}'", key);
> +                    continue;
> +                }
> +
> +                let values: Vec<String> = value_str
> +                    .split_ascii_whitespace()
> +                    .map(|value| value.to_string())
> +                    .collect();
> +
> +                match key {
> +                    "Types" => {
> +                        let mut types = Vec::<APTRepositoryPackageType>::new();
> +                        for package_type in values {
> +                            types.push((&package_type[..]).try_into()?);
> +                        }
> +                        if !repo.types.is_empty() {

Would it make sense to check this first, like the other cases, or do you
explicitly want the errors from `try_into()` to take precedence?

> +                            eprintln!("key 'Types' was defined twice");
> +                        }
> +                        repo.types = types;
> +                    }
> +                    "URIs" => {
> +                        if !repo.uris.is_empty() {
> +                            eprintln!("key 'URIs' was defined twice");
> +                        }
> +                        repo.uris = values;
> +                    }
> +                    "Suites" => {
> +                        if !repo.suites.is_empty() {
> +                            eprintln!("key 'Suites' was defined twice");
> +                        }
> +                        repo.suites = values;
> +                    }
> +                    "Components" => {
> +                        if !repo.components.is_empty() {
> +                            eprintln!("key 'Components' was defined twice");
> +                        }
> +                        repo.components = values;
> +                    }
> +                    "Enabled" => {
> +                        repo.set_enabled(Self::string_to_bool(value_str, true));
> +                    }
> +                    _ => repo.options.push(APTRepositoryOption {
> +                        key: key.to_string(),
> +                        values,
> +                    }),
> +                }
> +            } else {
> +                bail!("got invalid line - '{:?}'", line);
> +            }
> +
> +            got_something = true;
> +        }
> +
> +        if !got_something {
> +            return Ok(None);
> +        }
> +
> +        repo.comment = self.comment.clone();

^ same as earlier (std::mem::take())

> +
> +        Ok(Some(repo))
> +    }
> +
> +    /// Helper function for `parse_repositories`
> +    fn try_parse_stanza(
> +        &mut self,
> +        lines: &str,
> +        repos: &mut Vec<APTRepository>,
> +    ) -> Result<(), Error> {
> +        match self.parse_stanza(&lines[..]) {
> +            Ok(Some(repo)) => {
> +                repos.push(repo);
> +                self.comment.clear();
> +                self.stanza_nr += 1;
> +            }
> +            Ok(None) => (),
> +            Err(err) => {
> +                bail!(
> +                    "malformed entry in '{}' stanza {} - {}",
> +                    self.path,
> +                    self.stanza_nr,
> +                    err,
> +                );
> +            }
> +        }
> +
> +        Ok(())
> +    }
> +}
> +
> +impl<R: BufRead> APTRepositoryParser for APTSourcesFileParser<R> {
> +    fn parse_repositories(&mut self) -> Result<Vec<APTRepository>, Error> {
> +        let mut repos = Vec::<APTRepository>::new();
> +        let mut lines = String::new();
> +        let mut offset: usize = 0;
> +
> +        loop {

Can we just

    let offset = lines.len();

here or am I missing something? (Maybe call it "previous_len" though?)

> +            match self.input.read_line(&mut lines) {
> +                Err(err) => bail!("input error for '{}' - {}", self.path, err),
> +                Ok(0) => {
> +                    self.try_parse_stanza(&lines[..], &mut repos)?;
> +                    break;
> +                }
> +                Ok(bytes_read) => {
> +                    if (&lines[offset..])
> +                        .trim_matches(|c| char::is_ascii_whitespace(&c))
> +                        .is_empty()
> +                    {
> +                        // detected end of stanza
> +
> +                        self.try_parse_stanza(&lines[..], &mut repos)?;
> +                        lines.clear();
> +                        offset = 0;
> +                    } else {
> +                        offset += bytes_read;
> +                    }
> +                }
> +            }
> +        }
> +
> +        Ok(repos)
> +    }
> +}
> diff --git a/src/repositories/writer.rs b/src/repositories/writer.rs
> new file mode 100644
> index 0000000..6feb1c7
> --- /dev/null
> +++ b/src/repositories/writer.rs
> @@ -0,0 +1,89 @@
> +use std::io::Write;
> +
> +use anyhow::{bail, Error};
> +
> +use crate::types::{APTRepository, APTRepositoryFileType};
> +
> +impl APTRepository {
> +    /// Writes a repository in the corresponding format followed by a blank.
> +    /// Expects that `basic_check()` for the repository was successful.
> +    pub fn write(&self, w: &mut dyn Write) -> Result<(), Error> {
> +        match self.file_type {
> +            APTRepositoryFileType::List => write_one_line(self, w),
> +            APTRepositoryFileType::Sources => write_stanza(self, w),
> +        }
> +    }
> +}
> +
> +/// Writes a repository in one-line format followed by a blank line.
> +/// Expects that `repo.file_type == APTRepositoryFileType::List`.
> +/// Expects that `basic_check()` for the repository was successful.
> +fn write_one_line(repo: &APTRepository, w: &mut dyn Write) -> Result<(), Error> {
> +    if repo.file_type != APTRepositoryFileType::List {
> +        bail!("not a .list repository");
> +    }
> +
> +    if !repo.comment.is_empty() {
> +        for line in repo.comment.lines() {
> +            writeln!(w, "#{}", line)?;
> +        }
> +    }
> +
> +    if !repo.enabled {
> +        write!(w, "# ")?;
> +    }
> +
> +    write!(w, "{} ", String::from(repo.types[0]))?;
> +
> +    if !repo.options.is_empty() {
> +        let option_string = repo
> +            .options
> +            .iter()
> +            .map(|option| format!("{}={}", option.key, option.values.join(",")))
> +            .collect::<Vec<String>>()
> +            .join(" ");

^ I'd prefer a `fold()` instead of `.collect().join()`, seems like
useless extra allocation with the Vec ;-)
In fact, the `fold` could probably be where the `.map()` is already?
Alternatively, you could just write!() directly to `w`?

> +
> +        write!(w, "[ {} ] ", option_string)?;
> +    };
> +
> +    write!(w, "{} ", repo.uris[0])?;
> +    write!(w, "{} ", repo.suites[0])?;
> +    writeln!(w, "{}", repo.components.join(" "))?;
> +
> +    writeln!(w)?;
> +
> +    Ok(())
> +}
> +
> +/// Writes a single stanza followed by a blank line.
> +/// Expects that `repo.file_type == APTRepositoryFileType::Sources`.
> +fn write_stanza(repo: &APTRepository, w: &mut dyn Write) -> Result<(), Error> {
> +    if repo.file_type != APTRepositoryFileType::Sources {
> +        bail!("not a .sources repository");
> +    }
> +
> +    if !repo.comment.is_empty() {
> +        for line in repo.comment.lines() {
> +            writeln!(w, "#{}", line)?;
> +        }
> +    }
> +
> +    let type_strings = repo
> +        .types
> +        .iter()
> +        .map(|package_type| String::from(*package_type))
> +        .collect::<Vec<String>>();

^ Similar case, `fold()` while including the `join` you do in the next
line for this one.

> +
> +    writeln!(w, "Types: {}", type_strings.join(" "))?;
> +    writeln!(w, "URIs: {}", repo.uris.join(" "))?;
> +    writeln!(w, "Suites: {}", repo.suites.join(" "))?;
> +    writeln!(w, "Components: {}", repo.components.join(" "))?;
> +
> +    for option in repo.options.iter() {
> +        writeln!(w, "{}: {}", option.key, option.values.join(" "))?;
> +    }
> +
> +    writeln!(w)?;
> +
> +    Ok(())
> +}
> diff --git a/src/types.rs b/src/types.rs
> new file mode 100644
> index 0000000..7a95daf
> --- /dev/null
> +++ b/src/types.rs
> @@ -0,0 +1,197 @@
> +use std::convert::{From, TryFrom};
> +
> +use anyhow::{bail, Error};
> +use serde::{Deserialize, Serialize};
> +
> +use proxmox::api::api;
> +
> +#[api()]

Nit: you can skip the `()` here.

> +#[derive(Debug, Copy, Clone, Serialize, Deserialize, PartialEq)]
> +#[serde(rename_all = "lowercase")]
> +pub enum APTRepositoryFileType {
> +    /// One-line-style format
> +    List,
> +    /// DEB822-style format
> +    Sources,
> +}
> +
> +impl TryFrom<&str> for APTRepositoryFileType {
> +    type Error = Error;
> +
> +    fn try_from(string: &str) -> Result<Self, Error> {
> +        match &string[..] {
> +            "list" => Ok(APTRepositoryFileType::List),
> +            "sources" => Ok(APTRepositoryFileType::Sources),
> +            _ => bail!("invalid file type '{}'", string),
> +        }
> +    }
> +}
> +
> +impl From<APTRepositoryFileType> for String {

Please implement `std::fmt::Display` for APTRepositoryFileType instead.

> +    fn from(file_type: APTRepositoryFileType) -> String {
> +        match file_type {
> +            APTRepositoryFileType::List => "list".to_string(),
> +            APTRepositoryFileType::Sources => "sources".to_string(),
> +        }
> +    }
> +}
> +
> +#[api()]
> +#[derive(Debug, Copy, Clone, Serialize, Deserialize, PartialEq)]
> +#[serde(rename_all = "kebab-case")]
> +pub enum APTRepositoryPackageType {
> +    /// Debian package
> +    Deb,
> +    /// Debian source package
> +    DebSrc,
> +}
> +
> +impl TryFrom<&str> for APTRepositoryPackageType {
> +    type Error = Error;
> +
> +    fn try_from(string: &str) -> Result<Self, Error> {
> +        match &string[..] {
> +            "deb" => Ok(APTRepositoryPackageType::Deb),
> +            "deb-src" => Ok(APTRepositoryPackageType::DebSrc),
> +            _ => bail!("invalid file type '{}'", string),
> +        }
> +    }
> +}
> +
> +impl From<APTRepositoryPackageType> for String {

Please implement `std::fmt::Display` for APTRepositoryPackageType
instead.

> +    fn from(package_type: APTRepositoryPackageType) -> String {
> +        match package_type {
> +            APTRepositoryPackageType::Deb => "deb".to_string(),
> +            APTRepositoryPackageType::DebSrc => "deb-src".to_string(),
> +        }
> +    }
> +}
> +
> +#[api(
> +    properties: {
> +        Key: {
> +            description: "Option key.",
> +            type: String,

Nit: simple standard types can be left out. Less typing ;-)

> +        },
> +        Values: {
> +            description: "Option values.",
> +            type: Array,
> +            items: {
> +                description: "Value.",
> +                type: String,
> +            },
> +        },
> +    },
> +)]
> +#[derive(Debug, Clone, Serialize, Deserialize)]
> +#[serde(rename_all = "PascalCase")] // for consistency
> +/// Additional options for an APT repository.
> +/// Used for both single- and mutli-value options.
> +pub struct APTRepositoryOption {
> +    /// Option key.
> +    pub key: String,
> +    /// Option value(s).
> +    pub values: Vec<String>,
> +}
> +
> +#[api(
> +    properties: {
> +        Types: {
> +            description: "List of package types.",
> +            type: Array,
> +            items: {
> +                type: APTRepositoryPackageType,
> +            },
> +        },
> +        URIs: {
> +            description: "List of repository URIs.",
> +            type: Array,
> +            items: {
> +                description: "Repository URI.",
> +                type: String,
> +            },
> +        },
> +        Suites: {
> +            description: "List of distributions.",
> +            type: Array,
> +            items: {
> +                description: "Package distribution.",
> +                type: String,
> +            },
> +        },
> +        Components: {
> +            description: "List of repository components.",
> +            type: Array,
> +            items: {
> +                description: "Repository component.",
> +                type: String,
> +            },
> +        },
> +        Options: {
> +            type: Array,
> +            optional: true,
> +            items: {
> +                type: APTRepositoryOption,
> +            },
> +        },
> +        Comment: {
> +            description: "Associated comment.",
> +            type: String,
> +            optional: true,
> +        },
> +        Path: {
> +            description: "Path to the defining file.",
> +            type: String,
> +        },
> +        Number: {
> +            description: "Line or stanza number.",
> +            type: Integer,
> +        },
> +        FileType: {
> +            type: APTRepositoryFileType,
> +        },
> +        Enabled: {
> +            description: "Whether the repository is enabled or not.",
> +            type: Boolean,
> +        },
> +    },
> +)]
> +#[derive(Debug, Clone, Serialize, Deserialize)]
> +#[serde(rename_all = "PascalCase")]
> +/// Describes an APT repository.
> +pub struct APTRepository {

Nit: With all those attributes I find it easier to read with newlines
between the fields...

> +    /// List of package types.
> +    #[serde(skip_serializing_if = "Vec::is_empty")]
> +    pub types: Vec<APTRepositoryPackageType>,
> +    /// List of repository URIs.
> +    #[serde(skip_serializing_if = "Vec::is_empty")]
> +    #[serde(rename = "URIs")]
> +    pub uris: Vec<String>,
> +    /// List of package distributions.
> +    #[serde(skip_serializing_if = "Vec::is_empty")]
> +    pub suites: Vec<String>,
> +    /// List of repository components.
> +    #[serde(skip_serializing_if = "Vec::is_empty")]
> +    pub components: Vec<String>,
> +    /// Additional options.
> +    #[serde(skip_serializing_if = "Vec::is_empty")]
> +    pub options: Vec<APTRepositoryOption>,
> +    /// Associated comment.
> +    #[serde(skip_serializing_if = "String::is_empty")]
> +    pub comment: String,
> +    /// Path to the defining file.
> +    #[serde(skip_serializing_if = "String::is_empty")]
> +    pub path: String,
> +    /// Line or stanza number.
> +    pub number: usize,
> +    /// Format of the defining file.
> +    pub file_type: APTRepositoryFileType,
> +    /// Whether the repository is enabled or not.
> +    pub enabled: bool,
> +}
> +
> +impl AsRef<APTRepository> for APTRepository {

This one deserves a comment as it is not clear why this is necessary.
Looks like an ergonomic decision since this crate itself doesn't really
*need* this (or rather, could be written without requiring this).

> +    fn as_ref(&self) -> &APTRepository {
> +        &self
> +    }
> +}
> diff --git a/tests/repositories.rs b/tests/repositories.rs
> new file mode 100644
> index 0000000..020e133
> --- /dev/null
> +++ b/tests/repositories.rs
> @@ -0,0 +1,73 @@
> +use std::collections::HashMap;
> +use std::path::PathBuf;
> +
> +use anyhow::{bail, format_err, Error};
> +
> +use proxmox_apt::repositories::{repositories_from_files, write_repositories};
> +use proxmox_apt::types::APTRepository;
> +
> +#[test]
> +fn test_parse_write() -> Result<(), Error> {
> +    let test_dir = std::env::current_dir()?.join("tests");
> +    let read_dir = test_dir.join("sources.list.d");
> +    let write_dir = test_dir.join("sources.list.d.actual");
> +    let expected_dir = test_dir.join("sources.list.d.expected");
> +
> +    let mut paths = Vec::<PathBuf>::new();
> +    for entry in std::fs::read_dir(read_dir)? {
> +        paths.push(entry?.path());
> +    }
> +
> +    let repos = repositories_from_files(&paths)?;
> +
> +    // used to mess up the order from parsing and to check that each repo has a
> +    // unique path:number
> +    let mut repo_hash = HashMap::<String, APTRepository>::new();
> +
> +    for mut repo in repos {
> +        let path = PathBuf::from(repo.path);
> +        let new_path = write_dir.join(path.file_name().unwrap());
> +
> +        repo.path = new_path.into_os_string().into_string().unwrap();
> +
> +        let key = format!("{}:{}", repo.path, repo.number);
> +
> +        if repo_hash.insert(key.clone(), repo).is_some() {
> +            bail!("key '{}' is not unique!", key);
> +        }
> +    }
> +
> +    if write_dir.is_dir() {
> +        std::fs::remove_dir_all(&write_dir)
> +            .map_err(|err| format_err!("unable to remove dir {:?} - {}", write_dir, err))?;
> +    }
> +
> +    std::fs::create_dir_all(&write_dir)
> +        .map_err(|err| format_err!("unable to create dir {:?} - {}", write_dir, err))?;
> +
> +    let repos_vec: Vec<&APTRepository> = repo_hash.values().collect();
> +    write_repositories(&repos_vec)?;
> +
> +    let mut expected_count = 0;
> +
> +    for entry in std::fs::read_dir(expected_dir)? {
> +        expected_count += 1;
> +
> +        let expected_path = entry?.path();
> +        let actual_path = write_dir.join(expected_path.file_name().unwrap());
> +
> +        let expected_contents = std::fs::read(&expected_path)
> +            .map_err(|err| format_err!("unable to read {:?} - {}", expected_path, err))?;
> +
> +        let actual_contents = std::fs::read(&actual_path)
> +            .map_err(|err| format_err!("unable to read {:?} - {}", actual_path, err))?;
> +
> +        assert_eq!(expected_contents, actual_contents);
> +    }
> +
> +    let actual_count = std::fs::read_dir(write_dir)?.count();
> +
> +    assert_eq!(expected_count, actual_count);
> +
> +    Ok(())
> +}
> diff --git a/tests/sources.list.d.expected/multiline.sources b/tests/sources.list.d.expected/multiline.sources
> new file mode 100644
> index 0000000..91f53c2
> --- /dev/null
> +++ b/tests/sources.list.d.expected/multiline.sources
> @@ -0,0 +1,8 @@
> +# comment in here
> +Types: deb deb-src
> +URIs: http://ftp.at.debian.org/debian
> +Suites: buster buster-updates
> +Components: main contrib
> +Languages: it de fr
> +Enabled: false
> +
> diff --git a/tests/sources.list.d.expected/options_comment.list b/tests/sources.list.d.expected/options_comment.list
> new file mode 100644
> index 0000000..f0952e4
> --- /dev/null
> +++ b/tests/sources.list.d.expected/options_comment.list
> @@ -0,0 +1,3 @@
> +# comment
> +deb [ lang=it,de arch=amd64 ] http://ftp.at.debian.org/debian buster main contrib
> +
> diff --git a/tests/sources.list.d.expected/pbs-enterprise.list b/tests/sources.list.d.expected/pbs-enterprise.list
> new file mode 100644
> index 0000000..acb2990
> --- /dev/null
> +++ b/tests/sources.list.d.expected/pbs-enterprise.list
> @@ -0,0 +1,2 @@
> +deb https://enterprise.proxmox.com/debian/pbs buster pbs-enterprise
> +
> diff --git a/tests/sources.list.d.expected/pve.list b/tests/sources.list.d.expected/pve.list
> new file mode 100644
> index 0000000..127a49a
> --- /dev/null
> +++ b/tests/sources.list.d.expected/pve.list
> @@ -0,0 +1,13 @@
> +deb http://ftp.debian.org/debian buster main contrib
> +
> +deb http://ftp.debian.org/debian buster-updates main contrib
> +
> +# PVE pve-no-subscription repository provided by proxmox.com,
> +# NOT recommended for production use
> +deb http://download.proxmox.com/debian/pve buster pve-no-subscription
> +
> +# deb https://enterprise.proxmox.com/debian/pve buster pve-enterprise
> +
> +# security updates
> +deb http://security.debian.org/debian-security buster/updates main contrib
> +
> diff --git a/tests/sources.list.d.expected/standard.list b/tests/sources.list.d.expected/standard.list
> new file mode 100644
> index 0000000..63c1b60
> --- /dev/null
> +++ b/tests/sources.list.d.expected/standard.list
> @@ -0,0 +1,7 @@
> +deb http://ftp.at.debian.org/debian buster main contrib
> +
> +deb http://ftp.at.debian.org/debian buster-updates main contrib
> +
> +# security updates
> +deb http://security.debian.org buster/updates main contrib
> +
> diff --git a/tests/sources.list.d.expected/standard.sources b/tests/sources.list.d.expected/standard.sources
> new file mode 100644
> index 0000000..56ce280
> --- /dev/null
> +++ b/tests/sources.list.d.expected/standard.sources
> @@ -0,0 +1,11 @@
> +Types: deb
> +URIs: http://ftp.at.debian.org/debian
> +Suites: buster buster-updates
> +Components: main contrib
> +
> +# security updates
> +Types: deb
> +URIs: http://security.debian.org
> +Suites: buster/updates
> +Components: main contrib
> +
> diff --git a/tests/sources.list.d/multiline.sources b/tests/sources.list.d/multiline.sources
> new file mode 100644
> index 0000000..c3a1ff0
> --- /dev/null
> +++ b/tests/sources.list.d/multiline.sources
> @@ -0,0 +1,9 @@
> +Types: deb deb-src
> +URIs: http://ftp.at.debian.org/debian
> +Suites: buster buster-updates
> +# comment in here
> +Components: main contrib
> +Languages: it
> + de
> +	fr
> +Enabled: off
> diff --git a/tests/sources.list.d/options_comment.list b/tests/sources.list.d/options_comment.list
> new file mode 100644
> index 0000000..e3f4112
> --- /dev/null
> +++ b/tests/sources.list.d/options_comment.list
> @@ -0,0 +1,2 @@
> +deb [ lang=it,de arch=amd64 ] http://ftp.at.debian.org/debian buster main contrib # comment
> +
> diff --git a/tests/sources.list.d/pbs-enterprise.list b/tests/sources.list.d/pbs-enterprise.list
> new file mode 100644
> index 0000000..5f8763c
> --- /dev/null
> +++ b/tests/sources.list.d/pbs-enterprise.list
> @@ -0,0 +1 @@
> +deb https://enterprise.proxmox.com/debian/pbs buster pbs-enterprise
> diff --git a/tests/sources.list.d/pve.list b/tests/sources.list.d/pve.list
> new file mode 100644
> index 0000000..6213f72
> --- /dev/null
> +++ b/tests/sources.list.d/pve.list
> @@ -0,0 +1,10 @@
> +deb http://ftp.debian.org/debian buster main contrib
> +deb http://ftp.debian.org/debian buster-updates main contrib
> +
> +# PVE pve-no-subscription repository provided by proxmox.com,
> +# NOT recommended for production use
> +deb http://download.proxmox.com/debian/pve buster pve-no-subscription
> +# deb https://enterprise.proxmox.com/debian/pve buster pve-enterprise
> +
> +# security updates
> +deb http://security.debian.org/debian-security buster/updates main contrib
> diff --git a/tests/sources.list.d/standard.list b/tests/sources.list.d/standard.list
> new file mode 100644
> index 0000000..26db887
> --- /dev/null
> +++ b/tests/sources.list.d/standard.list
> @@ -0,0 +1,6 @@
> +deb http://ftp.at.debian.org/debian buster main contrib
> +
> +deb http://ftp.at.debian.org/debian buster-updates main contrib
> +
> +# security updates
> +deb http://security.debian.org buster/updates main contrib
> diff --git a/tests/sources.list.d/standard.sources b/tests/sources.list.d/standard.sources
> new file mode 100644
> index 0000000..605202e
> --- /dev/null
> +++ b/tests/sources.list.d/standard.sources
> @@ -0,0 +1,10 @@
> +Types: deb
> +URIs: http://ftp.at.debian.org/debian
> +Suites: buster buster-updates
> +Components: main contrib
> +
> +# security updates
> +Types: deb
> +URIs: http://security.debian.org
> +Suites: buster/updates
> +Components: main contrib
> -- 
> 2.20.1
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
> 




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

* Re: [pbs-devel] [PATCH v2 proxmox-apt 04/10] add check_repositories function
  2021-02-26 15:09 ` [pbs-devel] [PATCH v2 proxmox-apt 04/10] add check_repositories function Fabian Ebner
@ 2021-03-10 15:17   ` Wolfgang Bumiller
  0 siblings, 0 replies; 13+ messages in thread
From: Wolfgang Bumiller @ 2021-03-10 15:17 UTC (permalink / raw)
  To: Fabian Ebner; +Cc: pbs-devel

On Fri, Feb 26, 2021 at 04:09:53PM +0100, Fabian Ebner wrote:
> which, for now, checks the suites.
> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
> 
> Changes from v1:
>     * split this and information about Proxmox repositories in two
>     * add tests
> 
>  src/repositories/check.rs                 | 78 ++++++++++++++++++++++-
>  src/repositories/mod.rs                   | 16 ++++-
>  src/types.rs                              | 30 +++++++++
>  tests/repositories.rs                     | 60 ++++++++++++++++-
>  tests/sources.list.d.expected/bad.sources | 20 ++++++
>  tests/sources.list.d/bad.sources          | 19 ++++++
>  6 files changed, 219 insertions(+), 4 deletions(-)
>  create mode 100644 tests/sources.list.d.expected/bad.sources
>  create mode 100644 tests/sources.list.d/bad.sources
> 
> diff --git a/src/repositories/check.rs b/src/repositories/check.rs
> index d0656cd..7726ff3 100644
> --- a/src/repositories/check.rs
> +++ b/src/repositories/check.rs
> @@ -1,6 +1,23 @@
>  use anyhow::{bail, Error};
>  
> -use crate::types::{APTRepository, APTRepositoryFileType};
> +use crate::types::{
> +    APTRepository, APTRepositoryFileType, APTRepositoryPackageType, APTRepositoryWarning,
> +};
> +
> +/// Checks if `suite` is some variant of `base_suite`, e.g. `buster-backports`
> +/// is a variant of `buster`.
> +#[allow(clippy::useless_format)]
> +fn suite_is_variant(suite: &str, base_suite: &str) -> bool {
> +    let variants = vec![
> +        format!("{}", base_suite),
> +        format!("{}-backports", base_suite),
> +        format!("{}-backports-sloppy", base_suite),
> +        format!("{}-updates", base_suite),
> +        format!("{}/updates", base_suite),
> +    ];
> +
> +    variants.iter().any(|variant| *variant == suite)

Shouldn't `.strip_prefix()` get this job done as well? ;-)

    matches!(
        suite.strip_prefix(base_suite),
        Some("")
            | Some("-backports")
            | Some("-backports-sloppy")
            | Some("-updates")
            | Some("/updates")
    )

> +}
>  
>  impl APTRepository {
>      /// Makes sure that all basic properties of a repository are present and




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

end of thread, other threads:[~2021-03-10 15:17 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-26 15:09 [pbs-devel] [PATCH/RFC-SERIES v2] APT repositories API/UI Fabian Ebner
2021-02-26 15:09 ` [pbs-devel] [PATCH v2 proxmox-apt 01/10] initial commit Fabian Ebner
2021-03-10 15:14   ` Wolfgang Bumiller
2021-02-26 15:09 ` [pbs-devel] [PATCH v2 proxmox-apt 02/10] add files for Debian packaging Fabian Ebner
2021-02-26 15:09 ` [pbs-devel] [PATCH v2 proxmox-apt 03/10] add functions to check for Proxmox repositories Fabian Ebner
2021-02-26 15:09 ` [pbs-devel] [PATCH v2 proxmox-apt 04/10] add check_repositories function Fabian Ebner
2021-03-10 15:17   ` Wolfgang Bumiller
2021-02-26 15:09 ` [pbs-devel] [PATCH v2 proxmox-backup 05/10] cargo: depend on new proxmox-apt crate Fabian Ebner
2021-02-26 15:09 ` [pbs-devel] [PATCH v2 proxmox-backup 06/10] api: apt: add repositories call Fabian Ebner
2021-02-26 15:09 ` [pbs-devel] [PATCH v2 widget-toolkit 07/10] add UI for APT repositories Fabian Ebner
2021-02-26 15:09 ` [pbs-devel] [PATCH v2 proxmox-backup 08/10] ui: add grid " Fabian Ebner
2021-02-26 15:09 ` [pbs-devel] [RFC v2 proxmox-backup 09/10] api: apt: add check_repositories_call Fabian Ebner
2021-02-26 15:09 ` [pbs-devel] [RFC v2 widget-toolkit 10/10] APT: show warnings for repositories Fabian Ebner

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