Skip to content
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

Extend ItemGroup/PropertyGroup grammar to allow more user-friendly Conditions for common use cases #11329

Open
baronfel opened this issue Jan 24, 2025 · 10 comments
Labels
Area: Language Issues impacting the MSBuild programming language. triaged

Comments

@baronfel
Copy link
Member

baronfel commented Jan 24, 2025

Summary

The MSBuild Condition syntax is very powerful, but it's also very open-ended and hard to create tooling around. There is value in making syntax for common project- and MSBuild-logic constraints more easy-to-use, because a more-restricted but easier-to-use syntax would be both more understandable to users (especially casual MSBuild users) and could disallow entire categories of errors that exist today (e.g. typos in common Condition usage).

Background and Motivation

Consider a user that wants to make a number of changes to Property values when they are targeting the Release configuration. For example, they might want to opt in to

  • embedded PDB generation
  • single-file publishing
  • AOT compilation
  • certain kinds of warnings/diagnostics

Today, this is all doable. A user might write some code like

<PropertyGroup>
  <PdbType Condition="'$(Configuration)' == 'Release'">embedded</PdbType>
  <PublishSingleFile>true</PublishSingleFile>
  <PublishAot>true</PublishAot>
  <NoWarn Condition="'$(Configuration)' == 'Release'">$(NoWarn);MSB12345</NoWarn>
</PropertyGroup>

Things to note here:

  • the user has duplicated the condition a few times
  • The user has made a decision to set some Properties unconditionally and some conditionally - why might that be?
  • The design of the features used (AOT, single-file publishing, etc) has to know about the impact of build-time vs publish-time, so the feature has been implemented with flags that are only 'checked' during Publishing - this requires some knowledge outside of the build!

Making changes

What if the user needs to add a pivot to the condition for any reason? Say for .NET 9 they need an additional NoWarn. The code might look like this:

<PropertyGroup>
  <PdbType Condition="'$(Configuration)' == 'Release'">embedded</PdbType>
  <PublishSingleFile>true</PublishSingleFile>
  <PublishAot>true</PublishAot>
- <NoWarn Condition="'$(Configuration)' == 'Release'">$(NoWarn);MSB12345</NoWarn>
+ <NoWarn Condition="'$(Configuration)' == 'Release' and '$(TargetFramework)' == 'net9.0'">$(NoWarn);MSB12345;NETSDK98765</NoWarn>
</PropertyGroup>

This change is hard to spot-check for most users.

Consolidating changes

Experienced MSBuild users might reach for a Condition on the PropertyGroup to reduce the amount of duplication on the properties, like so:

<PropertyGroup Condition="'$(Configuration)' == 'Release'">
  <PdbType>embedded</PdbType>
  <PublishSingleFile>true</PublishSingleFile>
  <PublishAot>true</PublishAot>
  <NoWarn>$(NoWarn);MSB12345</NoWarn>
</PropertyGroup>

This is great! However, many users do not realize that Conditions can be applied to these nodes, and the problem of not being able to easily-classify the changes remains. If you need to add another TFM, now you either have two levels of Condition (one at the PropertyGroup, one on the NoWarn Property) or you have two PropertyGroups.

We need a syntax that is easy to use for common pivots that users experience daily.

Proposed Feature

We should extend the grammar of PropertyGroups and ItemGroups to allow for metadata to be set that the engine would interpret as an automatically-correct Conditions. I propose that the initial set of Properties we consider for this treatment be

  • TargetFramework
  • Configuration
  • RuntimeIdentifier
  • Platform

With these properties, users would be able to construct targeted PropertyGroups or ItemGroups that scope changes easily. An example of the above in this fashion:

<PropertyGroup Configuration="Release">
  <PdbType>embedded</PdbType>
  <PublishSingleFile>true</PublishSingleFile>
  <PublishAot>true</PublishAot>
  <NoWarn>$(NoWarn);MSB12345</NoWarn>
</PropertyGroup>

During parsing, the engine would read this attribute and generate a Condition on this PropertyGroup equivalent to Condition="$(Configuration) == 'Release'". Implicit conditions would be additive - setting both Configuration="Release" and TargetFramework="net8.0" would result in a compound Condition of Condition="$(Configuration) == 'Release' and $(TargetFramework) == 'net8.0'".

The Groups would be read and applied in-order in the file according to the existing MSBuild passes, so this change would be a purely-semantic transformation - no changes to passes or the resulting project would be intended here.

Open Questions

Why only PropertyGroups/ItemGroups?

Properties cannot have metadata today, and Items already have metadata that may clobber the existing properties - it would be hard to do this in a safe way. It's natural to group changes to properties and items based on these properties, so this extension felt natural.

Is this set of properties sufficient?

We've chosen the common user pivots today, but the syntax isn't limited to/exhaustive. This feels like a kind of change that we should start small on to keep the problem space limited/well-understood. An immediate suggestion to add might be

  • LangVersion

Should we limit the set of properties at all?

Yes - if we make this change we'll be supporting the syntax forever. We need to be conservative in the set of values accepted here so that we have room to evolve in the future.

Should we support comparisons on things other than Properties?

Item-comparisons are common, but due to the grammar of MSBuild Item-based comparisons can only happen on ItemGroups - is this clear to users? Would it be a foot-gun?

Should the TFM condition sematics be Exactly Equal or IsTargetFrameworkCompatible?

The other property checks use exact-matching, and this seems like a safer/more limited default. We could change to use an is-compatible check based on user feedback.

How should this be represented in tooling/API?

The Object Model objects should have a fully-realized Condition, and there should be a new Well-Known metadata key on ItemGroups and PropertyGroups that provides a tooling-visible representation of the implicit Condition terms and their values.

Other notes

This syntax is very regular and I think could be more easily understood and well-formed-synthesized by LLM tools. The MSBuild language is not well known today by these tools, so something that is more simple and likely to be correct is a plus.

Could this be pluggable in some way?

It feels odd for MSBuild itself to know about specific Properties that may only be known to/used by a specific SDK. Could a given MSBuild SDK somehow provide this data?

Should compound values be allowed in the property comparisons (e.g. TargetFramework="net8.9;net9.0")

No - the intent of these comparisons is singular values only.

Alternative Designs

There are other ways to tackle this for sure -

  • have entirely different project file format (a la toml/ini/etc) that have a more natural pivoting syntax
  • make Property Functions for common conditions and keep the general Condition syntax intact
@baronfel baronfel added the Area: Language Issues impacting the MSBuild programming language. label Jan 24, 2025
@KalleOlaviNiemitalo
Copy link

Make <PropertyGroup TargetFramework="net8.0;net9.0"> an error.

@jnm2
Copy link

jnm2 commented Jan 24, 2025

This proposal makes a meaningful step towards user bliss. I resonate with the motivation.

A few points in favor of making the syntax If.Configuration="Release", or something in this vein:

  • The conditionality with If.Xyz is obvious. Configuration="Release" is a little less obviously a condition, and possibly able to be interpreted as still entering the group, but with some variable set that can affect evaluation.
  • The length is still quite short, and the meaning of If. is easy to digest.
  • It removes worries about conflicting with metadata, so this would work for items and properties and other types of elements too.
  • It provides a clear opportunity to extend this syntax to other conditions besides a hardcoded set in the future. Maybe automatically, or maybe with opt in.

For example, right off the bat I would miss being able to use this new syntax with TargetFrameworkIdentifier.

I also like the idea of ="net9.0" for exact match and ="net9.0+" for compatible match. It would be great if the design left room for future extensibility so that If.MyOwnLibraryProp="net9.0+" could be translated to the appropriate condition clause, rather than assuming a wooden translation to == 'net9.0+'.

@jnm2
Copy link

jnm2 commented Jan 24, 2025

Should compound values be allowed in the property comparisons (e.g. TargetFramework="net8.9;net9.0")

What will the experience be when TargetFramework="$(MyVariable)" contains a semicolon?

@Redth
Copy link
Member

Redth commented Jan 24, 2025

Definitely yearning for <PropertyGroup TargetPlatform="ios|maccatalyst"> (| to match any) and <PropertyGroup TargetPlatform="android">.

Properties I'd like to see:

  • TargetPlatform
  • TargetPlatformVersion
  • TargetFrameworkVersion

Ideally we'd be able to have conditions beside from just = like >, <, != ?

Lots of ways that syntax could be invented around this....

