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

pkgconf: add UAVIF_DLL for static linking with pkgconf #1904

Merged
merged 1 commit into from
Dec 28, 2023
Merged

pkgconf: add UAVIF_DLL for static linking with pkgconf #1904

merged 1 commit into from
Dec 28, 2023

Conversation

Biswa96
Copy link
Contributor

@Biswa96 Biswa96 commented Dec 25, 2023

allows for the usage of `pkgconf --static --cflags libavif' to produce
the proper CFLAGS for static linking. Relies on pkgconf rather than pkg-config.

This is as similar as https://gitlab.com/AOMediaCodec/SVT-AV1/-/merge_requests/1911

@kmilos @1480c1 @mechakotik

@vrabaud
Copy link
Collaborator

vrabaud commented Dec 28, 2023

Thx! If it's only for static linking, how about changing the code to:

if(BUILD_SHARED_LIBS)
    target_compile_definitions(avif_obj PRIVATE AVIF_DLL AVIF_BUILDING_SHARED_LIBS)
    set(AVIF_PKG_CONFIG_EXTRA_CFLAGS " -DAVIF_DLL")
    set(AVIF_PKG_CONFIG_EXTRA_CFLAGS_PRIVATE "")
else()
    set(AVIF_PKG_CONFIG_EXTRA_CFLAGS "")
    set(AVIF_PKG_CONFIG_EXTRA_CFLAGS_PRIVATE " -UAVIF_DLL")
endif()

@Biswa96
Copy link
Contributor Author

Biswa96 commented Dec 28, 2023

If it's only for static linking, how about changing the code to:

Yeah, this is a fix for static linking but the Cflags.private is only used when -static option is passed to pkgconf. So, having Cflags.private in pkgconfig file for shared library would not cause any issue. The svt-av1 project also has same change.

This is required in msys2/mingw packages. The projects provides both shared and static library in one package.

@vrabaud
Copy link
Collaborator

vrabaud commented Dec 28, 2023

Why even define a variable then? Just put -UAVIF_DLL right in libavif.pc.cmake no?

@Biswa96
Copy link
Contributor Author

Biswa96 commented Dec 28, 2023

Just put -UAVIF_DLL right in libavif.pc.cmake no?

Certainly, it is indeed a possibility. My approach was based on the definition of the AVIF_PKG_CONFIG_EXTRA_CFLAGS variable. If you prefer, I can remove the newly created variable as per your request.

@vrabaud
Copy link
Collaborator

vrabaud commented Dec 28, 2023

Yes please, remove it: there is a AVIF_PKG_CONFIG_EXTRA_CFLAGS because it can be overwritten when BUILD_SHARED_LIBS is on. But AVIF_PKG_CONFIG_EXTRA_CFLAGS_PRIVATE is only set once so we might as well set it for good in libavif.pc.cmake.

@kmilos
Copy link
Contributor

kmilos commented Dec 28, 2023

Please leave the private variable in even when building shared. In MSYS2 we ship both static and shared libs, but only one .pc (from the shared build run).

@vrabaud
Copy link
Collaborator

vrabaud commented Dec 28, 2023

@kmilos , so it's ok to only have it in libavif.pc.cmake then, no need for AVIF_PKG_CONFIG_EXTRA_CFLAGS_PRIVATE right?

allows for the usage of `pkgconf --static --cflags libavif' to produce
the proper CFLAGS for static linking. Relies on pkgconf rather than pkg-config.
@Biswa96
Copy link
Contributor Author

Biswa96 commented Dec 28, 2023

I have added -UAVIF_DLL in libavif.pc.cmake.

@vrabaud vrabaud merged commit 91db4e0 into AOMediaCodec:main Dec 28, 2023
21 checks passed
@Biswa96 Biswa96 deleted the pkgconf-cflags-private branch December 28, 2023 14:03
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