From mboxrd@z Thu Jan 1 00:00:00 1970 From: ludo@gnu.org (Ludovic =?utf-8?Q?Court=C3=A8s?=) Subject: Re: [PATCH 0/2] Avoiding incompatible locale data in LOCPATH Date: Fri, 02 Oct 2015 18:52:54 +0200 Message-ID: <87eghdcivt.fsf@gnu.org> References: <1443736716-8578-1-git-send-email-ludo@gnu.org> <87lhbmqglz.fsf@netris.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:38413) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zi3a0-00078l-GG for guix-devel@gnu.org; Fri, 02 Oct 2015 12:53:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zi3Zy-0008Ay-BN for guix-devel@gnu.org; Fri, 02 Oct 2015 12:53:08 -0400 In-Reply-To: <87lhbmqglz.fsf@netris.org> (Mark H. Weaver's message of "Thu, 01 Oct 2015 20:06:00 -0400") List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org To: Mark H Weaver Cc: guix-devel@gnu.org, beffa@ieee.org --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Mark H Weaver skribis: > For compatibility reasons, I think we should add "/2.22" _only_ to the > entries of GUIX_LOCPATH. IMO, it's unwise to change the meaning of > LOCPATH. Some programs, build systems, or user scripts might use > "localedef" in a directory, set LOCPATH to that directory, and expect it > to work. Yes, makes sense. I noticed that the recipe for =E2=80=98tre=E2=80=99 reli= es on =E2=80=98LOCPATH=E2=80=99, and I agree that it=E2=80=99s best to keep =E2= =80=98LOCPATH=E2=80=99 semantics. So, as discussed on IRC, here=E2=80=99s a second patch that: =E2=80=A2 introduces =E2=80=98GUIX_LOCPATH=E2=80=99, which is like =E2=80= =98LOCPATH=E2=80=99, except versioned; =E2=80=A2 keeps =E2=80=98LOCPATH=E2=80=99, without appending =E2=80=9C/X.= Y=E2=80=9D to its entries, and giving it less precedence than =E2=80=98GUIX_LOCPATH=E2=80=99; We would tell people to migrate away from =E2=80=98LOCPATH=E2=80=99 in favo= r of =E2=80=98GUIX_LOCPATH=E2=80=99. I think we should use this patch as a starting point for a discussion for the libc people. I=E2=80=99d argue that would be acceptable even for upstream (with s/GUIX_LOCPATH/LOCALE_PATH/ for instance.) Please review carefully. Ideally I=E2=80=99d like to push this patch or a variant thereof tomorrow, so we can start the full rebuild. TIA! Ludo=E2=80=99. --=-=-= Content-Type: text/x-patch; charset=utf-8 Content-Disposition: inline; filename=0001-gnu-glibc-Look-for-locale-data-in-versioned-sub-dire.patch Content-Transfer-Encoding: quoted-printable Content-Description: the patch >From 7258d8c6e2a464f911723cabc2e35f7e05c49192 Mon Sep 17 00:00:00 2001 From: =3D?UTF-8?q?Ludovic=3D20Court=3DC3=3DA8s?=3D Date: Thu, 1 Oct 2015 21:32:50 +0200 Subject: [PATCH] gnu: glibc: Look for locale data in versioned sub-directories. * gnu/packages/patches/glibc-versioned-locpath.patch: New file. * gnu-system.am (dist_patch_DATA): Add it. * gnu/packages/base.scm (glibc)[source]: Use it. [arguments]: Add explicit version sub-directory to libc_cv_localedir. [native-search-paths]: Use 'GUIX_LOCPATH' instead of 'LOCPATH'. (glibc-locales, glibc-utf8-locales): Write to a VERSION sub-directory. --- doc/guix.texi | 23 +- gnu-system.am | 1 + gnu/packages/base.scm | 17 +- gnu/packages/patches/glibc-versioned-locpath.patch | 240 +++++++++++++++++= ++++ 4 files changed, 268 insertions(+), 13 deletions(-) create mode 100644 gnu/packages/patches/glibc-versioned-locpath.patch diff --git a/doc/guix.texi b/doc/guix.texi index 68ee451..1ad4229 100644 --- a/doc/guix.texi +++ b/doc/guix.texi @@ -933,24 +933,24 @@ daemons on the same machine. @node Application Setup @section Application Setup =20 -When using Guix on top of GNU/Linux distribution other than GuixSD, a -few additional steps are needed to get everything in place. Here are -some of them. +When using Guix on top of GNU/Linux distribution other than GuixSD---a +so-called @dfn{foreign distro}---a few additional steps are needed to +get everything in place. Here are some of them. =20 @subsection Locales =20 @anchor{locales-and-locpath} @cindex locales, when not on GuixSD @vindex LOCPATH +@vindex GUIX_LOCPATH Packages installed @i{via} Guix will not use the host system's locale data. Instead, you must first install one of the locale packages -available with Guix and then define the @code{LOCPATH} environment -variable (@pxref{Locale Names, @code{LOCPATH},, libc, The GNU C Library -Reference Manual}): +available with Guix and then define the @code{GUIX_LOCPATH} environment +variable: =20 @example $ guix package -i glibc-locales -$ export LOCPATH=3D$HOME/.guix-profile/lib/locale +$ export GUIX_LOCPATH=3D$HOME/.guix-profile/lib/locale @end example =20 Note that the @code{glibc-locales} package contains data for all the @@ -958,6 +958,15 @@ locales supported by the GNU@tie{}libc and weighs in a= t around 110@tie{}MiB. Alternately, the @code{glibc-utf8-locales} is smaller but limited to a few UTF-8 locales. =20 +The @code{GUIX_LOCPATH} variable plays the exact same role as +@code{LOCPATH} (@pxref{Locale Names, @code{LOCPATH},, libc, The GNU C +Library Reference Manual}). However, since it is honored only by Guix's +libc, and not by the libc provided by foreign distros, using +@code{GUIX_LOCPATH} allows you to make sure the the foreign distro's +programs will not end up loading incompatible locale data. This is +important because the locale data format used by different libc versions +may be incompatible. + @subsection X11 Fonts =20 The majority of graphical applications use Fontconfig to locate and diff --git a/gnu-system.am b/gnu-system.am index f2f7e17..571e660 100644 --- a/gnu-system.am +++ b/gnu-system.am @@ -472,6 +472,7 @@ dist_patch_DATA =3D \ gnu/packages/patches/glibc-ldd-x86_64.patch \ gnu/packages/patches/glibc-locales.patch \ gnu/packages/patches/glibc-o-largefile.patch \ + gnu/packages/patches/glibc-versioned-locpath.patch \ gnu/packages/patches/gmp-arm-asm-nothumb.patch \ gnu/packages/patches/gnucash-price-quotes-perl.patch \ gnu/packages/patches/gnutls-doc-fix.patch \ diff --git a/gnu/packages/base.scm b/gnu/packages/base.scm index a7d9459..1402a44 100644 --- a/gnu/packages/base.scm +++ b/gnu/packages/base.scm @@ -476,6 +476,7 @@ store.") (modules '((guix build utils))) (patches (map search-patch '("glibc-ldd-x86_64.patch" + "glibc-versioned-locpath.patch" "glibc-o-largefile.patch"))))) (build-system gnu-build-system) =20 @@ -606,9 +607,11 @@ store.") =20 (native-search-paths ;; Search path for packages that provide locale data. This is useful - ;; primarily in build environments. + ;; primarily in build environments. Use 'GUIX_LOCPATH' rather than + ;; 'LOCPATH' to avoid interference with the host system's libc on fore= ign + ;; distros. (list (search-path-specification - (variable "LOCPATH") + (variable "GUIX_LOCPATH") (files '("lib/locale"))))) =20 (synopsis "The GNU C Library") @@ -649,10 +652,11 @@ the 'share/locale' sub-directory of this package.") (alist-delete 'install ,phases))) ((#:configure-flags flags) `(append ,flags - ;; Use $(libdir)/locale as is the case by default. + ;; Use $(libdir)/locale/X.Y as is the case by default. (list (string-append "libc_cv_localedir=3D" (assoc-ref %outputs "out") - "/lib/locale"))))))))) + "/lib/locale/" + ,(package-version glibc)))))))))) =20 (define-public glibc-utf8-locales (package @@ -661,7 +665,7 @@ the 'share/locale' sub-directory of this package.") (source #f) (build-system trivial-build-system) (arguments - '(#:modules ((guix build utils)) + `(#:modules ((guix build utils)) #:builder (begin (use-modules (srfi srfi-1) (guix build utils)) @@ -669,7 +673,8 @@ the 'share/locale' sub-directory of this package.") (let* ((libc (assoc-ref %build-inputs "glibc")) (gzip (assoc-ref %build-inputs "gzip")) (out (assoc-ref %outputs "out")) - (localedir (string-append out "/lib/locale"))) + (localedir (string-append out "/lib/locale/" + ,version))) ;; 'localedef' needs 'gzip'. (setenv "PATH" (string-append libc "/bin:" gzip "/bin= ")) =20 diff --git a/gnu/packages/patches/glibc-versioned-locpath.patch b/gnu/packa= ges/patches/glibc-versioned-locpath.patch new file mode 100644 index 0000000..bc76521 --- /dev/null +++ b/gnu/packages/patches/glibc-versioned-locpath.patch @@ -0,0 +1,240 @@ +The format of locale data can be incompatible between libc versions, and +loading incompatible data can lead to 'setlocale' returning EINVAL at best +or triggering an assertion failure at worst. See +https://lists.gnu.org/archive/html/guix-devel/2015-09/msg00717.html +for background information. + +To address that, this patch changes libc to honor a new 'GUIX_LOCPATH' +variable, and to look for locale data in version-specific sub-directories = of +that variable. So, if GUIX_LOCPATH=3D/foo:/bar, locale data is searched f= or in +/foo/X.Y and /bar/X.Y, where X.Y is the libc version number. + +That way, a single 'GUIX_LOCPATH' setting can work even if different libc +versions coexist on the system. + +--- a/locale/newlocale.c ++++ b/locale/newlocale.c +@@ -30,6 +30,7 @@ + /* Lock for protecting global data. */ + __libc_rwlock_define (extern , __libc_setlocale_lock attribute_hidden) +=20 ++extern error_t compute_locale_search_path (char **, size_t *); +=20 + /* Use this when we come along an error. */ + #define ERROR_RETURN \ +@@ -48,7 +49,6 @@ __newlocale (int category_mask, const char *locale, __lo= cale_t base) + __locale_t result_ptr; + char *locale_path; + size_t locale_path_len; +- const char *locpath_var; + int cnt; + size_t names_len; +=20 +@@ -102,17 +102,8 @@ __newlocale (int category_mask, const char *locale, _= _locale_t base) + locale_path =3D NULL; + locale_path_len =3D 0; +=20 +- locpath_var =3D getenv ("LOCPATH"); +- if (locpath_var !=3D NULL && locpath_var[0] !=3D '\0') +- { +- if (__argz_create_sep (locpath_var, ':', +- &locale_path, &locale_path_len) !=3D 0) +- return NULL; +- +- if (__argz_add_sep (&locale_path, &locale_path_len, +- _nl_default_locale_path, ':') !=3D 0) +- return NULL; +- } ++ if (compute_locale_search_path (&locale_path, &locale_path_len) !=3D 0) ++ return NULL; +=20 + /* Get the names for the locales we are interested in. We either + allow a composite name or a single name. */ +diff --git a/locale/setlocale.c b/locale/setlocale.c +index ead030d..0c0e314 100644 +--- a/locale/setlocale.c ++++ b/locale/setlocale.c +@@ -215,12 +215,65 @@ setdata (int category, struct __locale_data *data) + } + } +=20 ++/* Return in *LOCALE_PATH and *LOCALE_PATH_LEN the locale data search pat= h as ++ a colon-separated list. Return ENOMEN on error, zero otherwise. */ ++error_t ++compute_locale_search_path (char **locale_path, size_t *locale_path_len) ++{ ++ char* guix_locpath_var =3D getenv ("GUIX_LOCPATH"); ++ char *locpath_var =3D getenv ("LOCPATH"); ++ ++ if (guix_locpath_var !=3D NULL && guix_locpath_var[0] !=3D '\0') ++ { ++ /* Entries in 'GUIX_LOCPATH' take precedence over 'LOCPATH'. These ++ entries are systematically prefixed with "/X.Y" where "X.Y" is the ++ libc version. */ ++ if (__argz_create_sep (guix_locpath_var, ':', ++ locale_path, locale_path_len) !=3D 0 ++ || __argz_suffix_entries (locale_path, locale_path_len, ++ "/" VERSION) !=3D 0) ++ goto bail_out; ++ } ++ ++ if (locpath_var !=3D NULL && locpath_var[0] !=3D '\0') ++ { ++ char *reg_locale_path =3D NULL; ++ size_t reg_locale_path_len =3D 0; ++ ++ if (__argz_create_sep (locpath_var, ':', ++ ®_locale_path, ®_locale_path_len) !=3D 0) ++ goto bail_out; ++ ++ if (__argz_append (locale_path, locale_path_len, ++ reg_locale_path, reg_locale_path_len) !=3D 0) ++ goto bail_out; ++ ++ free (reg_locale_path); ++ } ++ ++ if (*locale_path !=3D NULL) ++ { ++ /* Append the system default locale directory. */ ++ if (__argz_add_sep (locale_path, locale_path_len, ++ _nl_default_locale_path, ':') !=3D 0) ++ goto bail_out; ++ } ++ ++ return 0; ++ ++ bail_out: ++ free (*locale_path); ++ *locale_path =3D NULL; ++ *locale_path_len =3D 0; ++ ++ return ENOMEM; ++} ++ + char * + setlocale (int category, const char *locale) + { + char *locale_path; + size_t locale_path_len; +- const char *locpath_var; + char *composite; +=20 + /* Sanity check for CATEGORY argument. */ +@@ -251,17 +304,10 @@ setlocale (int category, const char *locale) + locale_path =3D NULL; + locale_path_len =3D 0; +=20 +- locpath_var =3D getenv ("LOCPATH"); +- if (locpath_var !=3D NULL && locpath_var[0] !=3D '\0') ++ if (compute_locale_search_path (&locale_path, &locale_path_len) !=3D 0) + { +- if (__argz_create_sep (locpath_var, ':', +- &locale_path, &locale_path_len) !=3D 0 +- || __argz_add_sep (&locale_path, &locale_path_len, +- _nl_default_locale_path, ':') !=3D 0) +- { +- __libc_rwlock_unlock (__libc_setlocale_lock); +- return NULL; +- } ++ __libc_rwlock_unlock (__libc_setlocale_lock); ++ return NULL; + } +=20 + if (category =3D=3D LC_ALL) +diff --git a/string/Makefile b/string/Makefile +index 8424a61..f925503 100644 +--- a/string/Makefile ++++ b/string/Makefile +@@ -38,7 +38,7 @@ routines :=3D strcat strchr strcmp strcoll strcpy strcsp= n \ + swab strfry memfrob memmem rawmemchr strchrnul \ + $(addprefix argz-,append count create ctsep next \ + delete extract insert stringify \ +- addsep replace) \ ++ addsep replace suffix) \ + envz basename \ + strcoll_l strxfrm_l string-inlines memrchr \ + xpg-strerror strerror_l +diff --git a/string/argz-suffix.c b/string/argz-suffix.c +new file mode 100644 +index 0000000..505b0f2 +--- /dev/null ++++ b/string/argz-suffix.c +@@ -0,0 +1,56 @@ ++/* Copyright (C) 2015 Free Software Foundation, Inc. ++ This file is part of the GNU C Library. ++ Contributed by Ludovic Court=C3=A8s . ++ ++ The GNU C Library is free software; you can redistribute it and/or ++ modify it under the terms of the GNU Lesser General Public ++ License as published by the Free Software Foundation; either ++ version 2.1 of the License, or (at your option) any later version. ++ ++ The GNU C Library is distributed in the hope that it will be useful, ++ but WITHOUT ANY WARRANTY; without even the implied warranty of ++ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU ++ Lesser General Public License for more details. ++ ++ You should have received a copy of the GNU Lesser General Public ++ License along with the GNU C Library; if not, see ++ . */ ++ ++#include ++#include ++#include ++#include ++ ++ ++error_t ++__argz_suffix_entries (char **argz, size_t *argz_len, const char *suffix) ++ ++{ ++ size_t suffix_len =3D strlen (suffix); ++ size_t count =3D __argz_count (*argz, *argz_len); ++ size_t new_argz_len =3D *argz_len + count * suffix_len; ++ char *new_argz =3D malloc (new_argz_len); ++ ++ if (new_argz) ++ { ++ char *p =3D new_argz, *entry; ++ ++ for (entry =3D *argz; ++ entry !=3D NULL; ++ entry =3D argz_next (*argz, *argz_len, entry)) ++ { ++ p =3D stpcpy (p, entry); ++ p =3D stpcpy (p, suffix); ++ p++; ++ } ++ ++ free (*argz); ++ *argz =3D new_argz; ++ *argz_len =3D new_argz_len; ++ ++ return 0; ++ } ++ else ++ return ENOMEM; ++} ++weak_alias (__argz_suffix_entries, argz_suffix_entries) +diff --git a/string/argz.h b/string/argz.h +index bb62a31..d276a35 100644 +--- a/string/argz.h ++++ b/string/argz.h +@@ -134,6 +134,16 @@ extern error_t argz_replace (char **__restrict __argz, + const char *__restrict __str, + const char *__restrict __with, + unsigned int *__restrict __replace_count); ++ ++/* Suffix each entry of ARGZ & ARGZ_LEN with SUFFIX. Return 0 on success, ++ and ENOMEN if memory cannot be allocated. */ ++extern error_t __argz_suffix_entries (char **__restrict __argz, ++ size_t *__restrict __argz_len, ++ const char *__restrict __suffix); ++extern error_t argz_suffix_entries (char **__restrict __argz, ++ size_t *__restrict __argz_len, ++ const char *__restrict __suffix); ++ + + /* Returns the next entry in ARGZ & ARGZ_LEN after ENTRY, or NULL if there + are no more. If entry is NULL, then the first entry is returned. This --=20 2.5.0 --=-=-=--