Re: [OE-core] pseudo: Outdated records for newly-ignored paths in database cause mismatches

["Mike Crowe" <yocto@mac.mcrowe.com>]

[-- Attachment #1: Type: text/plain, Size: 1281 bytes --]

On Monday 09 August 2021 at 09:09:16 -0500, Seebs wrote:
> On Mon, 9 Aug 2021 13:19:51 +0100
> "Mike Crowe via lists.openembedded.org"
> <yocto=mac.mcrowe.com@lists.openembedded.org> wrote:
> 
> > Cleaning the work directory makes the problem go away because that
> > deletes the pseudo databases.
> > 
> > Does the above make sense as an explanation for these errors? If so,
> > is there a good way to avoid these errors?
> 
> Good diagnostic work, makes sense to me. It would make some sense for
> pseudo to ignore mismatches involving ignored paths, but it wasn't
> originally designed with the ignored paths concept, so it currently
> doesn't.

Thanks for the review.

I have a test case and patch for pseudo (attached) to detect newly-ignored
paths and warn rather than abort on them, but I'm not really convinced that
it is the right solution. Ideally the errant entry would be removed from
the database too in order to avoid having to continue to consult the ignore
list.

It's not even clear to me that oe-core continuing to use an existing pseudo
database after the value of PSEUDO_CLIENT_IGNORE_PATH changes is a sane
thing to expect to work. Perhaps we could just arrange to force a whole new
work directory in that case?

Thanks.

Mike.

[-- Attachment #2: 0001-pseudo-Path-mismatch-on-now-ignored-path-should-not-.patch --]
[-- Type: text/x-diff, Size: 3523 bytes --]

From e81aeff391148280d76609e5782bf7f0a115f72e Mon Sep 17 00:00:00 2001
From: Mike Crowe <mcrowe@brightsign.biz>
Date: Wed, 11 Aug 2021 15:55:55 +0100
Subject: [PATCH] pseudo: Path mismatch on now-ignored path should not be fatal

If a database survives from before a change to PSEUDO_CLIENT_IGNORE_PATH
then there's a risk that the now-ignored files have been deleted and
their inodes re-used without pseudo noticing. Such files are reported as
path mismatches which provoke aborts.

Let's check to see whether the database filename would now be ignored,
and if so just warn about the mismatch rather than aborting.

Unfortunately the test case for this doesn't fit into the existing
infrastructure since the server must be restarted during the test.

Signed-off-by: Mike Crowe <mac@mcrowe.com>
---
 pseudo.c                                      |  2 +
 run_tests.sh                                  | 16 ++++++++
 .../standalone-test-newly-ignored-mismatch.sh | 41 +++++++++++++++++++
 3 files changed, 59 insertions(+)
 create mode 100755 test/standalone-test-newly-ignored-mismatch.sh

diff --git a/pseudo.c b/pseudo.c
index 528fe1b..30b0a36 100644
--- a/pseudo.c
+++ b/pseudo.c
@@ -695,6 +695,8 @@ pseudo_op(pseudo_msg_t *msg, const char *program, const char *tag, char **respon
 					 */
 					pseudo_debug(PDBGF_FILE, "inode mismatch for '%s' -- old one was marked for deletion.\n",
 						msg->path);
+				} else if (path_by_ino && pseudo_client_ignore_path(path_by_ino)) {
+					pseudo_diag("path mismatch on now-ignored '%s'", path_by_ino);
 				} else {
 					pseudo_diag("path mismatch [%d link%s]: ino %llu db '%s' req '%s'.\n",
 						msg->nlink,
diff --git a/run_tests.sh b/run_tests.sh
index c637c27..a0b8675 100755
--- a/run_tests.sh
+++ b/run_tests.sh
@@ -48,5 +48,21 @@ do
     fi
     rm -rf var/pseudo/*
 done
+for file in test/standalone-test*.sh
+do
+    filename=${file#test/}
+    let num_tests++
+    mkdir -p var/pseudo
+    $file ${opt_verbose}
+    if [ "$?" -eq "0" ]; then
+        let num_passed_tests++
+        if [ "${opt_verbose}" == "-v" ]; then
+            echo "${filename%.sh}: Passed."
+        fi
+    else
+        echo "${filename/%.sh}: Failed."
+    fi
+    rm -rf var/pseudo/*
+done
 echo "${num_passed_tests}/${num_tests} test(s) passed."
 
diff --git a/test/standalone-test-newly-ignored-mismatch.sh b/test/standalone-test-newly-ignored-mismatch.sh
new file mode 100755
index 0000000..bf7d5f7
--- /dev/null
+++ b/test/standalone-test-newly-ignored-mismatch.sh
@@ -0,0 +1,41 @@
+#!/bin/bash
+#
+# SPDX-License-Identifier: LGPL-2.1-only
+#
+export PSEUDO_PREFIX=${PWD}
+pseudo=bin/pseudo
+
+trap "rm -rf testdir" EXIT
+rm -rf testdir
+mkdir testdir || exit 1
+
+mkdir -p testdir/to-be-ignored
+mkdir -p testdir/not-ignored
+
+create_files() {
+    for i in a b c d e f g h i j; do
+	for j in 0 1 2 3 4 5 6 7 8 9; do
+	    touch testdir/to-be-ignored/$i$j
+	done
+    done
+}
+
+test_results() {
+    for i in a b c d e f g h i j; do
+	for j in 0 1 2 3 4 5 6 7 8 9; do
+	    touch testdir/not-ignored/$i$j
+	done
+    done
+}
+
+export -f create_files
+export -f test_results
+
+export PSEUDO_IGNORE_PATHS=/initial
+
+$pseudo /bin/bash -c "create_files"
+rm testdir/to-be-ignored/*
+
+# Kill server so that we can change the value of PSEUDO_IGNORE_PATHS
+$pseudo -S
+PSEUDO_IGNORE_PATHS=${PWD}/testdir/to-be-ignored $pseudo /bin/bash -c "test_results"
-- 
2.30.2