Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

System.IO.Compression.ZipFile.CreateFromDirectory: overloads to write to a stream rather than a file #1555

Closed
halgab opened this issue May 24, 2019 · 9 comments · Fixed by #85491
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-System.IO.Compression
Milestone

Comments

@halgab
Copy link
Contributor

halgab commented May 24, 2019

Rationale and usage

The ZipFile.CreateFromDirectory signatures all assume that we want to write the resulting file directly to the file system. But for small archives that are directly uploaded to cloud storage, this is not very efficient.

Symmetrically, ZipFile.ExtractToDirectory overloads that take a zip stream rather than a file would be convenient to unzip cloud archives directly onto a drive.

Proposed API

Zipping

namespace System.IO.Compression;
public static class ZipFile
{
+   public static void CreateFromDirectory(string sourceDirectoryName, Stream destination)
+   public static void CreateFromDirectory(string sourceDirectoryName, Stream destination, CompressionLevel compressionLevel, bool includeBaseDirectory)
+   public static void CreateFromDirectory(string sourceDirectoryName, Stream destination, CompressionLevel compressionLevel, bool includeBaseDirectory, Encoding? entryNameEncoding)
    public static void CreateFromDirectory(string sourceDirectoryName, string destinationArchiveFileName)
    public static void CreateFromDirectory(string sourceDirectoryName, string destinationArchiveFileName, CompressionLevel compressionLevel, bool includeBaseDirectory)
    public static void CreateFromDirectory(string sourceDirectoryName, string destinationArchiveFileName, CompressionLevel compressionLevel, bool includeBaseDirectory, Encoding? entryNameEncoding)
}

Unzipping

namespace System.IO.Compression;
public static class ZipFile
{
+   public static void ExtractToDirectory(Stream source, string destinationDirectoryName) { }
+   public static void ExtractToDirectory(Stream source, string destinationDirectoryName, bool overwriteFiles) { }
+   public static void ExtractToDirectory(Stream source, string destinationDirectoryName, Encoding? entryNameEncoding) { }
+   public static void ExtractToDirectory(Stream source, string destinationDirectoryName, Encoding? entryNameEncoding, bool overwriteFiles) { }
    public static void ExtractToDirectory(string sourceArchiveFileName, string destinationDirectoryName) { }
    public static void ExtractToDirectory(string sourceArchiveFileName, string destinationDirectoryName, bool overwriteFiles) { }
    public static void ExtractToDirectory(string sourceArchiveFileName, string destinationDirectoryName, Encoding? entryNameEncoding) { }
    public static void ExtractToDirectory(string sourceArchiveFileName, string destinationDirectoryName, Encoding? entryNameEncoding, bool overwriteFiles)
}

There is a discussion below to also add overloads to ZipFile.Open that take a stream instead of a string, but there doesn't seem to be much value on adding those because the user can simply use new ZipArchive(Stream, ZipArchiveMode).

These APIs are analogous to the ones we already added in System.Formats.Tar.TarFile.

@halgab halgab changed the title System.IO.Compression.ZipFile: overloads to write to a stream rather than a file System.IO.Compression.ZipFile.CreateFromDirectory: overloads to write to a stream rather than a file May 24, 2019
@wcontayon
Copy link
Contributor

The overload will need to write a new signature on these functions Open and DoCreateFromDirectory of the System.IO.Compression.ZipFile

public static ZipArchive Open(Stream archiveStream, ZipArchiveMode mode, Encoding entryNameEncoding);
private static void DoCreateFromDirectory(string sourceDirectoryName, Stream destinationStram, CompressionLevel? compressionLevel, bool includeBaseDirectory, Encoding entryNameEncoding);

It'll be the same with a little bit of difference for ExtractToDirectory.

@wfurt , I could try a PR for this

@wfurt
Copy link
Member

wfurt commented Jun 10, 2019

@halgab
Copy link
Contributor Author

halgab commented Aug 9, 2019

I've updated the issue to better stick to the guidelines. Please tell me if it's still lacking. Thanks for considering this!

@carlossanlop
Copy link
Member

Thank you @halgab for making the changes.

Please tell me if it's still lacking

We have another page in our documentation that provides better details about the API proposal stage:
dotnet/corefx/blob/master/Documentation/project-docs/api-review-process.md#steps

Step 3 in particular states the following:

we want to make sure that the proposal is actionable, i.e. has a concrete design, a sketch of the APIs and some code samples that show how it should be used. If changes are necessary, the requester is encouraged to edit the issue description. This allows folks joining later to understand the most recent proposal. To avoid confusion, the requester should maintain a tiny change log, like a bolded "Updates:" followed by a bullet point list of the updates that were being made.

So can you please add the code usage examples and the change log? And if you feel like adding more details to the concrete design descriptions (Rationale and usage, Proposed API), feel free to do so. Let us know if you have any questions.

@carlossanlop
Copy link
Member

Triage:
This looks like a legitimate scenario. We should polish the API design.
Potential alternative: add members to ZipArchive to create entries from directories and files.

@carlossanlop
Copy link
Member

@carlossanlop carlossanlop transferred this issue from dotnet/corefx Jan 9, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.IO.Compression untriaged New issue has not been triaged by the area owner labels Jan 9, 2020
@carlossanlop carlossanlop added api-suggestion Early API idea and discussion, it is NOT ready for implementation and removed untriaged New issue has not been triaged by the area owner labels Jan 9, 2020
@carlossanlop carlossanlop added this to the Future milestone Jan 9, 2020
@LeaFrock
Copy link
Contributor

Any progress on this issue? I've met a requirement similar to this. It's a web api which generates several files(with MemoryStream format), then zip them together, and finally response back to client side. Why not add a ZipStream similar to ZLibStream etc..?

@carlossanlop carlossanlop self-assigned this Apr 25, 2023
@carlossanlop
Copy link
Member

carlossanlop commented Apr 25, 2023

I explored the implementation of these APIs and didn't find any hidden or unexpected blockers that could prevent adding them. It's feasible to implement these for .NET 8.

I don't see much value in adding the ZipFile.Open overload because the user can easily just use the ZipArchive(Stream, ZipArchiveMode) constructor instead.

I am going to edit the main proposal to include all the stream-based ZipFile overloads that mirror the string-based ones, then I'm marking it ready for review.

@carlossanlop carlossanlop modified the milestones: Future, 8.0.0 Apr 25, 2023
@carlossanlop carlossanlop added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Apr 25, 2023
@bartonjs
Copy link
Member

bartonjs commented Apr 27, 2023

Video

Looks good as proposed

namespace System.IO.Compression;

public static partial class ZipFile
{
    public static void CreateFromDirectory(string sourceDirectoryName, Stream destination);
    public static void CreateFromDirectory(string sourceDirectoryName, Stream destination, CompressionLevel compressionLevel, bool includeBaseDirectory);
    public static void CreateFromDirectory(string sourceDirectoryName, Stream destination, CompressionLevel compressionLevel, bool includeBaseDirectory, Encoding? entryNameEncoding);

    public static void ExtractToDirectory(Stream source, string destinationDirectoryName) { }
    public static void ExtractToDirectory(Stream source, string destinationDirectoryName, bool overwriteFiles) { }
    public static void ExtractToDirectory(Stream source, string destinationDirectoryName, Encoding? entryNameEncoding) { }
    public static void ExtractToDirectory(Stream source, string destinationDirectoryName, Encoding? entryNameEncoding, bool overwriteFiles) { }
}

@bartonjs bartonjs added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Apr 27, 2023
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Apr 27, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label May 30, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Jun 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.IO.Compression
Projects
None yet
7 participants