search

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

[REGRESSION] Fulltext search crashes on javascript libzim since version 8.2.1 #828

Closed kelson42 closed 10 months ago

kelson42 commented 11 months ago

See https://github.com/openzim/javascript-libzim/issues/58

Jaifroid commented 10 months ago

@mgautierfr How do you suggest we proceed? Given that v8.2.1 can be compiled from source correctly (see sample run here and outcome of test on this here), can we adjust the build pipeline for libzim.a here to match? If we can identify exactly which source packages are different, we can eliminate them one-by-one until we identify the culprit.

More info on testing: tests of basic functions (loading ZIM, checking article count, loading an article by path, searching with Xapian) can be run against compiled versions of the WASM either from libzim.a or from source code. See instructions here: https://github.com/openzim/javascript-libzim#tests. Any libzim.a can be easily compiled by placing it in /build/lib/, making sure nothing else is there, and any previously built WASMs are removed (e.g. by running make clean first), then following the (updated) instructions to build with docker here: https://github.com/openzim/javascript-libzim#steps-to-recompile-from-source-with-docker (despite the title of those instructions, if libzim.a is already in /build/lib, it shouldn't attempt to recompile it).

I'm happy to do any testing.

mgautierfr commented 10 months ago

Can you test with https://tmp.kiwix.org/ci/dev_preview/update_icu/libzim_wasm-emscripten-2023-10-25.tar.gz ?

This is a test build with ICU 71.1. Not yet ready for publishing but I least we could know if it is the root cause.

Jaifroid commented 10 months ago

Thanks, will do.

Jaifroid commented 10 months ago

@mgautierfr Yay!

image

I'll make a test PR to see if I can replicate this on GitHub, but I don't see why it shouldn't work. Just good to double-check.

Thank you!

Jaifroid commented 10 months ago

Confirmed on GitHub with this run:

https://github.com/openzim/javascript-libzim/actions/runs/6645686934/job/18057535301

image

Jaifroid commented 10 months ago

Also passing when built as a release artefact (rather than as a nightly). Not actually released of course. See https://github.com/openzim/javascript-libzim/pull/63#issuecomment-1780018137.

mgautierfr commented 10 months ago

Good to know ! It confirms it is a ICU version issue. We have to update it to a recent version but it may take a bit of time as it has impact in the way we publish kiwix-android.

Jaifroid commented 10 months ago

@mgautierfr No problem. Possibly it could be used in the nightly release first, or would that break the Android nightly build chain? It would allow the javascript-libzim nightlies to start working again.

kelson42 commented 10 months ago

Good to know ! It confirms it is a ICU version issue. We have to update it to a recent version but it may take a bit of time as it has impact in the way we publish kiwix-android.

@mgautierfr Please create a dedicated ticket so we understand all the challenges. We will anyway put this high on the prio list.

kelson42 commented 10 months ago

@mgautierfr What is the status? Is this ticket fixed by https://github.com/kiwix/kiwix-build/pull/642 ? @Jaifroid All good with the last nightly builds?

Jaifroid commented 10 months ago

@kelson42 Yes last nightly build succeeded. As far as I'm concerned, this ticket can be closed.