Re: [PATCH] localedata: Avoid concurrently written locales in gen-locale.sh
Adhemerval Zanella Netto <[email protected]>
| Newsgroups | gmane.comp.lib.glibc.alpha |
|---|---|
| Organization | Linaro |
| Message-ID | <[email protected]> |
On 08/07/26 09:18, Sam James wrote: > Florian Weimer <[email protected]> writes: > >> There is no cross-directory exclusion of concurrent $(gen-locales) >> usage. Parallel localedef calls can clobber locale data as it is >> being loaded by tests. >> >> With --no-hard-links, the separate touch invocation is no longer >> required. >> >> --- >> localedata/gen-locale.sh | 22 +++++++++++++++------- >> 1 file changed, 15 insertions(+), 7 deletions(-) >> >> diff --git a/localedata/gen-locale.sh b/localedata/gen-locale.sh >> index 4762c04284..b0ce27fbae 100644 >> --- a/localedata/gen-locale.sh >> +++ b/localedata/gen-locale.sh >> @@ -32,15 +32,23 @@ generate_locale () >> out=$3 >> flags=$4 >> ret=0 >> + >> + # Use a staging area to avoid writing to locales concurrently. >> + # While this process is running, $$ is sufficiently unique. > > Yes, this should be OK. > >> + # Use --no-hard-links to prevent localedef from accessing >> + # other staging areas. >> + stage="${common_objpfx}localedata/gen-locale.$$.tmp" >> + >> ${localedef_before_env} ${run_program_env} I18NPATH=../localedata \ >> - ${localedef_after_env} $flags -f $charmap -i $input \ >> - ${common_objpfx}localedata/$out || ret=$? >> - if [ $ret -eq 0 ]; then >> - # The makefile checks the timestamp of the LC_CTYPE file, >> - # but localedef won't have touched it if it was able to >> - # hard-link it to an existing file. >> - touch ${common_objpfx}localedata/$out/LC_CTYPE >> + ${localedef_after_env} $flags --no-hard-links -f $charmap -i $input \ >> + $stage || ret=$? > > This could be improved to directly test the command but it can be done > later, no need to do it now. Agree, I think we should fix for the release and this approach seems ok. > >> + if [ $ret -eq 0 ] ; then >> + # Ignore errors in case some other process has created the same >> locale. > > OK. > >> + # (Some versions of mv do not report an error here.) >> + mv -Tn $stage ${common_objpfx}localedata/$out 2>/dev/null || true > > I'm sorry to ask this, but is mv -T sufficiently portable? It's not > POSIX but I don't recall if we require any extensions for coreutils > elsewhere in glibc. Hum I was not aware of that, maybe we can adjust it after the release if required. > >> + rm -rf $stage >> else >> + rm -rf $stage >> echo "Charmap: \"${charmap}\" Inputfile: \"${input}\"" \ >> "Outputdir: \"${out}\" failed" >> exit 1 > > The patch otherwise looks good, but please wait a brief period for > Adhemerval to comment before committing. Looks good to me. Just curious Florian, how did you notice this issue?