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

Wolfgang Bumiller w.bumiller at proxmox.com
Fri Jul 8 15:07:46 CEST 2022


applied this one & bumped

On Fri, Jul 08, 2022 at 01:55:53PM +0200, Fabian Ebner wrote:
> while introducing a 'product' parameter to the relevant functions and
> adding wrappers for backwards-compatibility.
> 
> Suggested-by: Wolfgang Bumiller <w.bumiller at proxmox.com>
> Signed-off-by: Fabian Ebner <f.ebner at 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





More information about the pve-devel mailing list