Skip to content

Conversation

@mmitche
Copy link
Member

@mmitche mmitche commented Jan 26, 2023

After the sleet removal, we should be able to remove a long standing workound that required that the arcade SDK load a very old version on NuGet.Versioning because of a conflicting dependency with sleet. I have unified the references under NuGet.Packaging's version, so that source-build can properly override the NuGet version.

The relevant issues around the old workaround are #6014 and #1965

To double check:

After the sleet removal, we should be able to remove a long standing workound that required that the arcade SDK load a very old version on NuGet.Versioning because of a conflicting dependency with sleet.  I have unified the references under NuGet.Packaging's version, so that source-build can properly override the NuGet version.

The relevant issues around the old workaround are dotnet#6014 and dotnet#1965
@mmitche mmitche requested review from MattGal and chcosta January 26, 2023 18:27
@mmitche mmitche enabled auto-merge (squash) January 26, 2023 18:45
@mmitche mmitche merged commit 3900335 into dotnet:main Jan 26, 2023
@dougbu
Copy link
Contributor

dougbu commented Aug 24, 2023

looking at release/6.0 and release/7.0, we still have $(NuGetVersion) and $(NuGetVersioningVersion) in both branches. any objection to cleaning that up (using just $(NuGetPackagingVersion) as you did here) and closing the old issues❔

@mmitche
Copy link
Member Author

mmitche commented Aug 24, 2023

I don't think I removed the sleet dependency out of 6 and 7, so it may still be required there.

@dougbu
Copy link
Contributor

dougbu commented Aug 24, 2023

oh well.

@dougbu
Copy link
Contributor

dougbu commented Aug 24, 2023

is it a massive job to remove the sleet dependency❔ we're on the latest version of DotNet.SleetLib. it's from late 2019 and has some very old dependencies.

@mmitche
Copy link
Member Author

mmitche commented Aug 24, 2023

Unsure. But probably. The way it got removed in main was a recognition that we no longer hit any of those code paths during Maestro publishing. This would include 6.0 and 7.0, because they use the main branch of arcade when publishing. So no one was using sleet in our main publishing codepaths (what happens when you do darc add-build-to-channel or have a default channel), so it could be removed.

Now, my hunch is that in repos that depend on the 6.0 and 7.0 branches, the only active codepaths in the Build.Tasks.Feed package are those rooted in the PushToAzureDevOpsArtifacts task (the thing that sets up the publishing manifest and pushes to the build artifacts). Everything else is dead as it was generally from older publishing infra in the first place. Thus, the hypothesis is that if you eliminated everything but PushToAzureDevOpsArtifacts, you'd eliminate the sleet dependency and most of the code, and everything will still work just fine and dandy. Then you can remove the NuGet workaround.

Now, how to find out if this will be a problem? A bit of AzDO searching for any use of targets or tasks from that package, cross referenced with the arcade SDK the repo is on would be my first start. I'm guessing that turns out nothing obvious. The next thing I would do if it all looks good to go is remove all the code paths, leaving behind shells of tasks/targets that spit user-helpful errors ("@dougbu did this to you.")

@mmitche
Copy link
Member Author

mmitche commented Aug 24, 2023

image

@dougbu
Copy link
Contributor

dougbu commented Aug 25, 2023

@mmitche I'm now confused about both release/6.0 and release/7.0. $(NuGetVersioningVersion) is used only in Microsoft.DotNet.Arcade.Sdk.csproj and its tests. Sleetlib is only imported in Microsoft.DotNet.Build.Tasks.Feed.csproj and its tests. Those two projects don't import each other, even indirectly. Please help me understand how SleetLib requires the $(NuGetVersioningVersion) workaround.


We have other cases where our NuGet.Versioning version is held back for unknown reasons:

  • Microsoft.DotNet.Helix.Sdk.csproj depends on v6.0.0-preview.4.230
  • Microsoft.DotNet.Helix.Sdk.Tests.csproj depends on v6.1.0
  • Microsoft.DotNet.SignCheckLibrary depends on v4.7.0

@joeloff is there a reason Microsoft.DotNet.SignCheckLibrary uses old versions of many packages❔

dougbu added a commit to dougbu/arcade that referenced this pull request Aug 30, 2023
- small part of dotnet/dnceng#345
- bump Azure.Data.Tables to v12.8.1
- replace Microsoft.IdentityModel.Clients.ActiveDirectory with Microsoft.Identity.Client
  - part of dotnet/dnceng#629
- bump NuGet.* to v6.7.0
  - part of https://dev.azure.com/dnceng/internal/_workitems/edit/5128
  - part of https://dev.azure.com/dnceng/internal/_workitems/edit/5129
  - left both `$(NuGetVersioningVersion)` and `$(NuGetVersion)` to minimize disruption
    - see dotnet#12333 change in 'main' for a counter-example
  - react to deprecation of `ContentItemCollection.FindItemGroups(...)`
  - react to removal of `SignatureVerificationProviderFactory`
  - react to `VerifySignaturesResult` API changes (`Valid` to `IsValid`)
- bump NewtonSoft.Json to v13.0.3
- nits:
  - remove unused `$(...Version)` properties
  - remove a bit of dead code and unused `using`s in changed files
dougbu added a commit to dougbu/arcade that referenced this pull request Aug 30, 2023
- small part of dotnet/dnceng#345
- bump Azure.Data.Tables to v12.8.1
- replace Microsoft.IdentityModel.Clients.ActiveDirectory with Microsoft.Identity.Client
  - part of dotnet/dnceng#629
- bump NuGet.* to v6.7.0
  - part of https://dev.azure.com/dnceng/internal/_workitems/edit/5127
  - part of https://dev.azure.com/dnceng/internal/_workitems/edit/5128
  - part of https://dev.azure.com/dnceng/internal/_workitems/edit/5129
  - part of https://dev.azure.com/dnceng/internal/_workitems/edit/5137
  - left both `$(NuGetVersioningVersion)` and `$(NuGetVersion)` to minimize disruption
    - see dotnet#12333 change in 'main' for a counter-example
  - react to deprecation of `ContentItemCollection.FindItemGroups(...)`
  - react to removal of `SignatureVerificationProviderFactory`
  - react to `VerifySignaturesResult` API changes (`Valid` to `IsValid`)
- bump NewtonSoft.Json to v13.0.3
- nits:
  - remove unused `$(...Version)` properties
  - remove a bit of dead code and unused `using`s in changed files
dougbu added a commit to dougbu/arcade that referenced this pull request Aug 30, 2023
- small part of dotnet/dnceng#345
- bump Azure.Data.Tables to v12.8.1
- replace Microsoft.IdentityModel.Clients.ActiveDirectory with Microsoft.Identity.Client
  - part of dotnet/dnceng#629
- bump NuGet.* to v6.7.0
  - part of https://dev.azure.com/dnceng/internal/_workitems/edit/5128
  - part of https://dev.azure.com/dnceng/internal/_workitems/edit/5129
  - left both `$(NuGetVersioningVersion)` and `$(NuGetVersion)` to minimize disruption
    - see dotnet#12333 change in 'main' for a counter-example
  - react to deprecation of `ContentItemCollection.FindItemGroups(...)`
  - react to removal of `SignatureVerificationProviderFactory`
  - react to `VerifySignaturesResult` API changes (`Valid` to `IsValid`)
- bump NewtonSoft.Json to v13.0.3
- nits:
  - remove unused `$(...Version)` properties
  - remove a bit of dead code and unused `using`s in changed files
mmitche pushed a commit that referenced this pull request Sep 14, 2023
- small part of dotnet/dnceng#345
- bump Azure.Data.Tables to v12.8.1
- replace Microsoft.IdentityModel.Clients.ActiveDirectory with Microsoft.Identity.Client
  - part of dotnet/dnceng#629
- bump NuGet.* to v6.7.0
  - part of https://dev.azure.com/dnceng/internal/_workitems/edit/5128
  - part of https://dev.azure.com/dnceng/internal/_workitems/edit/5129
  - left both `$(NuGetVersioningVersion)` and `$(NuGetVersion)` to minimize disruption
    - see #12333 change in 'main' for a counter-example
  - react to deprecation of `ContentItemCollection.FindItemGroups(...)`
  - react to removal of `SignatureVerificationProviderFactory`
  - react to `VerifySignaturesResult` API changes (`Valid` to `IsValid`)
- bump NewtonSoft.Json to v13.0.3
- nits:
  - remove unused `$(...Version)` properties
  - remove a bit of dead code and unused `using`s in changed files
mmitche pushed a commit that referenced this pull request Sep 14, 2023
- small part of dotnet/dnceng#345
- bump Azure.Data.Tables to v12.8.1
- replace Microsoft.IdentityModel.Clients.ActiveDirectory with Microsoft.Identity.Client
  - part of dotnet/dnceng#629
- bump NuGet.* to v6.7.0
  - part of https://dev.azure.com/dnceng/internal/_workitems/edit/5127
  - part of https://dev.azure.com/dnceng/internal/_workitems/edit/5128
  - part of https://dev.azure.com/dnceng/internal/_workitems/edit/5129
  - part of https://dev.azure.com/dnceng/internal/_workitems/edit/5137
  - left both `$(NuGetVersioningVersion)` and `$(NuGetVersion)` to minimize disruption
    - see #12333 change in 'main' for a counter-example
  - react to deprecation of `ContentItemCollection.FindItemGroups(...)`
  - react to removal of `SignatureVerificationProviderFactory`
  - react to `VerifySignaturesResult` API changes (`Valid` to `IsValid`)
- bump NewtonSoft.Json to v13.0.3
- nits:
  - remove unused `$(...Version)` properties
  - remove a bit of dead code and unused `using`s in changed files
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.

4 participants