[pbs-devel] [PATCH v8 pxar 06/69] encoder: allow split output writer for archive creation

[Dominik Csapak]

one tiny nit inline, but that does not warrant a new version IMHO,
just maybe consider it if you send a new version anyway

On 5/28/24 11:42, Christian Ebner wrote:
[snip]
> diff --git a/src/encoder/sync.rs b/src/encoder/sync.rs
> index 48a97af..9d39658 100644
> --- a/src/encoder/sync.rs
> +++ b/src/encoder/sync.rs
> @@ -9,7 +9,7 @@ use crate::decoder::sync::StandardReader;
>   use crate::encoder::{self, LinkOffset, SeqWrite};
>   use crate::format;
>   use crate::util::poll_result_once;
> -use crate::Metadata;
> +use crate::{Metadata, PxarVariant};
>   
>   /// Blocking `pxar` encoder.
>   ///
> @@ -28,7 +28,7 @@ impl<'a, T: io::Write + 'a> Encoder<'a, StandardWriter<T>> {
>       /// Encode a `pxar` archive into a regular `std::io::Write` output.
>       #[inline]
>       pub fn from_std(output: T, metadata: &Metadata) -> io::Result<Encoder<'a, StandardWriter<T>>> {
> -        Encoder::new(StandardWriter::new(output), metadata)
> +        Encoder::new(PxarVariant::Unified(StandardWriter::new(output)), metadata)
>       }
>   }
>   
> @@ -39,7 +39,7 @@ impl<'a> Encoder<'a, StandardWriter<std::fs::File>> {
>           metadata: &'b Metadata,
>       ) -> io::Result<Encoder<'a, StandardWriter<std::fs::File>>> {
>           Encoder::new(
> -            StandardWriter::new(std::fs::File::create(path.as_ref())?),
> +            PxarVariant::Unified(StandardWriter::new(std::fs::File::create(path.as_ref())?)),
>               metadata,
>           )
>       }
> @@ -50,9 +50,18 @@ impl<'a, T: SeqWrite + 'a> Encoder<'a, T> {
>       ///
>       /// Note that the `output`'s `SeqWrite` implementation must always return `Poll::Ready` and is
>       /// not allowed to use the `Waker`, as this will cause a `panic!`.
> -    pub fn new(output: T, metadata: &Metadata) -> io::Result<Self> {
> +    // Optionally attach a dedicated writer to redirect the payloads of regular files to a separate
> +    // output.
> +    pub fn new(output: PxarVariant<T, T>, metadata: &Metadata) -> io::Result<Self> {
> +        let output = match output {
> +            PxarVariant::Unified(output) => PxarVariant::Unified(output.into()),
> +            PxarVariant::Split(output, payload_output) => {
> +                PxarVariant::Split(output.into(), payload_output)
> +            }
> +        };
> +

nit: this could be shortened with the `wrap_multi` helper:

output.wrap_multi(|a| a.into, |p| p);

and then it could probably be inlined as well

>           Ok(Self {
> -            inner: poll_result_once(encoder::EncoderImpl::new(output.into(), metadata))?,
> +            inner: poll_result_once(encoder::EncoderImpl::new(output, metadata))?,
>           })
>       }
>