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

Update to Arcade 8 #8672

Merged
merged 16 commits into from
Jul 14, 2023
Merged

Update to Arcade 8 #8672

merged 16 commits into from
Jul 14, 2023

Conversation

rainersigwald
Copy link
Member

@rainersigwald rainersigwald commented Apr 17, 2023

Fixes #8969

  • Make IDE0251 a suggestion
  • Arcade 8
  • Put expected and actual in the right places for some tlbimp tests
  • Explicit System.IO.Compression reference

@JanKrivanek
Copy link
Member

Naive question - can the eng\common* changes associated with the arcade update be made as a maestro flow PR from arcade? (with all other fixes needed for it to go through to be added to that PR)

At least those changes comes from arcade (are not msbuild specific) and hence do not need to be reviewed in detail - is that correct?

@rainersigwald
Copy link
Member Author

I generated the big change in this PR with a darc update-dependencies command that does locally what the darc subscriptions would create a PR for--I expected this change to break so I didn't want to change the subscription and break our 6.0 flow. You're right that eng/common/* belongs to Arcade and doesn't need much in the way of review.

But this is broken so don't review it at all yet :)

@JaynieBai
Copy link
Member

/azp run

@azure-pipelines
Copy link

Pull request contains merge conflicts.

@JanKrivanek
Copy link
Member

@ViktorHofer we are trying to update MSBuild TFM and Arcade version both to net8.0 and currently we are stuck at

##[error].dotnet\sdk\8.0.100-preview.3.23178.7\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.ApiCompat.ValidatePackage.targets(39,5): error PKV006: (NETCORE_ENGINEERING_TELEMETRY=Build) Target framework net7.0 is no longer supported in the latest version..

Should we temporarily disable the package validation, produce a successfull build (targetting net8.0) and then point baseline to that successfull build? Or is there alternative suggestion how to satisfy baseline validation check with our current baseline (targetting net7.0)?

FYI @JaynieBai, @GangWang01

@ViktorHofer
Copy link
Member

ViktorHofer commented Jul 10, 2023

Should we temporarily disable the package validation, produce a successfull build (targetting net8.0) and then point baseline to that successfull build? Or is there alternative suggestion how to satisfy baseline validation check with our current baseline (targetting net7.0)?

This is telling you that you are dropping a TFM in your packages which is considered a breaking change. If that's intentional, you can suppress these errors by updating the suppression file via the following flag: dotnet build /p:GenerateCompatibilitySuppressionFile=true. Note that with .NET 7, the flag got renamed to dotnet build /p:ApiCompatGenerateSuppressionFile=true but the old one still works.

Note that you could also NoWarn the error but that would disable the validation entirely for all TFM changes.

Copy link
Member

@JanKrivanek JanKrivanek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to go to me

It seems that only missing changes to satisfy the #8969 as well are updates to readmes:

  • src/Framework/README.md
  • src/Utilities/README.md

It's a minimal and nonfunctional change - please bundle it into this PR

@JanKrivanek JanKrivanek marked this pull request as ready for review July 11, 2023 06:29
@GangWang01 GangWang01 mentioned this pull request Jul 11, 2023
Directory.Build.props Outdated Show resolved Hide resolved
@GangWang01 GangWang01 self-assigned this Jul 12, 2023
src/Utilities/README.md Outdated Show resolved Hide resolved
src/Build/CompatibilitySuppressions.xml Show resolved Hide resolved
global.json Show resolved Hide resolved
@GangWang01 GangWang01 force-pushed the arcade8 branch 2 times, most recently from 35b7fa8 to 1aa2d03 Compare July 13, 2023 09:35
@GangWang01
Copy link
Member

After I pushed the commit that resolved review comment, the new run failed. So I reverted the commit back. The run still failed with the error below. I'm looking into it.

stage1\bin\bootstrap\net8.0\MSBuild\NuGet.RestoreEx.targets#L19
stage1\bin\bootstrap\net8.0\MSBuild\NuGet.RestoreEx.targets(19,5): error : (NETCORE_ENGINEERING_TELEMETRY=Restore) Static graph-based restore encountered an unhandled exception. Please file an issue at https://github.com/NuGet/Home.  The exception was:
System.IO.FileNotFoundException: Could not load file or assembly 'System.Runtime, Version=8.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'. The system cannot find the file specified.
File name: 'System.Runtime, Version=8.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'
   at NuGet.Build.Tasks.Console.MSBuildStaticGraphRestore..ctor(Boolean debug)
   at NuGet.Build.Tasks.Console.Program.Main(String[] args)

@JanKrivanek
Copy link
Member

After I pushed the commit that resolved review comment, the new run failed. So I reverted the commit back. The run still failed with the error below. I'm looking into it.

stage1\bin\bootstrap\net8.0\MSBuild\NuGet.RestoreEx.targets#L19
stage1\bin\bootstrap\net8.0\MSBuild\NuGet.RestoreEx.targets(19,5): error : (NETCORE_ENGINEERING_TELEMETRY=Restore) Static graph-based restore encountered an unhandled exception. Please file an issue at https://github.com/NuGet/Home.  The exception was:
System.IO.FileNotFoundException: Could not load file or assembly 'System.Runtime, Version=8.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'. The system cannot find the file specified.
File name: 'System.Runtime, Version=8.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'
   at NuGet.Build.Tasks.Console.MSBuildStaticGraphRestore..ctor(Boolean debug)
   at NuGet.Build.Tasks.Console.Program.Main(String[] args)

Looks like clash with #8856

global.json Outdated Show resolved Hide resolved
Copy link
Member Author

@rainersigwald rainersigwald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't formally approve because I started the PR but this looks good to me, let's get it in on Monday @rokonec :)

@rainersigwald rainersigwald enabled auto-merge (squash) July 14, 2023 15:32
@rainersigwald rainersigwald merged commit 1e7d9a4 into dotnet:main Jul 14, 2023
@rainersigwald rainersigwald deleted the arcade8 branch July 14, 2023 16:24
YuliiaKovalova pushed a commit to YuliiaKovalova/msbuild that referenced this pull request Jul 18, 2023
* Make IDE0251 a suggestion

It fires in some very funky places like Dispose methods
where it's not _wrong_ but it would be _weird_.

* Put expected and actual in the right places for some tlbimp tests

* Explicit System.IO.Compression reference on framework

* Suppress the error PKV006 for Net 7.0, since we're intentionally dropping it

Co-authored-by: Jenny Bai <v-jennybai@microsoft.com>
Co-authored-by: Gang Wang <v-gaw@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update to net8.0
5 participants