Skip to content

Fix cross-os symbol generation #20481

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

Merged
merged 4 commits into from
Sep 3, 2021
Merged

Fix cross-os symbol generation #20481

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b97f6d7
Fix cross-os symbol generation
davidwrighton Sep 1, 2021
c02ca44
Fix tests to specify composite accurately to TestProjectPublishing_In…
davidwrighton Sep 1, 2021
ada541a
Fix tests
davidwrighton Sep 2, 2021
da8623e
MoreTestFix
davidwrighton Sep 3, 2021
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
32 changes: 28 additions & 4 deletions src/Tasks/Microsoft.NET.Build.Tasks/PrepareForReadyToRunCompilation.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,30 @@ public class PrepareForReadyToRunCompilation : TaskBase
private List<ITaskItem> _r2rCompositeReferences = new List<ITaskItem>();
private List<ITaskItem> _r2rCompositeInput = new List<ITaskItem>();

private bool IsTargetWindows
{
get
{
// Crossgen2 V6 and above always has TargetOS metadata available
if (ReadyToRunUseCrossgen2 && !string.IsNullOrEmpty(Crossgen2Tool.GetMetadata(MetadataKeys.TargetOS)))
return Crossgen2Tool.GetMetadata(MetadataKeys.TargetOS) == "windows";
else
return RuntimeInformation.IsOSPlatform(OSPlatform.Windows);
}
}

private bool IsTargetLinux
{
get
{
// Crossgen2 V6 and above always has TargetOS metadata available
if (ReadyToRunUseCrossgen2 && !string.IsNullOrEmpty(Crossgen2Tool.GetMetadata(MetadataKeys.TargetOS)))
return Crossgen2Tool.GetMetadata(MetadataKeys.TargetOS) == "linux";
else
return RuntimeInformation.IsOSPlatform(OSPlatform.Linux);
}
}

