diff mbox series

test-container: Add $complocaledir and mkdirp.

Message ID 6cf7432e-53ff-7c13-cb7a-55316faab495@redhat.com
State New
Headers show
Series test-container: Add $complocaledir and mkdirp. | expand

Commit Message

Carlos O'Donell April 26, 2020, 7:17 p.m. UTC
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(-)

Comments

Florian Weimer April 27, 2020, 7:49 a.m. UTC | #1
* 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
Carlos O'Donell April 28, 2020, 7:59 p.m. UTC | #2
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 mbox series

Patch

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]);