-
Notifications
You must be signed in to change notification settings - Fork 5.6k
[wasm] Resolve crossgen2 through the SDK #133413
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
f51d9f7
8000026
3cf530b
0f1de9b
fe80022
e725e4b
8a4585d
42d1feb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,6 +25,10 @@ | |
| native.wasm.targets uses RuntimeFlavor to gate the CoreCLR-only relink registry. --> | ||
| <RuntimeFlavor Condition="'$(RuntimeFlavor)' == ''">CoreCLR</RuntimeFlavor> | ||
|
|
||
| <!-- Request crossgen2 here: WebAssembly.Pack is not imported before the first restore. | ||
| Only skip an explicit opt-out; otherwise native relinking may be inferred later. --> | ||
| <RequiresCrossgen2Pack Condition="'$(RequiresCrossgen2Pack)' == '' and '$(WasmBuildNative)' != 'false'">true</RequiresCrossgen2Pack> | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: It's more common to compare |
||
|
|
||
| <!-- EH and SIMD are always enabled for CoreCLR — detect user overrides before forcing true --> | ||
| <_CoreCLRInvalidEH Condition="'$(WasmEnableExceptionHandling)' != '' and '$(WasmEnableExceptionHandling)' != 'true'">true</_CoreCLRInvalidEH> | ||
| <_CoreCLRInvalidSIMD Condition="'$(WasmEnableSIMD)' != '' and '$(WasmEnableSIMD)' != 'true'">true</_CoreCLRInvalidSIMD> | ||
|
|
@@ -640,6 +644,15 @@ | |
|
|
||
| <!-- ======================== Generate Managed-to-Native ======================== --> | ||
|
|
||
| <!-- Check overrides after all imports; the SDK resolver cannot use the in-repo crossgen2 layout. | ||
| Keep CallTarget separate so its output items reach the generator. --> | ||
| <Target Name="_CoreCLRResolvePortableCallHelpersGenerator" | ||
| BeforeTargets="_CoreCLRGenerateManagedToNative" | ||
| DependsOnTargets="ResolveFrameworkReferences"> | ||
| <CallTarget Targets="ResolveReadyToRunCompilers" | ||
| Condition="'$(Crossgen2Path)' == '' and '$(Crossgen2InBuildDir)' == '' and '$(Crossgen2ToolPath)' == '' and '@(ResolvedCrossgen2Pack)' != ''" /> | ||
| </Target> | ||
|
|
||
| <Target Name="_CoreCLRGenerateManagedToNative"> | ||
| <PropertyGroup> | ||
| <_WasmPInvokeTablePath>$(_WasmIntermediateOutputPath)callhelpers-pinvoke.cpp</_WasmPInvokeTablePath> | ||
|
|
@@ -694,21 +707,20 @@ | |
| <_WasmManagedAssemblies Include="$(_CoreLibPath)" /> | ||
| </ItemGroup> | ||
|
|
||
| <!-- crossgen2 has the type system that knows the wasm ABI, so it generates the helpers itself. | ||
| In the repo it comes from the build output; outside it, from the crossgen2 pack | ||
| the wasm-tools workload acquires, which sets $(Crossgen2ToolPath) from its Sdk.props. --> | ||
| <!-- crossgen2 provides the wasm type layout needed to generate call helpers. --> | ||
| <PropertyGroup> | ||
| <_Crossgen2ExeSuffix Condition="'$(OS)' == 'Windows_NT'">.exe</_Crossgen2ExeSuffix> | ||
| <Crossgen2Path Condition="'$(Crossgen2Path)' == '' and '$(Crossgen2InBuildDir)' != ''">$([MSBuild]::NormalizePath('$(Crossgen2InBuildDir)', 'crossgen2$(_Crossgen2ExeSuffix)'))</Crossgen2Path> | ||
| <Crossgen2Path Condition="'$(Crossgen2Path)' == ''">$(Crossgen2ToolPath)</Crossgen2Path> | ||
| <Crossgen2Path Condition="'$(Crossgen2Path)' == ''">@(Crossgen2Tool)</Crossgen2Path> | ||
| </PropertyGroup> | ||
|
|
||
| <Error Condition="'$(Crossgen2Path)' == ''" | ||
| Text="crossgen2 was not found, so the portable call helpers cannot be generated. Install the 'wasm-tools' workload, or set %24(Crossgen2Path) to a crossgen2 executable." /> | ||
| Text="Could not resolve crossgen2. Update the .NET SDK and restore the project, or set %24(Crossgen2Path) to a crossgen2 executable." /> | ||
| <Error Condition="'$([System.IO.Path]::GetExtension(`$(Crossgen2Path)`))' == '.dll'" | ||
| Text="%24(Crossgen2Path) is '$(Crossgen2Path)'. It has to name a crossgen2 executable; an IL assembly cannot be launched directly." /> | ||
| Text="Set %24(Crossgen2Path) to a crossgen2 executable, not the DLL '$(Crossgen2Path)'." /> | ||
| <Error Condition="!Exists('$(Crossgen2Path)')" | ||
| Text="%24(Crossgen2Path) is '$(Crossgen2Path)', which does not exist. In the repo, build crossgen2 first; outside it, install the 'wasm-tools' workload." /> | ||
| Text="crossgen2 executable not found: '$(Crossgen2Path)'. Build or restore crossgen2, or correct %24(Crossgen2Path)." /> | ||
|
|
||
| <PropertyGroup> | ||
| <_Crossgen2GeneratorRsp>$(_WasmIntermediateOutputPath)callhelpers-generator.rsp</_Crossgen2GeneratorRsp> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -151,6 +151,72 @@ public void PublishReadyToRunDirectoryMatchesSdkOutputCasing() | |
| Assert.Equal(Path.Combine(GetObjDir(config), "R2R") + Path.DirectorySeparatorChar, match.Groups[1].Value); | ||
| } | ||
|
|
||
| [Theory] | ||
| [InlineData(false)] | ||
| [InlineData(true)] | ||
| public void NativeRelinkResolvesCrossgen2WithoutReadyToRun(bool publish) | ||
| { | ||
| ProjectInfo info = CopyTestAsset( | ||
| Configuration.Debug, | ||
| aot: false, | ||
| TestAsset.WasmBasicTestApp, | ||
| "coreclr_sdk_crossgen2", | ||
| extraProperties: """ | ||
| <PublishReadyToRun>false</PublishReadyToRun> | ||
| <WasmBuildNative>true</WasmBuildNative> | ||
| """, | ||
| insertAtEnd: $$""" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Please refactor it to a targets file linked in the csproj to improve maintainability. For example as |
||
| <Target Name="CheckCrossgen2PackRequest" BeforeTargets="ProcessFrameworkReferences"> | ||
| <Error Condition="'$(RequiresCrossgen2Pack)' != 'true'" | ||
| Text="Expected RequiresCrossgen2Pack=true before restore." /> | ||
| </Target> | ||
| <Target Name="CheckSdkCrossgen2" AfterTargets="_CoreCLRGenerateManagedToNative"> | ||
| <Error Condition="'{{publish}}' == 'true' and '$(WasmBuildingForNestedPublish)' != 'true'" | ||
| Text="Expected helper generation during nested publish." /> | ||
| <Error Condition="'$(PublishReadyToRun)' == 'true'" | ||
| Text="Expected PublishReadyToRun=false." /> | ||
| <Error Condition="'$(Crossgen2InBuildDir)' != '' or '$(Crossgen2ToolPath)' != ''" | ||
| Text="Expected SDK resolution without crossgen2 path overrides." /> | ||
| <Error Condition="'@(Crossgen2Tool)' == '' or '$(Crossgen2Path)' != '@(Crossgen2Tool)'" | ||
| Text="Expected the generator path to match the SDK Crossgen2Tool item." /> | ||
| <Error Condition="!Exists('$(_WasmPInvokeTablePath)') or !Exists('$(_WasmReversePInvokeTablePath)') or !Exists('$(_WasmInterpToNativeTablePath)')" | ||
| Text="A generated call-helper table is missing." /> | ||
| <Error Text="Stopping after validating SDK crossgen2" /> | ||
| </Target> | ||
| """); | ||
|
|
||
| // Run the generator, then stop before native compilation. | ||
| string output = publish | ||
| ? PublishProject(info, Configuration.Debug, new PublishOptions(ExpectSuccess: false)).buildOutput | ||
| : BuildProject(info, Configuration.Debug, new BuildOptions(ExpectSuccess: false)).buildOutput; | ||
|
|
||
| Assert.Contains("Stopping after validating SDK crossgen2", output); | ||
| } | ||
|
|
||
| [Theory] | ||
| [InlineData(false)] | ||
| [InlineData(true)] | ||
| public void NativeRelinkWithoutCrossgen2PackReportsMissingGenerator(bool publish) | ||
| { | ||
| ProjectInfo info = CopyTestAsset( | ||
| Configuration.Debug, | ||
| aot: false, | ||
| TestAsset.WasmBasicTestApp, | ||
| "coreclr_missing_crossgen2", | ||
| extraProperties: """ | ||
| <PublishReadyToRun>false</PublishReadyToRun> | ||
| <RequiresCrossgen2Pack>false</RequiresCrossgen2Pack> | ||
| <WasmBuildNative>true</WasmBuildNative> | ||
| """); | ||
|
|
||
| string output = publish | ||
| ? PublishProject(info, Configuration.Debug, new PublishOptions(ExpectSuccess: false)).buildOutput | ||
| : BuildProject(info, Configuration.Debug, new BuildOptions(ExpectSuccess: false)).buildOutput; | ||
|
|
||
| Assert.Contains("Could not resolve crossgen2. Update the .NET SDK and restore the project, or set $(Crossgen2Path) to a crossgen2 executable.", output); | ||
| Assert.DoesNotContain("NETSDK1094", output); | ||
| } | ||
|
|
||
| private string? BuildAndGetWasmBuildNativeLine(string projectPrefix, string extraProperties, bool expectSuccess) | ||
| => BuildAndGetOutput(projectPrefix, extraProperties, extraItems: "", expectSuccess).line; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -175,12 +175,15 @@ private static void AddCoreClrProjectProperties(ref string extraProperties, ref | |||||||||||||||||
| """; | ||||||||||||||||||
| insertAtEnd += | ||||||||||||||||||
| $$""" | ||||||||||||||||||
| <Target Name="_UpdateKnownWebAssemblySdkPack" BeforeTargets="ProcessFrameworkReferences" | ||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This target is already removed on main. The packs are correctly resolved from the installed workload in
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I still see it on main runtime/src/mono/wasm/Wasm.Build.Tests/Templates/WasmTemplateTestsBase.cs Lines 178 to 185 in 906f6c7
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, my mistake. I looked at wrong branch. |
||||||||||||||||||
| <Target Name="_UpdateKnownCoreClrWebAssemblyPacks" BeforeTargets="ProcessFrameworkReferences" | ||||||||||||||||||
| Condition="'$(RuntimeIdentifier)' == 'browser-wasm'"> | ||||||||||||||||||
| <ItemGroup> | ||||||||||||||||||
| <KnownWebAssemblySdkPack Update="@(KnownWebAssemblySdkPack)"> | ||||||||||||||||||
| <WebAssemblySdkPackVersion Condition="'%(KnownWebAssemblySdkPack.TargetFramework)' == '{{DefaultTargetFramework}}'">{{runtimePackVersion}}</WebAssemblySdkPackVersion> | ||||||||||||||||||
| </KnownWebAssemblySdkPack> | ||||||||||||||||||
| <KnownCrossgen2Pack Update="@(KnownCrossgen2Pack)"> | ||||||||||||||||||
| <Crossgen2PackVersion Condition="'%(KnownCrossgen2Pack.TargetFramework)' == '{{DefaultTargetFramework}}'">{{runtimePackVersion}}</Crossgen2PackVersion> | ||||||||||||||||||
| </KnownCrossgen2Pack> | ||||||||||||||||||
|
radekdoulik marked this conversation as resolved.
|
||||||||||||||||||
| </ItemGroup> | ||||||||||||||||||
| </Target> | ||||||||||||||||||
| """; | ||||||||||||||||||
|
|
||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This means the crossgen2 will be required only when the workload is installed, but according to discussion with Pavel, R2R should work even without workload. Is that true?
If that's the case, we need to more this to
dotnet/sdkwhere root skeleton for the SDK lives. Putting it there results in requesting the crossgen2 before nuget restore.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
still yes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is independent from R2R compilation and this path is generator specific. R2R compilation adds the pack when
ReadyToRunEnabled && ReadyToRunUseCrossgen2. https://github.com/dotnet/sdk/blob/02d39b6a25a62e5ac313ab8449704c4428fa68f3/src/Tasks/Microsoft.NET.Build.Tasks/ProcessFrameworkReferences.cs#L482The relinking with generator currently needs workload as it still produces C files.