[PATCH] Ensure that nsswitch.conf for nss/tst-reload[12] are really synced.

[PATCH] Ensure that nsswitch.conf for nss/tst-reload[12] are really synced.

[Stefan Liebler]

Both tests, nss/tst-reload1 and nss/tst-reload2 are test-container
tests and both provide a root directory for the container.

Unfortunately on a fresh git clone, all the nsswitch files have
the same size of 40bytes and timestamp:
nss/tst-reload1.root/etc/nsswitch.conf2
nss/tst-reload2.root/etc/nsswitch.conf
nss/tst-reload2.root/subdir/etc/nsswitch.conf
nss/tst-reload1.root/etc/nsswitch.conf

After running tst-reload1, the <objpfx>/testroot.root/etc/nsswitch.conf
file has the content of nss/tst-reload1.root/etc/nsswitch.conf2.

Rerunning with "make t=nss/tst-reload1 test" leads to fails
as nsswitch.conf is not synced by test-container.c.

Therefore this patch just adds different comments to those files
to ensure that test-container.c is always syncing them.
---
 nss/tst-reload1.root/etc/nsswitch.conf  | 4 ++++
 nss/tst-reload1.root/etc/nsswitch.conf2 | 3 +++
 nss/tst-reload2.root/etc/nsswitch.conf  | 2 ++
 3 files changed, 9 insertions(+)

diff --git a/nss/tst-reload1.root/etc/nsswitch.conf b/nss/tst-reload1.root/etc/nsswitch.conf
index 606d8f51d6..56248fc8a3 100644
--- a/nss/tst-reload1.root/etc/nsswitch.conf
+++ b/nss/tst-reload1.root/etc/nsswitch.conf
@@ -1,3 +1,7 @@
+# This nsswitch.conf file should be synced by test-container.c while starting
+# tst-reload1.  Ensure that the size differs compared to the other
+# nsswitch.conf[2] files of tst-reload1 or tst-reload2.  Otherwise
+# test-container.c won't sync it.
 passwd:	test1
 group:	test1
 hosts:	test1
diff --git a/nss/tst-reload1.root/etc/nsswitch.conf2 b/nss/tst-reload1.root/etc/nsswitch.conf2
index 2e93977efb..762dcc3f36 100644
--- a/nss/tst-reload1.root/etc/nsswitch.conf2
+++ b/nss/tst-reload1.root/etc/nsswitch.conf2
@@ -1,3 +1,6 @@
+# This nsswitch.conf will be used while running tst-reload2.
+# Ensure that the size differs compared to the other nsswitch.conf[2] files of
+# tst-reload1 or tst-reload2.  Otherwise test-container.c won't sync it.
 passwd:	test2
 group:	test2
 hosts:	test2
diff --git a/nss/tst-reload2.root/etc/nsswitch.conf b/nss/tst-reload2.root/etc/nsswitch.conf
index 688a589519..fbe057480f 100644
--- a/nss/tst-reload2.root/etc/nsswitch.conf
+++ b/nss/tst-reload2.root/etc/nsswitch.conf
@@ -1,3 +1,5 @@
+# Ensure that the size differs compared to the other nsswitch.conf[2] files of
+# tst-reload1 or tst-reload2.  Otherwise test-container.c won't sync it.
 passwd: test1
 group: test2
 hosts: files
-- 
2.28.0

Re: [PATCH] Ensure that nsswitch.conf for nss/tst-reload[12] are really synced.

[H.J. Lu]

On Wed, Mar 10, 2021 at 8:06 AM Stefan Liebler via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> Both tests, nss/tst-reload1 and nss/tst-reload2 are test-container
> tests and both provide a root directory for the container.
>
> Unfortunately on a fresh git clone, all the nsswitch files have
> the same size of 40bytes and timestamp:
> nss/tst-reload1.root/etc/nsswitch.conf2
> nss/tst-reload2.root/etc/nsswitch.conf
> nss/tst-reload2.root/subdir/etc/nsswitch.conf
> nss/tst-reload1.root/etc/nsswitch.conf
>
> After running tst-reload1, the <objpfx>/testroot.root/etc/nsswitch.conf
> file has the content of nss/tst-reload1.root/etc/nsswitch.conf2.
>
> Rerunning with "make t=nss/tst-reload1 test" leads to fails
> as nsswitch.conf is not synced by test-container.c.
>
> Therefore this patch just adds different comments to those files
> to ensure that test-container.c is always syncing them.
> ---
>  nss/tst-reload1.root/etc/nsswitch.conf  | 4 ++++
>  nss/tst-reload1.root/etc/nsswitch.conf2 | 3 +++
>  nss/tst-reload2.root/etc/nsswitch.conf  | 2 ++
>  3 files changed, 9 insertions(+)
>
> diff --git a/nss/tst-reload1.root/etc/nsswitch.conf b/nss/tst-reload1.root/etc/nsswitch.conf
> index 606d8f51d6..56248fc8a3 100644
> --- a/nss/tst-reload1.root/etc/nsswitch.conf
> +++ b/nss/tst-reload1.root/etc/nsswitch.conf
> @@ -1,3 +1,7 @@
> +# This nsswitch.conf file should be synced by test-container.c while starting
> +# tst-reload1.  Ensure that the size differs compared to the other
> +# nsswitch.conf[2] files of tst-reload1 or tst-reload2.  Otherwise
> +# test-container.c won't sync it.
>  passwd:        test1
>  group: test1
>  hosts: test1
> diff --git a/nss/tst-reload1.root/etc/nsswitch.conf2 b/nss/tst-reload1.root/etc/nsswitch.conf2
> index 2e93977efb..762dcc3f36 100644
> --- a/nss/tst-reload1.root/etc/nsswitch.conf2
> +++ b/nss/tst-reload1.root/etc/nsswitch.conf2
> @@ -1,3 +1,6 @@
> +# This nsswitch.conf will be used while running tst-reload2.
> +# Ensure that the size differs compared to the other nsswitch.conf[2] files of
> +# tst-reload1 or tst-reload2.  Otherwise test-container.c won't sync it.
>  passwd:        test2
>  group: test2
>  hosts: test2
> diff --git a/nss/tst-reload2.root/etc/nsswitch.conf b/nss/tst-reload2.root/etc/nsswitch.conf
> index 688a589519..fbe057480f 100644
> --- a/nss/tst-reload2.root/etc/nsswitch.conf
> +++ b/nss/tst-reload2.root/etc/nsswitch.conf
> @@ -1,3 +1,5 @@
> +# Ensure that the size differs compared to the other nsswitch.conf[2] files of
> +# tst-reload1 or tst-reload2.  Otherwise test-container.c won't sync it.
>  passwd: test1
>  group: test2
>  hosts: files
> --
> 2.28.0
>

Does this fix

https://sourceware.org/bugzilla/show_bug.cgi?id=27537

If yes, please mention it in the commit log.

Thanks.

-- 
H.J.

Re: [PATCH] Ensure that nsswitch.conf for nss/tst-reload[12] are really synced.

[Stefan Liebler]

On 3/10/21 5:26 PM, H.J. Lu wrote:
> On Wed, Mar 10, 2021 at 8:06 AM Stefan Liebler via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
>>
>> Both tests, nss/tst-reload1 and nss/tst-reload2 are test-container
>> tests and both provide a root directory for the container.
>>
>> Unfortunately on a fresh git clone, all the nsswitch files have
>> the same size of 40bytes and timestamp:
>> nss/tst-reload1.root/etc/nsswitch.conf2
>> nss/tst-reload2.root/etc/nsswitch.conf
>> nss/tst-reload2.root/subdir/etc/nsswitch.conf
>> nss/tst-reload1.root/etc/nsswitch.conf
>>
>> After running tst-reload1, the <objpfx>/testroot.root/etc/nsswitch.conf
>> file has the content of nss/tst-reload1.root/etc/nsswitch.conf2.
>>
>> Rerunning with "make t=nss/tst-reload1 test" leads to fails
>> as nsswitch.conf is not synced by test-container.c.
>>
>> Therefore this patch just adds different comments to those files
>> to ensure that test-container.c is always syncing them.
>> ---
>>  nss/tst-reload1.root/etc/nsswitch.conf  | 4 ++++
>>  nss/tst-reload1.root/etc/nsswitch.conf2 | 3 +++
>>  nss/tst-reload2.root/etc/nsswitch.conf  | 2 ++
>>  3 files changed, 9 insertions(+)
>>
>> diff --git a/nss/tst-reload1.root/etc/nsswitch.conf b/nss/tst-reload1.root/etc/nsswitch.conf
>> index 606d8f51d6..56248fc8a3 100644
>> --- a/nss/tst-reload1.root/etc/nsswitch.conf
>> +++ b/nss/tst-reload1.root/etc/nsswitch.conf
>> @@ -1,3 +1,7 @@
>> +# This nsswitch.conf file should be synced by test-container.c while starting
>> +# tst-reload1.  Ensure that the size differs compared to the other
>> +# nsswitch.conf[2] files of tst-reload1 or tst-reload2.  Otherwise
>> +# test-container.c won't sync it.
>>  passwd:        test1
>>  group: test1
>>  hosts: test1
>> diff --git a/nss/tst-reload1.root/etc/nsswitch.conf2 b/nss/tst-reload1.root/etc/nsswitch.conf2
>> index 2e93977efb..762dcc3f36 100644
>> --- a/nss/tst-reload1.root/etc/nsswitch.conf2
>> +++ b/nss/tst-reload1.root/etc/nsswitch.conf2
>> @@ -1,3 +1,6 @@
>> +# This nsswitch.conf will be used while running tst-reload2.
>> +# Ensure that the size differs compared to the other nsswitch.conf[2] files of
>> +# tst-reload1 or tst-reload2.  Otherwise test-container.c won't sync it.
>>  passwd:        test2
>>  group: test2
>>  hosts: test2
>> diff --git a/nss/tst-reload2.root/etc/nsswitch.conf b/nss/tst-reload2.root/etc/nsswitch.conf
>> index 688a589519..fbe057480f 100644
>> --- a/nss/tst-reload2.root/etc/nsswitch.conf
>> +++ b/nss/tst-reload2.root/etc/nsswitch.conf
>> @@ -1,3 +1,5 @@
>> +# Ensure that the size differs compared to the other nsswitch.conf[2] files of
>> +# tst-reload1 or tst-reload2.  Otherwise test-container.c won't sync it.
>>  passwd: test1
>>  group: test2
>>  hosts: files
>> --
>> 2.28.0
>>
> 
> Does this fix
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=27537
> 
> If yes, please mention it in the commit log.
> 
> Thanks.
> 

I'm getting the same output as mentioned in the bugzilla:
$ cd <builddir>
$ rm -f testroot.root/etc/nsswitch.conf
$ make t=nss/tst-reload1 test
PASS: nss/tst-reload1
$ cat testroot.root/etc/nsswitch.conf
passwd:	test2
group:	test2
hosts:	test2

$ make t=nss/tst-reload2 test
tst-reload2.c:112: numeric comparison failure
   left: 5 (0x5); from: pw->pw_uid
  right: 1234 (0x4d2); from: 1234
error: tst-reload2.c:130: not true: pw->pw_uid != 2468
tst-reload2.c:136: numeric comparison failure
   left: 5 (0x5); from: pw->pw_uid
  right: 1234 (0x4d2); from: 1234
error: tst-reload2.c:146: not true: he != NULL
error: 4 test failures

$ cat testroot.root/etc/nsswitch.conf
passwd:	test2
group:	test2
hosts:	test2

According to tst-reload2.c and current nsswitch.conf, it is using
pwd_table2, which is PWD_N (5, "test1"). According to the
nss/tst-reload2.root/etc/nsswitch.conf, it should use PWD_N (1234,
"test1") of pwd_table1.

Thus, yes, I assume this is the same issue.

Bye,
Stefan

Re: [PATCH] Ensure that nsswitch.conf for nss/tst-reload[12] are really synced.

[DJ Delorie]

This is a bit genius for figuring this out, but it makes me wary that it
might happen again.

How about we *force* copies from the .root directories?  That shouldn't
add much time since we *should* copy those anyway, and the .root
directories should only hold a few (hopefully) small support files.

We can still use regular symc from the testroot.pristine I think.

