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

[API Proposal]: Add TarExtractOptions/TarCreateOptions #69780

Open
tmds opened this issue May 25, 2022 · 6 comments
Open

[API Proposal]: Add TarExtractOptions/TarCreateOptions #69780

tmds opened this issue May 25, 2022 · 6 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Formats.Tar
Milestone

Comments

@tmds
Copy link
Member

tmds commented May 25, 2022

Background and motivation

The Tar API that is introduced in #65951 currently provides one option while creating: includeBaseDirectory, and one option while extracting: overwriteFiles.

There are many more options that would make sense, for example controlling the owner/group that gets stored in the tar-file when creating, and controlling whether the exact permissions should be restored (including the setgroup/setuser/sticky bit). See man tar for other options that a user may want to control.

Maybe we want to introduce option classes immediately, to avoid adding additional overloads to the Create/Extract methods.

API Proposal

namespace System.Formats.Tar;

public static partial class TarFile
{
-    public static void CreateFromDirectory(string sourceDirectoryName, Stream destination, bool includeBaseDirectory);
-    public static void CreateFromDirectory(string sourceDirectoryName, string destinationFileName, bool includeBaseDirectory);
-    public static void ExtractToDirectory(Stream source, string destinationDirectoryName, bool overwriteFiles);
-    public static void ExtractToDirectory(string sourceFileName, string destinationDirectoryName, bool overwriteFiles);
+    public static void CreateFromDirectory(string sourceDirectoryName, Stream destination, TarCreateOptions? options = null);
+    public static void CreateFromDirectory(string sourceDirectoryName, string destinationFileName, TarCreateOptions? options = null);
+    public static void ExtractToDirectory(Stream source, string destinationDirectoryName, TarExtractOptions? options = null);
+    public static void ExtractToDirectory(string sourceFileName, string destinationDirectoryName, TarExtractOptions? options = null);
}
public abstract partial class TarEntry
{
-    public void ExtractToFile(string destinationFileName, bool overwrite);    
+    public void ExtractToFile(string destinationFileName, TarExtractOptions? options = null);
}

+public sealed class TarExtractOptions
+{
+    public bool OverwriteFiles { get; set; } = true;
+}
+public sealed class TarCreateOptions
+{
+    public bool IncludeBaseDirectory { get; set; } = true;
+}

API Usage

TarFile.CreateFromDirectory("D:/SourceDirectory/", "D:/destination.tar", new() { IncludeBaseDirectory = false });

Alternative Designs

No response

Risks

No response

@tmds tmds added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label May 25, 2022
@ghost ghost added the untriaged New issue has not been triaged by the area owner label May 25, 2022
@ghost
Copy link

ghost commented May 25, 2022

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

The Tar API that is introduced in #65951 currently provides one option while creating: includeBaseDirectory, and one option while extracting: overwriteFiles.

There are many more options that would make sense, for example controlling the owner/group that gets stored in the tar-file when creating, and controlling whether the exact permissions should be restored (including the setgroup/setuser/sticky bit). See man tar for other options that a user may want to control.

Maybe we want to introduce option classes immediately, to avoid adding additional overloads to the Create/Extract methods.

API Proposal

public static partial class TarFile
{
-    public static void CreateFromDirectory(string sourceDirectoryName, System.IO.Stream destination, bool includeBaseDirectory);
-    public static void CreateFromDirectory(string sourceDirectoryName, string destinationFileName, bool includeBaseDirectory);
-    public static void ExtractToDirectory(System.IO.Stream source, string destinationDirectoryName, bool overwriteFiles);
-    public static void ExtractToDirectory(string sourceFileName, string destinationDirectoryName, bool overwriteFiles);
+    public static void CreateFromDirectory(string sourceDirectoryName, System.IO.Stream destination, TarCreateOptions? options = null);
+    public static void CreateFromDirectory(string sourceDirectoryName, string destinationFileName, TarCreateOptions? options = null);
+    public static void ExtractToDirectory(System.IO.Stream source, string destinationDirectoryName, TarExtractOptions? options = null);
+    public static void ExtractToDirectory(string sourceFileName, string destinationDirectoryName, TarExtractOptions? options = null);
}
public abstract partial class TarEntry
{
-    public void ExtractToFile(string destinationFileName, bool overwrite);    
+    public void ExtractToFile(string destinationFileName, TarExtractOptions? options = null);
}

+public sealed class TarExtractOptions
+{
+    public bool OverwriteFiles { get; set; } = true;
+}
+public sealed class TarCreateOptions
+{
+    public bool IncludeBaseDirectory { get; set; } = true;
+}

API Usage

TarFile.CreateFromDirectory("D:/SourceDirectory/", "D:/destination.tar", new() { IncludeBaseDirectory = false });

Alternative Designs

No response

Risks

No response

Author: tmds
Assignees: -
Labels:

api-suggestion, area-System.IO, untriaged

Milestone: -

@tmds
Copy link
Member Author

tmds commented May 25, 2022

cc @carlossanlop

@GSPP
Copy link

GSPP commented May 25, 2022

This issue has affected quite a few other .NET classes. It starts with one innocuous bool, and over time expands into a jungle of overloads. It seems wise to immediately use the options approach, and only that.

An alternative is to create a flags enum. Of course, that can only control boolean options.

@tmds
Copy link
Member Author

tmds commented May 30, 2022

@adamsitnik
Copy link
Member

@carlossanlop PTAL

@carlossanlop
Copy link
Member

carlossanlop commented Jul 25, 2022

We did discuss these options classes in the early design stages:

+public sealed class TarExtractOptions
+{
+    public bool OverwriteFiles { get; set; } = true;
+}
+public sealed class TarCreateOptions
+{
+    public bool IncludeBaseDirectory { get; set; } = true;
+}

But since we only had one setting to add, we didn't think they were needed. So the current static methods are sufficient with their current signature. I would definitely welcome creating the two proposed classes as soon as we have more than one setting to include.

There are many more options that would make sense, for example controlling the owner/group that gets stored in the tar-file when creating, and controlling whether the exact permissions should be restored (including the setgroup/setuser/sticky bit)

I'd be interested in exploring these items you mention, making sure they are proposed in a cross-platform way, because I can see some of these settings not being Windows friendly.

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.Formats.Tar
Projects
None yet
Development

No branches or pull requests

5 participants