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 nuget/nuget.client #35241

Merged
merged 3 commits into from Sep 11, 2023

Conversation

dotnet-maestro[bot]
Copy link
Contributor

@dotnet-maestro dotnet-maestro bot commented Sep 8, 2023

This pull request updates the following dependencies

From https://github.com/nuget/nuget.client

  • Subscription: fcb199f6-ff33-44a0-f3ef-08d8e97c775d
  • Build: 6.8.0.110
  • Date Produced: September 9, 2023 8:49:46 AM UTC
  • Commit: 11a62174beb847bc2a59ffbb08b70eaa84781d25
  • Branch: dev

…6.8.0.104

Microsoft.Build.NuGetSdkResolver , NuGet.Build.Tasks , NuGet.Build.Tasks.Console , NuGet.Build.Tasks.Pack , NuGet.CommandLine.XPlat , NuGet.Commands , NuGet.Common , NuGet.Configuration , NuGet.Credentials , NuGet.DependencyResolver.Core , NuGet.Frameworks , NuGet.LibraryModel , NuGet.Packaging , NuGet.ProjectModel , NuGet.Protocol , NuGet.Versioning
 From Version 6.8.0-preview.1.90 -> To Version 6.8.0-rc.104
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-CodeFlow untriaged Request triage from a team member labels Sep 8, 2023
@marcpopMSFT
Copy link
Member

@dotnet/nuget-team it looks like ya'll updated your newtonsoft version to 13.0.3. We can update in the SDK as well but we usually try to update all together since multiple components that flow into sdk rely on newtonsoft.

@heng-liu
Copy link
Contributor

heng-liu commented Sep 8, 2023

Thanks @marcpopMSFT for the info!
Shall we hold off this PR for now? If yes, may I know when can we get this PR merged?
Or shall we revert the commit of updating newtonsoft.json to 13.0.3 and insert again into SDK with another PR? Thanks!

@zivkan
Copy link
Member

zivkan commented Sep 8, 2023

upgrading newtonsoft.json was requested by the source build team: NuGet/NuGet.Client#5385

and it's already been upgraded in SDK's 8.0.1xx branch: #35008

when it is going to flow into the main branch? I believe that SDK's 8.0.1xx and main branches both currently subscribe to nuget's VS17.8 channel.

@zivkan
Copy link
Member

zivkan commented Sep 8, 2023

It's probably not interesting, but here's the same version of Nuget getting inserted into 8.0.1xx:

Since source build's upgrade of Newtonsoft.Json to 13.0.3 is already in that branch, CI didn't fail for the same reason as the main branch.

@marcpopMSFT
Copy link
Member

@zivkan Thanks for the details. Looks like it was updated in 8.0.1xx while I was out last week.

@mthalman are you ensuring that newtonsoft gets updated in installer, vstest, and anywhere else that needs to update? I believe we have 8 copies on disk including the user-secrets tool and razor folders.

Last time we updated this we coordinated across a bunch of different teams to ensure we all updated at the same time so I want to ensure that happens again. We also try not to flow changes in the eng folder across branches so that could be why the automated codeflow didn't update main to 13.0.3.

Adding @v-wuzhai as we may want to update the version of newtonsoft we use in tests to 13.0.3 as well in 8.0.1xx and main.

@mthalman
Copy link
Member

mthalman commented Sep 8, 2023

@mthalman are you ensuring that newtonsoft gets updated in installer, vstest, and anywhere else that needs to update? I believe we have 8 copies on disk including the user-secrets tool and razor folders.

Yes, you can find all the backreferencing PRs at the bottom of this one: dotnet/source-build-externals#195.

The change I made for sdk was in #35008, which targeted release/8.0.1xx. The version upgrade change in source-build-externals should have failed in the source-build-externals-to-sdk dependency flow for main, but now I see there hasn't been one ever since the .NET 9 channel was created. That's because there is no .NET 9 channel for source-build-externals. @mmitche is going to take care of that next week.

