Created attachment 111567 [details] [review]
Patch

Related historical bugs include bug #115772, bug #85217, bug #59386, maybe bug #70627

And of course I meant to create this with my non-nokia account :) (for clarification)

I'm not necessarily against using autopoint, but --disable-nls is a weak argument, since we don't support building GLib without nls.

Why don't you support it?

It was brought up on the mailing list that it would disable utf-8 utility functions, but that's not true.

Can I get an answer?

I think the answer is that we don't support it because we don't.

(In reply to comment #7)
> I think the answer is that we don't support it because we don't.
> 

hrm... that is not a clear reason. let's not play, and just get a clarification.

(In reply to comment #8)
> (In reply to comment #7)
> > I think the answer is that we don't support it because we don't.
> > 
> 
> hrm... that is not a clear reason. let's not play, and just get a
> clarification.

This is just my personal opinion.  I believe it's not supported because the maintainers do not have enough time to care about the case of disabled nls.

"Why don't you support it" is not the right question.  What can be a right question for example is presenting legitimate and widely-enough useful use case for disabled nls.

The decision was made for GLib 2.0 that any app that uses glib can 
count reliably on having gettext support. (And we succesfully campaigned
to get gnu libintl relicensed under the LGPL to avoid having to 
rewrite the code ourself.)

An ability to configure without gettext support could potentially be supported
for embedded systems where the set of apps is constrained and there is no
need to support the official GLib API, though I think there is no interest among 
the GLib maintainers in doing that work themselves. 

But GLib must not accidentally build without gettext if the user forgot
to install the right devel headers or similar.

Do you have example where the system doesn't already provides libintl and some library requires it? On the other hand there are real-world use-cases for which a GLib without NLS makes sense.

I will try to compile GLib for win32 with these patches and without libintl to see what happens, but I believe a big warning "internationalization is disabled" would be enough. Anyone with the guts to compile their own GLib in a system without libintl must be wise enough to see that.

If you don't agree with there's always the possibility to always fail if libintl is not there, unless --disable-nls is specified. Or you can include libintl into GLib if there isn't already one in the system.

Some patch review:

+AM_GNU_GETTEXT([external])

We use dngettext, so this should probably be

AM_GNU_GETTEXT([external],[need-ngettext])


+if test "$USE_NLS" = "yes"; then
+	GETTEXT_PACKAGE=[glib20]
+else
+	GETTEXT_PACKAGE=[NULL]
 fi

As Owen pointed out, we still want to error out unless an explicit commandline switch was given to disable nls.


Index: po/LINGUAS

We already switched to po/LINGUAS, so your patch probably needs some updates


-$(srcdir)/$(GETTEXT_PACKAGE).pot: $(POTFILES)
-	$(XGETTEXT) --default-domain=$(GETTEXT_PACKAGE) --directory=$(top_srcdir) \
-	  --add-comments --keyword=_ --keyword=N_ \
-          --flag=g_strdup_printf:1:c-format \
-          --flag=g_string_printf:2:c-format \
-          --flag=g_string_append_printf:2:c-format \
-          --flag=g_error_new:3:c-format \
-          --flag=g_set_error:4:c-format \
-          --flag=g_markup_printf_escaped:1:c-format \
-          --flag=g_log:3:c-format \
-          --flag=g_print:1:c-format \
-          --flag=g_printerr:1:c-format \
-          --flag=g_printf:1:c-format \
-          --flag=g_fprintf:2:c-format \
-          --flag=g_sprintf:2:c-format \
-          --flag=g_snprintf:3:c-format \
-          --flag=g_scanner_error:2:c-format \
-          --flag=g_scanner_warn:2:c-format \

All these xgettext options should be moved over to po/Makevars instead
of just dropping them.

Created attachment 126405 [details] [review]
Updated patch.

I'm attaching a new patch with all the comments from Matthias.

Now an error message is displayed unless --disable-nls is specified.

Looks mostly ok now, though a few questions remain:

- Whats the story with GETTEXT_PACKAGE vs PACKAGE ? Previously, po/Makefile.in.in used GETTEXT_PACKAGE, now it uses PACKAGE. May not make any difference.

- Running autogen.sh before and after the patch yields some differences in config.h:

-/* Define to 1 if you have the `bind_textdomain_codeset' function. */
-#define HAVE_BIND_TEXTDOMAIN_CODESET 1
-

This is bad, since the define is used in glib/gutils.c

+/* Define if you have the iconv() function and it works. */
+/* #undef HAVE_ICONV */
+

This is curious, since I certainly have a working iconv, but should have no other bad effects.


- More serious, after applying the patch, I don't see po/Makefile.in or po/Makefile being generated by autogen.sh

-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/143.