Re: [PATCH] Ensure that nsswitch.conf for nss/tst-reload[12] are really synced.

[DJ Delorie]

> Unfortunately on a fresh git clone, all the nsswitch files have
> the same size of 40bytes and timestamp:

Please try this:

diff --git a/support/test-container.c b/support/test-container.c
index 28cc44d9f1..94498d3901 100644
--- a/support/test-container.c
+++ b/support/test-container.c
@@ -481,7 +481,7 @@ need_sync (char *ap, char *bp, struct stat *a, struct stat *b)
 }
 
 static void
-rsync_1 (path_buf * src, path_buf * dest, int and_delete)
+rsync_1 (path_buf * src, path_buf * dest, int and_delete, int force_copies)
 {
   DIR *dir;
   struct dirent *de;
@@ -491,8 +491,9 @@ rsync_1 (path_buf * src, path_buf * dest, int and_delete)
   r_append ("/", dest);
 
   if (verbose)
-    printf ("sync %s to %s %s\n", src->buf, dest->buf,
-	    and_delete ? "and delete" : "");
+    printf ("sync %s to %s%s%s\n", src->buf, dest->buf,
+	    and_delete ? " and delete" : "",
+	    force_copies ? " (forced)" : "");
 
   size_t staillen = src->len;
 
@@ -521,10 +522,10 @@ rsync_1 (path_buf * src, path_buf * dest, int and_delete)
 	 missing.  */
       lstat (dest->buf, &d);
 
-      if (! need_sync (src->buf, dest->buf, &s, &d))
+      if (! force_copies && ! need_sync (src->buf, dest->buf, &s, &d))
 	{
 	  if (S_ISDIR (s.st_mode))
-	    rsync_1 (src, dest, and_delete);
+	    rsync_1 (src, dest, and_delete, force_copies);
 	  continue;
 	}
 
@@ -559,7 +560,7 @@ rsync_1 (path_buf * src, path_buf * dest, int and_delete)
 	  if (verbose)
 	    printf ("+D %s\n", dest->buf);
 	  maybe_xmkdir (dest->buf, (s.st_mode & 0777) | 0700);
-	  rsync_1 (src, dest, and_delete);
+	  rsync_1 (src, dest, and_delete, force_copies);
 	  break;
 
 	case S_IFLNK:
@@ -639,12 +640,12 @@ rsync_1 (path_buf * src, path_buf * dest, int and_delete)
 }
 
 static void
-rsync (char *src, char *dest, int and_delete)
+rsync (char *src, char *dest, int and_delete, int force_copies)
 {
   r_setup (src, &spath);
   r_setup (dest, &dpath);
 
-  rsync_1 (&spath, &dpath, and_delete);
+  rsync_1 (&spath, &dpath, and_delete, force_copies);
 }
 
 \f
@@ -846,11 +847,11 @@ main (int argc, char **argv)
     do_ldconfig = true;
 
   rsync (pristine_root_path, new_root_path,
-	 file_exists (concat (command_root, "/preclean.req", NULL)));
+	 file_exists (concat (command_root, "/preclean.req", NULL)), 0);
 
   if (stat (command_root, &st) >= 0
       && S_ISDIR (st.st_mode))
-    rsync (command_root, new_root_path, 0);
+    rsync (command_root, new_root_path, 0, 1);
 
   new_objdir_path = xstrdup (concat (new_root_path,
 				    support_objdir_root, NULL));
@@ -1044,7 +1045,7 @@ main (int argc, char **argv)
 
 	  /* Child has exited, we can post-clean the test root.  */
 	  printf("running post-clean rsync\n");
-	  rsync (pristine_root_path, new_root_path, 1);
+	  rsync (pristine_root_path, new_root_path, 1, 0);
 
 	  if (WIFEXITED (status))
 	    exit (WEXITSTATUS (status));

Re: [PATCH] Ensure that nsswitch.conf for nss/tst-reload[12] are really synced.

[Stefan Liebler]