I'll create a PR with the necessary changes to update the version in main.

@mthalman
Copy link
Member

mthalman commented Sep 8, 2023

The change I made for sdk was in #35008, which targeted release/8.0.1xx.

I guess the other question is why didn't this automatically get merged by the flow from release/8.0.1xx to main?

@marcpopMSFT
Copy link
Member

@mthalman are you ensuring that newtonsoft gets updated in installer, vstest, and anywhere else that needs to update? I believe we have 8 copies on disk including the user-secrets tool and razor folders.

Yes, you can find all the backreferencing PRs at the bottom of this one: dotnet/source-build-externals#195.

I don't see a vstest PR in that list. @mthalman we should follow up with them to get vstest updated as well. Also, did you end up testing the SDK layout once all these changes flowed (have they all flowed yet) to ensure that all of the newtonsoft versions on disk are 13.0.3? That would be the best way to confirm we didn't miss anywhere else (besides vstest).

@marcpopMSFT
Copy link
Member

I guess the other question is why didn't this automatically get merged by the flow from release/8.0.1xx to main?

Per my other comment, our guidance to our vendor on merge conflicts in the codeflow is to not merge any changes in the eng folder as typically those should come through codeflow. That's not 100% true (we have build scripts in there and helix scripts and also not everything in the versions.props comes from codeflow) but that guidance works 90+% of the time.

dotnet-maestro bot and others added 2 commits September 9, 2023 15:20
…6.8.0.110

Microsoft.Build.NuGetSdkResolver , NuGet.Build.Tasks , NuGet.Build.Tasks.Console , NuGet.Build.Tasks.Pack , NuGet.CommandLine.XPlat , NuGet.Commands , NuGet.Common , NuGet.Configuration , NuGet.Credentials , NuGet.DependencyResolver.Core , NuGet.Frameworks , NuGet.LibraryModel , NuGet.Packaging , NuGet.ProjectModel , NuGet.Protocol , NuGet.Versioning
 From Version 6.8.0-preview.1.90 -> To Version 6.8.0-rc.110
@dotnet-maestro dotnet-maestro bot merged commit 73b3d39 into main Sep 11, 2023
20 checks passed
@dotnet-maestro dotnet-maestro bot deleted the darc-main-3103abcf-995a-4787-a92f-c38a42b72174 branch September 11, 2023 05:43
@mthalman
Copy link
Member

I don't see a vstest PR in that list. @mthalman we should follow up with them to get vstest updated as well. Also, did you end up testing the SDK layout once all these changes flowed (have they all flowed yet) to ensure that all of the newtonsoft versions on disk are 13.0.3? That would be the best way to confirm we didn't miss anywhere else (besides vstest).

@marcpopMSFT - I don't know why vstest didn't show up there. The change for vstest is here: microsoft/vstest#4681. They were only willing to upgrade the version for source-build. So the Microsoft build is still using 13.0.1 for vstest. Is that a problem?

@marcpopMSFT
Copy link
Member

I don't see a vstest PR in that list. @mthalman we should follow up with them to get vstest updated as well. Also, did you end up testing the SDK layout once all these changes flowed (have they all flowed yet) to ensure that all of the newtonsoft versions on disk are 13.0.3? That would be the best way to confirm we didn't miss anywhere else (besides vstest).

@marcpopMSFT - I don't know why vstest didn't show up there. The change for vstest is here: microsoft/vstest#4681. They were only willing to upgrade the version for source-build. So the Microsoft build is still using 13.0.1 for vstest. Is that a problem?

If there's an eventual security bug, they'll have to scramble to update the version everywhere and people will complain to the SDK about it but for the moment, it's not a problem. They were on version 9.0.1 for a long time before moving forward to 13.0.1 and it wasn't a problem until there was a security update. so I recommend they upgrade but understand why they are hesitant to do so.

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.

None yet

5 participants