only #include <xlocale.h> when it is actually needed #18936

tonycoz · 2021-06-25T13:35:31Z

This header was originally only needed for builds on darwin, but
was being included whenever it was detected.

This has caused problems when what was an internal header was
removed (from glibc) and in general wasn't needed anyway.

cygwin also hint it away

Leont · 2021-06-25T14:07:54Z

Yeah, xlocale can make that glibc upgrade quite a pain, this sounds like a welcome improvement

jkeenan · 2021-06-25T14:53:56Z

This header was originally only needed for builds on darwin, but
was being included whenever it was detected.

This has caused problems when what was an internal header was
removed (from glibc) and in general wasn't needed anyway.

cygwin also hint it away

I tried out this branch on FreeBSD-12 with my usual configuration. I got many locale-related test failures.

$ cat xlocale-as-needed.failures.edited.txt
../cpan/version/t/07locale.t .. 
1..8
ok 1 - use version;
ok 2 - Not using locale yet
Failed 6/8 subtests 
../ext/POSIX/t/posix.t ........ 
1..96
ok 1 - O_RDONLY with open
ok 2 -     with read
not ok 3 -     read to array element # TODO read to array element not working
#   Failed (TODO) test '    read to array element'
#   at t/posix.t line 65.
#          got: undef
#     expected: 'perl
# '
ok 4
ok 5 - POSIX::pipe

[snip]

ok 49 - Can import autoloaded constants
Failed 47/96 subtests 
	(less 1 skipped subtest: 48 okay)
../lib/locale.t ............... 
ok 1 Verify locales_enabled('ALL') returns 0 or 1
ok 2    and locales_enabled('LC_ALL') returns the same value
ok 3 Verify locales_enabled('COLLATE') returns 0 or 1
ok 4    and locales_enabled('LC_COLLATE') returns the same value

[snip]

