[release/10.0.1xx] [dotnet] Fix handling binding frameworks embedded in binding assemblies remotely.#24538
Conversation
…es remotely. Adjust the FilterStaticFrameworks and GetFullPaths tasks to not copy any empty files from Windows to Mac, because that would most likely overwrite an existing non-empty file on the Mac. Fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2679691.
This reverts commit d79b463. wrong branch
There was a problem hiding this comment.
Pull request overview
This PR fixes a remote-build edge case where zero-length placeholder files on Windows could be copied to the Mac and overwrite real (non-empty) framework files, and adds a regression test scenario involving a binding project that embeds a framework.
Changes:
- Update remote MSBuild tasks (
FilterStaticFrameworks,GetFullPaths) to skip copying empty files from Windows to the Mac. - Add a new RemoteWindows unit test plus new dotnet test projects to reproduce/build an app referencing a binding project with an embedded framework.
- Allow overriding
NoBindingEmbeddingin an existing bindings test project by making its default conditional.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/dotnet/UnitTests/WindowsTest.cs | Adds a RemoteWindows build test for an app that references a binding project with an embedded framework. |
| tests/dotnet/EmbeddedFrameworkInBindingProjectApp/shared.mk | Shared make configuration for the new test app across platforms. |
| tests/dotnet/EmbeddedFrameworkInBindingProjectApp/shared.csproj | Shared project configuration and reference to the binding project for the new test app. |
| tests/dotnet/EmbeddedFrameworkInBindingProjectApp/AppDelegate.cs | Simple entry point that invokes a P/Invoke from the binding assembly to ensure the embedded framework is used. |
| tests/dotnet/EmbeddedFrameworkInBindingProjectApp/iOS/Makefile | Platform make entry for building the new test app (iOS). |
| tests/dotnet/EmbeddedFrameworkInBindingProjectApp/iOS/EmbeddedFrameworkInBindingProjectApp.csproj | iOS TFM project for the new test app. |
| tests/dotnet/EmbeddedFrameworkInBindingProjectApp/tvOS/Makefile | Platform make entry for building the new test app (tvOS). |
| tests/dotnet/EmbeddedFrameworkInBindingProjectApp/tvOS/EmbeddedFrameworkInBindingProjectApp.csproj | tvOS TFM project for the new test app. |
| tests/dotnet/EmbeddedFrameworkInBindingProjectApp/macOS/Makefile | Platform make entry for building the new test app (macOS). |
| tests/dotnet/EmbeddedFrameworkInBindingProjectApp/macOS/EmbeddedFrameworkInBindingProjectApp.csproj | macOS TFM project for the new test app. |
| tests/dotnet/EmbeddedFrameworkInBindingProjectApp/MacCatalyst/Makefile | Platform make entry for building the new test app (Mac Catalyst). |
| tests/dotnet/EmbeddedFrameworkInBindingProjectApp/MacCatalyst/EmbeddedFrameworkInBindingProjectApp.csproj | Mac Catalyst TFM project for the new test app. |
| tests/dotnet/BindingWithEmbeddedFramework/shared.mk | Shared make configuration for the new binding project across platforms. |
| tests/dotnet/BindingWithEmbeddedFramework/shared.csproj | Binding project configuration including NativeReference to a framework and enabling binding embedding. |
| tests/dotnet/BindingWithEmbeddedFramework/ApiDefinition.cs | Minimal ApiDefinition source for the new binding project. |
| tests/dotnet/BindingWithEmbeddedFramework/StructsAndEnums.cs | Defines a P/Invoke entry point into the embedded framework. |
| tests/dotnet/BindingWithEmbeddedFramework/MyClass.cs | Additional source file for the new binding project test fixture. |
| tests/dotnet/BindingWithEmbeddedFramework/iOS/Makefile | Platform make entry for building the new binding project (iOS). |
| tests/dotnet/BindingWithEmbeddedFramework/iOS/BindingWithEmbeddedFramework.csproj | iOS TFM project for the new binding project. |
| tests/dotnet/BindingWithEmbeddedFramework/tvOS/Makefile | Platform make entry for building the new binding project (tvOS). |
| tests/dotnet/BindingWithEmbeddedFramework/tvOS/BindingWithEmbeddedFramework.csproj | tvOS TFM project for the new binding project. |
| tests/dotnet/BindingWithEmbeddedFramework/macOS/Makefile | Platform make entry for building the new binding project (macOS). |
| tests/dotnet/BindingWithEmbeddedFramework/macOS/BindingWithEmbeddedFramework.csproj | macOS TFM project for the new binding project. |
| tests/dotnet/BindingWithEmbeddedFramework/MacCatalyst/Makefile | Platform make entry for building the new binding project (Mac Catalyst). |
| tests/dotnet/BindingWithEmbeddedFramework/MacCatalyst/BindingWithEmbeddedFramework.csproj | Mac Catalyst TFM project for the new binding project. |
| tests/bindings-framework-test/dotnet/shared.csproj | Makes NoBindingEmbedding default conditional so it can be overridden by callers. |
| msbuild/Xamarin.MacDev.Tasks/Tasks/GetFullPaths.cs | Avoids copying empty files to the remote Mac during GetFullPaths remote execution. |
| msbuild/Xamarin.MacDev.Tasks/Tasks/FilterStaticFrameworks.cs | Avoids copying empty files to the remote Mac when staging framework contents for remote execution. |
| } | ||
| if (!Directory.Exists (fw)) | ||
| continue; | ||
| foreach (var file in Directory.EnumerateFiles (fw, "*.*", SearchOption.AllDirectories)) { |
There was a problem hiding this comment.
GetAdditionalItemsToBeCopied still yields every file in the framework directory without filtering out zero-length files. If the Windows side contains placeholder/empty files inside the framework directory, they will still be copied to the Mac and can overwrite non-empty files there. Consider skipping any enumerated file where new FileInfo (file).Length == 0 (and/or reusing the same empty-file logic for each yielded file).
| foreach (var file in Directory.EnumerateFiles (fw, "*.*", SearchOption.AllDirectories)) { | |
| foreach (var file in Directory.EnumerateFiles (fw, "*.*", SearchOption.AllDirectories)) { | |
| var fileInfo = new FileInfo (file); | |
| if (fileInfo.Length == 0) { | |
| // an empty file is most likely an output file from the Mac, so don't overwrite the corresponding file on the Mac with the empty output file from Windows | |
| Log.LogMessage (MessageImportance.Low, "Not copying {0} to the Mac, it's an empty file.", file); | |
| continue; | |
| } |
✅ [CI Build #f157327] Build passed (Build packages) ✅Pipeline on Agent |
✅ [PR Build #f157327] Build passed (Detect API changes) ✅Pipeline on Agent |
✅ API diff for current PR / commitNET (empty diffs)✅ API diff vs stableNET (empty diffs)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
✅ [CI Build #f157327] Build passed (Build macOS tests) ✅Pipeline on Agent |
💻 [CI Build #f157327] Tests on macOS X64 - Mac Sonoma (14) passed 💻✅ All tests on macOS X64 - Mac Sonoma (14) passed. Pipeline on Agent |
💻 [CI Build #f157327] Tests on macOS M1 - Mac Monterey (12) passed 💻✅ All tests on macOS M1 - Mac Monterey (12) passed. Pipeline on Agent |
💻 [CI Build #f157327] Tests on macOS M1 - Mac Ventura (13) passed 💻✅ All tests on macOS M1 - Mac Ventura (13) passed. Pipeline on Agent |
💻 [CI Build #f157327] Tests on macOS arm64 - Mac Sequoia (15) passed 💻✅ All tests on macOS arm64 - Mac Sequoia (15) passed. Pipeline on Agent |
💻 [CI Build #f157327] Tests on macOS arm64 - Mac Tahoe (26) passed 💻✅ All tests on macOS arm64 - Mac Tahoe (26) passed. Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
🚀 [CI Build #f157327] Test results 🚀Test results✅ All tests passed on VSTS: test results. 🎉 All 128 tests passed 🎉 Tests counts✅ cecil: All 1 tests passed. Html Report (VSDrops) Download Pipeline on Agent |
Adjust the FilterStaticFrameworks and GetFullPaths tasks to not copy any empty files
from Windows to Mac, because that would most likely overwrite an existing non-empty
file on the Mac.
Fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2679691.
Backport of #24518.