Skip to content

Fix generation of pkg-config file with absolute includedir/libdir. #1199

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

Merged
merged 1 commit into from
Jul 20, 2020
Merged

Fix generation of pkg-config file with absolute includedir/libdir. #1199

merged 1 commit into from
Jul 20, 2020

Conversation

lopsided98
Copy link
Contributor

The CMake variables CMAKE_INSTALL_LIBDIR and CMAKE_INSTALL_INCLUDEDIR are not guaranteed to be relative paths. If they are absolute, an invalid pkg-config file is generated.

See https://github.com/jtojnar/cmake-snips#concatenating-paths-when-building-pkg-config-files for more information.

@coveralls
Copy link

coveralls commented Jul 5, 2020

Coverage Status

Coverage remained the same at 93.801% when pulling 789f29e on lopsided98:pkgconfig-paths into bf0cfa5 on open-source-parsers:master.

@dota17
Copy link
Member

dota17 commented Jul 13, 2020

LGTM.

CMakeLists.txt Outdated
@@ -1,5 +1,7 @@
# vim: et ts=4 sts=4 sw=4 tw=0

set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this line move down a bit? I think there should be a better place.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved it down. If you have somewhere specific you want it let me know.

@dota17 dota17 merged commit 5be07bd into open-source-parsers:master Jul 20, 2020
@dota17
Copy link
Member

dota17 commented Jul 20, 2020

Thanks.

@lopsided98 lopsided98 deleted the pkgconfig-paths branch July 20, 2020 16:28
@ferdnyc
Copy link
Contributor

ferdnyc commented Oct 6, 2021

Related to this PR (but also just jsoncpp installation in general), note that work is underway to (hopefully) come up with a design for built-in pkg-config features for CMake's install(EXPORT ...) mode. (The notion is that EXPORT would optionally generate .pc files to complement CMake's native EXPORTED configurations, so that CMake-built and -installed dependencies are visible to build systems other than CMake.)

One major hurdle to overcome in the design so far has been the question of input dependency handling. Particularly in a presumptive future where CMake, when generating the build system for some project, may have to contend with dependencies that are defined by a mix of native CMake EXPORTED target configurations, .pc files that CMake also generated, and .pc files that came from another build system. And some of those may be redundant with each other.

As a CMake project which also generates and installs .pc files, jsoncpp is very much the target audience for the (proposed) new features. I'd encourage anyone interested to look over the design-in-progress and offer any input you may have, so that the final implementation hopefully will meet or exceed both jsoncpp's current and expected future requirements.

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