Bug 241719

Summary: print/texinfo: fix NLS build, part 2
Product: Ports & Packages Reporter: John Hein <jcfyecrayz>
Component: Individual Port(s)Assignee: Sunpoet Po-Chuan Hsieh <sunpoet>
Status: New ---    
Severity: Affects Some People CC: sunpoet
Priority: --- Flags: bugzilla: maintainer-feedback? (sunpoet)
Version: Latest   
Hardware: Any   
OS: Any   
Description Flags
[patch] fix texinfo build problems with libintl.h detection
[patch] [v2] fix texinfo gettext/libintl/iconv issues
[patch] [v3] v2 fixes plus fix for non-default PREFIX jcfyecrayz: maintainer-approval? (sunpoet)

Description John Hein 2019-11-05 02:32:15 UTC
After ports/r515761, the build now gets missing prototype warnings like so:

parsetexi/api.c:74:7: warning: implicit declaration of function 'bindtextdomain' is invalid in C99 [-Wimplicit-function-declaration]
      bindtextdomain (PACKAGE, s);
parsetexi/api.c:89:5: warning: implicit declaration of function 'bindtextdomain' is invalid in C99 [-Wimplicit-function-declaration]
    bindtextdomain (PACKAGE, LOCALEDIR);
parsetexi/api.c:91:3: warning: implicit declaration of function 'textdomain' is invalid in C99 [-Wimplicit-function-declaration]
  textdomain (PACKAGE);

There are a few problems involved:

 (a) The top level configure does not detect libintl.h, so it does not define HAVE_LIBINTL_H (because it doesn't look in /usr/local/include).

 (b) Even if it did set HAVE_LIBINTL_H (add USES=localbase would do the job), the configure that is run in tp/Texinfo/XS is different.  It does not even try to detect libintl.h and so it also does not set HAVE_LIBINTL_H.  This means libintl.h is never included which triggers the compiler warnings.

Thus, the change in ports/r515761 effectively just comment out the include of libintl.h.  An #if 0 would have done just as well.

A fix for (a) is "NLS_USES=gettext localbase" (instead of NLS_USES=gettext).

A fix for (b) is to teach tp/Texinfo/XS/configure(.ac) to look for libintl.h - perhaps use the same method the top level configure uses.
Comment 1 John Hein 2019-11-05 03:01:25 UTC
Created attachment 208867 [details]
[patch] fix texinfo build problems with libintl.h detection

The attached patch fixes (a) and (b) for the FreeBSD port.  It borrows the libintl.h detection from the top level configure script and applies it to the configure that is run in tp/Texinfo/XS.

The fix submitted upstream should presumably address libintl.h detection in tp/Texinfo/XS/configure.ac.  The attached patch does not include that.
Comment 2 John Hein 2019-11-05 14:40:20 UTC
Hmm... also ports/r515761 should probably reverted and instead use something to disable the parsetexi compilation.  That code does links with functions that are supplied by libintl.  Since turning off the NLS option pretty much means not to use libintl, anything that unconditionally requires libintl should not be built as part of the package.
Comment 3 John Hein 2019-11-05 17:01:41 UTC
Created attachment 208884 [details]
[patch] [v2] fix texinfo gettext/libintl/iconv issues

This patch reverts ports/r515761.  It's not needed since gettext-runtime is always necessary for this port.  It links with libintl and fails if it's not there and it also expects libintl.h to be present.  There's no easy knob that I found to disable the gettext-runtime dependency (--disable-perl-xs doesn't help, for instance).

So NLS off just means don't install the .mo files, not that gettext-runtime is not needed.  This patch uses that interpretation of the NLS option knob.

The patch also forces the build to not use libiconv (by setting LTLIBICONV=) in case the port happens to be installed - iconv in base is sufficient.  And if it links with /usr/local/lib/libiconv.so, then a Q/A check fails (DEVELOPER=yes) with the following:

Error: /usr/local/lib/texinfo/Parsetexi.so is linked to /usr/local/lib/libiconv.so.2 from converters/libiconv but it is not declared as a dependency

The patch also only depends on gettext-runtime if NLS is off.  If NLS is on, it adds gettext-tools (build dep).

Finally the patch adds USES=localbase until upstream fixes tp/Texinfo/XS/configure.ac to detect libintl.h and set -I flags accordingly.  But it's really just luck that -I/usr/local/include is set when building tp/Texinfo/XS - it gets that from perl config (the CPPFLAGS from Uses/localbase.mk are overridden by PERL_EXT_CPPFLAGS - setting that in CONFIGURE_ARGS is probably really the right thing to do - that could be added, but I didn't bother in this version of the patch.  Feel free to add that (CONFIGURE_ARGS+=PERL_EXT_CPPFLAGS=${CPPFLAGS}), too.
Comment 4 John Hein 2019-11-06 00:14:45 UTC
Created attachment 208898 [details]
[patch] [v3] v2 fixes plus fix for non-default PREFIX

In addition to the things fixed in v2 (see comment 3), v3 of the patch fixes non-default PREFIX.

If you build with 'make stage check-plist PREFIX=/opt', you get failures like:

===> Checking for items in STAGEDIR missing from pkg-plist
Error: Orphaned: /usr/loc/%%TEXMFDIR%%/tex/generic/epsf/epsf.tex
Error: Orphaned: /usr/loc/%%TEXMFDIR%%/tex/texinfo/texinfo-ja.tex
Error: Orphaned: /usr/loc/%%TEXMFDIR%%/tex/texinfo/texinfo.tex
Error: Orphaned: @dir /usr/loc/%%TEXMFDIR%%
===> Checking for items in pkg-plist which are not in STAGEDIR
Error: Missing: %%TEXMFDIR%%/tex/generic/epsf/epsf.tex
Error: Missing: %%TEXMFDIR%%/tex/texinfo/texinfo-ja.tex
Error: Missing: %%TEXMFDIR%%/tex/texinfo/texinfo.tex
Error: Missing: %%TEXMFDIR%%/tex/texinfo/txi-ca.tex
===> Error: Plist issues found.
===> Warning: Test was done with PREFIX != LOCALBASE
===> Warning: The port may not be properly installing into PREFIX
*** Error code 1

v3 also adds PERL_EXT_CPPFLAGS="${CPPFLAGS}" to explicitly add flags for the compiler to find libintl.h (instead of accidentally relying on perl config's -I flags) - as discussed in comment 3.