Handle frameworks and binding resource packages in Hot Restart #16605

mattjohnsonpint · 2022-11-06T20:40:12Z

Supports either regular framework or xcframework files coming from nuget packages
~~Updates the Unzip task to properly create symbolic links, which may occur within resource zips containing xcframeworks~~ (removed per comment)

Fixes #16571

@rolfbjarne

mattjohnsonpint · 2022-11-06T20:46:47Z

@microsoft-github-policy-service agree

github-actions · 2022-11-07T15:58:24Z

⚠️ Your code has been reformatted. ⚠️

If this is not desired, add the actions-disable-autoformat label, and revert the reformatting commit.

If files unrelated to your change were modified, try reverting the reformatting commit + merging with the target branch (and push those changes).

msbuild/Xamarin.iOS.Tasks.Windows/Xamarin.iOS.HotRestart.targets

msbuild/Xamarin.iOS.Tasks.Windows/Zip.cs

dalexsoto · 2022-11-07T19:00:19Z

/azp run

azure-pipelines · 2022-11-07T19:00:48Z

Azure Pipelines successfully started running 1 pipeline(s).

github-actions · 2022-11-07T19:03:45Z

⚠️ Your code has been reformatted. ⚠️

If this is not desired, add the actions-disable-autoformat label, and revert the reformatting commit.

If files unrelated to your change were modified, try reverting the reformatting commit + merging with the target branch (and push those changes).

dalexsoto · 2022-11-08T16:00:08Z

/azp run

azure-pipelines · 2022-11-08T16:00:36Z

Azure Pipelines successfully started running 1 pipeline(s).

dalexsoto · 2022-11-08T18:21:17Z

Looks like there is a build error going on

Build FAILED.

       "/Users/builder/azdo/_work/4/s/xamarin-macios/msbuild/Xamarin.MacDev.Tasks.sln" (default target) (1:2) ->
       "/Users/builder/azdo/_work/4/s/xamarin-macios/msbuild/Xamarin.iOS.Tasks.Windows/Xamarin.iOS.Tasks.Windows.csproj" (default target) (4:6) ->
       (CoreCompile target) -> 
         /Users/builder/azdo/_work/4/s/xamarin-macios/msbuild/Xamarin.iOS.Tasks.Windows/Zip.cs(55,37): error CS1061: 'FileInfo' does not contain a definition for 'LinkTarget' and no accessible extension method 'LinkTarget' accepting a first argument of type 'FileInfo' could be found (are you missing a using directive or an assembly reference?) [/Users/builder/azdo/_work/4/s/xamarin-macios/msbuild/Xamarin.iOS.Tasks.Windows/Xamarin.iOS.Tasks.Windows.csproj]
         /Users/builder/azdo/_work/4/s/xamarin-macios/msbuild/Xamarin.iOS.Tasks.Windows/Zip.cs(68,15): error CS0117: 'Directory' does not contain a definition for 'CreateSymbolicLink' [/Users/builder/azdo/_work/4/s/xamarin-macios/msbuild/Xamarin.iOS.Tasks.Windows/Xamarin.iOS.Tasks.Windows.csproj]
         /Users/builder/azdo/_work/4/s/xamarin-macios/msbuild/Xamarin.iOS.Tasks.Windows/Zip.cs(76,11): error CS0117: 'File' does not contain a definition for 'CreateSymbolicLink' [/Users/builder/azdo/_work/4/s/xamarin-macios/msbuild/Xamarin.iOS.Tasks.Windows/Xamarin.iOS.Tasks.Windows.csproj]
         /Users/builder/azdo/_work/4/s/xamarin-macios/msbuild/Xamarin.iOS.Tasks.Windows/Zip.cs(100,73): error CS1061: 'ZipArchiveEntry' does not contain a definition for 'ExternalAttributes' and no accessible extension method 'ExternalAttributes' accepting a first argument of type 'ZipArchiveEntry' could be found (are you missing a using directive or an assembly reference?) [/Users/builder/azdo/_work/4/s/xamarin-macios/msbuild/Xamarin.iOS.Tasks.Windows/Xamarin.iOS.Tasks.Windows.csproj]

    4 Warning(s)
    4 Error(s)

mattjohnsonpint · 2022-11-09T04:47:33Z

Am I not allowed to use .NET 6 APIs?

mattjohnsonpint · 2022-11-09T05:57:20Z

I added a net6.0 target, and updated the code to only extract symlinks for net6+. Is that allowed? Will the net6 version make its way to the correct package to be used by VS at runtime? I'm not set up to fully build and test end-to-end, nor can I see the azp build results, so I'm flying blind a bit here. I'd appreciate any assistance.

BTW - The other approach I considered would be to not rely on these new symlink APIs but instead to re-implement the readlink algorithm in C# to work directly against the zip archive. The result would be files on disk are always fully materialized, even if there's multiple copies of them. It is much more difficult than it seems though, as links can point to links, or contain links as portions of the path.

rolfbjarne · 2022-11-09T07:54:09Z

So there are a few problems here:

I added a net6.0 target, and updated the code to only extract symlinks for net6+. Is that allowed?

Yes, it's allowed, but...

Will the net6 version make its way to the correct package to be used by VS at runtime?

No, not at the moment. We'll eventually get there, but we're not there yet.

BTW - The other approach I considered would be to not rely on these new symlink APIs but instead to re-implement the readlink algorithm in C# to work directly against the zip archive. The result would be files on disk are always fully materialized, even if there's multiple copies of them. It is much more difficult than it seems though, as links can point to links, or contain links as portions of the path.

Unfortunately there are a few problems here:

I believe Windows may require admin privileges to create symlinks in some cases.
I don't think the code that installs the app on the device knows about windows symlinks.

On the other hand I realize that zipped xcframeworks might contain symlinks that won't be needed (if they're for a different platform), so it certainly makes sense to find a solution here.

However, given that implementing and testing this solution is likely to take a while, my suggestion would be to split this PR in two: one PR that only adds Hot Restart support for (xc)frameworks and binding resource packages, and then another one that tackles the symlink problem.

From what you've done in this PR it looks like the first part would only contain changes to .props and .targets files, so that's much easier to test yourself (you can just change these files locally).

mattjohnsonpint · 2022-11-09T07:59:09Z

OK, I will update this tomorrow and remove the zip changes.

Indeed in my own library, we only have symlink in the maccatalyst target. The ios target has none, and the zip extract finds them just fine. Since only the ios target is needed for hot restart, I think it is fine to not support symlinks for now.

rolfbjarne · 2022-11-09T19:19:19Z

Thanks! I'll have a look and see if I can write tests for this (and update the PR accordingly).

mattjohnsonpint · 2022-11-14T22:58:31Z

Found and fixed a small but important bug with an extra trailing backslash, which was throwing off some subsequent checks such as creating the correct stamp file in the DynamicFrameworks folder.

rolfbjarne · 2022-11-15T08:42:21Z

/azp run

azure-pipelines · 2022-11-15T08:42:47Z

Azure Pipelines successfully started running 1 pipeline(s).

vs-mobiletools-engineering-service2 · 2022-11-15T12:01:08Z

✅ API diff for current PR / commit

Legacy Xamarin (No breaking changes)

~~iOS~~ (no change detected)
~~tvOS~~ (no change detected)
~~watchOS~~ (no change detected)
~~macOS~~ (no change detected)

NET (empty diffs)

~~iOS~~: (empty diff detected)
~~tvOS~~: (empty diff detected)
~~MacCatalyst~~: (empty diff detected)
~~macOS~~: (empty diff detected)

❗ API diff vs stable (Breaking changes)

Legacy Xamarin (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)

iOS: vsdrops gist (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
tvOS: vsdrops gist (No breaking changes)
watchOS: vsdrops gist (No breaking changes)
macOS: vsdrops gist (No breaking changes)

.NET (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)

iOS: vsdrops gist (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
tvOS: vsdrops gist (No breaking changes)
MacCatalyst: vsdrops gist (No breaking changes)
macOS: vsdrops gist (No breaking changes)
Microsoft.iOS vs Microsoft.MacCatalyst: vsdrops gist

Legacy Xamarin (stable) vs .NET

iOS: vsdrops gist
tvOS: vsdrops gist
macOS: vsdrops gist

✅ Generator diff

Generator diff is empty

Pipeline on Agent
Hash: b5e6db47a305c26f97208c4722b79c0dbb025f16 [PR build]

vs-mobiletools-engineering-service2 · 2022-11-15T12:06:22Z

📚 [PR Build] Artifacts 📚

Packages generated

View packages

Pipeline on Agent XAMBOT-1100.Monterey'
Hash: b5e6db47a305c26f97208c4722b79c0dbb025f16 [PR build]

vs-mobiletools-engineering-service2 · 2022-11-15T12:07:10Z

💻 [PR Build] Tests on macOS M1 - Mac Big Sur (11.5) passed 💻

✅ All tests on macOS M1 - Mac Big Sur (11.5) passed.

Pipeline on Agent
Hash: b5e6db47a305c26f97208c4722b79c0dbb025f16 [PR build]

vs-mobiletools-engineering-service2 · 2022-11-15T15:24:18Z

🚀 [CI Build] Test results 🚀

Test results

✅ All tests passed on VSTS: simulator tests.

🎉 All 223 tests passed 🎉

Tests counts

✅ bcl: All 69 tests passed. Html Report (VSDrops) Download
✅ cecil: All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests: All 1 tests passed. Html Report (VSDrops) Download
✅ fsharp: All 7 tests passed. Html Report (VSDrops) Download
✅ framework: All 8 tests passed. Html Report (VSDrops) Download
✅ generator: All 2 tests passed. Html Report (VSDrops) Download
✅ interdependent_binding_projects: All 7 tests passed. Html Report (VSDrops) Download
✅ install_source: All 1 tests passed. Html Report (VSDrops) Download
✅ introspection: All 8 tests passed. Html Report (VSDrops) Download
✅ linker: All 65 tests passed. Html Report (VSDrops) Download
✅ mac_binding_project: All 1 tests passed. Html Report (VSDrops) Download
✅ mmp: All 2 tests passed. Html Report (VSDrops) Download
✅ mononative: All 12 tests passed. Html Report (VSDrops) Download
✅ monotouch: All 23 tests passed. Html Report (VSDrops) Download
✅ msbuild: All 2 tests passed. Html Report (VSDrops) Download
✅ mtouch: All 1 tests passed. Html Report (VSDrops) Download
✅ xammac: All 3 tests passed. Html Report (VSDrops) Download
✅ xcframework: All 8 tests passed. Html Report (VSDrops) Download
✅ xtro: All 2 tests passed. Html Report (VSDrops) Download

Pipeline on Agent
Hash: $(GIT_HASH) [PR build]

mandel-macaque · 2023-01-27T18:54:47Z

@rolfbjarne can you take a second view on this.

rolfbjarne · 2023-01-30T07:22:22Z

@rolfbjarne can you take a second view on this.

Yes, I'm working on adding tests for this scenario.

dalexsoto

Awesome work! Once get the tests up we'll merge it! Thanks a lot @mattjohnsonpint ❤️

mattjohnsonpint · 2023-02-01T03:11:51Z

Thanks! Now if we can just #16001 fixed, we'll have all the iOS on Windows stuff working well. 🙂

rolfbjarne · 2023-02-01T06:10:41Z

Thanks! Now if we can just #16001 fixed, we'll have all the iOS on Windows stuff working well. 🙂

Yep, I'm working on that one together with adding a test case for this issue.

rolfbjarne · 2023-05-08T15:41:02Z

So I wrote a test for this scenario, and it revealed that this wasn't as simple as we first thought - in fact it was much more complicated.

The good news is that I fixed all those issues, and the corresponding PR as been merged, so this PR is no longer needed (this means that xcframeworks and binding projects will finally work correctly with Hot Restart - but since the changes required were quite invasive, they won't be shipped until .NET 8. If you're willing to try a preview, they'll be available in the upcoming .NET 8 Preview 4).

mattjohnsonpint added 2 commits November 6, 2022 12:36

Collect frameworks from packages

698b1d2

Handle symbolic links when unzipping

2cf94b3

mattjohnsonpint requested review from emaf, mauroa and rolfbjarne as code owners November 6, 2022 20:40

mattjohnsonpint changed the title ~~Handle frameworks and binding packages in Hot Restart~~ Handle frameworks and binding resource packages in Hot Restart Nov 6, 2022

mattjohnsonpint and others added 2 commits November 7, 2022 07:23

Merge branch 'main' into fix-hotrestart

815fa53

Auto-format source code

6dc1759

rolfbjarne requested changes Nov 7, 2022

View reviewed changes

msbuild/Xamarin.iOS.Tasks.Windows/Xamarin.iOS.HotRestart.targets Show resolved Hide resolved

msbuild/Xamarin.iOS.Tasks.Windows/Zip.cs Outdated Show resolved Hide resolved

mattjohnsonpint added 2 commits November 7, 2022 08:38

Invoke new code and target

afd8193

fix edit

3796da3

mattjohnsonpint mentioned this pull request Nov 7, 2022

Cannot create native types when deployment is executed to iPhone from VS for Windows without pairing to MAC dotnet/maui#10800

Closed

mattjohnsonpint added 3 commits November 7, 2022 09:10

Import namespaces

e9e32f7

Add defensive tests

34cc44c

fix edit fail

3e1bcb8

Auto-format source code

08b0ed9

emaf approved these changes Nov 7, 2022

View reviewed changes

mattjohnsonpint requested review from rolfbjarne and removed request for mauroa November 8, 2022 02:15

This comment has been minimized.

Sign in to view

Extract symlinks on net6+ only

488e581

mattjohnsonpint added 3 commits November 9, 2022 07:59

Revert all changes to Zip.cs

82194eb

.

eba4590

Add comment

76b55e6

rolfbjarne self-assigned this Nov 9, 2022

Fix extra trailing backslash

396aaf2

Merge branch 'main' into fix-hotrestart

b5e6db4

rolfbjarne added the community Community contribution ❤ label Dec 21, 2022

Merge branch 'main' into fix-hotrestart

32511f1

mandel-macaque approved these changes Jan 27, 2023

View reviewed changes

dalexsoto approved these changes Feb 1, 2023

View reviewed changes

rolfbjarne closed this May 8, 2023

mattjohnsonpint mentioned this pull request May 10, 2023

[iOS] Path too long for MAUI device connected deploys getsentry/sentry-dotnet#2363

Closed

Handle frameworks and binding resource packages in Hot Restart #16605

Handle frameworks and binding resource packages in Hot Restart #16605

Uh oh!

Conversation

mattjohnsonpint commented Nov 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mattjohnsonpint commented Nov 6, 2022

Uh oh!

github-actions bot commented Nov 7, 2022

⚠️ Your code has been reformatted. ⚠️

Uh oh!

Uh oh!

Uh oh!

dalexsoto commented Nov 7, 2022

Uh oh!

azure-pipelines bot commented Nov 7, 2022

Uh oh!

github-actions bot commented Nov 7, 2022

⚠️ Your code has been reformatted. ⚠️

Uh oh!

dalexsoto commented Nov 8, 2022

Uh oh!

azure-pipelines bot commented Nov 8, 2022

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

dalexsoto commented Nov 8, 2022

Uh oh!

mattjohnsonpint commented Nov 9, 2022

Uh oh!

mattjohnsonpint commented Nov 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rolfbjarne commented Nov 9, 2022

Uh oh!

mattjohnsonpint commented Nov 9, 2022

Uh oh!

rolfbjarne commented Nov 9, 2022

Uh oh!

mattjohnsonpint commented Nov 14, 2022

Uh oh!

rolfbjarne commented Nov 15, 2022

Uh oh!

azure-pipelines bot commented Nov 15, 2022

Uh oh!

vs-mobiletools-engineering-service2 commented Nov 15, 2022

✅ API diff for current PR / commit

❗ API diff vs stable (Breaking changes)

✅ Generator diff

Uh oh!

vs-mobiletools-engineering-service2 commented Nov 15, 2022

📚 [PR Build] Artifacts 📚

Packages generated

Uh oh!

vs-mobiletools-engineering-service2 commented Nov 15, 2022

💻 [PR Build] Tests on macOS M1 - Mac Big Sur (11.5) passed 💻

Uh oh!

vs-mobiletools-engineering-service2 commented Nov 15, 2022

🚀 [CI Build] Test results 🚀

Test results

Tests counts

Uh oh!

mandel-macaque commented Jan 27, 2023

Uh oh!

rolfbjarne commented Jan 30, 2023

Uh oh!

dalexsoto left a comment

Choose a reason for hiding this comment

Uh oh!

mattjohnsonpint commented Feb 1, 2023

Uh oh!

rolfbjarne commented Feb 1, 2023

Uh oh!

rolfbjarne commented May 8, 2023

Uh oh!

Reviewers

Assignees

Labels

mattjohnsonpint commented Nov 6, 2022 •

edited

Loading

mattjohnsonpint commented Nov 9, 2022 •

edited

Loading