protected override void ExecuteCore()
{
if (ReadyToRunUseCrossgen2)
Expand Down Expand Up @@ -140,13 +164,13 @@ private void ProcessInputFileList(

if (EmitSymbols)
{
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows) && hasValidDiaSymReaderLib)
if (IsTargetWindows && hasValidDiaSymReaderLib)
{
outputPDBImage = Path.ChangeExtension(outputR2RImage, "ni.pdb");
outputPDBImageRelativePath = Path.ChangeExtension(outputR2RImageRelativePath, "ni.pdb");
crossgen1CreatePDBCommand = $"/CreatePDB \"{Path.GetDirectoryName(outputPDBImage)}\"";
}
else if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux))
else if (IsTargetLinux)
{
string perfmapExtension;
if (ReadyToRunUseCrossgen2 && !_crossgen2IsVersion5 && _perfmapFormatVersion >= 1)
Expand Down Expand Up @@ -246,12 +270,12 @@ private void ProcessInputFileList(
{
string compositePDBImage = null;
string compositePDBRelativePath = null;
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows) && hasValidDiaSymReaderLib)
if (IsTargetWindows && hasValidDiaSymReaderLib)
{
compositePDBImage = Path.ChangeExtension(compositeR2RImage, ".ni.pdb");
compositePDBRelativePath = Path.ChangeExtension(compositeR2RImageRelativePath, ".ni.pdb");
}
else if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux))
else if (IsTargetLinux)
{
string perfmapExtension = (_perfmapFormatVersion >= 1 ? ".ni.r2rmap" : ".ni.{composite}.map");
compositePDBImage = Path.ChangeExtension(compositeR2RImage, perfmapExtension);
Expand Down
2 changes: 1 addition & 1 deletion src/Tasks/Microsoft.NET.Build.Tasks/RunReadyToRunCompiler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ private string GenerateCrossgen2ResponseFile()
// 5.0 Crossgen2 doesn't support PDB generation.
if (!Crossgen2IsVersion5 && _emitSymbols)
{
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
if (Crossgen2Tool.GetMetadata(MetadataKeys.TargetOS) == "windows")
{
result.AppendLine("--pdb");
result.AppendLine($"--pdb-path:{Path.GetDirectoryName(_outputPDBImage)}");
Expand Down
2 changes: 1 addition & 1 deletion src/Tests/Microsoft.NET.Publish.Tests/GivenThatWeWantToPublishASingleFileApp.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ public void It_excludes_ni_pdbs_from_single_file()

var intermediateDirectory = publishCommand.GetIntermediateDirectory(targetFramework, runtimeIdentifier: RuntimeInformation.RuntimeIdentifier);
var mainProjectDll = Path.Combine(intermediateDirectory.FullName, $"{TestProjectName}.dll");
var niPdbFile = GivenThatWeWantToPublishReadyToRun.GetPDBFileName(mainProjectDll, framework);
var niPdbFile = GivenThatWeWantToPublishReadyToRun.GetPDBFileName(mainProjectDll, framework, RuntimeInformation.RuntimeIdentifier);

string[] expectedFiles = { SingleFile, PdbFile, niPdbFile };
GetPublishDirectory(publishCommand)
Expand Down
150 changes: 133 additions & 17 deletions src/Tests/Microsoft.NET.Publish.Tests/GivenThatWeWantToPublishReadyToRun.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Reflection.Metadata;
using System.Reflection.PortableExecutable;
using System.Runtime.CompilerServices;
Expand Down Expand Up @@ -86,8 +87,8 @@ public void It_creates_readytorun_images_for_all_assemblies_except_excluded_ones
NuGetFramework framework = NuGetFramework.Parse(targetFramework);

publishDirectory.Should().NotHaveFiles(new[] {
GetPDBFileName(mainProjectDll, framework),
GetPDBFileName(classLibDll, framework),
GetPDBFileName(mainProjectDll, framework, testProject.RuntimeIdentifier),
GetPDBFileName(classLibDll, framework, testProject.RuntimeIdentifier),
});
}

Expand All @@ -112,7 +113,7 @@ public void It_creates_readytorun_symbols_when_switch_is_used(string targetFrame
[InlineData("net6.0")]
public void It_supports_framework_dependent_publishing(string targetFramework)
{
TestProjectPublishing_Internal("FrameworkDependent", targetFramework, isSelfContained: false, emitNativeSymbols:true, identifier: targetFramework);
TestProjectPublishing_Internal("FrameworkDependent", targetFramework, isSelfContained: false, composite: false, emitNativeSymbols:true, identifier: targetFramework);
}

[Theory]
Expand Down Expand Up @@ -199,7 +200,7 @@ public void It_warns_when_targetting_netcoreapp_2_x_readytorun()
[InlineData("net6.0")]
public void It_can_publish_readytorun_for_library_projects(string targetFramework)
{
TestProjectPublishing_Internal("LibraryProject1", targetFramework, isSelfContained: false, makeExeProject: false, identifier: targetFramework);
TestProjectPublishing_Internal("LibraryProject1", targetFramework, isSelfContained: false, composite: false, makeExeProject: false, identifier: targetFramework);
}

[RequiresMSBuildVersionTheory("17.0.0.32901")]
Expand All @@ -208,7 +209,7 @@ public void It_can_publish_readytorun_for_library_projects(string targetFramewor
[InlineData("net6.0")]
public void It_can_publish_readytorun_for_selfcontained_library_projects(string targetFramework)
{
TestProjectPublishing_Internal("LibraryProject2", targetFramework, isSelfContained:true, makeExeProject: false, identifier: targetFramework);
TestProjectPublishing_Internal("LibraryProject2", targetFramework, isSelfContained:true, composite: true, makeExeProject: false, identifier: targetFramework);
}

[RequiresMSBuildVersionTheory("17.0.0.32901")]
Expand Down Expand Up @@ -264,6 +265,103 @@ public void It_supports_libraries_when_using_crossgen2(string targetFramework)
publishCommand.Execute().Should().Pass();
}

[RequiresMSBuildVersionTheory("17.0.0.32901")]
[InlineData("net6.0", "linux-x64", "windows,linux,osx", "X64,Arm64", "_", "_")]
[InlineData("net6.0", "linux-x64", "windows,linux,osx", "X64,Arm64", "composite", "selfcontained")] // Composite in .NET 6.0 is only supported for self-contained builds
// In .NET 6.0 building targetting Windows on linux or osx doesn't support emitting native symbols.
[InlineData("net6.0", "win-x64", "windows", "X64,Arm64", "composite", "selfcontained")] // Composite in .NET 6.0 is only supported for self-contained builds
[InlineData("net6.0", "osx-arm64", "windows,linux,osx", "X64,Arm64", "_", "_")]
// In .NET 6.0 building targetting Windows on linux or osx doesn't support emitting native symbols.
[InlineData("net6.0", "win-x86", "windows", "X86,X64,Arm64,Arm", "_", "_")]
public void It_supports_crossos_arch_compilation(string targetFramework, string runtimeIdentifier, string sdkSupportedOs, string sdkSupportedArch, string composite, string selfcontained)
Copy link
Member

Choose a reason for hiding this comment

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

I would use either just It_supports_cross_compilation or It_supports_cross_os_and_arch_compilation.

{
var projectName = $"CrossArchOs{targetFramework}{runtimeIdentifier.Replace("-",".")}{composite}{selfcontained}";
string sdkOs = "NOTHING";
if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux))
{
sdkOs = "linux";
}
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
sdkOs = "windows";
}
if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
{
sdkOs = "osx";
}

Assert.NotEqual("NOTHING", sdkOs); // We should know which OS we are running on
Log.WriteLine($"sdkOs = {sdkOs}");
if (!sdkSupportedOs.Contains(sdkOs))
{
Log.WriteLine("Running test on OS that doesn't support this cross platform build");
return;
}

string sdkArch = RuntimeInformation.ProcessArchitecture.ToString();
Log.WriteLine($"sdkArch = {sdkArch}");
Assert.Contains(sdkArch, new string[]{"Arm", "Arm64", "X64", "X86"}); // Assert that the Architecture in use is a known architecture
if (!sdkSupportedArch.Split(',').Contains(sdkArch))
{
Log.WriteLine("Running test on processor architecture that doesn't support this cross platform build");
return;
}

TestProjectPublishing_Internal(projectName, targetFramework, isSelfContained: selfcontained == "selfcontained", emitNativeSymbols: true, useCrossgen2: true, composite: composite == "composite", identifier: targetFramework, runtimeIdentifier: runtimeIdentifier);
}

