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

pkg-config files are broken when CMAKE_INSTALL_{INCLUDE,LIB}DIR is absolute #8992

Closed
alexshpilkin opened this issue May 17, 2022 · 3 comments · Fixed by #9022
Closed

pkg-config files are broken when CMAKE_INSTALL_{INCLUDE,LIB}DIR is absolute #8992

alexshpilkin opened this issue May 17, 2022 · 3 comments · Fixed by #9022
Assignees
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@alexshpilkin
Copy link

What component of google-cloud-cpp is this related to?

All of them as far as I can see.

Describe the bug

As per title: external/googleapis/config.pc.in, google/cloud/config.pc.in, and google/cloud/*/config.pc.in have

prefix=${pcfiledir}/../..
exec_prefix=${prefix}/@CMAKE_INSTALL_BINDIR@
libdir=${prefix}/@CMAKE_INSTALL_LIBDIR@
includedir=${prefix}/@CMAKE_INSTALL_INCLUDEDIR@

and so can’t handle absolute paths in CMAKE_INSTALL_{INCLUDE,LIB}DIR. This leads to broken .pc files on NixOS in particular.

The generation of these buggy templates seems to have been automated in generator/internal/scaffold_generator.cc.

Identical to libjxl/libjxl#1400 (fixed). See “Concatenating paths when building pkg-config files” for a thorough discussion of the problem and a suggested fix, or KDE’s extra-cmake-modules for a simpler approach.

Operating system:

$ uname -a
Linux localhost 5.15.36 #1-NixOS SMP Wed Apr 27 12:39:02 UTC 2022 x86_64 GNU/Linux

What version of google-cloud-cpp are you using?

1.38.0, but inspection confirms issue exists in main as well

@alexshpilkin alexshpilkin added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels May 17, 2022
@coryan coryan self-assigned this May 18, 2022
@coryan
Copy link
Contributor

coryan commented May 18, 2022

Thanks for the detailed but report and the links to solutions. Do you have any suggestions on how to test this?

@alexshpilkin
Copy link
Author

@coryan In a way that’s amenable to your automation? No clue :) In general it should suffice to build things with -DCMAKE_INSTALL_*DIR=/something/absolute and grep the results for //something, which is an blatant hack but should work fine (cf NixOS/nixpkgs#172347). This will cost you a full additional build, unfortunately. (The only things you really need out of it are the pkg-config files—and maybe the CMake modules for safety—but I’ve no idea if you could get the sprawling build system here to produce only those.)

@coryan
Copy link
Contributor

coryan commented May 19, 2022

@coryan In a way that’s amenable to your automation? No clue :)

Good point, I was not trying to make you figure out our CI system 😃

In general it should suffice to build things with -DCMAKE_INSTALL_*DIR=/something/absolute and grep the results for //something, which is an blatant hack but should work fine

Thanks, that is what I needed. This is not the worst hack I am guilty of, not even in the top 50 I would think.

For my future self: I reasoned that just grep is good enough for this, we have existing builds that verify the generated .pc files are usable. These builds default values for CMAKE_INSTALL_*, but those builds + grep give us enough coverage.

The only things you really need out of it are the pkg-config files—and maybe the CMake modules for safety—but I’ve no idea if you could get the sprawling build system here to produce only those.

We had a build that is close enough for our purposes. If things get messy, we can add another build. Burning more CPUs is not much of a concern for us. Google has more than 7 machines, as the saying goes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants