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.

Sign up for GitHub

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

Remove more private reflection from compiler #32257

Merged
merged 8 commits into from
Jan 16, 2019
Merged

Remove more private reflection from compiler #32257

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ebd4e3d
Fix multi-target
jaredpar Dec 17, 2018
6620be0
Remove private reflection in the compiler
jaredpar Jan 8, 2019
f0b8d80
Clean up project files
jaredpar Jan 8, 2019
b62e661
More cleanup
jaredpar Jan 8, 2019
1d8334b
Respond to PR feedback
jaredpar Jan 8, 2019
ea900cc
Fix extension handling
jaredpar Jan 9, 2019
bed1068
Respond to PR feedback
jaredpar Jan 15, 2019
ff96f4f
Use DotNet instead of Dotnet
jaredpar Jan 15, 2019
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
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Test/Emit/Attributes/InternalsVisibleToAndStrongNameTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1373,7 +1373,7 @@ private void ConfirmModuleAttributePresentAndAddingToAssemblyResultsInSignedOutp
//confirm file does not claim to be signed
Assert.Equal(0, (int)(flags & CorFlags.StrongNameSigned));

var corlibName = CoreClrShim.IsRunningOnCoreClr ? "netstandard" : "mscorlib";
var corlibName = RuntimeUtilities.IsCoreClrRuntime ? "netstandard" : "mscorlib";
EntityHandle token = metadata.Module.GetTypeRef(metadata.Module.GetAssemblyRef(corlibName), "System.Runtime.CompilerServices", "AssemblyAttributesGoHere");
Assert.False(token.IsNil); //could the type ref be located? If not then the attribute's not there.
var attrInfos = metadata.Module.FindTargetAttributes(token, expectedModuleAttr);
Expand Down
29 changes: 8 additions & 21 deletions src/Compilers/CSharp/csc/csc.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,28 +27,15 @@
<PackageReference Include="Microsoft.DiaSymReader.Native" Version="$(MicrosoftDiaSymReaderNativeVersion)"/>
</ItemGroup>
<ItemGroup>
<Compile Include="..\..\Shared\RuntimeHostInfo.cs" />
<Compile Include="..\..\Shared\NamedPipeUtil.cs" />
<Compile Include="..\..\Shared\BuildClient.cs">
<Link>BuildClient.cs</Link>
Copy link
Contributor

Choose a reason for hiding this comment

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

This is documented in https://docs.microsoft.com/en-us/visualstudio/msbuild/common-msbuild-project-items?view=vs-2017 as follows:
Link | Optional string. The notational path to be displayed when the file is physically located outside the influence of the project file.

Does this mean this only affects the build logs, not the behavior?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm unsure if it affects the build logs or not but it does affect the UI in Visual Studio. You can use this field to change how the file is displayed, and in what folder, in the VS solution explore.

</Compile>
<Compile Include="..\..\Shared\BuildServerConnection.cs">
<Link>BuildServerConnection.cs</Link>
</Compile>
<Compile Include="..\..\Shared\CoreClrAnalyzerAssemblyLoader.cs">
<Link>CoreClrAnalyzerAssemblyLoader.cs</Link>
</Compile>
<Compile Include="..\..\Shared\DesktopBuildClient.cs">
<Link>DesktopBuildClient.cs</Link>
</Compile>
<Compile Include="..\..\Shared\DesktopAnalyzerAssemblyLoader.cs">
<Link>DesktopAnalyzerAssemblyLoader.cs</Link>
</Compile>
<Compile Include="..\..\Shared\ExitingTraceListener.cs">
<Link>ExitingTraceListener.cs</Link>
</Compile>
<Compile Include="..\..\Shared\Csc.cs">
<Link>Csc.cs</Link>
</Compile>
<Compile Include="..\..\Shared\BuildClient.cs" />
<Compile Include="..\..\Shared\BuildServerConnection.cs" />
<Compile Include="..\..\Shared\CoreClrAnalyzerAssemblyLoader.cs" />
<Compile Include="..\..\Shared\DesktopBuildClient.cs" />
<Compile Include="..\..\Shared\DesktopAnalyzerAssemblyLoader.cs" />
<Compile Include="..\..\Shared\ExitingTraceListener.cs" />
<Compile Include="..\..\Shared\Csc.cs" />
</ItemGroup>
<ItemGroup>
<InternalsVisibleTo Include="Microsoft.CodeAnalysis.CSharp.CommandLine.UnitTests" />
Expand Down
63 changes: 16 additions & 47 deletions src/Compilers/Core/MSBuildTask/ManagedToolTask.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.IO;
using Microsoft.Build.Utilities;

namespace Microsoft.CodeAnalysis.BuildTasks
Expand All @@ -17,6 +18,15 @@ public abstract class ManagedToolTask : ToolTask

protected abstract string PathToManagedTool { get; }

protected string PathToManagedToolWithoutExtension
{
get
{
var extension = Path.GetExtension(PathToManagedTool);
return PathToManagedTool.Substring(0, PathToManagedTool.Length - extension.Length);
}
}

/// <summary>
/// Note: "Native" here does not necessarily mean "native binary".
/// "Native" in this context means "native invocation", and running the executable directly.
Expand All @@ -31,14 +41,9 @@ public abstract class ManagedToolTask : ToolTask
protected sealed override string GenerateCommandLineCommands()
{
var commandLineArguments = ToolArguments;
if (IsManagedTool && IsCliHost(out string pathToDotnet))
if (IsManagedTool)
{
var pathToTool = PathToManagedTool;
if (pathToTool is null)
{
Log.LogErrorWithCodeFromResources("General_ToolFileNotFound", ToolName);
}
commandLineArguments = PrependFileToArgs(pathToTool, commandLineArguments);
(_, commandLineArguments, _) = RuntimeHostInfo.GetProcessInfo(PathToManagedToolWithoutExtension, commandLineArguments);
}

return commandLineArguments;
Expand All @@ -51,21 +56,9 @@ protected sealed override string GenerateCommandLineCommands()
/// </summary>
protected sealed override string GenerateFullPathToTool()
{
if (IsManagedTool)
{
if (IsCliHost(out string pathToDotnet))
{
return pathToDotnet;
}
else
{
return PathToManagedTool;
}
}
else
{
return PathToNativeTool;
}
return IsManagedTool
? RuntimeHostInfo.GetProcessInfo(PathToManagedToolWithoutExtension, string.Empty).processFilePath
: PathToNativeTool;
}

protected abstract string ToolNameWithoutExtension { get; }
Expand All @@ -80,30 +73,6 @@ protected sealed override string GenerateFullPathToTool()
/// as the implementation of IsManagedTool calls this property. See the comment in
/// <see cref="ManagedCompiler.HasToolBeenOverridden"/>.
/// </remarks>
protected sealed override string ToolName
=> $"{ToolNameWithoutExtension}.{(CoreClrShim.IsRunningOnCoreClr ? "dll" : "exe")}";

private static bool IsCliHost(out string pathToDotnet)
{
if (CoreClrShim.IsRunningOnCoreClr)
{
pathToDotnet = Environment.GetEnvironmentVariable("DOTNET_HOST_PATH");
return !string.IsNullOrEmpty(pathToDotnet);
}
else
{
pathToDotnet = null;
return false;
}
}

private static string PrependFileToArgs(string pathToTool, string commandLineArgs)
{
var builder = new CommandLineBuilderExtension();
builder.AppendFileNameIfNotNull(pathToTool);
builder.AppendTextUnquoted(" ");
builder.AppendTextUnquoted(commandLineArgs);
return builder.ToString();
}
protected sealed override string ToolName => $"{ToolNameWithoutExtension}.{RuntimeHostInfo.ToolExtension}";
}
}
34 changes: 9 additions & 25 deletions src/Compilers/Core/MSBuildTask/Microsoft.Build.Tasks.CodeAnalysis.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,31 +35,15 @@
</ItemGroup>
<ItemGroup>
<Compile Include="..\..\Shared\NamedPipeUtil.cs" />
<Compile Include="..\..\Shared\BuildServerConnection.cs">
<Link>BuildServerConnection.cs</Link>
</Compile>
<Compile Include="..\..\Shared\CoreClrShim.cs">
<Link>CoreClrShim.cs</Link>
</Compile>
<Compile Include="..\Portable\CommitHashAttribute.cs" Link="CommitHashAttribute.cs" />
<Compile Include="..\Portable\InternalUtilities\CommandLineUtilities.cs">
<Link>CommandLineUtilities.cs</Link>
</Compile>
<Compile Include="..\Portable\InternalUtilities\CompilerOptionParseUtilities.cs">
<Link>CompilerOptionParseUtilities.cs</Link>
</Compile>
<Compile Include="..\Portable\InternalUtilities\IReadOnlySet.cs">
<Link>IReadOnlySet.cs</Link>
</Compile>
<Compile Include="..\Portable\InternalUtilities\PlatformInformation.cs">
<Link>PlatformInformation.cs</Link>
</Compile>
<Compile Include="..\Portable\InternalUtilities\ReflectionUtilities.cs">
<Link>ReflectionUtilities.cs</Link>
</Compile>
<Compile Include="..\Portable\InternalUtilities\UnicodeCharacterUtilities.cs">
<Link>UnicodeCharacterUtilities.cs</Link>
</Compile>
<Compile Include="..\..\Shared\BuildServerConnection.cs" />
<Compile Include="..\..\Shared\RuntimeHostInfo.cs" />
<Compile Include="..\Portable\CommitHashAttribute.cs" />
<Compile Include="..\Portable\InternalUtilities\CommandLineUtilities.cs" />
<Compile Include="..\Portable\InternalUtilities\CompilerOptionParseUtilities.cs" />
<Compile Include="..\Portable\InternalUtilities\IReadOnlySet.cs" />
<Compile Include="..\Portable\InternalUtilities\PlatformInformation.cs" />
<Compile Include="..\Portable\InternalUtilities\ReflectionUtilities.cs" />
<Compile Include="..\Portable\InternalUtilities\UnicodeCharacterUtilities.cs" />
<Compile Update="ErrorString.Designer.cs">
<AutoGen>True</AutoGen>
<DesignTime>True</DesignTime>
Expand Down
46 changes: 7 additions & 39 deletions src/Compilers/Core/MSBuildTask/Utilities.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,49 +191,17 @@ internal static string TryGetAssemblyPath(Assembly assembly)
}

/// <summary>
/// Try to get the directory this assembly is in. Returns null if assembly
/// was in the GAC or DLL location can not be retrieved.
/// Generate the full path to the tool that is deployed with our build tasks.
/// </summary>
public static string GenerateFullPathToTool(string toolName)
internal static string GenerateFullPathToTool(string toolName)
{
string toolLocation = null;

var buildTask = typeof(Utilities).GetTypeInfo().Assembly;
var assemblyPath = TryGetAssemblyPath(buildTask);

if (assemblyPath != null)
{
var assemblyDirectory = Path.GetDirectoryName(assemblyPath);
var desktopToolLocalLocation = Path.Combine(assemblyDirectory, toolName);
var cliToolLocalLocation = Path.Combine(assemblyDirectory, "bincore", toolName);

if (File.Exists(desktopToolLocalLocation))
{
toolLocation = desktopToolLocalLocation;
}
else if (File.Exists(cliToolLocalLocation))
{
toolLocation = cliToolLocalLocation;
}
}

if (toolLocation == null)
{
// Roslyn only deploys to the 32Bit folder of MSBuild, so request this path on all architectures.
var pathToBuildTools = ToolLocationHelper.GetPathToBuildTools(ToolLocationHelper.CurrentToolsVersion, DotNetFrameworkArchitecture.Bitness32);

if (pathToBuildTools != null)
{
var toolMSBuildLocation = Path.Combine(pathToBuildTools, MSBuildRoslynFolderName, toolName);

if (File.Exists(toolMSBuildLocation))
{
toolLocation = toolMSBuildLocation;
}
}
}
var assemblyPath = buildTask.Location;
var assemblyDirectory = Path.GetDirectoryName(assemblyPath);

return toolLocation;
return RuntimeHostInfo.IsDesktopRuntime
? Path.Combine(assemblyDirectory, toolName)
: Path.Combine(assemblyDirectory, "bincore", toolName);
}
}
}
5 changes: 1 addition & 4 deletions src/Compilers/Core/Portable/Microsoft.CodeAnalysis.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,7 @@
</Content>
</ItemGroup>
<ItemGroup>
<Compile Include="..\..\Shared\CoreClrShim.cs" Link="InternalUtilities\CoreClrShim.cs" />
<Compile Include="..\..\Shared\DesktopShim.cs">
<Link>DesktopShim.cs</Link>
</Compile>
<Compile Include="..\..\Shared\DesktopShim.cs" />
<Compile Update="CodeAnalysisResources.Designer.cs">
<AutoGen>True</AutoGen>
<DesignTime>True</DesignTime>
Expand Down
29 changes: 8 additions & 21 deletions src/Compilers/Server/VBCSCompiler/VBCSCompiler.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,28 +28,15 @@
<PackageReference Include="Microsoft.DiaSymReader.Native" Version="$(MicrosoftDiaSymReaderNativeVersion)"/>
</ItemGroup>
<ItemGroup>
<Compile Include="..\..\Shared\RuntimeHostInfo.cs" />
<Compile Include="..\..\Shared\NamedPipeUtil.cs" />
<Compile Include="..\..\Shared\BuildClient.cs">
<Link>BuildClient.cs</Link>
</Compile>
<Compile Include="..\..\Shared\BuildServerConnection.cs">
<Link>BuildServerConnection.cs</Link>
</Compile>
<Compile Include="..\..\Shared\CoreClrAnalyzerAssemblyLoader.cs">
<Link>CoreClrAnalyzerAssemblyLoader.cs</Link>
</Compile>
<Compile Include="..\..\Shared\DesktopAnalyzerAssemblyLoader.cs">
<Link>DesktopAnalyzerAssemblyLoader.cs</Link>
</Compile>
<Compile Include="..\..\Shared\DesktopBuildClient.cs">
<Link>DesktopBuildClient.cs</Link>
</Compile>
<Compile Include="..\..\Shared\ExitingTraceListener.cs">
<Link>ExitingTraceListener.cs</Link>
</Compile>
<Compile Include="..\..\Shared\ShadowCopyAnalyzerAssemblyLoader.cs">
<Link>ShadowCopyAnalyzerAssemblyLoader.cs</Link>
</Compile>
<Compile Include="..\..\Shared\BuildClient.cs" />
<Compile Include="..\..\Shared\BuildServerConnection.cs" />
<Compile Include="..\..\Shared\CoreClrAnalyzerAssemblyLoader.cs" />
<Compile Include="..\..\Shared\DesktopAnalyzerAssemblyLoader.cs" />
<Compile Include="..\..\Shared\DesktopBuildClient.cs" />
<Compile Include="..\..\Shared\ExitingTraceListener.cs" />
<Compile Include="..\..\Shared\ShadowCopyAnalyzerAssemblyLoader.cs" />
</ItemGroup>
<ItemGroup>
<None Include="App.config" />
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/Server/VBCSCompilerTests/CompilerServerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ private static DisposableFile GetResultFile(TempDirectory directory, string resu

private static void RunCompilerOutput(TempFile file, string expectedOutput)
{
if (!CoreClrShim.IsRunningOnCoreClr)
if (RuntimeHostInfo.IsDesktopRuntime)
{
var result = ProcessUtilities.Run(file.Path, "", Path.GetDirectoryName(file.Path));
Assert.Equal(expectedOutput.Trim(), result.Output.Trim());
Expand Down
16 changes: 6 additions & 10 deletions src/Compilers/Shared/BuildClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
using System.Runtime.InteropServices;
using System.Threading;
using System.Threading.Tasks;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.CommandLine
{
Expand Down Expand Up @@ -48,14 +49,9 @@ internal abstract class BuildClient
/// </summary>
public static string GetSystemSdkDirectory()
{
if (CoreClrShim.IsRunningOnCoreClr)
{
return null;
}
else
{
return RuntimeEnvironment.GetRuntimeDirectory();
}
return RuntimeHostInfo.IsCoreClrRuntime
? null
: RuntimeEnvironment.GetRuntimeDirectory();
}

/// <summary>
Expand Down Expand Up @@ -236,12 +232,12 @@ private static bool UseNativeArguments()
return false;
}

if (Type.GetType("Mono.Runtime") != null)
if (PlatformInformation.IsRunningOnMono)
{
return false;
}

if (CoreClrShim.IsRunningOnCoreClr)
if (RuntimeHostInfo.IsCoreClrRuntime)
{
// The native invoke ends up giving us both CoreRun and the exe file.
// We've decided to ignore backcompat for CoreCLR,
Expand Down
Loading