Build primarily with `dotnet msbuild` #22017

dougbu · 2020-05-20T03:38:05Z

make dotnet msbuild the default on Windows too
enable building managed projects depending on native assets
revert move to VS2019.Pre queues

dougbu · 2020-05-28T18:49:01Z

See https://dev.azure.com/dnceng/internal/_build/results?buildId=662159 for internal validation of this change.

dougbu · 2020-05-28T18:53:31Z

.azure/pipelines/ci.yml

@@ -140,32 +141,35 @@ stages:
      - script: ./build.cmd
                -ci
                -nobl
+                -noBuildRepoTasks


This and the -noBuildNative additions are minor optimizations that also decrease the noise in the builds

.azure/pipelines/ci.yml

dougbu · 2020-05-28T18:57:34Z

.azure/pipelines/ci.yml

        displayName: Build shared fx
-      - script: .\restore.cmd -ci -nobl /p:BuildInteropProjects=true
+      - script: ./build.cmd -ci -nobl -noBuildRepoTasks -restore -noBuild -projects src/Grpc/**/*.csproj


Previously this would restore everything again, plus the interop projects. That's a waste and doesn't work correctly because ./restore.cmd adds --all but ./build.ps1 will pass $(BuildInteropProjects) into the VS build, causing the restore op to fail. So, this replacement is both a correction and an optimization.

dougbu · 2020-05-28T18:58:11Z

.azure/pipelines/jobs/default-build.yml

-          queue: BuildPool.Windows.10.Amd64.VS2019.Pre.Open
+          ${{ if ne(parameters.isTestingJob, true) }}:
+            # Visual Studio Build Tools
+            queue: BuildPool.Server.Amd64.VS2019.BT.Open


Yup, we don't need the preview queues anymore 😺

dougbu · 2020-05-28T18:59:42Z

eng/Build.props

+      <ItemGroup Condition=" '$(TargetOsName)' == 'win' AND ('$(TargetArchitecture)' == 'x86' OR '$(TargetArchitecture)' == 'x64') ">
+        <NativeProjects Include="$(RepoRoot)src\**\*.vcxproj" Exclude="@(ProjectToExclude)" AdditionalProperties="Platform=x64" />
+        <NativeProjects Include="$(RepoRoot)src\**\*.vcxproj" Exclude="@(ProjectToExclude)" AdditionalProperties="Platform=Win32" />
+      </ItemGroup>


I was concerned about doing both of the $(Platform)s in one build but saw no flakiness in multiple trial builds (mostly while I was working to get CodeCheck.ps1 working again)

eng/Version.Details.xml

dougbu · 2020-05-28T19:37:33Z

eng/scripts/GenerateProjectList.ps1

+  Remove-Item variable:global:_BuildTool -ErrorAction Ignore
+  Remove-Item variable:global:_DotNetInstallDir -ErrorAction Ignore
+  Remove-Item variable:global:_ToolsetBuildProj -ErrorAction Ignore
+  Remove-Item variable:global:_MSBuildExe -ErrorAction Ignore


Copied this stuff from our /build.ps1 file because invoking tools.ps1 corrupts global state if this script, eng/scripts/GenerateReferenceAssemblies.ps1, or eng/scripts/CodeCheck.ps1 are invoked in an interactive PowerShell session.

dougbu · 2020-05-28T19:38:33Z

eng/scripts/GenerateReferenceAssemblies.ps1

-& "$repoRoot\eng\common\msbuild.ps1" -ci:$ci "$repoRoot/eng/CodeGen.proj" `
+try {
+  # eng/common/msbuild.ps1 builds the Debug configuration unless there's a $Configuration variable. Match that here.
+  & "$repoRoot\build.ps1"  -ci:$ci -nobl -buildNative -configuration Debug


Building native assets to avoid warnings when running the GenerateReferenceSources target.

How long does it take to build native assets?

Looking at hhttps://dev.azure.com/dnceng/internal/_build/results?buildId=662159&view=logs&j=1b89928a-2219-5ef9-602f-f95beb3da4dc&t=bcce746a-741e-59b7-adec-a4a82d8c5e52&l=313 for example, 7 minutes and 40 seconds. I believe this aligns with a speedup in the next MSBuild invocation.

The overall build time goes down across all jobs comparing builds of this PR (2h 3m 45s average of 5 builds) with 'master' (2h 30m 45s average of 17 builds in the last 7 days). Performance wasn't the goal but it still seemed to improve. That said, 5 builds isn't that many.

Part of the improvement may come from only needing to build native assets once per build job. -noBuildNative was not possible everywhere before because the native assets were built only for a single architecture.

dougbu · 2020-05-28T19:40:33Z

src/Framework/src/Microsoft.AspNetCore.App.Runtime.csproj

      <!-- Custom attribute used to distinguish managed from native references. -->
      <IsNativeImage>true</IsNativeImage>
      <!-- A custom item group to be used in targets later.  -->
      <OutputItemType>_ResolvedNativeProjectReferencePaths</OutputItemType>
      <!-- C++ projects don't invoke GetTargetPath by default when building. -->
      <Targets>Build;GetTargetPath</Targets>
    </ProjectReference>
+
+    <_ResolvedNativeProjectReferencePaths Condition=" '$(BuildIisNativeProjects)' == 'true' AND !$(BuildNative) "


Unlike other managed projects with references to native projects, this one didn't have a fallback to find the right files and pack them up.

dougbu · 2020-05-28T19:41:59Z

build.ps1

-        $MSBuildArguments += "/bl:" + (Join-Path $LogDir "Build.binlog")
+        $dotnetBuildArguments += "/bl:" + (Join-Path $LogDir "Build.binlog")
+        $MSBuildArguments += "/bl:" + (Join-Path $LogDir "Build.native.binlog")
+        $ToolsetBuildArguments += "/bl:" + (Join-Path $LogDir "Build.repotasks.binlog")


Not relevant in CI but will be helpful when we re-enable binary logs.

.azure/pipelines/helix-matrix.yml

dougbu · 2020-05-28T22:58:07Z

/ping reviewers (between trainings of course): Latest build (after merging changes from master) was successful.

wtgodbe · 2020-05-29T17:45:52Z

Directory.Build.props

@@ -180,7 +180,7 @@
    <!-- Projects which reference Microsoft.AspNetCore.Mvc.Testing should import this targets file to ensure dependency .deps.json files are copied into test output. -->
    <MvcTestingTargets>$(MSBuildThisFileDirectory)src\Mvc\Mvc.Testing\src\Microsoft.AspNetCore.Mvc.Testing.targets</MvcTestingTargets>
    <!-- IIS native projects can only be built on Windows for x86 and x64. -->
-    <BuildIisNativeProjects Condition=" $(BuildNative) AND ('$(TargetArchitecture)' == 'x86' OR '$(TargetArchitecture)' == 'x64') ">true</BuildIisNativeProjects>
+    <BuildIisNativeProjects Condition=" '$(TargetOsName)' == 'win' AND ('$(TargetArchitecture)' == 'x86' OR '$(TargetArchitecture)' == 'x64') ">true</BuildIisNativeProjects>


What's the motivation for this logical change?

The $(BuildIisNativeProjects) property is really badly named. It actually controls whether the native assets are even looked for. We had the infrastructure in place to find the assets if they were build earlier but weren't using it. So, without this change, the native build would produce assemblies the managed build ignored.

wtgodbe

Looks good outside of the few questions

JunTaoLuo · 2020-05-29T19:08:28Z

eng/AfterSolutionBuild.targets

@@ -5,7 +5,8 @@
  <Import Project="SharedFramework.Local.props" />

  <!-- This is temporary until we can use FrameworkReference to build our own packages. -->
-  <Target Name="RemoveSharedFrameworkOnlyRefsFromNuspec" AfterTargets="Pack">
+  <Target Name="RemoveSharedFrameworkOnlyRefsFromNuspec" AfterTargets="Pack"
+      Condition=" '$(MSBuildRuntimeType)' == 'core' ">


why was the condition added?

A single build.cmd invocation now runs the MSBuild PowerShell function twice (three times including the repo tasks) and both would redo this step if the dev had built the repo earlier. Since the fixup only matters for managed packages and we only build managed packages in the dotnet msbuild invocation, this is safe and quiets things down.

- add step using desktop `msbuild` when native builds may be involved - `-All` (without `-NoBuildNative`), `-BuildNative` or `-BuildInstallers` run this step - but `-ForceCoreMsbuild` unconditionally skips this step nits: - add binary log for RepoTasks build if `$BinaryLog` (echoes the `dotnet msbuild` command) - add blank lines between build steps

- ensure native assets are included in GenerateReferenceAssemblies.ps1 build - clean up the global state that tools.ps1 corrupts

- splitting native builds out confuses these projects - use `$(BuildNative)` less, only to control actual building (not bundling) - build both native platforms in one `msbuild` invocation

This reverts part of commit b67d161 - was "[release/5.0-preview5] Update dependencies from dotnet/aspnetcore-tooling (#21710)"

- this reverts commit 58cf230

- build native assets and repo tasks once per CI job - only cleanup framework references after packing managed projects nits: - wrap a few long lines - remove extra `-forceCoreMsbuild` options in SiteExtensions' build.cmd

- restore.cmd doesn't work well with `-projects`; script unconditionally adds `-all`

- missed a couple of places `-noBuildRepoTasks` helps

dougbu · 2020-05-29T23:44:24Z

🆙📅 to rebase on 'master', undo -arch x64 removals, and add a few more -nobl and -noBuildRepoTools command-line arguments.

dougbu · 2020-05-30T02:00:31Z

🆗 looking good for PR validation and https://dev.azure.com/dnceng/internal/_build/results?buildId=664525 built the commit right before the fix-ups of scripts that aren't exercised in internal builds.

Pilchie added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label May 20, 2020

dougbu force-pushed the dougbu/dotnet.msbuild branch 14 times, most recently from c65e879 to 1f5f442 Compare May 27, 2020 05:50

dougbu mentioned this pull request May 27, 2020

[GenAPI] Give generated attributes a reliable order dotnet/arcade#5572

Merged

dougbu force-pushed the dougbu/dotnet.msbuild branch from 1f5f442 to fc607e9 Compare May 28, 2020 04:52

dougbu commented May 28, 2020

View reviewed changes

dougbu requested review from HaoK and a team May 28, 2020 19:43

dougbu marked this pull request as ready for review May 28, 2020 19:43

HaoK reviewed May 28, 2020

View reviewed changes

.azure/pipelines/helix-matrix.yml Show resolved Hide resolved

wtgodbe reviewed May 29, 2020

View reviewed changes

wtgodbe approved these changes May 29, 2020

View reviewed changes

JunTaoLuo approved these changes May 29, 2020

View reviewed changes

dougbu added 3 commits May 29, 2020 15:51

Adjust generation scripts to explicitly choose the MSBuild engine

a5741d5

- ensure native assets are included in GenerateReferenceAssemblies.ps1 build - clean up the global state that tools.ps1 corrupts

Enable building managed projects depending on native assets

ac3fe49

- splitting native builds out confuses these projects - use `$(BuildNative)` less, only to control actual building (not bundling) - build both native platforms in one `msbuild` invocation

dougbu added 6 commits May 29, 2020 15:51

Revert move to VS2019.Pre queues

dd70fd5

This reverts part of commit b67d161 - was "[release/5.0-preview5] Update dependencies from dotnet/aspnetcore-tooling (#21710)"

Revert "!temporary! Require msbuild from VS2019 16.6"

089e624

- this reverts commit 58cf230

Reduce build duplication in pipelines

fdd6ccc

- build native assets and repo tasks once per CI job - only cleanup framework references after packing managed projects nits: - wrap a few long lines - remove extra `-forceCoreMsbuild` options in SiteExtensions' build.cmd

Fix Helix jobs

9f038b5

- restore.cmd doesn't work well with `-projects`; script unconditionally adds `-all`

!fixup! Reduce duplications further

648cc82

- missed a couple of places `-noBuildRepoTasks` helps

Cleanup: Remove a few dangling binary logs

7d9dad7

dougbu force-pushed the dougbu/dotnet.msbuild branch from 51e391d to 7d9dad7 Compare May 29, 2020 23:42

dougbu added 2 commits May 29, 2020 17:12

!fixup! Correct typos in generation scripts

3b149b5

!fixup! Another typo in the generation scripts

5baad6b

dougbu merged commit 14d6b6e into master May 30, 2020

dougbu deleted the dougbu/dotnet.msbuild branch May 30, 2020 02:02

sebastienros mentioned this pull request Jan 18, 2022

Respect -noBuildNative for Kestrel #39619

Closed

HaoK mentioned this pull request Jan 20, 2022

Rename BuildIisNativeProjects -> UpdateIISNativeAssets #39654

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Build primarily with `dotnet msbuild` #22017

Build primarily with `dotnet msbuild` #22017

dougbu commented May 20, 2020

dougbu commented May 28, 2020

dougbu May 28, 2020

dougbu May 28, 2020

dougbu May 28, 2020

dougbu May 28, 2020

dougbu May 28, 2020

dougbu May 28, 2020

wtgodbe May 29, 2020

dougbu May 29, 2020

dougbu May 28, 2020

dougbu May 28, 2020

dougbu commented May 28, 2020

wtgodbe May 29, 2020

dougbu May 29, 2020

wtgodbe left a comment

JunTaoLuo May 29, 2020

dougbu May 29, 2020

dougbu commented May 29, 2020

dougbu commented May 30, 2020

Build primarily with dotnet msbuild #22017

Build primarily with dotnet msbuild #22017

Conversation

dougbu commented May 20, 2020

dougbu commented May 28, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dougbu commented May 28, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wtgodbe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dougbu commented May 29, 2020

dougbu commented May 30, 2020

Build primarily with `dotnet msbuild` #22017

Build primarily with `dotnet msbuild` #22017