Consolidate .csproj files

[PaulusParssinen]

This PR consolidates some common properties from the .csproj files to common Directory.Build.props file. The build output directory is changed from form bin\$(Platform)\$(Configuration)\$(TFM) to the dotnet CLI default, e.g. bin\AnyCPU\Release\net8.0\ to bin\Release\net8.0\.

This PR also changes all NuGet package versioning to use Central Package Management to allow easier management of 3rd party dependencies in Directory.Packages.props

This PR also updated the Microsoft.* NuGet dependencies to match the latest SDK release batch that is 8.0.0

There's still more consolidation possible, e.g. moving from .nuspec to new SDK style NuGet configuration and combining various properties under flags such as IsTestProject/IsPackable etc. I'll leave that for future.

[PaulusParssinen]

I realized while doing this.. AnyCPU shouldn't be valid concept on .NET (Core) anymore.. It's just using x86 riight?

Random rambling

edit: It seems to switch per build platform..? maybe.

edit: Wait is that why there is a QEMU in docker CI/CD.. edit: nope, they specify arch

final edit: Restoring original path behavior now that I found the Azure CI responsible for NuGet pushes. I'll leave RID tinkering for the future to keep the PR focused

[darrenge]

This PR goes deep and has wide reaching affect as we have private Azure Dev Ops pipelines that we connect to this repo to do security checks, releases etc. It might be a few days before I can approve this.

[TedHartMS]

This PR goes deep and has wide reaching affect as we have private Azure Dev Ops pipelines that we connect to this repo to do security checks, releases etc. It might be a few days before I can approve this.

Assuming no problems here, the changes look good

[PaulusParssinen]

This PR goes deep and has wide reaching affect as we have private Azure Dev Ops pipelines that we connect to this repo to do security checks, releases etc. It might be a few days before I can approve this.

Take all the time that is needed to evaluate this as I'm not familiar with the AzDO parts of the infrastructure 👍

If the PR is too complex and warrants it, I can atleast split it into two; general .csproj consolidation & NuGet CPM (The SDK package upgrades can be split too).

[TedHartMS]

Per #324, please add
bin$(Platform)$(Configuration)
to Directory.Build.props

[PaulusParssinen]

Per #324, please add bin$(Platform)$(Configuration) to Directory.Build.props

Oh well, I think I restored that OutputPath in 2d89e0e for all of the projects already?

[badrishc]

Nice reorganization! Need to make sure none of the pipelines (e.g., garnet-internal-release) or GitHub Actions will be impacted, in case paths have changed.

[PaulusParssinen]

I am getting this error when I run Release ADO pipeline so the paths need to be fixed: "File not found: 'libs\server\bin\AnyCPU\Release\net6.0\Garnet.server.dll"

The Nuget Pack line in the ADO pipeline is: task: NuGetCommand@2 displayName: nuget pack Garnet enabled: True inputs: command: custom arguments: pack Garnet.nuspec -OutputDirectory (Build.ArtifactStagingDirectory)−PropertiesConfiguration=(BuildConfiguration) -Symbols -SymbolPackageFormat snupkg -version $(Build.BuildNumber) -Verbosity Detailed

Can you see where the

garnet/.azure/pipelines/azure-pipelines-internal-release.yml

Lines 45 to 49 in df2a3c3

	- task: DotNetCoreCLI@2
	displayName: dotnet build
	inputs:
	projects: '**/Garnet.*.csproj'
	arguments: -c Release

step is currently placing the built .dlls? Is it somehow ignoring the Directory.Build.props? Is it because it's been explicitly given to only build **/Garnet.*.csproj?

[PaulusParssinen]

Also I fear that the NuGetCommand@2 is using NuGet.exe which is intended for .NET FX projects.

This task also uses NuGet.exe and works with .NET Framework apps. For .NET Core and .NET Standard apps, use the .NET Core task.
If you are working with .NET Core or .NET Standard, use the .NET Core task, which has full support for all package scenarios and is currently supported by dotnet.

So I'm interested how the pipeline worked in the first place 🤔 The .NET Cli step should have built the binaries respecting the <OutputPath> from Directory.Build.props. Does AzDO show whether the CopyFiles copied did anything?

[badrishc]

Does AzDO show whether the CopyFiles copied did anything?

The copy step never copied any files, but that did not prevent the run from succeeding. we did not get to removing that step yet, but that step can be ignored. the subsequent pack operated in the source path, i think.

[darrenge]

I was thinking about this more. The only pipeline that fails is the external pipeline so I am thinking it would just be easier \ better to merge this and then I will have a separate PR that adapts \ cleans up the ADO release pipeline. It would just be easier that way since @PaulusParssinen doesn't have access to running our ADO pipelines.
[EDIT] - actually - let's do it this way. I have a branch based on this fork, so I will fix the release pipeline on that branch and see if there are any issues. This way if there is something funky that we can't get working, we won't have to roll back the whole thing.

[PaulusParssinen]

