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

capstone.pc file contains broken includedir #1339

Closed
berrange opened this issue Jan 11, 2019 · 8 comments

Comments

Projects
None yet
5 participants
@berrange
Copy link

commented Jan 11, 2019

The capstone.pc file that is built in 4.0.1 has regressed in its includedir path:

includedir=/usr/include

compared to 3.0.5 which had

includedir=/usr/include/capstone

This problem was supposedly fixed in #1276 but that only fixes the problem when building with cmake. When using plain make, the capstone.pc.in file is ignored and the 'generate-pkgconfig' make target is called instead.

@rwmjones

This comment has been minimized.

Copy link
Contributor

commented Jan 11, 2019

I'm interested whether it's the intention of capstone developers that the header file is included using:

#include <capstone/capstone.h>

or

#include <capstone.h>

Note the first variant is used by at least one test: https://github.com/aquynh/capstone/blob/master/tests/test_basic.c

@berrange

This comment has been minimized.

Copy link
Author

commented Jan 11, 2019

Changing this has the effect of breaking existing applications (eg QEMU) that use capstone though, as they'll all need to make their '#include' directives conditional depending on which version of capstone they're using.

@disconnect3d

This comment has been minimized.

Copy link
Contributor

commented Jan 11, 2019

@berrange Or they could add a -I/usr/include/capstone or -I${INCLUDE_PATH}/capstone for latest capstone version.

@rwmjones

This comment has been minimized.

Copy link
Contributor

commented Jan 11, 2019

Well no, not really. The point of using pkg-config is that adds the right -I flags to the command line.

@aquynh

This comment has been minimized.

Copy link
Owner

commented Jan 11, 2019

@rwmjones

This comment has been minimized.

Copy link
Contributor

commented Jan 11, 2019

I can certainly hack something, but it's not very easy to fix properly. Ideally the capstone.pc.in file would be the source of truth for both build systems, but at the moment the Makefile has effectively its own copy of that template. However modifying the Makefile to use the template isn't easy either as there are no substitute variables to use. Why does this project have two separate build systems?

rwmjones added a commit to rwmjones/capstone that referenced this issue Jan 11, 2019

Fix include path in pkg-config for Makefile too (aquynh#1339).
Commit 0a39b78 fixed the pkg-config include path when using cmake.
However it didn't fix it for the Makefile.  This fixes the Makefile
path.

Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
@bonzini

This comment has been minimized.

Copy link

commented Jan 11, 2019

Looks good to me.

aquynh added a commit that referenced this issue Jan 12, 2019

Fix include path in pkg-config for Makefile too (#1339). (#1340)
Commit 0a39b78 fixed the pkg-config include path when using cmake.
However it didn't fix it for the Makefile.  This fixes the Makefile
path.

Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
@aquynh

This comment has been minimized.

Copy link
Owner

commented Jan 12, 2019

fixed with #1340, thanks.

@aquynh aquynh closed this Jan 12, 2019

aquynh added a commit that referenced this issue Jan 13, 2019

Fix include path in pkg-config for Makefile too (#1339). (#1340)
Commit 0a39b78 fixed the pkg-config include path when using cmake.
However it didn't fix it for the Makefile.  This fixes the Makefile
path.

Signed-off-by: Richard W.M. Jones <rjones@redhat.com>

aquynh added a commit that referenced this issue Jan 13, 2019

Fix include path in pkg-config for Makefile too (#1339). (#1340)
Commit 0a39b78 fixed the pkg-config include path when using cmake.
However it didn't fix it for the Makefile.  This fixes the Makefile
path.

Signed-off-by: Richard W.M. Jones <rjones@redhat.com>

mopsfelder pushed a commit to mopsfelder/fedora-rpms-qemu that referenced this issue Feb 8, 2019

Revert "Add a temporary patch to fix capstone header location."
This has now been fixed in capstone, see
aquynh/capstone#1339

This reverts commit e0155fb.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.