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

[main] Update dependencies from dotnet/msbuild #42641

Merged
merged 21 commits into from
Sep 2, 2024

Conversation

dotnet-maestro[bot]
Copy link
Contributor

@dotnet-maestro dotnet-maestro bot commented Aug 9, 2024

This pull request updates the following dependencies

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

  • Subscription: 51256791-e30b-4b96-f2b9-08daf1d75f3f
  • Build: 20240829.1
  • Date Produced: August 29, 2024 5:02:23 PM UTC
  • Commit: 01e53f4161996ce73408117d012f24c2c1737e58
  • Branch: refs/heads/main

…0809.1

Microsoft.SourceBuild.Intermediate.msbuild , Microsoft.Build , Microsoft.Build.Localization
 From Version 17.12.0-preview-24407-03 -> To Version 17.12.0-preview-24409-01
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-CodeFlow untriaged Request triage from a team member labels Aug 9, 2024
…0809.3

Microsoft.SourceBuild.Intermediate.msbuild , Microsoft.Build , Microsoft.Build.Localization
 From Version 17.12.0-preview-24409-01 -> To Version 17.12.0-preview-24409-03
…0812.1

Microsoft.SourceBuild.Intermediate.msbuild , Microsoft.Build , Microsoft.Build.Localization
 From Version 17.12.0-preview-24409-03 -> To Version 17.12.0-preview-24412-01
@Forgind
Copy link
Member

Forgind commented Aug 12, 2024

@dotnet/kitten @maridematte

I noticed on this line that you changed the name of a constraint on a public type. It looks like it's in the M.B.Experimental namespace, so I don't think the change needs to be reverted, but it seems to be causing trouble with the VMR build:

/vmr/.dotnet/sdk/9.0.100-preview.7.24380.2/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.ApiCompat.ValidatePackage.targets(39,5): error CP0021: (NETCORE_ENGINEERING_TELEMETRY=Build) Cannot remove constraint 'T:Microsoft.Build.Experimental.BuildCheck.AnalysisData' on type parameter 'T' of 'Microsoft.Build.Experimental.BuildCheck.BuildCheckDataContext<T>'.

What do we need to do to resolve that?

…0812.2

Microsoft.SourceBuild.Intermediate.msbuild , Microsoft.Build , Microsoft.Build.Localization
 From Version 17.12.0-preview-24412-01 -> To Version 17.12.0-preview-24412-02
@maridematte
Copy link
Contributor

We renamed our BuildCheck structure from Analyzers to Check, and since it is a public class we would like to keep the consistency with the CheckData name. What I'm wondering is why this is just coming up on the SDK insertion and nowhere else.

…0813.2

Microsoft.SourceBuild.Intermediate.msbuild , Microsoft.Build , Microsoft.Build.Localization
 From Version 17.12.0-preview-24412-02 -> To Version 17.12.0-preview-24413-02
…0815.2

Microsoft.SourceBuild.Intermediate.msbuild , Microsoft.Build , Microsoft.Build.Localization
 From Version 17.12.0-preview-24413-02 -> To Version 17.12.0-preview-24415-02
dotnet-maestro bot and others added 2 commits August 16, 2024 12:04
…0815.4

Microsoft.SourceBuild.Intermediate.msbuild , Microsoft.Build , Microsoft.Build.Localization
 From Version 17.12.0-preview-24415-02 -> To Version 17.12.0-preview-24415-04
@dsplaisted
Copy link
Member

@dotnet/source-build-internal It looks like MSBuild has some API changes in its experimental namespace, and this is failing an API compatibility check in the VMR build. Is it possible to an exception for those APIs for the compat check?

@MichaelSimons
Copy link
Member

@dotnet/source-build-internal It looks like MSBuild has some API changes in its experimental namespace, and this is failing an API compatibility check in the VMR build. Is it possible to an exception for those APIs for the compat check?

The source-build leg appears to be passing. This is a unified build question is better suited for @dotnet/product-construction

@ViktorHofer
Copy link
Member

From what I can tell, this did show up in the PR which is why this file got checked in by @JanKrivanek: dotnet/msbuild@600c4a6

We need to figure out why the suppression isn't honored in the VMR build.

@ViktorHofer
Copy link
Member

Ah i know why. This is a new compatibility analysis rule that was added to APICompat / Package Validation in the .NET 9 timeframe. The MSBuild repo is still on an 8.0 SDK. But inside the VMR, we build everything with a .NET 9 SDK.

When you update your repo to a .NET 9 SDK you will see the error. I recommend to update the suppression file and add these new errors.

dotnet pack src\Build /p:ApiCompatGenerateSuppressionFile=true with a .NET 9 SDK (ideally the latest released one, P7)

