Conversation
Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com>
Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com>
Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com>
Add [Obsolete] attribute to ReaderOptions.DefaultBufferSize for backward compatibility
Fix grammatical errors in ArcFactory comment documentation
…ement minor versions
…into adam/buffer-size-consolidation
…tion (Release) Buffer size consolidation
Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com>
Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com>
Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…gression Fix ZIP parsing failure on non-seekable streams with short reads
Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com>
Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com>
Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…guous-streams Fix SevenZipReader to maintain contiguous stream state for solid archives
# Conflicts: # src/SharpCompress/Archives/ArchiveFactory.cs # src/SharpCompress/Archives/AutoArchiveFactory.cs # src/SharpCompress/Archives/SevenZip/SevenZipArchive.cs # src/SharpCompress/Archives/Zip/ZipArchive.cs # src/SharpCompress/Factories/AceFactory.cs # src/SharpCompress/Factories/ArcFactory.cs # src/SharpCompress/Factories/ArjFactory.cs # src/SharpCompress/Factories/Factory.cs # src/SharpCompress/Factories/GZipFactory.cs # src/SharpCompress/Factories/IFactory.cs # src/SharpCompress/Factories/RarFactory.cs # src/SharpCompress/Factories/SevenZipFactory.cs # src/SharpCompress/Factories/TarFactory.cs # src/SharpCompress/Factories/ZStandardFactory.cs # src/SharpCompress/Factories/ZipFactory.cs # src/SharpCompress/IO/SharpCompressStream.cs # src/SharpCompress/Readers/AbstractReader.cs # src/SharpCompress/Utility.cs
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (8 files)
|
There was a problem hiding this comment.
Pull request overview
This pull request merges the release branch to master, focusing on modernizing SharpCompress's stream handling infrastructure, improving build versioning, and cleaning up obsolete configuration. The changes replace the complex SharpCompressStream buffering system with simpler stream wrappers (NonDisposingStream, CountingStream, RewindableStream), standardize buffer sizes through Constants.BufferSize, and add async support for compression formats.
Changes:
- Removed legacy Copilot agent configuration and planning documents
- Enhanced build versioning to increment patch version on release branches, minor version on other branches
- Refactored stream handling to remove
SharpCompressStreamwrapper, using simpler alternatives - Added async support for multiple compression formats (Squeezed, Reduce, Shrink, Arc, filters)
- Improved 7Zip solid archive stream reuse with diagnostic support
Reviewed changes
Copilot reviewed 193 out of 193 changed files in this pull request and generated 68 comments.
Show a summary per file
| File | Description |
|---|---|
.copilot-agent.yml |
Removed Copilot agent manifest |
.github/agents/copilot-agent.yml |
Removed detailed agent policies |
.github/prompts/plan-*.prompt.md |
Removed planning documents |
build/Program.cs |
Enhanced versioning logic with branch detection |
src/SharpCompress/Utility.Async.cs |
Added async stream extension methods using new extension syntax |
src/SharpCompress/Common/Constants.cs |
Added global buffer size constants |
src/SharpCompress/IO/* |
New stream wrappers and refactored stream handling |
src/SharpCompress/Compressors/* |
Added async support for compression formats |
tests/SharpCompress.Test/Zip/ZipShortReadTests.cs |
New tests for ZIP short read scenarios |
| Multiple test files | Updated to use NonDisposingStream instead of SharpCompressStream |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| void IStreamStack.SetPosition(long position) { } | ||
| private readonly Stream _stream; | ||
| private readonly bool _useSyncOverAsyncDispose; |
There was a problem hiding this comment.
The field _useSyncOverAsyncDispose is declared but never assigned a value in the constructor. This will always be false, potentially causing incorrect behavior. The constructor should accept and assign this parameter.
| { | ||
| while ( | ||
| (_low ^ (_low + _range)) < RANGE_TOP | ||
| || _range < RANGE_BOTTOM && ((_range = (uint)-_low & (RANGE_BOTTOM - 1)) != 0 || true) |
There was a problem hiding this comment.
The expression 'A || true' is always 'true'.
| private bool _isFinalBlock; | ||
| private long _totalBytesLeftToRead; | ||
| private bool _isDisposed; | ||
| private bool _useSyncOverAsyncDispose; |
There was a problem hiding this comment.
Field '_useSyncOverAsyncDispose' can be 'readonly'.
| public virtual Stream BaseStream() => stream; | ||
|
|
||
| private readonly Stream stream; | ||
| private MemoryStream bufferStream = new MemoryStream(); |
There was a problem hiding this comment.
Field 'bufferStream' can be 'readonly'.
| if (inSize + _read > _tail.Length) | ||
| { | ||
| _transformed = Transform(_window, 0, inSize + _read); | ||
| } | ||
| else | ||
| { | ||
| _transformed = inSize + _read; | ||
| } |
There was a problem hiding this comment.
Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.
| if (inSize + _read > _tail.Length) | ||
| { | ||
| _transformed = Transform(_window, 0, inSize + _read); | ||
| } | ||
| else | ||
| { | ||
| _transformed = inSize + _read; | ||
| } |
There was a problem hiding this comment.
Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.
This pull request includes several changes across configuration, build tooling, and the core library, focusing on improving versioning logic, stream handling, and internal diagnostics for 7z archives. The most significant updates are the removal of legacy agent and planning files, enhancements to the build versioning strategy, and improvements to stream management and diagnostics in the core archive code.
Configuration and Documentation Cleanup:
.copilot-agent.ymland.github/agents/copilot-agent.yml, eliminating old Copilot agent configuration and policies. [1] [2].github/prompts/plan-async.prompt.mdand.github/prompts/plan-for-next.prompt.md, cleaning up project documentation. [1] [2]Build and Versioning Improvements:
build/Program.csto increment the patch version for release branches and the minor version for other branches, with branch detection supporting both local and CI environments. [1] [2] [3]Core Library Enhancements:
ArchiveFactory.csandArchiveFactory.Async.csby removing unnecessary wrapping withSharpCompressStream.Create, simplifying stream management for archive operations. [1] [2]IArchiveEntryExtensions.csto use a sharedConstants.BufferSizefor all copy operations, ensuring consistency and maintainability. [1] [2] [3]GZipWritableArchiveEntry.csto useNonDisposingStreaminstead ofSharpCompressStream.Createfor entry streams, improving resource handling.SevenZip Archive Diagnostics and Internal Improvements:
SevenZipArchive.SevenZipReader, exposing properties for test diagnostics and clarifying folder stream handling for solid archives. Also improved documentation and code comments for maintainability. [1] [2] [3]API Simplification:
IsArchivemethods inArchiveFactory.csby removing thebufferSizeparameter and streamlining overloads.These changes collectively improve code clarity, maintainability, and diagnostics, while also cleaning up obsolete configuration and documentation files.