Discussion for methods of Including certain PackageReference items "By Default"

[leusbj]

During evaluation of Pull Request several discussion points arose that should have some discussion before being incorporated into this Sdk.

Let's use this forum to discuss the best way to incorporate certain desirable PackageReference items. And choose the mechanism that combines backward compatibility with this Sdk's existing control properties, providing reasonable values for the Version metadata attribute even when centralized versioning is being utilized by a consuming project repo (Both CentralPackageVersioning as well as CentralPackageManagement), and potentially following patterns found in Microsoft.NET.Sdk props/targets files.

Area Of Concern	Ideas
Control Version metadata	This Sdk currently offers consuming projects the chance to alter the version of the PackageReference via a named Property. This means the adjustment can set just about anywhere in the project repo (the project file itself, Directory.Build.props, or Directory.Build.targets, or many other mechanisms) even when not using centralized versioning since Property Evaluation is calculated prior to ItemGroup evaluation. Action: Maintain this capability, both from backward compatibility for consuming projects as they upgrade, as well as it is an easier mechanism for consuming projects rather than making them edit/update an ItemGroup.
Some vs All Packages	This Sdk offers the ability to participate in "some but not all" packages when NOT using a centralized version mechanism, by setting a specific package's named property to blank (empty string). Action: This seems to be an unintended side-effect rather than a feature, and can/should be removed going forward?
Temporary ItemGroup	The Microsoft.NET.Sdk.BeforeCommon.targets snippet below uses a temporary ItemGroup to accumulate References that should be included into the project. Action: Similarly this Sdk should collect those packages that should eventually be added to the well known PackageReference ItemGroup
Evaluation Timing	The Microsoft.NET.Sdk.BeforeCommon.targets snippet below occurs "after" the project file, so it can prevent duplication by removing items from the temporary ItemGroup that were directly included by the project file prior to adding items into the well known ItemGroup. Action: Similarly this Sdk can/should prevent duplicates by incorporating items into the well known PackageReference ItemGroup "after" the project file and "before" the Microsoft.NET.Sdk/Sdk.targets. Is there preference to accumulate these items "early" in a props file, or would it be fine to accumulate them "immediately" before including them into the well known group like is done in Microsoft.NET.Sdk.BeforeCommon.targets?
TargetFrameworkIdentity Compatibility	The Microsoft.NET.Sdk.BeforeCommon.targets snippet below includes guard conditions to take action only when building a .NETFramework project. Action: However unlikely that anyone would use this Sdk to attempt to build a "Standard" or "Core" app, it wouldn't hurt to include similar protection into this Sdk's implementation, but is there any desire to add this?
TargetFrameworkVersion Compatibility	The Microsoft.NET.Sdk.BeforeCommon.targets snippet below uses some logic for build Compatibility against previous versions of .NETFramework (as certain dlls were incorporated with different versions of the Framework). Action: Only 4.6.2+ is still "In Support" and for the most part the "OOB" VS Project templates for Web Application Projects have been consistent since that time. Is there any desire to do any Framework Version checking prior to incorporating certain packages?
Centralized Versioning - Defaults	Currently this Sdk detects if Centralized Versioning Mechanism is being used, and does not set the Version metadata attribute of certain packages to prevent the Centralized Versioning tooling from generating errors about setting it directly when Centralized Versioning is in place. Action: This Sdk should detect which Centralized Versioning mechanism is being used and provide the Version metadata in the appropriate style to match the detected mechanism (careful not to duplicate/overwrite any Version properties provided by the project repo).
Centralized Versioning - Timing	Action: By applying Version metadata "after" Microsoft.NET.Sdk/Sdk.targets (and thus after the packages have been included in the well known PackageReference ItemGroup), this Sdk can detect the Version applied by the in place Centralized Versioning tooling, and can avoid overriding or duplicating any rules already in place by the project repo.
Centralized Versioning - Messages	Action: This Sdk should provide Messaging (Not Build Failures) that it has detected Centralized Versioning tooling, and did provide a default, but that project repo owner should consider explicitly adding Version metadata for specific packages into the right place. Poposed Message would be like "The PackageReference '{PackageName}' was included by the SystemWeb Sdk defaulting to Version='{DefaultVersionApplied}'. Centralized management of package versions was detected, consider adding this entry '{Tooling Specific Verbiage}' to your centralized package version listing." with a link to the appropriate file

