Skip to content

Commit 5408323

Browse files
Address some System.Formats.Tar TODOs (infra and syscalls) (#69107)
* Fix Design time build errors by removing src project as dependency of the test project. * Add Browser to target platform identifiers. Ensure Browser can consume the Unix specific ArchivingUtils file too. * Remove nullable enable from csproj since it's now default * Use FileStream constructor with FileMode.CreateNew to detect and throw if destination file exists when creating archive. * No error checking on syscalls that do not set errno. * Add RDev field in FileStatus and retrieve it with stat/lstat so devmajor and devminor get their correct values. * Simplify some File.Open calls with direct FileStream constructor calls. Simplify FileStreamOptions objects too. * Implement and consume p/invokes to retrieve uname from uid and gname from gid. * size_t variables should not be checked for negative values * FileStream calls simplified to File.OpenRead * Remove check for RDev > 0 * Use dictionary to preserve repeated unames and gnames mapped to uids and gids * Missing documentation in pal_uid.h new PALEXPORT methods. * Adjust syscalls to thread-safe ones. Start with stackalloc, then use loop. * Make dicts readonly and non-nullable, use TryGetValue * Reuse 'GetNameFromUid' from pal_networking.c, move to pal_uid.c, use similar logic for Gid method. Simplify Interop.Sys method. * Remove unnecessary comment Co-authored-by: Adam Sitnik <[email protected]> * Put TargetFrameworks back in the first position of the PropertyGroup. * Address eerhardt suggestions * Update src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntry.File.Tests.Unix.cs * Clarify in pal_uid.h methods comments that new memory is returned Co-authored-by: carlossanlop <[email protected]> Co-authored-by: Adam Sitnik <[email protected]>
1 parent ade698f commit 5408323

File tree

18 files changed

+184
-120
lines changed

18 files changed

+184
-120
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,6 @@ internal static int CreateCharacterDevice(string pathName, uint mode, uint major
2323
private static partial int MkNod(string pathName, uint mode, uint major, uint minor);
2424

2525
[LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetDeviceIdentifiers", SetLastError = true)]
26-
internal static unsafe partial int GetDeviceIdentifiers(ulong dev, uint* majorNumber, uint* minorNumber);
26+
internal static unsafe partial void GetDeviceIdentifiers(ulong dev, uint* majorNumber, uint* minorNumber);
2727
}
2828
}
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System.Runtime.InteropServices;
5+
using System.Buffers;
6+
using System.Text;
7+
using System;
8+
using System.Collections.Generic;
9+
using System.Reflection;
10+
11+
internal static partial class Interop
12+
{
13+
internal static partial class Sys
14+
{
15+
/// <summary>
16+
/// Gets the group name associated to the specified group ID.
17+
/// </summary>
18+
/// <param name="gid">The group ID.</param>
19+
/// <returns>On success, return a string with the group name. On failure, throws an IOException.</returns>
20+
internal static string GetGroupName(uint gid) => GetGroupNameInternal(gid) ?? throw GetIOException(GetLastErrorInfo());
21+
22+
/// <summary>
23+
/// Gets the user name associated to the specified user ID.
24+
/// </summary>
25+
/// <param name="uid">The user ID.</param>
26+
/// <returns>On success, return a string with the user name. On failure, throws an IOException.</returns>
27+
internal static string GetUserName(uint uid) => GetUserNameInternal(uid) ?? throw GetIOException(GetLastErrorInfo());
28+
29+
[LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetGroupName", StringMarshalling = StringMarshalling.Utf8, SetLastError = true)]
30+
private static unsafe partial string? GetGroupNameInternal(uint uid);
31+
32+
[LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetUserName", StringMarshalling = StringMarshalling.Utf8, SetLastError = true)]
33+
private static unsafe partial string? GetUserNameInternal(uint uid);
34+
}
35+
}

src/libraries/Common/src/Interop/Unix/System.Native/Interop.Stat.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ internal struct FileStatus
3131
internal long BirthTime;
3232
internal long BirthTimeNsec;
3333
internal long Dev;
34+
internal long RDev;
3435
internal long Ino;
3536
internal uint UserFlags;
3637
}

src/libraries/System.Formats.Tar/src/System.Formats.Tar.csproj

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
<Project Sdk="Microsoft.NET.Sdk">
22
<PropertyGroup>
3-
<TargetFrameworks>$(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)-Unix;$(NetCoreAppCurrent)</TargetFrameworks>
3+
<TargetFrameworks>$(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)-Unix;$(NetCoreAppCurrent)-Browser;$(NetCoreAppCurrent)</TargetFrameworks>
44
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
55
</PropertyGroup>
66

@@ -57,10 +57,14 @@
5757
<Compile Include="$(CommonPath)Interop\Unix\Interop.Errors.cs" Link="Common\Interop\Unix\System.Native\Interop.Errors.cs" />
5858
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.DeviceFiles.cs" Link="Common\Interop\Unix\System.Native\Interop.DeviceFiles.cs" />
5959
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.FChMod.cs" Link="Common\Interop\Unix\System.Native\Interop.FChMod.cs" />
60+
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.GroupNameUserName.cs" Link="Common\Interop\Unix\System.Native\Interop.GroupNameUserName.cs" />
6061
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.Link.cs" Link="Common\Interop\Unix\System.Native\Interop.Link.cs" />
6162
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.MkFifo.cs" Link="Common\Interop\Unix\System.Native\Interop.MkFifo.cs" />
6263
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.Permissions.cs" Link="Common\Interop\Unix\Interop.Permissions.cs" />
6364
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.Stat.cs" Link="Common\Interop\Unix\Interop.Stat.cs" />
65+
</ItemGroup>
66+
<!-- Unix and Browser specific files -->
67+
<ItemGroup Condition="'$(TargetPlatformIdentifier)' == 'Unix' or '$(TargetPlatformIdentifier)' == 'Browser'">
6468
<Compile Include="$(CommonPath)System\IO\Archiving.Utils.Unix.cs" Link="Common\System\IO\Archiving.Utils.Unix.cs" />
6569
</ItemGroup>
6670
<ItemGroup>

src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarEntry.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -431,7 +431,7 @@ private void ExtractAsRegularFile(string destinationFileName)
431431
{
432432
Debug.Assert(!Path.Exists(destinationFileName));
433433

434-
FileStreamOptions fileStreamOptions = new FileStreamOptions()
434+
FileStreamOptions fileStreamOptions = new()
435435
{
436436
Access = FileAccess.Write,
437437
Mode = FileMode.CreateNew,

src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarFile.cs

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -72,12 +72,8 @@ public static void CreateFromDirectory(string sourceDirectoryName, string destin
7272
throw new DirectoryNotFoundException(string.Format(SR.IO_PathNotFound_Path, sourceDirectoryName));
7373
}
7474

75-
if (Path.Exists(destinationFileName))
76-
{
77-
throw new IOException(string.Format(SR.IO_FileExists_Name, destinationFileName));
78-
}
79-
80-
using FileStream fs = File.Create(destinationFileName, bufferSize: 0x1000, FileOptions.None);
75+
// Throws if the destination file exists
76+
using FileStream fs = new(destinationFileName, FileMode.CreateNew, FileAccess.Write);
8177

8278
CreateFromDirectoryInternal(sourceDirectoryName, fs, includeBaseDirectory, leaveOpen: false);
8379
}
@@ -170,15 +166,7 @@ public static void ExtractToDirectory(string sourceFileName, string destinationD
170166
throw new DirectoryNotFoundException(string.Format(SR.IO_PathNotFound_Path, destinationDirectoryName));
171167
}
172168

173-
FileStreamOptions fileStreamOptions = new()
174-
{
175-
Access = FileAccess.Read,
176-
BufferSize = 0x1000,
177-
Mode = FileMode.Open,
178-
Share = FileShare.Read
179-
};
180-
181-
using FileStream archive = File.Open(sourceFileName, fileStreamOptions);
169+
using FileStream archive = File.OpenRead(sourceFileName);
182170

183171
ExtractToDirectoryInternal(archive, destinationDirectoryName, overwriteFiles, leaveOpen: false);
184172
}

src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarWriter.Unix.cs

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
using System.Collections.Generic;
45
using System.Diagnostics;
56
using System.IO;
67

@@ -9,6 +10,9 @@ namespace System.Formats.Tar
910
// Unix specific methods for the TarWriter class.
1011
public sealed partial class TarWriter : IDisposable
1112
{
13+
private readonly Dictionary<uint, string> _userIdentifiers = new Dictionary<uint, string>();
14+
private readonly Dictionary<uint, string> _groupIdentifiers = new Dictionary<uint, string>();
15+
1216
// Unix specific implementation of the method that reads an entry from disk and writes it into the archive stream.
1317
partial void ReadFileFromDiskAndWriteToArchiveStreamAsEntry(string fullPath, string entryName)
1418
{
@@ -41,13 +45,13 @@ partial void ReadFileFromDiskAndWriteToArchiveStreamAsEntry(string fullPath, str
4145
_ => throw new FormatException(string.Format(SR.TarInvalidFormat, Format)),
4246
};
4347

44-
if ((entryType is TarEntryType.BlockDevice or TarEntryType.CharacterDevice) && status.Dev > 0)
48+
if (entryType is TarEntryType.BlockDevice or TarEntryType.CharacterDevice)
4549
{
4650
uint major;
4751
uint minor;
4852
unsafe
4953
{
50-
Interop.CheckIo(Interop.Sys.GetDeviceIdentifiers((ulong)status.Dev, &major, &minor));
54+
Interop.Sys.GetDeviceIdentifiers((ulong)status.RDev, &major, &minor);
5155
}
5256

5357
entry._header._devMajor = (int)major;
@@ -60,12 +64,23 @@ partial void ReadFileFromDiskAndWriteToArchiveStreamAsEntry(string fullPath, str
6064

6165
entry._header._mode = (status.Mode & 4095); // First 12 bits
6266

63-
entry.Uid = (int)status.Uid;
64-
entry.Gid = (int)status.Gid;
67+
// Uid and UName
68+
entry._header._uid = (int)status.Uid;
69+
if (!_userIdentifiers.TryGetValue(status.Uid, out string? uName))
70+
{
71+
uName = Interop.Sys.GetUserName(status.Uid);
72+
_userIdentifiers.Add(status.Uid, uName);
73+
}
74+
entry._header._uName = uName;
6575

66-
// TODO: Add these p/invokes https://github.com/dotnet/runtime/issues/68230
67-
entry._header._uName = "";// Interop.Sys.GetUName();
68-
entry._header._gName = "";// Interop.Sys.GetGName();
76+
// Gid and GName
77+
entry._header._gid = (int)status.Gid;
78+
if (!_groupIdentifiers.TryGetValue(status.Gid, out string? gName))
79+
{
80+
gName = Interop.Sys.GetGroupName(status.Gid);
81+
_groupIdentifiers.Add(status.Gid, gName);
82+
}
83+
entry._header._gName = gName;
6984

7085
if (entry.EntryType == TarEntryType.SymbolicLink)
7186
{
@@ -74,16 +89,8 @@ partial void ReadFileFromDiskAndWriteToArchiveStreamAsEntry(string fullPath, str
7489

7590
if (entry.EntryType is TarEntryType.RegularFile or TarEntryType.V7RegularFile)
7691
{
77-
FileStreamOptions options = new()
78-
{
79-
Mode = FileMode.Open,
80-
Access = FileAccess.Read,
81-
Share = FileShare.Read,
82-
Options = FileOptions.None
83-
};
84-
8592
Debug.Assert(entry._header._dataStream == null);
86-
entry._header._dataStream = File.Open(fullPath, options);
93+
entry._header._dataStream = File.OpenRead(fullPath);
8794
}
8895

8996
WriteEntry(entry);

src/libraries/System.Formats.Tar/tests/System.Formats.Tar.Tests.csproj

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
<Compile Include="$(CommonPath)Interop\Unix\Interop.IOErrors.cs" Link="Common\Interop\Unix\Interop.IOErrors.cs" />
5757
<Compile Include="$(CommonPath)Interop\Unix\Interop.Libraries.cs" Link="Common\Interop\Unix\Interop.Libraries.cs" />
5858
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.DeviceFiles.cs" Link="Common\Interop\Unix\System.Native\Interop.DeviceFiles.cs" />
59+
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.GroupNameUserName.cs" Link="Common\Interop\Unix\System.Native\Interop.GroupNameUserName.cs" />
5960
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.Link.cs" Link="Common\Interop\Unix\System.Native\Interop.Link.cs" />
6061
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.MkFifo.cs" Link="Common\Interop\Unix\System.Native\Interop.MkFifo.cs" />
6162
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.Stat.cs" Link="Common\Interop\Unix\System.Native\Interop.Stat.cs" />
@@ -66,7 +67,4 @@
6667
<ItemGroup>
6768
<PackageReference Include="System.Formats.Tar.TestData" Version="$(SystemFormatsTarTestDataVersion)" />
6869
</ItemGroup>
69-
<ItemGroup>
70-
<ProjectReference Include="..\src\System.Formats.Tar.csproj" />
71-
</ItemGroup>
7270
</Project>

src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.File.Tests.cs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -100,10 +100,7 @@ public void Extract_Archive_File_OverwriteFalse()
100100

101101
string filePath = Path.Join(destination.Path, "file.txt");
102102

103-
using (StreamWriter writer = File.CreateText(filePath))
104-
{
105-
writer.WriteLine("My existence should cause an exception");
106-
}
103+
File.Create(filePath).Dispose();
107104

108105
Assert.Throws<IOException>(() => TarFile.ExtractToDirectory(sourceArchiveFileName, destination.Path, overwriteFiles: false));
109106
}

src/libraries/System.Formats.Tar/tests/TarReader/TarReader.File.Tests.cs

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -713,13 +713,8 @@ private void Verify_Archive_BlockDevice(PosixTarEntry blockDevice, IReadOnlyDict
713713
Assert.True(blockDevice.ModificationTime > DateTimeOffset.UnixEpoch);
714714
Assert.Equal(expectedFileName, blockDevice.Name);
715715
Assert.Equal(AssetUid, blockDevice.Uid);
716-
717-
// TODO: Figure out why the numbers don't match https://github.com/dotnet/runtime/issues/68230
718-
// Assert.Equal(AssetBlockDeviceMajor, blockDevice.DeviceMajor);
719-
// Assert.Equal(AssetBlockDeviceMinor, blockDevice.DeviceMinor);
720-
// Remove these two temporary checks when the above is fixed
721-
Assert.True(blockDevice.DeviceMajor > 0);
722-
Assert.True(blockDevice.DeviceMinor > 0);
716+
Assert.Equal(AssetBlockDeviceMajor, blockDevice.DeviceMajor);
717+
Assert.Equal(AssetBlockDeviceMinor, blockDevice.DeviceMinor);
723718
Assert.Equal(AssetGName, blockDevice.GroupName);
724719
Assert.Equal(AssetUName, blockDevice.UserName);
725720

@@ -749,13 +744,8 @@ private void Verify_Archive_CharacterDevice(PosixTarEntry characterDevice, IRead
749744
Assert.True(characterDevice.ModificationTime > DateTimeOffset.UnixEpoch);
750745
Assert.Equal(expectedFileName, characterDevice.Name);
751746
Assert.Equal(AssetUid, characterDevice.Uid);
752-
753-
// TODO: Figure out why the numbers don't match https://github.com/dotnet/runtime/issues/68230
754-
//Assert.Equal(AssetBlockDeviceMajor, characterDevice.DeviceMajor);
755-
//Assert.Equal(AssetBlockDeviceMinor, characterDevice.DeviceMinor);
756-
// Remove these two temporary checks when the above is fixed
757-
Assert.True(characterDevice.DeviceMajor > 0);
758-
Assert.True(characterDevice.DeviceMinor > 0);
747+
Assert.Equal(AssetCharacterDeviceMajor, characterDevice.DeviceMajor);
748+
Assert.Equal(AssetCharacterDeviceMinor, characterDevice.DeviceMinor);
759749
Assert.Equal(AssetGName, characterDevice.GroupName);
760750
Assert.Equal(AssetUName, characterDevice.UserName);
761751

0 commit comments

Comments
 (0)