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

Building universal macOS app may fail due to different .car file #12410

Closed
filipnavara opened this issue Aug 11, 2021 · 9 comments · Fixed by #12847
Closed

Building universal macOS app may fail due to different .car file #12410

filipnavara opened this issue Aug 11, 2021 · 9 comments · Fixed by #12847
Assignees
Labels
bug If an issue is a bug or a pull request a bug fix dotnet An issue or pull request related to .NET (6) dotnet-pri1 .NET 6: important for stable release
Projects
Milestone

Comments

@filipnavara
Copy link
Contributor

filipnavara commented Aug 11, 2021

I'll try to provide more details later but here's the gist of the problem:

  • net6.0-macos application with several ImageAsset elements in .csproj and <RuntimeIdentifiers>osx-x64;osx-arm64</RuntimeIdentifiers> in .csproj
  • Built using dotnet build MailClient/MailClient.csproj

The build failed with the following error:

/usr/local/share/dotnet/packs/Microsoft.macOS.Sdk/12.0.100-preview.7228/targets/Xamarin.Shared.Sdk.targets(308,3): error : Unable to merge the file 'Contents/Resources/Assets.car', it's different between the input app bundles. [/Users/filipnavara/Projects/emclient/MailClient/MailClient.csproj]
/usr/local/share/dotnet/packs/Microsoft.macOS.Sdk/12.0.100-preview.7228/targets/Xamarin.Shared.Sdk.targets(308,3): error : App bundle file #1: /Users/filipnavara/Projects/emclient/MailClient/bin/Debug/net6.0-macos/osx-x64/MailClient.app/Contents/Resources/Assets.car [/Users/filipnavara/Projects/emclient/MailClient/MailClient.csproj]
/usr/local/share/dotnet/packs/Microsoft.macOS.Sdk/12.0.100-preview.7228/targets/Xamarin.Shared.Sdk.targets(308,3): error : App bundle file #2: /Users/filipnavara/Projects/emclient/MailClient/bin/Debug/net6.0-macos/osx-arm64/MailClient.app/Contents/Resources/Assets.car [/Users/filipnavara/Projects/emclient/MailClient/MailClient.csproj]
    354 Warning(s)
    3 Error(s)

The files in question have indentical size but the content differs by one byte:
image

Note that the build takes quite a long time so the .car files were built roughly one minute apart from each other.

@spouliot spouliot added dotnet An issue or pull request related to .NET (6) dotnet-pri1 .NET 6: important for stable release labels Aug 11, 2021
@spouliot spouliot added this to the .NET 6 milestone Aug 11, 2021
@spouliot spouliot added the need-info Waiting for more information before the bug can be investigated label Aug 11, 2021
@spouliot
Copy link
Contributor

