Repository navigation
Merge master into main (Zip64 save fix) - #3
Conversation
…ling at the last step
ZipPackage.Save created its ZipOutputStream setting only CompressionLevel, so EnableZip64
kept the constructor default of Zip64Option.Never. That capped every workbook at the classic
ZIP limits: 4 GB of size or offset, and 65,534 entries.
A workbook past either limit threw during the save, after all of its content had been built.
Found when a report against a 25,663-resource estate ran for eleven hours and then lost its
spreadsheet to:
ZipException: Compressed or Uncompressed size, or offset exceeds the maximum value.
Consider setting the UseZip64WhenSaving property on the ZipFile instance.
at ZipEntry.SetZip64Flags() ... at ZipPackage.Save(Stream) ... at ExcelPackage.Save()
AsNecessary rather than Always, so a workbook that fits the classic format is still written
in it and stays readable by older tools. Only a workbook that would otherwise fail switches.
Zip64Tests covers the entry-count limit, which is reachable in a unit test, and pins the
Zip64Option.Never default so those tests are demonstrably about the real defect. The 4 GB
limit shares that one cause. The full suite is unchanged by this: 156 passed and 5 failed
both with and without the fix, those 5 being pre-existing failures from missing Workbooks
test data.
Also restores Directory.Packages.props. The projects use central package management with
bare PackageReference items, but the file had been added and then removed, so nothing
restored (NU1015) and the repository did not build at all. The removed version listed
xunit.v3 and a coverage package, neither of which this repository uses; versions here are
recovered from the last commits that still carried them. The solution now builds with
0 errors, including SampleApp and SampleWebApp.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> # Conflicts: # Directory.Packages.props # PanoramicData.EPPlus/PanoramicData.EPPlus.csproj
Up to standards ✅🟢 Issues
|
There was a problem hiding this comment.
Pull Request Overview
The pull request aims to enable Zip64 support and resolve dependency conflicts; however, the provided diff contains no actual code changes, which contradicts the stated intent. Furthermore, a known build failure (NU5033: PackageLicenseExpression and PackageLicenseFile both set) has been identified in the PR description but remains unaddressed. These gaps prevent verification of the implementation and would likely cause the build to fail in CI.
About this PR
- The build failure NU5033 (conflicting PackageLicenseExpression and PackageLicenseFile) must be resolved. Setting both properties is disallowed by NuGet.
- The PR contains no code changes in the diff, which is inconsistent with the intent to enable Zip64 support and resolve conflicts. Please ensure the merge commit (88c14a8) was correctly pushed and included in this PR.
Test suggestions
- Verify that saving a package with Zip64 enabled completes successfully.
- Verify that existing Encrypt and CompoundDoc tests pass on net10.
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Verify that saving a package with Zip64 enabled completes successfully.
2. Verify that existing Encrypt and CompoundDoc tests pass on net10.
Low confidence findings
- Local test results indicate 13 skipped tests. Please verify if these skips are expected and ensure the Zip64 changes do not negatively impact existing encryption or CompoundDoc functionality.
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
Raise Codacy grade: braces, coverage upload (stacked on #3)
Brings the one commit that
masterhas andmainlacks (88c14a8, Roland Banks: permit Zip64 when saving) intomain. The branches diverged at eac13a7;mainis 21 commits ahead. Merged with--no-ff, so authorship is kept.Conflicts:
Directory.Packages.props: kept main's side (newer versions: Microsoft.Extensions.Configuration.Json 10.0.12, System.Drawing.Common 10.0.12, Nerdbank.GitVersioning 3.10.94, Microsoft.NET.Test.Sdk 18.10.1). master'sSystem.Text.Encoding.CodePagesandSystem.Data.SqlCliententries are not needed on net10 / were replaced by Microsoft.Data.SqlClient on main.PanoramicData.EPPlus.csproj: kept main's side; it already carries the Zip64 release note, and master's other changes were net9 settings that main has deliberately moved past.Verified locally: Zip64, Encrypt, CompoundDoc and ExcelPackage tests pass (5 passed, 13 skipped, 0 failed) on net10. Note
dotnet buildof the library fails on NU5033 (PackageLicenseExpression and PackageLicenseFile both set) with GeneratePackageOnBuild; this is already the case on main and is untouched here (I built tests with-p:GeneratePackageOnBuild=false).🤖 Generated with Claude Code