Use dotnet CLI exclusively (no MSBuild)#4168
Conversation
0721576 to
98cb968
Compare
benrr101
left a comment
There was a problem hiding this comment.
In general I think this will be good - but let me know what your proposed timeline for getting this through is so I can get prepared for merge conflicts in my build2.proj work :)
|
@benrr101 - This PR will wait for your work to complete. I will deal with any conflicts here. |
There was a problem hiding this comment.
Pull request overview
This PR updates the repo’s build/test orchestration to use the dotnet CLI end-to-end (instead of invoking MSBuild directly), aligning OneBranch + CI templates and developer documentation with the upcoming build2.proj workflow.
Changes:
- Updated OneBranch pipeline steps to invoke
dotnet build/DotNetCoreCLI@2for.proj-driven builds, packing, and Roslyn analyzer runs. - Updated CI templates to use
DotNetCoreCLI@2for.projtargets and removed themsbuildagent demand where it’s no longer needed. - Updated BUILDGUIDE and internal docs/instructions to recommend
dotnet build build.proj ...and removed MSBuild-specific wording.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| eng/pipelines/onebranch/steps/roslyn-analyzers-sqlclient-step.yml | Switch Roslyn analyzer build command to dotnet build for build2.proj. |
| eng/pipelines/onebranch/steps/roslyn-analyzers-csproj-step.yml | Switch Roslyn analyzer build command to dotnet build for build.proj. |
| eng/pipelines/onebranch/steps/pack-sqlclient-step.yml | Replace MSBuild task with DotNetCoreCLI@2 build for PackMds. |
| eng/pipelines/onebranch/steps/pack-csproj-step.yml | Replace MSBuild task with DotNetCoreCLI@2 build for pack targets. |
| eng/pipelines/onebranch/steps/build-sqlclient-step.yml | Replace MSBuild task with DotNetCoreCLI@2 build for BuildMds. |
| eng/pipelines/onebranch/steps/build-csproj-step.yml | Replace MSBuild task with DotNetCoreCLI@2 build for csproj build targets. |
| eng/pipelines/common/templates/steps/run-all-tests-step.yml | Convert Windows test orchestration from MSBuild task usage to DotNetCoreCLI@2 build. |
| eng/pipelines/common/templates/steps/ci-project-build-step.yml | Convert project build steps to DotNetCoreCLI@2 build with equivalent properties. |
| eng/pipelines/common/templates/jobs/ci-build-nugets-job.yml | Remove msbuild demand and use DotNetCoreCLI@2 for packing AKV provider. |
| doc/apps/AzureAuthentication/README.md | Remove MSBuild-specific wording for build properties. |
| BUILDGUIDE.md | Update guidance/examples to use dotnet build build.proj ... and adjust prerequisites text. |
| .github/plans/apicompat-ref-assembly-validation.md | Update example commands from dotnet msbuild to dotnet build. |
| .github/instructions/testing.instructions.md | Update testing instructions to use dotnet build build.proj ... and generalize IDE guidance. |
| .github/instructions/architecture.instructions.md | Replace “MSBuild property” wording with “build property”. |
| .github/instructions/ado-pipelines.instructions.md | Rename dotnetVerbosity description away from MSBuild-specific wording. |
| .github/copilot-instructions.md | Update repo build description to reflect dotnet build usage. |
| displayName: 'Create AKV Provider NuGet Package' | ||
| inputs: | ||
| solution: build.proj | ||
| msbuildArchitecture: x64 |
There was a problem hiding this comment.
msBuildArchitecture is irrelevant and has been removed everywhere.
| @@ -154,17 +154,16 @@ steps: | |||
| -p:DotnetPath=${{ parameters.dotnetx86RootPath }} | |||
There was a problem hiding this comment.
The DotnetPath decides which dotnet runtime architecture to use. msBuildArchitecture above wasn't useful here anyway.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4168 +/- ##
==========================================
- Coverage 65.95% 64.31% -1.65%
==========================================
Files 277 272 -5
Lines 42989 65783 +22794
==========================================
+ Hits 28354 42309 +13955
- Misses 14635 23474 +8839
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
98cb968 to
fc203bc
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 19 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
build.proj:559
GetSqlClientPackageVersionCommandswitched fromdotnet msbuildtodotnet build, but the command still uses MSBuild-only switches (-nologo,-verbosity:quiet,-getProperty:...).dotnet builddoesn't reliably accept/forward these unless passed after--, and-getPropertyin particular is intended fordotnet msbuild. This will likely breakPackSqlClientwhen it tries to evaluateSqlClientPackageVersion.
Consider reverting this one command to dotnet msbuild, or use dotnet build ... -- -getProperty:SqlClientPackageVersion -nologo -verbosity:quiet so the switches are forwarded to MSBuild correctly.
<GetSqlClientPackageVersionCommand>
"$(DotnetPath)dotnet" build "$(SqlClientProjectPath)"
-nologo
-verbosity:quiet
-getProperty:SqlClientPackageVersion
- Removed use of MSBuild in pipelines and documentation. - Legacy build.proj still uses it, and will be replaced shortly by build2.proj which uses the dotnet CLI exclusively.
- Rebased to the latest build.proj and updated everything to use dotnet CLI.
- Converted the CI AKV build/pack steps to use build.proj targets.
| -p:PackageVersionSqlClient=${{ parameters.mdsPackageVersion }} | ||
| -p:PackageVersionAkvProvider=${{ parameters.akvPackageVersion }} | ||
|
|
||
| - task: CopyFiles@2 |
There was a problem hiding this comment.
The PackAkvProvider target doesn't put the NuGet files in the $(packagePath), so we copy them there. This is a stop-gap measure until the new CI pipelines build/pack all of the products the same way.
| - name: buildArguments | ||
| value: >- | ||
| --configuration ${{ parameters.buildConfiguration }} | ||
| -p:Configuration=${{ parameters.buildConfiguration }} |
There was a problem hiding this comment.
Specifying the build configuration now consistently uses -p: syntax.
14486bf to
d276965
Compare
Description
Testing