[API Proposal]: Merge TarEntry's DeviceMajor and DeviceMinor into a single API

[carlossanlop]

Background and motivation

In the initial PR implementing the new System.Formats.Tar APIs, @stephentoub suggested we consider merging the DeviceMajor and DeviceMajor public properties into one.

The device major and device minor values are used in Unix to uniquely identify character device and block device files.

The suggestion to merge them into a single API makes sense because:

• We would rarely need to consume the properties individually. If we use one, we will most likely use the other one too.
• The P/Invoke already retrieves major and minor into a single one and then sets both major and minor at the same time in the TarEntry header.
• When extracting a character or block device into a Unix system, we read both values together to pass them to the P/Invoke that creates the file.

API Proposal

The major and minor numbers are available in Ustar, Pax and Gnu.

public abstract partial class PosixTarEntry : TarEntry
{
        internal PosixTarEntry() { }
-       public int DeviceMajor { get; set; }
-       public int DeviceMinor { get; set; }
        public string GroupName { get; set; }
        public string UserName { get; set; }
+      public (int, int) GetDeviceIdentifiers();
+      public void SetDeviceIdentifiers(int deviceMajor, int deviceMinor);
}

API Usage

Instead of:

// Get
int major = entry.DeviceMajor;
int minor = entry.DeviceMinor;

// Set
entry.DeviceMajor = newMajor;
entry.DeviceMinor = newMinor;

We would have:

// Get
(int major, int minor) = entry.GetDeviceIdentifiers();
// Set
entry.SetDeviceIdentifiers(newMajor, newMinor);

Or with the alternative design:

// Get
entry.GetDeviceIdentifiers(out int major, out int minor);

Alternative Designs

a) Instead of returning a tuple, get the values as out:

public abstract partial class PosixTarEntry : TarEntry
{
+      public void GetDeviceIdentifiers(out int deviceMajor, out int deviceMinor);
}

b) Keep the APIs as they are, letting the user consume the major and minor separately, even if they are internally retrieved or set together.

Risks

Creating character or block devices is rare, and they are file types that are only available in some Unix filesystems. I am not aware of any particular use cases where the user would only need to retrieve the device major individually or the device minor individually. @tmds can you think of any?

These are all new APIs in .NET 7.0, so we have time to decide better designs where applicable.

@bartonjs @adamsitnik @jozkee @jeffhandley let me know what you think.

[ghost]

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

In the initial PR implementing the new System.Formats.Tar APIs, @stephentoub suggested we consider merging the DeviceMajor and DeviceMajor public properties into one.

The device major and device minor values are used in Unix to uniquely identify character device and block device files.

The suggestion to merge them into a single API makes sense because:

• We would rarely need to consume the properties individually. If we use one, we will most likely use the other one too.
• The P/Invoke already retrieves major and minor into a single one and then sets both major and minor at the same time in the TarEntry header.
• When extracting a character or block device into a Unix system, we read both values together to pass them to the P/Invoke that creates the file.

API Proposal

The major and minor numbers are available in Ustar, Pax and Gnu.

public abstract partial class PosixTarEntry : TarEntry
{
        internal PosixTarEntry() { }
-       public int DeviceMajor { get; set; }
-       public int DeviceMinor { get; set; }
        public string GroupName { get; set; }
        public string UserName { get; set; }
+      public (int, int) GetDeviceIdentifiers();
+      public void SetDeviceIdentifiers(int deviceMajor, int deviceMinor);
}

API Usage

Instead of:

// Get
int major = entry.DeviceMajor;
int minor = entry.DeviceMinor;

// Set
entry.DeviceMajor = newMajor;
entry.DeviceMinor = newMinor;

We would have:

// Get
(int major, int minor) = entry.GetDeviceIdentifiers();
// Set
entry.SetDeviceIdentifiers(newMajor, newMinor);

Or with the alternative design:

// Get
entry.GetDeviceIdentifiers(out int major, out int minor);

Alternative Designs

a) Instead of returning a tuple, get the values as out:

public abstract partial class PosixTarEntry : TarEntry
{
+      public void GetDeviceIdentifiers(out int deviceMajor, out int deviceMinor);
}

