Skip to content

Conversation

@TwanVanDongen
Copy link
Contributor

Initial implementation of the ARJ (Archived by Robert Jung) format, supporting 'store' for single file archives. Multi-file archives and all compression algorithms still require implementations.

…upporting 'store' for single file archives. Multi-file archives and all compression algorithms still require implementations.
@adamhathcock
Copy link
Owner

Thanks for this! Not sure why windows only tests are failing for this PR and I'm currently window machineless

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for the ARJ archive format to SharpCompress. The implementation includes readers, headers, and factory classes for handling ARJ archives.

Key Changes

  • Added ARJ format support with reader and header parsing functionality
  • Registered ARJ factory in the factory system
  • Added test cases for ARJ archive reading
  • Updated package dependency from Microsoft.NET.ILLink.Tasks 8.0.17 to 8.0.21
  • Updated error message to include ARJ in supported formats list

Reviewed Changes

Copilot reviewed 17 out of 18 changed files in this pull request and generated 11 comments.

Show a summary per file
File Description
src/SharpCompress/Readers/Arj/ArjReader.cs New ARJ reader implementation for reading ARJ archives
src/SharpCompress/Common/Arj/Headers/*.cs ARJ header parsing classes for main, local, and base headers
src/SharpCompress/Common/Arj/ArjEntry.cs ARJ entry implementation representing files in ARJ archives
src/SharpCompress/Common/Arj/ArjFilePart.cs File part implementation for ARJ entries
src/SharpCompress/Common/Arj/ArjVolume.cs Volume implementation for ARJ archives
src/SharpCompress/Factories/ArjFactory.cs Factory implementation for ARJ format detection and reader creation
src/SharpCompress/Factories/Factory.cs Registered ARJ factory in the static factory system
src/SharpCompress/Common/ArchiveType.cs Added Arj to the ArchiveType enumeration
src/SharpCompress/Readers/ReaderFactory.cs Updated error message to include Arj in supported formats
tests/SharpCompress.Test/Arj/ArjReaderTests.cs Test class for ARJ reader functionality
tests/TestArchives/Archives/Arj.store.arj Test archive file for ARJ format validation
src/SharpCompress/packages.lock.json Updated Microsoft.NET.ILLink.Tasks to version 8.0.21

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +10 to +11
using SharpCompress.Common.Zip;
using SharpCompress.Common.Zip.Headers;
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These Zip-related using directives appear to be unnecessary. The ArjReader doesn't reference any Zip types in its implementation. These should be removed to avoid confusion and unnecessary dependencies.

Suggested change
using SharpCompress.Common.Zip;
using SharpCompress.Common.Zip.Headers;

Copilot uses AI. Check for mistakes.

private void ProcessArchive(string archiveName)
{
// Process a given archive by its name
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Call to 'System.IO.Path.Combine'.

Suggested change
// Process a given archive by its name
// Process a given archive by its name
if (Path.IsPathRooted(archiveName) || archiveName != Path.GetFileName(archiveName))
{
throw new ArgumentException("Invalid archive name: must be a simple file name.", nameof(archiveName));
}

Copilot uses AI. Check for mistakes.
return Array.Empty<byte>();

byte[] crc = new byte[4];
read = stream.Read(crc, 0, 4);
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assignment to read is useless, since its value is never read.

Suggested change
read = stream.Read(crc, 0, 4);
if (stream.Read(crc, 0, 4) < 4)
throw new EndOfStreamException("Unexpected end of stream while reading header CRC.");

Copilot uses AI. Check for mistakes.
}

byte headerSize = headerBytes[offset++];
byte archiverVersionNumber = headerBytes[offset++];
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assignment to archiverVersionNumber is useless, since its value is never read.

Suggested change
byte archiverVersionNumber = headerBytes[offset++];
offset++;

Copilot uses AI. Check for mistakes.

byte headerSize = headerBytes[offset++];
byte archiverVersionNumber = headerBytes[offset++];
byte minVersionToExtract = headerBytes[offset++];
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assignment to minVersionToExtract is useless, since its value is never read.

Suggested change
byte minVersionToExtract = headerBytes[offset++];
offset++; // Skip minVersionToExtract, value not used

Copilot uses AI. Check for mistakes.
byte archiverVersionNumber = headerBytes[offset++];
byte minVersionToExtract = headerBytes[offset++];
HostOS hostOS = (HostOS)headerBytes[offset++];
byte arjFlags = headerBytes[offset++];
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assignment to arjFlags is useless, since its value is never read.

Suggested change
byte arjFlags = headerBytes[offset++];
offset++;

Copilot uses AI. Check for mistakes.
byte minVersionToExtract = headerBytes[offset++];
HostOS hostOS = (HostOS)headerBytes[offset++];
byte arjFlags = headerBytes[offset++];
CompressionMethod compressionMethod = CompressionMethodFromByte(headerBytes[offset++]);
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assignment to compressionMethod is useless, since its value is never read.

Suggested change
CompressionMethod compressionMethod = CompressionMethodFromByte(headerBytes[offset++]);

Copilot uses AI. Check for mistakes.
HostOS hostOS = (HostOS)headerBytes[offset++];
byte arjFlags = headerBytes[offset++];
CompressionMethod compressionMethod = CompressionMethodFromByte(headerBytes[offset++]);
FileType fileType = FileTypeFromByte(headerBytes[offset++]);
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assignment to fileType is useless, since its value is never read.

Suggested change
FileType fileType = FileTypeFromByte(headerBytes[offset++]);

Copilot uses AI. Check for mistakes.
protected override IEnumerable<ArjEntry> GetEntries(Stream stream)
{
var headerReader = new ArjMainHeader(new ArchiveEncoding());
var mainHeader = headerReader.Read(stream);
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assignment to mainHeader is useless, since its value is never read.

Suggested change
var mainHeader = headerReader.Read(stream);
headerReader.Read(stream);

Copilot uses AI. Check for mistakes.
Comment on lines +29 to +32
catch
{
DateTime = DateTime.MinValue;
}
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generic catch clause.

Suggested change
catch
{
DateTime = DateTime.MinValue;
}
catch (ArgumentOutOfRangeException)
{
DateTime = DateTime.MinValue;
}
catch (ArgumentException)
{
DateTime = DateTime.MinValue;
}

Copilot uses AI. Check for mistakes.
@adamhathcock
Copy link
Owner

#1000 might fix things

@TwanVanDongen
Copy link
Contributor Author

all tests pass on my windows box; but I see some obvious fixes that I need to make, will have to amend the PR.

@adamhathcock
Copy link
Owner

I'll keep an eye out for your fixes!

@TwanVanDongen
Copy link
Contributor Author

@adamhathcock , still all tests pass on my windows box, the error seems to be unrelated to my changes.

@adamhathcock adamhathcock merged commit d90b610 into adamhathcock:master Nov 1, 2025
1 of 2 checks passed
@adamhathcock
Copy link
Owner

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants