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

AddDefaultNamespaces removing processing instructions #1667

Open
ottoman52 opened this issue Nov 1, 2024 · 8 comments
Open

AddDefaultNamespaces removing processing instructions #1667

ottoman52 opened this issue Nov 1, 2024 · 8 comments

Comments

@ottoman52
Copy link

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

@oleg-shilo
Copy link
Owner

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

@ottoman52
Copy link
Author

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
Copy link
Owner

Great, thank you. Will check it on weekend

@oleg-shilo
Copy link
Owner

oleg-shilo commented Nov 8, 2024

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
Copy link
Author

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
Copy link
Author

ottoman52 commented Nov 8, 2024

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

@oleg-shilo
Copy link
Owner

Great, glad it helped you :)

@oleg-shilo
Copy link
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants