Skip to content
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

[msbuild] Fix computing the output path for SceneKitAsset items with custom Link metadata. Fixes #15104. #15186

Merged

Conversation

rolfbjarne
Copy link
Member

All files in a *.scnassets directory are included as SceneKitAsset items, but
when we process these files, Apple's tool take the *.scnassets directory as
input. This means that we use the first file inside a *.scnassets directory,
compute the path to the actual *.scnassets directory, and work with that path
from then on. The problem was that the Link metadata (if present) would be
copied from the file's metadata to the directory's metadata, and thus any
computations involving the Link metadata for the directory would be wrong.

Fix this by computing the correct Link metadata for the directory if the file
has it.

Also add a test.

Fixes #15104.

…custom Link metadata. Fixes xamarin#15104.

All files in a *.scnassets directory are included as SceneKitAsset items, but
when we process these files, Apple's tool take the *.scnassets directory as
input. This means that we use the first file inside a *.scnassets directory,
compute the path to the actual *.scnassets directory, and work with that path
from then on. The problem was that the Link metadata (if present) would be
copied from the file's metadata to the directory's metadata, and thus any
computations involving the Link metadata for the directory would be wrong.

Fix this by computing the correct Link metadata for the directory if the file
has it.

Also add a test.

Fixes xamarin#15104.
@rolfbjarne rolfbjarne added the bug If an issue is a bug or a pull request a bug fix label Jun 2, 2022
@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

// so we need to update the Link metadata accordingly (if it exists).
var link = scnassetsItem.GetMetadata ("Link");
if (!string.IsNullOrEmpty (link)) {
link = link.Substring (0, link.Length - (asset.ItemSpec.Length - scnassets.Length));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will link.Length always be greater than asset.ItemSpec.Length - scnassets.Length?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed!

@vs-mobiletools-engineering-service2

This comment has been minimized.

@rolfbjarne
Copy link
Member Author

Test failures are unrelated (https://github.com/xamarin/maccore/issues/2558).

@vs-mobiletools-engineering-service2
Copy link
Collaborator

🔥 [PR Build] Build failed 🔥

Build failed for the job 'Generate API diff'

Pipeline on Agent
Hash:

@vs-mobiletools-engineering-service2
Copy link
Collaborator

📚 [PR Build] Artifacts 📚

Packages generated

View packages

Pipeline on Agent XAMBOT-1109.Monterey'
Hash: d5ab674e70d78d4576e452306af909647db06a48

@vs-mobiletools-engineering-service2
Copy link
Collaborator

💻 [PR Build] Tests on macOS Mac Catalina (10.15) passed 💻

All tests on macOS Mac Catalina (10.15) passed.

Pipeline on Agent
Hash: d5ab674e70d78d4576e452306af909647db06a48

@vs-mobiletools-engineering-service2
Copy link
Collaborator

❌ [CI Build] Tests failed on VSTS: simulator tests iOS ❌

Tests failed on VSTS: simulator tests iOS.

Test results

5 tests failed, 143 tests passed.

Failed tests

  • monotouch-test/watchOS 32-bits - simulator/Debug: TimedOut
  • monotouch-test/watchOS 32-bits - simulator/Debug (LinkSdk): Crashed
  • monotouch-test/watchOS 32-bits - simulator/Debug (static registrar): Crashed
  • monotouch-test/watchOS 32-bits - simulator/Release (all optimizations): Crashed
  • fsharp/watchOS 32-bits - simulator/Debug: Crashed

Pipeline on Agent XAMBOT-1109.Monterey'
Merge d5ab674 into ac97efd

Copy link
Contributor

@chamons chamons left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One spacing nit.

…askBase.cs

Co-authored-by: Chris Hamons <chris.hamons@xamarin.com>
@rolfbjarne
Copy link
Member Author

Test failures are unrelated (https://github.com/xamarin/maccore/issues/2558)

@rolfbjarne rolfbjarne merged commit dcecc4d into xamarin:main Jun 7, 2022
@rolfbjarne rolfbjarne deleted the msbuild-fix-compilescenekitassets branch June 7, 2022 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug If an issue is a bug or a pull request a bug fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Build fails with .scnassets in .NET MAUI
7 participants