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

[OUTDATED] Fix the pkgconfig file generation #63

Closed
wants to merge 1 commit into from

Conversation

GiulioRomualdi
Copy link
Contributor

This PR fixes #62

@traversaro
Copy link

Hi @bradbell , if there is anything we can do to help the process of merging this PR, feel free to let us know, thanks!

@bradbell
Copy link
Contributor

bradbell commented Nov 26, 2020

This PR fixes #62

The title of #62 is:
Generated pkgconfig file not compliant with the standard
Where can I view the standard ?

I have been using the following documentation for pkgconfig:
https://people.freedesktop.org/~dbn/pkg-config-guide.html

It seems to say that the definitions at the top are just an example. In fact, exec_prefix is equal to prefix and there is no discussion of how it might be different. I can guess at the specifications for the other definitions: prefix, includedir, and libdir.

There is another issue here. I recently made use of Libs.private and Requires.private to get cppad_mixed.pc to work properly. I should be making use of them in the cppad.pc file.

@bradbell
Copy link
Contributor

bradbell commented Nov 28, 2020

I am working on a new version of how pkg-config is used by cppad during the cmake command and need to better understand this pull request in terms of its requirements. For example, what is wrong with the current version of the cppad.pc file ?

@traversaro
Copy link

Hi @bradbell, thanks for the reply. I was pinging the issue that @GiulioRomualdi (another member of my lab, as we both work in the same research institute opened). I think that @GiulioRomualdi assume that the prefix variable must be defined from this comment of @Neumann-A in microsoft/vcpkg#12560 (comment) (for context, vcpkg is a C++ package manager, see https://docs.microsoft.com/en-us/cpp/build/vcpkg?view=msvc-160) :

Prefix must be defined. Patch the *.pc file if it is not defined in there. pkg-config does not work if the prefix is not defined (with a few exceptions).

However, by inspecting the pkg-config documentation I also fail to find any reference to the fact that the prefix variable must by defined, so it is possible that @Neumann-A referred to a vcpkg-only constraint, in that case probably it make sense that the patch to fix this remains a vcpkg-specific patch and is not ported upstream.

@GiulioRomualdi
Copy link
Contributor Author

Hi all, I'm not an expert of pkgconfig and I followed what @Neumann-A suggested in microsoft/vcpkg#12560 (comment)

@Neumann-A
Copy link

However, by inspecting the pkg-config documentation I also fail to find any reference to the fact that the prefix variable must by defined,

Since the variables are expanded recursively, this is very helpful when used in conjunction with autoconf derived paths.

This means any used variable needs to be defined because otherwise the variable expansion will fail. There are some exception from the rules since on windows prefix gets redefined under some circumstances (e.g. if the path ends in lib/pkgconfig and pkg-config has been build for windows) but otherwise all variables need to be defined.
Looking at the *.pc.in file I see it is not using any commonly used variables and seems to encode everything using absolute paths?
A *.pc file should at least use prefix as a variable since pkg-config has hardcoded features overwriting that variable (windows only: --dont-define-prefix --prefix-variable=PREFIX). By not using it you are basically breaking users expectation using the mentioned commands.

@traversaro
Copy link

Thanks @Neumann-A for chiming in!

@bradbell
Copy link
Contributor

bradbell commented Dec 1, 2020

As I understand, vcpkg has some special requirements for the pkg-config files *.pc. There should be a place in the vcpkg documentation where theses requirements are specified. It not, perhaps you could list them on this issue page.

@bradbell
Copy link
Contributor

bradbell commented Dec 2, 2020

I think I have the definitions I am looking for:
https://www.gnu.org/prep/standards/html_node/Directory-Variables.html

@bradbell
Copy link
Contributor

bradbell commented Jan 9, 2021

The newer version of cppad.pc,in
https://github.com/coin-or/CppAD/blob/master/pkgconfig/cppad.pc.in
has the following definitions at the top:

prefix=@cppad_prefix@
exec_prefix=$(prefix)
includedir=${prefix}/@cppad_includedir@
libdir=${exec_prefix}/@cppad_libdir@

Does this fufill the requirements of this pull requrest ?

@GiulioRomualdi
Copy link
Contributor Author

GiulioRomualdi commented Feb 8, 2021

Hi @bradbell I tried to use the latest version of cppad.pc.in, when I was trying to include cppad/cppad.hpp into my project I add the following error ami-iit/bipedal-locomotion-framework#194

[ 66%] Building CXX object src/AutoDiff/tests/CMakeFiles/CppADTestUnitTests.dir/CppADTest.cpp.o
In file included from /home/gromualdi/robot-code/robotology-superbuild/src/bipedal-locomotion-framework/src/AutoDiff/tests/CppADTest.cpp:11:
/home/gromualdi/robot-code/robotology-superbuild/src/bipedal-locomotion-framework/src/AutoDiff/include/BipedalLocomotion/AutoDiff/CppAD.h:12:10: fatal error: cppad/cppad.hpp: No such file or directory
   12 | #include <cppad/cppad.hpp>
      |          ^~~~~~~~~~~~~~~~~
compilation terminated.

So I investigated a bit and I noticed some typos in the cmake machinery.
This patch completely fixes the problem: GiulioRomualdi@6d10b8d

Let me know if you are interested in a PR. (I can close this one and open a new one)

@GiulioRomualdi GiulioRomualdi changed the title Fix the pkgconfig file generation [OUTDATED] Fix the pkgconfig file generation Feb 8, 2021
@GiulioRomualdi
Copy link
Contributor Author

Closing in favor of the #95

@GiulioRomualdi GiulioRomualdi deleted the fix/pkgconfig branch February 8, 2021 14:01
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.

Generated pkgconfig file not compliant with the standard
4 participants