kiwix / kiwix-build

Kiwix & openZIM build engine
GNU General Public License v3.0
87 stars 42 forks source link

Build CoreKiwix.xcframework #653

Closed rgaudin closed 10 months ago

rgaudin commented 10 months ago

Fixes https://github.com/kiwix/kiwix-build/issues/455 Fixes https://github.com/kiwix/kiwix-build/issues/590

CoreKiwix.xcframework is the packaging format required by Kiwix apple (macOS/iOS) reader for libkiwix. Naming will hopefully be revisited later (libkiwix.xcframework?)

This folder is a combination of all static archive (*.a): dependencies and libkiwix itself for all apple-supported platforms: macOS x86_64, macOS arm64, iOS arm64 and iOS x86_64. It also includes the headers. Note: while iOS archs are separated in the framework, the macOS archs are bundled as a fat binary.

This thus adds:

Ref:

https://developer.apple.com/documentation/xcode/creating-a-multi-platform-binary-framework-bundle

⚠️ DO NOT MERGE YET I want to investigate #650 first

mgautierfr commented 10 months ago

How xcframework (and new dependency AppleXCFramework) compare the iOS_multi (and the ios_fat_lib dependency) ?

Is iOS_multi still used ? (@automactic) xcframework use fatlib for macos but not iOS. Why we use fatlib for iOS elsewhere ?

rgaudin commented 10 months ago

I've thoroughly described what the xcframework includes. iOS_multi is not used anymore by kiwix. I suggest, given our low throughput on apple platform to only keep what we use until there's a request for something else. But that's out of the scope of this PR IMO

kelson42 commented 10 months ago

@rgaudin Great but what was ios_multi?

rgaudin commented 10 months ago

a fat binary of both arch for iOS

rgaudin commented 10 months ago

dylib I believe

rgaudin commented 10 months ago

@kelson42 you edited the PR comment to indicate it fixes #590, that's not the case… but the ticket is not precise, so to be clear:

So I'm not sure what to do with that ticket 🤷‍♂️

kelson42 commented 10 months ago

@rgaudin Your PR "does the job" and ticket can be closed. I will put a link to your last comment to be even more clear.

rgaudin commented 10 months ago

OK so as I suspected, there's a CI bug that makes a build using base_deps2 from succeeding:

[BUILD]
build pugixml (macOS_x86_64):
WARNING: qmake command not found.
  configure pugixml : ERROR
Stopping build due to errors
run command 'meson . /Users/runner/./BUILD_macOS_x86_64/pugixml-1.2 --buildtype=release  --default-library=static  --prefix=/Users/runner/./BUILD_macOS_x86_64/INSTALL --libdir=lib --cross-file /Users/runner/./BUILD_macOS_x86_64/meson_cross_file.txt'
current directory is '/Users/runner/SOURCE/pugixml-1.2'
ERROR: Neither source directory '.' nor build directory '/Users/runner/./BUILD_macOS_x86_64/pugixml-1.2' contain a build file meson.build.
WARNING: Running the setup command as `meson [options]` instead of `meson setup [options]` is ambiguous and deprecated.

If I remove that file, then the CI properly builds its deps, (uploads them) and compiles fine.

It means that for some reason, the uploaded+downloaded deps differ from just having them built in the same job. @mgautierfr does that ring a bell?

mgautierfr commented 10 months ago

@mgautierfr does that ring a bell?

Yes.

You probably need a files_to_archive += HOME.glob("BUILD_macos*/**/.*_ok") just after files_to_archive += HOME.glob("BUILD_android*/**/.*_ok")

The .*_ok files are use to trace if the command has been run. If not present kiwix-build tries to do the command, else it skip it (search for try_skip). So we must keep theses files else we want to run the configure/compile even if we have saved the INSTALL directory (and even if we don't have saved the sources)

rgaudin commented 10 months ago

Ah ok I wondered what those were

rgaudin commented 10 months ago

@mgautierfr I eventually fixed the base_deps but what I had to do might not be the best practice. Please take a look at the last commits.

mgautierfr commented 10 months ago

Hum, it seems that it is because we skip the configure. (This should be broken since a long time, but as we don't launch android (only) build, we never found it).

I think it would be better to remove this line to configure all our projects (even if we have a .*_ok file). (As we do for build and install).

It means that kiwix-build run localy will reconfigure the project everytime it is run but I can live with it (or patch it to a real solution)

rgaudin commented 10 months ago

@mgautierfr you mean to remove that line instead of 15fed1c right? Or would it be in addition to it?

mgautierfr commented 10 months ago

Instead of (if I am right about the issue here. Don't lose the commit)

rgaudin commented 10 months ago

@mgautierfr OK we're now green (appveyor finishin). Should we keep the last commit (libzim version bump) or not ? Let me know if I can rewrite history

mgautierfr commented 10 months ago

Yes you can rewrite the history.

Please check with a change of base_deps_meta_version:

It is a pity as this double run take 2h long but it the price to check everything is ok.

Should we keep the last commit (libzim version bump) or not ?

We may keep it in the PR to check CI is passing but better to remove it to not mix things.

rgaudin commented 10 months ago

Please check with a change of base_deps_meta_version:

Indeed ; I forgot this affected all builds. It's now OK.

Let me know if that's OK to rewrite/rebase

mgautierfr commented 10 months ago

Let me know if that's OK to rewrite/rebase

Yes. That's OK.

kelson42 commented 10 months ago

FYI @automactic has made the xcf with a .zip file, and we do .tar.gz, not sure what is the best, I just noticed.

rgaudin commented 10 months ago

We discussed it during the meeting (!). It's just a transport format and supported on macOS so no impact but it prevents additional code for a special case on kiwix-build