test-container: ability to specify exec path

test-container: ability to specify exec path

[DJ Delorie]

Carlos asked for something like this for $ORIGIN testing...

diff --git a/nss/tst-nss-test3.root/tst-nss-test3.script b/nss/tst-nss-test3.root/tst-nss-test3.script
index a10beb1e6c..03ba5c5276 100644
--- a/nss/tst-nss-test3.root/tst-nss-test3.script
+++ b/nss/tst-nss-test3.root/tst-nss-test3.script
@@ -1,2 +1,3 @@
 cp $B/nss/libnss_test1.so $L/libnss_test1.so.2
 cp $B/nss/libnss_test2.so $L/libnss_test2.so.2
+exec /usr/bin/foo
diff --git a/support/test-container.c b/support/test-container.c
index 5d08979df3..d4ecfe8a98 100644
--- a/support/test-container.c
+++ b/support/test-container.c
@@ -95,6 +95,7 @@ int verbose = 0;
          mv FILE FILE
 	 cp FILE FILE
 	 rm FILE
+	 exec PATH
 	 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)
@@ -104,6 +105,7 @@ int verbose = 0;
          - 'mv': A minimal move files command.
          - 'cp': A minimal copy files command.
          - 'rm': A minimal remove files command.
+	 - 'exec': change where test is run from (may end in /)
    * mytest.root/postclean.req causes fresh rsync (with delete) after
      test if present
 
@@ -868,7 +870,9 @@ main (int argc, char **argv)
 		  the_words[i] = concat (new_root_path,
 					 support_libdir_prefix,
 					 the_words[i] + 2, NULL);
-		else if (the_words[i][0] == '/')
+		/* "exec" uses inside-root paths.  */
+		else if (strcmp (the_words[0], "exec") != 0
+			 && the_words[i][0] == '/')
 		  the_words[i] = concat (new_root_path,
 					 the_words[i], NULL);
 	      }
@@ -912,6 +916,33 @@ main (int argc, char **argv)
 	      {
 		maybe_xunlink (the_words[1]);
 	      }
+	    else if (nt == 2 && strcmp (the_words[0], "exec") == 0)
+	      {
+		char *new_exec_path = the_words[1];
+		char *test_basename = new_child_proc[0];
+
+		/* Find the base name of the test.  */
+		if (strrchr (test_basename, '/') != NULL)
+		  test_basename = strrchr (test_basename, '/') + 1;
+
+		/* If the new exec path ends with a slash, that's the
+		 * directory, and use the old test base name.  */
+		if (new_exec_path [strlen(new_exec_path) - 1] == '/')
+		    new_exec_path = concat (new_exec_path,
+					    test_basename,
+					    NULL);
+
+
+		/* new_child_proc is in the build tree, so has the
+		   same path inside the chroot as outside.  The new
+		   exec path is, by definition, relative to the
+		   chroot.  */
+		copy_one_file (new_child_proc[0],  concat (new_root_path,
+							   new_exec_path,
+							   NULL));
+
+		new_child_proc[0] = strdup (new_exec_path);
+	      }
 	    else if (nt == 1 && strcmp (the_words[0], "su") == 0)
 	      {
 		be_su = 1;

Re: test-container: ability to specify exec path

[Florian Weimer]

* DJ Delorie:

> Carlos asked for something like this for $ORIGIN testing...
>
> diff --git a/nss/tst-nss-test3.root/tst-nss-test3.script b/nss/tst-nss-test3.root/tst-nss-test3.script
> index a10beb1e6c..03ba5c5276 100644
> --- a/nss/tst-nss-test3.root/tst-nss-test3.script
> +++ b/nss/tst-nss-test3.root/tst-nss-test3.script
> @@ -1,2 +1,3 @@
>  cp $B/nss/libnss_test1.so $L/libnss_test1.so.2
>  cp $B/nss/libnss_test2.so $L/libnss_test2.so.2
> +exec /usr/bin/foo
> diff --git a/support/test-container.c b/support/test-container.c
> index 5d08979df3..d4ecfe8a98 100644
> --- a/support/test-container.c
> +++ b/support/test-container.c
> @@ -95,6 +95,7 @@ int verbose = 0;
>           mv FILE FILE
>  	 cp FILE FILE
>  	 rm FILE
> +	 exec PATH
>  	 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)
> @@ -104,6 +105,7 @@ int verbose = 0;
>           - 'mv': A minimal move files command.
>           - 'cp': A minimal copy files command.
>           - 'rm': A minimal remove files command.
> +	 - 'exec': change where test is run from (may end in /)

So it's the path to the executable in the container, and the test
program is copied there?

I think you should make this more explicit, because the comment above
sounds like as this changes the current directory for the test.

Thanks,
Florian

Re: test-container: ability to specify exec path

[DJ Delorie]

Florian Weimer <fweimer@redhat.com> writes:
> So it's the path to the executable in the container, and the test
> program is copied there?

Yes.

> I think you should make this more explicit, because the comment above
> sounds like as this changes the current directory for the test.

Do we need that too?

Re: test-container: ability to specify exec path

[Florian Weimer]

* DJ Delorie:

> Florian Weimer <fweimer@redhat.com> writes:
>> So it's the path to the executable in the container, and the test
>> program is copied there?
>
> Yes.
>
>> I think you should make this more explicit, because the comment above
>> sounds like as this changes the current directory for the test.
>
> Do we need that too?

Maybe?  I haven't seen a need for this yet.

Thanks,
Florian

Re: V2 test-container: ability to specify exec path

[DJ Delorie]

Florian Weimer <fweimer@redhat.com> writes:
>>> I think you should make this more explicit, because the comment above
>>> sounds like as this changes the current directory for the test.
>>
>> Do we need that too?
>
> Maybe?  I haven't seen a need for this yet.

This one adds "exec" (with optional argv[0] override) as well as "cwd"
to set the CWD of the test.

diff --git a/nss/tst-nss-test3.root/tst-nss-test3.script b/nss/tst-nss-test3.root/tst-nss-test3.script
index a10beb1e6c..d5b437ceb0 100644
--- a/nss/tst-nss-test3.root/tst-nss-test3.script
+++ b/nss/tst-nss-test3.root/tst-nss-test3.script
@@ -1,2 +1,4 @@
 cp $B/nss/libnss_test1.so $L/libnss_test1.so.2
 cp $B/nss/libnss_test2.so $L/libnss_test2.so.2
+cwd /usr/bin
+exec /usr/bin/foo bar
diff --git a/support/test-container.c b/support/test-container.c
index 5d08979df3..efdad46cc8 100644
--- a/support/test-container.c
+++ b/support/test-container.c
@@ -95,6 +95,8 @@ int verbose = 0;
          mv FILE FILE
 	 cp FILE FILE
 	 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)
@@ -104,6 +106,8 @@ int verbose = 0;
          - 'mv': A minimal move files command.
          - 'cp': A minimal copy files command.
          - 'rm': A minimal remove files command.
+	 - 'cwd': set test working directory
+	 - 'exec': change test binary location (may end in /)
    * mytest.root/postclean.req causes fresh rsync (with delete) after
      test if present
 
@@ -670,11 +674,13 @@ main (int argc, char **argv)
   char *new_objdir_path;
   char *new_srcdir_path;
   char **new_child_proc;
+  char *new_child_exec;
   char *command_root;
   char *command_base;
   char *command_basename;
   char *so_base;
   int do_postclean = 0;
+  char *change_cwd = NULL;
 
   int pipes[2];
   char pid_buf[20];
@@ -752,6 +758,7 @@ main (int argc, char **argv)
 				  "/testroot.root", NULL));
   new_cwd_path = get_current_dir_name ();
   new_child_proc = argv + 1;
+  new_child_exec = argv[0];
 
   lock_fd = open (concat (pristine_root_path, "/lock.fd", NULL),
 		 O_CREAT | O_TRUNC | O_RDWR, 0666);
@@ -868,7 +875,10 @@ main (int argc, char **argv)
 		  the_words[i] = concat (new_root_path,
 					 support_libdir_prefix,
 					 the_words[i] + 2, NULL);
-		else if (the_words[i][0] == '/')
+		/* "exec" and "cwd" use inside-root paths.  */
+		else if (strcmp (the_words[0], "exec") != 0
+			 && strcmp (the_words[0], "cwd") != 0
+			 && the_words[i][0] == '/')
 		  the_words[i] = concat (new_root_path,
 					 the_words[i], NULL);
 	      }
@@ -912,13 +922,54 @@ main (int argc, char **argv)
 	      {
 		maybe_xunlink (the_words[1]);
 	      }
+	    else if (nt >= 2 && strcmp (the_words[0], "exec") == 0)
+	      {
+		/* The first argument is the desired location and name
+		   of the test binary as we wish to exec it; we will
+		   copy the binary there.  The second (optional)
+		   argument is the value to pass as argv[0], it
+		   defaults to the same as the first argument.  */
+		char *new_exec_path = the_words[1];
+		char *test_basename = new_child_proc[0];
+
+		/* Find the base name of the test.  */
+		if (strrchr (test_basename, '/') != NULL)
+		  test_basename = strrchr (test_basename, '/') + 1;
+
+		/* If the new exec path ends with a slash, that's the
+		 * directory, and use the old test base name.  */
+		if (new_exec_path [strlen(new_exec_path) - 1] == '/')
+		    new_exec_path = concat (new_exec_path,
+					    test_basename,
+					    NULL);
+
+
+		/* new_child_proc is in the build tree, so has the
+		   same path inside the chroot as outside.  The new
+		   exec path is, by definition, relative to the
+		   chroot.  */
+		copy_one_file (new_child_proc[0],  concat (new_root_path,
+							   new_exec_path,
+							   NULL));
+
+		new_child_exec =  strdup (new_exec_path);
+		if (the_words[2])
+		  new_child_proc[0] = strdup (the_words[2]);
+		else
+		  new_child_proc[0] = new_child_exec;
+	      }
+	    else if (nt == 2 && strcmp (the_words[0], "cwd") == 0)
+	      {
+		change_cwd = strdup (the_words[1]);
+	      }
 	    else if (nt == 1 && strcmp (the_words[0], "su") == 0)
 	      {
 		be_su = 1;
 	      }
 	    else if (nt > 0 && the_words[0][0] != '#')
 	      {
-		printf ("\033[31minvalid [%s]\033[0m\n", the_words[0]);
+		fprintf (stderr, "\033[31minvalid [%s]\033[0m\n", the_words[0]);
+		exit (1);
 	      }
 	  }
 	fclose (f);
@@ -1089,8 +1140,14 @@ main (int argc, char **argv)
   write (GMAP, tmp, strlen (tmp));
   xclose (GMAP);
 
+  if (change_cwd)
+    {
+      if (chdir (change_cwd) < 0)
+	FAIL_EXIT1 ("Can't cd to %s inside container - ", change_cwd);
+    }
+
   /* Now run the child.  */
-  execvp (new_child_proc[0], new_child_proc);
+  execvp (new_child_exec, new_child_proc);
 
   /* Or don't run the child?  */
   FAIL_EXIT1 ("Unable to exec %s\n", new_child_proc[0]);

Re: V2 test-container: ability to specify exec path

[Florian Weimer]

* DJ Delorie:

> +		/* Find the base name of the test.  */
> +		if (strrchr (test_basename, '/') != NULL)
> +		  test_basename = strrchr (test_basename, '/') + 1;

Can you use the basename function here?

> +		/* If the new exec path ends with a slash, that's the
> +		 * directory, and use the old test base name.  */
> +		if (new_exec_path [strlen(new_exec_path) - 1] == '/')
> +		    new_exec_path = concat (new_exec_path,
> +					    test_basename,
> +					    NULL);
> +
> +
> +		/* new_child_proc is in the build tree, so has the
> +		   same path inside the chroot as outside.  The new
> +		   exec path is, by definition, relative to the
> +		   chroot.  */
> +		copy_one_file (new_child_proc[0],  concat (new_root_path,
> +							   new_exec_path,
> +							   NULL));
> +
> +		new_child_exec =  strdup (new_exec_path);
> +		if (the_words[2])
> +		  new_child_proc[0] = strdup (the_words[2]);
> +		else
> +		  new_child_proc[0] = new_child_exec;
> +	      }
> +	    else if (nt == 2 && strcmp (the_words[0], "cwd") == 0)
> +	      {
> +		change_cwd = strdup (the_words[1]);

Use xstrdup (twice)?

Rest looks okay to me, based on cursory glance.

Thanks,
Florian

Re: V3 test-container: ability to specify exec path

[DJ Delorie]

> Can you use the basename function here?

That does simplify things :-)

> Use xstrdup (twice)?

Well, I had used strdup throughout, so I did a general cleanup of it.

From c50ef7b7aa014532bab7f7ff25e499c0e3f038aa Mon Sep 17 00:00:00 2001
From: DJ Delorie <dj@redhat.com>
Date: Fri, 13 Dec 2019 13:37:30 -0500
Subject: test-container: add exec, cwd

exec <path_to_test_binary> [optional_argv_0]

  copies test binary to specified location and runs it from
  there.  If the second argument is provided, that will
  be used for argv[0]

cwd <directory>

  attempts to chdir(directory) before running test

Note: "cwd" not "cd" as it takes effect just before the
test binary runs, not when it's encountered in the script,
so it can't be used as a path shortcut like "cd" would imply.

cleanup: use xstrdup() instead of strdup()

diff --git a/support/test-container.c b/support/test-container.c
index 5d08979df3..62d888c05e 100644
--- a/support/test-container.c
+++ b/support/test-container.c
@@ -95,6 +95,8 @@ int verbose = 0;
          mv FILE FILE
 	 cp FILE FILE
 	 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)
@@ -104,6 +106,8 @@ int verbose = 0;
          - 'mv': A minimal move files command.
          - 'cp': A minimal copy files command.
          - 'rm': A minimal remove files command.
+	 - 'cwd': set test working directory
+	 - 'exec': change test binary location (may end in /)
    * mytest.root/postclean.req causes fresh rsync (with delete) after
      test if present
 
@@ -147,7 +151,7 @@ maybe_xmkdir (const char *path, mode_t mode)
 }
 
 /* Temporarily concatenate multiple strings into one.  Allows up to 10
-   temporary results; use strdup () if you need them to be
+   temporary results; use xstrdup () if you need them to be
    permanent.  */
 static char *
 concat (const char *str, ...)
@@ -670,11 +674,13 @@ main (int argc, char **argv)
   char *new_objdir_path;
   char *new_srcdir_path;
   char **new_child_proc;
+  char *new_child_exec;
   char *command_root;
   char *command_base;
   char *command_basename;
   char *so_base;
   int do_postclean = 0;
+  char *change_cwd = NULL;
 
   int pipes[2];
   char pid_buf[20];
@@ -746,12 +752,13 @@ main (int argc, char **argv)
 	}
     }
 
-  pristine_root_path = strdup (concat (support_objdir_root,
+  pristine_root_path = xstrdup (concat (support_objdir_root,
 				       "/testroot.pristine", NULL));
-  new_root_path = strdup (concat (support_objdir_root,
+  new_root_path = xstrdup (concat (support_objdir_root,
 				  "/testroot.root", NULL));
   new_cwd_path = get_current_dir_name ();
   new_child_proc = argv + 1;
+  new_child_exec = argv[0];
 
   lock_fd = open (concat (pristine_root_path, "/lock.fd", NULL),
 		 O_CREAT | O_TRUNC | O_RDWR, 0666);
@@ -778,10 +785,10 @@ main (int argc, char **argv)
     command_root = concat (support_srcdir_root,
 			   argv[1] + strlen (support_objdir_root),
 			   ".root", NULL);
-  command_root = strdup (command_root);
+  command_root = xstrdup (command_root);
 
   /* This cuts off the ".root" we appended above.  */
-  command_base = strdup (command_root);
+  command_base = xstrdup (command_root);
   command_base[strlen (command_base) - 5] = 0;
 
   /* This is the basename of the test we're running.  */
@@ -792,7 +799,7 @@ main (int argc, char **argv)
     ++command_basename;
 
   /* Shared object base directory.  */
-  so_base = strdup (argv[1]);
+  so_base = xstrdup (argv[1]);
   if (strrchr (so_base, '/') != NULL)
     strrchr (so_base, '/')[1] = 0;
 
@@ -806,9 +813,9 @@ main (int argc, char **argv)
       && S_ISDIR (st.st_mode))
     rsync (command_root, new_root_path, 0);
 
-  new_objdir_path = strdup (concat (new_root_path,
+  new_objdir_path = xstrdup (concat (new_root_path,
 				    support_objdir_root, NULL));
-  new_srcdir_path = strdup (concat (new_root_path,
+  new_srcdir_path = xstrdup (concat (new_root_path,
 				    support_srcdir_root, NULL));
 
   /* new_cwd_path starts with '/' so no "/" needed between the two.  */
@@ -868,7 +875,10 @@ main (int argc, char **argv)
 		  the_words[i] = concat (new_root_path,
 					 support_libdir_prefix,
 					 the_words[i] + 2, NULL);
-		else if (the_words[i][0] == '/')
+		/* "exec" and "cwd" use inside-root paths.  */
+		else if (strcmp (the_words[0], "exec") != 0
+			 && strcmp (the_words[0], "cwd") != 0
+			 && the_words[i][0] == '/')
 		  the_words[i] = concat (new_root_path,
 					 the_words[i], NULL);
 	      }
@@ -912,13 +922,49 @@ main (int argc, char **argv)
 	      {
 		maybe_xunlink (the_words[1]);
 	      }
+	    else if (nt >= 2 && strcmp (the_words[0], "exec") == 0)
+	      {
+		/* The first argument is the desired location and name
+		   of the test binary as we wish to exec it; we will
+		   copy the binary there.  The second (optional)
+		   argument is the value to pass as argv[0], it
+		   defaults to the same as the first argument.  */
+		char *new_exec_path = the_words[1];
+
+		/* If the new exec path ends with a slash, that's the
+		 * directory, and use the old test base name.  */
+		if (new_exec_path [strlen(new_exec_path) - 1] == '/')
+		    new_exec_path = concat (new_exec_path,
+					    basename (new_child_proc[0]),
+					    NULL);
+
+
+		/* new_child_proc is in the build tree, so has the
+		   same path inside the chroot as outside.  The new
+		   exec path is, by definition, relative to the
+		   chroot.  */
+		copy_one_file (new_child_proc[0],  concat (new_root_path,
+							   new_exec_path,
+							   NULL));
+
+		new_child_exec =  xstrdup (new_exec_path);
+		if (the_words[2])
+		  new_child_proc[0] = xstrdup (the_words[2]);
+		else
+		  new_child_proc[0] = new_child_exec;
+	      }
+	    else if (nt == 2 && strcmp (the_words[0], "cwd") == 0)
+	      {
+		change_cwd = xstrdup (the_words[1]);
+	      }
 	    else if (nt == 1 && strcmp (the_words[0], "su") == 0)
 	      {
 		be_su = 1;
 	      }
 	    else if (nt > 0 && the_words[0][0] != '#')
 	      {
-		printf ("\033[31minvalid [%s]\033[0m\n", the_words[0]);
+		fprintf (stderr, "\033[31minvalid [%s]\033[0m\n", the_words[0]);
+		exit (1);
 	      }
 	  }
 	fclose (f);
@@ -1089,8 +1135,14 @@ main (int argc, char **argv)
   write (GMAP, tmp, strlen (tmp));
   xclose (GMAP);
 
+  if (change_cwd)
+    {
+      if (chdir (change_cwd) < 0)
+	FAIL_EXIT1 ("Can't cd to %s inside container - ", change_cwd);
+    }
+
   /* Now run the child.  */
-  execvp (new_child_proc[0], new_child_proc);
+  execvp (new_child_exec, new_child_proc);
 
   /* Or don't run the child?  */
   FAIL_EXIT1 ("Unable to exec %s\n", new_child_proc[0]);

V3 test-container: ability to specify exec path

[DJ Delorie]

* Fixed Florian's comments from 12 Dec.
* Fixed new_child_name argv[0] bug
* Fixed prog name in usage (was "containerize ...")

From 1904ed5a5a34dafcb8a8a9810317ffc78494a0ab Mon Sep 17 00:00:00 2001
From: DJ Delorie <dj@redhat.com>
Date: Fri, 13 Dec 2019 13:37:30 -0500
Subject: test-container: add exec, cwd

exec <path_to_test_binary> [optional_argv_0]

  copies test binary to specified location and runs it from
  there.  If the second argument is provided, that will
  be used for argv[0]

cwd <directory>

  attempts to chdir(directory) before running test

Note: "cwd" not "cd" as it takes effect just before the
test binary runs, not when it's encountered in the script,
so it can't be used as a path shortcut like "cd" would imply.

cleanup: use xstrdup() instead of strdup()

diff --git a/support/test-container.c b/support/test-container.c
index 4c58254558..e970eb2c18 100644
--- a/support/test-container.c
+++ b/support/test-container.c
@@ -95,6 +95,8 @@ int verbose = 0;
          mv FILE FILE
 	 cp FILE FILE
 	 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)
@@ -104,6 +106,8 @@ int verbose = 0;
          - 'mv': A minimal move files command.
          - 'cp': A minimal copy files command.
          - 'rm': A minimal remove files command.
+	 - 'cwd': set test working directory
+	 - 'exec': change test binary location (may end in /)
    * mytest.root/postclean.req causes fresh rsync (with delete) after
      test if present
 
@@ -147,7 +151,7 @@ maybe_xmkdir (const char *path, mode_t mode)
 }
 
 /* Temporarily concatenate multiple strings into one.  Allows up to 10
-   temporary results; use strdup () if you need them to be
+   temporary results; use xstrdup () if you need them to be
    permanent.  */
 static char *
 concat (const char *str, ...)
@@ -670,11 +674,13 @@ main (int argc, char **argv)
   char *new_objdir_path;
   char *new_srcdir_path;
   char **new_child_proc;
+  char *new_child_exec;
   char *command_root;
   char *command_base;
   char *command_basename;
   char *so_base;
   int do_postclean = 0;
+  char *change_cwd = NULL;
 
   int pipes[2];
   char pid_buf[20];
@@ -701,7 +707,7 @@ main (int argc, char **argv)
 
   if (argc < 2)
     {
-      fprintf (stderr, "Usage: containerize <program to run> <args...>\n");
+      fprintf (stderr, "Usage: test-container <program to run> <args...>\n");
       exit (1);
     }
 
@@ -746,12 +752,13 @@ main (int argc, char **argv)
 	}
     }
 
-  pristine_root_path = strdup (concat (support_objdir_root,
+  pristine_root_path = xstrdup (concat (support_objdir_root,
 				       "/testroot.pristine", NULL));
-  new_root_path = strdup (concat (support_objdir_root,
+  new_root_path = xstrdup (concat (support_objdir_root,
 				  "/testroot.root", NULL));
   new_cwd_path = get_current_dir_name ();
   new_child_proc = argv + 1;
+  new_child_exec = argv[1];
 
   lock_fd = open (concat (pristine_root_path, "/lock.fd", NULL),
 		 O_CREAT | O_TRUNC | O_RDWR, 0666);
@@ -778,10 +785,10 @@ main (int argc, char **argv)
     command_root = concat (support_srcdir_root,
 			   argv[1] + strlen (support_objdir_root),
 			   ".root", NULL);
-  command_root = strdup (command_root);
+  command_root = xstrdup (command_root);
 
   /* This cuts off the ".root" we appended above.  */
-  command_base = strdup (command_root);
+  command_base = xstrdup (command_root);
   command_base[strlen (command_base) - 5] = 0;
 
   /* This is the basename of the test we're running.  */
@@ -792,7 +799,7 @@ main (int argc, char **argv)
     ++command_basename;
 
   /* Shared object base directory.  */
-  so_base = strdup (argv[1]);
+  so_base = xstrdup (argv[1]);
   if (strrchr (so_base, '/') != NULL)
     strrchr (so_base, '/')[1] = 0;
 
@@ -806,9 +813,9 @@ main (int argc, char **argv)
       && S_ISDIR (st.st_mode))
     rsync (command_root, new_root_path, 0);
 
-  new_objdir_path = strdup (concat (new_root_path,
+  new_objdir_path = xstrdup (concat (new_root_path,
 				    support_objdir_root, NULL));
-  new_srcdir_path = strdup (concat (new_root_path,
+  new_srcdir_path = xstrdup (concat (new_root_path,
 				    support_srcdir_root, NULL));
 
   /* new_cwd_path starts with '/' so no "/" needed between the two.  */
@@ -868,7 +875,10 @@ main (int argc, char **argv)
 		  the_words[i] = concat (new_root_path,
 					 support_libdir_prefix,
 					 the_words[i] + 2, NULL);
-		else if (the_words[i][0] == '/')
+		/* "exec" and "cwd" use inside-root paths.  */
+		else if (strcmp (the_words[0], "exec") != 0
+			 && strcmp (the_words[0], "cwd") != 0
+			 && the_words[i][0] == '/')
 		  the_words[i] = concat (new_root_path,
 					 the_words[i], NULL);
 	      }
@@ -912,13 +922,49 @@ main (int argc, char **argv)
 	      {
 		maybe_xunlink (the_words[1]);
 	      }
+	    else if (nt >= 2 && strcmp (the_words[0], "exec") == 0)
+	      {
+		/* The first argument is the desired location and name
+		   of the test binary as we wish to exec it; we will
+		   copy the binary there.  The second (optional)
+		   argument is the value to pass as argv[0], it
+		   defaults to the same as the first argument.  */
+		char *new_exec_path = the_words[1];
+
+		/* If the new exec path ends with a slash, that's the
+		 * directory, and use the old test base name.  */
+		if (new_exec_path [strlen(new_exec_path) - 1] == '/')
+		    new_exec_path = concat (new_exec_path,
+					    basename (new_child_proc[0]),
+					    NULL);
+
+
+		/* new_child_proc is in the build tree, so has the
+		   same path inside the chroot as outside.  The new
+		   exec path is, by definition, relative to the
+		   chroot.  */
+		copy_one_file (new_child_proc[0],  concat (new_root_path,
+							   new_exec_path,
+							   NULL));
+
+		new_child_exec =  xstrdup (new_exec_path);
+		if (the_words[2])
+		  new_child_proc[0] = xstrdup (the_words[2]);
+		else
+		  new_child_proc[0] = new_child_exec;
+	      }
+	    else if (nt == 2 && strcmp (the_words[0], "cwd") == 0)
+	      {
+		change_cwd = xstrdup (the_words[1]);
+	      }
 	    else if (nt == 1 && strcmp (the_words[0], "su") == 0)
 	      {
 		be_su = 1;
 	      }
 	    else if (nt > 0 && the_words[0][0] != '#')
 	      {
-		printf ("\033[31minvalid [%s]\033[0m\n", the_words[0]);
+		fprintf (stderr, "\033[31minvalid [%s]\033[0m\n", the_words[0]);
+		exit (1);
 	      }
 	  }
 	fclose (f);
@@ -1089,11 +1135,17 @@ main (int argc, char **argv)
   write (GMAP, tmp, strlen (tmp));
   xclose (GMAP);
 
+  if (change_cwd)
+    {
+      if (chdir (change_cwd) < 0)
+	FAIL_EXIT1 ("Can't cd to %s inside container - ", change_cwd);
+    }
+
   /* Now run the child.  */
-  execvp (new_child_proc[0], new_child_proc);
+  execvp (new_child_exec, new_child_proc);
 
   /* Or don't run the child?  */
-  FAIL_EXIT1 ("Unable to exec %s\n", new_child_proc[0]);
+  FAIL_EXIT1 ("Unable to exec %s\n", new_child_exec);
 
   /* Because gcc won't know error () never returns...  */
   exit (EXIT_UNSUPPORTED);

Re: V3 test-container: ability to specify exec path

[Carlos O'Donell]

On 1/28/20 3:38 PM, DJ Delorie wrote:
> 
> * Fixed Florian's comments from 12 Dec.
> * Fixed new_child_name argv[0] bug
> * Fixed prog name in usage (was "containerize ...")

I'm excited to see this go in, I have like 50+ tests I used for
$ORIGIN testing and DST parsing that I can use this for.

The next step would be to also support setuid running inside
the container for testing because many code paths rely on this.

OK for master when it reopens.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> From 1904ed5a5a34dafcb8a8a9810317ffc78494a0ab Mon Sep 17 00:00:00 2001
> From: DJ Delorie <dj@redhat.com>
> Date: Fri, 13 Dec 2019 13:37:30 -0500
> Subject: test-container: add exec, cwd
> 
> exec <path_to_test_binary> [optional_argv_0]
> 
>   copies test binary to specified location and runs it from
>   there.  If the second argument is provided, that will
>   be used for argv[0]

OK.

> 
> cwd <directory>
> 
>   attempts to chdir(directory) before running test

OK.

> 
> Note: "cwd" not "cd" as it takes effect just before the
> test binary runs, not when it's encountered in the script,
> so it can't be used as a path shortcut like "cd" would imply.

OK.

> 
> cleanup: use xstrdup() instead of strdup()
> 
> diff --git a/support/test-container.c b/support/test-container.c
> index 4c58254558..e970eb2c18 100644
> --- a/support/test-container.c
> +++ b/support/test-container.c
> @@ -95,6 +95,8 @@ int verbose = 0;
>           mv FILE FILE
>  	 cp FILE FILE
>  	 rm FILE
> +	 cwd PATH
> +	 exec FILE

OK.

>  	 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)
> @@ -104,6 +106,8 @@ int verbose = 0;
>           - 'mv': A minimal move files command.
>           - 'cp': A minimal copy files command.
>           - 'rm': A minimal remove files command.
> +	 - 'cwd': set test working directory
> +	 - 'exec': change test binary location (may end in /)

OK.

>     * mytest.root/postclean.req causes fresh rsync (with delete) after
>       test if present
>  
> @@ -147,7 +151,7 @@ maybe_xmkdir (const char *path, mode_t mode)
>  }
>  
>  /* Temporarily concatenate multiple strings into one.  Allows up to 10
> -   temporary results; use strdup () if you need them to be
> +   temporary results; use xstrdup () if you need them to be

OK.

>     permanent.  */
>  static char *
>  concat (const char *str, ...)
> @@ -670,11 +674,13 @@ main (int argc, char **argv)
>    char *new_objdir_path;
>    char *new_srcdir_path;
>    char **new_child_proc;
> +  char *new_child_exec;

OK.

>    char *command_root;
>    char *command_base;
>    char *command_basename;
>    char *so_base;
>    int do_postclean = 0;
> +  char *change_cwd = NULL;

OK.

>  
>    int pipes[2];
>    char pid_buf[20];
> @@ -701,7 +707,7 @@ main (int argc, char **argv)
>  
>    if (argc < 2)
>      {
> -      fprintf (stderr, "Usage: containerize <program to run> <args...>\n");
> +      fprintf (stderr, "Usage: test-container <program to run> <args...>\n");

OK.

>        exit (1);
>      }
>  
> @@ -746,12 +752,13 @@ main (int argc, char **argv)
>  	}
>      }
>  
> -  pristine_root_path = strdup (concat (support_objdir_root,
> +  pristine_root_path = xstrdup (concat (support_objdir_root,
>  				       "/testroot.pristine", NULL));

OK.

> -  new_root_path = strdup (concat (support_objdir_root,
> +  new_root_path = xstrdup (concat (support_objdir_root,
>  				  "/testroot.root", NULL));

OK.

>    new_cwd_path = get_current_dir_name ();
>    new_child_proc = argv + 1;
> +  new_child_exec = argv[1];

OK.

>  
>    lock_fd = open (concat (pristine_root_path, "/lock.fd", NULL),
>  		 O_CREAT | O_TRUNC | O_RDWR, 0666);
> @@ -778,10 +785,10 @@ main (int argc, char **argv)
>      command_root = concat (support_srcdir_root,
>  			   argv[1] + strlen (support_objdir_root),
>  			   ".root", NULL);
> -  command_root = strdup (command_root);
> +  command_root = xstrdup (command_root);
>  
>    /* This cuts off the ".root" we appended above.  */
> -  command_base = strdup (command_root);
> +  command_base = xstrdup (command_root);
>    command_base[strlen (command_base) - 5] = 0;

OK.

>  
>    /* This is the basename of the test we're running.  */
> @@ -792,7 +799,7 @@ main (int argc, char **argv)
>      ++command_basename;
>  
>    /* Shared object base directory.  */
> -  so_base = strdup (argv[1]);
> +  so_base = xstrdup (argv[1]);

OK.

>    if (strrchr (so_base, '/') != NULL)
>      strrchr (so_base, '/')[1] = 0;
>  
> @@ -806,9 +813,9 @@ main (int argc, char **argv)
>        && S_ISDIR (st.st_mode))
>      rsync (command_root, new_root_path, 0);
>  
> -  new_objdir_path = strdup (concat (new_root_path,
> +  new_objdir_path = xstrdup (concat (new_root_path,
>  				    support_objdir_root, NULL));
> -  new_srcdir_path = strdup (concat (new_root_path,
> +  new_srcdir_path = xstrdup (concat (new_root_path,
>  				    support_srcdir_root, NULL));

OK.

>  
>    /* new_cwd_path starts with '/' so no "/" needed between the two.  */
> @@ -868,7 +875,10 @@ main (int argc, char **argv)
>  		  the_words[i] = concat (new_root_path,
>  					 support_libdir_prefix,
>  					 the_words[i] + 2, NULL);
> -		else if (the_words[i][0] == '/')
> +		/* "exec" and "cwd" use inside-root paths.  */
> +		else if (strcmp (the_words[0], "exec") != 0
> +			 && strcmp (the_words[0], "cwd") != 0
> +			 && the_words[i][0] == '/')
>  		  the_words[i] = concat (new_root_path,
>  					 the_words[i], NULL);

OK.

>  	      }
> @@ -912,13 +922,49 @@ main (int argc, char **argv)
>  	      {
>  		maybe_xunlink (the_words[1]);
>  	      }
> +	    else if (nt >= 2 && strcmp (the_words[0], "exec") == 0)
> +	      {
> +		/* The first argument is the desired location and name
> +		   of the test binary as we wish to exec it; we will
> +		   copy the binary there.  The second (optional)
> +		   argument is the value to pass as argv[0], it
> +		   defaults to the same as the first argument.  */
> +		char *new_exec_path = the_words[1];

OK.

> +
> +		/* If the new exec path ends with a slash, that's the
> +		 * directory, and use the old test base name.  */
> +		if (new_exec_path [strlen(new_exec_path) - 1] == '/')
> +		    new_exec_path = concat (new_exec_path,
> +					    basename (new_child_proc[0]),
> +					    NULL);

OK.

> +
> +
> +		/* new_child_proc is in the build tree, so has the
> +		   same path inside the chroot as outside.  The new
> +		   exec path is, by definition, relative to the
> +		   chroot.  */
> +		copy_one_file (new_child_proc[0],  concat (new_root_path,
> +							   new_exec_path,
> +							   NULL));

OK.

> +
> +		new_child_exec =  xstrdup (new_exec_path);

OK.

> +		if (the_words[2])
> +		  new_child_proc[0] = xstrdup (the_words[2]);
> +		else
> +		  new_child_proc[0] = new_child_exec;

OK. Set new_child_proc[0]

> +	      }
> +	    else if (nt == 2 && strcmp (the_words[0], "cwd") == 0)
> +	      {
> +		change_cwd = xstrdup (the_words[1]);

OK.


> +	      }
>  	    else if (nt == 1 && strcmp (the_words[0], "su") == 0)
>  	      {
>  		be_su = 1;
>  	      }
>  	    else if (nt > 0 && the_words[0][0] != '#')
>  	      {
> -		printf ("\033[31minvalid [%s]\033[0m\n", the_words[0]);
> +		fprintf (stderr, "\033[31minvalid [%s]\033[0m\n", the_words[0]);
> +		exit (1);

OK.

>  	      }
>  	  }
>  	fclose (f);
> @@ -1089,11 +1135,17 @@ main (int argc, char **argv)
>    write (GMAP, tmp, strlen (tmp));
>    xclose (GMAP);
>  
> +  if (change_cwd)
> +    {
> +      if (chdir (change_cwd) < 0)
> +	FAIL_EXIT1 ("Can't cd to %s inside container - ", change_cwd);
> +    }

OK.

> +
>    /* Now run the child.  */
> -  execvp (new_child_proc[0], new_child_proc);
> +  execvp (new_child_exec, new_child_proc);

OK.

>  
>    /* Or don't run the child?  */
> -  FAIL_EXIT1 ("Unable to exec %s\n", new_child_proc[0]);
> +  FAIL_EXIT1 ("Unable to exec %s\n", new_child_exec);

OK.

>  
>    /* Because gcc won't know error () never returns...  */
>    exit (EXIT_UNSUPPORTED);
> 


-- 
Cheers,
Carlos.

Re: V3 test-container: ability to specify exec path

[DJ Delorie]

"Carlos O'Donell" <codonell@redhat.com> writes:
> On 1/28/20 3:38 PM, DJ Delorie wrote:
>> 
>> * Fixed Florian's comments from 12 Dec.
>> * Fixed new_child_name argv[0] bug
>> * Fixed prog name in usage (was "containerize ...")
>
> I'm excited to see this go in, I have like 50+ tests I used for
> $ORIGIN testing and DST parsing that I can use this for.
>
> OK for master when it reopens.

Thanks.  Pushed!