[msbuild/dotnet] Rework how we collect and process native references. #17554

rolfbjarne · 2023-02-16T12:00:14Z

Previously, we'd do this:

Collect all possible native references.
Extract any compressed native references (*.framework.zip, *.xcframework.zip,
*.resources.zip) to disk.
Resolve the resulting native references.

This doesn't work very well on Windows (in non-connected/Hot Restart mode),
because some compressed files may contain symlinks (in particular compressed
xcframeworks). If those symlinks are for any other platform than the one we're
building for, they shouldn't matter, but if we extract the entire compressed
xcframework before figuring out what we need from it, we'd run into symlinks
and not knowing whether they should be ignored or not.

So rework the process to:

Collect all possible native references.
Resolve the resulting native references, peeking into zip files if need be.
Extract any compressed native references, but only the parts of the zip we need.

This way we won't run into any symlinks unless we really need them, and it
should also improve build performance slightly, even on macOS, since we're not
extracting files we won't need (which can be significant for xcframeworks).

Additionally:

Add support for unzipping on Windows by using System.IO.Compression.
Show an error if attempting to extract a symlink in the last step in the
reworked process on Windows.
Some tests had to be updated (since they poked into internals of the
ResolveNativeReferences task, and those internals have changed).

github-actions · 2023-02-16T12:05:15Z

⚠️ 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).

Previously, we'd do this: * Collect all possible native references. * Extract any compressed native references (*.framework.zip, *.xcframework.zip, *.resources.zip) to disk. * Resolve the resulting native references. This doesn't work very well on Windows (in non-connected/Hot Restart mode), because some compressed files may contain symlinks (in particular compressed xcframeworks). If those symlinks are for any other platform than the one we're building for, they shouldn't matter, but if we extract the entire compressed xcframework before figuring out what we need from it, we'd run into symlinks and not knowing whether they should be ignored or not. So rework the process to: * Collect all possible native references. * Resolve the resulting native references, peeking into zip files if need be. * Extract any compressed native references, but only the parts of the zip we need. This way we won't run into any symlinks unless we really need them, and it should also improve build performance slightly, even on macOS, since we're not extracting files we won't need (which can be significant for xcframeworks). Additionally: * Add support for unzipping on Windows by using System.IO.Compression. * Show an error if attempting to extract a symlink in the last step in the reworked process on Windows. * Some tests had to be updated (since they poked into internals of the ResolveNativeReferences task, and those internals have changed).

github-actions · 2023-03-22T08:18:52Z

⚠️ 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).

emaf · 2023-03-28T22:05:08Z

msbuild/Xamarin.MacDev.Tasks/Tasks/ResolveNativeReferencesBase.cs


 #nullable enable

 namespace Xamarin.MacDev.Tasks {

+	// We can get numerous types of native references:
+	//
+	//	*.dylib


nit: comment indentation

@emaf fixed!

…mpression

vs-mobiletools-engineering-service2 · 2023-03-29T10:09:53Z

💻 [CI Build] Windows Integration Tests passed 💻

✅ All Windows Integration Tests passed.

Pipeline on Agent
Hash: 0395fdfbbc629d0776f07c0205952d269760fb6d [PR build]

vs-mobiletools-engineering-service2 · 2023-03-29T10:10:01Z

✅ 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

Legacy Xamarin (No breaking changes)

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

.NET (No breaking changes)

iOS: vsdrops gist (No breaking changes)
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: 0395fdfbbc629d0776f07c0205952d269760fb6d [PR build]

vs-mobiletools-engineering-service2 · 2023-03-29T10:13:11Z

💻 [PR Build] Tests on macOS M1 - Mac Ventura (13.0) passed 💻

✅ All tests on macOS M1 - Mac Ventura (13.0) passed.

Pipeline on Agent
Hash: 0395fdfbbc629d0776f07c0205952d269760fb6d [PR build]

vs-mobiletools-engineering-service2 · 2023-03-29T10:16:21Z

📚 [PR Build] Artifacts 📚

Packages generated

View packages

Pipeline on Agent XAMMINI-015.Ventura
Hash: 0395fdfbbc629d0776f07c0205952d269760fb6d [PR build]

vs-mobiletools-engineering-service2 · 2023-03-29T10:18:05Z

💻 [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: 0395fdfbbc629d0776f07c0205952d269760fb6d [PR build]

vs-mobiletools-engineering-service2 · 2023-03-29T18:04:26Z

🚀 [CI Build] Test results 🚀

Test results

✅ All tests passed on VSTS: simulator tests.

🎉 All 225 tests passed 🎉

Tests counts

✅ bcl: All 69 tests passed. [attempt 2] 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 25 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: 0395fdfbbc629d0776f07c0205952d269760fb6d [PR build]

emaf · 2023-03-29T21:39:03Z

msbuild/Xamarin.MacDev.Tasks/Decompress.cs

+			bool rv;
+			if (Environment.OSVersion.Platform == PlatformID.Win32NT) {
+				rv = TryDecompressUsingSystemIOCompression (log, zip, resource, decompressionDir);
+			} else if (!string.IsNullOrEmpty (Environment.GetEnvironmentVariable ("XAMARIN_USE_SYSTEM_IO_COMPRESSION"))) {


Shouldn't this be part of the previous if statement using an or operator instead of in an else if statement since both execute the exact same line?

emaf · 2023-03-29T21:47:11Z

msbuild/Xamarin.MacDev.Tasks/Decompress.cs

+				if (entryPath.Length == 0)
+					continue;
+
+				if (entryPath.StartsWith (resourceAsDir, StringComparison.Ordinal)) {


Similar question here, is it written in this way instead of using a single if statement to make it easier to read?

Yes, they're both written this way to make them easier to read.

dalexsoto

❤️