b) Keep the APIs as they are, letting the user consume the major and minor separately, even if they are internally retrieved or set together.

Risks

Creating character or block devices is rare, and they are file types that are only available in some Unix filesystems. I am not aware of any particular use cases where the user would only need to retrieve the device major individually or the device minor individually. @tmds can you think of any?

These are all new APIs in .NET 7.0, so we have time to decide better designs where applicable.

@bartonjs @adamsitnik @jozkee let me know what you think.

Author:	carlossanlop
Assignees:	carlossanlop
Labels:	api-suggestion, area-System.IO
Milestone:	7.0.0

[bartonjs]

I don't think Steve's suggestion was to merge these in the public API, just to merge the two SystemNative functions into one.

For the API I quite strongly believe they should just be the parallel properties like they already are.

[stephentoub]

I don't think Steve's suggestion was to merge these in the public API, just to merge the two SystemNative functions into one.

I believe I:

• suggested we should merge the SystemNative functions so as to minimize the overhead of the native calls
• asked whether it would make sense to also merge the public properties.

For the latter, what is the use case for these? How are they consumed? Will developers need to compare versions, and if so, we expect them to do so with major and minor independently?

[carlossanlop]

How are they consumed? Will developers need to compare versions, and if so, we expect them to do so with major and minor independently?

In all my use cases while working on tar and device files, they are consumed together. I have not found a case where they need to be consumed separately. I'd love to hear @tmds opinion, maybe there is a case where they are needed separately.

[carlossanlop]

suggested we should merge the SystemNative functions so as to minimize the overhead of the native calls

BTW I already did that :)

runtime/src/libraries/Common/src/Interop/Unix/System.Native/Interop.DeviceFiles.cs

Lines 25 to 26 in 2389815

	[LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetDeviceIdentifiers", SetLastError = true)]
	internal static unsafe partial int GetDeviceIdentifiers(ulong dev, uint* majorNumber, uint* minorNumber);

runtime/src/native/libs/System.Native/pal_io.c

Lines 773 to 779 in 2389815

	int32_t SystemNative_GetDeviceIdentifiers(uint64_t dev, uint32_t* majorNumber, uint32_t* minorNumber)
	{
	dev_t castedDev = (dev_t)dev;
	*majorNumber = (uint32_t)major(castedDev);
	*minorNumber = (uint32_t)minor(castedDev);
	return ConvertErrorPlatformToPal(errno);
	}

[bartonjs]

asked whether it would make sense to also merge the public properties.

Ah, I missed that part. My bad.

what is the use case for these?

99.99999%: No use case, just exposing data from the file format. The epsilon target is someone writing their own file extractor and they want to re-create a block or character device node. In that case, whatever API they want to call will be one of a) two separate calls, b) one call with SxS parameters, or c) one call with ValueTuple'd parameters.

For (a) and (b), SxS here are better, since we're not packing them up then unpacking them again. For (c) it doesn't really matter, since it's easy enough to pack the two properties into a ValueTuple on their call. And, given that dev_t is a non-portable type, they're probably coming in SxS parameters.

As a ValueTuple:

• Harder to use in expected use cases (exempting the "didn't use it" case)
• Requires us to name the major/minor tuple ("GetDeviceIdentifiers")
• Doesn't match what they are in the spec

As SxS properties:

• Easier to use
• Avoids naming an artificial construct
• Matches the spec

In my mind, clearly, the ValueTuple-ification is all cost, no gain.

[carlossanlop]

Thanks for the input, I agree that having the separate properties DeviceMajor and DeviceMinor makes it easy for the user to understand, and having a setter for both and a getter for both just adds complication, even if they are almost always retrieved or set together.

@stephentoub I'm inclined to close this proposal based on the above. Do you have any objections?

[stephentoub]

@stephentoub I'm inclined to close this proposal based on the above. Do you have any objections?

From Jeremy's description it sounds like we don't actually expect anyone to use these, and they're only here in the name of fidelity to the underlying data. In which case usability doesn't matter and it's fine to keep them separate.