WPF projects that use a versioned custom SDK fail to build

[ThomasGoulet73]

Fixes #5239

Description:

WPF projects that use a versioned custom SDK fail to build: <Project Sdk="My.Custom.Sdk/1.2.3" />. Parse and add an explicit Version attribute when creating the temporary WPF .csproj.

Customer Impact:

Any customer that requires a WPF project with a versioned custom SDK with SourceGenerator/PackageReference support will be blocked until this is fixed.

Regression:

Yes, partially. Using a custom SDK with a version works in 5.0 and 6.0 with PackageReference support turned off.

Testing:

Added/ran new unit tests and confirmed custom SDKs work with fix. Verified WPF Samples build against markup compiler containing change. Performed additional manual validation.

Risk:

Low. The Sdk attribute parsing uses the same public method from Microsoft.Build.Framework used by msbuild.

[clairernovotny]

@ryalanms is there a way to get this in 6.0?

[ryalanms]

Thank you, @ThomasGoulet73. @clairernovotny: Yes, either the first servicing release or GA.

[dotMorten]

This is a showstopper for us for shipping on top of .NET6. I would really appreciate getting this for GA

[ryalanms]

@SamBent: Can you take a look at this? Unit tests looks good, all the WPF samples are building, and the original regression (custom versioned SDK that requires source generators) is now building correctly.

@vishalmsft: Nothing in the internal test automation covers this scenario (or the markup compiler, in general). Additional unit tests had to be written.

[ThomasGoulet73]

@ryalanms Is this something new in MSBuild or specific to WPF ? I cannot find anything about this in the doc and it does not seem to work using .Net SDK 6.0.100-rc.2.21505.57.

[ryalanms]

Yes, I don't think this is documented anywhere other than the API reference for msbuild.

https://docs.microsoft.com/en-us/dotnet/api/microsoft.build.framework.sdkreference.tryparse?view=msbuild-16-netcore#Microsoft_Build_Framework_SdkReference_TryParse_System_String_Microsoft_Build_Framework_SdkReference__

WPF needs to match msbuild's behavior when parsing the Project Sdk attributes, so the generated .csproj for the temp project build succeeds and fails the same way a non-WPF project does.

[ThomasGoulet73]

I didn't know about this API. Should we call SdkReference.TryParse directly instead of reproducing the parse here ? It's a public API and it does not require another dependency.

[ThomasGoulet73]

I tried using SdkReference.TryParse locally and it seems to be working. For now, I have it as a local commit. Let me know it you want me to push it, it's also a lot cleaner.

[ryalanms]

Yes, please push. Thanks. I will run through the tests again locally.

If returning early due to a parsing failure, make sure to not remove the element (edit: Sdk attribute, not element).

   static private void ReplaceImplicitImports(XmlDocument xmlProjectDoc)
        {
            if (xmlProjectDoc == null)
            {
                // When the parameters are not valid, simply return it, instead of throwing exceptions.
                return;
            }

            XmlNode root = xmlProjectDoc.DocumentElement;

            for (int i = 0; i < root.Attributes.Count; i++)
            {
                XmlAttribute xmlAttribute = root.Attributes[i] as XmlAttribute;

                if (xmlAttribute.Name.Equals("Sdk", StringComparison.OrdinalIgnoreCase))
                {
                    //  <Project Sdk="Microsoft.NET.Sdk">
                    //  <Project Sdk="My.Custom.Sdk/1.0.0" />
                    //  <Project Sdk="My.Custom.Sdk/min=1.0.0" />
            
                    var sdkValue = xmlAttribute.Value;

                    SdkReference sdkReference;
                    SdkReference.TryParse(sdkValue, out sdkReference);

                    if (sdkReference == null) return;

                    // Remove Sdk attribute
                    root.Attributes.Remove(xmlAttribute);

                    //
                    // Add explicit top import
                    //
                    //  <Import Project="Sdk.props" Sdk="Microsoft.NET.Sdk" />
                    //
                    XmlNode nodeImportProps = CreateImportProjectSdkNode(xmlProjectDoc, "Sdk.props", sdkReference);

                    // Prepend this Import to the root of the XML document
                    root.PrependChild(nodeImportProps);

                    //
                    // Add explicit bottom import
                    //
                    //  <Import Project="Sdk.targets" Sdk="Microsoft.NET.Sdk" 
                    //
                    XmlNode nodeImportTargets = CreateImportProjectSdkNode(xmlProjectDoc, "Sdk.targets", sdkReference);

                    // Append this Import to the end of the XML document
                    root.AppendChild(nodeImportTargets);
                }
            }
        }

        // Creates an XmlNode that contains an Import Project element
        //
        //  <Import Project="Sdk.props" Sdk="Microsoft.NET.Sdk" />
        static XmlNode CreateImportProjectSdkNode(XmlDocument xmlProjectDoc, string projectAttributeValue, SdkReference sdkReference)
        {
            XmlNode nodeImport = xmlProjectDoc.CreateElement("Import", xmlProjectDoc.DocumentElement.NamespaceURI);
            XmlAttribute projectAttribute = xmlProjectDoc.CreateAttribute("Project");
            projectAttribute.Value = projectAttributeValue;
            XmlAttribute sdkAttributeProps = xmlProjectDoc.CreateAttribute("Sdk");
            sdkAttributeProps.Value = sdkReference.Name;
            nodeImport.Attributes.Append(projectAttribute);
            nodeImport.Attributes.Append(sdkAttributeProps);

            if (!string.IsNullOrEmpty(sdkReference.Version))
            {
                XmlAttribute sdkVersionAttributeProps = xmlProjectDoc.CreateAttribute("Version");
                sdkVersionAttributeProps.Value = sdkReference.Version;
                nodeImport.Attributes.Append(sdkVersionAttributeProps);
            }

            if (!string.IsNullOrEmpty(sdkReference.MinimumVersion))
            {
                XmlAttribute sdkVersionAttributeProps = xmlProjectDoc.CreateAttribute("MinimumVersion");
                sdkVersionAttributeProps.Value = sdkReference.MinimumVersion;
                nodeImport.Attributes.Append(sdkVersionAttributeProps);
            }

            return nodeImport;
        }
    }

[ThomasGoulet73]

Done in c164066. I also modified some comments in the code and added other exemples of explicit imports after the transformation (Here and Here).

[SamBent]

LGTM, especially using MSBuild's parser instead of rolling our own.

[ryalanms]

All unit tests passed with the most recent revision. I'll submit this for approval if the WPF samples and AppCompat applications build without any issues.

[ryalanms]

@ThomasGoulet73: If this is approved, it will need to be retargeted to release/6.0.

[ThomasGoulet73]

@ryalanms Would you prefer that I open a separate PR for release/6.0 and keep this one targeted to main ?

I'm not familiar with the way you merge PR in previous release but in dotnet/runtime, they seem to merge PRs in main and then backport them to an older release.

But I don't mind retargeting to release/6.0 once/if approved and letting you handle porting to main.

[ryalanms]

This was approved by .NET Tactics in email. @ThomasGoulet73: I'll open another PR for 6.0. This one can be merged.

[dotMorten]

Thank you @ryalanms !

[ryalanms]

Thank you @ThomasGoulet73 for the investigation and fix.

[ThomasGoulet73]

@ryalanms I'm glad to see this merged, and thank you for the help!