private enum TargetOSEnum
{
Windows,
Linux,
OsX
}

private static TargetOSEnum GetTargetOS(string runtimeIdentifier)
{
if (runtimeIdentifier.Contains("osx"))
{
return TargetOSEnum.OsX;
}
else if (runtimeIdentifier.Contains("win"))
{
return TargetOSEnum.Windows;
}
else if (runtimeIdentifier.Contains("linux") ||
runtimeIdentifier.Contains("ubuntu") ||
runtimeIdentifier.Contains("alpine") ||
runtimeIdentifier.Contains("android") ||
runtimeIdentifier.Contains("centos") ||
runtimeIdentifier.Contains("debian") ||
runtimeIdentifier.Contains("fedora") ||
runtimeIdentifier.Contains("gentoo") ||
runtimeIdentifier.Contains("suse") ||
runtimeIdentifier.Contains("rhel") ||
runtimeIdentifier.Contains("sles") ||
runtimeIdentifier.Contains("tizen"))
{
return TargetOSEnum.Linux;
}

Assert.True(false, $"{runtimeIdentifier} could not be converted into a known OS type. Adjust the if statement above until this does not happen");
return TargetOSEnum.Windows;
}

private static bool IsTargetOsOsX(string runtimeIdentifier)
{
return GetTargetOS(runtimeIdentifier) == TargetOSEnum.OsX;
}

private static bool IsTargetOsWindows(string runtimeIdentifier)
{
return GetTargetOS(runtimeIdentifier) == TargetOSEnum.Windows;
}

private static bool IsTargetOsLinux(string runtimeIdentifier)
{
return GetTargetOS(runtimeIdentifier) == TargetOSEnum.Linux;
}

private void TestProjectPublishing_Internal(string projectName,
string targetFramework,
bool makeExeProject = true,
Expand All @@ -272,13 +370,15 @@ private void TestProjectPublishing_Internal(string projectName,
bool useCrossgen2 = false,
bool composite = true,
[CallerMemberName] string callingMethod = "",
string identifier = null)
string identifier = null,
string runtimeIdentifier = null)
{
var testProject = CreateTestProjectForR2RTesting(
targetFramework,
projectName,
"ClassLib",
isExeProject: makeExeProject);
isExeProject: makeExeProject,
runtimeIdentifier: runtimeIdentifier);

testProject.AdditionalProperties["PublishReadyToRun"] = "True";
testProject.AdditionalProperties["PublishReadyToRunEmitSymbols"] = emitNativeSymbols ? "True" : "False";
Expand Down Expand Up @@ -307,18 +407,34 @@ private void TestProjectPublishing_Internal(string projectName,
else
publishDirectory.Should().NotHaveFile("System.Private.CoreLib.dll");

if (emitNativeSymbols && !RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
if (emitNativeSymbols && !IsTargetOsOsX(testProject.RuntimeIdentifier))
{
NuGetFramework framework = NuGetFramework.Parse(targetFramework);
Log.WriteLine("Checking for symbol files");
IEnumerable<string> pdbFiles;

publishDirectory.Should().HaveFiles(new[] {
GetPDBFileName(mainProjectDll, framework),
GetPDBFileName(classLibDll, framework),
});
if (composite)
{
pdbFiles = new[] { GetPDBFileName(Path.ChangeExtension(mainProjectDll, "r2r.dll"), framework, testProject.RuntimeIdentifier) };
Copy link
Member

Choose a reason for hiding this comment

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

How would the composite tests pass without this line?

Copy link
Member Author

Choose a reason for hiding this comment

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

The tests which set composite used to not set selfcontained. This resulted in the compiler not actually producing a composite build, so the test continued to pass. The tests have been adjusted to explicitly pass composite false now.

}
else
{
pdbFiles = new[] {
GetPDBFileName(mainProjectDll, framework, testProject.RuntimeIdentifier),
GetPDBFileName(classLibDll, framework, testProject.RuntimeIdentifier),
};
}

foreach (string s in pdbFiles)
{
Log.WriteLine($"{publishDirectory.FullName} {s}");
}

publishDirectory.Should().HaveFiles(pdbFiles);
}
}

private TestProject CreateTestProjectForR2RTesting(string targetFramework, string mainProjectName, string referenceProjectName, bool isExeProject = true)
private TestProject CreateTestProjectForR2RTesting(string targetFramework, string mainProjectName, string referenceProjectName, bool isExeProject = true, string runtimeIdentifier = null)
{
var referenceProject = new TestProject()
{
Expand All @@ -340,7 +456,7 @@ public string Func()
Name = mainProjectName,
TargetFrameworks = targetFramework,
IsExe = isExeProject,
RuntimeIdentifier = EnvironmentInfo.GetCompatibleRid(targetFramework),
RuntimeIdentifier = runtimeIdentifier ?? EnvironmentInfo.GetCompatibleRid(targetFramework),
ReferencedProjects = { referenceProject },
};
testProject.SourceFiles[$"{mainProjectName}.cs"] = @"
Expand All @@ -356,14 +472,14 @@ public static void Main()
return testProject;
}

public static string GetPDBFileName(string assemblyFile, NuGetFramework framework)
public static string GetPDBFileName(string assemblyFile, NuGetFramework framework, string runtimeIdentifier)
{
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
if (IsTargetOsWindows(runtimeIdentifier))
{
return Path.GetFileName(Path.ChangeExtension(assemblyFile, "ni.pdb"));
}

if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux))
if (IsTargetOsLinux(runtimeIdentifier))
{
if (framework.Version.Major >= 6)
{
Expand Down