DMTF / libredfish

libRedfish is a C client library that allows for Creation of Entities (POST), Read of Entities (GET), Update of Entities (PATCH), Deletion of Entities (DELETE), running Actions (POST), receiving events, and providing some basic query abilities.
Other
48 stars 21 forks source link

Fix-Cmake: missing dependency at link edition for redfishcli #152

Closed matsKNY closed 3 years ago

matsKNY commented 3 years ago

(Hereinbelow comment associated with tests on RHEL8.3)

On RHEL8, the readline-devel RPM is not available (even when EPEL8 is taken into account). As a result, it is compulsory to compile and install readline from source.

At compile time (i.e while running make after the Makefile was generated through the action of cmake), it appears that some symbols used by libreadline are not defined:

/usr/local/lib/libreadline.so: undefined reference to 'tgetstr'
/usr/local/lib/libreadline.so: undefined reference to 'tputs'
/usr/local/lib/libreadline.so: undefined reference to 'BC'
/usr/local/lib/libreadline.so: undefined reference to 'tgetent'
/usr/local/lib/libreadline.so: undefined reference to 'tgetflag'
/usr/local/lib/libreadline.so: undefined reference to 'tgoto'
/usr/local/lib/libreadline.so: undefined reference to 'UP'
/usr/local/lib/libreadline.so: undefined reference to 'tgetnum'
/usr/local/lib/libreadline.so: undefined reference to 'PC'

Those symbols are defined by libtinfo, which can be installed from the official RHEL8 repositories. As a result, to be able to build redfishcli, which depends on libreadline, it seems that it is required to also link on libtinfo.

To do so, FindReadline.cmake was modified so as to look for the path to libtinfo, and return it into ${Tinfo_LIBRARY}. CMakeLists.txt was then modified to link to ${Tinfo_LIBRARY} when building redfishcli.

pboyd04 commented 3 years ago

I need to look into this a little more. We shouldn't need to list libraries that are dependencies of dependencies in our makefile. I'll see about getting a CentOS 8 box setup to test with in the next week or so.

pboyd04 commented 3 years ago

Redhat 8 (or at least the CentOS/AlmaLinux clones of them) have the libreadline-devel... https://centos.pkgs.org/8/centos-baseos-x86_64/readline-devel-7.0-10.el8.x86_64.rpm.html not sure why you are having issue with it...

pboyd04 commented 3 years ago

Pull #153 will have this starting to release EL8 (RHEL8, CentOS 8, AlamaLinux, etc) compatible RPMs.

matsKNY commented 3 years ago

First of all, thank you for your answers.

Redhat 8 (or at least the CentOS/AlmaLinux clones of them) have the libreadline-devel... https://centos.pkgs.org/8/centos-baseos-x86_64/readline-devel-7.0-10.el8.x86_64.rpm.html not sure why you are having issue with it...

Clones of RHEL8 do have the libreadline-devel RPM. But, as of now, neither is this RPM included in the RHEL8 official repositories nor it is in the EPEL8 ones. At least, no trace of it when I execute yum search libreadline-devel on an up-to-date RHEL8.3 compute node, with RHEL8 and EPEL8 official repositories configured and up-to-date. Additionally, I confirmed it with some googling:

I need to look into this a little more. We shouldn't need to list libraries that are dependencies of dependencies in our makefile. I'll see about getting a CentOS 8 box setup to test with in the next week or so.

It makes sense. Let me just point out that in the original FindReadline.cmake, ncurses is looked for, as it also defines the missing symbols. I changed it to look for libtinfo as it seemed more lightweight.

The only workaround I found to compile redfishcli on RHEL8 is to link to libtinfo (or another library defining the missing symbols). If another workaround does exist, I will be glad to know about it.

pboyd04 commented 3 years ago

Ah I see your problem. It's readline-devel not libreadline-devel. No idea why since it is a library, but yeah checking my companies internal RHEL 8 repo, the readline-devel package is still there for RHEL 8.X

matsKNY commented 3 years ago

Oh. Apologies for the mistake. I think this PR has no more reason to exist. Many thanks for your answers, time, and patience.