[PATCH 0/1] gnu: vulkan-loader: Skip x86-specific tests on non-x86 platforms.

  • Open
  • quality assurance status badge
Details
2 participants
  • Efraim Flashner
  • Simon South
Owner
unassigned
Submitted by
Simon South
Severity
normal
S
S
Simon South wrote on 9 May 2023 22:43
(address . guix-patches@gnu.org)
cover.1683661648.git.simon@simonsouth.net
This patch fixes the build of vulkan-loader on aarch64-linux by
causing five tests that require an x86 or x86-64 host to be skipped.

I've tested this change on x86-64 (to confirm the patch has no real
effect) and AArch64 and everything seems fine.

The tests that are skipped use pre-built, dummy libraries supplied in
the source package to check the loader's ability to detect a request
for a 32-bit library on a 64-bit system, and vice-versa. This is done
by checking for a specific error message generated by a call to
dlopen(). The dummy libraries provided are built only for x86 and
x86-64, though, so on other platforms dlopen() produces a different
error ("No such file" instead of "wrong ELF class") and the tests
fail.

The source package supplies a makefile that could be used to
regenerate these dummy libraries, but as written the makefile assumes
that

- gcc recognizes "-m32" and "-m64" options to select a target
subarchitecture, which is true on x86 and PowerPC but not on
AArch64 or RISC-V, for instance; and that

- The system's linker is able to output libraries for multiple
architectures, requiring (among other things) that multiple
versions of glibc be available, and even using "cross-gcc" and
"cross-libc" I haven't found a straightforward way to achieve this
within a single package build.

So I expect modifying things to make these tests work on other
platforms would be more trouble than it's worth.

---

Simon South (1):
gnu: vulkan-loader: Skip x86-specific tests on non-x86 platforms.

gnu/local.mk | 1 +
...ulkan-loader-skip-incompatible-tests.patch | 62 +++++++++++++++++++
gnu/packages/vulkan.scm | 2 +
3 files changed, 65 insertions(+)
create mode 100644 gnu/packages/patches/vulkan-loader-skip-incompatible-tests.patch


base-commit: c1ffe2f21bd1b9ba6bd527bbabe130144a69af71
--
2.39.2
S
S
Simon South wrote on 9 May 2023 22:45
[PATCH 1/1] gnu: vulkan-loader: Skip x86-specific tests on non-x86 platforms.
(address . 63400@debbugs.gnu.org)
1f4f4283b03ab0020443a9e23c01dd840819e1f0.1683661648.git.simon@simonsouth.net
* gnu/packages/patches/vulkan-loader-skip-incompatible-tests.patch: New file.
* gnu/local.mk (dist_patch_DATA): Add it.
* gnu/packages/vulkan.scm (vulkan-loader)[source]: Apply it.
---
gnu/local.mk | 1 +
...ulkan-loader-skip-incompatible-tests.patch | 62 +++++++++++++++++++
gnu/packages/vulkan.scm | 2 +
3 files changed, 65 insertions(+)
create mode 100644 gnu/packages/patches/vulkan-loader-skip-incompatible-tests.patch

