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

libdnf: Add include directories and LDFLAGS from librepo #266

Merged
merged 1 commit into from
Feb 6, 2023

Conversation

mcrha
Copy link
Contributor

@mcrha mcrha commented Feb 6, 2023

It's not possible to build the project when the librepo is built in a non-standard prefix, especially when no other project is build with that custom prefix.

Using the returned include directories and whole LDFLAGS fixes it.

It's not possible to build the project when the librepo is
built in a non-standard prefix, especially when no other
project is build with that custom prefix.

Using the returned include directories and whole LDFLAGS
fixes it.
@m-blaha
Copy link
Member

m-blaha commented Feb 6, 2023

Thanks for the patch!

@m-blaha m-blaha merged commit 3388f6f into rpm-software-management:main Feb 6, 2023
@jan-kolarik
Copy link
Member

jan-kolarik commented Feb 14, 2023

After rebasing to this PR local builds seem broken, at least for me... Lot of errors like this:

make[1]: *** [CMakeFiles/Makefile2:3444: test/libdnf/CMakeFiles/run_tests.dir/all] Error 2
/usr/bin/ld: ../libdnf/libdnf5.so.1: undefined reference to `lr_gpg_list_keys'
/usr/bin/ld: ../libdnf/libdnf5.so.1: undefined reference to `lr_gpg_subkey_get_id'
/usr/bin/ld: ../libdnf/libdnf5.so.1: undefined reference to `lr_gpg_key_get_next'
/usr/bin/ld: ../libdnf/libdnf5.so.1: undefined reference to `lr_gpg_subkey_get_next'
/usr/bin/ld: ../libdnf/libdnf5.so.1: undefined reference to `lr_gpg_key_get_userids'
/usr/bin/ld: ../libdnf/libdnf5.so.1: undefined reference to `lr_gpg_keys_free'
/usr/bin/ld: ../libdnf/libdnf5.so.1: undefined reference to `lr_gpg_import_key_from_fd'
/usr/bin/ld: ../libdnf/libdnf5.so.1: undefined reference to `lr_gpg_key_get_subkeys'
/usr/bin/ld: ../libdnf/libdnf5.so.1: undefined reference to `lr_gpg_subkey_get_can_sign'
/usr/bin/ld: ../libdnf/libdnf5.so.1: undefined reference to `lr_gpg_subkey_get_fingerprint'
/usr/bin/ld: ../libdnf/libdnf5.so.1: undefined reference to `lr_gpg_import_key_from_memory'
/usr/bin/ld: ../libdnf/libdnf5.so.1: undefined reference to `lr_gpg_key_get_raw_key'
/usr/bin/ld: ../libdnf/libdnf5.so.1: undefined reference to `lr_gpg_subkey_get_timestamp'

I have latest librepo built from sources. Also tried applying possibly related change from there without luck.

When switching back to target_link_libraries(libdnf ${LIBREPO_LIBRARIES}), the situation is fixed. @mcrha Do you know what could be the issue?

@mcrha
Copy link
Contributor Author

mcrha commented Feb 14, 2023

I get a lot of missing symbols when I configure the project with:

LDFLAGS='-Wl,--as-needed -Wl,-z -Wl,relro -Wl,-z -Wl,now -Wl,-z -Wl,defs'

It fixes itself when I configure the project with:

LDFLAGS='-Wl,--as-needed'

I reconfigure the project from scratch, meaning my _build directory is deleted (and created again) before I run CMake. Stale build files sometimes trick the compiler.

If it won't work, then maybe try to add both variables: target_link_libraries(libdnf ${LIBREPO_LDFLAGS} ${LIBREPO_LIBRARIES})

@jan-kolarik
Copy link
Member

jan-kolarik commented Feb 14, 2023

I tried adding both variables with the single target_link_libraries call and also with two separate calls without success. But anyway, without deep CMake knowledge TBH, the LIBREPO_LDFLAGS is not defined anywhere in the librepo upstream whereas the LIBREPO_INCLUDE_DIRS and the original LIBREPO_LIBRARIES is defined here. So I am still wondering if this is only related to a custom, manually configured project?

@mcrha
Copy link
Contributor Author

mcrha commented Feb 14, 2023

I tried adding both variables with the single target_link_libraries call and also with two separate calls without success.

That's odd. If LIBREPO_LDFLAGS is empty for you and LIBREPO_LIBRARIES works on its own, then using them both should just work for both of us, no?

@mcrha
Copy link
Contributor Author

mcrha commented Feb 14, 2023

For what it's worth, the Fedora's 5.0.6 build, which contains this change, works properly:
https://koji.fedoraproject.org/koji/buildinfo?buildID=2150570

Could you add the following line into libdnf/CMakeLists.txt below the librepo check, please? I have it on line 92:

message(WARNING "LIBREPO_LDFLAGS:'${LIBREPO_LDFLAGS}' LIBREPO_LIBRARIES:'${LIBREPO_LIBRARIES}' LIBREPO_INCLUDE_DIRS:'${LIBREPO_INCLUDE_DIRS}'")

It says here this:

CMake Warning at libdnf/CMakeLists.txt:92 (message):
  LIBREPO_LDFLAGS:'-L${PREFIX}/lib;-lrepo;-L/usr/lib64;-lglib-2.0'
  LIBREPO_LIBRARIES:'repo;glib-2.0'
  LIBREPO_INCLUDE_DIRS:'${PREFIX}/include;/usr/include/glib-2.0;/usr/lib64/glib-2.0/include;/usr/include/sysprof-4;/usr/include/libxml2'

where ${PREFIX} is the prefix I configure the project with.

You can check it as:

   mkdir _b2 && cd _b2 && cmake -G "Unix Makefiles" -DCMAKE_INSTALL_PREFIX=/tmp/dnf5 .. >/dev/null

Also, what do you get when running:

   pkg-config --modversion librepo
   pkg-config --cflags librepo
   pkg-config --libs librepo

I get:

1.15.1

-I${PREFIX}/include -D_FILE_OFFSET_BITS=64 -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/sysprof-4 -pthread -I/usr/include/libxml2 

-L${PREFIX}/lib -lrepo -lglib-2.0 

@jan-kolarik
Copy link
Member

@mcrha Thank you very much for the detailed answer.

Now I guess the problem is that I am using the dbox container for the development.

This is my output:

LIBREPO_LDFLAGS:'-L/usr/lib64;-lrepo;-lglib-2.0'
LIBREPO_LIBRARIES:'repo;glib-2.0'
LIBREPO_INCLUDE_DIRS:'/usr/include;/usr/include/glib-2.0;/usr/lib64/glib-2.0/include;/usr/include/sysprof-4;/usr/include/libxml2'

1.15.1
-I/usr/include -D_FILE_OFFSET_BITS=64 -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/sysprof-4 -pthread -I/usr/include/libxml2
-lrepo -lglib-2.0

In the /usr/lib64 there are the libraries installed, including librepo, but I use the latest development version from sources in different directory within the container. dbox setups the LD_LIBRARY_PATH variable, so I got there ...:/root/dbox/librepo/_install/fedora-latest/usr/lib64:... which was until this change used as the path for the linker I guess. But now the command target_link_libraries(libdnf ${LIBREPO_LDFLAGS}) overrides this path and the installed version of librepo is passed which don't have the mentioned symbols - undefined references.

This is I think the first time this command with *_LDFLAGS is used in any of the rpm-software-management projects, therefore I haven't encountered this problem before 🙂

@mcrha
Copy link
Contributor Author

mcrha commented Feb 15, 2023

Ah, ordering of the link libraries ... that's a nightmare...

From what you said, I suppose using:
target_link_libraries(libdnf ${LIBREPO_LDFLAGS} ${LIBREPO_LIBRARIES})
and
target_link_libraries(libdnf ${LIBREPO_LIBRARIES} ${LIBREPO_LDFLAGS})
will produce different results (the later will work).

The question is: why does pkg-config find the system-installed librepo, instead of your private build? Do you have set also PKG_CONFIG_PATH to your custom PREFIX? I'd expect something like: PKG_CONFIG_PATH=/root/dbox/librepo/_install/fedora-latest/usr/lib64/pkgconfig (no need to add the other standard paths here).

@jan-kolarik
Copy link
Member

I've already tried the reordering before, not working... I've always removed the _build directory before that to be sure.

In the PKG_CONFIG_PATH I have only the custom development paths, this is also done by the dbox I didn't change anything there:

[root@2944397590db dnf5]# echo $PKG_CONFIG_PATH
/root/dbox/dnf5/_install/fedora-latest/usr/lib64/pkgconfig:/root/dbox/ci-dnf-stack/_install/fedora-latest/usr/lib64/pkgconfig:/root/dbox/dnf-plugins-extras/_install/fedora-latest/usr/lib64/pkgconfig:/root/dbox/dnf-plugins-core/_install/fedora-latest/usr/lib64/pkgconfig:/root/dbox/dnf/_install/fedora-latest/usr/lib64/pkgconfig:/root/dbox/createrepo_c/_install/fedora-latest/usr/lib64/pkgconfig:/root/dbox/libdnf/_install/fedora-latest/usr/lib64/pkgconfig:/root/dbox/librepo/_install/fedora-latest/usr/lib64/pkgconfig:/root/dbox/libcomps/_install/fedora-latest/usr/lib64/pkgconfig:/root/dbox/libsolv/_install/fedora-latest/usr/lib64/pkgconfig:/root/dbox/libmodulemd/_install/fedora-latest/usr/lib64/pkgconfig:/root/dbox/rpm/_install/fedora-latest/usr/lib64/pkgconfig

@mcrha
Copy link
Contributor Author

mcrha commented Feb 15, 2023

There's something misbehaving on your machine for sure. If the librepo custom build installed files are in one of the PKG_CONFIG_PATH paths, then the pkg-config should find it before the system-installed version. I mean, having pkg-config finding the correct version you get from pkg-config --cflags librepo the -I${PREFIX}/..., just as I do, not -I/usr/include ..... I suppose the --modversion output is also wrong, as the locally built version should be higher.

What if you move away /usr/lib64/pkgconfig/librepo.pc file, is the pkg-config able to find the custom build?

@jan-kolarik
Copy link
Member

Moving away the /usr/lib64/pkgconfig/librepo.pc has no effect on the situation. Building with target_link_libraries(libdnf ${LIBREPO_LDFLAGS}) still doesn't work while target_link_libraries(libdnf ${LIBREPO_LIBRARIES}) is OK...

@mcrha
Copy link
Contributor Author

mcrha commented Feb 15, 2023

Let's focus on the pkg-config output - it cannot claim there is a system-installed librepo, when you drop the pkg-config file... How many librepo.pc files are installed/available in your build environment, please? Does the one in the custom prefix contain correct paths? Correct paths are those pointing to where the actual librepo files are installed, thus the custom prefix.

@jan-kolarik
Copy link
Member

I think this is the way how the dbox container is working. It links the libraries using the LD_LIBRARY_PATH variable. When adding the LIBREPO_LDFLAGS to the CMake file it results in adding the -L/usr/lib64 which I guess overtakes the LD_LIBRARY_PATH and the devel library is not found then. As I said, I am not any expert in using CMake and build options, it's just the dbox used to be a standard way how to deploy the dnf devel stack and this seems to be breaking the stuff.

@jan-kolarik
Copy link
Member

jan-kolarik commented Feb 15, 2023

I have only the system-installed librepo and the development one. When I remove both .pc then pkg-config doesn't find it then:

[root@2944397590db dnf5]# pkg-config --libs librepo
Package librepo was not found in the pkg-config search path.
Perhaps you should add the directory containing `librepo.pc'
to the PKG_CONFIG_PATH environment variable
Package 'librepo', required by 'virtual:world', not found

This is the default librepo configuration from the dbox stack:

make ../..
    -DPYTHON_DESIRED=3
    -DCMAKE_MODULE_PATH=$CMAKE_MODULE_PATH
    -DCMAKE_PREFIX_PATH=$CMAKE_PREFIX_PATH
    -DCMAKE_CXX_FLAGS="$(pkg-config -cflags libsolv)"
    -DCMAKE_C_FLAGS="$(pkg-config -cflags libsolv)"
    -DCMAKE_INSTALL_PREFIX=/usr/
    -DCMAKE_BUILD_TYPE=Debug

AFAIK this is purely handled with environmental variables by dbox:

# environment variables used in the path tweaks
PATHS_ENV = {
    "LIBDIR": sysconfig.get_config_var("LIBDIR"),
}

# paths to be used in project path tweaks
PATHS = {
    "CMAKE_MODULE_PATH": ["/usr/share/cmake/Modules"],
    "CMAKE_PREFIX_PATH": ["/usr"],
    "CPATH": ["/usr/include"],
    "LD_LIBRARY_PATH": [PATHS_ENV["LIBDIR"]],
    "LIBRARY_PATH": [PATHS_ENV["LIBDIR"]],
    "PATH": ["/usr/sbin", "/usr/bin"],
    "PKG_CONFIG_PATH": [sysconfig.get_config_var("LIBPC")],
    "PKG_CONFIG_SYSTEM_INCLUDE_PATH": ["/usr/include"],
    "PYTHONPATH": site.getsitepackages(),
}

