-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow TFM/TPM checks in a concise, understandable way #5171
Comments
This is now tracked here: dotnet/msbuild#5171
To summarize some of the concerns:
@rainersigwald would something like this fly? <PropertyGroup Condition="$IsCompatibleWith($(TargetFramework), 'ios12.0')">
<SomeProperty>Some value that applies to iOS 12 or later<SomeProperty>
</PropertyGroup> Basically, could we allow top-level functions? |
Yes, I think we could do <PropertyGroup Condition="IsCompatibleWith($(TargetFramework), 'ios12.0')">
<SomeProperty>Some value that applies to iOS 12 or later<SomeProperty>
</PropertyGroup> which differs from yours only in the initial Right now we have only |
I dig it! |
I would add |
What about these functions then?
You mean as a function? |
I am a little bit concerned by how many components are currently being forced to understand the new format (MSBuild, SDK, NuGet). Presumably you can calculate the properties from each identifier in Just thinking aloud I guess. From user perspective the proposed functions above would likely work. One thing that MSBuild.Sdk.Extras does is an ability to dispatch to inner builds based on |
We can do that after dispatching it to the inner build too. Since, the property
There is no need for doing these comparisons (in the user side) if only |
While that's not totally unreasonable, it makes authoring props and targets harder because it bifurcates the world. Ideally, you should be able to write conditions that work regardless of whether the project is a single target build or or multi target build. |
Note that this problem already exist today. The properties |
@terrajobst I do understand the properties exist today. The parsing currently happens in SDK and NuGet code as far as I can tell. This would introduce a third place with the parsing logic. Please correct me if I am wrong. I am just concerned that three places with the parsing logic starts to be too much to maintain, especially if you consider that not all these projects are necessarily distributed as single cohesive package today. Visual Studio was still using the NetFX version of MSBuild by the time .NET Core 3 was developed which resulted in incompatible behavior between builds in VS and builds in |
What about version ranges - for the docs platform, we have a concept of range, so instead of combining clauses, a range can be parsed |
I agree but this approach seems way into the .NET world. We have C++ and other project types too. MSBuild is an orchestration engine and should always be that and no more. |
I wouldn't want to do this any way other than making the MSBuild functionality a thin wrapper over NuGet functionality. As you say, it's no good if the implementations diverge.
Can you give an example? We work pretty hard to make sure MSBuild, NuGet, and SDK versions match across matching versions of the products (for example, VS 16.3 and SDK 3.0.100). |
I am not necessarily saying it is still happening. We were consuming .NET Core 3.0 since the very first previews. When building from VS it was using MSBuild that supported binary resources with the old code that does de/serialization. Meanwhile |
With something like As for duplicated logic, I think this proposal will actually allow us to eliminate the duplication. The MSBuild intrinsics would be implemented by calling APIs from NuGet. Then we'd update the logic in the SDK which does its own @rainersigwald @nkolev92 Are there still concerns with MSBuild depending on and loading NuGet assemblies? I'm looking at the code and seeing comments like this: |
Yes, but I think they're resolvable:
|
All these are .NET related, not just NuGet specific, so why not under Putting it under MSBuild, atleast for me, is not clear. Also once it's shipped, we can never change it, right? So, shouldn't these kind of decisions be made early on? Also if there's a potential to have custom property functions in the future. Isn't it better to have an MSBuild language feature, so that internal/3rd party can provide custom functions along with the SDK package instead? |
That's a new concept that does not appear anywhere else. NuGet is the layer of the .NET stack where TargetFramework compatibility is defined.
It is consistent with all the other property functions that expose non-BCL behavior. I think that's a strong argument in its favor.
This is not likely to happen:
|
Those existing properties can be used in C++ and other custom project types. These are .NET specific and have no meaning in other project types. Isn't it better to classify them as such.
You're thinking in terms of
This is where it gets interesting, when more and more people embrace MSBuild (Unless you don't want them to) they could have their own project system with custom functions similar to what .NET is doing here. When and If C++ projects move to Sdk style, they could have custom functions to make project files easier to author.
We could use attributes on a method call to detect the custom functions in the referenced lib. On top of my head, this is what I came up with... With similar to existing task-declaration <Project>
<!-- Example Task declaration -->
<UsingTask Name="Micosoft.Build.Tasks.MSBuild" Assembly="Microsoft.Build.Tasks.dll" />
<UsingTask Namespace="Micosoft.Build.Tasks" Assembly="Microsoft.Build.Tasks.dll" /> <!-- NEW: Import all tasks under this namespace -->
<!-- Similar to UsingTask -->
<UsingFunction Alias="MSBuild.GetVsInstallRoot()" Name="VisualStudio.Build.Extensions.MSBuildFunctions.GetInstalledPath()" Assembly="Microsoft.VisualStudio.Build.Extensions.dll" />
<!-- [DotNet]:: -->
<UsingFunction Alias="DotNet" Class="Microsoft.NET.Build.Extensions.MSBuildFunctions" Assembly="Microsoft.NET.Build.Extensions.dll" />
<!-- [MSBuild]:: -->
<UsingFunction Alias="MSBuild" Class="Microsoft.Build.Evaluation.IntrinsicFunctions" Assembly="Microsoft.Build.dll" />
<!-- [SampleLib.MyClass]:: -->
<UsingFunction Class="SampleLib.MyClass" Assembly="MSBuild.SampleLib.dll" />
<Project> using MSBuild.Framework;
namespace SampleLib
{
[IntrinsicFunctionContainer] // or something similar but you get the idea!
public static class MyClass
{
}
}
Tasks and Sdks are in the same boat. Yet, we did make them work! |
Implementing the first few intrinsic properties from #5171
Is the final design documented anywhere? |
@KirillOsenkov not formally, but we essentially went with this comment. |
@sfoslund is there a doc bug/issue tracking the need to get this documented somewhere? |
@clairernovotny issue is here: MicrosoftDocs/visualstudio-docs#5599 |
Sorry for replying to a closed thread but I'm wondering if the following behavior is right:
Output:
In the last case for example, shouldn't this be true? |
Maybe I'm getting something wrong. What's the meaning of |
Ah I see, it's the other way around. Target can reference candidate:
Output:
|
Yeah, this was brough up before. Sadly, for a two argument function there isn't a canonical way to indicate who can reference whom... |
Documentation that shows up when googling the function's name would already be sufficient :) Do we have a tracking issue for the documentation part? |
Yes, documentation will be released along with 16.8, see this comment: #5171 (comment) |
As part of the spec for the .NET 5 TFM work we identified an issue with TFM checks in conditions.
Background on MSBuild evaluation
In SDK-style projects there are two kinds of MSBuild files that are automatically included into each project:
*.props
: These files are included at the top of the user's project file and are used to define a set of default properties that the user's project file can use.*.targets
. These files are included at the bottom of the user's project file, usually meant to define build targets and additional properties/items that need to depend on properties defined by the user.Furthermore, MSBuild has a multi-pass evaluation model where properties are evaluated before items.
Why is all of this important? Because it controls which properties the user can rely on in their project file.
Often, a user wants to express a condition like "include this file if you're compiling for .NET 5 or higher". Logically one would like to express it like this:
but this doesn't work because that would be a string comparison, not a version comparison. Instead, the user has to write it like this:
This works for conditions on item groups because they are evaluated after properties. Since the user's project file defines the
TargetFramework
property, the SDK logic that expands it into the other properties such asTargetFrameworkIdentifier
andTargetFrameworkVersion
has to live in*.targets
, i.e. at the bottom of the project file. That means these automatically expanded properties aren't available for the user when defining other properties. This happens to work for items because items are evaluated after all properties are evaluated.Due to MSBuild evaluation order the user cannot define properties like this:
In the past, we've seen people working this around by using string processing functions against the
TargetFramework
property, which is less than ideal.Option using attributes
Ideally, we'd expose functionality such that the user can do version checks:
The idea is:
TargetFramework
andTargetPlatform
.==
,!=
,<
,<=
,>
, and>=
. If the operator is omitted,==
is assumed.TargetFramework
supports comparisons with a friendly TFM name. This can include an OS flavor for symmetry. If theTargetFramework
property includes an OS flavor but the attribute doesn't, the comparison only applies to the TFM without the OS flavor. In other words a condition ofTargetFramework=">=net5.0"
will result intrue
if the project targetsnet5.0
,net6.0
, as well asnet6.0-android12.0
.Option via new syntax
We could also invent new syntax that allows parsing of constitutes like this:
Option via functions
We could also just define new intrinsic functions on some type, but this will make using them a mouthful:
I am not married to any of these ideas; I'm just spitballing here. Thoughts?
The text was updated successfully, but these errors were encountered: