-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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 cases/linuxlike/14 static dynamic linkage: Add matrix by static #13842
Open
dememax
wants to merge
1
commit into
mesonbuild:master
Choose a base branch
from
dememax:optional-zlib-linkage-test
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
52 changes: 25 additions & 27 deletions
52
test cases/linuxlike/14 static dynamic linkage/meson.build
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,36 +1,34 @@ | ||
# This test uses zlib to verify static and dynamic linkages. | ||
# A C program is used to call a single zlib function (zlibVersion). | ||
# The nm utility is then used to check for the existence of this symbol | ||
# in the resulting binary. | ||
|
||
project('static dynamic', 'c') | ||
|
||
# Solaris does not ship static libraries | ||
if host_machine.system() == 'sunos' | ||
has_static = false | ||
else | ||
has_static = true | ||
s = get_option('static') | ||
|
||
if s and host_machine.system() == 'sunos' | ||
error('MESON_SKIP_TEST: static zlib linkage is not supported on SunOS by default') | ||
endif | ||
|
||
cc = meson.get_compiler('c') | ||
|
||
z_default = cc.find_library('z') | ||
if has_static | ||
z_static = cc.find_library('z', static: true) | ||
endif | ||
z_dynamic = cc.find_library('z', static: false) | ||
z = cc.find_library('z', static: s) | ||
|
||
exe_default = executable('main_default', 'main.c', dependencies: [z_default]) | ||
if has_static | ||
exe_static = executable('main_static', 'main.c', dependencies: [z_static]) | ||
endif | ||
exe_dynamic = executable('main_dynamic', 'main.c', dependencies: [z_dynamic]) | ||
exe = executable('print_zlib_version', 'main.c', dependencies: [z]) | ||
|
||
test('test default', exe_default) | ||
if has_static | ||
test('test static', exe_static) | ||
endif | ||
test('test dynamic', exe_dynamic) | ||
# first step: the executable should compile and work | ||
test('test zlib', exe) | ||
|
||
if has_static | ||
test('verify static linking', find_program('verify_static.py'), | ||
args: ['--platform=' + host_machine.system(), exe_static.full_path()]) | ||
endif | ||
test('verify dynamic linking', find_program('verify_static.py'), | ||
args: ['--platform=' + host_machine.system(), exe_dynamic.full_path()], | ||
should_fail: true) | ||
# to check the zlib static/dynamic symbols in the resulting binary | ||
find_program('nm') | ||
|
||
# second step: static linkage | ||
test('verify static zlib linking', find_program('verify_zlib_linkage.py'), | ||
args: ['--platform=' + host_machine.system(), '--static', exe.full_path()], | ||
should_fail: not s) | ||
|
||
# third step: dynamic linkage | ||
test('verify dynamic zlib linking', find_program('verify_zlib_linkage.py'), | ||
args: ['--platform=' + host_machine.system(), exe.full_path()], | ||
should_fail: s) |
1 change: 1 addition & 0 deletions
1
test cases/linuxlike/14 static dynamic linkage/meson_options.txt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
option('static', type: 'boolean', value: false) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
{ | ||
"matrix": { | ||
"options": { | ||
"static": [ | ||
{ "val": "true", "skip_on_env": [ "SKIP_STATIC_ZLIB" ] }, | ||
{ "val": "false" } | ||
] | ||
} | ||
} | ||
} |
37 changes: 0 additions & 37 deletions
37
test cases/linuxlike/14 static dynamic linkage/verify_static.py
This file was deleted.
Oops, something went wrong.
49 changes: 49 additions & 0 deletions
49
test cases/linuxlike/14 static dynamic linkage/verify_zlib_linkage.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
#!/usr/bin/env python3 | ||
"""Test script that checks if zlib was statically or dynamically linked to executable""" | ||
import subprocess | ||
import sys | ||
import argparse | ||
|
||
def check_zlib_symbol_common(path, is_static): | ||
"""Tests if the binary contains zlibVersion symbol (non-Cygwin version).""" | ||
try: | ||
sym_opt = '--defined-only' if is_static else '--undefined-only' | ||
output = subprocess.check_output(['nm', sym_opt, '-P', '-A', path]).decode('utf-8') | ||
except subprocess.CalledProcessError: | ||
# some NMs only support -U. Older binutils only supports --defined-only. | ||
opts = '-UPA' if is_static else '-uPA' | ||
output = subprocess.check_output(['nm', opts, path]).decode('utf-8') | ||
# POSIX format. Prints all *defined* symbols, looks like this: | ||
# builddir/main_static: zlibVersion T 1190 39 | ||
# or | ||
# builddir/main_static: zlibVersion D 1fde0 30 | ||
if ': zlibVersion ' in output: | ||
return 0 | ||
return 1 | ||
|
||
def check_zlib_symbol_cygwin(path, is_static): | ||
"""Tests if the binary contains zlibVersion symbol (Cygwin case).""" | ||
output = subprocess.check_output(['nm', path]).decode('utf-8') | ||
# No matter static or dynamic, the name must exist in nm output | ||
if ' zlibVersion' not in output: | ||
return 2 | ||
is_dynamic = ('I __imp_zlibVersion' in output) or ('D __imp_zlibVersion' in output) | ||
if is_dynamic == is_static: # expected/got mismatch? | ||
return 3 | ||
return 0 | ||
|
||
def main(): | ||
"""Main function""" | ||
parser = argparse.ArgumentParser() | ||
parser.add_argument('path', help='executable path') | ||
parser.add_argument('-p', '--platform') | ||
parser.add_argument('-s', '--static', action='store_true', default=False) | ||
args = parser.parse_args() | ||
if args.platform == 'cygwin': | ||
return check_zlib_symbol_cygwin(args.path, args.static) | ||
else: | ||
return check_zlib_symbol_common(args.path, args.static) | ||
|
||
|
||
if __name__ == '__main__': | ||
sys.exit(main()) |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably still explicitly skip on Solaris. Your new approach would cause test failure if SKIP_STATIC_ZLIB is not exported, even though we know that
if s and host_machine.system() == 'sunos'
there will not be a static library.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are perfectly right that this changes the behavior, and it's better to keep the previous logic if we don't want to break something. Nice catch!
I will do what you are asking, but let's go a little farther...
When my new approach cause test failure
As far as I know, there are no SunOS tests in the current CI. And, we both know, the green status is showing this.
So there should be some test cases outside the project, the dark matter, where people will have this issue.
When my new approach fixes problem where it could possibly cause test failure
AFAIK, SunOS could be configured to have the static zlib version in addition to the default dynamic one. In this case, keeping the old condition is a bug.
Let's ask the author of those lines @alanc
With my approach the user of this test case will be able to change his environment to adopt the usage on SunOS for his/her setup.
Symmetry / Special case
What is so special about SunOS to include it into this build script?
What about the default configuration on MS Windows (no Cygwin) or MacOS?
What about IBM i (AS/400) and BSD families?
Gentoo setup is configured for Boost with the help of SKIP_STATIC_BOOST in the GHA yaml file. Why could not SunOS be configured in the same way?
Bottom line
As I said, I will fix this to keep this special case for SunOS, but IMHO it must go.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While there is no SunOS (Solaris or illumos) instance in the CI, we run the test suite on Solaris when building the meson packages for Solaris. It is true that it is not impossible for a user to build and install a static version of libz on Solaris - but since we provide a dynamic library for libz as part of the OS packages, I don't know of anyone who bothers to build their own static version. I can't speak to other OS'es or why they didn't hit the problem - perhaps their default configuration doesn't have dynamic libz either instead of just dynamic and not static.
I'm not opposed to moving this from a line of code to a configuration file - what's the "GHA yaml file"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For all our docker-based CI testing we generate the docker images themselves with
/ci/env_vars.sh
as a kind of "/etc/profile for CI".For Cygwin, which isn't docker-based, we use the GitHub Actions (GHA) yaml file
.github/workflows/cygwin.yml
. We don't maintain any such thing for Solaris' packaging.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @alanc !
@eli-schwartz, should we throw away the SunOS special condition, or merge the PR in the current state?