[Feature Request]: Provide a way to remove specific properties from the implicit restore triggered by /restore

baronfel commented 6 months ago

Summary

In https://github.com/dotnet/sdk/pull/38956 we had to jump through a bunch of hoops to navigate the situations where the SDK needs to trigger a 'separate restore' - meaning a Restore operation that doesn't include various properties that the user may want to be sent to the subsequent Build/Publish/etc invocation. The most common scenario here is TargetFramework.

Because TargetFramework negatively influences Restore, we remove it, perform a Restore, then perform the original call the user requested. This is both fragile, and it creates overhead in the build from running an entire separate MSBuild invocation.

There should be some mechanism to either use the existing -restoreProperty or a new flag to explicitly un-set properties for the Restore triggered by /restore, so that these call patterns could be accounted for by the engine itself, instead of callers having to patch it together.

Background and Motivation

This most clearly surfaces when people try to use the new -getProperty family of flags in conjunction with a build call that includes TFMs.

Proposed Feature

A new -rrp/-removeRestoreProperty flag that takes a list of properties to unset for a Restore call.

Alternative Designs

-rp could be extended to allow unset via -rp:Property=, though this would be ambiguous with an explicit set to an empty value.

rainersigwald commented 6 months ago

-rp could be extended to allow unset via -rp:Property=, though this would be ambiguous with an explicit set to an empty value.

This was my first instinct. I'm surprised we allow explicit set to an empty value, since that's not really expressible in a project (it's ambiguous whether it's set to empty or unset-and-thus-returns-empty).

baronfel commented 6 months ago

Since the same issue also exists for 'normal' build invocations, we may want to consider generalizing this. Similar to how we have both -p and -rp, we should have an --unset-restore-property and -unset-property flag that could apply to both the implicit restore and the 'normal' requested build. This would make it easier for scripted-build scenarios where a script might be aggregating some kind of 'final' command line.

Proposal:

--unset-property [PROPERTYNAME] - if provided, the property PROPERTYNAME will be removed from the global property collection used for evaluating the requested project
- short form: --up
--unset-restore-property [PROPERTYNAME] - if provided, the property PROPERTYNAME will be removed from the global property collection used for evaluating the requested project
- short form: --urp

All -- prefixes here should be interpreted to be the full set of valid MSBuild flag prefixes: --, -, /.

Both flags can be specified multiple times, or take a comma-separated list of values, and both apply after all -p/-rp flags have been processed.

Example usages:

> dotnet msbuild /restore /t:Publish -p:TargetFramework=net8.0 -urp:TargetFramework

dotnet / msbuild