-
Notifications
You must be signed in to change notification settings - Fork 548
[windows][msbuild] Copy entire directory when the native reference is a framework #11868
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
Conversation
… a framework The entire `.framework` directory needs to be copied back to Windows when a native reference is a [xc]framework. Otherwise important files will be missing and the app bundle will be unusable. Fix https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1339824
| namespace Xamarin.iOS.Tasks | ||
| { | ||
| class CustomMTouchTask : MTouchTaskBase | ||
| class CustomMTouchTask : MTouch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The GetAdditionalItemsToBeCopied I modified (and added tests to) is not part of MTouchTaskBase, only from theMTouch subclass.
@emaf will correct me if needed :)
*Base tasks generally have all the logic required for running on the Mac.
The version running on Windows subclass them to add the behaviour required to remote the execution.
The version running on Mac also subclass *Base but it's empty (on purpose). That makes the msbuild target files identical (same names for tasks).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's correct, but there are no more empty subclasses now :). Because of .NET6 we "unified" both task implementations (Windows and Mac) on this repo, but what Sebastien said is still valid:
- Base classes have the core logic of the task
- The unified subclass contains code that enables running the task remotely (from Windows), but if it's being executed from a Mac it directly executes the base class https://github.com/xamarin/xamarin-macios/blob/main/msbuild/Xamarin.iOS.Tasks/Tasks/MTouch.cs#L16-L17. MTouch is one of the most complex tasks we have, ACTool is another good example because it's way simpler https://github.com/xamarin/xamarin-macios/blob/main/msbuild/Xamarin.iOS.Tasks/Tasks/ACTool.cs#L10-L13. ACTool does not implement ITaskCallback so there's no implementation for
GetAdditionalItemsToBeCopiedin that case, that's just for customizing what files need to be copied when executing remotely.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔥 Tests failed catastrophically on Build (no summary found). 🔥Result file $(TEST_SUMMARY_PATH) not found. GitHub pagesResults can be found in the following github pages (it might take some time to publish): |
|
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
✅ [PR Build] Tests passed on Build. ✅Tests passed on Build. API diff✅ API Diff from stable View API diffAPI & Generator diff✅ API Diff (from PR only) (no change) GitHub pagesResults can be found in the following github pages (it might take some time to publish): 🎉 All 110 tests passed 🎉Pipeline on Agent XAMBOT-1100.BigSur' |
|
/sudo backport d16-10 |
|
Backport Job to branch d16-10 Created! The magic is happening here |
|
Hooray! Backport succeeded! Please see https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=4852658 for more details. |
|
@spouliot I might still be hitting this with VS17.1preview and .NET MAUI: https://developercommunity.visualstudio.com/t/remote-deploy-to-iphone-fails-with-packageinspecti/1600912 |
|
@dotMorten I'm no longer working for Microsoft. You better file a bug report and link it to this PR. FWIW it'a more likely a similar issue then the same one since there tests were added, in the same PR, for the original problem. |
|
@spouliot Thanks and best of luck in your new adventure. Bug already logged (link right above) |
The entire
.frameworkdirectory needs to be copied back to Windowswhen a native reference is a [xc]framework. Otherwise important files
will be missing and the app bundle will be unusable.
Fix https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1339824