Add alternative option for writing TAR archives with USTAR header format#1063
Conversation
- TarWriterOptions now has a property that allows the user to select writing the TAR using the USTAR header format - if unspecified, will default to the original modern GNU TAR header format - default behavior is unchanged
|
This is great, thanks! If you could format it as I'll add a test or two with copilot. |
There was a problem hiding this comment.
Pull request overview
This PR adds support for writing TAR archives using the legacy USTAR header format as an alternative to the default GNU TAR format. This enables compatibility with older tools that only accept USTAR format.
- Introduces
TarHeaderWriteFormatenum to specify the header format (GNU or USTAR) - Extends
TarWriterOptionswith an optionalHeaderFormatparameter defaulting to GNU format - Implements USTAR format writing with filename splitting logic for paths exceeding 100 characters
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 20 comments.
| File | Description |
|---|---|
TarHeaderWriteFormat.cs |
New enum defining available TAR header formats (GNU_TAR_LONG_LINK and USTAR) |
TarWriterOptions.cs |
Updated constructor to accept optional header format parameter and store it as a property |
TarWriter.cs |
Added field to track header format and pass it to TarHeader during construction |
TarHeader.cs |
Refactored Write method to dispatch based on format; implemented WriteUstar method with name splitting logic for long filenames |
Comments suppressed due to low confidence (1)
src/SharpCompress/Writers/Tar/TarWriter.cs:18
- Field 'headerFormat' can be 'readonly'.
private TarHeaderWriteFormat headerFormat;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public class TarWriter : AbstractWriter | ||
| { | ||
| private readonly bool finalizeArchiveOnClose; | ||
| private TarHeaderWriteFormat headerFormat; |
There was a problem hiding this comment.
The field headerFormat should be readonly since it's only assigned in the constructor and never modified afterward. This makes the code's intent clearer and prevents accidental modification.
| private TarHeaderWriteFormat headerFormat; | |
| private readonly TarHeaderWriteFormat headerFormat; |
| if (this.ArchiveEncoding.GetEncoding().GetByteCount(namePrefix) >= 155) | ||
| throw new Exception($"Tar header USTAR format can not fit file name \"{fullName}\" of length {nameByteCount}! Try using GNU Tar format instead!"); | ||
|
|
||
| if (this.ArchiveEncoding.GetEncoding().GetByteCount(name) >= 100) |
There was a problem hiding this comment.
The prefix this. is inconsistent with the rest of the codebase where ArchiveEncoding is accessed without the qualifier. For consistency, remove this. from lines 105 and 108.
| if (this.ArchiveEncoding.GetEncoding().GetByteCount(namePrefix) >= 155) | |
| throw new Exception($"Tar header USTAR format can not fit file name \"{fullName}\" of length {nameByteCount}! Try using GNU Tar format instead!"); | |
| if (this.ArchiveEncoding.GetEncoding().GetByteCount(name) >= 100) | |
| if (ArchiveEncoding.GetEncoding().GetByteCount(namePrefix) >= 155) | |
| throw new Exception($"Tar header USTAR format can not fit file name \"{fullName}\" of length {nameByteCount}! Try using GNU Tar format instead!"); | |
| if (ArchiveEncoding.GetEncoding().GetByteCount(name) >= 100) |
| } | ||
| else | ||
| { | ||
| WriteStringBytes(ArchiveEncoding.Encode(Name.NotNull("Name is null")), buffer, 100); |
There was a problem hiding this comment.
This call to WriteStringBytes is missing the offset parameter. It should be WriteStringBytes(ArchiveEncoding.Encode(Name.NotNull("Name is null")), buffer, 0, 100); to write the name starting at offset 0. Without the offset parameter, this will use the wrong overload and write to an incorrect location.
| WriteStringBytes(ArchiveEncoding.Encode(Name.NotNull("Name is null")), buffer, 100); | |
| WriteStringBytes(ArchiveEncoding.Encode(Name.NotNull("Name is null")), buffer, 100, 100); |
| internal TarWriterOptions(WriterOptions options) | ||
| : this(options.CompressionType, true) => ArchiveEncoding = options.ArchiveEncoding; |
There was a problem hiding this comment.
The internal constructor doesn't pass the headerFormat parameter, which means it will always use the default GNU_TAR_LONG_LINK format. This could lead to unexpected behavior when creating a TarWriter from generic WriterOptions. Consider adding a parameter or explicitly setting the header format in this constructor.
| internal TarWriterOptions(WriterOptions options) | |
| : this(options.CompressionType, true) => ArchiveEncoding = options.ArchiveEncoding; | |
| internal TarWriterOptions(WriterOptions options, TarHeaderWriteFormat headerFormat = TarHeaderWriteFormat.GNU_TAR_LONG_LINK) | |
| : this(options.CompressionType, true, headerFormat) => ArchiveEncoding = options.ArchiveEncoding; |
| throw new Exception($"Tar header USTAR format can not fit file name \"{fullName}\" of length {nameByteCount}! Directory separator not found! Try using GNU Tar format instead!"); | ||
| } | ||
|
|
||
| string namePrefix = fullName.Substring(0, splitIndex); | ||
| string name = fullName.Substring(splitIndex + 1); | ||
|
|
||
| if (this.ArchiveEncoding.GetEncoding().GetByteCount(namePrefix) >= 155) | ||
| throw new Exception($"Tar header USTAR format can not fit file name \"{fullName}\" of length {nameByteCount}! Try using GNU Tar format instead!"); | ||
|
|
||
| if (this.ArchiveEncoding.GetEncoding().GetByteCount(name) >= 100) | ||
| throw new Exception($"Tar header USTAR format can not fit file name \"{fullName}\" of length {nameByteCount}! Try using GNU Tar format instead!"); |
There was a problem hiding this comment.
The phrase "can not" should be written as one word: "cannot".
| throw new Exception($"Tar header USTAR format can not fit file name \"{fullName}\" of length {nameByteCount}! Directory separator not found! Try using GNU Tar format instead!"); | |
| } | |
| string namePrefix = fullName.Substring(0, splitIndex); | |
| string name = fullName.Substring(splitIndex + 1); | |
| if (this.ArchiveEncoding.GetEncoding().GetByteCount(namePrefix) >= 155) | |
| throw new Exception($"Tar header USTAR format can not fit file name \"{fullName}\" of length {nameByteCount}! Try using GNU Tar format instead!"); | |
| if (this.ArchiveEncoding.GetEncoding().GetByteCount(name) >= 100) | |
| throw new Exception($"Tar header USTAR format can not fit file name \"{fullName}\" of length {nameByteCount}! Try using GNU Tar format instead!"); | |
| throw new Exception($"Tar header USTAR format cannot fit file name \"{fullName}\" of length {nameByteCount}! Directory separator not found! Try using GNU Tar format instead!"); | |
| } | |
| string namePrefix = fullName.Substring(0, splitIndex); | |
| string name = fullName.Substring(splitIndex + 1); | |
| if (this.ArchiveEncoding.GetEncoding().GetByteCount(namePrefix) >= 155) | |
| throw new Exception($"Tar header USTAR format cannot fit file name \"{fullName}\" of length {nameByteCount}! Try using GNU Tar format instead!"); | |
| if (this.ArchiveEncoding.GetEncoding().GetByteCount(name) >= 100) | |
| throw new Exception($"Tar header USTAR format cannot fit file name \"{fullName}\" of length {nameByteCount}! Try using GNU Tar format instead!"); |
|
Thanks! |
Changes Summary
Issues
Context
Some time last year when I was using SharpCompress for a game mod manager, I ran into an issue where tar.bz2 files compressed with SharpCompressed were incompatible with some source ports of that game that would only accept the old USTAR format.
I didn't think of making a pull request at the time as it seems like a very niche thing, but now I'm thinking someone else might find it useful too.