openzim / libzim

Reference implementation of the ZIM specification
https://download.openzim.org/release/libzim/
GNU General Public License v2.0
164 stars 50 forks source link

Dont use bitcode as it's now deprecated #839

Closed rgaudin closed 10 months ago

rgaudin commented 10 months ago

See https://stackoverflow.com/questions/72543728/xcode-14-deprecates-bitcode-but-why

Fixes https://github.com/openzim/libzim/issues/834

codecov[bot] commented 10 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (c23d43e) 57.57% compared to head (cdc840b) 57.57%. Report is 1 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #839 +/- ## ======================================= Coverage 57.57% 57.57% ======================================= Files 99 99 Lines 4542 4542 Branches 1909 1909 ======================================= Hits 2615 2615 Misses 668 668 Partials 1259 1259 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

kelson42 commented 10 months ago

@rgaudin But CI fails :(

rgaudin commented 10 months ago

OK the issue is inside the deps file that is downloaded from tmp.kiwix.org: meson_cross_file.txt includes -fembed-bitcode. I'll come back to this PR once this gets updated.

btw, those are not versioned 🤷‍♂️

kelson42 commented 10 months ago

@mgautierfr We need to get something clean around this! See as well https://github.com/openzim/libzim/issues/834

mgautierfr commented 10 months ago

OK the issue is inside the deps file that is downloaded from tmp.kiwix.org: meson_cross_file.txt includes -fembed-bitcode.

The deps file is generated everynigth from kiwix-build main. As long as https://github.com/kiwix/kiwix-build/pull/650 is not merged, meson_cross_file.txt will includes -fembed-bicode.

I have added a commit (to remove before merge) which use the dev_preview.

kelson42 commented 10 months ago

@rgaudin @mgautierfr CI is green, so does that mean this PR is ready to merge, just blocker by merging https://github.com/kiwix/kiwix-build/pull/650?

mgautierfr commented 10 months ago

On my side, mostly yes:

rgaudin commented 10 months ago

@mgautierfr do we want to introduce -mmacosx-version-min=12.0 here or do we consider that's a CD concern and that issues with this would circle-back?

I updated meson to lastest version instead as that's what we do every where.

I see you also force-pushed https://github.com/openzim/libzim/compare/44e4de62b2035f087b16443414b6f95fb0ebb72b..5dc4577ebb529228c89b013797d4bcaf34cc03ac and I don't know what to do with it.

Update the matrix as you see fit.

This could be merged D+1 after https://github.com/kiwix/kiwix-build/pull/650 is merged I think

mgautierfr commented 10 months ago

I see you also force-pushed 44e4de62b2035f087b16443414b6f95fb0ebb72b..5dc4577ebb529228c89b013797d4bcaf34cc03ac (compare) and I don't know what to do with it.

This is about this discussion which is now resolved so you probably haven't seen it.

And I force push because their was another "[TOREMOVE]" commit to use the dev_preview deps archive and I wanted to keep at as the last commit of the PR.