Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Test tweaks #158

Merged
merged 3 commits into from
Mar 12, 2018
Merged

Test tweaks #158

merged 3 commits into from
Mar 12, 2018

Conversation

rossburton
Copy link
Contributor

Two patches:

One to add an option to disable the test suite entirely for eg distro builders that won't run it

The other checks if dlvsym is present as that is a glibc-specific function and the tests fail to build on musl. Note that there's an existing "if not apple" check in all the functions which use dlvsym(), if that is because dlvsym isn't present on apple then I can revise the patch to simplify the logic.

lluixhi added a commit to gentoo/musl that referenced this pull request Mar 1, 2018
test/meson.build Outdated
@@ -1,3 +1,4 @@
has_dlvsym = cc.has_function('dlvsym', prefix : '#include<dlfcn.h>')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you need to also define _GNU_SOURCE, according to the man page for dlvsym:

has_dlvsym = cc.has_function(
  'dlvsym',
  prefix: '''#define _GNU_SOURCE
#include <dlfcn.h>''',
)

@mgorny
Copy link
Contributor

mgorny commented Mar 2, 2018

commit 756dcaf6a5e07ead098457efba15f5be75bf2123
Author: Eric Anholt <eric@anholt.net>
Date:   Sat Mar 29 00:38:19 2014

    Disable the dlwrap-based tests on apple.
    
    We'd need a dlvsym() equivalent.

So yes, please simplify the logic ;-).

By the way, the same file also uses unportable asprintf(), so you may want to check for both.

@mgorny
Copy link
Contributor

mgorny commented Mar 2, 2018

Finally, the whole purpose of that file is to find dlsym() with glibc-specific version, so you could just disable it completely on any non-glibc system because it will fail to find it anyway.

@rossburton rossburton force-pushed the master branch 3 times, most recently from afb5d18 to c8a5284 Compare March 2, 2018 17:02
@rossburton
Copy link
Contributor Author

Pushed revised commits.

Passing GNU_SOURCE isn't required as has_function doesn't care about headers, but I do need to find libdl as a dependency first.

Removed all traces of build_apple.

@ebassi
Copy link
Collaborator

ebassi commented Mar 6, 2018

Thanks, @rossburton. One last hurdle: as much as I'd like to ignore the Autotools build, we're still supporting it, so BUILD_APPLE needs to go away from configure.ac and tests/Makefile.am as well.

@rossburton
Copy link
Contributor Author

Another patch pushed. You owe me 🍺 for making me wrestle autoconf after the glory of meson. 😉

build_apple was introduced in 756dca as a proxy for the fact that Apple's libc
doesn't have dlvsym(), which is glibc-specific so also isn't present in other
libc implementations such as musl.

Instead of detecting whether we are building for Apple or not, just probe the to
see if we have dlvsym.
As per the previous commit, instead of assuming that Apple doesn't have dlvsym
but everywhere else does, actually check for dlvsym() existing as that function
is glibc-specific.
@ebassi ebassi merged commit 6769ba2 into anholt:master Mar 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants