Skip to content

Bump base (sdcv 0.5.2; fix *buntu 17.10 build & segfault)#3378

Merged
Frenzie merged 1 commit intokoreader:masterfrom
Frenzie:bump-base
Oct 20, 2017
Merged

Bump base (sdcv 0.5.2; fix *buntu 17.10 build & segfault)#3378
Frenzie merged 1 commit intokoreader:masterfrom
Frenzie:bump-base

Conversation

@Frenzie
Copy link
Copy Markdown
Member

@Frenzie Frenzie commented Oct 20, 2017

Fixes #3302. Fixes #3071.

@Frenzie Frenzie merged commit 122c920 into koreader:master Oct 20, 2017
@Frenzie Frenzie deleted the bump-base branch October 20, 2017 10:42
@poire-z
Copy link
Copy Markdown
Contributor

poire-z commented Oct 21, 2017

Can you check on your device how sdcv works with today's nightly?
For me, on kobo glo hd, it is slower: 20 to 30 s when it usually took 5 to 10 seconds with 10 dicts.
And disabling some dicts (-u option used), with some non-ascii dict name still enabled, fails (like seen when locale is not utf8).

@Frenzie
Copy link
Copy Markdown
Member Author

Frenzie commented Oct 21, 2017

I'll check the nightly but I noticed no slowdowns testing my own builds.

@Frenzie
Copy link
Copy Markdown
Member Author

Frenzie commented Oct 21, 2017

Hard to judge; would need objective measurements to tell.

@poire-z
Copy link
Copy Markdown
Contributor

poire-z commented Oct 21, 2017

Well, it seems now quite ok :|. I'll check after a reboot.
How does it do when you add Académie Française dict, and disable an other one?
locale is already LANG=en_US.UTF-8, and adding it or a variation of it to sdcv command line (a trick that worked on my latin1 linux) does not make things work.
edit: this trick koreader/koreader-base#550 (comment) does not work, and outputs (with or without) :
Invalid command line arguments: Invalid byte sequence in conversion input

Additionally, but I must be the only one using that feature, whe, you have dicts in dict_ext/, and you give -u for dicts that do not exist in the dict/ or dict_ext/ that is used for the sdcv invocation, it fails (older version didn't mind having -u "non existant dict"...

@Frenzie
Copy link
Copy Markdown
Member Author

Frenzie commented Oct 21, 2017

Additionally, but I must be the only one using that feature

Doesn't matter if you add unit tests. :-P But I thought you already said you were going to submit a patch for that upstream? Upstream sdcv also has unit tests and CI since the move to GitHub, so it would make the most sense to add them there.

Rerunning the test from #2934 (comment) I do indeed see substantially worse values. My hypothesis based on the few quick tests below is that the worse results seem to be related to synonyms and fuzzy search. I ran these tests on my H2O.

Regular (with synonyms)

0.4 KOReader sdcv fork

# for w in `echo "épuisant épuisante épuisantes pleurésie caparaçonnée pre-consonantal peche
resse pécheresse"`; do echo $w ; time ./sdcv-0.4-koreader-fork --utf8-input --data-dir data/dict -nj $w 1>/dev/nul
l; done
épuisant
real	0m 0.10s
user	0m 0.02s
sys	0m 0.06s
épuisante
real	0m 0.78s
user	0m 0.71s
sys	0m 0.07s
épuisantes
real	0m 0.77s
user	0m 0.72s
sys	0m 0.05s
pleurésie
real	0m 0.04s
user	0m 0.01s
sys	0m 0.03s
caparaçonnée
real	0m 0.69s
user	0m 0.62s
sys	0m 0.06s
pre-consonantal
real	0m 0.49s
user	0m 0.43s
sys	0m 0.06s
pecheresse
real	0m 0.79s
user	0m 0.70s
sys	0m 0.08s
pécheresse
real	0m 0.04s
user	0m 0.01s
sys	0m 0.03s

0.5.2 nightly

# for w in `echo "épuisant épuisante épuisantes pleurésie caparaçonnée pre-consonantal peche
resse pécheresse"`; do echo $w ; time ./sdcv --utf8-input --data-dir data/dict -nj $w 1>/dev/null; done
épuisant
real	0m 1.12s
user	0m 1.06s
sys	0m 0.05s
épuisante
real	0m 1.06s
user	0m 1.00s
sys	0m 0.06s
épuisantes
real	0m 2.59s
user	0m 2.47s
sys	0m 0.11s
pleurésie
real	0m 1.07s
user	0m 0.98s
sys	0m 0.08s
caparaçonnée
real	0m 2.42s
user	0m 2.31s
sys	0m 0.10s
pre-consonantal
real	0m 2.04s
user	0m 1.94s
sys	0m 0.09s
pecheresse
real	0m 2.63s
user	0m 2.54s
sys	0m 0.07s
pécheresse
real	0m 1.27s
user	0m 1.21s
sys	0m 0.05s

Oddly enough it's consistently ever so slightly faster with the one I built myself, but still with the significant slowdown compared to older sdcv.

0.5.2 compiled on my PC

# for w in `echo "épuisant épuisante épuisantes pleurésie caparaçonnée pre-consonantal peche
resse pécheresse"`; do echo $w ; time ./sdcv --utf8-input --data-dir data/dict -nj $w 1>/dev/null; done
épuisant
real	0m 1.10s
user	0m 0.99s
sys	0m 0.10s
épuisante
real	0m 1.06s
user	0m 1.02s
sys	0m 0.03s
épuisantes
real	0m 2.46s
user	0m 2.31s
sys	0m 0.14s
pleurésie
real	0m 1.06s
user	0m 1.00s
sys	0m 0.05s
caparaçonnée
real	0m 2.29s
user	0m 2.20s
sys	0m 0.08s
pre-consonantal
real	0m 1.91s
user	0m 1.79s
sys	0m 0.12s
pecheresse
real	0m 2.49s
user	0m 2.37s
sys	0m 0.11s
pécheresse
real	0m 1.06s
user	0m 1.02s
sys	0m 0.03s

I haven't tested this and I'm probably slightly less motivated than your exceptional situation, but note that support for synonyms was added quite recently upstream in Dushistov/sdcv#20

One of my dictionaries has a 2 MB syn file. Moving it out of the way, but keeping one with a 100 KiB syn file results in this:

Without 2 MB syn file

It looks like this helps a great deal.

KOReader 0.4.x fork

# for w in `echo "épuisant épuisante épuisantes pleurésie caparaçonnée pre-consonantal peche
resse pécheresse"`; do echo $w ; time ./sdcv-0.4-koreader-fork --utf8-input --data-dir data/dict -nj $w 1>/dev/nul
l; done
épuisant
real	0m 0.09s
user	0m 0.03s
sys	0m 0.04s
épuisante
real	0m 0.67s
user	0m 0.62s
sys	0m 0.05s
épuisantes
real	0m 0.67s
user	0m 0.63s
sys	0m 0.03s
pleurésie
real	0m 0.04s
user	0m 0.00s
sys	0m 0.03s
caparaçonnée
real	0m 0.59s
user	0m 0.55s
sys	0m 0.04s
pre-consonantal
real	0m 0.41s
user	0m 0.38s
sys	0m 0.03s
pecheresse
real	0m 0.67s
user	0m 0.64s
sys	0m 0.03s
pécheresse
real	0m 0.04s
user	0m 0.01s
sys	0m 0.02s

0.5.2

# for w in `echo "épuisant épuisante épuisantes pleurésie caparaçonnée pre-consonantal peche
resse pécheresse"`; do echo $w ; time ./sdcv --utf8-input --data-dir data/dict -nj $w 1>/dev/null; done
épuisant
real	0m 0.23s
user	0m 0.16s
sys	0m 0.07s
épuisante
real	0m 0.18s
user	0m 0.13s
sys	0m 0.05s
épuisantes
real	0m 1.39s
user	0m 1.32s
sys	0m 0.06s
pleurésie
real	0m 0.18s
user	0m 0.15s
sys	0m 0.03s
caparaçonnée
real	0m 1.24s
user	0m 1.14s
sys	0m 0.09s
pre-consonantal
real	0m 0.91s
user	0m 0.82s
sys	0m 0.09s
pecheresse
real	0m 1.41s
user	0m 1.30s
sys	0m 0.10s
pécheresse
real	0m 0.18s
user	0m 0.14s
sys	0m 0.04s

No syn file

Finally, getting rid of the last syn-file containing dictionary. Note how the 0.5.2 times are actually significantly worse when it can't find anything.

KOReader 0.4.x sdcv fork

# for w in `echo "épuisant épuisante épuisantes pleurésie caparaçonnée pre-consonantal peche
resse pécheresse"`; do echo $w ; time ./sdcv-0.4-koreader-fork --utf8-input --data-dir data/dict -nj $w 1>/dev/nul
l; done
épuisant
real	0m 0.59s
user	0m 0.52s
sys	0m 0.05s
épuisante
Nothing similar to \u00e9puisante, sorry :(
real	0m 0.55s
user	0m 0.52s
sys	0m 0.03s
épuisantes
Nothing similar to \u00e9puisantes, sorry :(
real	0m 0.56s
user	0m 0.51s
sys	0m 0.04s
pleurésie
Nothing similar to pleur\u00e9sie, sorry :(
real	0m 0.55s
user	0m 0.50s
sys	0m 0.05s
caparaçonnée
Nothing similar to capara\u00e7onn\u00e9e, sorry :(
real	0m 0.51s
user	0m 0.42s
sys	0m 0.08s
pre-consonantal
real	0m 0.37s
user	0m 0.34s
sys	0m 0.03s
pecheresse
Nothing similar to pecheresse, sorry :(
real	0m 0.56s
user	0m 0.53s
sys	0m 0.02s
pécheresse
Nothing similar to p\u00e9cheresse, sorry :(
real	0m 0.55s
user	0m 0.51s
sys	0m 0.04s

0.5.2

(the Nothing similar to message apparently switched from stderr to stdout? it's still there if you just search for a word)

# for w in `echo "épuisant épuisante épuisantes pleurésie caparaçonnée pre-consonantal peche
resse pécheresse"`; do echo $w ; time ./sdcv --utf8-input --data-dir data/dict -nj $w 1>/dev/null; done
épuisant
real	0m 1.07s
user	0m 0.98s
sys	0m 0.09s
épuisante
real	0m 1.06s
user	0m 1.03s
sys	0m 0.02s
épuisantes
real	0m 1.06s
user	0m 1.00s
sys	0m 0.05s
pleurésie
real	0m 1.07s
user	0m 1.03s
sys	0m 0.03s
caparaçonnée
real	0m 0.95s
user	0m 0.85s
sys	0m 0.10s
pre-consonantal
real	0m 0.71s
user	0m 0.63s
sys	0m 0.06s
pecheresse
real	0m 1.07s
user	0m 1.00s
sys	0m 0.07s
pécheresse
real	0m 1.05s
user	0m 0.96s
sys	0m 0.08s

I (or someone else) will need to repeat these tests on a desktop PC to see if it's "only" some compilation flag or if it's a serious upstream performance degradation.

@poire-z
Copy link
Copy Markdown
Contributor

poire-z commented Oct 21, 2017

But I thought you already said you were going to submit a patch for that upstream?

Never said and never even thought about that :) It didn't need any patch, -u "non existant dict" caused no problem with previous version.

Anyway, 0.5.2 would accept french and chinese dicts names in -u with a simple patch:

--- a/src/sdcv.cpp
+++ b/src/sdcv.cpp
@@ -62,7 +62,7 @@ using StrArr = ResourceWrapper<gchar *, gchar *, free_str_array>;
 static void list_dicts(const std::list<std::string> &dicts_dir_list, bool use_json);

 int main(int argc, char *argv[]) try {
-    setlocale(LC_ALL, "");
+    setlocale(LC_ALL, "en_US.UTF-8");
 #if ENABLE_NLS
     bindtextdomain("sdcv",
                    //"./locale"//< for testing

I don't get why it would need to erase LC_ALL, when later it uses glib stuff to parse command line, and this glib stuff must surely look at locale to convert command line arguments, and output Invalid byte sequence in conversion input when it finds UTF8, cause I guess with LC_ALL empty, it thinks it is in the C ASCII locale...

@Frenzie
Copy link
Copy Markdown
Member Author

Frenzie commented Oct 21, 2017

ahem /whistles
koreader/koreader-base#550 (comment)

I'll check tomorrow if I have time if a small patch can solve that.

@poire-z
Copy link
Copy Markdown
Contributor

poire-z commented Oct 21, 2017

oh, right :) forgot about that as setting a correct locale solved the problem ... some days ago, with the sdcv build you provided, and seem to still do on the emulator.
Could be because I set it in LANG=, and there's a whole locale stuff on my linux, so may be glib found its way with that, while there's no locale stuff on Kobo.
The problem with french or chinese dict name also happen on android...
(Anyway, I would patch here before thinking about upstream.)

@Frenzie
Copy link
Copy Markdown
Member Author

Frenzie commented Oct 21, 2017

We can apply patches before they're accepted upstream in some cases, but I strongly dislike the wasted effort in backporting patches let alone features. ;-)

@Frenzie
Copy link
Copy Markdown
Member Author

Frenzie commented Oct 21, 2017

I've attached x86_64 desktop builds of 0.4.2 (KOReader fork) and 0.5.2 (upstream).

sdcv-x86_64-linux-gnu.zip

@Frenzie
Copy link
Copy Markdown
Member Author

Frenzie commented Oct 21, 2017

Ah yes, this is an issue to be reported upstream. I'll quickly try some earlier upstream commits from around July before I do.

KOReader 0.4.2

$ for w in `echo "épuisant épuisante épuisantes pleurésie caparaçonnée pre-consonantal pecheresse pécheresse"`; do echo $w ; time ./sdcv-0.4.2 --utf8-input --data-dir data/dict -nj $w 1>/dev/null; done
épuisant

real	0m0.046s
user	0m0.040s
sys	0m0.000s
épuisante

real	0m0.041s
user	0m0.036s
sys	0m0.004s
épuisantes
Nothing similar to épuisantes, sorry :(

real	0m0.040s
user	0m0.036s
sys	0m0.000s
pleurésie

real	0m0.003s
user	0m0.000s
sys	0m0.000s
caparaçonnée

real	0m0.036s
user	0m0.028s
sys	0m0.004s
pre-consonantal
Nothing similar to pre-consonantal, sorry :(

real	0m0.029s
user	0m0.024s
sys	0m0.004s
pecheresse

real	0m0.041s
user	0m0.036s
sys	0m0.000s
pécheresse

real	0m0.001s
user	0m0.000s
sys	0m0.000s

KOReader 0.5.2

$ for w in `echo "épuisant épuisante épuisantes pleurésie caparaçonnée pre-consonantal pecheresse pécheresse"`; do echo $w ; time ./sdcv-0.5.2 --utf8-input --data-dir data/dict -nj $w 1>/dev/null; done
épuisant

real	0m0.387s
user	0m0.384s
sys	0m0.000s
épuisante

real	0m0.459s
user	0m0.448s
sys	0m0.008s
épuisantes

real	0m0.462s
user	0m0.448s
sys	0m0.012s
pleurésie

real	0m0.387s
user	0m0.384s
sys	0m0.000s
caparaçonnée

real	0m0.452s
user	0m0.440s
sys	0m0.012s
pre-consonantal

real	0m0.442s
user	0m0.436s
sys	0m0.004s
pecheresse

real	0m0.458s
user	0m0.452s
sys	0m0.004s
pécheresse

real	0m0.387s
user	0m0.384s
sys	0m0.000s

Individual sdcv 0.5.2 (just mkdir build && cd build && cmake .. && make)

$ for w in `echo "épuisant épuisante épuisantes pleurésie caparaçonnée pre-consonantal pecheresse pécheresse"`; do echo $w ; time ./sdcv --utf8-input --data-dir data/dict -nj $w 1>/dev/null; done
épuisant

real	0m0.397s
user	0m0.400s
sys	0m0.000s
épuisante

real	0m0.468s
user	0m0.464s
sys	0m0.000s
épuisantes

real	0m0.465s
user	0m0.452s
sys	0m0.012s
pleurésie

real	0m0.399s
user	0m0.392s
sys	0m0.004s
caparaçonnée

real	0m0.460s
user	0m0.456s
sys	0m0.000s
pre-consonantal

real	0m0.445s
user	0m0.420s
sys	0m0.024s
pecheresse

real	0m0.465s
user	0m0.460s
sys	0m0.004s
pécheresse

real	0m0.395s
user	0m0.384s
sys	0m0.008s

@Frenzie
Copy link
Copy Markdown
Member Author

Frenzie commented Oct 21, 2017

My hypothesis about synonyms proved incorrect. Bisection shows Dushistov/sdcv@3105823 to be responsible.

@poire-z
Copy link
Copy Markdown
Contributor

poire-z commented Oct 21, 2017

And this one solves the crash with non existent (because of encoding or because of alternate dict_ext) dict name:

--- a/src/sdcv.cpp
+++ b/src/sdcv.cpp
@@ -182,7 +182,9 @@ int main(int argc, char *argv[]) try {
         // add bookname to list
         gchar **p = get_impl(use_dict_list);
         while (*p) {
-            order_list.push_back(bookname_to_ifo.at(*p));
+            if (bookname_to_ifo.count(*p) > 0) {
+                order_list.push_back(bookname_to_ifo.at(*p));
+            }
             ++p;
         }
     } else {

@Frenzie
Copy link
Copy Markdown
Member Author

Frenzie commented Oct 21, 2017

I'll leave analyzing the difference between my non-performance degrading JSON patch from May 2016 and upstream commit Dushistov/sdcv@3105823 to @ecraven but sounds like it's time for one or two PRs on your side. ;-)

@Dushistov
Copy link
Copy Markdown

@poire-z

I don't get why it would need to erase LC_ALL

It doesn't erase, you can read in man setlocale:

If locale is an empty string, "", each part of the locale that should
be modified is set according to the environment variables.

You can find similar code in many GNU projects.

@poire-z
Copy link
Copy Markdown
Contributor

poire-z commented Oct 21, 2017

@Dushistov : any idea why then, launching:
LANG.en_US.UTF-8 LC_ALL=en_US.UTF-8 ./sdcv -u "Dictionnaire de l’Académie Française, 8ème édition (1935)." test (or a variation with en_US.utf8) would fail on our Kobo with:
Invalid command line arguments: Invalid byte sequence in conversion input
On our Kobo, we have:

LANG=en_US.UTF-8
LC_ALL=en_US.UTF-8

I don't know anything about how locale stuff work, and I don't know how glib goes from that.

But it does work on a debian linux if I set LANG=en_US.UTF-8 where I previsouly had en_US (latin1).
On my linux, I have a /usr/share/i18n/charmaps/UTF-8.gz, but I don't see any such file on my kobo. Dunno if that matters.

edit: would you accept the if (bookname_to_ifo.count(*p) > 0) {order_list.push_back(bookname_to_ifo.at(*p));} patch above, ignoring invalid -u, or do you somehow consider it a feature to fail on invalid bookname ?

@poire-z
Copy link
Copy Markdown
Contributor

poire-z commented Oct 22, 2017

Unfortunatly, the setlocale(LC_ALL, "en_US.UTF-8"); part of my patch does not help, neither on Kobo nor Android :( We still get: Invalid command line arguments: Invalid byte sequence in conversion input

@poire-z
Copy link
Copy Markdown
Contributor

poire-z commented Oct 22, 2017

OK, what I can conclude so far:
sdcv 0.5.2 uses glib's g_option stuff (https://developer.gnome.org/glib/stable/glib-Commandline-option-parser.html) to parse command line arguments.
This g_option_stuff insists on converting string arguments to utf8 according to current locale, and can't be made (except maybe with a trick, see below) to not do that and let the bytestring as is (https://developer.gnome.org/glib/stable/glib-running.html#setlocale).
If LC_ALL=en_US.UTF-8, it should however let the string as is, as it is utf8. But we saw this morning that didnt work.
The reason, I think, is that, for that, glib needs some locale infrastructure on the system, which our Kobo (and I guess Android) do not ship one. (Dunno if in the glib build, there are some option to make it embed that infrastructure...)
If I copy /usr/lib/locale (containing only a C.UTF-8/ subdir) from my linux to my kobo (./sdcv.orig being the one from yesterday, not today's patched one):

/mnt/onboard/.adds/koreader # LANG=C.UTF-8 LC_ALL=C.UTF-8 ./sdcv.orig  -nj -u "toto" test
Internal error: map::at (= good, no problem)
/mnt/onboard/.adds/koreader # LANG=C.UTF-8 LC_ALL=C.UTF-8 ./sdcv.orig  -nj -u "adémie" test
Invalid command line arguments: Invalid byte sequence in conversion input

/mnt/onboard/.adds/koreader # LOCPATH=/mnt/onboard/.adds/locale LANG=C.UTF-8 LC_ALL=C.UTF-8 ./sdcv.orig  -nj -u "toto" test
Internal error: map::at (= good, no problem)
/mnt/onboard/.adds/koreader # LOCPATH=/mnt/onboard/.adds/locale LANG=C.UTF-8 LC_ALL=C.UTF-8 ./sdcv.orig  -nj -u "adémie" test
Internal error: map::at (= good, no problem)

So, it seems to need some locale infrastructure, and/or LOCPATH= pointing to where there is one.

A trick (somehow tested on my linux, that seems like it could work on our device), is to patch sdcv.cpp so that it does not consider these options as STRING, but as FILENAME (where the above doc, and https://github.com/GNOME/glib/blob/9814898df6a2ea3006d5cbb457bd8419d8e815d4/glib/goption.c#L1305-L1347 says that on unix, the string will be used as is with no conversion, a simple strdup()). I'll update our patch so we can try that on next nightly.

Obviously, this trick may be OK for our devices, probably not for upstream sdcv, unless there's another option that trigger one behaviour or the other.

@poire-z
Copy link
Copy Markdown
Contributor

poire-z commented Oct 22, 2017

Btw, had/could you try your benchmark tests with just the -j (json output) removed?

@Frenzie
Copy link
Copy Markdown
Member Author

Frenzie commented Oct 22, 2017

Makes no difference but I suppose it's potentially misleading the way I shared the log.

@poire-z
Copy link
Copy Markdown
Contributor

poire-z commented Oct 23, 2017

2nd sdcv patch makes it work on Kobo.
But not on Android. On my android phone, I have LANG and LC_ALL empty, so I guess Glib thinks it is C and is still trying to convert to utf8 and outputs Invalid byte sequence...

@Frenzie
Copy link
Copy Markdown
Member Author

Frenzie commented Oct 23, 2017

Note that you can run the Android x86 binary on your regular PC.

@poire-z
Copy link
Copy Markdown
Contributor

poire-z commented Oct 23, 2017

Some tests on Android:

/data/data/org.koreader.launcher/files # ./sdcv -nj -u "toto" test
Invalid command line arguments: Conversion from character set âen_USâ to âUTF-8â is not supported
(that was because my ssh was setting LANG=en_US, so it was still trying to convert to utf8)

/data/data/org.koreader.launcher/files # LANG= LC_ALL= ./sdcv -nj -u "toto" test
g_mkdir failed: Read-only file system (= GOOD)
/data/data/org.koreader.launcher/files # LANG= LC_ALL= ./sdcv --data-dir /sdcard/koreader/data/dict  -nj -u "toto" test
g_mkdir failed: Read-only file system
/data/data/org.koreader.launcher/files # LANG= LC_ALL= ./sdcv --data-dir /sdcard/koreader/data/dict  -nj test -u "toto" --utf8-input --utf8-output
g_mkdir failed: Read-only file system
/data/data/org.koreader.launcher/files # LANG= LC_ALL= ./sdcv --data-dir /sdcard/koreader/data/dict  -nj test -u "adémie" --utf8-input --utf8-output
Invalid command line arguments: Invalid byte sequence in conversion input
/data/data/org.koreader.launcher/files # LANG= LC_ALL=en_US.UTF-8 ./sdcv --data-dir /sdcard/koreader/data/dict  -nj test -u "adémie" --utf8-input --utf8-output
g_mkdir failed: Read-only file system

There's no obvious way to set environment variables in the android version of io.popen (A.stdout), so I guess I'll have to put back in our sdcv patch what was in my 1st try:

-    setlocale(LC_ALL, "");
+    setlocale(LC_ALL, "en_US.UTF-8");

@Frenzie
Copy link
Copy Markdown
Member Author

Frenzie commented Oct 23, 2017

It's funny, isn't it. Android is not really Linux and Android is not really Java either. Both of which cause much frustration.

@poire-z
Copy link
Copy Markdown
Contributor

poire-z commented Oct 24, 2017

Still not working on Android :|

/data/data/org.koreader.launcher/files # ./sdcv --data-dir /sdcard/koreader/data/dict  -nj test -u "adémie" --utf8-input --utf8-output
Invalid command line arguments: Conversion from character set âen_USâ to âUTF-8â is not supported
  (ok, my ssh environment LANG was en_US)
/data/data/org.koreader.launcher/files # LANG= ./sdcv --data-dir /sdcard/koreader/data/dict  -nj test -u "adémie" --utf8-input --utf8-output
Invalid command line arguments: Invalid byte sequence in conversion input
/data/data/org.koreader.launcher/files # LANG=en_US.UTF-8  ./sdcv --data-dir /sdcard/koreader/data/dict  -nj test -u "adémie" --utf8-input --utf8-output
g_mkdir failed: Read-only file system

So, I guess we need to also setlocale(LANG, "en_US.UTF-8"); ...

@Frenzie
Copy link
Copy Markdown
Member Author

Frenzie commented Oct 24, 2017

Incidentally, the glib mailing list contained a brief and incomplete overview of some ways in which Android is actively hostile.

https://mail.gnome.org/archives/gtk-devel-list/2011-March/msg00096.html

General

In a POSIXer's view, Android system is CRIPPLED: thus the porting of existing
libraries onto it is TOUGH. Please have this in mind.
The basic obstacles in porting:
1.Android does not have or offers less/no access in its
trimmed bionic C lib :
1>No SYS V shm.h/sem.h
2>Not standard pwd.h and getpwuid_r(),etc. methods
3>No locale.h
4>Not standard IP/IPV6 headers/support such as arpa/*.h ones
5>Not standard pthread.
6>No iconv funcs.
7>No intl/gettext funcs
8>No i18n support and wchar_t.

@poire-z
Copy link
Copy Markdown
Contributor

poire-z commented Oct 24, 2017

Hostile, that's the word :|
And we can't even use setlocale(LANG, "en_US.UTF-8"), it only accepts the LC_* constants.
I'd be tempted to try:

    g_setenv("LANG", "en_US.UTF-8", true);
    setlocale(LC_ALL, "en_US.UTF-8");

but I'm not even sure setting env variable from inside the program affetcts the program itself, only subprocesses.
Any chance you can reproduce this problem on your android emulator and try that kind of things?

@Frenzie
Copy link
Copy Markdown
Member Author

Frenzie commented Oct 24, 2017

I can probably give it a try tonight. All I need is a dictionary containing something like Académie?

Btw, it's simple enough to set up. All you need is way too many GB. I have a full 256 GB SSD dedicated to my Linux, not even for storage (except for some stuff like Dropbox), and I have a mere 6 GB free after creating one or two extra emulators and moving some 20 GB of recently ripped music over to storage. Android stuff in particular eats into it like crazy.

screenshot_2017-10-24_13-38-30

@poire-z
Copy link
Copy Markdown
Contributor

poire-z commented Oct 24, 2017

Btw, about the slowness:
I did some quick timing with just one word on the emulator "epuisante":
80ms on our old sdcv fork fro june:
190ms on new scv0.5.2

When tweaking this in CMakeCache.txt:

//Choose the type of build, options are: None(CMAKE_CXX_FLAGS or
// CMAKE_C_FLAGS used) Debug Release RelWithDebInfo MinSizeRel.
CMAKE_BUILD_TYPE:STRING=

empty, as it is: 190ms
Debug: 190ms
Release: 90ms

So it may be that by default, a Debug build is made.
Dunno how to pass Release to the cmake build process, but I guess you may know :)
(Can you check how your benchmarks do with it?)

@Frenzie
Copy link
Copy Markdown
Member Author

Frenzie commented Oct 24, 2017

On the emulator I changed the default to a build with debugging symbols enabled in #3379. That doesn't affect anything else.

(But I'll double-check if it makes any difference to vanilla sdcv.)

@poire-z
Copy link
Copy Markdown
Contributor

poire-z commented Oct 24, 2017

You'd need 3 dicts: 2 normal with ascii names, and 1 with a non-ascii name (the Académie one, or any chinese one). And in Installed dictionaries:

  • enable 2, disable the one with non-ascii name: everything works, you get results from the 2 with ascii names
  • enable 2, disable one of the 2 with ascii name: you get no result at all, because the -u "Académie" makes sdcv failt at startup while parsing command lines arguments

(not sure I want to dig much into android sdk world :)

@Frenzie
Copy link
Copy Markdown
Member Author

Frenzie commented Oct 24, 2017

So it may be that by default, a Debug build is made.

On vanilla sdcv debug is indeed basically twice as slow. I would expect a no-debug build (./kodev run --no-debug; requires clearing out the sdcv executable and build first of course) to show the same kind of difference.

@poire-z
Copy link
Copy Markdown
Contributor

poire-z commented Oct 24, 2017

But I worry this requires explicitely to write "Release" somewhere in the CMake stuff so it finds its way into CMakeCache.txt, otherwise, some slow build is made - on the build server.
Or is that all cmake business and is already dealt with by the build scripts or your --no-debug ?!

@Frenzie
Copy link
Copy Markdown
Member Author

Frenzie commented Oct 24, 2017

"None" and "Release" are the same on vanilla.

For us they're not the same because we pass our own CXX_FLAGS, which are our own versions of "Release" and "Debug" just without CMake messing it all up by passing random flags. So we have "None (Release)" and "None (Debug)".

If you believe CMake-brainwashed people, the Right Way™ is to vaguely attempt to "influence" CMake into doing this or that. @#% that.

@poire-z
Copy link
Copy Markdown
Contributor

poire-z commented Oct 24, 2017

OK, I trust you on this :)

It seems putting or not g_setenv("LANG", "en_US.UTF-8", true) in sdcv.cpp does change the behaviour on my linux when messing with LANG and LC_ALL. It's not a no-op as I expected, so I may give it a try for next nightly... try4 ...

@poire-z
Copy link
Copy Markdown
Contributor

poire-z commented Oct 25, 2017

Confirming it's finally working on Android, and still is on Kobo.

@Frenzie
Copy link
Copy Markdown
Member Author

Frenzie commented Oct 25, 2017

Alright, cool.

I don't know what the right solution is on Kobo but perhaps @Dushistov would be open to the possibility of an #ifdef __ANDROID__ around

    g_setenv("LANG", "en_US.UTF-8", true);
    setlocale(LC_ALL, "en_US.UTF-8");

(In fact I wouldn't mind that in our patch, ugly Android hack polluting behavior on every other platform. =P)

@poire-z
Copy link
Copy Markdown
Contributor

poire-z commented Oct 25, 2017

It's also that we make it run on devices with minimal/crippled infrastructure. On Kobo (at least mine, with old firmware), we have LANG=en_US.UTF-8 and same for LC_ALL. But we're not sure that all and next devices will still have these environment variables. I guess one could have LANG=C.

There's the G_OPTION_ARG_STRING_ARRAY needed to be replaced by G_OPTION_ARG_FILENAME_ARRAY for when no locale infrastructure is present. But on any normal linux, it should stay G_OPTION_ARG_STRING_ARRAY (except may be data-dir which is more a FILENAME than an arbitraty string).

It's just that GLib expects to be on a full infrastructure, and is probably not the best lib for embedded stuff.

@Frenzie
Copy link
Copy Markdown
Member Author

Frenzie commented Oct 25, 2017

On the Lua side we default to LANG=C if nothing else is around in e.g. gettext. But I'm sure there were good reasons to pick glib.

In any case, there's a big difference between more or less sensibly limited (Kobo, Kindle, PocketBook too?) and lobotomized Android. :-)

mwoz123 added a commit to mwoz123/koreader that referenced this pull request Nov 4, 2017
* Timer Reader with normal time format (koreader#3244)

* [fix, UX] Remove rounded corners from fullscreen History (koreader#3256)

Prevent visibility of readers's footer edges.

* [UX] Smaller bottom menu icons (koreader#3257)

And unrelated small optimisation in imagewidget

* Adapt ND to changes in code where 0 means no limit (koreader#3258)

+ reorganize config, improved comments

* Bump base for smoother image scaling (koreader#3259)

* README: fix typo

* ImageWidget: use MuPDF for scaling images (koreader#3260)

* Cache scaled images with scale_for_dpi

* Don't update original self.scale_for_dpi

A same ImageWidget object can be free()'d and re-render()ed,
and we would then lose scale_for_dpi on next renderings

* [fix] Some Android issues (koreader#3262)

Bump base for versioned Android libs.

Fixes koreader#3214. Fixes koreader#3112. Fixes koreader#3091. Fixes koreader#3039. Fixes koreader#2976. Fixes koreader#2960. Fixes koreader#2947.

* [UX] Add home button (koreader#3263)

Tap to go HOME. Hold to set current folder as HOME. When no HOME is set yet tap also asks to set current folder as HOME.

See koreader#2957 (comment)

Closes koreader#3200.

* [fix, Android] restore patch.lua and fix odd anonymous function issue

See koreader#3214 (comment) and koreader#3118 (comment)

* [Android] Enable CoverBrowser (koreader#3264)

I was reminded by @KenMaltby that this is disabled on Android.

https://www.mobileread.com/forums/showthread.php?p=3586162#post3586162

* Add back button to statistics (koreader#3267)

Closes: koreader#3236 and koreader#3235
Details:

> Also, when looking in days, we see info how many hours we read. It would be great if we could tap a day, it opens info on book(s) we read that day with info pertaining to that day, and if we tap on book, we receive book info for that book.

>  Statistics plugin is really great, I started to look in my reading statistics now much more, but all the time I can go only in one way. And I need to exit at lowest level of info. It would be great if I can use statistics plugin like part of menu, to go up and down within statistics data and exit at my wish, not when I have to.

Also added:

 * new statistics book by week
 * new statistics book by month
 * last year by week now we can see days in selected week

* [UX] Smaller borders (koreader#3266)

For years they've been smaller on higher DPI devices and likely very few people realized it was technically a bug. These values round up on lower DPI and smaller screen devices.

References koreader#3265

* README: update Android NDK/SDK info

* [fix] Avoid crash when set home (koreader#3269)

* README: various style, grammar, clarifications & updates

* [fix] Add hold_callback to IconButton (koreader#3271)

Also fix path display update.

Fixes koreader#3270.

* CoverBrowser: use MuPDF for down-scaling images (koreader#3272)

* README: more grammar

* README: update info about screen size/DPI

* README: typo and grammar

* CoverBrowser: use a cache for crengine books extraction (koreader#3277)

We previously disabled cache, but that may cause excessive
memory usage with big books. We now use a temporary
cache directory, that we clean when no more needed.

* Rename bookmark (koreader#3275)

* Allow for colored rendering (koreader#3276)

* Allow for colored rendering

Available with all engines (CRE, PDF, Images).
Needs to manually add setting: "color_rendering" = true

* Disable color for djvudocument

* Use Screen:isColorEnabled()

* Bump base

* [lang] OPDSCatalog ConfirmBox ok_text/cancel_text (koreader#3281)

* Fix refresh in Time range (koreader#3278)

* kodev: run with catchsegv by default (koreader#3283)

See koreader#2878 (comment)

Also fix `./kodev run -h` as alias for `--help` as it's always overwritten by `-h` in the sense of the much more important `--screen-height`.

* Time reader with normal time format v2 (koreader#3279)

* Allow for toggling color rendering

New menu item in Screen submenu.
hasColorScreen enabled for SDL device.

* Enable color rendering on Android

* [Fix] Show 'Follow Link' even when no dict installed

* [Fix] Full refresh when showing ImageViewer

* [fix] KOSync plugin server location (koreader#3288)

This is only step one. It remains to be determined why the connection is downgraded to sslv3, which will result in a handshake failure.

* Inform once about color rendering on supported devices (koreader#3289)

* [fix] KOSync server TLS connection (koreader#3291)

Bumps base for updated luasec.

The forum reminded me Sync was broken https://www.mobileread.com/forums/showthread.php?p=3587993 (as I don't use it myself). This finishes 23e2183.

Thanks to @houqp for helping with debugging Cloudflare. See https://support.cloudflare.com/hc/en-us/articles/203135314-How-can-I-enable-SSLv3-

Fixes koreader#2738. Fixes koreader#2178. (Duplicate bug reports by @Hzj-jie. :-P) Fixes koreader#3159. Fixes koreader#3158. Fixes koreader#2877.

* README: Change gcc requirement to 4.8

Technically 4.7 is probably fine for all but debugging symbols, for which a simple version check and workaround could be added. However, since all of the CI tests run 4.8 there is no regression prevention and it would be more future maintenance trouble than it's worth.

* credocument reader optimisation (avoid multiple TOC builds) (koreader#3292)

Avoid unnecessary work in ReaderView:onSetViewMode() and
ReaderRolling:updatePos() that would result in TOC being reset
and rebuilt (which can take time on books with huge TOC) during
reader setup.

* CoverBrowser: avoid crash when indexing some cre documents (koreader#3293)

* [feat] Pocketbook840 enable frontlight (koreader#3294)

* Detect PocketBook840 by GetSoftwareVersion()

* implement Frontlight-support for 840 via inkview

* [fix] Android frontlight control

* Revert 2 commits that caused crengine scroll mode side effects (koreader#3295)

394be8a (koreader#2855) and 2b3b310 (koreader#3183) introduced side effects (scroll mode
crashing and TOC being reset and rebuild). This reverts parts of them
not yet reverted.

* README: small typo

* kodev: add run android convenience shortcut (koreader#3297)

* [fix] kodev: default NDKABI=14 if not set for NDK 15 standalone toolkit

* ISSUE_TEMPLATE: add request to share crash.log (koreader#3296)

* Migrate Goodreads to https (koreader#3298)

* Bump base (koreader#3301)

Includes:

* [fix] Android viewport (fixes koreader#3148)
* Makefile: add luajit-clean target and auto-call it (koreader/koreader-base#531)

* [fix] Avoid multiple refreshes when opening credocuments (koreader#3300)

Only noticeable on Kindle (which uses REAGL as partial refresh).

* [fix] Android screen blackout on first light change (koreader#3303)

* Fix footer, stats, TOC position with cre in scroll mode (koreader#3304)

This gives the page position to these modules even in scroll mode.
Also, in readerrolling: don't query battery/charging status
when crengine does not need it (used only when it shows its top
progress bar).

* [fix] Crash on highlight in some situations (koreader#3306)

* Fix crash with keyboard navigation of onHold buttontables (koreader#3307)

* Fix crash with keyboard navigation of onHold buttontables

Would crash when encountering a separator or when the number
of buttons in a row changes.

* Reset previous selected item on new buttontable

* refresh footer (koreader#3313)

* Bump base to prevent Travis LuaJIT rebuild (koreader#3314)

* Avoid some full refreshes on Kindle (koreader#3315)

"partial" refresh causes a full (without black flash) refresh on
Kindle (which uses REAGL mode for partial refresh). This causes a
full redraw of widgets, which is a bit distracting with some of them:
- dictquicklookup: when showing next definition
- infomessage: when displaying a new one (Wikipedia Save as epub)

Also fix bottom menu, that even when closed, would still register
bottom area as dirty: this would cause top menu navigation to
cause a full partial refresh, only noticable on Kindle.

* [ReadTimer] Time from now (koreader#3311)

* Add LuaData and Dictionary Lookup History (koreader#3161)

* Add dictionary history

Fixes koreader#2033, fixes koreader#2998.

* Add LuaData

* table handling in base settings

* Add LuaData spec

* Option to disable show bottom menu on top menu activation (koreader#3316)

* Bump base (koreader#3318)

Includes koreader/koreader-base#539.

Changes to koptcontext.lua. Look here if anything weird changes in PDFs.

* [fix, Android] Don't steal frontlight control on start (koreader#3319)

I believe this should be `if isKobo()`, or better yet that the entire
block should be moved to `KoboPowerD:init()` because afaik that is the
only platform where the system doesn't provide trustworthy frontlight
information. But to be absolutely sure that I don't break anything (and I
don't want to spend any time on this atm) I'm temporarily excluding only
Android where this behavior is known to be problematic.

See discussion in koreader#3118 (comment)

References koreader#3118 (using keyword "references" because phrases like "possibly fixes"
result in GH autoclose).

* Add CircleCI (koreader#3321)

* CircleCI fixes

* shellcheck 0.4.5 fix `LC_ALL: en_US.UTF8` (can be removed for shellcheck 0.4.6)

* hush updated luacheck on `reader.lua`; we're not assigning any variables but `= nil` is redundant

* Better vertical centering of text in its box

Decide baseline vertical position according to font metrics, instead
of the hardcoded 0.7 (in textwidget, which made the text a little
bit up in its box), and 0.75 (in toggleswitch, which made the text a
little bit down in its box). This usually gives a value around 0.72
or 0.73 with our ui fonts, which looks about right.
ReaderFooter: add bottom padding, now that our text goes a few pixels lower

* Normalize some widgets appearance (those using ButtonTable)

This makes button heights similar in all uses of ButtonTable.
It depended on how the ButtonTable was used in each widget
(previously, first and last row may have different sizes than
the others).

buttontable.lua: more even buttons height whether zero_sep or not
framecontainer.lua: added padding_top/bottom/left/right (similar to
what was done for iconbutton)

The following widgets have been adapted for this, with some
additional fixes:

buttondialog.lua
buttondialogtitle.lua: wider title with adequate padding
confirmbox.lua + multiconfirmbox.lua: dismissable via tap outside
inputdialog.lua + multiinputdialog.lua: more even vertical padding between elements
imageviewer.lua
textviewer.lua
datewidget.lua
timewidget.lua

Additionaly: frontlightwidget.lua: fixed width of progress bar that
was exceeding window width since the Size scaling adjustements

* Flash KeyValuePage item when callback (koreader#3322)

* Fix readerfooter_spec (koreader#3326)

My bad, leftover from koreader#3323 but not caught due to some Travis → CircleCI migration birth pains.

* CircleCI docs (koreader#3327)

* Environment variable key replaced by CircleCI GitHub push key.
  See <https://circleci.com/docs/1.0/adding-read-write-deployment-key/>.

* Temporarily added `|| true` to luacov so it won't cause a fail.

* Fix stats in cre scroll mode (koreader#3331)

* CircleCI cache fix (koreader#3329)

* Update luarocks/shellcheck/shfmt check based on `deps-{{ arch }}-{{ checksum ".ci/install.sh" }}`
* We generate a git-rev-base to check whether we can trust the cache
  with `build-{{ arch }}-{{ checksum "git-rev-base" }}`

Binary dependencies require `{{ arch }}` because there are different CPUs used on the servers.
More information here: https://discuss.circleci.com/t/use-the-arch-cache-template-key-if-you-rely-on-cached-compiled-binary-dependencies/16129

* [fix] Statistics: onPosUpdate and save stats when closing document (koreader#3332)

* Fix cre scroll page update and allow jumping to page (koreader#3333)

This makes Go to, Skim to and TOC page selection work in
scroll mode, and page given to other module more accurate
(previously, we were one action lagging).

* CircleCI parallelization

* Enable parallelism in .circleci/config.yml
* Add BUSTED_SPEC_FILE to Makefile testfront
* Use it in .ci/test.sh with xargs for test parallelization

NB This is the dumb method of improving test time.
Ideally we want a workflow fan-in/fan-out approach.

* [fix] MockTime spec

* [fix] #nocov on broken scroll mode tests

* CircleCI: run docs & coverage in first container only (koreader#3335)

I wrote this whole complicated wofkflow-based config file, but except
in the case of a base rebuild it wouldn't really be any faster than
this simple tweak.

```
defaults: &defaults
    docker:
      - image: houqp/kobase:0.0.5
        environment:
          EMULATE_READER: 1
          # this is for shellcheck 0.4.5 and lower; can be removed for 0.4.6
          LC_ALL: en_US.UTF8

version: 2
jobs:
  install-and-build:
    <<: *defaults
    steps:
      - checkout
      - restore_cache:
          keys:
            # binary dependencies require {{ arch }} because there are different CPUs in use on the servers
            - deps-{{ arch }}-{{ checksum ".ci/install.sh" }}
      # need to init some stuff first or git will complain when sticking in base cache
      - run: git submodule init base && git submodule update base && pushd base && git submodule init && git submodule update && popd
      # we can't use command output directly for cache check so we write it to git-rev-base
      - run: pushd base && git_rev_base=$(git describe HEAD) && popd && echo $git_rev_base && echo $git_rev_base >git-rev-base
      - restore_cache:
          keys:
            - build-{{ arch }}-{{ checksum "git-rev-base" }}
      - run: echo 'export PATH=${HOME}/bin:${PATH}' >> $BASH_ENV
      - run:
          name: setup
          command: .ci/before_install.sh
      - run:
          name: install
          command: .ci/install.sh
      - save_cache:
          key: deps-{{ arch }}-{{ checksum ".ci/install.sh" }}
          paths:
            - "/home/ko/bin"
            - "/home/ko/.luarocks"
            # compiled luarocks binaries
            - "install"
      - run:
          name: fetch
          command: .ci/fetch.sh
      - run:
          name: check
          command: .ci/check.sh
      - run:
          name: build
          command: .ci/build.sh
      - save_cache:
          key: build-{{ checksum "base/git-rev" }}
          paths:
            - "/home/ko/.ccache"
            - "base"
      - persist_to_workspace:
          # Must be an absolute path, or relative path from working_directory
          root: "./"
          # Must be relative path from root
          paths:
            # front build
            - "koreader-emulator-x86_64-linux-gnu/koreader"

  test:
    <<: *defaults
    parallelism: 4
    steps:
      - checkout
      - restore_cache:
          keys:
            # binary dependencies require {{ arch }} because there are different CPUs in use on the servers
            - deps-{{ arch }}-{{ checksum ".ci/install.sh" }}
      # need to init some stuff first or git will complain when sticking in base cache
      - run: git submodule init base && git submodule update base && pushd base && git submodule init && git submodule update && popd
      # we can't use command output directly for cache check so we write it to git-rev-base
      - run: pushd base && git_rev_base=$(git describe HEAD) && popd && echo $git_rev_base && echo $git_rev_base >git-rev-base
      - restore_cache:
          keys:
            - build-{{ arch }}-{{ checksum "git-rev-base" }}
      - run: echo 'export PATH=${HOME}/bin:${PATH}' >> $BASH_ENV
      - attach_workspace:
          # Must be absolute path or relative path from working_directory
          at: "./"
      - run:
          name: test
          command: .ci/test.sh

  docs-and-coverage:
    <<: *defaults
    steps:
      - checkout
      - restore_cache:
          keys:
            # binary dependencies require {{ arch }} because there are different CPUs in use on the servers
            - deps-{{ arch }}-{{ checksum ".ci/install.sh" }}
      # need to init some stuff first or git will complain when sticking in base cache
      - run: git submodule init base && git submodule update base && pushd base && git submodule init && git submodule update && popd
      # we can't use command output directly for cache check so we write it to git-rev-base
      - run: pushd base && git_rev_base=$(git describe HEAD) && popd && echo $git_rev_base && echo $git_rev_base >git-rev-base
      - restore_cache:
          keys:
            - build-{{ arch }}-{{ checksum "git-rev-base" }}
      - run: echo 'export PATH=${HOME}/bin:${PATH}' >> $BASH_ENV
      - attach_workspace:
          # Must be absolute path or relative path from working_directory
          at: "./"
      - run:
          name: cleanup
          command: .ci/after_success.sh

workflows:
  version: 2
  build-and-test:
    jobs:
      - install-and-build
      - test:
          requires:
            - install-and-build
      - docs-and-coverage:
          requires:
            - install-and-build
```

* [CI] Switch to Codecov (koreader#3336)

The debug output from Coveralls was rubbish. `"Build processing error"`
Besides some basics like checking if we were actually sending
valid JSON that gives us absolutely nothing to work with.

* [feat] VirtualKeyboard cursor navigation (koreader#3290)

Still lacks Japanese due to insufficient knowledge of the language.

* README: style updates (koreader#3337)

Improved some stylistic issues.

* Updated Travis CI reference to CircleCI.
* Switched to CircleCI "shield" style badge to fit in better with the other badges.

* Small visual fixes to Menu (koreader#3338)

Rationalize horizontal construction of Menu items (TOC,
Bookmarks, Classic file views) for more even padding.
Align "x" close button diagonaly with top right border and title.

Also add forgotten scale_for_dpi to MultiConfirmBox

* Allow for disabling flashing of menu, icons and buttons  (koreader#3339)

* CircleCI: finishing touches (koreader#3340)

* junit test results; unfortunately this seems to conflict with the verbose out

* fix deps cache: two files can change independently

* verbose print obsoleted by gtest in upsream busted

* [fix] verbose_print deprecated by gtest

* [fix] gettext: die already you stupid language not found error (koreader#3341)

* Bump base (fixes landscape on Android) (koreader#3342)

Includes koreader/koreader-base#542

* Add gettext_spec stub (koreader#3343)

* dbg_spec: setVerbose test

* [chore] Rework util spec, rework util.secondsToClock: round seconds to minutes in 00:00 mode + spec

Most of the tests in util_spec were the wrong way around.
It's `assert(expected, given)`.

* [chore] Ignore empty files and tables in DocSettings (koreader#3348)

* calibre 'series' metadata fixes (koreader#3349)

Decode XML entities in series metadata and display decimal in series number
if any.

* Menu (TOC, bookmarks): add padding before right text (koreader#3350)

* FileBrowser: change page to show last file or previous subdir (koreader#3351)

When going from reader to filemanager, we are in the directory
containing the last_file. With this, we will also be on the page
showing this file.
When in filemanager and going up (".."), we will also be on the
page containing the directory we came from.

* Avoid recalculation of partial_md5_checksum at each opening (koreader#3352)

This is done by/for kosync plugin at each opening, because
the docsettings was re-opened and saved for this, but later
overwritten by the current koreader docsettings - so it was
redone each time. This correctly adds this partial_md5_checksum
to the current koreader docsettings, which will be saved on
document closing - so it will not be redone next time.
Note: this partial_md5_checksum is not (yet) used by anything.

* kodev: Add $ANDROID_ARCH to enable x86 build (koreader#3353)

You'll still have to call it with `ANDROID_ARCH=x86 ./kodev build/release/run android`.

Don't forget to `./mk-luajit.sh clean` in luajit-launcher when changing architectures.

* Bump android-luajit-launcher
  This includes the fix for Android 8. Fixes koreader#3126.
* Bump base

* [android] fix hide nav bar on activity create (koreader#3357)

* [Goodreads] lookup improvement (koreader#3354)

* add navigation button (next, prev) on the bottom of screen
* flash item when selecting
* small lookup improvements
* fix refresh "Please wait..."

* Small visual fixes to top menu (koreader#3356)

To make it more alike bottom menu:
- left and right border not displayed
- line below icons extends to screen borders
- same bottom border size
And make separator lines have same padding on both sides

* [Android] Enable DjVu (bump base) (koreader#3358)

Closes koreader#1534.

* djvu: enable color rendering (koreader#3361)

* djvu: enable color rendering

* Bump base

* README: fix "unarchieve" typo (koreader#3362)

* Bump base (koreader#3364)

*  Change JPG rendering to RGB in openJPGDocumentFromMem
*  fix build on Mac OSX and fix SDL input issues on OSX
* And some other bumps (glib, crengine, libiconv, libjpeg-turbo).

* Fix some widget title height and close button alignment (koreader#3366)

* close button alignment (koreader#3367)

* Enable Edit (rename bookmark) when tap on highlight (koreader#3369)

Also fix a few crash possibilities when unhighlighting.
Also fix bug with binary search that could not be able to remove bookmark
when there are multiple bookmarks/highlights on the same page.

* [fix] util.secondsToClock 00:60 should be 01:00 (koreader#3371)

* [Kobo] Add preliminary Kobo Aura H2O2 definition (koreader#3372)

Cf. ongoing discussion in KSM8 thread: https://www.mobileread.com/forums/showthread.php?p=3596020#post3596020

* Bump base (sdcv 0.5.2; fix *buntu 17.10 build & segfault) (koreader#3378)

Fixes koreader#3302. Fixes koreader#3071.

* kodev: add debug flags (koreader#3379)

* `--gdb=X`
* `--valgrind=X`

* Book Information: added file size (koreader#3380)

* Added util.getFriendlySize() (koreader#3381)

* Added util.getFriendlySize()

* Allow for GB

* Fix location of progress bar ticks (koreader#3382)

* Added util.getFormattedSize() (koreader#3383)

* Bump android-luajit-launcher (contains x86 nightly build fix) (koreader#3384)

* FileBrowser: optimize 'change page to show last file'

This feature, introduced some days ago, was actually
doing 2 updateItems calls: the initial one, and a second
to switch to focused_file page (cheap with classic display mode,
less cheap with CoverBrowser modes).
This change allows doing that in a single call.

* CoverBrowser: some optimizations

Speedup initialization (needs to be done only once, and avoid
saving current mode to sqlite each time) and reader-to-filebrowser
switch, by doing a single rendering (instead of 2 or 3 previously).
ListMenu: cache sidecar file parsing results (page/percent
completed) for the browsing session duration.
Fix "No choice available" when on last page and changing
to a display mode with less pages.

* Android NetworkManager (koreader#3386)

* Bump base (sdcv patch to allow enable/disable dicts) (koreader#3393)

* Fullscreen android - resize window after toggle fullscreen (koreader#3392)

* [fix, Android] adjust touch location for koreader#3392 (koreader#3394)

* NewsDownloader promptWifiOn (koreader#3388)

* [feat] SkimToWidget chapter markers & next/prev chapter/bookmark (koreader#3389)

koreader#2819 (comment)
+ refactoring code in skimtowidget
+ add chapter markers
+ add next/prev chapter buttons
+ add next/prev bookmark buttons

* [Android] Bump luajit-launcher (more lib loading debug info) (koreader#3401)

* [PocketBook] Use inkview-calls for battery-percentage/charging (koreader#3402)

* PocketBook: Basic device-detection /  PocketBook631 (Touch HD) support (koreader#3403)

Add basic device-detection via libinkview, support PocketBook631. Fixes koreader#3312

* Bump base (sdcv -u patch and framebuffer setViewport dynamically fix) (koreader#3404)

* KeyValuePage: option to align value to the right when half-overflow (koreader#3407)

Applied in FileManagerBookInfo.

* Bump base (sdcv -u patch for android) (koreader#3413)

* [UX] Fix dict title width to not go over CloseButton (koreader#3414)

* Wiki save as epub: close highlight menu when switching document (koreader#3417)

* separate strings for singular and plural (koreader#3420)

* Bump base (sdcv -u patch for android again) (koreader#3421)

* DataStorage: also create data/tessdata and minor string concat performance improvements (koreader#3430)

* [fix] Dictionary no fuzzy search flag (koreader#3433)

Upstream uses `-e` rather than the `-f` that was first introduced in our fork.

References koreader#3429 (comment)

* NewsDownloader: use feed.description as news context instead download full web page (koreader#3426)

Fixes koreader#3425.

* Bump base (updated libk2pdfopt) (koreader#3437)

* kodev: add assert_ret_zero after make android-ndk (koreader#3438)

Fixes koreader#3408.

* [build] Add -debug suffix to KODEBUG builds (koreader#3439)

* NewsDownloader: Remove NewsDownloader files from history  (koreader#3424)

* kodev: added setup_env to run android

Otherwise it didn't pick up on the -debug suffix.

* Bump android-luajit-launcher

* [Android] Wifi status in footer (koreader#3396)

Implemented through the Android API so it's reasonably cheap.

* NewsDownloader: wifi off prompt after download (koreader#3395)

* [UX] Improve ConfigDialog (koreader#3443)

* [CI] Also run coverage on plugins (koreader#3447)

* NewsDownloader: Compatibility for users with previous configuration files (koreader#3445)

* Bump base (fix wpa crash by @civalin) (koreader#3449)

For:
* [fix] filter out more bad lines in wpa scan result (koreader/lj-wpaclient#3)

Fixes koreader#3436

* [CI] apply patch for busted junit testcase time (koreader#3450)

Apply junit testcase time fix so CircleCI can actually do something useful
with the results. This can be removed once there is a busted 2.0.rc13 or final

See lunarmodules/busted@830f175

* [fix, CI] silly typo in busted junit output patch

* README: tiny grammar fix (koreader#3453)

* ConfigDialog improvements 2 (koreader#3455)

+ Fix height for ToggleWidget
+ Able ToggleWidget width more than half of width of screen

* [fix] Disable Highlight button when text is not selected (koreader#3457)

* i10n/README: small stylistic changes

* doc/Unit tests: minor extra explanation (koreader#3462)

* More contrast settings (koreader#3463)

Close: koreader#2133 
More info: koreader#2133
poire-z referenced this pull request in koreader/koreader-base Mar 14, 2019
This module is like FetchContent in CMake 3.11, but for the moment it may be more inviting to hackers not to require that yet.

The principle is explained here: https://crascit.com/2015/07/25/cmake-gtest/

It allows us to work with CMake in a more natural environment for CMake-submodules, which are otherwise nigh-uncontrollable in between escaping variables in CMake syntax and CMake being extremely bad at taking command-line arguments. (A fact I utterly despise as a regular user just downloading a project to compile & try.)

Upping the minimum CMake version over 3.3 should also allow us to get rid of a few braindead Android-related workarounds for sdcv, where we had to completely force-feed the `.a` suffix.
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.

3 participants