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: CreateEntriesFromDirectory #1546

Open
patricksadowski opened this issue Nov 1, 2016 · 11 comments
Open

System.IO.Compression.ZipFile: CreateEntriesFromDirectory #1546

patricksadowski opened this issue Nov 1, 2016 · 11 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.IO.Compression
Milestone

Comments

@patricksadowski
Copy link

Problem

When creating or updating zip archives there is no option to add a directory with its content. The content of a directory can only be added with custom code by hand. ZipFile.CreateFromDirectory is not an option when building complex zip archives.

Proposed API

namespace System.IO.Compression
{
    public static partial class ZipFile
    {
        public static void CreateFromDirectory(string sourceDirectoryName, string destinationArchiveFileName) { }
        public static void CreateFromDirectory(string sourceDirectoryName, string destinationArchiveFileName, System.IO.Compression.CompressionLevel compressionLevel, bool includeBaseDirectory) { }
        public static void CreateFromDirectory(string sourceDirectoryName, string destinationArchiveFileName, System.IO.Compression.CompressionLevel compressionLevel, bool includeBaseDirectory, System.Text.Encoding entryNameEncoding) { }
        public static void ExtractToDirectory(string sourceArchiveFileName, string destinationDirectoryName) { }
        public static void ExtractToDirectory(string sourceArchiveFileName, string destinationDirectoryName, System.Text.Encoding entryNameEncoding) { }
        public static System.IO.Compression.ZipArchive Open(string archiveFileName, System.IO.Compression.ZipArchiveMode mode) { throw null; }
        public static System.IO.Compression.ZipArchive Open(string archiveFileName, System.IO.Compression.ZipArchiveMode mode, System.Text.Encoding entryNameEncoding) { throw null; }
        public static System.IO.Compression.ZipArchive OpenRead(string archiveFileName) { throw null; }
    }
    [System.ComponentModel.EditorBrowsableAttribute((System.ComponentModel.EditorBrowsableState)(1))]
    public static partial class ZipFileExtensions
    {
        public static System.IO.Compression.ZipArchiveEntry CreateEntryFromFile(this System.IO.Compression.ZipArchive destination, string sourceFileName, string entryName) { throw null; }
        public static System.IO.Compression.ZipArchiveEntry CreateEntryFromFile(this System.IO.Compression.ZipArchive destination, string sourceFileName, string entryName, System.IO.Compression.CompressionLevel compressionLevel) { throw null; }
+       public static System.Collections.Generic.IEnumerable<System.IO.Compression.ZipArchiveEntry> CreateEntriesFromDirectory(this System.IO.Compression.ZipArchive destination, string sourceDirectoryName, string baseEntryName) { throw null; }
+       public static System.Collections.Generic.IEnumerable<System.IO.Compression.ZipArchiveEntry> CreateEntriesFromDirectory(this System.IO.Compression.ZipArchive destination, string sourceDirectoryName, string baseEntryName, System.IO.Compression.CompressionLevel compressionLevel, bool includeBaseDirectory) { throw null; }
        public static void ExtractToDirectory(this System.IO.Compression.ZipArchive source, string destinationDirectoryName) { }
        public static void ExtractToFile(this System.IO.Compression.ZipArchiveEntry source, string destinationFileName) { }
        public static void ExtractToFile(this System.IO.Compression.ZipArchiveEntry source, string destinationFileName, bool overwrite) { }
    }
}
@ianhays
Copy link
Contributor

ianhays commented Nov 21, 2016

  •   public static System.IO.Compression.ZipArchiveEntry CreateEntryFromDirectory(this System.IO.Compression.ZipArchive destination, string sourceDirectoryName, string entryName) { throw null; }
    

If you want a directory with all of the items within that directory, you would need to return a list of ZipArchiveEntries. Also, entryName doesn't make much sense when you're adding multiple files unless you want that to be the base path for entries within the directory.

I could see the usefulness of this. The implementation would be easily merge-able with the existing CreateFromDirectory source for a minimal required amount of additional code.

Though there is a simple solution when all files within the directory are at the root e.g.

public static IEnumerable<ZipArchiveEntry> CreateEntriesFromDirectory(this ZipArchive destination, string sourceDirectoryName, CompressionLevel compressionLevel)
{
    foreach (string entry in Directory.EnumerateFileSystemEntries(sourceDirectoryName, "*", SearchOption.TopDirectoryOnly))
        yield return destination.CreateEntryFromFile(entry, Path.GetFileName(entry), compressionLevel);
}

it gets more complex when subfolders are involved as we have to calculate the relative paths for the entry names. Luckily, we already have a function for that.

@patricksadowski
Copy link
Author

Yes, the return type could be changed to IEnumerable<ZipArchive>. entryName should be the base path for entries within the directory. I altered my proposal to make the functionality more clear. The API is now based on ZipFile.CreateFromDirectory. I'm not sure if we should add a parameter Encoding entryNameEncoding.

Given a directory structure on local storage

foo/
 |- bar/
     |-info.txt
 |- empty/

I can call the first overload with entryName: "update" to create

/update/bar/info.txt
/update/empty/

The second overload would produce with entryName: "update" and base directory inclusion

/update/foo/bar/info.txt
/update/foo/empty/

@ianhays
Copy link
Contributor

ianhays commented Nov 22, 2016

entryName should be the base path for entries within the directory.

I think that makes sense, though entryName isn't a very fitting title anymore. Maybe something like baseEntryPath or baseEntryName?

I'm not sure if we should add a parameter Encoding entryNameEncoding.

My vote would be to leave it out since we don't include it in any of the other extension methods.

The second overload would produce with entryName: "update" and base directory inclusion

The includeBaseDirectory parameter doesn't seem necessary since you could easily add the base directory name to "entryName"/baseEntryName/whatever.

@patricksadowski
Copy link
Author

Maybe something like baseEntryPath or baseEntryName?

I changed it to baseEntryName. baseEntryPath doesn't seem to be consistent with other ZipFile and ZipArchive methods.

The includeBaseDirectory parameter doesn't seem necessary since you could easily add the base directory name to "entryName"/baseEntryName/whatever.

Well, I'd like to know why the parameter exists in method ZipFile.CreateFromDirectory. Your explanation leads to the conclusion to also remove the parameter from ZipFile.CreateFromDirectory. Is there a important historical reason to keep the parameter or should we drop it?

@ianhays
Copy link
Contributor

ianhays commented Nov 23, 2016

I changed it to baseEntryName. baseEntryPath doesn't seem to be consistent with other ZipFile and ZipArchive methods.

good call.

Well, I'd like to know why the parameter exists in method ZipFile.CreateFromDirectory. Your explanation leads to the conclusion to also remove the parameter from ZipFile.CreateFromDirectory. Is there a important historical reason to keep the parameter or should we drop it?

We've pretty well already made our bed with ZipFile.CreateFromDirectory - if we change it now we break compat. I'm on the fence about including it in CreateEntriesFromDirectory. If you feel that we should to be consistent with CreateFromDirectory then I'm fine with that.

@patricksadowski
Copy link
Author

The method should be named CreateEntryFromDirectoryContent to make the includeBaseDirectory parameter obsolete.

It would be nice to have the includeBaseDirectory functionality on board to reduce the amount of custom code for the users. We can add either the includeBaseDirectory parameter or two distinct methods CreateEntryFromDirectory and CreateEntryFromDirectoryContent.

@ianhays
Copy link
Contributor

ianhays commented Dec 5, 2016

It would be nice to have the includeBaseDirectory functionality on board to reduce the amount of custom code for the users.

I'm fine with that. Go ahead and include it and we'll see what the API reviewers think once we get it locked down. No need to add a differently-named method.

@patricksadowski patricksadowski changed the title System.IO.Compression: CreateEntryFromDirectory System.IO.Compression.ZipFile: CreateEntriesFromDirectory May 24, 2017
@patricksadowski
Copy link
Author

Is there still API work needed? I would like to implement the proposed API.

@ianhays
Copy link
Contributor

ianhays commented May 26, 2017

I think the API is good and we're at a reasonable place in the design discussion. The next step before moving forward is to wait for more support. We generally prefer to avoid adding API unless it's something that has wide support from the community or a very strong use-case that provides new functionality. Since this addition so far has neither, it's better to wait until the demand is more clear so that it can be better championed during the API review process.

@carlossanlop
Copy link
Member

Triage:
Sounds reasonable, we would like to continue with API design and review.

@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 area-System.IO.Compression and removed area-System.IO.Compression untriaged New issue has not been triaged by the area owner labels Jan 9, 2020
@carlossanlop carlossanlop modified the milestone: Future Jan 9, 2020
@RenderMichael
Copy link
Contributor

I found myself in need of this API today; is there anything else this proposal needs to be ready for review?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.IO.Compression
Development

No branches or pull requests

5 participants