Skip to content

[main] Update dependencies from dotnet/msbuild#48212

Closed
dotnet-maestro[bot] wants to merge 5 commits intomainfrom
darc-main-4f7fbd23-80ff-4420-b507-6b958c8cd321
Closed

[main] Update dependencies from dotnet/msbuild#48212
dotnet-maestro[bot] wants to merge 5 commits intomainfrom
darc-main-4f7fbd23-80ff-4420-b507-6b958c8cd321

Conversation

@dotnet-maestro
Copy link
Contributor

@dotnet-maestro dotnet-maestro bot commented Apr 7, 2025

This pull request updates the following dependencies

From https://github.com/dotnet/msbuild

…0406.1

Microsoft.SourceBuild.Intermediate.msbuild , Microsoft.Build , Microsoft.Build.Localization
 From Version 17.15.0-preview-25204-04 -> To Version 17.15.0-preview-25206-01
@ghost ghost added Area-CodeFlow untriaged Request triage from a team member labels Apr 7, 2025
@dotnet-maestro
Copy link
Contributor Author

dotnet-maestro bot commented Apr 7, 2025

Notification for subscribed users from https://github.com/dotnet/msbuild:

@dotnet/kitten

Action requested: Please take a look at this failing automated dependency-flow pull request's checks; failures may be related to changes which originated in your repo.

  • This pull request contains changes from your source repo (https://github.com/dotnet/msbuild) and seems to have failed checks in this PR. Please take a peek at the failures and comment if they seem relevant to your changes.
  • If you're being tagged in this comment it is due to an entry in the related Maestro Subscription of the Build Asset Registry. If you feel this entry has added your GitHub login or your GitHub team in error, please update the subscription to reflect this.
  • For more details, please read the Arcade Darc documentation

@v-wuzhai
Copy link
Contributor

v-wuzhai commented Apr 7, 2025

@rainersigwald Could you take a look at the failures here?

@JanProvaznik
Copy link
Member

Microsoft.IO.Redist was upgraded in msbuild to 6.1.0, while in sdk it's defined as 6.0.1 here

<MicrosoftIORedistPackageVersion>6.0.1</MicrosoftIORedistPackageVersion>

probably should be bumped

@ericstj
Copy link
Member

ericstj commented Apr 7, 2025

Why does the SDK even have MicrosoftIORedistPackageVersion? It shouldn't be referencing this package at all. I bet that was added to workaround servicing update. Might be better just to remove it.

@akoeplinger
Copy link
Member

It was added in #42317

@ericstj
Copy link
Member

ericstj commented Apr 7, 2025

It was added in #42317

Yeah, if we now have an MSBuild with a newer version then lets just delete that reference.

@rainersigwald
Copy link
Member

@marcpopMSFT Are you aiming for "comprehensive transitive pinning so we should bump the version in SDK to match MSBuild" or ok with "delete the pinned version here because MSBuild's is high enough now"?

@marcpopMSFT
Copy link
Member

@marcpopMSFT Are you aiming for "comprehensive transitive pinning so we should bump the version in SDK to match MSBuild" or ok with "delete the pinned version here because MSBuild's is high enough now"?

@rainersigwald how likely are we to see the version needed go ahead of MSBuild? I'd been going with the theory that the SDK of all places needed comprehensive transitive pinning because we have so many upstream dependencies we don't control that may not update frequently enough.

@ericstj
Copy link
Member

ericstj commented Apr 7, 2025

IMO you shouldn't even be touching this package. The SDK doesn't even ship it. The only time it ever appears is as a transitive reference of a task assembly built for netfx. The task doesn't ship MSBuild. So long as you make sure you mark all those references to MSBuild as compile only CG won't tell you to update any more.

@ericstj
Copy link
Member

ericstj commented Apr 7, 2025

I think the build failures here are due to tasks compiling against the latest inserted MSBuild but running against an older machine-wide MSBuild (without redirects for new dependencies).

Failure is occurring in ReadStaticWebAssetsManifestFile which is loaded from
// C:\scratch\sdk48212\correlation-payload\d\sdk\10.0.100-ci\Sdks\Microsoft.NET.Sdk.StaticWebAssets\tasks\net472\Microsoft.NET.Sdk.StaticWebAssets.Tasks.dll

Which references
// System.Memory, Version=4.0.2.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51

However the MSBuild being used, (at least on my machine) only redirects to 4.0.1.2.

      <dependentAssembly>
        <assemblyIdentity name="System.Memory" publicKeyToken="cc7b13ffcd2ddd51" culture="neutral" />
        <bindingRedirect oldVersion="0.0.0.0-4.0.1.2" newVersion="4.0.1.2" />
      </dependentAssembly>

@ericstj
Copy link
Member

ericstj commented Apr 7, 2025

And that update to System.Memory is coming from Microsoft.IO.Redist update:

      "Microsoft.IO.Redist/6.1.0": {
        "type": "package",
        "dependencies": {
          "System.Buffers": "4.6.0",
          "System.Memory": "4.6.0"
        },
        "compile": {
          "lib/net472/Microsoft.IO.Redist.dll": {
            "related": ".xml"
          }
        },
        "runtime": {
          "lib/net472/_._": {
            "related": ".xml"
          }
        }
      },

https://dev.azure.com/dnceng-public/_apis/resources/Containers/46031915/FullFramework_windows_x64?itemPath=FullFramework_windows_x64%2Flog%2FRelease%2FBuild.binlog

Which this change is applying regardless of Microsoft.Build version (minimum or latest). So the change I suggested above will coincidentally fix this break.

ericstj and others added 3 commits April 7, 2025 14:16
IO.Redist is only a dependency of MSBuild on .NETFramework.  It is not redistributed by SDK.  It should not be referenced at all by the SDK.
…0407.13

Microsoft.SourceBuild.Intermediate.msbuild , Microsoft.Build , Microsoft.Build.Localization
 From Version 17.15.0-preview-25204-04 -> To Version 17.15.0-preview-25207-13
@ViktorHofer ViktorHofer closed this Apr 8, 2025
@ViktorHofer ViktorHofer deleted the darc-main-4f7fbd23-80ff-4420-b507-6b958c8cd321 branch April 8, 2025 08:39
@ViktorHofer
Copy link
Member

Closing in favor of #48251

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

Labels

Area-CodeFlow untriaged Request triage from a team member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants