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
Received on Mon Feb 28 20:52:20 2005