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

Add -DFRIBIDI_LIB_STATIC to libfribidi_dep #187

Merged
merged 1 commit into from Apr 6, 2022
Merged

Conversation

xclaesse
Copy link
Contributor

It is needed when fribidi is used as subproject and is static linked on
Windows. The pkg-config file already contains it for the same reason.

It is needed when fribidi is used as subproject and is static linked on
Windows. The pkg-config file already contains it for the same reason.
@xclaesse
Copy link
Contributor Author

CC @nirbheek

@nirbheek
Copy link
Contributor

nirbheek commented Apr 6, 2022

LGTM.

@xclaesse
Copy link
Contributor Author

xclaesse commented Apr 6, 2022

Thanks but I don't have merge permission, do you?

@dov
Copy link
Contributor

dov commented Apr 6, 2022

Any idea of why one of the build tests failed? Is the patching causing the build to be static by default? Is this the common convention?

@tp-m
Copy link
Contributor

tp-m commented Apr 6, 2022

Any idea of why one of the build tests failed? Is the patching causing the build to be static by default? Is this the common convention?

It looks like this is an issue that's unrelated to this PR?

It looks like it's the autotools distcheck that fails (run_tests.sh in particular).

Unfortunately I don't see any useful output in the build log with more information what failed exactly, nor are there any artefacts like test logs uploaded by the ci.

I don't see how anything in this PR which only modifies the Meson build description could possibly cause this (unless you do a silent meson build under the hood from the autotools test hooks, which I don't think you do afaict).

Is the patching causing the build to be static by default? Is this the common convention?

The patch doesn't change whether the lib is built statically or dynamically by default in meson, it just makes sure that if it was configured to be built statically then consumers of the meson dependency (that consume fribidi as a meson subproject) get the right defines passed for the static build config. On Windows this matters because without that define it will try and do dllexport stuff, which it shouldn't be doing for a static library.

@dov dov merged commit 62bbf0d into fribidi:master Apr 6, 2022
@dov
Copy link
Contributor

dov commented Apr 6, 2022

Thanks @tp-m . You convinced me. 👍

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.

None yet

4 participants