[pve-devel] [PATCH v3 proxmox-perl-rs] move apt repositories module to common

[Fabian Ebner <f.ebner@proxmox.com>]

while introducing a 'product' parameter to the relevant functions and
adding wrappers for backwards-compatibility.

Suggested-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Changes from v2:
    * Turn Rust side into wrappers rahter than adding Perl wrappers.
    * Avoid moving files between packages.

pve-manager (respectively pmg-api) depends and build-depends on
libproxmox-rs-perl and libpve-rs-perl (respectively libpmg-rs-perl).
Both are needed, because just upgrading libproxmox-rs-perl doesn't
make the new functionality available in the product-specific shared
lib.

 Makefile                       |   4 +-
 common/src/apt/mod.rs          |   1 +
 common/src/apt/repositories.rs | 164 +++++++++++++++++++++++++++++++++
 common/src/mod.rs              |   1 +
 pmg-rs/src/apt/repositories.rs | 142 ++--------------------------
 pve-rs/src/apt/repositories.rs | 142 ++--------------------------
 6 files changed, 183 insertions(+), 271 deletions(-)
 create mode 100644 common/src/apt/mod.rs
 create mode 100644 common/src/apt/repositories.rs

diff --git a/Makefile b/Makefile
index 6f9b597..c0212a8 100644
--- a/Makefile
+++ b/Makefile
@@ -41,7 +41,9 @@ pve pmg:
 gen:
 	$(call package_template,PMG,pmg_rs)
 	$(call package_template,PVE,pve_rs)
-	perl ./scripts/genpackage.pl Common Proxmox::RS::CalendarEvent
+	perl ./scripts/genpackage.pl Common \
+	  Proxmox::RS::APT::Repositories \
+	  Proxmox::RS::CalendarEvent
 	perl ./scripts/genpackage.pl PVE \
 	  PVE::RS::APT::Repositories \
 	  PVE::RS::OpenId \
