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

FMT_LIB_NAME not properly substituted in pkg-config file fmt.pc #1597

Closed
codedump opened this issue Mar 18, 2020 · 15 comments
Closed

FMT_LIB_NAME not properly substituted in pkg-config file fmt.pc #1597

codedump opened this issue Mar 18, 2020 · 15 comments

Comments

@codedump
Copy link

Hello,

I'm compiling libfmt with the following command line:

cmake -D CMAKE_INSTALL_PREFIX=$PREFIX  \
             -D CMAKE_INSTALL_LIBDIR=lib \
             -D FMT_LIB_DIR=lib \
             -D FMT_CMAKE_DIR=lib/cmake
             -D FMT_PKGCONFIg_DIR=lib/pkgconfig

make VERBOSE=1 -j$NUMCORES
make install

After compiling, the installed fmt.pc file looks like this:

prefix=/path/to/prefix
exec_prefix=/path/to/prefix
libdir=${exec_prefix}/lib
includedir=${prefix}/include

Name: fmt
Description: A modern formatting library
Version: 6.1.3
Libs: -L${libdir} -lFMT_LIB_NAME-NOTFOUNDd
Cflags: -I${includedir}

In particular, the library name (expected -lfmt or -lfmtd) is not properly set.

I'm using today's HEAD, i.e. 3cf619d

Any ideas?

Thanks,
fb.

@vitaut
Copy link
Contributor

vitaut commented Mar 18, 2020

FMT_LIB_NAME was introduced in 6c30f41 and is defined in

get_target_property(FMT_LIB_NAME fmt OUTPUT_NAME)

I'm not sure why the output name property is not found.

@vitaut
Copy link
Contributor

vitaut commented Mar 18, 2020

cc @ambitslix, the author of #1534 that introduced FMT_LIB_NAME.

@ambitslix
Copy link
Contributor

ambitslix commented Mar 18, 2020

Hmmm somehow this line is missing now from CMakeLists.txt.

set_target_properties(fmt PROPERTIES
OUTPUT_NAME "fmt"
VERSION ${FMT_VERSION} SOVERSION ${CPACK_PACKAGE_VERSION_MAJOR}
DEBUG_POSTFIX "${FMT_DEBUG_POSTFIX}")

Do you want to merge this? I've created a pull request for the fix.

@vitaut
Copy link
Contributor

vitaut commented Mar 18, 2020

I thought the OUTPUT_NAME property is set automatically by CMake.

@vitaut
Copy link
Contributor

vitaut commented Mar 18, 2020

It's probably easier to just set FMT_LIB_NAME to fmt instead of going through properties.

@ambitslix
Copy link
Contributor

ambitslix commented Mar 19, 2020

It's probably easier to just set FMT_LIB_NAME to fmt instead of going through properties.

You would think but it's not the case.

I thought the OUTPUT_NAME property is set automatically by CMake.

Not in this case. You're welcome to try alternate ways. A few other libraries have this issue with pkg-config, and there aren't seem to be good solutions. See here:

google/benchmark#928

I think it's best to drop pkg-config support altogether or use the fix provided. I don't use pkg-config with cmake projects anymore.

@codedump
Copy link
Author

I think it's best to drop pkg-config support altogether or use the fix provided. I don't use pkg-config with cmake projects anymore.

Please don't do that. You may not be using pkg-config, but a lot of people do. We don't always have the choice of using purely cmake or purely pkg-config/autotools. Even if there's a bit of a holy war going on about which one is better, ditching pkg-config support is essentially making life harder for half of your users out there over a line of code's worth. And given llibfmt's popularity, that's a lot of users.

In our case, for example, we're using Conan (the C++ source-package manager) to manage package dependencies for 3 platforms and a number of difficult-to-manage packages. As far as our own packages go, we're sometimes using autotools/pkg-config, so in Conan terminology this means we're using a so-called pkg-config generator to generate dependency information out of package manifest files of all the underlying libraries, including libfmt. In other words, Conan is generating a .pc file for libfmt for us to use.

However, we do want to be able to also support compiling of our library outside of the Conan framework (sometimes you're working on your laptop and just don't want to set up all the Conan infrastructure just to make a quick change while you're riding the train). Now if libfmt ditches pkg-config support, we'd have to implement two ways of recognizing libfmt instead of one: a pkg-config way to go with Conan, and a cmake way to go with everything else.

BTW, in the link you're providing, it's not pkg-config that performs badly, it's a buggy build system configuration which uses pkg-config.

TL;DR: +1 for setting that FMT_LIB_NAME to fmt, if I get to vote :-) Ditching pkg-config support increases complexity of build systems of libfmt users for no benefit at all.

@vitaut
Copy link
Contributor

vitaut commented Mar 19, 2020

Merged @ambitslix's PR (#1598) which should fix the issue. Thanks!

@vitaut vitaut closed this as completed Mar 19, 2020
@codedump
Copy link
Author

Thanks a lot!

@vitaut
Copy link
Contributor

vitaut commented Mar 19, 2020

You would think but it's not the case.

@ambitslix, could you elaborate why it is not the case, i.e. why won't setting FMT_LIB_NAME directly (without using properties) help?

@vitaut
Copy link
Contributor

vitaut commented Mar 19, 2020

@codedump, could you by any chance check if 52d0e1b works too?

@ambitslix
Copy link
Contributor

This issue is a bit deeper then what it seems at first. The way I see it after working on this here and for other libs as well is that pkg-config and cmake don't really work together well. You are using a library in this case fmt that uses primarily cmake, and cmake provides for the same functionality that pkg-config does (configuring build dependencies). So you're expecting a tool such cmake to maintain functionality that it already has buit for an independent tool (pkg-config). Unfortunately currently cmake support is kind of broken for pkg-config. So to make all this work you have to do ugly tricks in cmake. So I recommend that for cmake project use cmake for pkg-config project use pkg-config. When the cmake developers fix these cmake issues perhaps we can go back to using cmake to configure with pkg-config as well and have that convenience. But remember cmake already has the functionality that pkg-config provides. As for providing answers to why all that works the way it does, I don't remember sorry, I've looked at too many thousands of other lines of code in the last 3 months that made me forget why pkg-config exactly doesn't work with cmake. It has to do with variables (library names) needed by pkg-config (pc) file configuration not being available, and configure time to cmake. So you might as well call an external script to get it right.

@codedump
Copy link
Author

@codedump, could you by any chance check if 52d0e1b works too?

@vitaut: 52d0e1b works as expected.

FYI, the commit before that doesn't get the debug extension (-lfmt vs -lftmd) right when building a debug version.

@vitaut
Copy link
Contributor

vitaut commented Mar 20, 2020

Thanks for checking!

@codedump
Copy link
Author

No problem, thanks for the rapid fix! :-)

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

No branches or pull requests

3 participants