@StephaneDelcroix may have some thoughts given this is xml based, as he's thought through a lot of similar patterns for XAML.

@KalleOlaviNiemitalo
Copy link

The default LangVersion and TargetFrameworkVersion are set by .NET SDK after the project has been evaluated and TargetFramework is known. So they are usable in ItemGroup conditions but generally not in PropertyGroup. I do not want MSBuild to special-case If.TargetFrameworkVersion="8.0" to guess the future value based on TargetFramework if TargetFrameworkVersion has not been set yet. Such guessing would risk going wrong if the project uses custom TargetFramework not recognised by .NET SDK and will instead set TargetFrameworkVersion in some other way. More about that in NuGet/Home#5154.

@hez2010
Copy link

hez2010 commented Jan 24, 2025

We already have When in MSBuild:

 <Choose>
     <When Condition="$(Configuration)=='Test'">
        <PropertyGroup>
            <DebugSymbols>true</DebugSymbols>
            <DebugType>full</DebugType>
            <Optimize>false</Optimize>
            <DefineConstants>DEBUG;TRACE</DefineConstants>
        </PropertyGroup>
    </When>
</Choose>

I think it would be intuitive to use When.<PropertyName>="Value" instead of special casing those limited properties:

<PropertyGroup When.Configuration="Test">
    <DebugSymbols>true</DebugSymbols>
    <DebugType>full</DebugType>
    <Optimize>false</Optimize>
    <DefineConstants>DEBUG;TRACE</DefineConstants>
</PropertyGroup>

This can even be extended to support multiple conditions:

<PropertyGroup>
    <When>
        <Configuration>Release</Configuration>
        <Platform>x64</Platform>
    </When>

    <DebugSymbols>true</DebugSymbols>
    <DebugType>full</DebugType>
    <Optimize>false</Optimize>
    <DefineConstants>DEBUG;TRACE</DefineConstants>
</PropertyGroup>

@rainersigwald
Copy link
Member

This can even be extended to support multiple conditions:

<PropertyGroup>
    <When>
        <Configuration>Release</Configuration>
        <Platform>x64</Platform>
    </When>

    <DebugSymbols>true</DebugSymbols>
    <DebugType>full</DebugType>
    <Optimize>false</Optimize>
    <DefineConstants>DEBUG;TRACE</DefineConstants>
</PropertyGroup>

I expected this to be a breaking change; I expected it to define a property $(When) with some XML contents. But it looks like that's specifically prevented: error MSB4004: The "When" property is reserved, and cannot be modified.

Still, it looks so much like a regular property definition that I'm not a fan of this approach.

@rainersigwald
Copy link
Member

See also related #820.

@StephaneDelcroix
Copy link

StephaneDelcroix commented Jan 27, 2025

my .02€

It's always a headache to invent a comparison DSL to extend XML... the issue we're trying to solve is pattern matching with property pattern, and in that context, 'When' (<PropertyGroup When.Configuration="Release">) makes more sense than 'If', and both are clearer than <PropertyGroup Configuration="Release"> (unless it's a breaking change).

Multiple conditions could indeed be specified like in #11329 (comment), or simply using <PropertyGroup When.Configuration="Release" When.TargetPlatform="ios">

About multiple values, I read <PropertyGroup When.TargetPlatform="ios;maccatalyst"> as when target platform is in the list of values, so the semicolon makes sense as liste separator, better than |. (| would mean a & syntax exists, but that can't be true).

While we're inventing a syntax, we could have pattern matching <PropertyGroup When.Configuration="R*">, not <PropertyGroup When.Configuration="!Release"> or comparison <PropertyGroup When.TargetPlatformVersion=">=9.0">

In any form, a feature like this would be a big win for developers with non-trivial cases, and would make conditions easier to read, and write.

(feel free to delete those thoughts if they derail the OP's intend)

@KalleOlaviNiemitalo
Copy link

Re tooling changes, consider also how https://source.dot.net/ generated by https://github.com/KirillOsenkov/SourceBrowser searches for references to MSBuild properties and items. If an MSBuild project is using the When.TargetPlatform attribute syntax, then the web user should be able to click that and navigate to TargetPlatform definitions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Language Issues impacting the MSBuild programming language. triaged
Projects
None yet
Development

No branches or pull requests

8 participants