Repository: alexcrichton/tar-rs (language: rust)

Issue:
Alternative append API
At the moment in the API, I have to provide a `Read` and then the `append` function(s) will `io::copy` over the bytes in one go. This is fine for many use cases, but not always desirable.

As an alternative that may work better in some cases, I propose having an API that instead of taking a `Read` returns a `Write`. One can then write into there in order to write into the archive. Lifetimes are use to enforce that no other functions can be called on the archive in the meantime, and the `Drop` implementation takes care of finalizing the file (i.e. add the padding).


Discussion / hints:
I'm researching on a seekable compressed TAR format, and I would like to see this API too, so that I can write the TAR header and body separately and compress them in different compression blocks.
Note that providing a handle that implements `Write` is strictly more generic than having a method that accepts `Read`, because [`std::io::copy`](https://doc.rust-lang.org/stable/std/io/fn.copy.html) can be used to transfer data from any reader into a writer.
A use case on URLO: https://users.rust-lang.org/t/streaming-a-reqwest-get-paylod-into-a-flate2-decoder/90670

Fix patch:
diff --git a/src/builder.rs b/src/builder.rs
index 9b799329..0f4ef8ff 100644
--- a/src/builder.rs
+++ b/src/builder.rs
@@ -165,6 +165,47 @@ impl<W: Write> Builder<W> {
         self.append(&header, data)
     }
 
+    /// Adds a new entry to this archive and returns an [`EntryWriter`] for
+    /// adding its contents.
+    ///
+    /// This function is similar to [`Self::append_data`] but returns a
+    /// [`io::Write`] implementation instead of taking data as a parameter.
+    ///
+    /// Similar constraints around the position of the archive and completion
+    /// apply as with [`Self::append_data`]. It requires the underlying writer
+    /// to implement [`Seek`] to update the header after writing the data.
+    ///
+    /// # Errors
+    ///
+    /// This function will return an error for any intermittent I/O error which
+    /// occurs when either reading or writing.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use std::io::Cursor;
+    /// use std::io::Write as _;
+    /// use tar::{Builder, Header};
+    ///
+    /// let mut header = Header::new_gnu();
+    ///
+    /// let mut ar = Builder::new(Cursor::new(Vec::new()));
+    /// let mut entry = ar.append_writer(&mut header, "hi.txt").unwrap();
+    /// entry.write_all(b"Hello, ").unwrap();
+    /// entry.write_all(b"world!\n").unwrap();
+    /// entry.finish().unwrap();
+    /// ```
+    pub fn append_writer<'a, P: AsRef<Path>>(
+        &'a mut self,
+        header: &'a mut Header,
+        path: P,
+    ) -> io::Result<EntryWriter<'a>>
+    where
+        W: Seek,
+    {
+        EntryWriter::start(self.get_mut(), header, path.as_ref())
+    }
+
     /// Adds a new link (symbolic or hard) entry to this archive with the specified path and target.
     ///
     /// This function is similar to [`Self::append_data`] which supports long filenames,
@@ -440,6 +481,92 @@ impl<W: Write> Builder<W> {
     }
 }
 
+trait SeekWrite: Write + Seek {
+    fn as_write(&mut self) -> &mut dyn Write;
+}
+
+impl<T: Write + Seek> SeekWrite for T {
+    fn as_write(&mut self) -> &mut dyn Write {
+        self
+    }
+}
+
+/// A writer for a single entry in a tar archive.
+///
+/// This struct is returned by [`Builder::append_writer`] and provides a
+/// [`Write`] implementation for adding content to an archive entry.
+///
+/// After writing all data to the entry, it must be finalized either by
+/// explicitly calling [`EntryWriter::finish`] or by letting it drop.
+pub struct EntryWriter<'a> {
+    obj: &'a mut dyn SeekWrite,
+    header: &'a mut Header,
+    written: u64,
+}
+
+impl EntryWriter<'_> {
+    fn start<'a>(
+        obj: &'a mut dyn SeekWrite,
+        header: &'a mut Header,
+        path: &Path,
+    ) -> io::Result<EntryWriter<'a>> {
+        prepare_header_path(obj.as_write(), header, path)?;
+
+        // Reserve space for header, will be overwritten once data is written.
+        obj.write_all([0u8; 512].as_ref())?;
+
+        Ok(EntryWriter {
+            obj,
+            header,
+            written: 0,
+        })
+    }
+
+    /// Finish writing the current entry in the archive.
+    pub fn finish(self) -> io::Result<()> {
+        let mut this = std::mem::ManuallyDrop::new(self);
+        this.do_finish()
+    }
+
+    fn do_finish(&mut self) -> io::Result<()> {
+        // Pad with zeros if necessary.
+        let buf = [0u8; 512];
+        let remaining = u64::wrapping_sub(512, self.written) % 512;
+        self.obj.write_all(&buf[..remaining as usize])?;
+        let written = (self.written + remaining) as i64;
+
+        // Seek back to the header position.
+        self.obj.seek(io::SeekFrom::Current(-written - 512))?;
+
+        self.header.set_size(self.written);
+        self.header.set_cksum();
+        self.obj.write_all(self.header.as_bytes())?;
+
+        // Seek forward to restore the position.
+        self.obj.seek(io::SeekFrom::Current(written))?;
+
+        Ok(())
+    }
+}
+
+impl Write for EntryWriter<'_> {
+    fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
+        let len = self.obj.write(buf)?;
+        self.written += len as u64;
+        Ok(len)
+    }
+
+    fn flush(&mut self) -> io::Result<()> {
+        self.obj.flush()
+    }
+}
+
+impl Drop for EntryWriter<'_> {
+    fn drop(&mut self) {
+        let _ = self.do_finish();
+    }
+}
+
 fn append(mut dst: &mut dyn Write, header: &Header, mut data: &mut dyn Read) -> io::Result<()> {
     dst.write_all(header.as_bytes())?;
     let len = io::copy(&mut data, &mut dst)?;
diff --git a/src/lib.rs b/src/lib.rs
index 52251cd2..78d89a05 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -24,7 +24,7 @@
 use std::io::{Error, ErrorKind};
 
 pub use crate::archive::{Archive, Entries};
-pub use crate::builder::Builder;
+pub use crate::builder::{Builder, EntryWriter};
 pub use crate::entry::{Entry, Unpacked};
 pub use crate::entry_type::EntryType;
 pub use crate::header::GnuExtSparseHeader;


Test patch:
diff --git a/tests/all.rs b/tests/all.rs
index 27a6fcf1..5ac28ce5 100644
--- a/tests/all.rs
+++ b/tests/all.rs
@@ -1043,6 +1043,45 @@ fn linkname_literal() {
     }
 }
 
+#[test]
+fn append_writer() {
+    let mut b = Builder::new(Cursor::new(Vec::new()));
+
+    let mut h = Header::new_gnu();
+    h.set_uid(42);
+    let mut writer = t!(b.append_writer(&mut h, "file1"));
+    t!(writer.write_all(b"foo"));
+    t!(writer.write_all(b"barbaz"));
+    t!(writer.finish());
+
+    let mut h = Header::new_gnu();
+    h.set_uid(43);
+    let long_path: PathBuf = repeat("abcd").take(50).collect();
+    let mut writer = t!(b.append_writer(&mut h, &long_path));
+    let long_data = repeat(b'x').take(513).collect::<Vec<u8>>();
+    t!(writer.write_all(&long_data));
+    t!(writer.finish());
+
+    let contents = t!(b.into_inner()).into_inner();
+    let mut ar = Archive::new(&contents[..]);
+    let mut entries = t!(ar.entries());
+
+    let e = &mut t!(entries.next().unwrap());
+    assert_eq!(e.header().uid().unwrap(), 42);
+    assert_eq!(&*e.path_bytes(), b"file1");
+    let mut r = Vec::new();
+    t!(e.read_to_end(&mut r));
+    assert_eq!(&r[..], b"foobarbaz");
+
+    let e = &mut t!(entries.next().unwrap());
+    assert_eq!(e.header().uid().unwrap(), 43);
+    assert_eq!(t!(e.path()), long_path.as_path());
+    let mut r = Vec::new();
+    t!(e.read_to_end(&mut r));
+    assert_eq!(r.len(), 513);
+    assert!(r.iter().all(|b| *b == b'x'));
+}
+
 #[test]
 fn encoded_long_name_has_trailing_nul() {
     let td = t!(TempBuilder::new().prefix("tar-rs").tempdir());