On 3/10/21 8:06 PM, DJ Delorie wrote:
> 
>> Unfortunately on a fresh git clone, all the nsswitch files have
>> the same size of 40bytes and timestamp:
> 
> Please try this:
> 
> diff --git a/support/test-container.c b/support/test-container.c
> index 28cc44d9f1..94498d3901 100644
> --- a/support/test-container.c
> +++ b/support/test-container.c
> @@ -481,7 +481,7 @@ need_sync (char *ap, char *bp, struct stat *a, struct stat *b)
>  }
>  
>  static void
> -rsync_1 (path_buf * src, path_buf * dest, int and_delete)
> +rsync_1 (path_buf * src, path_buf * dest, int and_delete, int force_copies)
>  {
>    DIR *dir;
>    struct dirent *de;
> @@ -491,8 +491,9 @@ rsync_1 (path_buf * src, path_buf * dest, int and_delete)
>    r_append ("/", dest);
>  
>    if (verbose)
> -    printf ("sync %s to %s %s\n", src->buf, dest->buf,
> -	    and_delete ? "and delete" : "");
> +    printf ("sync %s to %s%s%s\n", src->buf, dest->buf,
> +	    and_delete ? " and delete" : "",
> +	    force_copies ? " (forced)" : "");
>  
>    size_t staillen = src->len;
>  
> @@ -521,10 +522,10 @@ rsync_1 (path_buf * src, path_buf * dest, int and_delete)
>  	 missing.  */
>        lstat (dest->buf, &d);
>  
> -      if (! need_sync (src->buf, dest->buf, &s, &d))
> +      if (! force_copies && ! need_sync (src->buf, dest->buf, &s, &d))
>  	{
>  	  if (S_ISDIR (s.st_mode))
> -	    rsync_1 (src, dest, and_delete);
> +	    rsync_1 (src, dest, and_delete, force_copies);
>  	  continue;
>  	}
>  
> @@ -559,7 +560,7 @@ rsync_1 (path_buf * src, path_buf * dest, int and_delete)
>  	  if (verbose)
>  	    printf ("+D %s\n", dest->buf);
>  	  maybe_xmkdir (dest->buf, (s.st_mode & 0777) | 0700);
> -	  rsync_1 (src, dest, and_delete);
> +	  rsync_1 (src, dest, and_delete, force_copies);
>  	  break;
>  
>  	case S_IFLNK:
> @@ -639,12 +640,12 @@ rsync_1 (path_buf * src, path_buf * dest, int and_delete)
>  }
>  
>  static void
> -rsync (char *src, char *dest, int and_delete)
> +rsync (char *src, char *dest, int and_delete, int force_copies)
>  {
>    r_setup (src, &spath);
>    r_setup (dest, &dpath);
>  
> -  rsync_1 (&spath, &dpath, and_delete);
> +  rsync_1 (&spath, &dpath, and_delete, force_copies);
>  }
>  
>  \f
> @@ -846,11 +847,11 @@ main (int argc, char **argv)
>      do_ldconfig = true;
>  
>    rsync (pristine_root_path, new_root_path,
> -	 file_exists (concat (command_root, "/preclean.req", NULL)));
> +	 file_exists (concat (command_root, "/preclean.req", NULL)), 0);
>  
>    if (stat (command_root, &st) >= 0
>        && S_ISDIR (st.st_mode))
> -    rsync (command_root, new_root_path, 0);
> +    rsync (command_root, new_root_path, 0, 1);
>  
>    new_objdir_path = xstrdup (concat (new_root_path,
>  				    support_objdir_root, NULL));
> @@ -1044,7 +1045,7 @@ main (int argc, char **argv)
>  
>  	  /* Child has exited, we can post-clean the test root.  */
>  	  printf("running post-clean rsync\n");
> -	  rsync (pristine_root_path, new_root_path, 1);
> +	  rsync (pristine_root_path, new_root_path, 1, 0);
>  
>  	  if (WIFEXITED (status))
>  	    exit (WEXITSTATUS (status));
> 

Hi DJ,

with your patch, the files are synced and I've successfully rerun
nss/tst-reload1 multiple times. Rerunning nss/tst-reload2 after
nss/tst-reload1 and the other way around is also fine.

Please commit your patch and also mention
"Bug 27537 - [2.33/2.34 Regression] FAIL: nss/tst-reload2"
(I've just added a comment there)

Then my patch is not needed anymore.

Bye,
Stefan