ok 380 verify that isn't tainted:  "foo.bar_baz" =~ /^(.*)[._](.*?)$/
ok 381 skipped: testing of locale 'ja_JP.SJIS' is skipped:
# Locale 'ja_JP.SJIS' may not work well.  Some characters in it are not recognized by Perl.
# The following characters (and maybe others) may not have the same meaning as the Perl program expects:
# \ ~
# ; codeset=SJIS
ok 382 skipped: testing of locale 'ja_JP.eucJP' is skipped:
# Locale 'ja_JP.eucJP' may not work well.  Some characters in it are not recognized by Perl.
# ; codeset=eucJP
ok 383 skipped: testing of locale 'ko_KR.CP949' is skipped:
# Locale 'ko_KR.CP949' may not work well.  Some characters in it are not recognized by Perl.
# ; codeset=eucKR
ok 384 skipped: testing of locale 'ko_KR.eucKR' is skipped:
# Locale 'ko_KR.eucKR' may not work well.  Some characters in it are not recognized by Perl.
# ; codeset=eucKR
ok 385 skipped: testing of locale 'zh_CN.GB18030' is skipped:
# Locale 'zh_CN.GB18030' may not work well.  Some characters in it are not recognized by Perl.
# ; codeset=GB18030
ok 386 skipped: testing of locale 'zh_CN.GB2312' is skipped:
# Locale 'zh_CN.GB2312' may not work well.  Some characters in it are not recognized by Perl.
# ; codeset=eucCN
ok 387 skipped: testing of locale 'zh_CN.GBK' is skipped:
# Locale 'zh_CN.GBK' may not work well.  Some characters in it are not recognized by Perl.
# ; codeset=GBK
ok 388 skipped: testing of locale 'zh_CN.eucCN' is skipped:
# Locale 'zh_CN.eucCN' may not work well.  Some characters in it are not recognized by Perl.
# ; codeset=eucCN
ok 389 skipped: testing of locale 'zh_TW.Big5' is skipped:
# Locale 'zh_TW.Big5' may not work well.  Some characters in it are not recognized by Perl.
# ; codeset=Big5
All 389 subtests passed 
../lib/locale_threads.t ....... 
ok 1 - Didn't segfault
All 1 subtests passed 
run/locale.t .................. 
# locales available: C C.UTF-8 POSIX af_ZA.ISO8859-1 af_ZA.ISO8859-15 af_ZA.UTF-8 am_ET.UTF-8 ar_AE.UTF-8 ar_EG.UTF-8 ar_JO.UTF-8 ar_MA.UTF-8 ar_QA.UTF-8 ar_SA.UTF-8 be_BY.CP1131 be_BY.CP1251 be_BY.ISO8859-5 be_BY.UTF-8 bg_BG.CP1251 bg_BG.UTF-8 ca_AD.ISO8859-1 ca_AD.ISO8859-15 ca_AD.UTF-8 ca_ES.ISO8859-1 ca_ES.ISO8859-15 ca_ES.UTF-8 ca_FR.ISO8859-1 ca_FR.ISO8859-15 ca_FR.UTF-8 ca_IT.ISO8859-1 ca_IT.ISO8859-15 ca_IT.UTF-8 cs_CZ.ISO8859-2 cs_CZ.UTF-8 da_DK.ISO8859-1 da_DK.ISO8859-15 da_DK.UTF-8 de_AT.ISO8859-1 de_AT.ISO8859-15 de_AT.UTF-8 de_CH.ISO8859-1 de_CH.ISO8859-15 de_CH.UTF-8 de_DE.ISO8859-1 de_DE.ISO8859-15 de_DE.UTF-8 el_GR.ISO8859-7 el_GR.UTF-8 en_AU.ISO8859-1 en_AU.ISO8859-15 en_AU.US-ASCII en_AU.UTF-8 en_CA.ISO8859-1 en_CA.ISO8859-15 en_CA.US-ASCII en_CA.UTF-8 en_GB.ISO8859-1 en_GB.ISO8859-15 en_GB.US-ASCII en_GB.UTF-8 en_HK.ISO8859-1 en_HK.UTF-8 en_IE.ISO8859-1 en_IE.ISO8859-15 en_IE.UTF-8 en_NZ.ISO8859-1 en_NZ.ISO8859-15 en_NZ.US-ASCII en_NZ.UTF-8 en_PH.UTF-8 en_SG.ISO8859-1 en_SG.UTF-8 en_US.ISO8859-1 en_US.ISO8859-15 en_US.US-ASCII en_US.UTF-8 en_ZA.ISO8859-1 en_ZA.ISO8859-15 en_ZA.US-ASCII en_ZA.UTF-8 es_AR.ISO8859-1 es_AR.UTF-8 es_CR.UTF-8 es_ES.ISO8859-1 es_ES.ISO8859-15 es_ES.UTF-8 es_MX.ISO8859-1 es_MX.UTF-8 et_EE.ISO8859-1 et_EE.ISO8859-15 et_EE.UTF-8 eu_ES.ISO8859-1 eu_ES.ISO8859-15 eu_ES.UTF-8 fi_FI.ISO8859-1 fi_FI.ISO8859-15 fi_FI.UTF-8 fr_BE.ISO8859-1 fr_BE.ISO8859-15 fr_BE.UTF-8 fr_CA.ISO8859-1 fr_CA.ISO8859-15 fr_CA.UTF-8 fr_CH.ISO8859-1 fr_CH.ISO8859-15 fr_CH.UTF-8 fr_FR.ISO8859-1 fr_FR.ISO8859-15 fr_FR.UTF-8 ga_IE.UTF-8 he_IL.UTF-8 hi_IN.ISCII-DEV hi_IN.UTF-8 hr_HR.ISO8859-2 hr_HR.UTF-8 hu_HU.ISO8859-2 hu_HU.UTF-8 hy_AM.ARMSCII-8 hy_AM.UTF-8 is_IS.ISO8859-1 is_IS.ISO8859-15 is_IS.UTF-8 it_CH.ISO8859-1 it_CH.ISO8859-15 it_CH.UTF-8 it_IT.ISO8859-1 it_IT.ISO8859-15 it_IT.UTF-8 ja_JP.UTF-8 kk_KZ.UTF-8 ko_KR.UTF-8 lt_LT.ISO8859-13 lt_LT.UTF-8 lv_LV.ISO8859-13 lv_LV.UTF-8 mn_MN.UTF-8 nb_NO.ISO8859-1 nb_NO.ISO8859-15 nb_NO.UTF-8 nl_BE.ISO8859-1 nl_BE.ISO8859-15 nl_BE.UTF-8 nl_NL.ISO8859-1 nl_NL.ISO8859-15 nl_NL.UTF-8 nn_NO.ISO8859-1 nn_NO.ISO8859-15 nn_NO.UTF-8 pl_PL.ISO8859-2 pl_PL.UTF-8 pt_BR.ISO8859-1 pt_BR.UTF-8 pt_PT.ISO8859-1 pt_PT.ISO8859-15 pt_PT.UTF-8 ro_RO.ISO8859-2 ro_RO.UTF-8 ru_RU.CP1251 ru_RU.CP866 ru_RU.ISO8859-5 ru_RU.KOI8-R ru_RU.UTF-8 se_FI.UTF-8 se_NO.UTF-8 sk_SK.ISO8859-2 sk_SK.UTF-8 sl_SI.ISO8859-2 sl_SI.UTF-8 sr_RS.ISO8859-2 sr_RS.ISO8859-5 sr_RS.UTF-8 sr_RS.UTF-8@latin sv_FI.ISO8859-1 sv_FI.ISO8859-15 sv_FI.UTF-8 sv_SE.ISO8859-1 sv_SE.ISO8859-15 sv_SE.UTF-8 tr_TR.ISO8859-9 tr_TR.UTF-8 uk_UA.CP1251 uk_UA.ISO8859-5 uk_UA.KOI8-U uk_UA.UTF-8 zh_CN.UTF-8 zh_HK.UTF-8 zh_TW.UTF-8
ok 1 - /il matching of [bracketed] doesn't skip POSIX class if fails individ char
ok 2 - /l matching of [bracketed] doesn't skip non-first POSIX class
# using non-C locale 'af_ZA.ISO8859-1'
ok 3 - retrieving current non-C LC_NUMERIC doesn't give 'C'
ok 4 - retrieving current non-C LC_ALL doesn't give 'C'
ok 5 - no locales where LC_NUMERIC breaks
ok 6 - LC_NUMERIC without environment nor setlocale() has no effect in any locale
All 6 subtests passed 