by one byte :(

what is shown if you run assetutil -I Assets.car on each file and diff them ?

@filipnavara
Copy link
Contributor Author

diff -u x64-assets.txt arm64-assets.txt
--- x64-assets.txt	2021-08-11 16:21:12.000000000 +0200
+++ arm64-assets.txt	2021-08-11 16:21:25.000000000 +0200
@@ -35,7 +35,7 @@
     "PlatformVersion" : "10.14",
     "SchemaVersion" : 5,
     "StorageVersion" : 17,
-    "Timestamp" : 1628686749
+    "Timestamp" : 1628686414
   },
   {
     "AssetType" : "Color",

@rolfbjarne rolfbjarne added this to Pri 1 - Not Started in .NET 6 Aug 11, 2021
@rolfbjarne
Copy link
Member

Yay timestamps 😒

I see two options:

  • Somehow parse the Assets.car files, determine that the differences are insignificant, and just pick one of them.
  • Adjust our build logic to only build Assets.car once.

@spouliot
Copy link
Contributor

Somehow parse the Assets.car files, determine that the differences are insignificant, and just pick one of them.

It's a BOM file so the header's location would need quite a bit of parsing, at least versus a static offset into the files :(

https://blog.timac.org/2018/1018-reverse-engineering-the-car-file-format/

@spouliot spouliot removed the need-info Waiting for more information before the bug can be investigated label Aug 11, 2021
@dalexsoto dalexsoto added the bug If an issue is a bug or a pull request a bug fix label Aug 19, 2021
@rolfbjarne
Copy link
Member

rolfbjarne commented Aug 26, 2021

The same thing happens for Mac Catalyst. Test app: xamarin/SubmissionSamples#39

@filipnavara
Copy link
Contributor Author

filipnavara commented Aug 27, 2021

So, funny story, the timestamp is actually unrelated. assetutil prints the modification or creation time of the file but it's not in the content so that was a red herring. The differences are at somewhat arbitrary places and it's always 0x00 vs 0x40 bytes. Now I had three of them in my last diff. They are not in any of the header blocks (as described in the reverse engineered file format). All the differences are in ISTC chunks (big endian CTSI, listed in the article above).

Further analysis shows that it's always the last byte of the name in the csimetadata. Since the name is zero-terminated this byte is irrelevant for the content.

@filipnavara
Copy link
Contributor Author

Here's a tool for doing semantic equality of the .car files: https://gist.github.com/filipnavara/9209449de9ed1fd50fa0e83f0374dc11

The algorithm is quite trivial but it's also unnecessary fragile.

@spouliot
Copy link
Contributor

namespace Wales
{
    class Cardiff

well played sir! 😄

wrt build performance I'd rather use option 2 (Adjust our build logic to only build Assets.car once) since building is not free and doing twice just to compare outputs only adds more time.

@rolfbjarne
Copy link
Member

build performance I'd rather use option 2

I agree, I'll figure out a way to do that.

@rolfbjarne rolfbjarne self-assigned this Sep 6, 2021
@rolfbjarne rolfbjarne moved this from Pri 1 - Not Started to September 2021 - In Progress in .NET 6 Sep 22, 2021
rolfbjarne added a commit to rolfbjarne/xamarin-macios that referenced this issue Sep 24, 2021
…multi-rid builds. Fixes xamarin#12410.

We don't need to compile project-level assets for every RuntimeIdentifier in multi-rid
builds, we can instead compile them just once in the outer build.

There is also a correctness issue here: we can't compile assets more than once and
expect to get the exact same compiled result every time (in particular actool seems
to be adding random bytes in to the compiled output), and this creates a problem
when trying to merge the different runtime-specific compiled output into a universal
binary.

We accomplish this by:

* Processing these assets in the outer build, before we execute the rid-specific
  inner builds.
* Store the paths to the assets we've processed in a file.
* In the inner builds, we read that file, and remove any matches from the corresponding
  item group.
* Make sure to copy the compiled assets to the app bundle at the end of the outer
  build.

These are the assets we currently handle this way:

* ImageAsset
* InterfaceDefinition
* SceneKitAsset
* Collada
* TextureAtlas
* CoreMLModel

Fixes xamarin#12410.
rolfbjarne added a commit to rolfbjarne/xamarin-macios that referenced this issue Sep 27, 2021
…multi-rid builds. Fixes xamarin#12410.

We don't need to compile project-level assets for every RuntimeIdentifier in multi-rid
builds, we can instead compile them just once in the outer build.

There is also a correctness issue here: we can't compile assets more than once and
expect to get the exact same compiled result every time (in particular actool seems
to be adding random bytes in to the compiled output), and this creates a problem
when trying to merge the different runtime-specific compiled output into a universal
binary.

We accomplish this by:

* Processing these assets in the outer build, before we execute the rid-specific
  inner builds.
* Store the paths to the assets we've processed in a file.
* In the inner builds, we read that file, and remove any matches from the corresponding
  item group.
* Make sure to copy the compiled assets to the app bundle at the end of the outer
  build.

These are the assets we currently handle this way:

* ImageAsset
* InterfaceDefinition
* SceneKitAsset
* Collada
* TextureAtlas
* CoreMLModel

Fixes xamarin#12410.
rolfbjarne added a commit to rolfbjarne/xamarin-macios that referenced this issue Sep 27, 2021
…multi-rid builds. Fixes xamarin#12410.

We don't need to compile project-level assets for every RuntimeIdentifier in multi-rid
builds, we can instead compile them just once in the outer build.

There is also a correctness issue here: we can't compile assets more than once and
expect to get the exact same compiled result every time (in particular actool seems
to be adding random bytes in to the compiled output), and this creates a problem
when trying to merge the different runtime-specific compiled output into a universal
binary.

We accomplish this by:

* Processing these assets in the outer build, before we execute the rid-specific
  inner builds.
* Store the paths to the assets we've processed in a file.
* In the inner builds, we read that file, and remove any matches from the corresponding
  item group.
* Make sure to copy the compiled assets to the app bundle at the end of the outer
  build.

These are the assets we currently handle this way:

* ImageAsset
* InterfaceDefinition
* SceneKitAsset
* Collada
* TextureAtlas
* CoreMLModel

Fixes xamarin#12410.
.NET 6 automation moved this from September 2021 - In Progress to September 2021 - Done Sep 30, 2021
rolfbjarne added a commit that referenced this issue Sep 30, 2021
…multi-rid builds. Fixes #12410. (#12847)

We don't need to compile project-level assets for every RuntimeIdentifier in
multi-rid builds, we can instead compile them just once in the outer build.

There is also a correctness issue here: we can't compile assets more than once
and expect to get the exact same compiled result every time (in particular
actool seems to be adding random bytes in to the compiled output), and this
creates a problem when trying to merge the different runtime-specific compiled
output into a universal binary.

We accomplish this by:

* Processing these assets in the outer build, before we execute the
  rid-specific inner builds.
* Store the paths to the assets we've processed in a file.
* In the inner builds, we read that file, and remove any matches from the
  corresponding item group.
* Make sure to copy the compiled assets to the app bundle at the end of the
  outer build.

These are the assets we currently handle this way:

* BundleResource
* ImageAsset
* InterfaceDefinition
* SceneKitAsset
* Collada
* TextureAtlas
* CoreMLModel

Also:

* Add a new test case (AppWithResource) that contains all these different
  types of assets.
* Add support for the ScnTool task on Mac Catalyst (which the new test case
  revealed was missing).

Fixes #12410.
@ghost ghost locked as resolved and limited conversation to collaborators Apr 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug If an issue is a bug or a pull request a bug fix dotnet An issue or pull request related to .NET (6) dotnet-pri1 .NET 6: important for stable release
Projects
No open projects
.NET 6
  
September 2021 - Done
4 participants