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

djvulibre: don't mangle locales #1565

Merged
merged 2 commits into from
Dec 16, 2022
Merged

djvulibre: don't mangle locales #1565

merged 2 commits into from
Dec 16, 2022

Conversation

NiLuJe
Copy link
Member

@NiLuJe NiLuJe commented Dec 15, 2022

It's generally a bad idea, because this runs in our main thread, and locales in POSIX are a mess.

May help w/ koreader/koreader#9918


This change is Reviewable

It's generally a bad idea, because this rusn in our main thread, and
locales in POSIX are a mess.

May help w/ koreader/koreader#9918
@NiLuJe
Copy link
Member Author

NiLuJe commented Dec 15, 2022

Picked a few upstream fixes while I was in there ;o).

Comment on lines +57 to +61
# define DJVU_LOCALE do { \
- setlocale(LC_ALL,""); \
- setlocale(LC_NUMERIC,"C"); \
djvu_programname(argv[0]); } while (0)
#endif
Copy link
Member

Choose a reason for hiding this comment

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

This one does seem a bit curious and in need of patching.

Copy link
Member Author

@NiLuJe NiLuJe Dec 16, 2022

Choose a reason for hiding this comment

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

It should only affect tools, though, which we probably don't use ;o).

The culprit is likely the context creation one, I would imagine.

(Again, I did a very wide sweep just to be safe).

Copy link
Member

Choose a reason for hiding this comment

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

I didn't look at the context in detail, common.h does sound quite, ahem, common though xp

Comment on lines +13 to +14
-#ifndef DO_CHANGELOCALE
-# define DO_CHANGELOCALE 0
Copy link
Member

Choose a reason for hiding this comment

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

Might be better to just -DDO_CHANGELOCALE=0 or whatever instead of doing a patch that needs to be maintained? (Although this particular dependency is fairly static.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I went with the nuclear option just to be safe (and because I already had a patch to generate anyway).

Copy link
Member

Choose a reason for hiding this comment

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

Alright.

Copy link
Member

@Frenzie Frenzie left a comment

Choose a reason for hiding this comment

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

lgtm, but I think the patch to libdjvu/GString.cpp may not be necessary as stated in the comment

@NiLuJe NiLuJe merged commit 36b8887 into koreader:master Dec 16, 2022
NiLuJe added a commit to NiLuJe/koreader that referenced this pull request Dec 19, 2022
NiLuJe added a commit to koreader/koreader that referenced this pull request Dec 19, 2022
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.

None yet

2 participants