[v2] intl/tst-gettext: fix failure with newest msgfmt

Message ID 20180208211336.28267-1-aurelien@aurel32.net
State New, archived
Headers

Commit Message

Aurelien Jarno Feb. 8, 2018, 9:13 p.m. UTC
  Since upstream gettext commit d13f165b83 (msgfmt: Remove
POT-Creation-Date field from the header in the output.), msgfmt does not
copy the POT-Creation-Date field in the header entry from the po file to
the mo file anymore. This breaks the assumption that we can test gettext
by comparing each message in the po files with the corresponding string
return by gettext. This makes the intl/tst-gettext to fail.

While it would have been possible to modify the po2test.awk script to
also strip the line POT-Creation-Date field when creating the msgs.h
file, it would not work with both the old and new msgfmt.

Instead create a tst-gettext-de.po file from de.po by removing the
POT-Creation-Date line. Another alternative would be to use a static
tst-gettext-de.po file, but I guess the reason for using de.po is to
also catch issues caused by newly added strings.

As tst-catgets also uses msg.h, it should also be updated. Instead of
using the new tst-gettext-de.po file, the patch modifies xopen-msg.awk
to avoid creating a second catgets->intl dependency.

Changelog:
	[BZ #21508]
	* catgets/xopen-msg.awk: Ignore POT-Creation-Date line.
	* intl/Makefile ($(objpfx)tst-gettext-de.po): Generate
	intl/tst-gettext-de.po from po/de.po by removing the
	POT-Creation-Date line.
	($(objpfx)msgs.h): Depend on $(objpfx)tst-gettext-de.po instead of
	../po/de.po.
	* intl/tst-gettext.sh: Use ${objpfx}tst-gettext-de.po instead of
	../po/de.po.
---
 ChangeLog             | 12 ++++++++++++
 catgets/xopen-msg.awk |  5 +++++
 intl/Makefile         |  6 +++++-
 intl/tst-gettext.sh   |  4 ++--
 4 files changed, 24 insertions(+), 3 deletions(-)

Changes v1 -> v2:
  - Do not use tst-gettext-de.po in catgets, instead mangle the file
    in xopen-msg.awk.
  

Comments

Zack Weinberg Feb. 8, 2018, 9:39 p.m. UTC | #1
On Thu, Feb 8, 2018 at 4:13 PM, Aurelien Jarno <aurelien@aurel32.net> wrote:
> Since upstream gettext commit d13f165b83 (msgfmt: Remove
> POT-Creation-Date field from the header in the output.), msgfmt does not
> copy the POT-Creation-Date field in the header entry from the po file to
> the mo file anymore. This breaks the assumption that we can test gettext
> by comparing each message in the po files with the corresponding string
> return by gettext. This makes the intl/tst-gettext to fail.
>
> While it would have been possible to modify the po2test.awk script to
> also strip the line POT-Creation-Date field when creating the msgs.h
> file, it would not work with both the old and new msgfmt.
>
> Instead create a tst-gettext-de.po file from de.po by removing the
> POT-Creation-Date line. Another alternative would be to use a static
> tst-gettext-de.po file, but I guess the reason for using de.po is to
> also catch issues caused by newly added strings.
>
> As tst-catgets also uses msg.h, it should also be updated. Instead of
> using the new tst-gettext-de.po file, the patch modifies xopen-msg.awk
> to avoid creating a second catgets->intl dependency.

LGTM, do any intl maintainers wish to comment?

zw
  
Aurelien Jarno Feb. 15, 2018, 10 p.m. UTC | #2
On 2018-02-08 16:39, Zack Weinberg wrote:
> On Thu, Feb 8, 2018 at 4:13 PM, Aurelien Jarno <aurelien@aurel32.net> wrote:
> > Since upstream gettext commit d13f165b83 (msgfmt: Remove
> > POT-Creation-Date field from the header in the output.), msgfmt does not
> > copy the POT-Creation-Date field in the header entry from the po file to
> > the mo file anymore. This breaks the assumption that we can test gettext
> > by comparing each message in the po files with the corresponding string
> > return by gettext. This makes the intl/tst-gettext to fail.
> >
> > While it would have been possible to modify the po2test.awk script to
> > also strip the line POT-Creation-Date field when creating the msgs.h
> > file, it would not work with both the old and new msgfmt.
> >
> > Instead create a tst-gettext-de.po file from de.po by removing the
> > POT-Creation-Date line. Another alternative would be to use a static
> > tst-gettext-de.po file, but I guess the reason for using de.po is to
> > also catch issues caused by newly added strings.
> >
> > As tst-catgets also uses msg.h, it should also be updated. Instead of
> > using the new tst-gettext-de.po file, the patch modifies xopen-msg.awk
> > to avoid creating a second catgets->intl dependency.
> 
> LGTM, do any intl maintainers wish to comment?

Given there are no further comments, I plan to commit that over the
week-end, unless someone objects in the meantime.
  
Carlos O'Donell Feb. 15, 2018, 10:09 p.m. UTC | #3
On 02/08/2018 01:13 PM, Aurelien Jarno wrote:
> Since upstream gettext commit d13f165b83 (msgfmt: Remove
> POT-Creation-Date field from the header in the output.), msgfmt does not
> copy the POT-Creation-Date field in the header entry from the po file to
> the mo file anymore. This breaks the assumption that we can test gettext
> by comparing each message in the po files with the corresponding string
> return by gettext. This makes the intl/tst-gettext to fail.
> 
> While it would have been possible to modify the po2test.awk script to
> also strip the line POT-Creation-Date field when creating the msgs.h
> file, it would not work with both the old and new msgfmt.
> 
> Instead create a tst-gettext-de.po file from de.po by removing the
> POT-Creation-Date line. Another alternative would be to use a static
> tst-gettext-de.po file, but I guess the reason for using de.po is to
> also catch issues caused by newly added strings.
> 
> As tst-catgets also uses msg.h, it should also be updated. Instead of
> using the new tst-gettext-de.po file, the patch modifies xopen-msg.awk
> to avoid creating a second catgets->intl dependency.
> 
> Changelog:
> 	[BZ #21508]
> 	* catgets/xopen-msg.awk: Ignore POT-Creation-Date line.
> 	* intl/Makefile ($(objpfx)tst-gettext-de.po): Generate
> 	intl/tst-gettext-de.po from po/de.po by removing the
> 	POT-Creation-Date line.
> 	($(objpfx)msgs.h): Depend on $(objpfx)tst-gettext-de.po instead of
> 	../po/de.po.
> 	* intl/tst-gettext.sh: Use ${objpfx}tst-gettext-de.po instead of
> 	../po/de.po.

LGTM.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>
  

Patch

diff --git a/ChangeLog b/ChangeLog
index 3a279d7e311..bbfd6c48afa 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@ 
+2018-02-08  Aurelien Jarno  <aurelien@aurel32.net>
+
+	[BZ #21508]
+	* catgets/xopen-msg.awk: Ignore POT-Creation-Date line.
+	* intl/Makefile ($(objpfx)tst-gettext-de.po): Generate
+	intl/tst-gettext-de.po from po/de.po by removing the
+	POT-Creation-Date line.
+	($(objpfx)msgs.h): Depend on $(objpfx)tst-gettext-de.po instead of
+	../po/de.po.
+	* intl/tst-gettext.sh: Use ${objpfx}tst-gettext-de.po instead of
+	../po/de.po.
+
 2018-02-07  Joseph Myers  <joseph@codesourcery.com>
 
 	[BZ #17979]
diff --git a/catgets/xopen-msg.awk b/catgets/xopen-msg.awk
index a13d6fa06a7..bab0f6b4efe 100644
--- a/catgets/xopen-msg.awk
+++ b/catgets/xopen-msg.awk
@@ -58,6 +58,11 @@  $1 ~ "msg(id|str)" {
     next
 }
 
+/^"POT-Creation-Date: [0-9-]+ [0-9:+-]+\\n"/ {
+    # Ignore POT-Creation-Date to match what is done in intl/Makefile.
+    next
+}
+
 /^".*"/ {
     # Append to current message
     sub(/^"/, "", $0)
diff --git a/intl/Makefile b/intl/Makefile
index 2219717a5ae..672edf1b387 100644
--- a/intl/Makefile
+++ b/intl/Makefile
@@ -120,7 +120,11 @@  $(objpfx)tst-gettext6.out: $(gen-locales)
 $(objpfx)tst-translit.out: $(gen-locales)
 endif
 
-$(objpfx)msgs.h: po2test.awk ../po/de.po
+$(objpfx)tst-gettext-de.po: ../po/de.po
+	$(make-target-directory)
+	LC_ALL=C $(AWK) '!/^"POT-Creation-Date: [0-9-]+ [0-9:+-]+\\n"$$/' $^ > $@
+
+$(objpfx)msgs.h: po2test.awk $(objpfx)tst-gettext-de.po
 	$(make-target-directory)
 	LC_ALL=C $(AWK) -f $^ > $@
 
diff --git a/intl/tst-gettext.sh b/intl/tst-gettext.sh
index af9c7d7d922..0c65583149f 100755
--- a/intl/tst-gettext.sh
+++ b/intl/tst-gettext.sh
@@ -42,9 +42,9 @@  mkdir -p ${objpfx}domaindir/existing-locale/LC_MESSAGES
 mkdir -p ${objpfx}domaindir/existing-locale/LC_TIME
 # Populate them.
 msgfmt -o ${objpfx}domaindir/existing-locale/LC_MESSAGES/existing-domain.mo \
-       -f ../po/de.po
+       -f ${objpfx}tst-gettext-de.po
 msgfmt -o ${objpfx}domaindir/existing-locale/LC_TIME/existing-time-domain.mo \
-       -f ../po/de.po
+       -f ${objpfx}tst-gettext-de.po
 
 # Now run the test.
 ${test_program_prefix_before_env} \