You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Erik Huelsmann <e....@gmx.net> on 2005/02/27 16:30:49 UTC

[PATCH] Fix svnserve's gettext use on platforms without LC_MESSAGES


The current svnserve implementation will send translated error messages over
the wire on systems which don't have LC_MESSAGES defined. The patch below
fixes that.

One minor nit: I think the new 'server_mode' parameter should have been
named 'minimal_locale'.

Comments?

bye,

Erik.

Log:
[[[
Fix svnserve error message translation by postponing locale initialization.
 
* subversion/include/svn_cmdline.h,
  subversion/libsvn_subr/cmdline.c (svn_cmdline_init): Deprecated.
  (svn_cmdline_init2): New. Derived from svn_cmdline_init, but takes
   an extra 'server_mode' boolean parameter.
 
* subversion/clients/cmdline/main.c,
  subversion/svnadmin/main.c,
  subversion/svndumpfilter/main.c,
  subversion/svnlook/main.c: Update calls to svn_cmdline_init2.
 
* subversion/svnserve/main.c: Set LC_ALL locale before calling _() (and
  thus gettext()) to ensure getting the correct translation. Also,
  don't translate messages which may be sent to the client.

]]]


Index: subversion/svnadmin/main.c
===================================================================
--- subversion/svnadmin/main.c  (revision 13176)
+++ subversion/svnadmin/main.c  (working copy)
@@ -987,7 +987,7 @@
   int i, num_opts = 0;
  
   /* Initialize the app. */
-  if (svn_cmdline_init ("svnadmin", stderr) != EXIT_SUCCESS)
+  if (svn_cmdline_init2 ("svnadmin", stderr, FALSE) != EXIT_SUCCESS)
     return EXIT_FAILURE;
  
   /* Create our top-level pool.  Use a seperate mutexless allocator,
Index: subversion/include/svn_cmdline.h
===================================================================
--- subversion/include/svn_cmdline.h    (revision 13176)
+++ subversion/include/svn_cmdline.h    (working copy)
@@ -37,14 +37,31 @@
 #endif /* __cplusplus */
  
  
-/** Set up the locale for character conversion, and initialize APR.
+/**
+ *
+ * @since New in 1.2
+ *
+ * Set up the locale and initialize APR and gettext.
+ * Only sets up LC_CTYPE when @a server_mode is true, LC_ALL otherwise.
  * If @a error_stream is non-null, print error messages to the stream,
  * using @a progname as the program name. Return @c EXIT_SUCCESS if
  * successful, otherwise @c EXIT_FAILURE.
  *
  * @note This function should be called exactly once at program startup,
  *       before calling any other APR or Subversion functions.
+ *
  */
+int svn_cmdline_init2 (const char *progname, FILE *error_stream,
+                       svn_boolean_t server_mode);
+
+
+/**
+ *
+ * @deprecated Provided for backward compatibility with 1.1 API
+ *
+ * Same as svn_cmdline_init2, but always sets up LC_ALL
+ *
+ */
 int svn_cmdline_init (const char *progname, FILE *error_stream);
  
  
Index: subversion/libsvn_subr/cmdline.c
===================================================================
--- subversion/libsvn_subr/cmdline.c    (revision 13176)
+++ subversion/libsvn_subr/cmdline.c    (working copy)
@@ -58,7 +58,8 @@
  
  
 int
-svn_cmdline_init (const char *progname, FILE *error_stream)
+svn_cmdline_init2 (const char *progname, FILE *error_stream,
+                   svn_boolean_t server_mode)
 {
   apr_status_t status;
   apr_pool_t *pool;
@@ -102,12 +103,13 @@
   /* C programs default to the "C" locale. But because svn is supposed
      to be i18n-aware, it should inherit the default locale of its
      environment.  */
-  if (!setlocale(LC_ALL, ""))
+  if (!setlocale(server_mode ? LC_CTYPE : LC_ALL, ""))
     {
       if (error_stream)
         {
           const char *env_vars[] = { "LC_ALL", "LC_CTYPE", "LANG", NULL };
-          const char **env_var = &env_vars[0], *env_val = NULL;
+          const char **env_var = &env_vars[server_mode ? 1 : 0];
+          const char *env_val = NULL;
           while (*env_var)
             {
               env_val = getenv(*env_var);
@@ -124,10 +126,11 @@
             }
  
           fprintf(error_stream,
-                  "%s: error: cannot set LC_ALL locale\n"
+                  "%s: error: cannot set %s locale\n"
                   "%s: error: environment variable %s is %s\n"
                   "%s: error: please check that your locale name is
correct\n",-                  progname, progname, *env_var, env_val,
progname);
+                  progname, server_mode ? "LC_CTYPE" : "LC_ALL",
+                  progname, *env_var, env_val, progname);
         }
       return EXIT_FAILURE;
     }
@@ -228,6 +231,13 @@
 }
  
  
+int
+svn_cmdline_init (const char *progname, FILE *error_stream)
+{
+  return svn_cmdline_init2 (progname, error_stream, FALSE);
+}
+
+
 svn_error_t *
 svn_cmdline_cstring_from_utf8 (const char **dest,
                                const char *src,
Index: subversion/svnlook/main.c
===================================================================
--- subversion/svnlook/main.c   (revision 13176)
+++ subversion/svnlook/main.c   (working copy)
@@ -1854,7 +1854,7 @@
   int i, num_opts = 0;
  
   /* Initialize the app. */
-  if (svn_cmdline_init ("svnlook", stderr) != EXIT_SUCCESS)
+  if (svn_cmdline_init2 ("svnlook", stderr, FALSE) != EXIT_SUCCESS)
     return EXIT_FAILURE;
  
   /* Create our top-level pool.  Use a seperate mutexless allocator,
Index: subversion/clients/cmdline/main.c
===================================================================
--- subversion/clients/cmdline/main.c   (revision 13176)
+++ subversion/clients/cmdline/main.c   (working copy)
@@ -780,7 +780,7 @@
   svn_config_t *cfg;
    
   /* Initialize the app. */
-  if (svn_cmdline_init ("svn", stderr) != EXIT_SUCCESS)
+  if (svn_cmdline_init2 ("svn", stderr, FALSE) != EXIT_SUCCESS)
     return EXIT_FAILURE;
  
   /* Create our top-level pool.  Use a seperate mutexless allocator,
Index: subversion/svndumpfilter/main.c
===================================================================
--- subversion/svndumpfilter/main.c     (revision 13176)
+++ subversion/svndumpfilter/main.c     (working copy)
@@ -1061,7 +1061,7 @@
  
  
   /* Initialize the app. */
-  if (svn_cmdline_init ("svndumpfilter", stderr) != EXIT_SUCCESS)
+  if (svn_cmdline_init2 ("svndumpfilter", stderr, FALSE) != EXIT_SUCCESS)
     return EXIT_FAILURE;
  
   /* Create our top-level pool.  Use a seperate mutexless allocator,
Index: subversion/svnserve/main.c
===================================================================
--- subversion/svnserve/main.c  (revision 13176)
+++ subversion/svnserve/main.c  (working copy)
@@ -128,9 +128,10 @@
   if (!progname)
     progname = "svnserve";
  
-  svn_error_clear (svn_cmdline_fprintf(stderr, pool,
-                                       _("Type '%s --help' for usage.\n"),
-                                       progname));
+  setlocale (LC_ALL, "");
+  svn_error_clear (svn_cmdline_fprintf (stderr, pool,
+                                        _("Type '%s --help' for usage.\n"),
+                                        progname));
   exit(1);
 }
  
@@ -138,6 +139,7 @@
 {
   apr_size_t i;
  
+  setlocale (LC_ALL, "");
   svn_error_clear (svn_cmdline_fputs(_("Usage: svnserve [options]\n"
                                        "\n"
                                        "Valid options:\n"),
@@ -154,6 +156,7 @@
  
 static svn_error_t * version(apr_getopt_t *os, apr_pool_t *pool)
 {
+  setlocale (LC_ALL, "");
   return svn_opt_print_help(os, "svnserve", TRUE, FALSE, NULL, NULL,
                             NULL, NULL, NULL, pool);
 }
@@ -248,7 +251,7 @@
   const char *host = NULL;
  
   /* Initialize the app. */
-  if (svn_cmdline_init("svn", stderr) != EXIT_SUCCESS)
+  if (svn_cmdline_init2("svn", stderr, TRUE) != EXIT_SUCCESS)
     return EXIT_FAILURE;
  
   /* Create our top-level pool. */
@@ -258,6 +261,7 @@
   err = check_lib_versions ();
   if (err)
     {
+      setlocale (LC_ALL, "");
       svn_handle_error (err, stderr, FALSE);
       svn_error_clear (err);
       svn_pool_destroy (pool);
@@ -328,16 +332,18 @@
  
         case 'R':
           params.read_only = TRUE;
+          /* Don't translate the warning below: we may stay in
+             server mode (and thus don't want to translate messages) */
           svn_error_clear
             (svn_cmdline_fprintf
                (stderr, pool,
-                _("Warning: -R is deprecated.\n"
-                  "Anonymous access is now read-only by default.\n"
-                  "To change, use conf/svnserve.conf in repos:\n"
-                  "  [general]\n"
-                  "  anon-access = read|write|none (default read)\n"
-                  "  auth-access = read|write|none (default write)\n"
-                  "Forcing all access to read-only for now\n")));
+                "Warning: -R is deprecated.\n"
+                "Anonymous access is now read-only by default.\n"
+                "To change, use conf/svnserve.conf in repos:\n"
+                "  [general]\n"
+                "  anon-access = read|write|none (default read)\n"
+                "  auth-access = read|write|none (default write)\n"
+                "Forcing all access to read-only for now\n"));
           break;
  
         case 'T':
@@ -350,6 +356,7 @@
  
   if (params.tunnel_user && run_mode != run_mode_tunnel)
     {
+      setlocale (LC_ALL, "");
       svn_error_clear
         (svn_cmdline_fprintf
            (stderr, pool,
@@ -359,9 +366,10 @@
  
   if (run_mode == run_mode_none)
     {
-      svn_error_clear
-        (svn_cmdline_fprintf
-           (stderr, pool, _("You must specify one of -d, -i, -t or
-X.\n")));
+      setlocale (LC_ALL, "");
+      svn_error_clear (svn_cmdline_fputs
+                       (_("You must specify one of -d, -i, -t or -X.\n"),
+                        stderr, pool));
       usage(argv[0], pool);
     }
  
@@ -386,10 +394,8 @@
 #endif
   if (status)
     {
-      svn_error_clear
-        (svn_cmdline_fprintf
-           (stderr, pool, _("Can't create server socket: %s\n"),
-            apr_strerror(status, errbuf, sizeof(errbuf))));
+      fprintf (stderr, "Can't create server socket: %s\n",
+               apr_strerror(status, errbuf, sizeof(errbuf)));
       exit(1);
     }
  
@@ -400,33 +406,21 @@
   status = apr_sockaddr_info_get(&sa, host, APR_INET, port, 0, pool);
   if (status)
     {
-      svn_error_clear
-        (svn_cmdline_fprintf
-           (stderr, pool, _("Can't get address info: %s\n"),
-            apr_strerror(status, errbuf, sizeof(errbuf))));
+      fprintf (stderr, "Can't get address info: %s\n",
+               apr_strerror(status, errbuf, sizeof(errbuf)));
       exit(1);
     }
  
   status = apr_socket_bind(sock, sa);
   if (status)
     {
-      svn_error_clear
-        (svn_cmdline_fprintf
-           (stderr, pool, _("Can't bind server socket: %s\n"),
-            apr_strerror(status, errbuf, sizeof(errbuf))));
+      fprintf (stderr, "Can't bind server socket: %s\n",
+               apr_strerror(status, errbuf, sizeof(errbuf)));
       exit(1);
     }
  
   apr_socket_listen(sock, 7);
  
-  /* svn_cmdline_init() sets up the locale, but when we serve clients, we
-     always want the "C" locale for messages. */
-  /* ### LC_MESSAGES isn't available on all platforms. TEMPORARILY disable
-     this call on those platforms. ### */
-#ifdef LC_MESSAGES
-  setlocale (LC_MESSAGES, "C");
-#endif
-
 #if APR_HAS_FORK
   if (run_mode != run_mode_listen_once && !foreground)
     apr_proc_detach(APR_PROC_DETACH_DAEMONIZE);

-- 
Lassen Sie Ihren Gedanken freien Lauf... z.B. per FreeSMS
GMX bietet bis zu 100 FreeSMS/Monat: http://www.gmx.net/de/go/mail

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Fix svnserve's gettext use on platforms without LC_MESSAGES

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Sun, 27 Feb 2005, Erik Huelsmann wrote:

>
> --- subversion/include/svn_cmdline.h    (revision 13176)
> +++ subversion/include/svn_cmdline.h    (working copy)
> @@ -37,14 +37,31 @@
>  #endif /* __cplusplus */
>
>
> -/** Set up the locale for character conversion, and initialize APR.
> +/**
> + *
> + * @since New in 1.2
> + *
> + * Set up the locale and initialize APR and gettext.
> + * Only sets up LC_CTYPE when @a server_mode is true, LC_ALL otherwise.
s/sets/set/

>   * If @a error_stream is non-null, print error messages to the stream,
>   * using @a progname as the program name. Return @c EXIT_SUCCESS if
>   * successful, otherwise @c EXIT_FAILURE.
>   *
>   * @note This function should be called exactly once at program startup,
>   *       before calling any other APR or Subversion functions.
> + *
>   */
> +int svn_cmdline_init2 (const char *progname, FILE *error_stream,
> +                       svn_boolean_t server_mode);
> +
> +
> +/**
> + *
> + * @deprecated Provided for backward compatibility with 1.1 API
> + *
"Provided for backwards compatibility with the 1.1 API."
(I think! I don't know how many times I've written tha phrase lately;(

> + * Same as svn_cmdline_init2, but always sets up LC_ALL

Missing trailing period. YOu could describe it in terms of the argument
instead of behaviour, like "...with @a server_mode set to @c FALSE."
> + *

Regards,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Fix svnserve's gettext use on platforms without LC_MESSAGES

Posted by Erik Huelsmann <e....@gmx.net>.
> It's good to have this problem addressed, but I have some concerns with
> this patch.  Maybe my concerns result from lack of understanding.
> 
> I see that a slightly modified version of this patch has already been
> committed.

Right, but Karl and Philip Martin expressed some concerns too with questions
I was asking or the code itself, so it's never too late. And better late
then never :-)

Besides, I was quick at committing.


> 
> Erik Huelsmann wrote:
> > Log:
> > [[[
> > Fix svnserve error message translation by postponing locale
> initialization.
> 
> Fixes it?  Do you mean it correctly translates all messages according to
> the 
> locale of the display on which they are printed?  Perhaps the log message
> could 
> be clearer.

Re: [PATCH] Fix svnserve's gettext use on platforms without LC_MESSAGES

Posted by Julian Foad <ju...@btopenworld.com>.
It's good to have this problem addressed, but I have some concerns with this 
patch.  Maybe my concerns result from lack of understanding.

I see that a slightly modified version of this patch has already been committed.


Erik Huelsmann wrote:
> Log:
> [[[
> Fix svnserve error message translation by postponing locale initialization.

Fixes it?  Do you mean it correctly translates all messages according to the 
locale of the display on which they are printed?  Perhaps the log message could 
be clearer.

> * subversion/include/svn_cmdline.h,
>   subversion/libsvn_subr/cmdline.c (svn_cmdline_init): Deprecated.
>   (svn_cmdline_init2): New. Derived from svn_cmdline_init, but takes
>    an extra 'server_mode' boolean parameter.
[...]
> * subversion/svnserve/main.c: Set LC_ALL locale before calling _() (and
>   thus gettext()) to ensure getting the correct translation. Also,
>   don't translate messages which may be sent to the client.

I assume that the reason for not translating some messages is because in some 
of its operating modes there is no way (yet) for this server to know the 
client's locale, and thus the messages should be sent in their original English 
form.

> 
> ]]]

> Index: subversion/include/svn_cmdline.h
> ===================================================================
> --- subversion/include/svn_cmdline.h    (revision 13176)
> +++ subversion/include/svn_cmdline.h    (working copy)
> @@ -37,14 +37,31 @@
>  #endif /* __cplusplus */
>   
>   
> -/** Set up the locale for character conversion, and initialize APR.
> +/**
> + *
> + * @since New in 1.2
> + *
> + * Set up the locale and initialize APR and gettext.
> + * Only sets up LC_CTYPE when @a server_mode is true, LC_ALL otherwise.

Why does a server process want to run with the LC_CTYPE of its environment, but 
not any other locale categories?  I'm not saying it's wrong, but I can't think 
of the reason.

>   * If @a error_stream is non-null, print error messages to the stream,
>   * using @a progname as the program name. Return @c EXIT_SUCCESS if
>   * successful, otherwise @c EXIT_FAILURE.
>   *
>   * @note This function should be called exactly once at program startup,
>   *       before calling any other APR or Subversion functions.
> + *
>   */
> +int svn_cmdline_init2 (const char *progname, FILE *error_stream,
> +                       svn_boolean_t server_mode);
[...]
> Index: subversion/svnserve/main.c
> ===================================================================
> --- subversion/svnserve/main.c  (revision 13176)
> +++ subversion/svnserve/main.c  (working copy)
> @@ -258,6 +261,7 @@
>    err = check_lib_versions ();
>    if (err)
>      {
> +      setlocale (LC_ALL, "");
>        svn_handle_error (err, stderr, FALSE);
>        svn_error_clear (err);
>        svn_pool_destroy (pool);
> @@ -328,16 +332,18 @@
>   
>          case 'R':
>            params.read_only = TRUE;
> +          /* Don't translate the warning below: we may stay in
> +             server mode (and thus don't want to translate messages) */

That doesn't make sense.  Surely all warnings that go to stderr/stdout should 
be translated according the locale of the local environment.

Are you doing this just because, if you turn translation on, you don't have a 
way of turning it off again?

>            svn_error_clear
>              (svn_cmdline_fprintf
>                 (stderr, pool,
> -                _("Warning: -R is deprecated.\n"
> -                  "Anonymous access is now read-only by default.\n"
> -                  "To change, use conf/svnserve.conf in repos:\n"
> -                  "  [general]\n"
> -                  "  anon-access = read|write|none (default read)\n"
> -                  "  auth-access = read|write|none (default write)\n"
> -                  "Forcing all access to read-only for now\n")));
> +                "Warning: -R is deprecated.\n"
> +                "Anonymous access is now read-only by default.\n"
> +                "To change, use conf/svnserve.conf in repos:\n"
> +                "  [general]\n"
> +                "  anon-access = read|write|none (default read)\n"
> +                "  auth-access = read|write|none (default write)\n"
> +                "Forcing all access to read-only for now\n"));
>            break;
>   
>          case 'T':
> @@ -350,6 +356,7 @@
>   
>    if (params.tunnel_user && run_mode != run_mode_tunnel)
>      {
> +      setlocale (LC_ALL, "");

So you explicitly turn translation on before printing _some_ error messages...

>        svn_error_clear
>          (svn_cmdline_fprintf
>             (stderr, pool,

(The message here has no replaceable parameters, so _fputs would do, but that's 
irrelevant.)

> @@ -359,9 +366,10 @@
>   
>    if (run_mode == run_mode_none)
>      {
> -      svn_error_clear
> -        (svn_cmdline_fprintf
> -           (stderr, pool, _("You must specify one of -d, -i, -t or -X.\n")));
> +      setlocale (LC_ALL, "");
> +      svn_error_clear (svn_cmdline_fputs
> +                       (_("You must specify one of -d, -i, -t or -X.\n"),
> +                        stderr, pool));

(Is there any particular reason why you changed this from _fprintf to _fputs, 
but didn't change the previous one?)

>        usage(argv[0], pool);
>      }
>   
> @@ -386,10 +394,8 @@
>  #endif
>    if (status)
>      {
> -      svn_error_clear
> -        (svn_cmdline_fprintf
> -           (stderr, pool, _("Can't create server socket: %s\n"),
> -            apr_strerror(status, errbuf, sizeof(errbuf))));
> +      fprintf (stderr, "Can't create server socket: %s\n",
> +               apr_strerror(status, errbuf, sizeof(errbuf)));

... but why don't you want this message (or the next two) translated?  These 
aren't messages that will be sent to the client, are they?

Also, why don't you want them converted to the native character encoding?

>        exit(1);
>      }
>   
> @@ -400,33 +406,21 @@
>    status = apr_sockaddr_info_get(&sa, host, APR_INET, port, 0, pool);
>    if (status)
>      {
> -      svn_error_clear
> -        (svn_cmdline_fprintf
> -           (stderr, pool, _("Can't get address info: %s\n"),
> -            apr_strerror(status, errbuf, sizeof(errbuf))));
> +      fprintf (stderr, "Can't get address info: %s\n",
> +               apr_strerror(status, errbuf, sizeof(errbuf)));
>        exit(1);
>      }
>   
>    status = apr_socket_bind(sock, sa);
>    if (status)
>      {
> -      svn_error_clear
> -        (svn_cmdline_fprintf
> -           (stderr, pool, _("Can't bind server socket: %s\n"),
> -            apr_strerror(status, errbuf, sizeof(errbuf))));
> +      fprintf (stderr, "Can't bind server socket: %s\n",
> +               apr_strerror(status, errbuf, sizeof(errbuf)));


Wouldn't the best thing be to leave the server running in its own locale, like 
it was, and just avoid translating (i.e. calling gettext on) any messages that 
are transmitted from server to client?  (They can still be marked for 
translation, with N_(), and the translations can be used when the client's 
locale is known.)

Apologies if I've got completely the wrong idea about this.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org