diff --git a/common/src/apt/mod.rs b/common/src/apt/mod.rs
new file mode 100644
index 0000000..21b552a
--- /dev/null
+++ b/common/src/apt/mod.rs
@@ -0,0 +1 @@
+pub mod repositories;
diff --git a/common/src/apt/repositories.rs b/common/src/apt/repositories.rs
new file mode 100644
index 0000000..26a20e4
--- /dev/null
+++ b/common/src/apt/repositories.rs
@@ -0,0 +1,164 @@
+#[perlmod::package(name = "Proxmox::RS::APT::Repositories")]
+pub mod export {
+    use std::convert::TryInto;
+
+    use anyhow::{bail, Error};
+    use serde::{Deserialize, Serialize};
+
+    use proxmox_apt::repositories::{
+        APTRepositoryFile, APTRepositoryFileError, APTRepositoryHandle, APTRepositoryInfo,
+        APTStandardRepository,
+    };
+
+    #[derive(Deserialize, Serialize)]
+    #[serde(rename_all = "kebab-case")]
+    /// Result for the repositories() function
+    pub struct RepositoriesResult {
+        /// Successfully parsed files.
+        pub files: Vec<APTRepositoryFile>,
+
+        /// Errors for files that could not be parsed or read.
+        pub errors: Vec<APTRepositoryFileError>,
+
+        /// Common digest for successfully parsed files.
+        pub digest: String,
+
+        /// Additional information/warnings about repositories.
+        pub infos: Vec<APTRepositoryInfo>,
+
+        /// Standard repositories and their configuration status.
+        pub standard_repos: Vec<APTStandardRepository>,
+    }
+
+    #[derive(Deserialize, Serialize)]
+    #[serde(rename_all = "kebab-case")]
+    /// For changing an existing repository.
+    pub struct ChangeProperties {
+        /// Whether the repository should be enabled or not.
+        pub enabled: Option<bool>,
+    }
+
+    /// Get information about configured repositories and standard repositories for `product`.
+    #[export]
+    pub fn repositories(product: &str) -> Result<RepositoriesResult, Error> {
+        let (files, errors, digest) = proxmox_apt::repositories::repositories()?;
+        let digest = hex::encode(&digest);
+
+        let suite = proxmox_apt::repositories::get_current_release_codename()?;
+
+        let infos = proxmox_apt::repositories::check_repositories(&files, suite);
+        let standard_repos =
+            proxmox_apt::repositories::standard_repositories(&files, product, suite);
+
+        Ok(RepositoriesResult {
+            files,
+            errors,
+            digest,
+            infos,
+            standard_repos,
+        })
+    }
+
+    /// Add the repository identified by the `handle` and `product`.
+    /// If the repository is already configured, it will be set to enabled.
+    ///
+    /// The `digest` parameter asserts that the configuration has not been modified.
+    #[export]
+    pub fn add_repository(handle: &str, product: &str, digest: Option<&str>) -> Result<(), Error> {
+        let (mut files, errors, current_digest) = proxmox_apt::repositories::repositories()?;
+
+        let handle: APTRepositoryHandle = handle.try_into()?;
+        let suite = proxmox_apt::repositories::get_current_release_codename()?;
+
+        if let Some(digest) = digest {
+            let expected_digest = hex::decode(digest)?;
+            if expected_digest != current_digest {
+                bail!("detected modified configuration - file changed by other user? Try again.");
+            }
+        }
+
+        // check if it's already configured first
+        for file in files.iter_mut() {
+            for repo in file.repositories.iter_mut() {
+                if repo.is_referenced_repository(handle, product, &suite.to_string()) {
+                    if repo.enabled {
+                        return Ok(());
+                    }
+
+                    repo.set_enabled(true);
+                    file.write()?;
+
+                    return Ok(());
+                }
+            }
+        }
+
+        let (repo, path) =
+            proxmox_apt::repositories::get_standard_repository(handle, product, suite);
+
+        if let Some(error) = errors.iter().find(|error| error.path == path) {
+            bail!(
+                "unable to parse existing file {} - {}",
+                error.path,
+                error.error,
+            );
+        }
+
+        if let Some(file) = files.iter_mut().find(|file| file.path == path) {
+            file.repositories.push(repo);
+
+            file.write()?;
+        } else {
+            let mut file = match APTRepositoryFile::new(&path)? {
+                Some(file) => file,
+                None => bail!("invalid path - {}", path),
+            };
+
+            file.repositories.push(repo);
+
+            file.write()?;
+        }
+
+        Ok(())
+    }
+
+    /// Change the properties of the specified repository.
+    ///
+    /// The `digest` parameter asserts that the configuration has not been modified.
+    #[export]
+    pub fn change_repository(
+        path: &str,
+        index: usize,
+        options: ChangeProperties,
+        digest: Option<&str>,
+    ) -> Result<(), Error> {
+        let (mut files, errors, current_digest) = proxmox_apt::repositories::repositories()?;
+
+        if let Some(digest) = digest {
+            let expected_digest = hex::decode(digest)?;
+            if expected_digest != current_digest {
+                bail!("detected modified configuration - file changed by other user? Try again.");
+            }
+        }
+
+        if let Some(error) = errors.iter().find(|error| error.path == path) {
+            bail!("unable to parse file {} - {}", error.path, error.error);
+        }
+
+        if let Some(file) = files.iter_mut().find(|file| file.path == path) {
+            if let Some(repo) = file.repositories.get_mut(index) {
+                if let Some(enabled) = options.enabled {
+                    repo.set_enabled(enabled);
+                }
+
+                file.write()?;
+            } else {
+                bail!("invalid index - {}", index);
+            }
+        } else {
+            bail!("invalid path - {}", path);
+        }
+
+        Ok(())
+    }
+}
diff --git a/common/src/mod.rs b/common/src/mod.rs
index fe377d7..738657e 100644
--- a/common/src/mod.rs
+++ b/common/src/mod.rs
@@ -1 +1,2 @@
+pub mod apt;
 mod calendar_event;
