Skip to content

Commit

Permalink
Revert "Address some System.Formats.Tar TODOs (infra and syscalls) (#…
Browse files Browse the repository at this point in the history
…69107)"

This reverts commit 5408323.
  • Loading branch information
carlossanlop authored May 17, 2022
1 parent 3f0c2cc commit c157f48
Show file tree
Hide file tree
Showing 18 changed files with 120 additions and 184 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,6 @@ internal static int CreateCharacterDevice(string pathName, uint mode, uint major
private static partial int MkNod(string pathName, uint mode, uint major, uint minor);

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

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ internal struct FileStatus
internal long BirthTime;
internal long BirthTimeNsec;
internal long Dev;
internal long RDev;
internal long Ino;
internal uint UserFlags;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>$(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)-Unix;$(NetCoreAppCurrent)-Browser;$(NetCoreAppCurrent)</TargetFrameworks>
<TargetFrameworks>$(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)-Unix;$(NetCoreAppCurrent)</TargetFrameworks>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
</PropertyGroup>

Expand Down Expand Up @@ -57,14 +57,10 @@
<Compile Include="$(CommonPath)Interop\Unix\Interop.Errors.cs" Link="Common\Interop\Unix\System.Native\Interop.Errors.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.DeviceFiles.cs" Link="Common\Interop\Unix\System.Native\Interop.DeviceFiles.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.FChMod.cs" Link="Common\Interop\Unix\System.Native\Interop.FChMod.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.GroupNameUserName.cs" Link="Common\Interop\Unix\System.Native\Interop.GroupNameUserName.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.Link.cs" Link="Common\Interop\Unix\System.Native\Interop.Link.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.MkFifo.cs" Link="Common\Interop\Unix\System.Native\Interop.MkFifo.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.Permissions.cs" Link="Common\Interop\Unix\Interop.Permissions.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.Stat.cs" Link="Common\Interop\Unix\Interop.Stat.cs" />
</ItemGroup>
<!-- Unix and Browser specific files -->
<ItemGroup Condition="'$(TargetPlatformIdentifier)' == 'Unix' or '$(TargetPlatformIdentifier)' == 'Browser'">
<Compile Include="$(CommonPath)System\IO\Archiving.Utils.Unix.cs" Link="Common\System\IO\Archiving.Utils.Unix.cs" />
</ItemGroup>
<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ private void ExtractAsRegularFile(string destinationFileName)
{
Debug.Assert(!Path.Exists(destinationFileName));

FileStreamOptions fileStreamOptions = new()
FileStreamOptions fileStreamOptions = new FileStreamOptions()
{
Access = FileAccess.Write,
Mode = FileMode.CreateNew,
Expand Down
18 changes: 15 additions & 3 deletions src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarFile.cs
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,12 @@ public static void CreateFromDirectory(string sourceDirectoryName, string destin
throw new DirectoryNotFoundException(string.Format(SR.IO_PathNotFound_Path, sourceDirectoryName));
}

// Throws if the destination file exists
using FileStream fs = new(destinationFileName, FileMode.CreateNew, FileAccess.Write);
if (Path.Exists(destinationFileName))
{
throw new IOException(string.Format(SR.IO_FileExists_Name, destinationFileName));
}

using FileStream fs = File.Create(destinationFileName, bufferSize: 0x1000, FileOptions.None);

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

using FileStream archive = File.OpenRead(sourceFileName);
FileStreamOptions fileStreamOptions = new()
{
Access = FileAccess.Read,
BufferSize = 0x1000,
Mode = FileMode.Open,
Share = FileShare.Read
};

using FileStream archive = File.Open(sourceFileName, fileStreamOptions);

ExtractToDirectoryInternal(archive, destinationDirectoryName, overwriteFiles, leaveOpen: false);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;
using System.Diagnostics;
using System.IO;

Expand All @@ -10,9 +9,6 @@ namespace System.Formats.Tar
// Unix specific methods for the TarWriter class.
public sealed partial class TarWriter : IDisposable
{
private readonly Dictionary<uint, string> _userIdentifiers = new Dictionary<uint, string>();
private readonly Dictionary<uint, string> _groupIdentifiers = new Dictionary<uint, string>();

// Unix specific implementation of the method that reads an entry from disk and writes it into the archive stream.
partial void ReadFileFromDiskAndWriteToArchiveStreamAsEntry(string fullPath, string entryName)
{
Expand Down Expand Up @@ -45,13 +41,13 @@ partial void ReadFileFromDiskAndWriteToArchiveStreamAsEntry(string fullPath, str
_ => throw new FormatException(string.Format(SR.TarInvalidFormat, Format)),
};

if (entryType is TarEntryType.BlockDevice or TarEntryType.CharacterDevice)
if ((entryType is TarEntryType.BlockDevice or TarEntryType.CharacterDevice) && status.Dev > 0)
{
uint major;
uint minor;
unsafe
{
Interop.Sys.GetDeviceIdentifiers((ulong)status.RDev, &major, &minor);
Interop.CheckIo(Interop.Sys.GetDeviceIdentifiers((ulong)status.Dev, &major, &minor));
}

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

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

// Uid and UName
entry._header._uid = (int)status.Uid;
if (!_userIdentifiers.TryGetValue(status.Uid, out string? uName))
{
uName = Interop.Sys.GetUserName(status.Uid);
_userIdentifiers.Add(status.Uid, uName);
}
entry._header._uName = uName;
entry.Uid = (int)status.Uid;
entry.Gid = (int)status.Gid;

// Gid and GName
entry._header._gid = (int)status.Gid;
if (!_groupIdentifiers.TryGetValue(status.Gid, out string? gName))
{
gName = Interop.Sys.GetGroupName(status.Gid);
_groupIdentifiers.Add(status.Gid, gName);
}
entry._header._gName = gName;
// TODO: Add these p/invokes https://github.com/dotnet/runtime/issues/68230
entry._header._uName = "";// Interop.Sys.GetUName();
entry._header._gName = "";// Interop.Sys.GetGName();

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

if (entry.EntryType is TarEntryType.RegularFile or TarEntryType.V7RegularFile)
{
FileStreamOptions options = new()
{
Mode = FileMode.Open,
Access = FileAccess.Read,
Share = FileShare.Read,
Options = FileOptions.None
};

Debug.Assert(entry._header._dataStream == null);
entry._header._dataStream = File.OpenRead(fullPath);
entry._header._dataStream = File.Open(fullPath, options);
}

WriteEntry(entry);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@
<Compile Include="$(CommonPath)Interop\Unix\Interop.IOErrors.cs" Link="Common\Interop\Unix\Interop.IOErrors.cs" />
<Compile Include="$(CommonPath)Interop\Unix\Interop.Libraries.cs" Link="Common\Interop\Unix\Interop.Libraries.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.DeviceFiles.cs" Link="Common\Interop\Unix\System.Native\Interop.DeviceFiles.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.GroupNameUserName.cs" Link="Common\Interop\Unix\System.Native\Interop.GroupNameUserName.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.Link.cs" Link="Common\Interop\Unix\System.Native\Interop.Link.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.MkFifo.cs" Link="Common\Interop\Unix\System.Native\Interop.MkFifo.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.Stat.cs" Link="Common\Interop\Unix\System.Native\Interop.Stat.cs" />
Expand All @@ -67,4 +66,7 @@
<ItemGroup>
<PackageReference Include="System.Formats.Tar.TestData" Version="$(SystemFormatsTarTestDataVersion)" />
</ItemGroup>
<ItemGroup>
<ProjectReference Include="..\src\System.Formats.Tar.csproj" />
</ItemGroup>
</Project>
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,10 @@ public void Extract_Archive_File_OverwriteFalse()

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

File.Create(filePath).Dispose();
using (StreamWriter writer = File.CreateText(filePath))
{
writer.WriteLine("My existence should cause an exception");
}

Assert.Throws<IOException>(() => TarFile.ExtractToDirectory(sourceArchiveFileName, destination.Path, overwriteFiles: false));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -713,8 +713,13 @@ private void Verify_Archive_BlockDevice(PosixTarEntry blockDevice, IReadOnlyDict
Assert.True(blockDevice.ModificationTime > DateTimeOffset.UnixEpoch);
Assert.Equal(expectedFileName, blockDevice.Name);
Assert.Equal(AssetUid, blockDevice.Uid);
Assert.Equal(AssetBlockDeviceMajor, blockDevice.DeviceMajor);
Assert.Equal(AssetBlockDeviceMinor, blockDevice.DeviceMinor);

// TODO: Figure out why the numbers don't match https://github.com/dotnet/runtime/issues/68230
// Assert.Equal(AssetBlockDeviceMajor, blockDevice.DeviceMajor);
// Assert.Equal(AssetBlockDeviceMinor, blockDevice.DeviceMinor);
// Remove these two temporary checks when the above is fixed
Assert.True(blockDevice.DeviceMajor > 0);
Assert.True(blockDevice.DeviceMinor > 0);
Assert.Equal(AssetGName, blockDevice.GroupName);
Assert.Equal(AssetUName, blockDevice.UserName);

Expand Down Expand Up @@ -744,8 +749,13 @@ private void Verify_Archive_CharacterDevice(PosixTarEntry characterDevice, IRead
Assert.True(characterDevice.ModificationTime > DateTimeOffset.UnixEpoch);
Assert.Equal(expectedFileName, characterDevice.Name);
Assert.Equal(AssetUid, characterDevice.Uid);
Assert.Equal(AssetCharacterDeviceMajor, characterDevice.DeviceMajor);
Assert.Equal(AssetCharacterDeviceMinor, characterDevice.DeviceMinor);

// TODO: Figure out why the numbers don't match https://github.com/dotnet/runtime/issues/68230
//Assert.Equal(AssetBlockDeviceMajor, characterDevice.DeviceMajor);
//Assert.Equal(AssetBlockDeviceMinor, characterDevice.DeviceMinor);
// Remove these two temporary checks when the above is fixed
Assert.True(characterDevice.DeviceMajor > 0);
Assert.True(characterDevice.DeviceMinor > 0);
Assert.Equal(AssetGName, characterDevice.GroupName);
Assert.Equal(AssetUName, characterDevice.UserName);

Expand Down
9 changes: 8 additions & 1 deletion src/libraries/System.Formats.Tar/tests/TarTestsBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,15 @@ protected static string GetTarFilePath(CompressionMethod compressionMethod, Test
protected static MemoryStream GetTarMemoryStream(CompressionMethod compressionMethod, TestTarFormat format, string testCaseName)
{
string path = GetTarFilePath(compressionMethod, format, testCaseName);
FileStreamOptions options = new()
{
Access = FileAccess.Read,
Mode = FileMode.Open,
Share = FileShare.Read

};
MemoryStream ms = new();
using (FileStream fs = File.OpenRead(path))
using (FileStream fs = new FileStream(path, options))
{
fs.CopyTo(ms);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,10 @@ public void Add_BlockDevice(TarFormat format)
VerifyPlatformSpecificMetadata(blockDevicePath, entry);
Assert.Equal(TestBlockDeviceMajor, entry.DeviceMajor);
Assert.Equal(TestBlockDeviceMinor, entry.DeviceMinor);
// TODO: Fix how these values are collected, the numbers don't match even though https://github.com/dotnet/runtime/issues/68230
// they come from stat's dev and from the major/minor syscalls
// Assert.Equal(TestBlockDeviceMajor, entry.DeviceMajor);
// Assert.Equal(TestBlockDeviceMinor, entry.DeviceMinor);
Assert.Null(reader.GetNextEntry());
}
Expand Down Expand Up @@ -136,8 +138,10 @@ public void Add_CharacterDevice(TarFormat format)
VerifyPlatformSpecificMetadata(characterDevicePath, entry);
Assert.Equal(TestCharacterDeviceMajor, entry.DeviceMajor);
Assert.Equal(TestCharacterDeviceMinor, entry.DeviceMinor);
// TODO: Fix how these values are collected, the numbers don't match even though https://github.com/dotnet/runtime/issues/68230
// they come from stat's dev and from the major/minor syscalls
// Assert.Equal(TestCharacterDeviceMajor, entry.DeviceMajor);
// Assert.Equal(TestCharacterDeviceMinor, entry.DeviceMinor);
Assert.Null(reader.GetNextEntry());
}
Expand All @@ -157,11 +161,8 @@ partial void VerifyPlatformSpecificMetadata(string filePath, TarEntry entry)

if (entry is PosixTarEntry posix)
{
string gname = Interop.Sys.GetGroupName(status.Gid);
string uname = Interop.Sys.GetUserName(status.Uid);

Assert.Equal(gname, posix.GroupName);
Assert.Equal(uname, posix.UserName);
Assert.Equal(DefaultGName, posix.GroupName);
Assert.Equal(DefaultUName, posix.UserName);

if (entry.EntryType is not TarEntryType.BlockDevice and not TarEntryType.CharacterDevice)
{
Expand Down
2 changes: 0 additions & 2 deletions src/native/libs/System.Native/entrypoints.c
Original file line number Diff line number Diff line change
Expand Up @@ -268,8 +268,6 @@ static const Entry s_sysNative[] =
DllImportEntry(SystemNative_GetEnv)
DllImportEntry(SystemNative_GetEnviron)
DllImportEntry(SystemNative_FreeEnviron)
DllImportEntry(SystemNative_GetGroupName)
DllImportEntry(SystemNative_GetUserName)
};

EXTERN_C const void* SystemResolveDllImport(const char* name);
Expand Down
9 changes: 7 additions & 2 deletions src/native/libs/System.Native/pal_io.c
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,6 @@ c_static_assert(PAL_IN_ISDIR == IN_ISDIR);
static void ConvertFileStatus(const struct stat_* src, FileStatus* dst)
{
dst->Dev = (int64_t)src->st_dev;
dst->RDev = (int64_t)src->st_rdev;
dst->Ino = (int64_t)src->st_ino;
dst->Flags = FILESTATUS_FLAGS_NONE;
dst->Mode = (int32_t)src->st_mode;
Expand Down Expand Up @@ -771,17 +770,23 @@ int32_t SystemNative_SymLink(const char* target, const char* linkPath)
return result;
}

void SystemNative_GetDeviceIdentifiers(uint64_t dev, uint32_t* majorNumber, uint32_t* minorNumber)
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);
}

int32_t SystemNative_MkNod(const char* pathName, uint32_t mode, uint32_t major, uint32_t minor)
{
dev_t dev = (dev_t)makedev(major, minor);

if (errno > 0)
{
return -1;
}

int32_t result;
while ((result = mknod(pathName, (mode_t)mode, dev)) < 0 && errno == EINTR);
return result;
Expand Down
4 changes: 2 additions & 2 deletions src/native/libs/System.Native/pal_io.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ typedef struct
int64_t BirthTime; // time the file was created
int64_t BirthTimeNsec; // nanosecond part
int64_t Dev; // ID of the device containing the file
int64_t RDev; // ID of the device if it is a special file
int64_t Ino; // inode number of the file
uint32_t UserFlags; // user defined flags
} FileStatus;
Expand Down Expand Up @@ -544,8 +543,9 @@ PALEXPORT int32_t SystemNative_SymLink(const char* target, const char* linkPath)

/**
* Given a device ID, extracts the major and minor and components and returns them.
* Return 0 on success; otherwise, returns -1 and errno is set.
*/
PALEXPORT void SystemNative_GetDeviceIdentifiers(uint64_t dev, uint32_t* majorNumber, uint32_t* minorNumber);
PALEXPORT int32_t SystemNative_GetDeviceIdentifiers(uint64_t dev, uint32_t* majorNumber, uint32_t* minorNumber);

/**
* Creates a special or ordinary file.
Expand Down
Loading

0 comments on commit c157f48

Please sign in to comment.