Always use .NET Zip library by setting read-write permissions for zip entries on non-windows OS

[bjorg]

Problem:
Zip file entries created on non-Windows operating systems, must provide their permissions flags in the ExternalAttributes property.

Description of changes:
This pull-request sets the ExternalAttributes property to give Read/Write permissions to the current user, and Read-only to group and world.

Furthermore, the code used to shell out to the native zip utility has been removed as it is no longer required to create a working zip archive for AWS Lambda.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[normj]

Thanks for the PR. We have a rework of this code in the package-custom-runtimes. The effort in that branch was to switch to a go executable to create the zip file on Windows so we could set a custom runtime bootstrap to be executable on Linux. We hadn't found a way to do that on Windows using the .NET ZIP api because ExternalAttributes is ignored when set on Windows.

Our first priority is to get this branch out which is going to cause conflicts with this merge request.

[bjorg]

The problem with setting the ExternalAttributes properties on Windows, is that you have to set be able to set the host system to Unix, which the built-in .NET library does not allow. I found SharpZipLib that got pretty close, but required a few fixes to get there. You can see the pull-request here: icsharpcode/SharpZipLib#325. In the meantime, I copied the fixed source into my project repo as a git subtree.

Unfortunately, SharpZipLib is ~6 slower. So I only use it when I need to set executable permissions. My use case was that I wanted to be able to create a Lambda Layer with a Linux executable that could be invoked by the Lambda function. And I wanted compilation of the package to work on any host system (including Windows). Getting this to work taught me a lot more about the zip format than I ever cared to learn! However, I believe this might cover your use case as well.

Just for context, here's a working example using ffmpeg to encode videos as GIFs that can be created on Windows and runs as expected.

[bjorg]

It occurred to me, you may actual care more about how I used it than seeing it in action! :)

Here's the beginning of the code: https://github.com/LambdaSharp/LambdaSharpTool/blob/master/src/LambdaSharp.Tool/Cli/Build/ModelFilesPackager.cs#L145

I chose the zipping library based on the need to have executable permissions or not. Then, when zipping with SharpZipLib , I create the entry with a specific host OS (Unix) on line 155 and then set the appropriate flags on line 158.

Hope that helps!

[bjorg]

@normj So you bundled an executable, because you didn't want to set the proper flags using the native zip library? That seems just a tad overkill, no? Or am I missing something?

Unless I'm confused, the following lines are literally all you were missing to make the native library work on all operating systems and not have to shell out:

var entry = zipArchive.CreateEntryFromFile(kvp.Value, kvp.Key);
if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
    // Set RW-R--R-- permission attributes on non-Windows operating system
    entry.ExternalAttributes = 0b1_000_000_110_100_100 << 16;
}

[normj]

The problem we were trying to solve was on Windows we do need to set the linux file permissions but the native zip library doesn't allow it. If you set entry.ExternalAttributes on Windows it just gets ignored.

The bundled executable is only used on Windows and currently we still rely on the zip executable. I was keeping your PR open so when I had time I could use it to get ride of the zip dependency on non-windows but use the bundled program when on windows.

[bjorg]

IIRC, you don't need the executable bit for the assemblies. I'm pretty sure I ran a lambda deployment with the proposed change to confirm it worked. However, you do need the executable bit if you ship an actual executable that you want to shell out to--or if you supply a custom runtime, I assume.

[normj]

It was the recent release of custom runtime support in the tooling that prompted this change since we need to make the bootstrap executable.