diff --git a/pmg-rs/src/apt/repositories.rs b/pmg-rs/src/apt/repositories.rs
index 596ea4d..f6ddb37 100644
--- a/pmg-rs/src/apt/repositories.rs
+++ b/pmg-rs/src/apt/repositories.rs
@@ -1,61 +1,13 @@
 #[perlmod::package(name = "PMG::RS::APT::Repositories")]
 mod export {
-    use std::convert::TryInto;
+    use anyhow::Error;
 
-    use anyhow::{bail, Error};
-    use serde::{Deserialize, Serialize};
-
-    use proxmox_apt::repositories::{
-        APTRepositoryFile, APTRepositoryFileError, APTRepositoryHandle, APTRepositoryInfo,
-        APTStandardRepository,
-    };
-
-    #[derive(Deserialize, Serialize)]
-    #[serde(rename_all = "kebab-case")]
-    /// Result for the repositories() function
-    pub struct RepositoriesResult {
-        /// Successfully parsed files.
-        pub files: Vec<APTRepositoryFile>,
-
-        /// Errors for files that could not be parsed or read.
-        pub errors: Vec<APTRepositoryFileError>,
-
-        /// Common digest for successfully parsed files.
-        pub digest: String,
-
-        /// Additional information/warnings about repositories.
-        pub infos: Vec<APTRepositoryInfo>,
-
-        /// Standard repositories and their configuration status.
-        pub standard_repos: Vec<APTStandardRepository>,
-    }
-
-    #[derive(Deserialize, Serialize)]
-    #[serde(rename_all = "kebab-case")]
-    /// For changing an existing repository.
-    pub struct ChangeProperties {
-        /// Whether the repository should be enabled or not.
-        pub enabled: Option<bool>,
-    }
+    use crate::common::apt::repositories::export as common;
 
     /// Get information about configured and standard repositories.
     #[export]
-    pub fn repositories() -> Result<RepositoriesResult, Error> {
-        let (files, errors, digest) = proxmox_apt::repositories::repositories()?;
-        let digest = hex::encode(&digest);
-
-        let suite = proxmox_apt::repositories::get_current_release_codename()?;
-
-        let infos = proxmox_apt::repositories::check_repositories(&files, suite);
-        let standard_repos = proxmox_apt::repositories::standard_repositories(&files, "pmg", suite);
-
-        Ok(RepositoriesResult {
-            files,
-            errors,
-            digest,
-            infos,
-            standard_repos,
-        })
+    pub fn repositories() -> Result<common::RepositoriesResult, Error> {
+        common::repositories("pmg")
     }
 
     /// Add the repository identified by the `handle`.
@@ -64,60 +16,7 @@ mod export {
     /// The `digest` parameter asserts that the configuration has not been modified.
     #[export]
     pub fn add_repository(handle: &str, digest: Option<&str>) -> Result<(), Error> {
-        let (mut files, errors, current_digest) = proxmox_apt::repositories::repositories()?;
-
-        let handle: APTRepositoryHandle = handle.try_into()?;
-        let suite = proxmox_apt::repositories::get_current_release_codename()?;
-
-        if let Some(digest) = digest {
-            let expected_digest = hex::decode(digest)?;
-            if expected_digest != current_digest {
-                bail!("detected modified configuration - file changed by other user? Try again.");
-            }
-        }
-
-        // check if it's already configured first
-        for file in files.iter_mut() {
-            for repo in file.repositories.iter_mut() {
-                if repo.is_referenced_repository(handle, "pmg", &suite.to_string()) {
-                    if repo.enabled {
-                        return Ok(());
-                    }
-
-                    repo.set_enabled(true);
-                    file.write()?;
-
-                    return Ok(());
-                }
-            }
-        }
-
-        let (repo, path) = proxmox_apt::repositories::get_standard_repository(handle, "pmg", suite);
-
-        if let Some(error) = errors.iter().find(|error| error.path == path) {
-            bail!(
-                "unable to parse existing file {} - {}",
-                error.path,
-                error.error,
-            );
-        }
-
-        if let Some(file) = files.iter_mut().find(|file| file.path == path) {
-            file.repositories.push(repo);
-
-            file.write()?;
-        } else {
-            let mut file = match APTRepositoryFile::new(&path)? {
-                Some(file) => file,
-                None => bail!("invalid path - {}", path),
-            };
-
-            file.repositories.push(repo);
-
-            file.write()?;
-        }
-
-        Ok(())
+        common::add_repository(handle, "pmg", digest)
     }
 
     /// Change the properties of the specified repository.
@@ -127,36 +26,9 @@ mod export {
     pub fn change_repository(
         path: &str,
         index: usize,
-        options: ChangeProperties,
+        options: common::ChangeProperties,
         digest: Option<&str>,
     ) -> Result<(), Error> {
-        let (mut files, errors, current_digest) = proxmox_apt::repositories::repositories()?;
-
-        if let Some(digest) = digest {
-            let expected_digest = hex::decode(digest)?;
-            if expected_digest != current_digest {
-                bail!("detected modified configuration - file changed by other user? Try again.");
-            }
-        }
-
-        if let Some(error) = errors.iter().find(|error| error.path == path) {
-            bail!("unable to parse file {} - {}", error.path, error.error);
-        }
-
-        if let Some(file) = files.iter_mut().find(|file| file.path == path) {
-            if let Some(repo) = file.repositories.get_mut(index) {
-                if let Some(enabled) = options.enabled {
-                    repo.set_enabled(enabled);
-                }
-
-                file.write()?;
-            } else {
-                bail!("invalid index - {}", index);
-            }
-        } else {
-            bail!("invalid path - {}", path);
-        }
-
-        Ok(())
+        common::change_repository(path, index, options, digest)
     }
 }
diff --git a/pve-rs/src/apt/repositories.rs b/pve-rs/src/apt/repositories.rs
index 2d5e1da..d5c2f56 100644
--- a/pve-rs/src/apt/repositories.rs
+++ b/pve-rs/src/apt/repositories.rs
@@ -1,61 +1,13 @@
 #[perlmod::package(name = "PVE::RS::APT::Repositories", lib = "pve_rs")]
 mod export {
-    use std::convert::TryInto;
+    use anyhow::Error;
 
-    use anyhow::{bail, Error};
-    use serde::{Deserialize, Serialize};
-
-    use proxmox_apt::repositories::{
-        APTRepositoryFile, APTRepositoryFileError, APTRepositoryHandle, APTRepositoryInfo,
-        APTStandardRepository,
-    };
-
-    #[derive(Deserialize, Serialize)]
-    #[serde(rename_all = "kebab-case")]
-    /// Result for the repositories() function
-    pub struct RepositoriesResult {
-        /// Successfully parsed files.
-        pub files: Vec<APTRepositoryFile>,
-
-        /// Errors for files that could not be parsed or read.
-        pub errors: Vec<APTRepositoryFileError>,
-
-        /// Common digest for successfully parsed files.
-        pub digest: String,
-
-        /// Additional information/warnings about repositories.
-        pub infos: Vec<APTRepositoryInfo>,
-
-        /// Standard repositories and their configuration status.
-        pub standard_repos: Vec<APTStandardRepository>,
-    }
-
-    #[derive(Deserialize, Serialize)]
-    #[serde(rename_all = "kebab-case")]
-    /// For changing an existing repository.
-    pub struct ChangeProperties {
-        /// Whether the repository should be enabled or not.
-        pub enabled: Option<bool>,
-    }
+    use crate::common::apt::repositories::export as common;
 
     /// Get information about configured and standard repositories.
     #[export]
-    pub fn repositories() -> Result<RepositoriesResult, Error> {
-        let (files, errors, digest) = proxmox_apt::repositories::repositories()?;
-        let digest = hex::encode(&digest);
-
-        let suite = proxmox_apt::repositories::get_current_release_codename()?;
-
-        let infos = proxmox_apt::repositories::check_repositories(&files, suite);
-        let standard_repos = proxmox_apt::repositories::standard_repositories(&files, "pve", suite);
-
-        Ok(RepositoriesResult {
-            files,
-            errors,
-            digest,
-            infos,
-            standard_repos,
-        })
+    pub fn repositories() -> Result<common::RepositoriesResult, Error> {
+        common::repositories("pve")
     }
 
     /// Add the repository identified by the `handle`.
@@ -64,60 +16,7 @@ mod export {
     /// The `digest` parameter asserts that the configuration has not been modified.
     #[export]
     pub fn add_repository(handle: &str, digest: Option<&str>) -> Result<(), Error> {
-        let (mut files, errors, current_digest) = proxmox_apt::repositories::repositories()?;
-
-        let handle: APTRepositoryHandle = handle.try_into()?;
-        let suite = proxmox_apt::repositories::get_current_release_codename()?;
-
-        if let Some(digest) = digest {
-            let expected_digest = hex::decode(digest)?;
-            if expected_digest != current_digest {
-                bail!("detected modified configuration - file changed by other user? Try again.");
-            }
-        }
-
-        // check if it's already configured first
-        for file in files.iter_mut() {
-            for repo in file.repositories.iter_mut() {
-                if repo.is_referenced_repository(handle, "pve", &suite.to_string()) {
-                    if repo.enabled {
-                        return Ok(());
-                    }
-
-                    repo.set_enabled(true);
-                    file.write()?;
-
-                    return Ok(());
-                }
-            }
-        }
-
-        let (repo, path) = proxmox_apt::repositories::get_standard_repository(handle, "pve", suite);
-
-        if let Some(error) = errors.iter().find(|error| error.path == path) {
-            bail!(
-                "unable to parse existing file {} - {}",
-                error.path,
-                error.error,
-            );
-        }
-
-        if let Some(file) = files.iter_mut().find(|file| file.path == path) {
-            file.repositories.push(repo);
-
-            file.write()?;
-        } else {
-            let mut file = match APTRepositoryFile::new(&path)? {
-                Some(file) => file,
-                None => bail!("invalid path - {}", path),
-            };
-
-            file.repositories.push(repo);
-
-            file.write()?;
-        }
-
-        Ok(())
+        common::add_repository(handle, "pve", digest)
     }
 
     /// Change the properties of the specified repository.
@@ -127,36 +26,9 @@ mod export {
     pub fn change_repository(
         path: &str,
         index: usize,
-        options: ChangeProperties,
+        options: common::ChangeProperties,
         digest: Option<&str>,
     ) -> Result<(), Error> {
-        let (mut files, errors, current_digest) = proxmox_apt::repositories::repositories()?;
-
-        if let Some(digest) = digest {
-            let expected_digest = hex::decode(digest)?;
-            if expected_digest != current_digest {
-                bail!("detected modified configuration - file changed by other user? Try again.");
-            }
-        }
-
-        if let Some(error) = errors.iter().find(|error| error.path == path) {
-            bail!("unable to parse file {} - {}", error.path, error.error);
-        }
-
-        if let Some(file) = files.iter_mut().find(|file| file.path == path) {
-            if let Some(repo) = file.repositories.get_mut(index) {
-                if let Some(enabled) = options.enabled {
-                    repo.set_enabled(enabled);
-                }
-
-                file.write()?;
-            } else {
-                bail!("invalid index - {}", index);
-            }
-        } else {
-            bail!("invalid path - {}", path);
-        }
-
-        Ok(())
+        common::change_repository(path, index, options, digest)
     }
 }
-- 
2.30.2