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

Update iOS native packaging #3044

Merged
merged 9 commits into from
Nov 14, 2021
Merged

Update iOS native packaging #3044

merged 9 commits into from
Nov 14, 2021

Conversation

huoyaoyuan
Copy link
Contributor

@huoyaoyuan huoyaoyuan commented Nov 27, 2019

Fixes #3030 .

I'm essentially following https://github.com/ericsink/SQLitePCL.raw/tree/master/src/SQLitePCLRaw.lib.e_sqlite3.ios , since sqlite should be the most used package with native asset in .NET world.

Please do a manual test because I cannot, neither CI does.

The change packs native libraries with assembly instead of nupkg. Test results on project reference should be safely applicable on package reference at game side.

  • Test framework tests on iOS
  • Test osu! on iOS via project reference (local framework)
  • Test osu! on iOS via nuget package

@swoolcock
Copy link
Collaborator

I know there was a reason I didn't do it this way, but I can't remember what it was. It might be worth moving the LinkerFlags in there too.

@huoyaoyuan
Copy link
Contributor Author

Another downside of using NativeReference is that Windows build will try to explain them as something else, and then provide warnings. The name conflicts with some COM stuff.

Comment on lines 32 to 34
<EmbeddedResource Include="$(MSBuildThisFileDirectory)*.a">
<WithCulture>false</WithCulture>
</EmbeddedResource>
Copy link
Member

@frenzibyte frenzibyte Nov 11, 2021

Choose a reason for hiding this comment

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

As-is, this caused the native libraries to be included as osu.Framework.iOS.libXYZ.a

CleanShot 2021-11-11 at 15 09 05@2x

While I was looking over why EmbeddedResource was used rather than ManifestResourceWithNoCulture (as in the SQlite project you linked) and that it's because the latter is deprecated, I've noticed that this is actually missing the LogicalName metadata (which is the cause of the issue above), and also the Type metadata to correctly replace the deprecated ManifestResourceWithNoCulture.

See xamarin/xamarin-macios#9525.


Resolved myself at f2706f7.

@pull-request-size pull-request-size bot added size/M and removed size/S labels Nov 11, 2021
@peppy peppy closed this Nov 11, 2021
@peppy peppy reopened this Nov 11, 2021
frenzibyte
frenzibyte previously approved these changes Nov 11, 2021
Copy link
Member

@frenzibyte frenzibyte left a comment

Choose a reason for hiding this comment

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

Size dropped from 150.3MB to 75.2MB, native libraries don't get duplicated anymore. 👍🏼

Thoroughly tested with clean builds to ensure no leftover libraries in mtouch cache interfere. Can confirm all libraries have been linked properly, along with the specified linker flags and frameworks.

@frenzibyte
Copy link
Member

frenzibyte commented Nov 12, 2021

Blocking momentarily while I figure out if building on Release configuration has actually been busted with this PR, or has always been the case. (it takes long to build on Release that it's hard to even ascertain)

@frenzibyte
Copy link
Member

Unblocking as the issue appeared to be unrelated from this PR, will resolve in a separate one.

@frenzibyte
Copy link
Member

Actually templates can't be updated without a framework release with this PR included first.

@peppy peppy enabled auto-merge November 14, 2021 10:41
@peppy peppy merged commit 3e47f61 into ppy:master Nov 14, 2021
@huoyaoyuan huoyaoyuan deleted the ios-native branch November 14, 2021 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

iOS framework package duplicates native libraries
5 participants