Message ID | 6cf7432e-53ff-7c13-cb7a-55316faab495@redhat.com |
---|---|
State | New |
Headers | show |
Series | test-container: Add $complocaledir and mkdirp. | expand |
* Carlos O'Donell via Libc-alpha: > From 925c47b47d597663606486402c38e2e729ed663c Mon Sep 17 00:00:00 2001 > From: Carlos O'Donell <carlos@redhat.com> > Date: Thu, 23 Jan 2020 09:45:00 -0500 > Subject: [PATCH] test-container: Add $complocaledir and mkdirp. > > In order to test localedef in the test-container framework it > is necessary to add support for one new variable and one new > command. For localedef to install a compiled locale in the default > location the directory needs to be created > e.g. mkdir -p $complocaledir/dir. This command requires both the > new command 'mkdirp' e.g. mkdir -p, and the new variable > $complocaledir. The only way to avoid this would be to install > locales into the pristine container root, but that would slow down > container testing (currently only has builtin C/POSIX available). You could create the directory unconditionally, without installing any locales into it? Thanks, Florian
On 4/27/20 3:49 AM, Florian Weimer wrote: > * Carlos O'Donell via Libc-alpha: > >> From 925c47b47d597663606486402c38e2e729ed663c Mon Sep 17 00:00:00 2001 >> From: Carlos O'Donell <carlos@redhat.com> >> Date: Thu, 23 Jan 2020 09:45:00 -0500 >> Subject: [PATCH] test-container: Add $complocaledir and mkdirp. >> >> In order to test localedef in the test-container framework it >> is necessary to add support for one new variable and one new >> command. For localedef to install a compiled locale in the default >> location the directory needs to be created >> e.g. mkdir -p $complocaledir/dir. This command requires both the >> new command 'mkdirp' e.g. mkdir -p, and the new variable >> $complocaledir. The only way to avoid this would be to install >> locales into the pristine container root, but that would slow down >> container testing (currently only has builtin C/POSIX available). > > You could create the directory unconditionally, without installing any > locales into it? That's a very good point. I know of no reason we would want the directory *not* to be present. My argument is about saving time during testing and avoiding needing to run 'make localedata/install-locales' and build all the SUPPORTED locales. I'm going to adjust the patches and break them out like this: - Create $(complocaledir) when making testroot.pristine - Add new test. - Fixup old test which uses xmkdirp to create $(complocaledir) since it is no longer required.
diff --git a/support/test-container.c b/support/test-container.c index dff2ff379e..deb2cc3ff5 100644 --- a/support/test-container.c +++ b/support/test-container.c @@ -97,9 +97,13 @@ int verbose = 0; rm FILE cwd PATH exec FILE - FILE must start with $B/, $S/, $I/, $L/, or / - (expands to build dir, source dir, install dir, library dir - (in container), or container's root) + mkdirp MODE DIR + + FILE must start with $B/, $S/, $I/, $L/, $complocaledir/ + or / (expands to build dir, source dir, install dir, + library dir (in container), compiled locale dir + (in container), or container's root) + details: - '#': A comment. - 'su': Enables running test as root in the container. @@ -108,6 +112,8 @@ int verbose = 0; - 'rm': A minimal remove files command. - 'cwd': set test working directory - 'exec': change test binary location (may end in /) + - 'mkdirp': A minimal "mkdir -p FILE" command. + * mytest.root/postclean.req causes fresh rsync (with delete) after test if present @@ -859,6 +865,7 @@ main (int argc, char **argv) int nt = tokenize (the_line, the_words, 3); int i; + /* Expand variables. */ for (i = 1; i < nt; ++i) { if (memcmp (the_words[i], "$B/", 3) == 0) @@ -875,6 +882,10 @@ main (int argc, char **argv) the_words[i] = concat (new_root_path, support_libdir_prefix, the_words[i] + 2, NULL); + else if (memcmp (the_words[i], "$complocaledir/", 15) == 0) + the_words[i] = concat (new_root_path, + support_complocaledir_prefix, + the_words[i] + 14, NULL); /* "exec" and "cwd" use inside-root paths. */ else if (strcmp (the_words[0], "exec") != 0 && strcmp (the_words[0], "cwd") != 0 @@ -892,6 +903,8 @@ main (int argc, char **argv) the_words[2] = concat (the_words[2], the_words[1], NULL); } + /* Run the command in the_words[0] with NT number of arguments + (including the command). */ if (nt == 2 && strcmp (the_words[0], "so") == 0) { the_words[2] = concat (new_root_path, support_libdir_prefix, @@ -961,6 +974,12 @@ main (int argc, char **argv) { be_su = 1; } + else if (nt == 3 && strcmp (the_words[0], "mkdirp") == 0) + { + long int m; + m = strtol (the_words[1], NULL, 0); + xmkdirp (the_words[2], m); + } else if (nt > 0 && the_words[0][0] != '#') { fprintf (stderr, "\033[31minvalid [%s]\033[0m\n", the_words[0]);
Despite me writing "necessary" in the commit message, it's obviously not entirely required since you can just use: + xmkdirp (support_complocaledir_prefix, 0777); in the test. The above does exactly the same as running the following from the script: mkdirp 0755 $complocaledir/ However, the patch below enables the test developer to choose how they want to create the directory, via a script (patch enalbes this), or inside the test. One might argue that locales should just be installed by default in the test chroot, but I would argue against it. I would say the framework should do: * Container tests by default should have no locales (as we have it today, fastest chroot creation). * Container tests should be able to specify they want a specific locale installed or all locales (enhancement request, not supported today). OK for master? 8< --- 8< --- 8< From 925c47b47d597663606486402c38e2e729ed663c Mon Sep 17 00:00:00 2001 From: Carlos O'Donell <carlos@redhat.com> Date: Thu, 23 Jan 2020 09:45:00 -0500 Subject: [PATCH] test-container: Add $complocaledir and mkdirp. In order to test localedef in the test-container framework it is necessary to add support for one new variable and one new command. For localedef to install a compiled locale in the default location the directory needs to be created e.g. mkdir -p $complocaledir/dir. This command requires both the new command 'mkdirp' e.g. mkdir -p, and the new variable $complocaledir. The only way to avoid this would be to install locales into the pristine container root, but that would slow down container testing (currently only has builtin C/POSIX available). These new features will be used by the new tst-localedef-hardlinks test. --- support/test-container.c | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-)