Toggle diff (95 lines)
diff --git a/gnu/local.mk b/gnu/local.mk
index 77abf61d3c..b431e4f8ce 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -2031,6 +2031,7 @@ dist_patch_DATA = \
%D%/packages/patches/vtk-7-gcc-10-compat.patch \
%D%/packages/patches/vtk-7-hdf5-compat.patch \
%D%/packages/patches/vtk-7-python-compat.patch \
+ %D%/packages/patches/vulkan-loader-skip-incompatible-tests.patch \
%D%/packages/patches/wacomtablet-add-missing-includes.patch \
%D%/packages/patches/wacomtablet-qt5.15.patch \
%D%/packages/patches/warsow-qfusion-fix-bool-return-type.patch \
diff --git a/gnu/packages/patches/vulkan-loader-skip-incompatible-tests.patch b/gnu/packages/patches/vulkan-loader-skip-incompatible-tests.patch
new file mode 100644
index 0000000000..92e3d03bb6
--- /dev/null
+++ b/gnu/packages/patches/vulkan-loader-skip-incompatible-tests.patch
@@ -0,0 +1,62 @@
+Certain tests that use the pre-built libraries provided in
+tests/framework/data/binaries presume they are running on a 32- or
+64-bit x86 host, as these are the only architectures for which the
+libraries are provided.
+
+Skip these tests on non-x86 platforms where the tests are certain to
+fail.
+
+diff --git a/tests/loader_regression_tests.cpp b/tests/loader_regression_tests.cpp
+index 7390596bd..1b0780c65 100644
+--- a/tests/loader_regression_tests.cpp
++++ b/tests/loader_regression_tests.cpp
+@@ -1014,6 +1014,9 @@ TEST(CreateDevice, ConsecutiveCreateWithoutDestruction) {
+ }
+
+ TEST(TryLoadWrongBinaries, WrongICD) {
++#if !defined(__x86_64__) && !defined(__i386__)
++ GTEST_SKIP() << "Skip this test as it is not compatible with this architecture.";
++#endif
+ FrameworkEnvironment env{};
+ env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2));
+ env.add_icd(TestICDDetails(CURRENT_PLATFORM_DUMMY_BINARY_WRONG_TYPE).set_is_fake(true));
+@@ -1041,6 +1044,9 @@ TEST(TryLoadWrongBinaries, WrongICD) {
+ }
+
+ TEST(TryLoadWrongBinaries, WrongExplicit) {
++#if !defined(__x86_64__) && !defined(__i386__)
++ GTEST_SKIP() << "Skip this test as it is not compatible with this architecture.";
++#endif
+ FrameworkEnvironment env{};
+ env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2));
+ env.get_test_icd().physical_devices.emplace_back("physical_device_0");
+@@ -1077,6 +1083,9 @@ TEST(TryLoadWrongBinaries, WrongExplicit) {
+ }
+
+ TEST(TryLoadWrongBinaries, WrongImplicit) {
++#if !defined(__x86_64__) && !defined(__i386__)
++ GTEST_SKIP() << "Skip this test as it is not compatible with this architecture.";
++#endif
+ FrameworkEnvironment env{};
+ env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2));
+ env.get_test_icd().physical_devices.emplace_back("physical_device_0");
+@@ -1114,6 +1123,9 @@ TEST(TryLoadWrongBinaries, WrongImplicit) {
+ }
+
+ TEST(TryLoadWrongBinaries, WrongExplicitAndImplicit) {
++#if !defined(__x86_64__) && !defined(__i386__)
++ GTEST_SKIP() << "Skip this test as it is not compatible with this architecture.";
++#endif
+ FrameworkEnvironment env{};
+ env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2));
+ env.get_test_icd().physical_devices.emplace_back("physical_device_0");
+@@ -1159,6 +1171,9 @@ TEST(TryLoadWrongBinaries, WrongExplicitAndImplicit) {
+ }
+
+ TEST(TryLoadWrongBinaries, WrongExplicitAndImplicitErrorOnly) {
++#if !defined(__x86_64__) && !defined(__i386__)
++ GTEST_SKIP() << "Skip this test as it is not compatible with this architecture.";
++#endif
+ FrameworkEnvironment env{};
+ env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2));
+ env.get_test_icd().physical_devices.emplace_back("physical_device_0");
diff --git a/gnu/packages/vulkan.scm b/gnu/packages/vulkan.scm
index b0d968938b..e9f287564c 100644
--- a/gnu/packages/vulkan.scm
+++ b/gnu/packages/vulkan.scm
@@ -222,6 +222,8 @@ (define-public vulkan-loader
(url "https://github.com/KhronosGroup/Vulkan-Loader")
(commit "v1.3.232")))
(file-name (git-file-name name version))
+ (patches
+ (search-patches "vulkan-loader-skip-incompatible-tests.patch"))
(sha256
(base32
"0w69sh669sx9pwlvv2rv92ds2hm2rbzsa6qqcmd8kcad0qfq7dz2"))))
--
2.39.2
E
E
Efraim Flashner wrote on 15 May 2023 21:02
Re: [bug#63400] [PATCH 0/1] gnu: vulkan-loader: Skip x86-specific tests on non-x86 platforms.
(name . Simon South)(address . simon@simonsouth.net)(address . 63400@debbugs.gnu.org)
ZGKB4A6xvaEPwYwv@3900XT
On Tue, May 09, 2023 at 04:43:27PM -0400, Simon South wrote:
Toggle quote (3 lines)
> This patch fixes the build of vulkan-loader on aarch64-linux by
> causing five tests that require an x86 or x86-64 host to be skipped.

I have to admit that I didn't check the bug tracker before I pushed my
change (which mirrored Chris's earlier patch) to just skip the tests
entirely on a bunch of architectures.

Toggle quote (12 lines)
> I've tested this change on x86-64 (to confirm the patch has no real
> effect) and AArch64 and everything seems fine.
>
> The tests that are skipped use pre-built, dummy libraries supplied in
> the source package to check the loader's ability to detect a request
> for a 32-bit library on a 64-bit system, and vice-versa. This is done
> by checking for a specific error message generated by a call to
> dlopen(). The dummy libraries provided are built only for x86 and
> x86-64, though, so on other platforms dlopen() produces a different
> error ("No such file" instead of "wrong ELF class") and the tests
> fail.

Another option would be to patch the tests to look for these strings
instead when we're building on those architectures.

Toggle quote (14 lines)
> The source package supplies a makefile that could be used to
> regenerate these dummy libraries, but as written the makefile assumes
> that
>
> - gcc recognizes "-m32" and "-m64" options to select a target
> subarchitecture, which is true on x86 and PowerPC but not on
> AArch64 or RISC-V, for instance; and that
>
> - The system's linker is able to output libraries for multiple
> architectures, requiring (among other things) that multiple
> versions of glibc be available, and even using "cross-gcc" and
> "cross-libc" I haven't found a straightforward way to achieve this
> within a single package build.

This also seems to be above-and-beyond for 5 tests out of ~400.

Toggle quote (3 lines)
> So I expect modifying things to make these tests work on other
> platforms would be more trouble than it's worth.

Having looked at these myself I agree that it's non-trivial to skip the
failing tests on the other architectures. Debian, for instance, runs the
tests, but if they fail ignores the outcome, and prints to the build log
that 23 tests are expected to fail.

Toggle quote (20 lines)
> ---
>
> Simon South (1):
> gnu: vulkan-loader: Skip x86-specific tests on non-x86 platforms.
>
> gnu/local.mk | 1 +
> ...ulkan-loader-skip-incompatible-tests.patch | 62 +++++++++++++++++++
> gnu/packages/vulkan.scm | 2 +
> 3 files changed, 65 insertions(+)
> create mode 100644 gnu/packages/patches/vulkan-loader-skip-incompatible-tests.patch
>
>
> base-commit: c1ffe2f21bd1b9ba6bd527bbabe130144a69af71
> --
> 2.39.2
>
>
>
>

--
Efraim Flashner <efraim@flashner.co.il> ????? ?????
GPG key = A28B F40C 3E55 1372 662D 14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted
-----BEGIN PGP SIGNATURE-----

iQIyBAABCAAdFiEEoov0DD5VE3JmLRT3Qarn3Mo9g1EFAmRigd0ACgkQQarn3Mo9
g1Gg2Q/46Adm3kEpzat8BHmC+yycNNKo6QRYxhQR/W5aosLqwJqHYPgCg+JOWPuX
LEDJ+fAY7/oq95G4YhM7fiNJuw2pFLJnxXKzXA03fWflFqf0Ewe3C+lCCJWFzVl8
7ABz1A2QQ3itcpYKahmit3tTYRz0qkQqASuTF4izWOkrGiPfD86n5znO4KF2oBA6
uZ6R5T+B7eESbCDuBBzd5r+NnrlPyfRzPLRfBLEPz88QRVq2zmZCAICtwZTkiZEf
DykW/fPlXmV8uTXidqGnqSiHDaMErY61DDUO3GMK8cS4owF9iidxP/hlOMfyBcmb
N4GGl14cWzN/i80ygnoKuqhouVUni9iqce3yh/Dq7bUKEAUPB6Z/VT3ViRWRT13e
RHgIRQieZVHwzZoz1h45leIOFcoAcDN5ZScbREmEY5kSTbN/tiu2CKj7InfJCZdX
YACbg8gFoM/IL82QfI85W+owNwpqhtw6jlpwztPmhcnIQV/mduSipRQMm+fMf0qr
/90RbkERMqFMVda6p5fcpjFBK4DYlOCtlZkvhXPbaFElOzpLJNtpmRJD69NgaGZI
ooY/nZbssscVIBYSd6clVYzKD0GYRWgoIkigDOiIjScTKlxh/TTB6Tlm98IjH1eJ
bs+Qg1OToCm8Skhq/lMmCnArvERoYaU3urdRbqEZsLaqd/ftgw==
=cH7o
-----END PGP SIGNATURE-----


S
S
Simon South wrote on 4 Jun 2023 15:11
[PATCH v2 1/2] gnu: vulkan-loader: Skip x86-specific tests on non-x86 platforms.
(address . 63400@debbugs.gnu.org)
0640e02e68cea6ccbacba3b5b436cc0d722497d0.1685883140.git.simon@simonsouth.net
* gnu/packages/patches/vulkan-loader-skip-incompatible-tests.patch: New file.
* gnu/local.mk (dist_patch_DATA): Add it.
* gnu/packages/vulkan.scm (vulkan-loader)[source]: Apply it.
---
gnu/local.mk | 1 +
...ulkan-loader-skip-incompatible-tests.patch | 62 +++++++++++++++++++
gnu/packages/vulkan.scm | 2 +
3 files changed, 65 insertions(+)
create mode 100644 gnu/packages/patches/vulkan-loader-skip-incompatible-tests.patch

Toggle diff (95 lines)
diff --git a/gnu/local.mk b/gnu/local.mk
index 9adf593318..8da6599110 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -2047,6 +2047,7 @@ dist_patch_DATA = \
%D%/packages/patches/vtk-7-gcc-10-compat.patch \
%D%/packages/patches/vtk-7-hdf5-compat.patch \
%D%/packages/patches/vtk-7-python-compat.patch \
+ %D%/packages/patches/vulkan-loader-skip-incompatible-tests.patch \
%D%/packages/patches/wacomtablet-add-missing-includes.patch \
%D%/packages/patches/wacomtablet-qt5.15.patch \
%D%/packages/patches/warsow-qfusion-fix-bool-return-type.patch \
diff --git a/gnu/packages/patches/vulkan-loader-skip-incompatible-tests.patch b/gnu/packages/patches/vulkan-loader-skip-incompatible-tests.patch
new file mode 100644
index 0000000000..92e3d03bb6
--- /dev/null
+++ b/gnu/packages/patches/vulkan-loader-skip-incompatible-tests.patch
@@ -0,0 +1,62 @@
+Certain tests that use the pre-built libraries provided in
+tests/framework/data/binaries presume they are running on a 32- or
+64-bit x86 host, as these are the only architectures for which the
+libraries are provided.
+
+Skip these tests on non-x86 platforms where the tests are certain to
+fail.
+
+diff --git a/tests/loader_regression_tests.cpp b/tests/loader_regression_tests.cpp
+index 7390596bd..1b0780c65 100644
+--- a/tests/loader_regression_tests.cpp
++++ b/tests/loader_regression_tests.cpp
+@@ -1014,6 +1014,9 @@ TEST(CreateDevice, ConsecutiveCreateWithoutDestruction) {
+ }
+
+ TEST(TryLoadWrongBinaries, WrongICD) {
++#if !defined(__x86_64__) && !defined(__i386__)
++ GTEST_SKIP() << "Skip this test as it is not compatible with this architecture.";
++#endif
+ FrameworkEnvironment env{};
+ env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2));
+ env.add_icd(TestICDDetails(CURRENT_PLATFORM_DUMMY_BINARY_WRONG_TYPE).set_is_fake(true));
+@@ -1041,6 +1044,9 @@ TEST(TryLoadWrongBinaries, WrongICD) {
+ }
+
+ TEST(TryLoadWrongBinaries, WrongExplicit) {
++#if !defined(__x86_64__) && !defined(__i386__)
++ GTEST_SKIP() << "Skip this test as it is not compatible with this architecture.";
++#endif
+ FrameworkEnvironment env{};
+ env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2));
+ env.get_test_icd().physical_devices.emplace_back("physical_device_0");
+@@ -1077,6 +1083,9 @@ TEST(TryLoadWrongBinaries, WrongExplicit) {
+ }
+
+ TEST(TryLoadWrongBinaries, WrongImplicit) {
++#if !defined(__x86_64__) && !defined(__i386__)
++ GTEST_SKIP() << "Skip this test as it is not compatible with this architecture.";
++#endif
+ FrameworkEnvironment env{};
+ env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2));
+ env.get_test_icd().physical_devices.emplace_back("physical_device_0");
+@@ -1114,6 +1123,9 @@ TEST(TryLoadWrongBinaries, WrongImplicit) {
+ }
+
+ TEST(TryLoadWrongBinaries, WrongExplicitAndImplicit) {
++#if !defined(__x86_64__) && !defined(__i386__)
++ GTEST_SKIP() << "Skip this test as it is not compatible with this architecture.";
++#endif
+ FrameworkEnvironment env{};
+ env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2));
+ env.get_test_icd().physical_devices.emplace_back("physical_device_0");
+@@ -1159,6 +1171,9 @@ TEST(TryLoadWrongBinaries, WrongExplicitAndImplicit) {
+ }
+
+ TEST(TryLoadWrongBinaries, WrongExplicitAndImplicitErrorOnly) {
++#if !defined(__x86_64__) && !defined(__i386__)
++ GTEST_SKIP() << "Skip this test as it is not compatible with this architecture.";
++#endif
+ FrameworkEnvironment env{};
+ env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2));
+ env.get_test_icd().physical_devices.emplace_back("physical_device_0");
diff --git a/gnu/packages/vulkan.scm b/gnu/packages/vulkan.scm
index 1d2e58f1d4..790a1c7c07 100644
--- a/gnu/packages/vulkan.scm
+++ b/gnu/packages/vulkan.scm
@@ -222,6 +222,8 @@ (define-public vulkan-loader
(url "https://github.com/KhronosGroup/Vulkan-Loader")
(commit "v1.3.232")))
(file-name (git-file-name name version))
+ (patches
+ (search-patches "vulkan-loader-skip-incompatible-tests.patch"))
(sha256
(base32
"0w69sh669sx9pwlvv2rv92ds2hm2rbzsa6qqcmd8kcad0qfq7dz2"))))
--
2.39.1
S
S
Simon South wrote on 4 Jun 2023 15:11
[PATCH v2 0/2] gnu: vulkan-loader: Skip x86-specific tests on non-x86 platforms.
(address . 63400@debbugs.gnu.org)
cover.1685883140.git.simon@simonsouth.net
Here's a v2 of this patch (now series) that's updated to match the code in
master. These changes now

- Patch the vulkan-loader source to disable x86-specific tests on non-x86
platforms,

- Re-enable the test suite on AArch64 (as it is now safe to do so) but

- Leave tests disabled on all other non-x86 platforms.

I've tested this on AArch64 and x86-64 and vulkan-loader builds correctly for
me on both platforms.

Thoughts?

--
Simon South
simon@simonsouth.net

---

Simon South (2):
gnu: vulkan-loader: Skip x86-specific tests on non-x86 platforms.
gnu: vulkan-loader: Re-enable tests on AArch64.

gnu/local.mk | 1 +
...ulkan-loader-skip-incompatible-tests.patch | 62 +++++++++++++++++++
gnu/packages/vulkan.scm | 4 +-
3 files changed, 66 insertions(+), 1 deletion(-)
create mode 100644 gnu/packages/patches/vulkan-loader-skip-incompatible-tests.patch


base-commit: d84517805080ec51bab1921ac34c28adf3d88596
--
2.39.1
S
S
Simon South wrote on 4 Jun 2023 15:11
[PATCH v2 2/2] gnu: vulkan-loader: Re-enable tests on AArch64.
(address . 63400@debbugs.gnu.org)
b19ea98f1f24cbeb7b058cd9271cc891c0d89ba1.1685883140.git.simon@simonsouth.net
* gnu/packages/vulkan.scm (vulkan-loader)[arguments]<#:tests?>: Enable on
AArch64.
---
gnu/packages/vulkan.scm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

Toggle diff (15 lines)
diff --git a/gnu/packages/vulkan.scm b/gnu/packages/vulkan.scm
index 790a1c7c07..fd6bd57602 100644
--- a/gnu/packages/vulkan.scm
+++ b/gnu/packages/vulkan.scm
@@ -233,7 +233,7 @@ (define-public vulkan-loader
;; As many as 23 tests are expected to fail per architecture.
;; Limit the tests to those architectures tested upstream.
#:tests? (and (%current-system)
- (target-x86?))
+ (or (target-aarch64?) (target-x86?)))
#:configure-flags
#~(list (string-append "-DVULKAN_HEADERS_INSTALL_DIR="
(dirname (dirname
--
2.39.1
?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 63400@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 63400
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch