From: Wolfgang Bumiller <w.bumiller@proxmox.com>
To: Fabian Ebner <f.ebner@proxmox.com>
Cc: pve-devel@lists.proxmox.com, pmg-devel@lists.proxmox.com
Subject: [pmg-devel] applied: [PATCH v3 proxmox-perl-rs] move apt repositories module to common
Date: Fri, 8 Jul 2022 15:07:46 +0200 [thread overview]
Message-ID: <20220708130746.iqdggvlgmjoygglr@wobu-vie.proxmox.com> (raw)
In-Reply-To: <20220708115555.82693-1-f.ebner@proxmox.com>
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@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
prev parent reply other threads:[~2022-07-08 13:07 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-08 11:55 [pmg-devel] " Fabian Ebner
2022-07-08 11:55 ` [pmg-devel] [PATCH v3 manager] api: apt: switch to common Proxmox::RS::APT::Repositories package Fabian Ebner
2022-07-15 10:40 ` [pmg-devel] applied: " Wolfgang Bumiller
2022-07-08 11:55 ` [pmg-devel] [PATCH v3 pmg-api] " Fabian Ebner
2022-07-13 9:12 ` [pmg-devel] applied: " Thomas Lamprecht
2022-07-08 13:07 ` Wolfgang Bumiller [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220708130746.iqdggvlgmjoygglr@wobu-vie.proxmox.com \
--to=w.bumiller@proxmox.com \
--cc=f.ebner@proxmox.com \
--cc=pmg-devel@lists.proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox