diff mbox

Fix bootstrap when user language is not english

Message ID AM4PR0701MB216262DA4723290D4B5B00EEE4530@AM4PR0701MB2162.eurprd07.prod.outlook.com
State New
Headers show

Commit Message

Bernd Edlinger June 13, 2016, 2:41 p.m. UTC
Hi,

as noted in PR bootstrap/71481, comment#4 currently
the trunk fails to bootstrap if the current language is
not english.  A workaround is possible by setting LANG=C,
but OTOH it is rather easy to fix, by translating the string
in the assertion, as it is the only place that is affected by
the language setting.


Boot-strapped and reg-tested on trunk with LANG=de_DE.UTF-8.
OK to commit?


Thanks
Bernd.

Comments

David Malcolm June 13, 2016, 3:27 p.m. UTC | #1
On Mon, 2016-06-13 at 14:41 +0000, Bernd Edlinger wrote:
> Hi,
> 
> as noted in PR bootstrap/71481, comment#4 currently
> the trunk fails to bootstrap if the current language is
> not english.  A workaround is possible by setting LANG=C,
> but OTOH it is rather easy to fix, by translating the string
> in the assertion, as it is the only place that is affected by
> the language setting.
> 
> 
> Boot-strapped and reg-tested on trunk with LANG=de_DE.UTF-8.
> OK to commit?

Sorry about the breakage.

I believe I can approve this with my "libcpp"/"diagnostics" hats on, so
LGTM.

That said, should we hardcode LANG=C when running the selftests from
gcc/Makefile.in?


Dave
Bernd Edlinger June 13, 2016, 3:39 p.m. UTC | #2
On 06/13/16 17:27, David Malcolm wrote:
> On Mon, 2016-06-13 at 14:41 +0000, Bernd Edlinger wrote:

>> Hi,

>>

>> as noted in PR bootstrap/71481, comment#4 currently

>> the trunk fails to bootstrap if the current language is

>> not english.  A workaround is possible by setting LANG=C,

>> but OTOH it is rather easy to fix, by translating the string

>> in the assertion, as it is the only place that is affected by

>> the language setting.

>>

>>

>> Boot-strapped and reg-tested on trunk with LANG=de_DE.UTF-8.

>> OK to commit?

>

> Sorry about the breakage.

>

> I believe I can approve this with my "libcpp"/"diagnostics" hats on, so

> LGTM.

>


Thanks.

> That said, should we hardcode LANG=C when running the selftests from

> gcc/Makefile.in?

>


Honestly, I am glad to see that there is some sort of unit test which
runs in a different LANG setting than the rest of the testsuite.
Because as this incident clearly shows, there _can_ be bugs that do not
show up in the default locale.

I would put the question this way: could it be possible to run also
some tests in the testsuite with a LANG setting different from "C"?



Bernd.
Jakub Jelinek June 13, 2016, 3:50 p.m. UTC | #3
On Mon, Jun 13, 2016 at 03:39:21PM +0000, Bernd Edlinger wrote:
> On 06/13/16 17:27, David Malcolm wrote:
> > On Mon, 2016-06-13 at 14:41 +0000, Bernd Edlinger wrote:
> >> Hi,
> >>
> >> as noted in PR bootstrap/71481, comment#4 currently
> >> the trunk fails to bootstrap if the current language is
> >> not english.  A workaround is possible by setting LANG=C,
> >> but OTOH it is rather easy to fix, by translating the string
> >> in the assertion, as it is the only place that is affected by
> >> the language setting.
> >>
> >>
> >> Boot-strapped and reg-tested on trunk with LANG=de_DE.UTF-8.
> >> OK to commit?
> >
> > Sorry about the breakage.
> >
> > I believe I can approve this with my "libcpp"/"diagnostics" hats on, so
> > LGTM.
> >
> 
> Thanks.

Please put PR bootstrap/71481 into the ChangeLog entry though.

> > That said, should we hardcode LANG=C when running the selftests from
> > gcc/Makefile.in?
> >
> 
> Honestly, I am glad to see that there is some sort of unit test which
> runs in a different LANG setting than the rest of the testsuite.
> Because as this incident clearly shows, there _can_ be bugs that do not
> show up in the default locale.
> 
> I would put the question this way: could it be possible to run also
> some tests in the testsuite with a LANG setting different from "C"?

I think running the s-selftest in C locale is a good idea, but maybe
we should have some test in gcc.dg or where that would run -fself-tests
in some other locale.  I think right now we force LC_ALL=C for all tests,
but perhaps /* { dg-set-compiler-env-var LC_ALL "something" } */
would work.  But perhaps we'd need some tcl test for whether the locale is
supported by the system.

	Jakub
diff mbox

Patch

2016-06-13  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* input.c (test_builtins): Fix an assertion.

Index: gcc/input.c
===================================================================
--- gcc/input.c	(Revision 237379)
+++ gcc/input.c	(Arbeitskopie)
@@ -1210,7 +1210,7 @@  test_unknown_location ()
 static void
 test_builtins ()
 {
-  assert_loceq ("<built-in>", 0, 0, BUILTINS_LOCATION);
+  assert_loceq (_("<built-in>"), 0, 0, BUILTINS_LOCATION);
   ASSERT_PRED1 (is_location_from_builtin_token, BUILTINS_LOCATION);
 }