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.

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

[wasm] Update to build for tfm=net8.0 #2816

Merged
merged 6 commits into from
Jan 12, 2023
Merged

Conversation

radical
Copy link
Member

@radical radical commented Jan 6, 2023

  • Blazor scenarios: Use 8.0.0-* nugets when building for net8.0 .
    • Without this change, when building for net7/net8, the aspnetcore assemblies were used from 6.0.0 packages.
  • Also, remove workarounds added earlier when we were targeting net7.0 but with the 8.0 sdk. This allows building for net8.0 with the 8.0 sdk.
  • Add a blazor_scenarios job for ubuntu
  • Add binlogs for blazor scenarios

Copy link
Member

@lewing lewing left a comment

Choose a reason for hiding this comment

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

Looks good to me but it might be nice to indicate the build this is first used in, I expect size regressions and I'm not sure what the best way to isolate them specifically is

<BlazorVersion Condition="'$(AspNetCoreVersion)' == ''">8.0.0-*</BlazorVersion>
<SystemNetHttpJsonVersion>8.0.0-*</SystemNetHttpJsonVersion>
</PropertyGroup>
<PropertyGroup Condition="'$(TargetFramework)' != 'net8.0'">
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason to not set net7.0 correctly here and in release/7.0 as well and maybe add a warning for net9?

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 don't know what the existing cases are, or what depends on these older versions, so I went with preserving older behavior except for the cases that I know about.

If there isn't anything depending on this, then we can set it to net7.0, and the blazor, and STJ versions to match.

@radical radical closed this Jan 10, 2023
@radical radical reopened this Jan 10, 2023
@radical radical marked this pull request as ready for review January 10, 2023 05:19
@radical radical requested a review from lewing January 10, 2023 22:24
@radical radical changed the title Blazor scenarios: Use 8.0.0-* nugets when building for net8.0 . [wasm] Update to build for tfm=net8.0 Jan 10, 2023
@radical
Copy link
Member Author

radical commented Jan 11, 2023

/azp run performance-ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Without this change, when building for net7/net8, the aspnetcore
assemblies were used from 6.0.0 packages.
- generate, and collect binlogs
- update to work for linux too
@radical
Copy link
Member Author

radical commented Jan 12, 2023

I'll add the blazor package versions to the report in a follow up PR.

@radical
Copy link
Member Author

radical commented Jan 12, 2023

The failures are known issues. The Windows/micro_net462 will be fixed by #2836 .

@radical radical merged commit b22f6f7 into dotnet:main Jan 12, 2023
@radical radical deleted the bump-blazor-vers branch January 12, 2023 16:12

<PerfLabTargetFrameworksEnvVar Condition="'$(TargetsWindows)' == 'true'">%PERFLAB_TARGET_FRAMEWORKS%</PerfLabTargetFrameworksEnvVar>
<PerfLabTargetFrameworksEnvVar Condition="'$(TargetsWindows)' != 'true'">%24{PERFLAB_TARGET_FRAMEWORKS}</PerfLabTargetFrameworksEnvVar>
<PublishArgs>--msbuild "/p:_TrimmerDumpDependencies=true" --msbuild /warnaserror:NU1602,NU1604 --msbuild-static AdditionalMonoLinkerOptions=%27&quot;%24(AdditionalMonoLinkerOptions) --dump-dependencies&quot;%27 --binlog $(LogDirectory)blazor_publish.binlog</PublishArgs>
Copy link
Member

Choose a reason for hiding this comment

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

Passing the same arg to python's argparser multiple times has an effect of ignoring all args but the last one. I doubt _TrimmerDumpDependencies=true was set.
E.g. running python3 ./pre.py publish -f net7.0 --readonly-dotnet --has-workload --msbuild "/p:HybridGlobalization=true" --msbuild "/p:RunAOTCompilation=true" --msbuild /warnaserror:NU1602,NU1604 results in:
dotnet publish /workspaces/performance/src/scenarios/blazorpizza/app/BlazingPizza.sln --configuration Release /p:NuGetPackageRoot=/workspaces/performance/artifacts/packages/ /p:RestorePackagesPath=/workspaces/performance/artifacts/packages/ /p:UseSharedCompilation=false /p:BuildInParallel=false /m:1 /p:PublishDir=/workspaces/performance/src/scenarios/blazorpizza/pub --framework net7.0 /warnaserror:NU1602,NU1604 -p:WasmNativeWorkload=false /p:EnableWindowsTargeting=true
and running with e.g. --msbuild "/p:HybridGlobalization=true" --msbuild "/p:RunAOTCompilation=true" results in /p:RunAOTCompilation=true -p:WasmNativeWorkload=false /p:EnableWindowsTargeting=true

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants