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

Fix build when using mGBA in unorthodox location #11566

Merged
merged 1 commit into from Feb 22, 2023

Conversation

v1993
Copy link
Contributor

@v1993 v1993 commented Feb 14, 2023

Includes use mgba/ and mgba-util/ as part of path, making compilation fail when relying on include directory supplied here was actually needed.

Discovered by having both mGBA and dolphin installed in ~/.local. Confirmed to fix the issue.

Includes use `mgba/` and `mgba-util/` as part of path, making compilation fail when relying on include directory supplied here was actually needed.
@v1993
Copy link
Contributor Author

v1993 commented Feb 14, 2023

To demonstrate what this changes: on my installation LIBMGBA_INCLUDE_DIR would previously end up pointing at ~/.local/include/mgba, while now it points at ~/.local/include.

@v1993
Copy link
Contributor Author

v1993 commented Feb 14, 2023

Note: resulting binary fails to run without setting LD_LIBRARY_PATH due to being unable to locate mGBA's shared library. I'll try to fix that too, but not so sure where to look (liking completes just fine).

Update: this is fixed by setting CMAKE_INSTALL_RPATH_USE_LINK_PATH variable. However, given that it overrides a specific and intentional behavior on CMake's part, I think this is best left unset by Dolphin project itself; end users can specify it themselves if needed (by passing -DCMAKE_INSTALL_RPATH_USE_LINK_PATH=TRUE to CMake). As a side note, mGBA doesn't seem to do anything special in its RPATH handling, yet its executables have runpath set even after installation.

@v1993
Copy link
Contributor Author

v1993 commented Feb 22, 2023

Would you guys mind merging this? This is a super minor change that fixes build in a rare usecase that I just so happen to have.

@AdmiralCurtiss
Copy link
Contributor

I'll be honest, I have no idea if this has any side-effects, but I guess we can just revert if it turns out that it does.

@AdmiralCurtiss AdmiralCurtiss merged commit c5775c5 into dolphin-emu:master Feb 22, 2023
14 checks passed
@v1993
Copy link
Contributor Author

v1993 commented Feb 22, 2023

Automated build isn't affected at all due to using mgba from externals, so this only affects users who build dolphin themselves and have libmgba headers installed. It has basically zero chance of actually breaking anyone's manual build too, since this just corrects a simple mistake of pointing at the wrong include directory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants