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

Fixes Raptor2 compile issue #395

Merged
merged 5 commits into from
Aug 20, 2016
Merged

Conversation

pvint
Copy link
Contributor

@pvint pvint commented Aug 12, 2016

See issue #342

I have tested this on several Gentoo installations and on Debian Jessie - all work fine (the gentoo installs failed to compile without this change, Jessie was OK either way).

@mauser
Copy link
Member

mauser commented Aug 17, 2016

Hi!

I'm unsure about this fix.. i'm afraid that this change breaks build which are not using pkgconfig but having the raptor issue described in #194 ? It would also be interesting to see why you have to use the PC_ prefix. Maybe you could have a look at the behaviour of FindHelper.cmake on your system? There are some lines which can be commented out to give more details about the populated variables..

In addition, i do not really understand why we need that include in the main CMakeLists.txt in the first place , since there is an similar statement in src/core/CMakeLists.txt. I believe that the #194 would be better fixed by including ${LRDF_INCLUDE_DIRS} in CMakeLists.txt. Could you investigate further in this direction? At the moment i don't have access to a gentoo system or such, so it is hard for me to look deeper into the problem.

@pvint
Copy link
Contributor Author

pvint commented Aug 17, 2016

I will look into it more... to be honest I don't remember anymore how I came up with the PC_ prefix some months ago.... all I know is I need it to make it compile. I now have two systems (one Gentoo and one Debian Jessie) and one needs this fix and one doesn't, so I can test fairly well.

I will test out src/core/CMakeLists.txt - I think you're right in that it's a more appropriate place.

I will also look into FindHelper.cmake.

Thanks @mauser

Paul

@pvint
Copy link
Contributor Author

pvint commented Aug 19, 2016

I'm glad you questioned this, @mauser as it was good to have a deeper look (and I learned a bit about cmake along the way!).

I have removed the whole if(LRDF_FOUND) bit in CMakelists.txt, and in src/core/CMakelists.txt I simply added "${RAPTOR_INCLUDE_DIRS}" to the includes.

This made more sense, as pkg-config returns the correct include dir on the machine with raptor2 in /usr/nclude/raptor2/raptor.h and on the machines with raptor.h in the default includes dir pkg_config returns nothing, which also works.

On my Gentoo system:
pvint@nautilus /usr/local/src/hydrogen $ pkg-config lrdf --cflags
-I/usr/include/raptor2

Let me know what you think

Paul

@mauser mauser merged commit fce9f5e into hydrogen-music:master Aug 20, 2016
@mauser
Copy link
Member

mauser commented Aug 20, 2016

Great! That looks good.

@schivmeister : Could you please check if hydrogen still compiles for you? This pull request modified a part of code from your isse #194.

@trebmuh
Copy link
Member

trebmuh commented Aug 20, 2016

@schivmeister
Copy link

Hey guys, I don't think I will be able to get to it in a timely manner as I have been away from my Linux systems. However, the proposed changes look like the better solution as mine was just a hack (and does not work for everyone as we can see from the newer bug reports).

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.

4 participants