I was thinking about this more. The only pipeline that fails is the external pipeline so I am thinking it would just be easier \ better to merge this and then I will have a separate PR that adapts \ cleans up the ADO release pipeline. It would just be easier that way since @PaulusParssinen doesn't have access to running our ADO pipelines.

[EDIT] - actually - let's do it this way. I have a branch based on this fork, so I will fix the release pipeline on that branch and see if there are any issues. This way if there is something funky that we can't get working, we won't have to roll back the whole thing.

Do you want me to close this PR or will your fixes be merged to this? Sounds good to me either way 👍

edit: Just adding here that I quickly tried to locally pack the NuGet using dotnet pack and specifying the .nuspec with:

dotnet pack -p:NuspecFile=C:/src/garnet/Garnet.nuspec -p:NuspecProperties="version=1.33.7" -p:NuspecBasePath=C:/src/garnet/ -o ./nuget -c Release

This seems to work and not work as the dotnet pack errors out but still manages to build valid .nupkg.

[darrenge]

Keep this PR as is and I will work on my private branch. After I figure things out, we can merge this and I can do a separate PR (or just add my changes to this PR). For now, don't do anything with this PR. Thanks.

[badrishc]

This seems to work and not work as the dotnet pack errors out

What error do you get?

[PaulusParssinen]

This seems to work and not work as the dotnet pack errors out

What error do you get?

Possibly related: NuGet/Home#4254

Wrote about this earlier on Garnet Discord but here's error in CLI and MSBuild Log viewer:

Essentially MSBuild is trying to pack them in parallel? But it still succeeds even if it errors 😄

If anyone is interested, does the error reproduce?

[darrenge]

Ok ... figured it out.

Nuget.exe pack works for us and it is part of an ADO task that is "blessed" to use, so I think we should just stay with that. The fix to get this PR in is quite simple. You just need to update the Garnet.nuspec. Update the paths for each one from AnyCPU\Release to Release. For example "src="libs\server\bin\AnyCPU\Release\net6.0\Garnet.server.pdb"
" to "src="libs\server\bin\Release\net6.0\Garnet.server.pdb"

For extra credit ... :)
That copy task & $(BuildConfiguration) aren't related to this failure but could be cleaned up.

In the file: "azure-pipelines-external-release.yml", the "Copy Files for Nuget package ..." task and the "$(BuildConfiguration)" setting in the pack task are not needed and can be remove. If you want to see what I did, check out my branch darrenge/consolidate-csproj

[PaulusParssinen]

Ok ... figured it out.

Nuget.exe pack works for us and it is part of an ADO task that is "blessed" to use, so I think we should just stay with that. The fix to get this PR in is quite simple. You just need to update the Garnet.nuspec. Update the paths for each one from AnyCPU\Release to Release. For example "src="libs\server\bin\AnyCPU\Release\net6.0\Garnet.server.pdb" " to "src="libs\server\bin\Release\net6.0\Garnet.server.pdb"

For extra credit ... :) That copy task & $(BuildConfiguration) aren't related to this failure but could be cleaned up.

In the file: "azure-pipelines-external-release.yml", the "Copy Files for Nuget package ..." task and the "$(BuildConfiguration)" setting in the pack task are not needed and can be remove. If you want to see what I did, check out my branch darrenge/consolidate-csproj

Alright, awesome! Thanks for investigating on your end.

So we are okay with removing the "Platform" i.e. AnyCPU from the OutputPath. Is there other pipelines that rely on the OutputPath containing AnyCPU that normally wouldn't be there for .NET Core.

edit: I think there isn't, trying that now 👍

[darrenge]

I only modified the nupsec file and the azure-pipelines-external-release.yml file and it was based on your fork from yesterday, so I can't confirm any other changes will be ok.

[darrenge]

Also - this was the only pipeline that was failing on your PR.

[PaulusParssinen]

Can you now test if any of the AzDO pipelines fail for this?

This change makes all the output paths be consistent and the NuGet.exe in the AzDO shouldn't be putting stuff into different path than local dotnet build would.

[darrenge]

Looks good. Will merge once the checks are finished.

[TedHartMS]

There's still $(Platform) in the DocumentationFile path, and a couple of Tsavorite csprojs

[PaulusParssinen]

There's still $(Platform) in the DocumentationFile path, and a couple of Tsavorite csprojs

Good catch! I removed the explicit paths so the xmldoc files are now placed automatically next to the build artifacts.

I'll leave the potential AzDO touches that @darrenge described as follow-up as I don't have good view what actually happens there.

[darrenge]

Cool - once the CIs finish (assuming they pass), I will merge this PR.

[darrenge]

I am seeing this GitHub Action Docker Image for Linux is now failing. https://github.com/microsoft/garnet/actions/runs/8854384439
@PaulusParssinen -- related to your change?

[darrenge]

FYI ... Badrish and I worked out a fix for this in a separate PR - #335

[PaulusParssinen]

FYI ... Badrish and I worked out a fix for this in a separate PR - #335

Oh, completely forgot about Docker and thought that the Docker CI had already been run during the time this PR was under review.. my bad! Good job figuring it out, seems like it wasn't too complex this time fortunately 👍