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

src/ydpdict.c: Fix call to undeclared function textdomain #4

Closed
wants to merge 1 commit into from

Conversation

listout
Copy link

@listout listout commented Jul 30, 2023

First discovered on Gentoo linux (musl llvm profile).

Bug: https://bugs.gentoo.org/894364

First discovered on Gentoo linux (musl llvm profile).

Bug: https://bugs.gentoo.org/894364
Signed-off-by: Brahmajit Das <brahmajit.xyz@gmail.com>
listout added a commit to listout/gentoo that referenced this pull request Jul 30, 2023
Upstream PR: wojtekka/ydpdict#4

Closes: https://bugs.gentoo.org/894364
Signed-off-by: Brahmajit Das <brahmajit.xyz@gmail.com>
@wojtekka
Copy link
Owner

wojtekka commented Aug 3, 2023

Actually I believe that wrapping textdomain() call with #ifdef ENABLE_NLS instead is a correct solution for this, i.e.:

#ifdef HAVE_LOCALE_H
    setlocale(LC_ALL, "");
#endif
#ifdef ENABLE_NLS
    textdomain("ydpdict");
#endif

Would you be able to check if this works in Gentoo?

@listout
Copy link
Author

listout commented Aug 3, 2023

Sure, I'll check this on gentoo.

@listout
Copy link
Author

listout commented Aug 3, 2023

@wojtekka Build output with your patch https://bpa.st/raw/67XQ

gentoo-bot pushed a commit to gentoo/gentoo that referenced this pull request Aug 19, 2023
Upstream PR: wojtekka/ydpdict#4

Closes: https://bugs.gentoo.org/894364
Signed-off-by: Brahmajit Das <brahmajit.xyz@gmail.com>
Closes: #32101
Signed-off-by: Joonas Niilola <juippis@gentoo.org>
MocioF pushed a commit to MocioF/gentoo that referenced this pull request Aug 23, 2023
Upstream PR: wojtekka/ydpdict#4

Closes: https://bugs.gentoo.org/894364
Signed-off-by: Brahmajit Das <brahmajit.xyz@gmail.com>
Closes: gentoo#32101
Signed-off-by: Joonas Niilola <juippis@gentoo.org>
@wojtekka
Copy link
Owner

Sorry for the delay. I re-read the gettext manual, applied the suggestions, removed the custom HAVE_LOCALE_H macro, ran gettextize again, used the recent autotools, updated configure.ac etc. Would you be so kind to check if the source code from the main branch build properly under Gentoo on a musl clang system? Or maybe point me to some VM or Docker image I could experiment on?

@listout
Copy link
Author

listout commented Oct 21, 2023

Yep, code from master compiles fine under musl-llvm profile. But needed a small patch

diff --git a/configure.ac b/configure.ac
index 2a3931e..f87548c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -14,6 +14,7 @@ AC_C_BIGENDIAN
 
 AC_CHECK_HEADERS([linux/soundcard.h])
 AM_GNU_GETTEXT([external])
+AM_GNU_GETTEXT_VERSION([0.14.4])
 
 AC_CONFIG_MACRO_DIRS([m4])
 

Probably due to required by autopoint executed by autoreconf. Inspiration came from SSSD/ding-libs@868a81b. Thanks.


On the VM part, it's a bit of a work but I'm using a LXD image (custom) from clang-llvm profile. You might get help from https://blogs.gentoo.org/gsoc/2023/07/24/creating-custom-lxd-gentoo-containers-from-stage-3-tarballs/

@wojtekka
Copy link
Owner

I applied your patch to the main branch, thank you. It looks like it builds properly under Gentoo (regular and musl-llvm), Fedora, Debian and Ubuntu. I will release a new version in a moment.

Oh, and I managed to get a musl-llvm container with plain old Docker, using stage3 as base image, then emerge-webrsync and emerge sys-devel/clang. It may not seem like much, but I've never used Gentoo before, so it was quite a journey for me ;-)

@wojtekka
Copy link
Owner

Fixed in the new release: https://github.com/wojtekka/ydpdict/releases/tag/1.0.4

@wojtekka wojtekka closed this Oct 21, 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.

None yet

2 participants