@dsplaisted
Copy link
Member

Ah i know why. This is a new compatibility analysis rule that was added to APICompat / Package Validation in the .NET 9 timeframe. The MSBuild repo is still on an 8.0 SDK. But inside the VMR, we build everything with a .NET 9 SDK.

When you update your repo to a .NET 9 SDK you will see the error. I recommend to update the suppression file and add these new errors.

dotnet pack src\Build /p:ApiCompatGenerateSuppressionFile=true with a .NET 9 SDK (ideally the latest released one, P7)

If I'm understanding correctly, this needs to be done in MSBuild. /cc @dotnet/kitten

@ViktorHofer
Copy link
Member

Correct

@rainersigwald
Copy link
Member

I think dotnet/msbuild#10484 will have taken care of this. @MichalPavlik is anything blocking that?

…0821.1

Microsoft.SourceBuild.Intermediate.msbuild , Microsoft.Build , Microsoft.Build.Localization
 From Version 17.12.0-preview-24415-04 -> To Version 17.12.0-preview-24421-01
@akoeplinger
Copy link
Member

should we add a VMR patch in the meantime?

…0822.1

Microsoft.SourceBuild.Intermediate.msbuild , Microsoft.Build , Microsoft.Build.Localization
 From Version 17.12.0-preview-24421-01 -> To Version 17.12.0-preview-24422-01
@MichalPavlik
Copy link
Member

I think dotnet/msbuild#10484 will have taken care of this. @MichalPavlik is anything blocking that?

IIRC dotnet/arcade#14991 needs to be resolved. @f-alizada has more info.

…0823.4

Microsoft.SourceBuild.Intermediate.msbuild , Microsoft.Build , Microsoft.Build.Localization
 From Version 17.12.0-preview-24422-01 -> To Version 17.12.0-preview-24423-04
…0823.8

Microsoft.SourceBuild.Intermediate.msbuild , Microsoft.Build , Microsoft.Build.Localization
 From Version 17.12.0-preview-24423-04 -> To Version 17.12.0-preview-24423-08
…0826.1

Microsoft.SourceBuild.Intermediate.msbuild , Microsoft.Build , Microsoft.Build.Localization
 From Version 17.12.0-preview-24423-08 -> To Version 17.12.0-preview-24426-01
@f-alizada
Copy link
Contributor

Hello. The PR is in progress: dotnet/msbuild#10484
The conflicts and PR reviews are being addressed at the moment.
I would say that the completion % is 95

@dsplaisted
Copy link
Member

should we add a VMR patch in the meantime?

It sounds like the VMR patch would just need to be the updated suppression file from the MSBuild PR. On the other hand it sounds like the MSBuild PR itself is close to being merged.

dotnet-maestro bot and others added 5 commits August 27, 2024 12:39
…0827.1

Microsoft.SourceBuild.Intermediate.msbuild , Microsoft.Build , Microsoft.Build.Localization
 From Version 17.12.0-preview-24426-01 -> To Version 17.12.0-preview-24427-01
…0828.6

Microsoft.SourceBuild.Intermediate.msbuild , Microsoft.Build , Microsoft.Build.Localization
 From Version 17.12.0-preview-24427-01 -> To Version 17.12.0-preview-24428-06
…0829.1

Microsoft.SourceBuild.Intermediate.msbuild , Microsoft.Build , Microsoft.Build.Localization
 From Version 17.12.0-preview-24428-06 -> To Version 17.12.0-preview-24429-01
@rainersigwald
Copy link
Member

The .NET 9 PR is in but required a couple of changes on the SDK side in #43078. We either need to port them or wait for the merge #43065.

cc 🐈 @MichalPavlik (today)/ @maridematte (next month)

@f-alizada
Copy link
Contributor

The .NET 9 PR is in but required a couple of changes on the SDK side in #43078. We either need to port them or wait for the merge #43065.

I was not aware of that, my bad. Could you please point to the changes you are refering to? I was looking at the:

<!-- We explicitly reference an older version of MSBuild here to support VS
for Mac and other VS scenarios. During source-build, we only have access to
the latest version, which targets NetCurrent. -->
<PropertyGroup>
projects description.
Is it expected that the targeted framework to be .net8 for the project?

@rainersigwald
Copy link
Member

No, that's a change that needs to be pulled here--we don't have to support VSMac as of tomorrow :)

@v-codyguan v-codyguan merged commit 7e3dcdc into main Sep 2, 2024
41 checks passed
@v-codyguan v-codyguan deleted the darc-main-4def985b-323a-4bc9-81dc-c5c9ebfd4bd7 branch September 2, 2024 06:12
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.