You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Peter Samuelson <pe...@p12n.org> on 2006/04/21 04:22:30 UTC

[PATCH] be more tolerant of locale errors

A Debian user complains [1] that subversion refuses to run when his
locale is broken.  (In the present instance it is apparently the fault
of something else in Debian, not his own fault.)  Now, I know why
Subversion insists on getting a valid locale - it is handling user
data, including filenames, and wants to be sure it is converting to and
from the correct character set.

  [1] http://bugs.debian.org/363893

But that did get me thinking: if that reasoning is correct, it only
applies to LC_CTYPE; if other locale variables (LC_MESSAGES, for
localised output messages, LC_COLLATE, for sort order) are unusable,
those should be mere warnings rather than errors.

Thus the subversion clients would be a little more robust against
problems with the user's environment.  Does anyone else think that this
would be a Good Thing?

NOTE: This patch is untested - I guess it's more for illustration than
to apply directly.  I'll test it Real Soon Now.

[[[
Make locale setting in the client a bit more forgiving.

* subversion/libsvn_subr/cmdline.c (svn_cmdline_init): Set LC_CTYPE
  separately from LC_ALL, and if LC_ALL fails, do not consider it to be
  a fatal error.
]]]

Index: subversion/libsvn_subr/cmdline.c
===================================================================
--- subversion/libsvn_subr/cmdline.c	(revisione 19429)
+++ subversion/libsvn_subr/cmdline.c	(copia locale)
@@ -55,6 +55,7 @@
 {
   apr_status_t status;
   apr_pool_t *pool;
+  svn_boolean_t lc_ctype_ok = FALSE;
 
 #ifndef WIN32
   {
@@ -104,6 +105,8 @@
   /* 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_CTYPE, ""))
+    lc_ctype_ok = TRUE;
   if (!setlocale(LC_ALL, ""))
     {
       if (error_stream)
@@ -130,8 +133,13 @@
                   "%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);
+          if (lc_ctype_ok)
+            fprintf(error_stream,
+                    "%s: LC_CTYPE was set successfully, continuing\n",
+                    progname);
         }
-      return EXIT_FAILURE;
+      if (!lc_ctype_ok)
+        return EXIT_FAILURE;
     }
 
   /* Initialize the APR subsystem, and register an atexit() function

Re: [PATCH] be more tolerant of locale errors

Posted by Peter Samuelson <pe...@p12n.org>.
>   [1] http://bugs.debian.org/363893

Sorry, typo, http://bugs.debian.org/363983

Re: [PATCH.1] be more tolerant of locale errors

Posted by Peter Samuelson <pe...@p12n.org>.
[Branko Cibej]
> Only if you write up a FAQ entry that we can refer those users to who
> complain that they've set the locale but Subversion doesn't print
> messages in their chosen language.

Subversion isn't the only Unix tool that will behave this way - all the
tools I know of except perl (perl warns you) will do exactly the same
thing - print unlocalised messages.  I don't see why it's specifically
subversion's responsibility to debug this.

I'll offer now, though: if this turns out to be a frequently asked
question, I'll write up an entry for it.  It won't be at all specific
to subversion, though.

Re: [PATCH.1] be more tolerant of locale errors

Posted by Branko Čibej <br...@xbc.nu>.
Peter Samuelson wrote:
> [Peter N. Lundblad]
>   
>> And in fact I'm wondering if this whole error out on locale error is
>> a false protection.  Nothing stops the user from running the client
>> with the wrong locale, which should be as likely (or more likely?) as
>> using a broken ditto.
>>     
>
> That's a good point.
>
>   
>> - Set LC_ALL category.
>> - If that fails:
>>   - Set LC_CTYPE category
>>   - If that fails, print a warning and continue.
>>     
>
> That sounds sensible too.
>
> [[[
> Make locale handling more forgiving in the clients.
>
> * subversion/libsvn_subr/cmdline.c (svn_cmdline_init): Try setting both
>   LC_ALL and LC_CTYPE locale variables, warning only if both fail.
>   Do not consider this a fatal error.
> ]]]
>
> Index: subversion/libsvn_subr/cmdline.c
> ===================================================================
> --- subversion/libsvn_subr/cmdline.c	(revisione 19429)
> +++ subversion/libsvn_subr/cmdline.c	(copia locale)
> @@ -104,7 +104,8 @@
>    /* 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(LC_ALL, "") &&
> +      !setlocale(LC_CTYPE, ""))
>   
Only if you write up a FAQ entry that we can refer those users to who 
complain that they've set the locale but Subversion doesn't print 
messages in their chosen language.

-- Brane



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

Re: [PATCH.1] be more tolerant of locale errors

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
Peter Samuelson writes:
 > 
 > [Peter N. Lundblad]
 > > And in fact I'm wondering if this whole error out on locale error is
 > > a false protection.  Nothing stops the user from running the client
 > > with the wrong locale, which should be as likely (or more likely?) as
 > > using a broken ditto.
 > 
 > That's a good point.
 > 
 > > - Set LC_ALL category.
 > > - If that fails:
 > >   - Set LC_CTYPE category
 > >   - If that fails, print a warning and continue.
 > 
 > That sounds sensible too.
 > 

Committed in r19445.

Thanks,
//Peter

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

[PATCH.1] be more tolerant of locale errors

Posted by Peter Samuelson <pe...@p12n.org>.
[Peter N. Lundblad]
> And in fact I'm wondering if this whole error out on locale error is
> a false protection.  Nothing stops the user from running the client
> with the wrong locale, which should be as likely (or more likely?) as
> using a broken ditto.

That's a good point.

> - Set LC_ALL category.
> - If that fails:
>   - Set LC_CTYPE category
>   - If that fails, print a warning and continue.

That sounds sensible too.

[[[
Make locale handling more forgiving in the clients.

* subversion/libsvn_subr/cmdline.c (svn_cmdline_init): Try setting both
  LC_ALL and LC_CTYPE locale variables, warning only if both fail.
  Do not consider this a fatal error.
]]]

Index: subversion/libsvn_subr/cmdline.c
===================================================================
--- subversion/libsvn_subr/cmdline.c	(revisione 19429)
+++ subversion/libsvn_subr/cmdline.c	(copia locale)
@@ -104,7 +104,8 @@
   /* 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(LC_ALL, "") &&
+      !setlocale(LC_CTYPE, ""))
     {
       if (error_stream)
         {
@@ -126,12 +127,11 @@
             }
 
           fprintf(error_stream,
-                  "%s: error: cannot set LC_ALL locale\n"
-                  "%s: error: environment variable %s is %s\n"
-                  "%s: error: please check that your locale name is correct\n",
+                  "%s: warning: cannot set LC_CTYPE locale\n"
+                  "%s: warning: environment variable %s is %s\n"
+                  "%s: warning: please check that your locale name is correct\n",
                   progname, progname, *env_var, env_val, progname);
         }
-      return EXIT_FAILURE;
     }
 
   /* Initialize the APR subsystem, and register an atexit() function

Re: [PATCH] be more tolerant of locale errors

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
Peter Samuelson writes:
 > 
 > But that did get me thinking: if that reasoning is correct, it only
 > applies to LC_CTYPE; if other locale variables (LC_MESSAGES, for
 > localised output messages, LC_COLLATE, for sort order) are unusable,
 > those should be mere warnings rather than errors.
 > 
 > Thus the subversion clients would be a little more robust against
 > problems with the user's environment.  Does anyone else think that this
 > would be a Good Thing?
 > 
I do.  And in fact I'm wondering if this whole error out on locale error
is a false protection.  Nothing stops the user from running the client with
the wrong locale, which should be as likely (or more likely?) as using a broken
ditto.  What about the following scheme:

- Set LC_ALL category.
- If that fails:
  - Set LC_CTYPE category
  - If that fails, print a warning and continue.

I think printing a warning if setting LC_ALL is just annoying and doesn't
do much good.  Warning if we can't set LC_CTYPE is good because it can explain
further strange behaviour.

Thoughts?

//Peter

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