Skip to content

Conversation

@Youssef1313
Copy link
Member

  • In many packages, we shouldn't need buildMultitargeting because buildMultitargeting is only relevant for the "outer" build. TestingPlatformBuilderHook is only relevant for the inner builds.
  • Instead of packing props/targets for each TFM, we can simply just pack in build/buildTransitive directly. We don't have any TFM-specific logic.

@Youssef1313 Youssef1313 requested a review from Copilot October 30, 2025 15:21
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the NuGet package layout structure for Microsoft.Testing.Platform and its extensions by simplifying build file organization and eliminating target framework-specific packaging. The changes consolidate build configurations from a multi-targeting approach to a simpler, unified structure.

Key changes:

  • Consolidate build file imports to reference build folders instead of buildMultiTargeting
  • Replace TfmSpecificPackageFile with simplified None items in project files
  • Move configuration from buildMultiTargeting folders into build folders for extension packages
  • Simplify MSBuild path references by removing one level of directory traversal

Reviewed Changes

Copilot reviewed 37 out of 37 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
Microsoft.Testing.Platform.targets (buildTransitive & build) Updated import paths to reference buildMultiTargeting with one less parent directory level
Microsoft.Testing.Platform.props (buildTransitive & build) Updated import paths to reference buildMultiTargeting with one less parent directory level
Microsoft.Testing.Platform.csproj Simplified package file inclusion from Content/TfmSpecificPackageFile to None items
Microsoft.Testing.Extensions.*.props (buildTransitive) Changed imports from buildMultiTargeting to build folder
Microsoft.Testing.Extensions.*.props (buildMultiTargeting) Deleted files - configuration moved to build folder
Microsoft.Testing.Extensions.*.props (build) Moved configuration content from buildMultiTargeting to build folder
Microsoft.Testing.Extensions.*.csproj Simplified package file inclusion and removed buildMultiTargeting references

@nohwnd
Copy link
Member

nohwnd commented Oct 30, 2025

Instead of packing props/targets for each TFM, we can simply just pack in build/buildTransitive directly. We don't have any TFM-specific logic.

This part is not implemented or am I blind? I see no changes touching tfm specific files.

also if we don't have tfm specific folders ,it will make restore the package for any tfm. Or maybe I am misunderstanding the sentence.

Comment on lines -23 to -28
<TfmSpecificPackageFile Include="buildTransitive/**">
<PackagePath>buildTransitive/$(TargetFramework)</PackagePath>
</TfmSpecificPackageFile>
<TfmSpecificPackageFile Include="build/**">
<PackagePath>build/$(TargetFramework)</PackagePath>
</TfmSpecificPackageFile>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nohwnd Here we were packing per-TFM. The updated logic doesn't.

@Youssef1313
Copy link
Member Author

Youssef1313 commented Oct 30, 2025

also if we don't have tfm specific folders ,it will make restore the package for any tfm.

Hmm, I thought it would fail if no compatible TFM is found in lib. Also note that for all modified packages, we target netstandard2.0 which is almost compatible with the whole world (small exceptions are netcoreapp1.0, netcoreapp1.1, net45, MonoAndroid70, etc).

@nohwnd
Copy link
Member

nohwnd commented Oct 30, 2025

hmm then I don't think there is a good solution.
image

I am not 100% sure without trying (since it is an "edge-case"), but imho this will make the package restore for everything, we had this problem with one of vstest packages.

Secondly the netstandard2.0 compatibility is unfortunate, but you can do this what system.io.hashing does (dotnet/runtime does that via some custom target generator) .

https://nuget.info/packages/System.IO.Hashing/10.0.0-rc.2.25502.107

<Project InitialTargets="NETStandardCompatError_System_IO_Hashing_net8_0">
  <Target Name="NETStandardCompatError_System_IO_Hashing_net8_0"
          Condition="'$(SuppressTfmSupportBuildWarnings)' == ''">
    <Warning Text="System.IO.Hashing 10.0.0-rc.2.25502.107 doesn't support $(TargetFramework) and has not been tested with it. Consider upgrading your TargetFramework to net8.0 or later. You may also set &lt;SuppressTfmSupportBuildWarnings&gt;true&lt;/SuppressTfmSupportBuildWarnings&gt; in the project file to ignore this warning and attempt to run in this unsupported configuration at your own risk." />
  </Target>
</Project>

they do that in buildTransitive/netstandard2.0 target.

I am not sure if we can combine this with actually supporting netstandard2.0 (unless we want to add netcore1.0 to tfms :D)

@nohwnd
Copy link
Member

nohwnd commented Oct 30, 2025

which is what we do in test.sdk, that was moved from the non-tfm specific build target folder:

image

@Youssef1313
Copy link
Member Author

Closing for now. Not worth spending time on this for now. We can revisit later.

@Youssef1313 Youssef1313 closed this Nov 2, 2025
@Youssef1313 Youssef1313 deleted the dev/ygerges/simplify-packaging branch November 2, 2025 13:10
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.

3 participants