Re: [pve-devel] [PATCH v3 proxmox-perl-rs 24/66] add PVE::RS::Notify module

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

On Wed, Jul 19, 2023 at 12:10:02PM +0200, Wolfgang Bumiller wrote:
> On Mon, Jul 17, 2023 at 05:00:09PM +0200, Lukas Wagner wrote:
> > Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> > ---
> >  pve-rs/Cargo.toml    |  1 +
> >  pve-rs/Makefile      |  1 +
> >  pve-rs/src/lib.rs    |  1 +
> >  pve-rs/src/notify.rs | 71 ++++++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 74 insertions(+)
> >  create mode 100644 pve-rs/src/notify.rs
> > 
> > diff --git a/pve-rs/Cargo.toml b/pve-rs/Cargo.toml
> > index 3076a13..b28c118 100644
> > --- a/pve-rs/Cargo.toml
> > +++ b/pve-rs/Cargo.toml
> > @@ -34,6 +34,7 @@ perlmod = { version = "0.13", features = [ "exporter" ] }
> >  
> >  proxmox-apt = "0.10"
> >  proxmox-http = { version = "0.9", features = ["client-sync", "client-trait"] }
> > +proxmox-notify = "0.1"
> >  proxmox-openid = "0.10"
> >  proxmox-resource-scheduling = "0.3.0"
> >  proxmox-subscription = "0.4"
> > diff --git a/pve-rs/Makefile b/pve-rs/Makefile
> > index de35c69..9d737c0 100644
> > --- a/pve-rs/Makefile
> > +++ b/pve-rs/Makefile
> > @@ -27,6 +27,7 @@ PERLMOD_GENPACKAGE := /usr/lib/perlmod/genpackage.pl \
> >  
> >  PERLMOD_PACKAGES := \
> >  	  PVE::RS::APT::Repositories \
> > +	  PVE::RS::Notify \
> >  	  PVE::RS::OpenId \
> >  	  PVE::RS::ResourceScheduling::Static \
> >  	  PVE::RS::TFA
> > diff --git a/pve-rs/src/lib.rs b/pve-rs/src/lib.rs
> > index eb6ae02..0d63c28 100644
> > --- a/pve-rs/src/lib.rs
> > +++ b/pve-rs/src/lib.rs
> > @@ -4,6 +4,7 @@
> >  pub mod common;
> >  
> >  pub mod apt;
> > +pub mod notify;
> >  pub mod openid;
> >  pub mod resource_scheduling;
> >  pub mod tfa;
> > diff --git a/pve-rs/src/notify.rs b/pve-rs/src/notify.rs
> > new file mode 100644
> > index 0000000..9677d8b
> > --- /dev/null
> > +++ b/pve-rs/src/notify.rs
> > @@ -0,0 +1,71 @@
> > +#[perlmod::package(name = "PVE::RS::Notify")]
> > +mod export {
> > +    use anyhow::{bail, Error};
> > +    use perlmod::Value;
> > +
> > +    use std::sync::Mutex;
> > +
> > +    use proxmox_notify::Config;
> > +
> > +    pub struct NotificationConfig {
> > +        config: Mutex<Config>,
> > +    }
> > +
> > +    perlmod::declare_magic!(Box<NotificationConfig> : &NotificationConfig as "PVE::RS::Notify");
> > +
> > +    /// Support `dclone` so this can be put into the `ccache` of `PVE::Cluster`.
> > +    #[export(name = "STORABLE_freeze", raw_return)]
> > +    fn storable_freeze(
> > +        #[try_from_ref] this: &NotificationConfig,
> > +        cloning: bool,
> > +    ) -> Result<Value, Error> {
> > +        if !cloning {
> > +            bail!("freezing Notification config not supported!");
> > +        }
> > +
> > +        let mut cloned = Box::new(NotificationConfig {
> > +            config: Mutex::new(this.config.lock().unwrap().clone()),
> > +        });
> > +        let value = Value::new_pointer::<NotificationConfig>(&mut *cloned);
> > +        let _perl = Box::leak(cloned);
> > +        Ok(value)
> > +    }
> > +
> > +    /// Instead of `thaw` we implement `attach` for `dclone`.
> > +    #[export(name = "STORABLE_attach", raw_return)]
> > +    fn storable_attach(
> > +        #[raw] class: Value,
> > +        cloning: bool,
> > +        #[raw] serialized: Value,
> > +    ) -> Result<Value, Error> {
> > +        if !cloning {
> > +            bail!("STORABLE_attach called with cloning=false");
> > +        }
> > +        let data = unsafe { Box::from_raw(serialized.pv_raw::<NotificationConfig>()?) };
> > +        Ok(perlmod::instantiate_magic!(&class, MAGIC => data))
> > +    }
> > +
> > +    #[export(raw_return)]
> > +    fn parse_config(
> > +        #[raw] class: Value,
> > +        raw_config: &str,
> > +        raw_private_config: &str,
> 
> I half-suspect that using &[u8] here - since that's what perl *actually*
> gives us if we just read the data out of files without setting the mode
> to 'utf8' or anything - might just fix the emoji issues.
> 
> Alternatively, perl would have to actually declare an encoding ;-)

Though ultimately the alter patches will probably need to do the same
for things where utf-8 matters (at least the comments).