See here.

@jan-kolarik
Copy link
Member

jan-kolarik commented Feb 15, 2023

So I guess I already see the issue there, that /usr prefix in the configuration which is in this case not the real path in the container, but in the end it gets probably transformed into something like this for librepo example: /root/dbox/librepo/_install/fedora-latest/usr.

@mcrha
Copy link
Contributor Author

mcrha commented Feb 15, 2023

-DCMAKE_INSTALL_PREFIX=/usr/

Right, you do configure the custom prefix, which matches the system prefix, then all the things are confused and break. A way to workaround it is to uninstall librepo-devel package. Then, when the linker links the libs together, it'll not find the one in the real /usr/lib64/, but under the right place.

I consider this a bug in your build environment (dbox). You've been really lucky this did not strike earlier. You do not know until the lib adds a new symbol and you use that new symbol. As you said earlier, the LD_LIBRARY_PATH could help here, but it seems it's added only after the target's target_link_libraries.

The unfortunate thing is that this change fixes things "for me", but breaks them "for you". I do not know how to fix it "for both of us" in the DNF5 project. The pkg-config file for the librepo in your build environment obviously lies, but whether there can be done anything about it I do not know.

@jan-kolarik
Copy link
Member

jan-kolarik commented Feb 15, 2023

Thanks again for you patience 🙂 The removal of librepo-devel package indeed helps, but of course it means modifying the default container state. I will try to find a solution that needs only modifying the dbox configuration if it's possible. But as you've said this is considered as a bug outside of the dnf5 component, therefore I am for keeping this change. It also seems I am the only one still using the dbox for the development in our team, so ... 😃

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.

3 participants