Repository navigation
Add XML output for retained type maps in ILC and ILLink - #135275
simonrozsival wants to merge 5 commits into
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: Successfully started running 4 pipeline(s). 12 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @agocke, @dotnet/illink |
Integrate NativeAOT output with ObjectDumper, pass the configured output path to both tools, and use shared library trimming tests for runtime, XML, and build-lifecycle coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
@MichalStrehovsky thanks for the review. Your suggestions helped simplify this quite a bit. ILC now uses an The output path comes from I moved the tests into the existing library trimming test so the same C# source checks runtime behavior and XML with both ILC and ILLink. The XML is copied beside the executable only for testing. This also covers cross-assembly mappings and universe isolation. The custom compiler/process harnesses and artificial metadata-name and null-key cases are gone. The extra type-name handling was an attempt to account for hypothetical unusual metadata names. I removed those special cases and the documentation caveat; ILC uses its existing formatter. I also removed the speculative unsupported-type diagnostic and format version, restored platform-default line endings, and moved the ILLink-only escaping helper out of the shared writer. Note This comment was drafted with GitHub Copilot. |
Generate the optional Import element only when additional targets are specified, avoiding empty Project attributes in other trimming test projects. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Enable typemap output with a non-empty TypeMapXmlFile instead of a separate boolean, and update the documentation. Remove the added XML and SDK lifecycle test setup, restoring the original TypeMap runtime test and shared trimming-test harness. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
@MichalStrehovsky addressed in 9af6503. Both ILC and ILLink now use I also removed the added trimming tests and their supporting project/import wiring, including the dependency project and XML checks. The original TypeMap runtime test and shared trimming-test infrastructure are unchanged. Note This comment was drafted with GitHub Copilot. |
|
My comment #135275 (comment) was specifically about what TypeMapTests.targets was doing: using MSBuild as a unit testing infrastructe (?). "Test cases" like "An unchanged publish regenerated the type map", "Changing the output path changed the type map contents.", "An unchanged publish regenerated the custom type map output.", "Type map output was generated while disabled". The one test that inspected the XML output was fine. The rest was... odd and we do that kind of testing in the SDK repo and write it in C# and XUnit, not in MSBuild. I don't mind either way, I think we can live without the XML reading test too. The native AOT changes look good to me, someone from @dotnet/illink should have a look at illink. |
sbomer
left a comment
There was a problem hiding this comment.
ILLink changes LGTM. I'd like to see this tested as a shared testcase under Mono.Linker.Tests.Cases. It'll need a few additions to our test infrastructure to pick up the output XML but nothing too bad.
Verify ILLink and NativeAOT typemap XML using a shared testcase and XPath expectations. Run production ILC compilation for output checks with a bounded compiler process lifetime. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
| var startInfo = new ProcessStartInfo | ||
| { | ||
| FileName = Assert.IsType<string>(AppContext.GetData("Mono.Linker.Tests.DotNetHostPath")), | ||
| UseShellExecute = false, | ||
| CreateNoWindow = true, | ||
| RedirectStandardOutput = true, | ||
| RedirectStandardError = true | ||
| }; | ||
| startInfo.ArgumentList.Add(compilerPath); | ||
| foreach (string inputFile in options.InputFilePaths.Values) | ||
| startInfo.ArgumentList.Add(inputFile); | ||
| AddArguments("--out", [options.OutputFileName]); | ||
| AddArguments("--output-typemaps", [typeMapFileName]); | ||
| startInfo.ArgumentList.Add("-O"); | ||
| startInfo.ArgumentList.Add("--scanreflection"); | ||
| AddArguments("--reference", options.ReferenceFilePaths.Values); | ||
| AddArguments("--initassembly", options.InitAssemblies); | ||
| AddArguments("--trim", options.TrimAssemblies); | ||
| AddArguments("--root", options.AdditionalRootAssemblies); | ||
| AddArguments("--conditionalroot", options.RootEntireAssemblies); | ||
| AddArguments("--descriptor", options.Descriptors); | ||
| AddArguments("--substitution", options.SubstitutionFiles); |
There was a problem hiding this comment.
@sbomer is this really what you want in #135275 (review)?
I shot down this approach earlier because I don't want us to maintain this fragile infrastructure that invokes ILC.exe
The TrimmingTest that was completely deleted in 9af6503#diff-400402d35472138ce4e15fb1f26b0de4341ee22daae3d5d2a58012d0a3a842e9 looked better - TrimmingTests just publish the project as usual so we exercise the entire thing E2E and then the test self-inspects the XML that was produced as part of the build. It covers a lot more than just a unit test, and it's not this sketchy thing that hopes to find ILC somewhere, and hopes that the arguments will still be correct, and we'll need to update it every six months.
I'd delete this, restore the TrimmingTest, but not restore the weird MSBuild-as-xunit code in TypeMapTests.targets because that's unreadable. If we really care about the fringe build-level cases that it was covering, those test should be in the sdk repo.
There was a problem hiding this comment.
Yeah, the last commit was not supposed to be pushed. Copilot is really struggling writing the tests and it keeps reinventing this test setup. I'll have to do it the old school way. Thanks for all the suggestions.
There was a problem hiding this comment.
Ah, I didn't realize that you already shot down this approach, or that it would require a new out-of-proc ILC test mode. Making it a TrimmingTest instead of a Mono.Linker.Tests.Cases test sounds like a good idea.
Adds opt-in XML output describing the type maps retained by ILC and ILLink, including external and proxy mappings grouped by universe.
This gives downstream build tools, such as Android’s R8 integration, the information needed to keep generated wrappers for surviving mappings without relying on dependency graphs or trimmed-assembly inspection.
Enable it with
TypeMapGenerateXmlFile=truein SDK builds or--output-typemapson the command line. The XML is an intermediate build artifact, not part of the published application, and does not change trimming behavior.The output uses a common XML format for both ILC and ILLink:
The root declares the format version, and each universe has separate sections for external (
string → Type) and proxy (Type → Type) mappings. External keys are preserved verbatim. Managed types use reflection-style names with simple assembly qualification.Closes #120204
/cc @rolfbjarne
Note
This PR description was drafted with GitHub Copilot.