[msbuild] Fix some issues with various types of resources when building from Windows. #21586

rolfbjarne · 2024-11-06T20:28:03Z

While adding a test for a new feature, it turned out that the new test uncovered existing bugs. So here I'm adding that new test, and fixing those bugs.

The main part is that computing the virtual project path for resource items on Windows wasn't correct, in particular when referencing resources outside the current project directory. The existing code didn't even have enough information to compute the correct values...

Sidenote: the virtual project path is used to decide where in an app bundle a particular resource should go.

So:

Add new metadata to resource items, for the path of the defining project + the path of the main project. This is required to compute the correct virtual project path.
Completely rewrite the code to compute virtual project path, prioritizing making the code legible and easy to understand (in particular minimizing differences in code paths between macOS and Windows), and add lots of comments.
Add unit tests for computing the virtual project path.
Add a new .NET project test for all variations of resources we support.
Run said new test on Windows as well, both in remote and local (Hot Restart) mode.

github-actions · 2024-11-07T10:09:33Z

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

This way we treat collada assets like any other compilable asset/resource.

…rce.GetLogicalName. * Completely rework the code to make it more readable add lots of comments explaining what it's doing. * Fix some issues on Windows: for each item whose virtual project path or logical name we want to compute, we need to know the path to the MSBuild file that defined the item in question ('DefiningProjectFullPath'), as well as the path to the project file ('MSBuildProjectFullPath'). Unfortunately, when executing remotely, these values are not the original values, which means we need to store them as additional metadata ('LocalDefiningProjectFullPath' and 'LocalMSBuildProjectFullPath', respectively) in our MSBuild logic, so that we can access them in the task. Do this by adding a new target ("_SetResourceMetadata") that sets these new metadata. * Add lots of comments explaining what the code does, since it's not the first time we've tinkered with these functions. Also add tests to make sure we don't regress. * Add optional tracing to ease future debugging. * Enable nullability and fix any issues.

github-actions · 2024-11-08T14:12: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).

…ce-tests-on-windows

vs-mobiletools-engineering-service2 · 2024-11-20T13:25:47Z

✅ API diff for current PR / commit

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

✅ API diff vs stable

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

ℹ️ Generator diff

Generator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes)

Pipeline on Agent
Hash: f083830fcadeb53ae0fc1b6fdb8b9998d9696020 [PR build]

vs-mobiletools-engineering-service2 · 2024-11-20T13:27:02Z

💻 [CI Build] Windows Integration Tests passed 💻

✅ All Windows Integration Tests passed.

Pipeline on Agent
Hash: f083830fcadeb53ae0fc1b6fdb8b9998d9696020 [PR build]

vs-mobiletools-engineering-service2 · 2024-11-20T13:28:42Z

💻 [CI Build] Tests on macOS M1 - Mac Monterey (12) passed 💻

✅ All tests on macOS M1 - Mac Monterey (12) passed.

Pipeline on Agent
Hash: f083830fcadeb53ae0fc1b6fdb8b9998d9696020 [PR build]

vs-mobiletools-engineering-service2 · 2024-11-20T13:32:28Z

💻 [CI Build] Tests on macOS X64 - Mac Sonoma (14) passed 💻

✅ All tests on macOS X64 - Mac Sonoma (14) passed.

Pipeline on Agent
Hash: f083830fcadeb53ae0fc1b6fdb8b9998d9696020 [PR build]

vs-mobiletools-engineering-service2 · 2024-11-20T13:35:12Z

💻 [CI Build] Tests on macOS M1 - Mac Ventura (13) passed 💻

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

Pipeline on Agent
Hash: f083830fcadeb53ae0fc1b6fdb8b9998d9696020 [PR build]

vs-mobiletools-engineering-service2 · 2024-11-20T16:49:21Z

🚀 [CI Build] Test results 🚀

Test results

✅ All tests passed on VSTS: test results.

🎉 All 109 tests passed 🎉

Tests counts

✅ cecil: All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (iOS): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (MacCatalyst): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (macOS): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (Multiple platforms): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (tvOS): All 1 tests passed. Html Report (VSDrops) Download
✅ framework: All 2 tests passed. Html Report (VSDrops) Download
✅ fsharp: All 4 tests passed. Html Report (VSDrops) Download
✅ generator: All 3 tests passed. Html Report (VSDrops) Download
✅ interdependent-binding-projects: All 4 tests passed. Html Report (VSDrops) Download
✅ introspection: All 6 tests passed. Html Report (VSDrops) Download
✅ linker: All 40 tests passed. Html Report (VSDrops) Download
✅ monotouch (iOS): All 9 tests passed. Html Report (VSDrops) Download
✅ monotouch (MacCatalyst): All 10 tests passed. Html Report (VSDrops) Download
✅ monotouch (macOS): All 9 tests passed. [attempt 2] Html Report (VSDrops) Download
✅ monotouch (tvOS): All 9 tests passed. Html Report (VSDrops) Download
✅ msbuild: All 2 tests passed. Html Report (VSDrops) Download
✅ xcframework: All 4 tests passed. Html Report (VSDrops) Download
✅ xtro: All 1 tests passed. Html Report (VSDrops) Download

Pipeline on Agent
Hash: f083830fcadeb53ae0fc1b6fdb8b9998d9696020 [PR build]

emaf · 2024-11-20T19:48:30Z

@rolfbjarne the changes look good. When building remotely and you have files that are outside of the project directory (like NuGet packages), are those still copied into project's directory in the Mac side?

In general, the problem with files that are not inside the project dir on the Windows side, is that we should not copy those to the Mac outside of the projects dir because those files could ed up anywhere in the Mac.

rolfbjarne · 2024-11-21T09:04:23Z

When building remotely and you have files that are outside of the project directory (like NuGet packages), are those still copied into project's directory in the Mac side?

I haven't tested resources from NuGet packages, but I'll test that and see what happens.

In general, the problem with files that are not inside the project dir on the Windows side, is that we should not copy those to the Mac outside of the projects dir because those files could end up anywhere in the Mac.

Yes, that's one of the problems I noticed when I wrote this test: resource files ended up outside the project directory in the Mac.

rolfbjarne · 2024-11-22T10:38:53Z

When building remotely and you have files that are outside of the project directory (like NuGet packages), are those still copied into project's directory in the Mac side?

I haven't tested resources from NuGet packages, but I'll test that and see what happens.

Everything I tried ended up copying files from NuGets into the project directory on the Mac (or failing to build before that point), so this looks good.

This comment has been minimized.

Sign in to view

rolfbjarne added 5 commits November 8, 2024 15:08

[tests] Add test.

5832c7a

[msbuild] Compile collada assets in library projects as well.

aeccac6

This way we treat collada assets like any other compilable asset/resource.

[devops] Provision simulator on remote Mac bots.

bc95b3c

[devops] Set the right environment variables.

820f1ec

rolfbjarne force-pushed the dev/rolf/tests-resource-tests-on-windows branch from 5de156d to e70ae8c Compare November 8, 2024 14:08

Auto-format source code

0da6360

This comment has been minimized.

Sign in to view

rolfbjarne added 2 commits November 19, 2024 11:50

Update test.

79f4d36

Merge remote-tracking branch 'origin/main' into dev/rolf/tests-resour…

fd25eae

…ce-tests-on-windows

This comment has been minimized.

Sign in to view

rolfbjarne added 3 commits November 20, 2024 12:59

Fix typo

3d9b974

Disable debug code

9a0a245

Merge remote-tracking branch 'origin/main' into dev/rolf/tests-resour…

f083830

…ce-tests-on-windows

rolfbjarne marked this pull request as ready for review November 20, 2024 12:03

rolfbjarne requested review from emaf and mauroa as code owners November 20, 2024 12:03

This comment has been minimized.

Sign in to view

dalexsoto approved these changes Nov 21, 2024

View reviewed changes

emaf approved these changes Nov 21, 2024

View reviewed changes

mauroa approved these changes Nov 21, 2024

View reviewed changes

rolfbjarne merged commit 746f531 into main Nov 22, 2024

rolfbjarne deleted the dev/rolf/tests-resource-tests-on-windows branch November 22, 2024 10:39

[msbuild] Fix some issues with various types of resources when building from Windows. #21586

[msbuild] Fix some issues with various types of resources when building from Windows. #21586

Uh oh!

Conversation

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

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Nov 7, 2024

⚠️ Your code has been reformatted. ⚠️

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Nov 8, 2024

⚠️ Your code has been reformatted. ⚠️

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

vs-mobiletools-engineering-service2 commented Nov 20, 2024

✅ API diff for current PR / commit

✅ API diff vs stable

ℹ️ Generator diff

Uh oh!

vs-mobiletools-engineering-service2 commented Nov 20, 2024

💻 [CI Build] Windows Integration Tests passed 💻

Uh oh!

vs-mobiletools-engineering-service2 commented Nov 20, 2024

💻 [CI Build] Tests on macOS M1 - Mac Monterey (12) passed 💻

Uh oh!

vs-mobiletools-engineering-service2 commented Nov 20, 2024

💻 [CI Build] Tests on macOS X64 - Mac Sonoma (14) passed 💻

Uh oh!

vs-mobiletools-engineering-service2 commented Nov 20, 2024

💻 [CI Build] Tests on macOS M1 - Mac Ventura (13) passed 💻

Uh oh!

This comment has been minimized.

vs-mobiletools-engineering-service2 commented Nov 20, 2024

🚀 [CI Build] Test results 🚀

Test results

Tests counts

Uh oh!

emaf commented Nov 20, 2024

Uh oh!

rolfbjarne commented Nov 21, 2024

Uh oh!

rolfbjarne commented Nov 22, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

rolfbjarne commented Nov 6, 2024 •

edited

Loading