Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 14 additions & 20 deletions src/Platform/Microsoft.Testing.Platform/IPC/NamedPipeServer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ internal sealed class NamedPipeServer : NamedPipeBase, IServer
;
#pragma warning restore CA1416 // Validate platform compatibility

private static bool IsUnix => Path.DirectorySeparatorChar == '/';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why have we moved to a simple separator check, rather than RuntimeInformation.IsOSPlatform?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I preferred to follow what Roslyn does which we know works very well for them.
But it's probably equivalent.

DirectorySeparatorChar on dotnet/runtime is hard coded on Windows to \ and on Unix to /.


private readonly Func<IRequest, Task<IResponse>> _callback;
private readonly IEnvironment _environment;
private readonly NamedPipeServerStream _namedPipeServerStream;
Expand Down Expand Up @@ -277,32 +279,24 @@ private async Task InternalLoopAsync(CancellationToken cancellationToken)
// If core MTP is updated, but old version of TRX is still used, it will try to call this overload at runtime.
// Without it, MissingMethodException will be thrown at runtime.
public static PipeNameDescription GetPipeName(string name)
=> GetPipeName(name, new SystemEnvironment());

public static PipeNameDescription GetPipeName(string name, IEnvironment environment)
{
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
if (!IsUnix)
{
return new PipeNameDescription($"testingplatform.pipe.{name.Replace('\\', '.')}", false);
return new PipeNameDescription($"testingplatform.pipe.{name.Replace('\\', '.')}");
}

#pragma warning disable RS0030 // Do not use banned APIs - We are using IEnvironment, but we still need the enum from the Environment class in BCL. This is safe.
string directoryId = Path.Combine(environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData, Environment.SpecialFolderOption.None), name);
#pragma warning disable RS0030 // Do not use banned APIs
Directory.CreateDirectory(directoryId);
return new PipeNameDescription(
!Directory.Exists(directoryId)
? throw new DirectoryNotFoundException(string.Format(
CultureInfo.InvariantCulture,
#if PLATFORM_MSBUILD
$"Directory: {directoryId} doesn't exist.",
#else
PlatformResources.CouldNotFindDirectoryErrorMessage,
#endif
directoryId))
: Path.Combine(directoryId, ".p"), true);
// Similar to https://github.com/dotnet/roslyn/blob/99bf83c7bc52fa1ff27cf792db38755d5767c004/src/Compilers/Shared/NamedPipeUtil.cs#L26-L42
return new PipeNameDescription(Path.Combine("/tmp", name));
}

// For compatibility only.
// Old versions of MTP used to have this overload without IEnvironment.
// Extensions (e.g, TRX) calls into this overload.
// If core MTP is updated, but old version of TRX is still used, it will try to call this overload at runtime.
// Without it, MissingMethodException will be thrown at runtime.
public static PipeNameDescription GetPipeName(string name, IEnvironment _)
=> GetPipeName(name);

public void Dispose()
{
if (_disposed)
Expand Down
25 changes: 3 additions & 22 deletions src/Platform/Microsoft.Testing.Platform/IPC/PipeNameDescription.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,32 +3,13 @@

namespace Microsoft.Testing.Platform.IPC;

internal sealed class PipeNameDescription(string name, bool isDirectory) : IDisposable
internal sealed class PipeNameDescription(string name) : IDisposable
{
private readonly bool _isDirectory = isDirectory;
private bool _disposed;

public string Name { get; } = name;

// This is available via IVT.
// Avoid removing it as it can be seen as a binary breaking change when users use newer version of core MTP but older version of one of the extensions.
public void Dispose()
{
if (_disposed)
{
return;
}

if (_isDirectory)
{
try
{
Directory.Delete(Path.GetDirectoryName(Name)!, true);
}
catch (IOException)
{
// This folder is created inside the temp directory and will be cleaned up eventually by the OS
}
}

_disposed = true;
Comment thread
Youssef1313 marked this conversation as resolved.
}
}