search

dotnet / deployment-tools

This repo contains the code to build the .NET deployment tools and installers for all supported platforms, as well as the sources to .NET deployment tools.
MIT License
170 stars 51 forks source link

Source-build should only target NetCurrent #252

Closed NikolaMilosavljevic closed 1 year ago

NikolaMilosavljevic commented 1 year ago

Fixes: https://github.com/dotnet/deployment-tools/issues/217

Microsoft.Deployment.DotNet.Releases is the only project that is building for source-build.

This change limits target TFM to NetCurrent, which, at the moment, is net8.0.

MichaelSimons commented 1 year ago

I don't think this change is necessary. Source-build supports targetting netstandard and netcurrent TFMs. Looking at the latest build of source-build in main, it looks like a terribly old version of deployment-tools is being built. 

<GitCommitHash>c3ad00ae84489071080a606f6a8e43c9a91a5cc2</GitCommitHash>
<OutputPackageVersion>1.0.0-preview5.1.22263.1</OutputPackageVersion>

That version still targets net452 which is why I think #217 was opened.

<TargetFrameworks>net452;netstandard2.0</TargetFrameworks>

I think all we need to do is to flow the latest deployment-tools into 8.0.

NikolaMilosavljevic commented 1 year ago

Sounds good - I was going to remove netstandard2.0 as the ideal goal, mentioned in the issue (https://github.com/dotnet/deployment-tools/issues/217) is to have a single (latest) TFM, and this is an easy fix in deployment-tools repo.

MichaelSimons commented 1 year ago

Having source-build conditionalized logic is not ideal IMO. Since it is unnecessary I see it as a detrimental change to make.

NikolaMilosavljevic commented 1 year ago

Having source-build conditionalized logic is not ideal IMO. Since it is unnecessary I see it as a detrimental change to make.

Makes sense - avoiding special-casing source-build, whenever possible, should be one of the goals. I'll follow that for other repos. Good thing that we clarified netstandard TFM, it will make things simpler.

joeloff commented 1 year ago

We do need .NET standard because this has to support .NET Framework as well and seemed a better option than targeting net472 or net481

NikolaMilosavljevic commented 1 year ago

Closing, as this is not needed. We need to enable automatic flow of deployment-tools to sdk repo.