oleg-shilo / wixsharp

Framework for building a complete MSI or WiX source code by using script files written with C# syntax.
MIT License
1.12k stars 175 forks source link

AddDefaultNamespaces removing processing instructions #1667

Open ottoman52 opened 3 weeks ago

ottoman52 commented 3 weeks ago

The call to AddDefaultNamespaces in BuildWxs seems to strip processing instructions that were injected earlier

oleg-shilo commented 2 weeks ago

Can you please give me the example of "processing instructions that were injected earlier"?

ottoman52 commented 2 weeks ago

Im trying to inject my own edited version of this file using the Compiler.WixSourceGenerated event: https://github.com/wixtoolset/wix/blob/main/src/ext/UI/wixlib/WixUI_InstallDir.wxs

It has this segment:

<?foreach WIXUIARCH in X86;X64;A64 ?>
    <Fragment>
        <UI Id="WixUI_InstallDir_$(WIXUIARCH)">
            <Publish Dialog="LicenseAgreementDlg" Control="Print" Event="DoAction" Value="WixUIPrintEula_$(WIXUIARCH)" />
            <Publish Dialog="BrowseDlg" Control="OK" Event="DoAction" Value="WixUIValidatePath_$(WIXUIARCH)" Order="3" Condition="NOT WIXUI_DONTVALIDATEPATH" />
            <Publish Dialog="InstallDirDlg" Control="Next" Event="DoAction" Value="WixUIValidatePath_$(WIXUIARCH)" Order="2" Condition="NOT WIXUI_DONTVALIDATEPATH" />
        </UI>

        <UIRef Id="WixUI_InstallDir" />
    </Fragment>
<?endforeach?>

but when AddDefaultNamespaces gets called the <?foreach WIXUIARCH in X86;X64;A64 ?> bit is removed. It looks like that function calls doc.Root.RemoveAll(), then adds back in Attributes and Elements but not XProcessingInstruction's.

oleg-shilo commented 2 weeks ago

Great, thank you. Will check it on weekend

oleg-shilo commented 2 weeks ago

OK, I see. What you are trying to do is inject in the XML an xml-like content that has some xml-illegal statements. Thus WiX4 becomes so strict that even <?xml version="1.0" encoding="utf-8"?> statement aborts the build before it even reaches WiX compilation.

And when I debugged your use-case I saw the non-compliant tags removed on injection even before they reached WixSharp. They are removed by the .NET XDocument API:

image

You can even try to trick the system by forcing the non-compliant tags but AddDefaultNamespaces uses internally XElement.Parse, which again removes all processing instructions (tags starting with <?).

While changing the parsing to XDocument.Parse seems like a valid way for preserving the instructions, it's problematic to do that as it's not clear who (MSBuild or WiX compiler) and how is going to to process these instructions. IE <?xml version="1.0" encoding="utf-8"?> upsets WiX.

Thus it's more beneficial to rely on transparent fully debuggable XML manipulation techniques that every C# dev is familiar with.

image

Even though I prefer to do it even without WixSourceGenerated. Injecting XML is easy and can be done in various ways:

project.AddWixFragment("Wix/Product", XElement.Parse(@"
                <Feature Id=""BinaryOnlyFeature"" Title=""Sample Product Feature"" Level=""1"">
                    <ComponentRef Id=""Component.myapp_exe"" />
                </Feature>"));

project.AddXml("Wix/Product", "<Property Id=\"Title\" Value=\"Properties Test\" />");

project.AddXmlElement("Wix/Product", "Property", "Id=Gritting; Value=Hello World!");

Compiler.WixSourceGenerated += document =>
    {
        // merge 'Wix/Product' elements of document with 'Wix/Product' elements of CommonProperies.wxs
        document.InjectWxs("CommonProperies.wxs");
        . . .
ottoman52 commented 2 weeks ago

Basically I was trying to take the Wix UI code as is from their libs and include it in my project with some minor changes (through the WixSourceGenerated event and adding only the fragments).

I have since got this working using project.WxsFiles.Add which seems to be the more appropriate way to do what I want.

If you are interested in supporting XProcessingInstruction injected through WixSourceGenerated though, I confirmed that changing this code fixed my issue:

internal static XDocument AddDefaultNamespaces( this XDocument doc )
{
    if ( CanSetXmlNamespaceSafely )
    {
        XNamespace ns = doc.Root.Name.NamespaceName;
        doc.Root.Descendants().ForEach(x =>
        {
            if ( x.Name.Namespace.NamespaceName.IsEmpty() )
                x.Name = ns + x.Name.LocalName;
        });
    }
    else
    {
        //Using simplistic, inefficient but safe string manipulation with regeneration of all elements
        var xml = doc.ToString().Replace("xmlns=\"\"", "");
        var newRoot = XElement.Parse(xml);

        doc.Root.RemoveAll();

               // Current Code, doesnt add XProcessingInstruction
                //need to add namespaces (via attributes) as well
        //doc.Root.Add(newRoot.Attributes());
        //doc.Root.Add(newRoot.Elements());

               // Walking all nodes will pick up XProcessingInstruction
        foreach ( XNode node in newRoot.Nodes() )
            doc.Root.Add(node);
    }
    return doc;
}
ottoman52 commented 2 weeks ago

As someone new to Wix, Wix# has been a lifesaver by the way. Great work!

oleg-shilo commented 2 weeks ago

Great, glad it helped you :)

oleg-shilo commented 2 weeks ago

If you are interested in supporting XProcessingInstruction injected...

Fantastic. Your change makes perfect sense. Done, I have already incorporated it in both wix3 and wix4 streams. Will be available in the very next release.