[PATCH 0/2] A couple of fixes for non-standard builddir
The other day somebody on #virt complained about test suite failing on a fresh checkout. Turned out, our mocking and some bash scripts are not prepared for a case when there's a space in builddir path. After these, there are still some tests failing, but my brain is too small to fix them: 1) Somehow, $builddir/docs/html/libvirt-libvirt-common.html (and friends) are generated into $builddir/html/libvirt-libvirt-common.html and I can't figure out why. This means, check-html and check-html-references tests are failing. 2) There's some problem with po_check as it fails to find generated sources (like src/remote/remote_client_bodies.h). I've taken look into our syntax-check.mk but I have no idea what's wrong nor how to properly escape paths in Makefile. Michal Prívozník (2): tests: mock: Accept spaces in build path tests: Allow spaces in path to virt-aa-helper tests/securityselinuxlabeltest.c | 2 +- tests/securityselinuxtest.c | 2 +- tests/testutils.c | 6 ++++-- tests/testutils.h | 12 +++++++++++- tests/viridentitytest.c | 2 +- tests/virt-aa-helper-test | 4 ++-- 6 files changed, 20 insertions(+), 8 deletions(-) -- 2.43.2
If path to the build directory contains spaces (e.g. meson setup 'a b') then our mocks don't work. The problem is in glibc where not just a colon but also a space character is a delimiter for LD_PRELOAD [1]. Hence, a test using mock tries to preload something like libvirt.git/a b/libsomethingmock.so which is interpreted by glibc as two separate strings: "libvirt.git/a", "b/libsomethingmock.so". One trick to get around this is to set LD_PRELOAD to just the shared object file (without path) and let glibc find the mock in paths specified in LD_LIBRARY_PATH (where only a colon or a semicolon are valid separators [1]). This can be seen in action by running say: LD_DEBUG=libs ./virpcitest 1: https://man7.org/linux/man-pages/man8/ld.so.8.html Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/securityselinuxlabeltest.c | 2 +- tests/securityselinuxtest.c | 2 +- tests/testutils.c | 6 ++++-- tests/testutils.h | 12 +++++++++++- tests/viridentitytest.c | 2 +- 5 files changed, 18 insertions(+), 6 deletions(-) diff --git a/tests/securityselinuxlabeltest.c b/tests/securityselinuxlabeltest.c index 04bffe4356..7b7cf53569 100644 --- a/tests/securityselinuxlabeltest.c +++ b/tests/securityselinuxlabeltest.c @@ -347,4 +347,4 @@ mymain(void) VIR_TEST_MAIN_PRELOAD(mymain, VIR_TEST_MOCK("domaincaps"), - abs_builddir "/libsecurityselinuxhelper.so") + "libsecurityselinuxhelper.so") diff --git a/tests/securityselinuxtest.c b/tests/securityselinuxtest.c index 6a0f1617ae..6aadc6154f 100644 --- a/tests/securityselinuxtest.c +++ b/tests/securityselinuxtest.c @@ -332,4 +332,4 @@ mymain(void) return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE; } -VIR_TEST_MAIN_PRELOAD(mymain, abs_builddir "/libsecurityselinuxhelper.so") +VIR_TEST_MAIN_PRELOAD(mymain, "libsecurityselinuxhelper.so") diff --git a/tests/testutils.c b/tests/testutils.c index 8d4e7f84bf..3c5c298293 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -837,8 +837,10 @@ int virTestMain(int argc, va_start(ap, func); while ((lib = va_arg(ap, const char *))) { - if (!virFileIsExecutable(lib)) { - perror(lib); + g_autofree char *abs_lib_path = g_strdup_printf("%s/%s", abs_builddir, lib); + + if (!virFileIsExecutable(abs_lib_path)) { + perror(abs_lib_path); va_end(ap); return EXIT_FAILURE; } diff --git a/tests/testutils.h b/tests/testutils.h index bb84327b1e..e5469c5aa0 100644 --- a/tests/testutils.h +++ b/tests/testutils.h @@ -123,11 +123,13 @@ int virTestMain(int argc, #ifdef __APPLE__ # define PRELOAD_VAR "DYLD_INSERT_LIBRARIES" +# define LD_LIBRARY_PATH "DYLD_LIBRARY_PATH" # define FORCE_FLAT_NAMESPACE \ g_setenv("DYLD_FORCE_FLAT_NAMESPACE", "1", TRUE); # define MOCK_EXT ".dylib" #else # define PRELOAD_VAR "LD_PRELOAD" +# define LD_LIBRARY_PATH "LD_LIBRARY_PATH" # define FORCE_FLAT_NAMESPACE # define MOCK_EXT ".so" #endif @@ -137,12 +139,20 @@ int virTestMain(int argc, const char *preload = getenv(PRELOAD_VAR); \ if (preload == NULL || strstr(preload, libs) == NULL) { \ char *newenv; \ + char *new_library_path; \ + const char *cur_library_path = g_getenv(LD_LIBRARY_PATH); \ if (!preload) { \ newenv = (char *) libs; \ } else { \ newenv = g_strdup_printf("%s:%s", libs, preload); \ } \ + if (!cur_library_path) { \ + new_library_path = (char *) abs_builddir; \ + } else { \ + new_library_path = g_strdup_printf("%s:%s", abs_builddir, cur_library_path); \ + } \ g_setenv(PRELOAD_VAR, newenv, TRUE); \ + g_setenv(LD_LIBRARY_PATH, new_library_path, TRUE); \ FORCE_FLAT_NAMESPACE \ execv(argv[0], argv); \ } \ @@ -153,7 +163,7 @@ int virTestMain(int argc, return virTestMain(argc, argv, func, __VA_ARGS__, NULL); \ } -#define VIR_TEST_MOCK(mock) (abs_builddir "/lib" mock "mock" MOCK_EXT) +#define VIR_TEST_MOCK(mock) ("lib" mock "mock" MOCK_EXT) virCaps *virTestGenericCapsInit(void); virCapsHostNUMA *virTestCapsBuildNUMATopology(int seq); diff --git a/tests/viridentitytest.c b/tests/viridentitytest.c index 73ed9f73df..74e3a03619 100644 --- a/tests/viridentitytest.c +++ b/tests/viridentitytest.c @@ -162,7 +162,7 @@ mymain(void) } #if WITH_SELINUX -VIR_TEST_MAIN_PRELOAD(mymain, abs_builddir "/libsecurityselinuxhelper.so") +VIR_TEST_MAIN_PRELOAD(mymain, "libsecurityselinuxhelper.so") #else VIR_TEST_MAIN(mymain) #endif -- 2.43.2
The virt-aa-helper bash script constructs a path to itself when it runs. But it isn't prepared for the case when there is a space in the path leading to the script (something, something, double quotes, something). Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/virt-aa-helper-test | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/virt-aa-helper-test b/tests/virt-aa-helper-test index 9a97168330..4c8d31c9d7 100755 --- a/tests/virt-aa-helper-test +++ b/tests/virt-aa-helper-test @@ -130,9 +130,9 @@ testme() { echo "': " >$output set +e if [ -n "$input" ]; then - LD_LIBRARY_PATH="$ld_library_path" $exe $extra_args $args < $input >"$tmpout" 2>&1 + LD_LIBRARY_PATH="$ld_library_path" "${exe}" $extra_args $args < $input >"$tmpout" 2>&1 else - LD_LIBRARY_PATH="$ld_library_path" $exe $extra_args $args >"$tmpout" 2>&1 + LD_LIBRARY_PATH="$ld_library_path" "${exe}" $extra_args $args >"$tmpout" 2>&1 fi rc="$?" cat "$tmpout" >"$output" -- 2.43.2
On a Friday in 2024, Michal Privoznik wrote:
The other day somebody on #virt complained about test suite failing on a fresh checkout. Turned out, our mocking and some bash scripts are not prepared for a case when there's a space in builddir path.
After these, there are still some tests failing, but my brain is too small to fix them:
1) Somehow, $builddir/docs/html/libvirt-libvirt-common.html (and friends) are generated into $builddir/html/libvirt-libvirt-common.html and I can't figure out why. This means, check-html and check-html-references tests are failing.
2) There's some problem with po_check as it fails to find generated sources (like src/remote/remote_client_bodies.h). I've taken look into our syntax-check.mk but I have no idea what's wrong nor how to properly escape paths in Makefile.
Michal Prívozník (2): tests: mock: Accept spaces in build path tests: Allow spaces in path to virt-aa-helper
tests/securityselinuxlabeltest.c | 2 +- tests/securityselinuxtest.c | 2 +- tests/testutils.c | 6 ++++-- tests/testutils.h | 12 +++++++++++- tests/viridentitytest.c | 2 +- tests/virt-aa-helper-test | 4 ++-- 6 files changed, 20 insertions(+), 8 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Michal Privoznik