Microsoft.NET.Sdk.BeforeCommon.targets has an example/basis from which we could start.

  <!-- Snippet as found in Microsoft.NET.Sdk.BeforeCommon.targets 5.0.4.16-->
  <ItemGroup Condition=" '$(DisableImplicitFrameworkReferences)' != 'true' and '$(TargetFrameworkIdentifier)' == '.NETFramework'">

    <_SDKImplicitReference Include="System"/>
    <_SDKImplicitReference Include="System.Data"/>
    <_SDKImplicitReference Include="System.Drawing"/>
    <_SDKImplicitReference Include="System.Xml"/>

    <!-- When doing greater than/less than comparisons between strings, MSBuild will try to parse the strings as Version objects and compare them as
         such if the parse succeeds. -->

    <!-- Framework assemblies introduced in .NET 3.5 -->
    <_SDKImplicitReference Include="System.Core" Condition=" '$(_TargetFrameworkVersionWithoutV)' >= '3.5' "/>
    <_SDKImplicitReference Include="System.Runtime.Serialization" Condition=" '$(_TargetFrameworkVersionWithoutV)' >= '3.5' "/>
    <_SDKImplicitReference Include="System.Xml.Linq" Condition=" '$(_TargetFrameworkVersionWithoutV)' >= '3.5' "/>

    <!-- Framework assemblies introduced in .NET 4.0 -->
    <_SDKImplicitReference Include="System.Numerics" Condition=" '$(_TargetFrameworkVersionWithoutV)' >= '4.0' "/>

    <!-- Framework assemblies introduced in .NET 4.5 -->
    <_SDKImplicitReference Include="System.IO.Compression.FileSystem" Condition=" '$(_TargetFrameworkVersionWithoutV)' >= '4.5' "/>

    <!-- Don't automatically reference System.IO.Compression or System.Net.Http to help avoid hitting https://github.com/Microsoft/msbuild/issues/1329. -->
    <!--<Reference Include="System.IO.Compression" Condition=" '$(_TargetFrameworkVersionWithoutV)' >= '4.5' "/>
    <_SDKImplicitReference Include="System.Net.Http" Condition=" '$(_TargetFrameworkVersionWithoutV)' >= '4.5' "/>-->

    <_SDKImplicitReference Update="@(_SDKImplicitReference)"
                           Pack="false"
                           IsImplicitlyDefined="true" />

    <!-- Don't duplicate any references that are explicit in the project file.  This means that if you do want to include a framework assembly in your
         NuGet package, you can just add the Reference to your project file. -->
    <_SDKImplicitReference Remove="@(Reference)" />

    <!-- Add the implicit references to @(Reference) -->
    <Reference Include="@(_SDKImplicitReference)" />
  </ItemGroup>

Looking for any feedback and comments

[CZEMacLeod]

Hi @leusbj.

A very concise summary of the situation and useful cross-reference to the Microsoft.NET.Sdk.BeforeCommon.targets file.
From my perspective, (other than wondering if System.Web should be added to _SDKImplicitReference) I think you have covered all the main bullet points.

You are right in that the idea of disabling a single package by defining the version number as '' was not intended as a feature.

I think it would be better to build up the list of SDKImplicitPackageReference (or similar) from the SDK in question, and then use an exclude of some sort - perhaps SDKExcludePackageReference to prevent it being added in.
This aligns with the idea of a Temporary ItemGroup and allows for excluding specific packages with a well-defined mechanism.

Although, in most cases, I don't see this being something that would be used, I feel it isn't a major addition to the design.
It would also mean that the excludes could be added later in the evaluation process which might be of benefit to some consumers.

The original ability to turn the packages off at all, was largely from CPM systems, or if you used a Directory.Build.xxxx file to pull in, say, the compilers to every project in your solution. I always assumed that if you didn't want the automatic mechanism, you turned it all off, then managed all the packages manually.

I don't think there is any real need to split the build up from the inclusion - especially if the whole thing can be turned off or an exclude group were available. I think we will continue to use properties to set the default version numbers so there is no need.

The whole SDK is designed only for .NET Framework 4.x so I don't know if there are any requirements to guard this.
I guess in the future, that when using something like the https://github.com/dotnet/systemweb-adapters project, you might want the RazorLibrary SDK to multitarget in some way, but for now it is probably not needed (or if anything a simple throw error saying unsupported just now if you try to target Standard or Core). The SDK requires you to use MSBuild and have VS installed (at least the build tools) due to the VSToolsPath and import of $(VSToolsPath)\WebApplications\Microsoft.WebApplication.targets.

As for the minimum TargetFrameworkVersion - I would say that we are targeting only supported versions of .NET Framework - and as far as I'm aware, all the packages that are brought in have support pretty far back anyway so it is probably not relevant just now.
I guess if/when we move to the new Microsoft.CodeDom.Providers.DotNetCompilerPlatform where V4+ only supports V4.7.2 or higher it might be relevant.

In the future, if we move to requiring a newer TargetFrameworkVersion due to some package or other feature, then either the consumer can stay on an older SDK version, or a new issue/PR can be raised to add support of the outdated/incompatible framework version.

[CZEMacLeod]

I think just change/set them in place. It might be nice to have a single file where all references are added for each SDK, but I don't see the point for 1 or 2 references only - nor do I think there is much point in adding another group (and possibly disable flag) for this, other than to enforce the metadata easily.

I think if anyone has a need to disable these automatic assembly references in the future a new PR could be done to generate a new group, a disable flag etc.

I don't really want the scope of this change to creap too much. I think a separate PR for just adding IsImplicitlyDefined to the Reference items for the SDKs is a better idea, as it can be tracked more easily.

[leusbj]

Agreed.

[leusbj]

There are 3 points of interest that will come with the IsImplicitlyDefined metadata... PackageReference items set this way are "immune" from the normal behaviors for Centralized Management.

Assuming something like this:

<ItemGroup>
  <PackageReference Include="Microsoft.AspNet.Mvc" Version="5.2.9" IsImplicitlyDefined="true" />
</ItemGroup>

Points of Interest:

• Both CentralPackageManagement and CentralPackageVersioning allow ImplicitlyDefined PackageReference items to directly set the Version metadata... Neither tooling will generate the normal error about a PackageReference that has a Version set directly, instead of using the central control.
• CentralPackageVersioning currently WILL NOT emit an error if a line present in the Packages.targets that controls the Version centrally of an implicitlydefined PackageReference.
• CentralPackageManagement currently WILL emit an error if a line present in the Directory.Packages.props that controls the Version centrally of an implicitlydefined PackageReference. (NU1009 "The packages Microsoft.AspNet.Mvc are implicitly referenced. You do not typically need to reference them from your project or in your central package versions management file. For more information, see https://aka.ms/sdkimplicitrefs" )

With that in mind...

