CMake: Force /usr/local/include to come last #11292
Merged
Conversation
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
|
Not fully effective on FreeBSD, Attached build log has From that log, the Include search order is |
|
@TheLastRar can you run cmake with this patch and post the output? 0001-Debug.patch |
|
|
And here is the generated CmakeCache, if it helps |
|
Can you try now (with the patch applied again)? |
|
Build still fails Here is the cmake configure log |
TheLastRar
reviewed
May 26, 2024
pcsx2/CMakeLists.txt
Outdated
| @@ -39,6 +39,8 @@ endif() | |||
| if(USE_LINKED_FFMPEG) | |||
| target_compile_definitions(PCSX2_FLAGS INTERFACE USE_LINKED_FFMPEG) | |||
| target_link_libraries(PCSX2_FLAGS INTERFACE FFMPEG::avcodec FFMPEG::avformat FFMPEG::avutil FFMPEG::swscale FFMPEG::swresample) | |||
| else() | |||
| target_include_directories(PCSX2_FLAGS INTERFACE "${FFMPEG_INCLUDE_DIRS}") | |||
There was a problem hiding this comment.
Does this need to be made into a target like was done with Wayland & Shaderc?
|
If I replace add_library(FFMPEG-Headers INTERFACE)
target_include_directories(FFMPEG-Headers INTERFACE ${FFMPEG_INCLUDE_DIRS})
target_link_libraries(PCSX2_FLAGS INTERFACE FFMPEG-Headers)I get a cmake build config log as The build completes, and has the following search order I did, however, note that common and pcsx2-qt have an incorrect search order, but still build without issue common has the following pcsx2-qt has |
TellowKrinkle
force-pushed
the
HeaderOrder
branch
from
a41e902 to
a10a836
Compare
|
Okay new solution where we copy all the dependencies' include directories to the top level |
|
That did the trick, Build completes successfully! Build log (with -v) BuildSuccess.txt Cmake config log for reference; |
TellowKrinkle
force-pushed
the
HeaderOrder
branch
from
a10a836 to
adc87c7
Compare
|
OK, I removed the debug messages so this should be good to go |
TheLastRar
approved these changes
May 28, 2024
TellowKrinkle
force-pushed
the
HeaderOrder
branch
from
adc87c7 to
1855be2
Compare
|
Curious, what are the blockers for merging this? We're encountering new build issues such as noted above, and this PR resolves them. |
Me forgetting to click the button |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description of Changes
Not a huge fan of this solution, but it was the best I could think of. If anyone has a better solution please say so.
Fixes the issue mentioned in #11287, as well as building on macOS with homebrew ffmpeg (that you want) and a homebrew-installed shaderc that you don't want.
There's actually two parts to the problem:
depsdirectory gets marked as a system include by one of the packages in it/usr/local/includedoes not (at least if the only thing in it is ffmpeg)/usr/local/includeends up coming first.So this patch adds a function that both marks matching include directories as system, and puts the dependencies that contain them at the end of the list.
It can only reorder dependencies at the same depth (e.g. if a depends on b and c, b depends on d, and c depends on e, the only possible orders would be a b d c e or a c e b d), but it should work for our current set of externally-imported libraries.Rationale behind Changes
Less broken compilation
Suggested Testing Steps
@TheLastRar please test freebsd