Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add --disable-tests option #475

Merged
merged 1 commit into from
Mar 23, 2023
Merged

Add --disable-tests option #475

merged 1 commit into from
Mar 23, 2023

Conversation

ffontaine
Copy link
Contributor

Add --disable-tests to allow the user to disable tests. As a side-effect, this will avoid the following build failure when check is found:

libstat_wrapper.c:11:10: fatal error: gnu/lib-names.h: No such file or directory
   11 | #include <gnu/lib-names.h>
      |          ^~~~~~~~~~~~~~~~~

This build failure is raised since version 2.0.5 and 78df90b

Fixes:

Signed-off-by: Fabrice Fontaine fontaine.fabrice@gmail.com

Add --disable-tests to allow the user to disable tests. As a
side-effect, this will avoid the following build failure when check is
found:

libstat_wrapper.c:11:10: fatal error: gnu/lib-names.h: No such file or directory
   11 | #include <gnu/lib-names.h>
      |          ^~~~~~~~~~~~~~~~~

This build failure is raised since version 2.0.5 and
78df90b

Fixes:
 - http://autobuild.buildroot.org/results/450cfc36d4fd6dc71c138bec45f05b5a2d92a08d

Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
@knet-ci-bot
Copy link

Can one of the admins verify this patch?

@fabbione
Copy link
Member

fabbione commented Nov 4, 2022

ok to test

@fabbione
Copy link
Member

fabbione commented Nov 4, 2022

Add --disable-tests to allow the user to disable tests. As a side-effect, this will avoid the following build failure when check is found:

I have no objections to add --disable-tests, tho:

libstat_wrapper.c:11:10: fatal error: gnu/lib-names.h: No such file or directory
   11 | #include <gnu/lib-names.h>
      |          ^~~~~~~~~~~~~~~~~

This build failure is raised since version 2.0.5 and 78df90b

this part I don´t understand. We run CI on each build and we have never seen this problem before.

What distribution / container / platform are you building on? Is the version of check too old?

@ffontaine
Copy link
Contributor Author

ffontaine commented Nov 4, 2022

The build failure is raised by buildroot autobuilders.
check is in version 0.15.2 but check doesn't provide gnu/lib-names.h.
This file is glibc-specific, it will raise the above build failure with uclibc(-ng) and musl.

@chrissie-c
Copy link
Contributor

I think it would be better just to disable that test. It's a good thing to test libqb against other libcs where possible.

@ffontaine
Copy link
Contributor Author

Indeed, you could disable or remove this test however it still makes sense to disable all tests to save build time and disk space.

@fabbione
Copy link
Member

fabbione commented Nov 4, 2022

The build failure is raised by buildroot autobuilders. check is in version 0.15.2 but check doesn't provide gnu/lib-names.h. This file is glibc-specific, it will raise the above build failure with uclibc(-ng) and musl.

Apologize for my complete ignorance, but what is buildroot.org?

+1 on disabling the test when gnu/lib-names.h is not available.

Regardless of the libc is in use, test suite should be built and executed. Build only doesn´t help much for the final results. Shaving 3/4 seconds to build is meaningless if the result doesn´t work on the libc in use.

@tpetazzoni
Copy link

Why is gnu/lib-names.h even included? I don't see anything from this header file that gets used in libstat_wrapper.c. Removing the gnu/lib-names.h solves the build issue.

@chrissie-c chrissie-c merged commit 1a32a60 into ClusterLabs:main Mar 23, 2023
@chrissie-c
Copy link
Contributor

I'll remove it. I suspect it was needed for some earlier OS version, but without that it's passed all the CI tests, so it's obviously fine without!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants