Re: meaningless msgmerge error message in case of charset problems (fwd)

[Bruno Haible]

Hans Ulrich Niedermann wrote:
> When running msgmerge on a ja.po file with
>
> ,-----
>
> |# Copyright \251 2000-2002 Free Software Foundation, Inc.
>
> `-----
>
> (note the iso-8859-1 (C) copyright character octal 251 in the comment)
> and with
>
> ,-----
>
> |"Content-Type: text/plain; charset=euc-jp\n"
>
> `-----
>
> in the specification, I got this meaningless error message:
>
> ,-----
>
> |msgmerge: conversion failure
>
> `-----
>
> It only has a meaning if you already know what the problem is. I suggest
> augmenting the error message

The appended patch has been applied to gettext mainline in response to
this report.

> Apart from that, it may possibly be desirable for msgmerge & Co. to
>   - only consider the strings in msgid "" and msgstr "" to be encoded in
>     the given charset
>   - ignore 8-bin characters found in .po file comments
> but I don't think this is important.

msgmerge sometimes need to convert a PO file from one encoding to another,
like msgconv does. When the input is not in the specified encoding, it'd
be fatal to ignore conversion problems even in comments. We wouldn't
gain much by sloppily allowing the comments to be in any other encoding.

Bruno


2005-05-01  Bruno Haible  <address@hidden>

        Improved error message.
        * msgl-iconv.h (struct conversion_context): New type.
        (convert_string): Add context argument.
        * msgl-iconv.c (conversion_error): New function.
        (convert_string, convert_string_list, convert_msgid, convert_msgstr):
        Add context argument.
        (iconv_message_list): Construct context for them.
        * xgettext.c (convert_string): Add context argument.
        (from_current_source_encoding): Construct context for convert_string.
        Reported by Hans Ulrich Niedermann <address@hidden>.

*** gettext-tools/src/msgl-iconv.c      19 Dec 2003 11:31:30 -0000      1.6
--- gettext-tools/src/msgl-iconv.c      3 May 2005 10:41:18 -0000       1.7
***************
*** 1,5 ****
  /* Message list charset and locale charset handling.
!    Copyright (C) 2001-2003 Free Software Foundation, Inc.
     Written by Bruno Haible <address@hidden>, 2001.
  
     This program is free software; you can redistribute it and/or modify
--- 1,5 ----
  /* Message list charset and locale charset handling.
!    Copyright (C) 2001-2003, 2005 Free Software Foundation, Inc.
     Written by Bruno Haible <address@hidden>, 2001.
  
     This program is free software; you can redistribute it and/or modify
***************
*** 171,178 ****
  #undef tmpbufsize
  }
  
  char *
! convert_string (iconv_t cd, const char *string)
  {
    size_t len = strlen (string) + 1;
    char *result = NULL;
--- 171,199 ----
  #undef tmpbufsize
  }
  
+ static void conversion_error (const struct conversion_context* context)
+ #if defined __GNUC__ && ((__GNUC__ == 2 && __GNUC_MINOR__ >= 5) || __GNUC__ > 
2)
+      __attribute__ ((noreturn))
+ #endif
+ ;
+ static void
+ conversion_error (const struct conversion_context* context)
+ {
+   if (context->to_code == po_charset_utf8)
+     /* If a conversion to UTF-8 fails, the problem lies in the input.  */
+     error (EXIT_FAILURE, 0, _("%s: input is not valid in \"%s\" encoding"),
+          context->from_filename, context->from_code);
+   else
+     error (EXIT_FAILURE, 0,
+          _("%s: error while converting from \"%s\" encoding to \"%s\" 
encoding"),
+          context->from_filename, context->from_code, context->to_code);
+   /* NOTREACHED */
+   abort ();
+ }
+ 
  char *
! convert_string (iconv_t cd, const char *string,
!               const struct conversion_context* context)
  {
    size_t len = strlen (string) + 1;
    char *result = NULL;
***************
*** 184,214 ****
        && strlen (result) == resultlen - 1)
        return result;
  
!   error (EXIT_FAILURE, 0, _("conversion failure"));
    /* NOTREACHED */
    return NULL;
  }
  
  static void
! convert_string_list (iconv_t cd, string_list_ty *slp)
  {
    size_t i;
  
    if (slp != NULL)
      for (i = 0; i < slp->nitems; i++)
!       slp->item[i] = convert_string (cd, slp->item[i]);
  }
  
  static void
! convert_msgid (iconv_t cd, message_ty *mp)
  {
!   mp->msgid = convert_string (cd, mp->msgid);
    if (mp->msgid_plural != NULL)
!     mp->msgid_plural = convert_string (cd, mp->msgid_plural);
  }
  
  static void
! convert_msgstr (iconv_t cd, message_ty *mp)
  {
    char *result = NULL;
    size_t resultlen;
--- 205,238 ----
        && strlen (result) == resultlen - 1)
        return result;
  
!   conversion_error (context);
    /* NOTREACHED */
    return NULL;
  }
  
  static void
! convert_string_list (iconv_t cd, string_list_ty *slp,
!                    const struct conversion_context* context)
  {
    size_t i;
  
    if (slp != NULL)
      for (i = 0; i < slp->nitems; i++)
!       slp->item[i] = convert_string (cd, slp->item[i], context);
  }
  
  static void
! convert_msgid (iconv_t cd, message_ty *mp,
!              const struct conversion_context* context)
  {
!   mp->msgid = convert_string (cd, mp->msgid, context);
    if (mp->msgid_plural != NULL)
!     mp->msgid_plural = convert_string (cd, mp->msgid_plural, context);
  }
  
  static void
! convert_msgstr (iconv_t cd, message_ty *mp,
!               const struct conversion_context* context)
  {
    char *result = NULL;
    size_t resultlen;
***************
*** 242,248 ****
          }
        }
  
!   error (EXIT_FAILURE, 0, _("conversion failure"));
  }
  
  #endif
--- 266,272 ----
          }
        }
  
!   conversion_error (context);
  }
  
  #endif
***************
*** 345,350 ****
--- 369,375 ----
      {
  #if HAVE_ICONV
        iconv_t cd;
+       struct conversion_context context;
        bool msgids_changed;
  
        /* Avoid glibc-2.1 bug with EUC-KR.  */
***************
*** 360,365 ****
--- 385,394 ----
  and iconv() does not support this conversion."),
               canon_from_code, canon_to_code, basename (program_name));
  
+       context.from_code = canon_from_code;
+       context.to_code = canon_to_code;
+       context.from_filename = from_filename;
+ 
        msgids_changed = false;
        for (j = 0; j < mlp->nitems; j++)
        {
***************
*** 367,376 ****
  
          if (!is_ascii_string (mp->msgid))
            msgids_changed = true;
!         convert_string_list (cd, mp->comment);
!         convert_string_list (cd, mp->comment_dot);
!         convert_msgid (cd, mp);
!         convert_msgstr (cd, mp);
        }
  
        iconv_close (cd);
--- 396,405 ----
  
          if (!is_ascii_string (mp->msgid))
            msgids_changed = true;
!         convert_string_list (cd, mp->comment, &context);
!         convert_string_list (cd, mp->comment_dot, &context);
!         convert_msgid (cd, mp, &context);
!         convert_msgstr (cd, mp, &context);
        }
  
        iconv_close (cd);
*** gettext-tools/src/msgl-iconv.h      24 Aug 2003 13:58:15 -0000      1.2
--- gettext-tools/src/msgl-iconv.h      3 May 2005 10:41:18 -0000       1.3
***************
*** 1,5 ****
  /* Message list character set conversion.
!    Copyright (C) 2001-2003 Free Software Foundation, Inc.
     Written by Bruno Haible <address@hidden>, 2001.
  
     This program is free software; you can redistribute it and/or modify
--- 1,5 ----
  /* Message list character set conversion.
!    Copyright (C) 2001-2003, 2005 Free Software Foundation, Inc.
     Written by Bruno Haible <address@hidden>, 2001.
  
     This program is free software; you can redistribute it and/or modify
***************
*** 32,39 ****
  
  
  #if HAVE_ICONV
  /* Converts the STRING through the conversion descriptor CD.  */
! extern char *convert_string (iconv_t cd, const char *string);
  #endif
  
  /* Converts the message list MLP to the (already canonicalized) encoding
--- 32,50 ----
  
  
  #if HAVE_ICONV
+ 
+ /* A context, used for accurate error messages.  */
+ struct conversion_context
+ {
+   const char *from_code;     /* canonicalized encoding name for input */
+   const char *to_code;       /* canonicalized encoding name for output */
+   const char *from_filename; /* file name where the input comes from */
+ };
+ 
  /* Converts the STRING through the conversion descriptor CD.  */
! extern char *convert_string (iconv_t cd, const char *string,
!                            const struct conversion_context* context);
! 
  #endif
  
  /* Converts the message list MLP to the (already canonicalized) encoding
*** gettext-tools/src/xgettext.c        8 Feb 2005 11:26:41 -0000       1.44
--- gettext-tools/src/xgettext.c        3 May 2005 10:41:18 -0000       1.45
***************
*** 1647,1653 ****
     xgettext_global_source_encoding and thus also for
     xgettext_current_source_encoding are ASCII and UTF-8.
     convert_string() should not be called in this case.  */
! #define convert_string(cd,string) (abort (), (string))
  #endif
  
  /* Convert the given string from xgettext_current_source_encoding to
--- 1647,1653 ----
     xgettext_global_source_encoding and thus also for
     xgettext_current_source_encoding are ASCII and UTF-8.
     convert_string() should not be called in this case.  */
! #define convert_string(cd,string,context) (abort (), (string))
  #endif
  
  /* Convert the given string from xgettext_current_source_encoding to
***************
*** 1677,1683 ****
        }
      }
    else if (xgettext_current_source_encoding != po_charset_utf8)
!     string = convert_string (xgettext_current_source_iconv, string);
  
    return (char *) string;
  }
--- 1677,1691 ----
        }
      }
    else if (xgettext_current_source_encoding != po_charset_utf8)
!     {
!       struct conversion_context context;
! 
!       context.from_code = xgettext_current_source_encoding;
!       context.to_code = po_charset_utf8;
!       context.from_filename = file_name;
! 
!       string = convert_string (xgettext_current_source_iconv, string, 
&context);
!     }
  
    return (char *) string;
  }