Skip to content

ConsolidatePackages: Use System.IO.Compression and slightly tweak extraction loop#1913

Open
borland wants to merge 2 commits intomainfrom
orion/system-zip
Open

ConsolidatePackages: Use System.IO.Compression and slightly tweak extraction loop#1913
borland wants to merge 2 commits intomainfrom
orion/system-zip

Conversation

@borland
Copy link
Copy Markdown

@borland borland commented May 5, 2026

Background

https://octopusdeploy.slack.com/archives/C03SG0LFJHX/p1777933912502139

Renovate pulled through an update to the SharpCompress library for Octopus Server. SharpCompress is at version 0.x and reserves the right to make breaking changes at any time, and they've moved a few things around and renamed some methods in the public API surface.

In particular, the ZipArchive.Open method is now ZipArchive.OpenArchive.

When we update Server to the new SharpCompress, Calamari.ConsolidatePackages is also loaded into the Server process, and is still expecting the old method signature. The code throws a Method Not Found exception at runtime.

System.MissingMethodException : Method not found: 'SharpCompress.Archives.Zip.ZipArchive SharpCompress.Archives.Zip.ZipArchive.Open(System.IO.Stream, SharpCompress.Readers.ReaderOptions)'.
   at Octopus.Calamari.ConsolidatedPackage.ConsolidatedPackageIndexLoader.Load(Stream zipStream)
   at Octopus.Calamari.ConsolidatedPackage.ConsolidatedPackageFactory.LoadFrom(IConsolidatedPackageStreamProvider streamProvider)
   at Octopus.Server.ExecutionsV2.Infrastructure.ExecutionProps.BundledConsolidatedPackageProvider.LoadConsolidatedPackage() in ./source/Octopus.Server/ExecutionsV2/Infrastructure/ExecutionProps/BundledConsolidatedPackageFactory.cs:line 40

Analysis

We could simply update the version of SharpCompress used by Calamari.ConsolidatePackages to align with Server, however our default preference is to use the builtin libraries. They are generally much more stable, better supported, and sometimes more performant than third-party libraries -- Unless there is some case-specific reason not to.

Calamari.ConsolidatePackages is just doing ordinary zip-reading things, it isn't complex and there's no niche requirements, we can easily drop SharpCompress here and switch to System.IO.Compression, avoiding any future breaking changes that SharpCompress may have.

Results

  • Removes SharpCompress package reference from Calamari.ConsolidatePackages
  • Swaps in equivalents from the BCL

This change does not require a Server PR.

Reducing Risk

This is the sticky part. The changes are simple, and to the best of my understanding, should be safe, but I don't have any knowledge or ability to test them. Are Calamari's TeamCity tests sufficient for us to be able to merge here? Do we need to do any extra testing? I would appreciate some advice from the owning team please.

{
foreach (var fileTransfer in platformFiles)
{
var sourceEntry = source.Entries.FirstOrDefault(e => e.Key is not null && e.Key.Equals(fileTransfer.Source));
Copy link
Copy Markdown
Author

@borland borland May 5, 2026

Choose a reason for hiding this comment

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

The nested loop means that if there were thousands of PlatformFiles and thousands of Entries, we'd end up doing millions of string-equals checks which can take meaningful amounts of time.

The numbers are probably smaller than that, and it's probably not a big deal, but why not pick the low-hanging fruit while we're in here 🤷

Also: The new code iterates the entries once in a forward-only direction, which is much more friendly to things like disk caches. Iterating in platformfiles order could potentially have us jumping around randomly. In the modern days of SSD's, random jumping around isn't so bad, but sequential reads are still better than random ones

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.

1 participant