Test Summary Report
-------------------
../cpan/version/t/07locale.t (Wstat: 139 Tests: 2 Failed: 0)
  Non-zero wait status: 139
  Parse errors: Bad plan.  You planned 8 tests but ran 2.
../ext/POSIX/t/posix.t      (Wstat: 139 Tests: 49 Failed: 0)
  Non-zero wait status: 139
  Parse errors: Bad plan.  You planned 96 tests but ran 49.
../lib/locale.t             (Wstat: 139 Tests: 389 Failed: 0)
  Non-zero wait status: 139
  Parse errors: No plan found in TAP output
../lib/locale_threads.t     (Wstat: 139 Tests: 1 Failed: 0)
  Non-zero wait status: 139
  Parse errors: No plan found in TAP output
run/locale.t                (Wstat: 139 Tests: 6 Failed: 0)
  Non-zero wait status: 139
  Parse errors: No plan found in TAP output
Files=5, Tests=447,  4 wallclock secs ( 0.02 usr  0.01 sys +  0.48 cusr  0.25 csys =  0.77 CPU)
Result: FAIL

[perlmonger: perl] $ gitcurr
xlocale-as-needed
[perlmonger: perl] $ extract-sha
674a97f27f
$ ./perl -Ilib -V:config_args
config_args='-des -Dusedevel -Duseithreads -Doptimize=-O2 -pipe -fstack-protector -fno-strict-aliasing';
$ uname -mrs
FreeBSD 12.2-STABLE amd64

jkeenan · 2021-06-25T15:24:01Z

I tried out this branch on FreeBSD-12 with my usual configuration. I got many locale-related test failures.

Segmentation fault (core dumped)

... in each of the 4 failing test files.

tonycoz · 2021-06-25T23:39:34Z

What did you see in config.sh for i_xlocale and xlocale_needed?

jkeenan · 2021-06-26T01:06:58Z

What did you see in config.sh for i_xlocale and xlocale_needed?

$ grep -n -E '(i_xlocale|xlocale_needed)' ./config.sh
847:i_xlocale='define'
1221:xlocale_needed='undef'

tonycoz · 2021-06-28T02:19:24Z

I tried out this branch on FreeBSD-12 with my usual configuration. I got many locale-related test failures.

Thanks, I've reproduced this and have some idea of what's going on.

jkeenan · 2021-06-28T14:59:47Z

I tried out this branch on FreeBSD-12 with my usual configuration. I got many locale-related test failures.

Thanks, I've reproduced this and have some idea of what's going on.

I tested your branch at v5.35.1-31-ged003c1a0d and all tests PASSED. The specifics of your changes to the C code are above my pay grade, so I hope other people will comment on them.

I recommend you create a smoke-me branch out of this so that we can get results from more platforms.

Thank you very much.
Jim Keenan

This header was originally only needed for builds on darwin and FreeBSD, but was being included whenever it was detected. This has caused problems when what was an internal header was removed (from glibc) and in general wasn't needed anyway. On FreeBSD only localeconv_l() requires xlocale.h, so we test specifically for that.

khwilliamson

LGTM; needs to be sent to metaconfig

jkeenan · 2021-07-02T13:04:58Z

I tested your branch at v5.35.1-31-ged003c1a0d and all tests PASSED. The specifics of your changes to the C code are above my pay grade, so I hope other people will comment on them.

I recommend you create a smoke-me branch out of this so that we can get results from more platforms.

Results on the smoke-me branch are good. No failures attributable to these changes.

Tux · 2021-08-10T12:56:32Z

If you guys are satified, just merge it, I'll backport

jkeenan · 2021-08-11T00:09:39Z

If you guys are satified, just merge it, I'll backport

@tonycoz, can you do the honors on this?

Tux · 2021-08-11T11:54:02Z

Backported to metaconfig (Perl/metaconfig@c7bd0d2855d404) and then regenerated back to CORE
Thanks for the work

tonycoz requested review from khwilliamson and Tux June 25, 2021 13:35

tonycoz force-pushed the xlocale-as-needed branch from ed003c1 to fcaf0ac Compare June 29, 2021 00:27

khwilliamson approved these changes Jun 29, 2021

View reviewed changes

tonycoz merged commit 68f1b6d into Perl:blead Aug 11, 2021

plicease mentioned this pull request Jun 18, 2022

Build fails on Ubuntu and Perl 5.30 PerlAlien/Alien-Build#307

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

only #include <xlocale.h> when it is actually needed #18936

only #include <xlocale.h> when it is actually needed #18936

Uh oh!

tonycoz commented Jun 25, 2021

Leont commented Jun 25, 2021

jkeenan commented Jun 25, 2021

jkeenan commented Jun 25, 2021

tonycoz commented Jun 25, 2021

jkeenan commented Jun 26, 2021

tonycoz commented Jun 28, 2021

jkeenan commented Jun 28, 2021

khwilliamson left a comment

jkeenan commented Jul 2, 2021

Tux commented Aug 10, 2021

jkeenan commented Aug 11, 2021

Tux commented Aug 11, 2021

only #include <xlocale.h> when it is actually needed #18936

only #include <xlocale.h> when it is actually needed #18936

Uh oh!

Conversation

tonycoz commented Jun 25, 2021

Leont commented Jun 25, 2021

jkeenan commented Jun 25, 2021

jkeenan commented Jun 25, 2021

tonycoz commented Jun 25, 2021

jkeenan commented Jun 26, 2021

tonycoz commented Jun 28, 2021

jkeenan commented Jun 28, 2021

khwilliamson left a comment

Choose a reason for hiding this comment

jkeenan commented Jul 2, 2021

Tux commented Aug 10, 2021

jkeenan commented Aug 11, 2021

Tux commented Aug 11, 2021