feat(app): Implement FirebaseLinkXmlProcessor to merge package linker rules natively during builds#1490
Conversation
… rules natively during builds
Wiz Scan Summary
To detect these findings earlier in the dev lifecycle, try using Wiz Code VS Code Extension. |
There was a problem hiding this comment.
Code Review
This pull request introduces FirebaseLinkXmlProcessor to automate the merging of link.xml files from Firebase UPM packages and Assets/Firebase into a single temporary linker file, preventing managed code stripping of reflection-accessed methods. Feedback suggests improving XML parsing robustness by validating the <linker> root element and filtering for element nodes, as well as narrowing the file search pattern from *link.xml to exactly link.xml to avoid processing unrelated files.
| if (doc.DocumentElement != null) { | ||
| // Traverse the child elements under the root <linker> element of each link.xml. | ||
| // NOTE: We only support and extract `<assembly>` elements at this time. Root-level | ||
| // `<type>` or `<linker>` attributes other than assembly declarations are ignored. | ||
| foreach (XmlNode child in doc.DocumentElement.ChildNodes) { | ||
| if (child.Name == "assembly") { | ||
| XmlNode importedNode = mergedLinkxmlDoc.ImportNode(child, true); | ||
| root.AppendChild(importedNode); | ||
| hasAssemblies = true; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
To ensure robustness and avoid processing unrelated XML files, we should validate that the root element of the XML document is indeed <linker>. Additionally, we should verify that the child node is an element node before checking its name, preventing potential issues with comments or whitespace nodes.
if (doc.DocumentElement != null && doc.DocumentElement.Name == "linker") {
// Traverse the child elements under the root <linker> element of each link.xml.
// NOTE: We only support and extract `<assembly>` elements at this time. Root-level
// `<type>` or `<linker>` attributes other than assembly declarations are ignored.
foreach (XmlNode child in doc.DocumentElement.ChildNodes) {
if (child.NodeType == XmlNodeType.Element && child.Name == "assembly") {
XmlNode importedNode = mergedLinkxmlDoc.ImportNode(child, true);
root.AppendChild(importedNode);
hasAssemblies = true;
}
}
}| private void FindLinkXmlFiles(string dir, HashSet<string> files) { | ||
| if (!Directory.Exists(dir)) return; | ||
| try { | ||
| foreach (var file in Directory.GetFiles(dir, "*link.xml", SearchOption.AllDirectories)) { |
There was a problem hiding this comment.
Using the wildcard *link.xml can match unrelated files (e.g., custom_link.xml or backup_link.xml) that are not actual Unity linker files. Since Unity specifically looks for files named exactly link.xml, we should restrict the search pattern to "link.xml" to prevent merging incorrect files.
foreach (var file in Directory.GetFiles(dir, "link.xml", SearchOption.AllDirectories)) {
✅ Integration test succeeded!Requested by @firebase-workflow-trigger[bot] on commit 52b36f9 |
| } | ||
|
|
||
| // Get all link.xml files from Assets/Firebase. | ||
| string assetsFirebasePath = Path.GetFullPath("Assets/Firebase"); |
There was a problem hiding this comment.
Talked offline, but good to verify this won't cause issues with the link xml files in the Assets essentially being double defined
There was a problem hiding this comment.
I have verified that the link.xml double inclusion does not impact the building or cause any errors.
…nfo logs, elevate missing assembly log to warning
Description
Implement automated linker rules merging for UPM packages to prevent managed code stripping from removing reflection-accessed methods (like FirebaseAuth and FirebaseAppCheck).
This will run as an editor script in unity.
Leverages https://docs.unity3d.com/6000.0/Documentation/ScriptReference/Build.IUnityLinkerProcessor.GenerateAdditionalLinkXmlFile.html
As suggested from https://discussions.unity.com/t/the-current-state-of-link-xml-in-packages/814559/5
Testing
Built UPM and Unity Packages and tested locally.
Will test again once the packages are built in CI
Type of Change
Place an
xthe applicable box: