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 Apple Platforms #638

Closed
wants to merge 4 commits into from
Closed

Update Apple Platforms #638

wants to merge 4 commits into from

Conversation

automactic
Copy link
Member

@automactic automactic commented Oct 19, 2023

  • Removed MacABI (aka UIKit on Mac, it is not needed)
  • 5 targets -- iOS_arm64, iOS_simulator_x86, iOS_simulator_arm64, macOS_arm64, macOS_x86
  • added mmacosx-version-min and mmacosx-version-min in various places
  • update min os version to macOS 12.0 & iOS 15.0

@kelson42 kelson42 requested a review from mgautierfr October 19, 2023 05:15
@automactic
Copy link
Member Author

As far as I can tell, the kiwix-build commands did succeed. But there are a bunch of other stuff going on after the build command, which I am not very familiar with... And something post build is failing

@kelson42
Copy link
Contributor

@mgautierfr Can you please have a look?

@mgautierfr
Copy link
Member

Haven't had a close look, but it missing at least a change in the build definition

(That's why https://github.com/kiwix/kiwix-build/actions/runs/6575114946/job/17861547005?pr=638)

@mgautierfr
Copy link
Member

This PR also show a usage of c++ feature deprecated in c++17 in zim-tools.
This is fixed in openzim/zim-tools#376

@kelson42
Copy link
Contributor

kelson42 commented Oct 20, 2023

I'm not in favour to abandon support of macOS11 for libzim without clear reason. Libzim bindings should better stay compatible with older versions of macOS as long as it does not create serious problems.

Having a ticket explaining why we do things would be helpful. PR without corresponding issues should be avoided or kept for very trivial changes.

@@ -15,49 +15,48 @@
# 'D' letter means we trigger the docker forkflow to build the docker image.
# If a cell contains several letters, all are done.
BUILD_DEF = """
Copy link
Member Author

Choose a reason for hiding this comment

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

⬇️ use hide whitespace on GitHub to see what's changed

| macos | iOS_simulator_x86 | B | B | | | | |
| macos | iOS_simulator_arm64 | B | B | | | | |
| macos | macOS_arm64 | B | B | | | | macos-arm64 |
| macos | macOS_x86 | B | B | | | | |
Copy link
Member Author

Choose a reason for hiding this comment

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

TBH I don't think I fully understand what the differences are between B, dB and BP (yes I did read the keys)

Obviously I am coming from a kiwix for macOS and iOS perspective, so correct me if something should be changed

@automactic
Copy link
Member Author

I'm not in favour to abandon support of macOS11 for libzim without clear reason. Libzim bindings should better stay compatible with older versions of macOS as long as it does not create serious problems.

@kelson42 Hey, are there anyone else that needs libzim on macOS 11? If I (i.e. kiwix for macOS and iOS app) is the only one, then that might be a good reason to no longer support macOS 11, since the kiwix app does not support macOS 11.

@mgautierfr
Copy link
Member

@kelson42
Copy link
Contributor

kelson42 commented Oct 31, 2023

@automactic @mgautierfr @rgaudin We are in an almost emergency situation to move forward on Apple custom apps. This is TOP PRIO that we CI+CD for libzim and libkiwix for both macOS and iOS work ASAP. Therefore this PR should be on the very TOP of the agenda.

@kelson42
Copy link
Contributor

kelson42 commented Nov 3, 2023

After discussion with @rgaudin regarding the min macOS version, we agreed:

  • We will use the rule applied for Kiwix for macOS/iOS which is -2 major macOS version supported
  • Which has directly consequence, that from now, min macOS supported (and on which we compile) is macOS 12

I will update the CI/CD.

@kelson42
Copy link
Contributor

kelson42 commented Nov 8, 2023

Probably superseeded by #650.

@kelson42
Copy link
Contributor

Superseeded by #653

@kelson42 kelson42 closed this Nov 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants