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

[libavif] Fix usage requirements export #42250

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

dg0yt
Copy link
Contributor

@dg0yt dg0yt commented Nov 20, 2024

Amends #41569.
Fixes #42112.
Alternative to #42151.

@MonicaLiu0311 MonicaLiu0311 added the category:port-bug The issue is with a library, which is something the port should already support label Nov 20, 2024
@BillyONeal
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@dg0yt
Copy link
Contributor Author

dg0yt commented Nov 20, 2024

CI still doesn't start @BillyONeal.

@BillyONeal
Copy link
Member

Yeah, something is broken. Having difficulty trying to restore it due to the storm damage last night https://www.seattletimes.com/seattle-news/weather/bomb-cyclone-over-450000-people-without-power-in-western-washington/

@BillyONeal
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@BillyONeal
Copy link
Member

I destroyed and recreated all the pools and that seems to have fixed it for now, macs will be slow until I wake them all back up since that is a manual process.

@BillyONeal
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@BillyONeal
Copy link
Member

:sigh: One more time

@BillyONeal
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@MonicaLiu0311 MonicaLiu0311 added the info:reviewed Pull Request changes follow basic guidelines label Nov 22, 2024
+ install(EXPORT ${PROJECT_NAME}-config DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/${PROJECT_NAME} FILE ${PROJECT_NAME}-targets.cmake)
+ if(NOT BUILD_SHARED_LIBS)
+ target_include_directories(avif_static PUBLIC $<INSTALL_INTERFACE:include>)
+ set(avif_static_link_libs yuv)
Copy link
Member

Choose a reason for hiding this comment

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

Stupid question: Why is this yuv rather than yuv::yuv like above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This port is taking separate paths for static vs. dynamic linkage.

yuv::yuv is from libavif's find module, used to link the shared libavif lib. No transitive usage requirements.

This PR just adds dependencies to take effect downstream when linking the static lib. yuv is vcpkg's unofficial target provided by find_dependency(libyuv CONFIG).

Copy link
Contributor Author

@dg0yt dg0yt Nov 23, 2024

Choose a reason for hiding this comment

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

(It is very hard to merge the separate paths. The static lib is merged with custom commands instead using plain cmake.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-bug The issue is with a library, which is something the port should already support info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[libavif] Can't use static build after 1.1.1 update
3 participants