See:
https://bugzilla.redhat.com/show_bug.cgi?id=156477#c36
This bugzilla comment contains a patch for our glibc package to reduce teh disk usage of locale-archive.
Please review for inclusion into rawhide.
On 03/02/2015 01:23 PM, Mike FABIAN wrote:
See:
https://bugzilla.redhat.com/show_bug.cgi?id=156477#c36
This bugzilla comment contains a patch for our glibc package to reduce teh disk usage of locale-archive.
Please review for inclusion into rawhide.
Overall this patch looks great.
Rather than get you to submit a second patch, I'm going to fixup the code to match my requests, and then check that in.
Please see below for the things I've changed and why.
Final patch that I will checkin is attached.
From 637c48329e764aee82f7327e35f567506372d7ee Mon Sep 17 00:00:00 2001 From: Mike FABIAN mfabian@redhat.com Date: Mon, 9 Feb 2015 11:56:11 +0100 Subject: [PATCH] Change build-locale-archive.c to support an option --install-langs and use it in the .spec file
- Use new option --install-langs and the macro %_install_langs in %post and %triggerin. To enable evaluation of the %_install_langs macro at runtime, the %post and %triggerin scripts need the -e option.
build-locale-archive.c | 205 +++++++++++++++++++++++++++++++++++++++++++++---- glibc.spec | 8 +- 2 files changed, 195 insertions(+), 18 deletions(-)
diff --git a/build-locale-archive.c b/build-locale-archive.c index 187c7e5..e6cde8e 100644 --- a/build-locale-archive.c +++ b/build-locale-archive.c @@ -8,6 +8,7 @@ #include <stdbool.h> #include <stdio.h> #include <stdlib.h> +#include <getopt.h>
OK.
#include <string.h> #include <sys/mman.h> #include <sys/stat.h> @@ -21,6 +22,7 @@ const char *alias_file = DATADIR "/locale/locale.alias"; const char *locar_file = PREFIX "/lib/locale/locale-archive"; const char *tmpl_file = PREFIX "/lib/locale/locale-archive.tmpl"; const char *loc_path = PREFIX "/lib/locale/"; +/* Flags set by ‘--verbose’ */
Follow GNU Coding standard (period and two spaces, and use ``): /* Flags set by `--verbose` option. */
int be_quiet = 1; int verbose = 0; int max_locarchive_open_retry = 10; @@ -122,7 +124,7 @@ open_tmpl_archive (struct locarhandle *ah) ah->mmaped = (head.sumhash_offset + head.sumhash_size * sizeof (struct sumhashent)); if (ah->mmaped > (unsigned long) st.st_size)
- error (EXIT_FAILURE, 0, "locale archite template file truncated");
- error (EXIT_FAILURE, 0, "locale archive template file truncated");
OK.
ah->mmaped = st.st_size; ah->reserved = st.st_size;
@@ -257,7 +259,9 @@ compute_data (struct locarhandle *ah, struct nameent *name, size_t sumused,
static int fill_archive (struct locarhandle *tmpl_ah,
const char *fname, size_t nlist, char *list[],
const char *fname,
size_t install_langs_count, char *install_langs_list[],
size_t nlist, char *list[],
OK.
const char *primary)
{ struct locarhandle ah; @@ -288,11 +292,35 @@ fill_archive (struct locarhandle *tmpl_ah, for (cnt = used = 0; cnt < head->namehash_size; ++cnt) if (namehashtab[cnt].locrec_offset != 0) {
- char * name;
- int i; assert (used < head->namehash_used);
- names[used].name = tmpl_ah->addr + namehashtab[cnt].name_offset;
- names[used++].locrec
= (struct locrecent *) ((char *) tmpl_ah->addr +
namehashtab[cnt].locrec_offset);
name = tmpl_ah->addr + namehashtab[cnt].name_offset;
if (install_langs_count == 0)
{
Added comment:
/* Always install the entry. */
names[used].name = name;
names[used++].locrec
= (struct locrecent *) ((char *) tmpl_ah->addr +
namehashtab[cnt].locrec_offset);
}
Added comment:
/* Only install the entry if the user asked for it via --install-langs. */
else
{
for (i = 0; i < install_langs_count; i++)
{
char install_lang[64] = "";
Made this dynamic.
strncpy (install_lang, install_langs_list[i], 64-2);
install_lang[len - 1] = '\0';
Note: strncpy can produce a non-NULL terminated output if an entry is as long as length, so we must terminate manually always to get a limit on the length.
The final code I wrote was:
/* Add one for "_" and one for the null terminator. */ size_t len = strlen (install_lang_list[i]) + 2; char *install_lang = (char *)xmalloc (len); strncpy (install_lang, install_langs_list[i], len - 2); install_lang[len - 1] = '\0'; if (strchr (install_lang, '_') == NULL) strcat (install_lang, "_"); if (strncmp (name, install_lang, strlen (install_lang)) == 0) { names[used].name = name; names[used++].locrec = (struct locrecent *) ((char *)tmpl_ah->addr + namehashtab[cnt].locrec_offset); } free (install_lang);
Haven't compiled it yet, but that's the idea of what I'll checkin.
if (!strchr(install_lang, '_'))
strcat(install_lang, "_");
if (strchr (install_lang, '_') == NULL) strcat(install_lang, "_");
Note: Avoid Boolean-coersion and check explicitly for NULL. Note: Use "foo ()" not "foo()" e.g. GNU Coding style adds the space after function name.
if (strncmp (name, install_lang, strlen(install_lang)) == 0)
{
names[used].name = name;
names[used++].locrec
= (struct locrecent *) ((char *) tmpl_ah->addr +
namehashtab[cnt].locrec_offset);
names[used++].locrec = (struct locrecent *) ((char *)tmpl_ah->addr + namehashtab[cnt].locrec_offset);
Note: Even if it goes over the line length limit, aim to cut the line at an operator and reindent to the innermost bracket. This way the operator makes it clear the line is a continuation.
}
}
} }
/* Sort the names. */
@@ -542,6 +570,61 @@ fill_archive (struct locarhandle *tmpl_ah, return result; }
+void usage() +{
- printf("Usage: build-locale-archive [OPTION]... [TEMPLATE-FILE] [ARCHIVE-FILE]\n");
- printf("Builds a locale archive from a template file.\n");
- printf("Options:\n");
- printf("-h, --help Print this usage message.\n");
- printf("-v, --verbose Verbose execution.\n");
- printf("-l, --install-langs=LIST Only include locales given\n");
- printf(" in LIST into the locale archive.\n");
- printf(" LIST is a colon separated list of\n");
- printf(" locale prefixes, for example "de:en:ja".\n");
- printf(" The special argument "all" means\n");
- printf(" to install all languages.\n");
- printf(" If the --install-langs option is\n");
- printf(" missing, all locales are installed.\n");
- printf(" The colon separated list can contain\n");
- printf(" any strings matching the beginning\n");
- printf(" of locale names. If a string does\n");
- printf(" not contain a "_", it is added.\n");
- printf(" Examples:\n");
- printf(" --install-langs="en"\n");
- printf(" installs en_US, enUS.iso88591,\n");
- printf(" en_US.iso885915, en_US.utf8,\n");
- printf(" en_GB ...\n");
- printf(" --install-langs="en_US.utf8"\n");
- printf(" installs only en_US.utf8.\n");
- printf(" --install-langs="ko"\n");
- printf(" installs ko_KR, ko_KR.euckr,\n");
- printf(" ko_KR.utf8 but *not* kok_IN\n");
- printf(" because "ko" does not contain\n");
- printf(" "_" and it is silently added\n");
- printf(" --install-langs"ko:kok"\n");
- printf(" installs ko_KR, ko_KR.euckr,\n");
- printf(" ko_KR.utf8, kok_IN, and\n");
- printf(" kok_IN.utf8.\n");
- printf(" --install-langs="POSIX" will\n");
- printf(" installs *no* locales at all\n");
- printf(" because POSIX matches none of\n");
- printf(" the locales. Actually, any string\n");
- printf(" matching nothing will do that.\n");
- printf(" POSIX and C will always be\n");
- printf(" available because they are\n");
- printf(" buildin.\n");
- printf(" Aliases are installed as well,\n");
- printf(" i.e. --install-langs="de"\n");
- printf(" will install not only every locale\n");
- printf(" starting with "de" but also the\n");
- printf(" the aliases "deutsch" and\n");
- printf(" and "german" although the latter\n");
- printf(" does not start with "de".\n");
- printf("If the arguments TEMPLATE-FILE and ARCHIVE-FILE are not given\n");
- printf("the locations where the glibc used expects these files are\n");
- printf("used by default.\n");
+}
Note: Rewritten as:
void usage() { printf ("\ Usage: build-locale-archive [OPTION]... [TEMPLATE-FILE] [ARCHIVE-FILE]\n\ Builds a locale archive from a template file.\n\ Options:\n\ -h, --help Print this usage message.\n\ -v, --verbose Verbose execution.\n\ -l, --install-langs=LIST Only include locales given in LIST into the \n\ locale archive. LIST is a colon separated list\n\ of locale prefixes, for example "de:en:ja".\n\ The special argument "all" means to install\n\ all languages. If the --install-langs option\n\ is missing, all locales are installed. The colon\n\ separated list can contain any strings matching\n\ the beginning of locale names. If a string does\n\ not contain a "_", it is added.\n\ ... \n\ If the arguments TEMPLATE-FILE and ARCHIVE-FILE are not given the locations\n\ where the glibc used expects these files are used by default.\n\ "); }
That makes it easy to edit in source and know exactly what the output will look like and count columns.
int main (int argc, char *argv[]) { char path[4096]; @@ -549,22 +632,113 @@ int main (int argc, char *argv[]) struct dirent64 *d; struct stat64 st; char *list[16384], *primary;
- char *lang;
- int install_langs_count = 0;
- int i;
- char *install_langs_arg;
- char *install_langs_list[16384];
Made this dynamic.
unsigned int cnt = 0; struct locarhandle tmpl_ah;
char *new_locar_fname = NULL; size_t loc_path_len = strlen (loc_path);
while (1)
{
int c;
static struct option long_options[] =
{
{"help", no_argument, 0, 'h'},
{"verbose", no_argument, 0, 'v'},
{"install-langs", required_argument, 0, 'l'},
{0, 0, 0, 0}
};
/* getopt_long stores the option index here. */
int option_index = 0;
c = getopt_long (argc, argv, "vhl:",
long_options, &option_index);
/* Detect the end of the options. */
if (c == -1)
break;
switch (c)
{
case 0:
printf ("unknown option %s", long_options[option_index].name);
if (optarg)
printf (" with arg %s", optarg);
printf ("\n");
usage ();
exit (1);
case 'v':
verbose = 1;
be_quiet = 0;
break;
case 'h':
usage ();
exit (0);
case 'l':
install_langs_arg = strdup(optarg);
/* If the argument to --install-lang is "all", do
not limit the list of languages to install, install
them all */
if (install_langs_arg && strncmp(install_langs_arg, "all", 3))
{
while (true)
{
lang = strtok(install_langs_arg, ":;,");
if (lang == NULL)
break;
install_langs_list[install_langs_count] = lang;
install_langs_count++;
install_langs_arg = NULL;
This is now dynamic, we count the number of args, xmalloc, then copy.
}
}
break;
case '?':
/* getopt_long already printed an error message. */
usage ();
exit (0);
default:
abort ();
}
- }
- tmpl_ah.fname = NULL;
- if (optind < argc)
- tmpl_ah.fname = argv[optind];
- if (optind + 1 < argc)
- new_locar_fname = argv[optind + 1];
- if (verbose)
- {
if (tmpl_ah.fname)
printf("input archive file specified on command line: %s\n",
tmpl_ah.fname);
else
printf("using default input archive file.\n");
if (new_locar_fname)
printf("output archive file specified on command line: %s\n",
new_locar_fname);
else
printf("using default output archive file.\n");
- }
- dirp = opendir (loc_path); if (dirp == NULL) error (EXIT_FAILURE, errno, "cannot open directory "%s"", loc_path);
/* Use the template file as specified on the command line. */
tmpl_ah.fname = NULL;
if (argc > 1)
tmpl_ah.fname = argv[1];
open_tmpl_archive (&tmpl_ah);
unlink (locar_file);
- if (new_locar_fname)
- unlink (new_locar_fname);
- else
- unlink (locar_file); primary = getenv ("LC_ALL"); if (primary == NULL) primary = getenv ("LANG");
@@ -575,7 +749,8 @@ int main (int argc, char *argv[]) && strncmp (primary, "zh", 2) != 0) { char *ptr = malloc (strlen (primary) + strlen (".utf8") + 1), *p, *q;
/* This leads to invalid locales sometimes:
if (ptr != NULL) { p = ptr;de_DE.iso885915@euro -> de_DE.utf8@euro */
@@ -640,7 +815,9 @@ int main (int argc, char *argv[]) closedir (dirp); /* Store the archive to the file specified as the second argument on the command line or the default locale archive. */
- fill_archive (&tmpl_ah, argc > 2 ? argv[2] : NULL, cnt, list, primary);
- fill_archive (&tmpl_ah, new_locar_fname,
install_langs_count, install_langs_list,
cnt, list, primary);
We now free the strdup'd strings, and the array of char*'s here.
close_archive (&tmpl_ah); truncate (tmpl_file, 0); char *tz_argv[] = { "/usr/sbin/tzdata-update", NULL }; diff --git a/glibc.spec b/glibc.spec index b5392de..a845182 100644 --- a/glibc.spec +++ b/glibc.spec @@ -1579,22 +1579,22 @@ end
%postun -p /sbin/ldconfig
-%triggerin common -p <lua> -- glibc +%triggerin common -e -p <lua> -- glibc if posix.stat("%{_prefix}/lib/locale/locale-archive.tmpl", "size") > 0 then pid = posix.fork() if pid == 0 then
- posix.exec("%{_prefix}/sbin/build-locale-archive")
- posix.exec("%{_prefix}/sbin/build-locale-archive", "--install-langs", "%%{_install_langs}")
OK.
elseif pid > 0 then posix.wait(pid) end end
-%post common -p <lua> +%post common -e -p <lua> if posix.access("/etc/ld.so.cache") then if posix.stat("%{_prefix}/lib/locale/locale-archive.tmpl", "size") > 0 then pid = posix.fork() if pid == 0 then
posix.exec("%{_prefix}/sbin/build-locale-archive")
posix.exec("%{_prefix}/sbin/build-locale-archive", "--install-langs", "%%{_install_langs}")
OK.
elseif pid > 0 then posix.wait(pid) end
-- 2.1.0
When I finish the build I'll checkin the changes.
Cheers, Carlos.
"Carlos O'Donell" carlos@redhat.com さんはかきました:
On 03/02/2015 01:23 PM, Mike FABIAN wrote:
See:
https://bugzilla.redhat.com/show_bug.cgi?id=156477#c36
This bugzilla comment contains a patch for our glibc package to reduce teh disk usage of locale-archive.
Please review for inclusion into rawhide.
Overall this patch looks great.
Rather than get you to submit a second patch, I'm going to fixup the code to match my requests, and then check that in.
Please see below for the things I've changed and why.
Final patch that I will checkin is attached.
Looks good, thank you!