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

Meson test api c files build fix #2259

Merged
merged 3 commits into from
Mar 14, 2020

Conversation

tp-m
Copy link
Contributor

@tp-m tp-m commented Mar 14, 2020

commit 1322eba:

[meson] use add_project_arguments() instead of add_global_arguments()
.. and simplify, can pass two languages in one go.

add_global_arguments() won't work if harfbuzz is used as a
meson subproject.

commit 7c24cc3:

[meson] fix spurious warning when building test/api C sources
Fixes compiler warning

  test-unicode.c:589:1: warning: ‘test_unicode_properties_lenient’ defined but not used

which didn't happen with autotools.

Reason it does with meson is that the setup for C was slightly wrong.
We would only add -DHAVE_CONFIG_H to cpp_args which is only valid when
compiling C++ code, but not plain C code, and many of these tests were
plain C.

Instead pass -DHAVE_CONFIG_H via add_project_arguments() and make sure
to set both c_args and cpp_args when building test executables.

Fixes https://github.com/harfbuzz/harfbuzz/issues/2257

.. and simplify, can pass two languages in one go.

add_global_arguments() won't work if harfbuzz is used as a
meson subproject.
Fixes compiler warning

  test-unicode.c:589:1: warning: ‘test_unicode_properties_lenient’ defined but not used

which didn't happen with autotools.

Reason it does with meson is that the setup for C was slightly wrong.
We would only add -DHAVE_CONFIG_H to cpp_args which is only valid when
compiling C++ code, but not plain C code, and many of these tests were
plain C.

Instead pass -DHAVE_CONFIG_H via add_project_arguments() and make sure
to set both c_args and cpp_args when building test executables.

Fixes harfbuzz#2257
@ebraminio
Copy link
Collaborator

Our meson-gcc bot https://github.com/harfbuzz/harfbuzz/blob/master/.circleci/config.yml#L227-L238 fails https://app.circleci.com/pipelines/github/harfbuzz/harfbuzz/jobs/133013 with the changes apparently, can you have a look? Thanks :)

@ebraminio ebraminio added the meson meson build system label Mar 14, 2020
@tp-m
Copy link
Contributor Author

tp-m commented Mar 14, 2020

Can reproduce the CI failure locally with -Damalgam=true, will take a look.

@tp-m tp-m force-pushed the meson-test-api-c-files-build-fix branch 2 times, most recently from efba55a to f329f1d Compare March 14, 2020 11:20
test-unicode.c:960: undefined reference to `hb_icu_get_unicode_funcs'
test-unicode.c:961: undefined reference to `hb_icu_get_unicode_funcs'

For now add the icu sources to libharfbuzz also for the amalgam
build, later we need to have a separate harfbuzz-icu module and
link against that, and/or generate harfbuzz.cc.
@tp-m tp-m force-pushed the meson-test-api-c-files-build-fix branch from f329f1d to cfadb01 Compare March 14, 2020 11:32
@ebraminio ebraminio merged commit 93b3e30 into harfbuzz:master Mar 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meson meson build system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants