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

[release/8.0] Update MicrosoftBuildVersion to latest #100595

Merged
merged 3 commits into from
Apr 16, 2024

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Apr 3, 2024

Backport of #98006 to release/8.0

/cc @ericstj @LoopedBard3

Customer Impact

  • Customer reported
  • Found internally

Component governance issue in runtime, due to dependencies MSBuild tasks built in repo.

Regression

  • Yes
  • No

Testing

Built in main.

Risk

Low - build only dependency. Package version does not appear in shipping assets, nor is binary redistributed.

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Apr 3, 2024
@ericstj
Copy link
Member

ericstj commented Apr 3, 2024

@ViktorHofer @MichaelSimons do we need a corresponding SBRP change in servicing to take this? I know we needed changes in main for SBRP when this was made - not sure if we branch that for servicing or not.

I think the SBRP changes were
dotnet/source-build-reference-packages#885
dotnet/source-build-reference-packages#886

@ericstj ericstj requested a review from ViktorHofer April 3, 2024 17:29
@MichaelSimons
Copy link
Member

Yes those will need to be backported. Because of all the infrastructure changes made in 9.0, a simple backport will not work without some changes to the project files.

@ericstj
Copy link
Member

ericstj commented Apr 8, 2024

Source build leg is failing because it needs dotnet/source-build-reference-packages#932.

@MichaelSimons
Copy link
Member

Yes those will need to be backported. Because of all the infrastructure changes made in 9.0, a simple backport will not work without some changes to the project files.

I retract this statement after seeing that 4.8.3 is targeting net8.0. Instead, I think we should build with n-1 in source-build (currently 4.8.5). To do this, we would need to add this dependency in the Version.Detail.xml in order to allow source-build to lift it. See https://github.com/dotnet/runtime/blob/release/8.0/eng/Version.Details.xml#L403.

@ericstj
Copy link
Member

ericstj commented Apr 9, 2024

So it sounds like we need a ToolsetDependencies entry. Can you let me know which version that should be? You're saying 4.8.5 and 4.8.3 - did you mean 17.9.5 and 17.8.3? cc @rainersigwald

@MichaelSimons
Copy link
Member

So it sounds like we need a ToolsetDependencies entry.

Yes.

Can you let me know which version that should be? You're saying 4.8.5 and 4.8.3 - did you mean 17.9.5 and 17.8.3?

Sorry I mixed up the version numbers. I meant 17.8.3 and 17.8.5.

It is fine to use 17.8.3.

To conclude, I think the following toolset dependencies should be added. The second is to prevent prebuilts from being reported in the runtime's source-build leg.

<Dependency Name="Microsoft.Build" Version="17.8.3">
  <Uri>https://github.com/dotnet/msbuild</Uri>
  <Sha>195e7f5a3a8e51c37d83cd9e54cb99dc3fc69c22</Sha>
</Dependency>
<Dependency Name="Microsoft.SourceBuild.Intermediate.msbuild" Version="17.8.3-preview-23613-06">
  <Uri>https://github.com/dotnet/msbuild</Uri>
  <Sha>195e7f5a3a8e51c37d83cd9e54cb99dc3fc69c22</Sha>
  <SourceBuild RepoName="msbuild" ManagedOnly="true" />
</Dependency>

@ericstj
Copy link
Member

ericstj commented Apr 9, 2024

I see - so simply adding those to VersionDetails.xml is enough? Because we're already using MicrosoftBuildVersion for our property sourcebuild will know to set this with the actual version?

@ericstj ericstj added Servicing-consider Issue for next servicing release review area-Infrastructure-libraries area-Infrastructure and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners area-Infrastructure-libraries labels Apr 9, 2024
@MichaelSimons
Copy link
Member

I see - so simply adding those to VersionDetails.xml is enough? Because we're already using MicrosoftBuildVersion for our property sourcebuild will know to set this with the actual version?

That's correct. The VersionDetails.xml entry signals to source-build to use the current version which it does by overriding the Version.props property.

@ericstj
Copy link
Member

ericstj commented Apr 12, 2024

@MichaelSimons -- looks like this MSBuild version did bring some new prebuilts:

System.Configuration.ConfigurationManager.7.0.0
System.Diagnostics.EventLog.7.0.0
System.Reflection.MetadataLoadContext.7.0.0
System.Security.Cryptography.ProtectedData.7.0.0

Should we resurrect the SBRP change for just those?

@ericstj
Copy link
Member

ericstj commented Apr 12, 2024

Looks like if we move to 17.9.5 it would have dependencies on 8.0.0 versions of these packages. I wonder if that's the better way to solve this?

@rbhanda rbhanda added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Apr 16, 2024
@ericstj
Copy link
Member

ericstj commented Apr 16, 2024

@steveisok -- is there any reason to fast-track this into May release, or is it ok to go in June?

@steveisok
Copy link
Member

It resolves a cg alert so taking it now would be preferred.

@ericstj ericstj added this to the 8.0.5 milestone Apr 16, 2024
@ericstj ericstj merged commit dce1737 into release/8.0-staging Apr 16, 2024
184 of 185 checks passed
@ericstj
Copy link
Member

ericstj commented Apr 16, 2024

/backport to release/8.0

Copy link
Contributor Author

Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/8712642220

ericstj added a commit that referenced this pull request Apr 16, 2024
* Update MicrosoftBuildVersion to latest to fix System.Security.Cryptography.XML component governance alert.

* Add info to VersionDetails to allow sourcebuild to update the MSBuild dependency

* Update SourceBuildPrebuiltBaseline.xml

---------

Co-authored-by: Parker Bibus <parkerbibus@microsoft.com>
Co-authored-by: Eric StJohn <ericstj@microsoft.com>
Co-authored-by: Michael Simons <msimons@microsoft.com>
ericstj added a commit that referenced this pull request Apr 16, 2024
* Update MicrosoftBuildVersion to latest to fix System.Security.Cryptography.XML component governance alert.

* Add info to VersionDetails to allow sourcebuild to update the MSBuild dependency

* Update SourceBuildPrebuiltBaseline.xml

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Parker Bibus <parkerbibus@microsoft.com>
Co-authored-by: Eric StJohn <ericstj@microsoft.com>
Co-authored-by: Michael Simons <msimons@microsoft.com>
@jkotas jkotas deleted the backport/pr-98006-to-release/8.0 branch April 17, 2024 01:25
dotnet-bot pushed a commit to dotnet/installer that referenced this pull request May 14, 2024
Fixes: dotnet/source-build#4344

dotnet/runtime#100595 introduced two issues:
- Lower MSBuild version dependency - 17.8.3, instead of 17.8.5. This caused transitive package downgrade errors.
- Source-build dependency for MSBuild, in runtime repo. This caused downgrade of MSBuild repo in VMR, from 17.8.5 to 17.8.3.

Besides removing direct package references, the fix is to add a direct MSBuild source-build dependency in `installer` repo.

I'm also adding the missing MSBuild dependencies to runtime, for proper PVP flow. Without the explicit dependencies, my validation build was hitting an issue with `Microsoft.Build.Framework` package downgrade in
`src/tasks/Microsoft.NET.Sdk.WebAssembly.Pack.Tasks/Microsoft.NET.Sdk.WebAssembly.Pack.Tasks.csproj`:
```
    /vmr/src/runtime/artifacts/source-build/self/src/src/tasks/Microsoft.NET.Sdk.WebAssembly.Pack.Tasks/Microsoft.NET.Sdk.WebAssembly.Pack.Tasks.csproj : error NU1605: Warning As Error: Detected package downgrade: Microsoft.Build.Framework from 17.8.5 to 17.8.3. Reference the package directly from the project to select a different version.  [/vmr/src/runtime/artifacts/source-build/self/src/Build.proj]
    /vmr/src/runtime/artifacts/source-build/self/src/src/tasks/Microsoft.NET.Sdk.WebAssembly.Pack.Tasks/Microsoft.NET.Sdk.WebAssembly.Pack.Tasks.csproj : error NU1605:  Microsoft.NET.Sdk.WebAssembly.Pack.Tasks -> Microsoft.Build 17.8.5 -> Microsoft.Build.Framework (>= 17.8.5)  [/vmr/src/runtime/artifacts/source-build/self/src/Build.proj]
    /vmr/src/runtime/artifacts/source-build/self/src/src/tasks/Microsoft.NET.Sdk.WebAssembly.Pack.Tasks/Microsoft.NET.Sdk.WebAssembly.Pack.Tasks.csproj : error NU1605:  Microsoft.NET.Sdk.WebAssembly.Pack.Tasks -> Microsoft.Build.Framework (>= 17.8.3) [/vmr/src/runtime/artifacts/source-build/self/src/Build.proj]
```

Fully validated with an internal pipeline run: https://dev.azure.com/dnceng/internal/_build/results?buildId=2436369&view=results
@github-actions github-actions bot locked and limited conversation to collaborators May 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Infrastructure Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants