Re: [PATCH proxmox-offline-mirror 2/3] use proxmox-pgp crate to replace verifier helper module

[Nicolas Frey <n.frey@proxmox.com>]

Thanks for the review! (almost forgot about this series)
I'll send a v2 with your proposals

On 7/22/26 2:44 PM, Erik Fastermann wrote:
> Comments inline.
> 
> With those fixed consider:
> 
> Reviewed-by: Erik Fastermann <e.fastermann@proxmox.com>
> 
> 
>> Signed-off-by: Nicolas Frey <n.frey@proxmox.com>
>> ---
>>   Cargo.toml    |  1 +
>>   src/config.rs | 33 +--------------------------------
>>   src/mirror.rs |  7 +++----
>>   3 files changed, 5 insertions(+), 36 deletions(-)
>>
>> diff --git a/Cargo.toml b/Cargo.toml
>> index ff54637..7aa285a 100644
>> --- a/Cargo.toml
>> +++ b/Cargo.toml
>> @@ -29,6 +29,7 @@ proxmox-apt-api-types = "2.0"
>>   proxmox-async = "0.5"
>>   proxmox-base64 = "1"
>>   proxmox-http = { version = "1", features = [ "client-sync",
>> "client-trait" ]}
>> +proxmox-pgp = "1"
>>   proxmox-router = { version = "3", features = [ "cli" ], default-
>> features = false }
>>   proxmox-schema = { version = "5", features = [ "api-macro" ] }
>>   proxmox-section-config = "3"
>> diff --git a/src/config.rs b/src/config.rs
>> index 0ca296e..c8916a0 100644
>> --- a/src/config.rs
>> +++ b/src/config.rs
>> @@ -2,6 +2,7 @@ use std::path::Path;
>>   use std::sync::LazyLock;
>>     use anyhow::{Error, bail};
>> +use proxmox_pgp::WeakCryptoConfig;
> 
> Nit: The use statement should be in the same block with all the other
> proxmox imports.
> 

ack

[snip]

>> @@ -207,7 +206,7 @@ fn fetch_release(
>>       println!("Verifying '{name}' signature using provided
>> repository key..");
>>       let content = fetched.data_ref();
>>       let verified =
>> -        helpers::verify_signature(content, &config.key,
>> sig.as_deref(), &config.weak_crypto)?;
>> +        proxmox_pgp::verify_signature(content, &config.key,
>> sig.as_deref(), &config.weak_crypto)?;
> 
> Consider documenting in the commit message that this change should
> have no user visible effect (AFAICT), except for some minor changes in
> error and status reporting introduced by the refactored
> `proxmox_pgp::verify_signature`.
> 

I suppose it's not clear here, as the code lives in a different crate
now. I'll add it as context to the commit message. Thanks!

>>       println!("Success");
>>         let sha512 = Some(openssl::sha::sha512(content));
>