• Most of the proposed logic to detect which tooling is being used then and provide appropriate items could/should be dropped, and instead just set the Version property directly,
• This might be considered a "Breaking Change" to consumers of the previous SDK rekease that also use CPM and have items in the Directory.Packages.props controlling any of the implicitly defined PackageReferences` will start to see build failures.

Do you have a prefered direction?

[CZEMacLeod]

Interesting. I'm now slightly unmoored in this whole process.

The original issue was that, with CPM systems, the default versioning system for the packages didn't work (already fixed).
The secondary issue was that if you were using a CPM and didn't provide version numbers, it 'failed'.
I never actually thought was really a problem - I mean the same is true if you add a new package reference yourself in a project, and if you are using a CPM, you should follow the error message the CPM gives and define all the package versions, but here we are anyway.

It seems that using IsImplicitlyDefined means we can always provide the version numbers and resolve both the original issue and the new one.
However, you are right in that it might 'break' people's builds if they are expecting a specific version from their CPM system and now get our defined version instead. An example might be if someone is using a newer Microsoft.CodeDom.Providers.DotNetCompilerPlatform in one of their projects that this SDK project references, you would get the NuGet package downgrade error. It is easily fixed with the version property but it would be a 'breaking change'.

I am torn between 2 options right now, so I'll put them here - I am not sure how complex/practical the second one is, although I am leaning towards it right now for the best of all worlds.

1. Just turn on IsImplicitlyDefined and force the version numbers.
   If someone wants to use a CPM system, they can put the version number property in their Directory.Build.props or whatever, then re-use that in their package versions file.
   (This is actually what I do just now and used global properties for the version numbers in projects before CPM systems existed).
   This is very much in line with many of the dotnet projects/packages that define common package numbers.

   • aspnet/AspNetKatana global versions / Microsoft.Owin project
   • dotnet/ef6 global versions / EntityFramework project

   However, as you say, this is probably a "Breaking Change".

2. Obey a flag (maybe we can reuse ApplySDKDefaultPackageVersions) to control the behaviour and, if we detect CPM, do not apply IsImplicitlyDefined.

   • The default can be auto (or maybe this is just not defined or `` and we reuse ApplySDKDefaultPackageVersions)
     • No CPM => IsImplicitlyDefined=true, version numbers applied
     • CPM detected => IsImplicitlyDefined=false, version numbers applied only if not defined, warning generated etc.
   • always (or maybe this is just true and we reuse ApplySDKDefaultPackageVersions)
     • IsImplicitlyDefined=true, version numbers applied always - maybe we add a warning if CPM is detected?
   • never (or maybe this is just false and we reuse ApplySDKDefaultPackageVersions)
     • IsImplicitlyDefined=false, version numbers never applied.
     • No CPM => nuget will complain about no version number or just resolve the lowest possible
     • CPM => CPM will error that no version is defined.
     • The project will have to include a <PackageReference Update="xxx" Version="xxx" /> line for each package automatically included.

My feeling is that most people would be happy with 1 - and if you are using a CPM system, a couple of lines setting the property used by the SDK globally isn't much to do. This really is the simple solution, and would greatly reduce the amount of work and logic that needs to be done.
However, I think that for maximum flexibility, and for 'backwards compatibility', option 2 is better. I think we can probably re-use the existing flag (maybe with some tweaks) - I would like to support the existing values obviously, but I like auto / always / never for explicitly defined settings rather than '' / true / false in this case as it shows there is more logic and implications to it.

I think we can do number 2 something like this without much extra effort, but I may be missing some combinations/use cases?

<ItemGroup Condition=" '$(ExcludeSDKDefaultPackages)' != 'true' and '$(DisableSDKPackageReferences)' != 'true'">
  <_SDKPackageReference Update="@(_SDKPackageReference)">
    <!-- Resolve the version to use, the override if defined, or the default if not set/empty -->
    <SDKResolvedVersion>$([MSBuild]::ValueOrDefault('%(SDKOverrideVersion)','%(SDKDefaultVersion)'))</SDKResolvedVersion>
    <!-- Mark as locked in IDE and bypass any CPM issues -->
    <IsImplicitlyDefined Condition=" '$(ApplySDKDefaultVersions)' == 'always' ">true</Version>
  </_SDKPackageReference>

  <PackageReference Include="@(_SDKPackageReference)">
    <!-- Apply the version number -->
    <Version Condition=" '$(ApplySDKDefaultVersions)' == 'always' ">%(SDKResolvedVersion)</Version>
  </PackageReference>
</ItemGroup>

obviously, we won't make the change I previously suggested to the ItemDefinitionGroup.
I think you do the ApplySDKDefaultPackageVersions property in Common.DefaultPackages.Versions.CPx.targets which is fine - it should be set to never instead of false, and either skip the include altogether if 'ApplySDKDefaultPackageVersions'!='auto', or use the condition in each include.
The remap and detect can be done in Common.DefaultPackages.Versions.targets.

Common.DefaultPackages.Versions.targets

  <PropertyGroup>
    <SdkProvidedPackageVersionDefaultWarningBeforeTargets>
      $(SdkProvidedPackageVersionDefaultWarningBeforeTargets);
      ResolvePackageAssets
    </SdkProvidedPackageVersionDefaultWarningBeforeTargets>
  </PropertyGroup>
  
  <Target Name="SdkProvidedPackageVersionDefaultWarning" BeforeTargets="$(SdkProvidedPackageVersionDefaultWarningBeforeTargets)"
          Condition=" '$(Disable    SdkExplicitPackageReference)'!='true' AND '$(SdkPackageReferenceVersionToolingTargets)' != '' AND @(SdkProvidedPackageVersionDefault->Count()) != 0 ">
    <Warning
      Text="The PackageReference '%(SdkProvidedPackageVersionDefault.Identity)' was included by the  Sdk defaulting to Version='%(SdkProvidedPackageVersionDefault.Version)'. Centralized management of package versions was detected, consider adding this entry '%(SdkProvidedPackageVersionDefault.Suggestion)' to your centralized package version listing."
      File="%(SdkProvidedPackageVersionDefault.ReferencedFile)"
      />
  </Target>

  <!-- CPV sets the EnableCentralPackageVersions property to false under certain circumstances... like file not present or if CPM is detected  -->
  <PropertyGroup Condition=" '$(UsingMicrosoftCentralPackageVersionsSdk)' == 'true' AND '$(EnableCentralPackageVersions)' != 'false'">
    <SdkPackageReferenceVersionToolingTargets>MSBuild.SDK..Common.DefaultPackages.Versions.CPV.targets</SdkPackageReferenceVersionToolingTargets>
  </PropertyGroup>
  <!-- CPM uses this same conditional to decide if CPM is really being used... checking for both the property and the file was consumed -->
  <PropertyGroup Condition=" '$(ManagePackageVersionsCentrally)' == 'true' AND '$(CentralPackageVersionsFileImported)' == 'true'">
    <SdkPackageReferenceVersionToolingTargets>MSBuild.SDK..Common.DefaultPackages.Versions.CPM.targets</SdkPackageReferenceVersionToolingTargets>
  </PropertyGroup>  
  
  <!-- Remap old property values -->
  <PropertyGroup>
    <ApplySDKDefaultPackageVersions Condition="'$(ApplySDKDefaultPackageVersions)'==''">auto</ApplySDKDefaultPackageVersions>
    <ApplySDKDefaultPackageVersions Condition="'$(ApplySDKDefaultPackageVersions)'!='true'">never</ApplySDKDefaultPackageVersions>
    <ApplySDKDefaultPackageVersions Condition="'$(ApplySDKDefaultPackageVersions)'=='true'">always</ApplySDKDefaultPackageVersions>
  </PropertyGroup>
  
  <!-- Incorporate the appropriate logic based on the tooling detected CPM or CPV -->
  <Import Project="$(SdkPackageReferenceVersionToolingTargets)" 
          Condition=" '$(ApplySDKDefaultPackageVersions)' == 'auto' AND 
                      '$(SdkPackageReferenceVersionToolingTargets)' != '' AND 
                      Exists('$(SdkPackageReferenceVersionToolingTargets)')"/>

  <PropertyGroup>
    <ApplySDKDefaultPackageVersions Condition="'$(ApplySDKDefaultPackageVersions)'=='auto'">always</ApplySDKDefaultPackageVersions>
    <!-- We should just alter the Common.DefaultPackages.Versions.CPx.targets file -->
    <ApplySDKDefaultPackageVersions Condition="'$(ApplySDKDefaultPackageVersions)'=='false'">never</ApplySDKDefaultPackageVersions>
  </PropertyGroup>

I think this would cover all the combinations -
auto does the detect, and fills in missing information if required including warnings.
never just adds the packages, no version numbers ever applied.
always adds the packages with versions regardless and uses IsImplicitlyDefined to lock and bypass and CPM system.

Let me know what you think? I haven't seen any of the other posters of the original issues coming in on this discussion, but maybe there will be some external input here?

[CZEMacLeod]

I'm leaning to keeping the ''/true/false settings now:

Common.DefaultPackages.Versions.targets

<Project>
    <PropertyGroup>
        <SdkProvidedPackageVersionDefaultWarningBeforeTargets>
      $(SdkProvidedPackageVersionDefaultWarningBeforeTargets);
      ResolvePackageAssets
    </SdkProvidedPackageVersionDefaultWarningBeforeTargets>
    </PropertyGroup>
    <Target Name="SdkProvidedPackageVersionDefaultWarning"
            BeforeTargets="$(SdkProvidedPackageVersionDefaultWarningBeforeTargets)"
            Condition=" '$(DisableSdkExplicitPackageReference)'!='true' AND '$(SdkPackageReferenceVersionToolingTargets)' != '' AND @(SdkProvidedPackageVersionDefault->Count()) != 0 ">
        <Warning Text="The PackageReference '%(SdkProvidedPackageVersionDefault.Identity)' was included by the  Sdk defaulting to Version='%(SdkProvidedPackageVersionDefault.Version)'. Centralized management of package versions was detected, consider adding this entry '%(SdkProvidedPackageVersionDefault.Suggestion)' to your centralized package version listing."
                 File="%(SdkProvidedPackageVersionDefault.ReferencedFile)"/>
    </Target>
    <!-- CPV sets the EnableCentralPackageVersions property to false under certain circumstances... like file not present or if CPM is detected  -->
    <PropertyGroup Condition=" '$(UsingMicrosoftCentralPackageVersionsSdk)' == 'true' AND '$(EnableCentralPackageVersions)' != 'false'">
        <SdkPackageReferenceVersionToolingTargets>MSBuild.SDK..Common.DefaultPackages.Versions.CPV.targets</SdkPackageReferenceVersionToolingTargets>
    </PropertyGroup>
    <!-- CPM uses this same conditional to decide if CPM is really being used... checking for both the property and the file was consumed -->
    <PropertyGroup Condition=" '$(ManagePackageVersionsCentrally)' == 'true' AND '$(CentralPackageVersionsFileImported)' == 'true'">
        <SdkPackageReferenceVersionToolingTargets>MSBuild.SDK..Common.DefaultPackages.Versions.CPM.targets</SdkPackageReferenceVersionToolingTargets>
    </PropertyGroup>
    <!-- Incorporate the appropriate logic based on the tooling detected CPM or CPV -->
    <Import Project="$(SdkPackageReferenceVersionToolingTargets)"
            Condition=" '$(ApplySDKDefaultPackageVersions)' == '' AND 
                      '$(SdkPackageReferenceVersionToolingTargets)' != '' AND 
                      Exists('$(SdkPackageReferenceVersionToolingTargets)')"/>
    <PropertyGroup>
        <ApplySDKDefaultPackageVersions Condition="'$(ApplySDKDefaultPackageVersions)'==''">true</ApplySDKDefaultPackageVersions>
    </PropertyGroup>
</Project>

and

Common.DefaultPackages.targets

<Project>
    <ItemDefinitionGroup>
        <_SDKPackageReference>
            <SDKImplicitPackage>true</SDKImplicitPackage>
        </_SDKPackageReference>
    </ItemDefinitionGroup>

    <ItemGroup>
        <_SDKPackageReference Include="Microsoft.Net.Compilers.Toolset"
                              SDKDefaultVersion="4.5.0"
                              SDKOverrideVersion="$(MicrosoftNetCompilersToolset_Version)"
                              PrivateAssets="All"
                              GeneratePathProperty="true"/>
        <_SDKPackageReference Include="Microsoft.CodeDom.Providers.DotNetCompilerPlatform"
                              SDKDefaultVersion="3.6.0"
                              SDKOverrideVersion="$(MicrosoftCodeDomProvidersDotNetCompilerPlatform_Version)"/>
    </ItemGroup>

    <ItemGroup Condition=" '$(ExcludeSDKDefaultPackages)' != 'true' and '$(DisableSDKPackageReferences)' != 'true'">
        <_SDKPackageReference Update="@(_SDKPackageReference)">
            <!-- Resolve the version to use, the override if defined, or the default if not set/empty -->
            <SDKResolvedVersion>$([MSBuild]::ValueOrDefault('%(SDKOverrideVersion)','%(SDKDefaultVersion)'))</SDKResolvedVersion>
            <!-- Mark as locked in IDE and bypass any CPM issues -->
            <IsImplicitlyDefined Condition=" '$(ApplySDKDefaultVersions)' == 'true' ">true</Version>
        </_SDKPackageReference>
        <PackageReference Include="@(_SDKPackageReference)">
            <!-- Apply the version number -->
            <Version Condition=" '$(ApplySDKDefaultVersions)' == 'true' ">%(SDKResolvedVersion)</Version>
        </PackageReference>
    </ItemGroup>
</Project>

This gives the following behaviours for ApplySDKDefaultPackageVersions

Value	Behaviour
''	Automatically detect CPM solutions. Apply version numbers if none detected, or if the CPM doesn't apply a version number
true	Always apply the version numbers and bypass any CPM solution. Applies the IsImplicitlyDefined metadata.
false	Never apply a version number. A CPM system must be used with appropriate versions defined, or the project file must include <PackageReference Update="xxxx" Version="a.b.c" /> tags for each package.

[leusbj]

For Centralized Version Tooling awareness, something like the following could be incorporated after the Microsoft.NET.Sdk/Sdk.targets... so that it is after the Directory.Build.targets. It can:

• Do the detection of tooling
• Have a target for outputing messages about the suggestedDefaults

###Common.DefaultPackages.Versions.targets

  <PropertyGroup>
    <SystemWebSdkProvidedPackageVersionDefaultWarningBeforeTargets>
      $(SystemWebSdkProvidedPackageVersionDefaultWarningBeforeTargets);
      ResolvePackageAssets
    </SystemWebSdkProvidedPackageVersionDefaultWarningBeforeTargets>
  </PropertyGroup>
  
  <Target Name="SystemWebSdkProvidedPackageVersionDefaultWarning" BeforeTargets="$(SystemWebSdkProvidedPackageVersionDefaultWarningBeforeTargets)"
          Condition=" '$(DisableSystemWebSdkExplicitPackageReference)'!='true' AND '$(SystemWebSdkPackageReferenceVersionToolingTargets)' != '' AND @(SystemWebSdkProvidedPackageVersionDefault->Count()) != 0 ">
    <Warning
      Text="The PackageReference '%(SystemWebSdkProvidedPackageVersionDefault.Identity)' was included by the SystemWeb Sdk defaulting to Version='%(SystemWebSdkProvidedPackageVersionDefault.Version)'. Centralized management of package versions was detected, consider adding this entry '%(SystemWebSdkProvidedPackageVersionDefault.Suggestion)' to your centralized package version listing."
      File="%(SystemWebSdkProvidedPackageVersionDefault.ReferencedFile)"
      />
  </Target>

  <!-- CPV sets the EnableCentralPackageVersions property to false under certain circumstances... like file not present or if CPM is detected  -->
  <PropertyGroup Condition=" '$(UsingMicrosoftCentralPackageVersionsSdk)' == 'true' AND '$(EnableCentralPackageVersions)' != 'false'">
    <SystemWebSdkPackageReferenceVersionToolingTargets>MSBuild.SDK.SystemWeb.Common.DefaultPackages.Versions.CPV.targets</SystemWebSdkPackageReferenceVersionToolingTargets>
  </PropertyGroup>
  <!-- CPM uses this same conditional to decide if CPM is really being used... checking for both the property and the file was consumed -->
  <PropertyGroup Condition=" '$(ManagePackageVersionsCentrally)' == 'true' AND '$(CentralPackageVersionsFileImported)' == 'true'">
    <SystemWebSdkPackageReferenceVersionToolingTargets>MSBuild.SDK.SystemWeb.Common.DefaultPackages.Versions.CPM.targets</SystemWebSdkPackageReferenceVersionToolingTargets>
  </PropertyGroup>  
  
  <!-- Incorporate the appropriate logic based on the tooling detected CPM or CPV -->
  <Import project="$(SystemWebSdkPackageReferenceVersionToolingTargets)" Condition=" '$(SystemWebSdkPackageReferenceVersionToolingTargets)' != '' AND Exists('$(SystemWebSdkPackageReferenceVersionToolingTargets)')"/>

Then add two files to other common files to the solution... one for CPM and one for CPV that

• Handles the specific ItemGroups that need to be manipulated by the specific tooling
• Collectts Items that were actually injected for later messaging

Common.DefaultPackages.Versions.CPM.targets

  <PropertyGroup>
    <ApplySDKDefaultPackageVersions>false</ApplySDKDefaultPackageVersions>
  </PropertyGroup>

  <ItemGroup>
    <!-- Find any PackageReferences added by this SDK, and provide Version when using Central Package Management -->
    <!-- Start with PackageReferences so looking at only those actually referenced, filter to only those added by this Sdk and Exclude items already present to avoid duplicates -->
    <PackageVersion
		  Include="@(PackageReference->WithMetadataValue('SystemWebSdkIncludedPackage','true')->HasMetadata('SystemWebSdkSuggestedVersion'))"
		  Exclude="@(PackageVersion)">
      <SystemWebSdkProvidedDefaultVersion>true</SystemWebSdkProvidedDefaultVersion>
      <Version>%(SystemWebSdkSuggestedVersion)</Version>
    </PackageVersion>
  </ItemGroup>

  <ItemGroup>
    <!-- Gether up entries for which this Sdk set the version metadata so Warnings can be emitted later-->
    <SystemWebSdkProvidedPackageVersionDefault
		  Include="@(PackageVersion->HasMetadata('SystemWebSdkProvidedDefaultVersion')->WithMetadataValue('SystemWebSdkProvidedDefaultVersion','true'))"
		  >
      <Suggestion>&lt;PackageVersion Include="%(Identity)" Version="%(Version)" /&gt;</Suggestion>
      <ReferencedFile>$(DirectoryPackagesPropsPath)</ReferencedFile>
    </SystemWebSdkProvidedPackageVersionDefault>

  </ItemGroup>

Common.DefaultPackages.Versions.CPV.targets

  <PropertyGroup>
    <ApplySDKDefaultPackageVersions>false</ApplySDKDefaultPackageVersions>
  </PropertyGroup>

  <ItemGroup>
    <!-- Find any PackageReferences added by this SDK, and provide Version when using Central Package Versions -->
    <!-- Start with PackageReferences so looking at only those actually referenced, filter to only those added by this Sdk, update Version property only when still not set by actual CPV file -->
    <PackageReference
		  Update="@(PackageReference->WithMetadataValue('SystemWebSdkIncludedPackage','true')->HasMetadata('SystemWebSdkSuggestedVersion'))"
			 >
      <SystemWebSdkProvidedDefaultVersion Condition="'%(Version)' == '' ">true</SystemWebSdkProvidedDefaultVersion>
      <Version Condition="'%(Version)' == '' ">%(SystemWebSdkSuggestedVersion)</Version>
    </PackageReference>
  </ItemGroup>

  <ItemGroup>
    <!-- Gether up entries for which this Sdk set the version metadata so Warnings can be emitted later-->
    <SystemWebSdkProvidedPackageVersionDefault
			Include="@(PackageReference->HasMetadata('SystemWebSdkProvidedDefaultVersion')->WithMetadataValue('SystemWebSdkProvidedDefaultVersion','true'))"
			>
      <Suggestion>&lt;PackageReference Update="%(Identity)" Version="%(Version)" /&gt;</Suggestion>
      <ReferencedFile>$(CentralPackagesFile)</ReferencedFile>
    </SystemWebSdkProvidedPackageVersionDefault>
  </ItemGroup>

  <PropertyGroup>
    <SystemWebSdkProvidedPackageVersionDefaultWarningBeforeTargets>
      $(SystemWebSdkProvidedPackageVersionDefaultWarningBeforeTargets);
      CheckPackageReferences
    </SystemWebSdkProvidedPackageVersionDefaultWarningBeforeTargets>
  </PropertyGroup>

[CZEMacLeod]

Other than perhaps making the properties and targets starting SystemWebSdk just start SDK, I like this. (Same for the warning message - SystemWeb Sdk becomes just SDK)
In general, I think that this should be written generically, such that it could be copy/pasted into any SDK and the property names/item groups would still apply.
In fact, the whole system for an SDK with package imports could be factored out and added as a 'sub-SDK' at some point, or even offered to the dotnet team to add to the main Microsoft.NET.Sdk.

[leusbj]

Ah, and I was thinking to name things specific to "System.Web and FullFramework" so to avoid potential propertyname clashing with potentially other outside Sdk's... while assuming that any "sub type" (and associated flavors... Web, WebAPI, WebForms, WebMvc, MvcRazorLibrary)within this repo still count as "SytemWebSdk" since they stem from the base one.

If you are considering to potentially extract/offer aspects of this sdk outside... I don't know that it is the right idea, but have you had any consideration to investigate the "Workload" and related tooling concepts that MAUI is using? I haven't read enough to know if it would be a good fit, but wanted to know if you were planning to head that way before going much further here.

[CZEMacLeod]

I'm not sure that the workload idea would be relevant here as we are targeting net framework. While you can pull in other SDKs (such as the CPM) I doubt it would happen much with this SDK as it is very specific in what it is for. I don't believe that any other SDK does this kind of stuff (just now) and even if it did, aligning on a common set of properties and ways of doing things would be better.
I don't think subtypes are likely, as you can mix and match most of these things. It might be possible to have them enabled via flags of some sort though.
I feel that in general though, this would be more likely to be options in the template, to strip down the Full to Empty, and perhaps not actually having 2 different templates, but 1 with options for all the things you can include.

[CZEMacLeod]

See #59 for a slight tweak to logic that may be needed for one of the common packages.

[CZEMacLeod]

@leusbj Have you moved forward with any of this? I know you still have the original PR open, but wondered if you planned on updating it with the results of this discussion, or if you want me to take a stab at bringing it all together?

[leusbj]

@CZEMacLeod Sorry I have not made any progress. I should probably just close the original PR as it was a convoluted history of changes, most of which is nolonger applicable. Feel free to take a stab.