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

Wato #161

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Wato #161

wants to merge 4 commits into from

Conversation

watologo1
Copy link

@pboyd04
Copy link
Contributor

pboyd04 commented Feb 24, 2023

This is breaking standard cmake builds. Additionally, the crypto library isn't directly used so target_link_libraries(redfish ${OPENSSL_SSL_LIBRARY}) should have be sufficient. What issues where you having?

@watologo1
Copy link
Author

watologo1 commented Feb 24, 2023

Without this line I see:

[    4s] /usr/bin/cc -fPIC -O2 -Wall -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -fstack-protector-strong -funwind-tables -fasynchronous-unwind-tables -fstack-clash-protection -Werror=return-type -flto=auto -D_FORTIFY_SOURCE=2 -Wall -Wextra -Wdeclaration-after-statement -Wshadow -Wformat=2 -ggdb3 -O2 -Wnull-dereference -Wduplicated-cond -Wduplicated-branches -Wlogical-op -O2 -g -DNDEBUG -flto=auto -Wl,--as-needed -Wl,--no-undefined -Wl,-z,now -shared -Wl,-soname,libredfish.so.1 -o lib/libredfish.so.1.99.0 CMakeFiles/redfish.dir/src/asyncEvent.c.o CMakeFiles/redfish.dir/src/asyncRaw.c.o CMakeFiles/redfish.dir/src/entities/chassis.c.o CMakeFiles/redfish.dir/src/entities/resource.c.o CMakeFiles/redfish.dir/src/main.c.o CMakeFiles/redfish.dir/src/payload.c.o CMakeFiles/redfish.dir/src/queue.c.o CMakeFiles/redfish.dir/src/redpath.c.o CMakeFiles/redfish.dir/src/service.c.o CMakeFiles/redfish.dir/src/util.c.o  -ljansson -lcurl -lczmq -lssl 
[    4s] /home/abuild/rpmbuild/BUILD/libredfish-1.3.6.4+git.a0788d4/src/service.c: In function 'createServiceEnumeratorBasicAuth':
[    4s] /home/abuild/rpmbuild/BUILD/libredfish-1.3.6.4+git.a0788d4/src/service.c:1344:19: warning: potential null pointer dereference [-Wnull-dereference]
[    4s]  1344 |     ret->versions = getVersions(ret, rootUri);
[    4s]       |                   ^
[    4s] /home/abuild/rpmbuild/BUILD/libredfish-1.3.6.4+git.a0788d4/src/service.c:1343:20: warning: potential null pointer dereference [-Wnull-dereference]
[    4s]  1343 |     ret->otherAuth = safeStrdup(userPass);
[    4s]       |                    ^
[    4s] /usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: /tmp/ccRpRkKn.ltrans0.ltrans.o: in function `listenOpenSSL':
[    4s] /home/abuild/rpmbuild/BUILD/libredfish-1.3.6.4+git.a0788d4/src/asyncEvent.c:1511: undefined reference to `EVP_PKEY_new'
[    4s] /usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: /tmp/ccRpRkKn.ltrans0.ltrans.o:/home/abuild/rpmbuild/BUILD/libredfish-1.3.6.4+git.a0788d4/src/asyncEvent.c:1512: undefined reference to `X509_new'
[    4s] /usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: /tmp/ccRpRkKn.ltrans0.ltrans.o:/home/abuild/rpmbuild/BUILD/libredfish-1.3.6.4+git.a0788d4/src/asyncEvent.c:1513: undefined reference to `RSA_new'
[    4s] /usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: /tmp/ccRpRkKn.ltrans0.ltrans.o:/home/abuild/rpmbuild/BUILD/libredfish-1.3.6.4+git.a0788d4/src/asyncEvent.c:1516: undefined reference to `BN_new'

and some more undefined references.
Things build sucessfully when -lcrypto is added.

@watologo1
Copy link
Author

watologo1 commented Feb 24, 2023

Here exactly the same build, but with -lcrypto:

/usr/bin/cc -fPIC -O2 -Wall -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -fstack-protector-strong -funwind-tables -fasynchronous-unwind-tables -fstack-clash-protection -Werror=return-type -flto=auto -D_FORTIFY_SOURCE=2 -Wall -Wextra -Wdeclaration-after-statement -Wshadow -Wformat=2 -ggdb3 -O2 -Wnull-dereference -Wduplicated-cond -Wduplicated-branches -Wlogical-op -O2 -g -DNDEBUG -flto=auto -Wl,--as-needed -Wl,--no-undefined -Wl,-z,now -shared -Wl,-soname,libredfish.so.1 -o lib/libredfish.so.1.99.0 CMakeFiles/redfish.dir/src/asyncEvent.c.o CMakeFiles/redfish.dir/src/asyncRaw.c.o CMakeFiles/redfish.dir/src/entities/chassis.c.o CMakeFiles/redfish.dir/src/entities/resource.c.o CMakeFiles/redfish.dir/src/main.c.o CMakeFiles/redfish.dir/src/payload.c.o CMakeFiles/redfish.dir/src/queue.c.o CMakeFiles/redfish.dir/src/redpath.c.o CMakeFiles/redfish.dir/src/service.c.o CMakeFiles/redfish.dir/src/util.c.o  -ljansson -lcurl -lczmq -lssl -lcrypto 
[    4s] /home/abuild/rpmbuild/BUILD/libredfish-1.3.6.4+git.a0788d4/src/service.c: In function 'createServiceEnumeratorBasicAuth':
[    4s] /home/abuild/rpmbuild/BUILD/libredfish-1.3.6.4+git.a0788d4/src/service.c:1344:19: warning: potential null pointer dereference [-Wnull-dereference]
[    4s]  1344 |     ret->versions = getVersions(ret, rootUri);
[    4s]       |                   ^
[    4s] /home/abuild/rpmbuild/BUILD/libredfish-1.3.6.4+git.a0788d4/src/service.c:1343:20: warning: potential null pointer dereference [-Wnull-dereference]
[    4s]  1343 |     ret->otherAuth = safeStrdup(userPass);
[    4s]       |                    ^
[    4s] /usr/bin/cmake -E cmake_symlink_library lib/libredfish.so.1.99.0 lib/libredfish.so.1 lib/libredfish.so
[    4s] make[2]: Leaving directory '/home/abuild/rpmbuild/BUILD/libredfish-1.3.6.4+git.a0788d4/build'
[    4s] make[1]: Entering directory '/home/abuild/rpmbuild/BUILD/libredfish-1.3.6.4+git.a0788d4/build'
[    4s] [ 61%] Built target redfish

@watologo1
Copy link
Author

watologo1 commented Feb 24, 2023

The failing Travis links seem not to come from the crypto change.

One breakage seem to come from the thread linking addition.
It seems as if it cannot reference Threads::Threads:
/usr/bin/ld: cannot find -lThreads::Threads
It should get -lpthreads

I could limit this to one build failing. I expect it has to do with cmake version. I already found hints that this may vary depending on the version, but I could not spot the cmake version used.

Windows builds should always fail, due to missing certificate?

@watologo1 watologo1 force-pushed the wato branch 2 times, most recently from 6b11e4c to 9d5c1ba Compare February 24, 2023 22:34
Otherwise one gets quite some undefined references...
For some reason this was not needed when I built in latest Tumbleweed env.
It seems as if phtread code moved into glibc or something more general,
but this helped to build against Leap 15.4 and Tumbleweed still built
successfully.
otherwise %cmake rpm macros will not be able to install things to
the right places.
This will generate a libredfish.pc file that can/should be placed to
(in openSUSE at least):
/usr/lib64/pkgconfig
So that Linux systems making use of pkg-config are aware of libredfish
library
@mraineri
Copy link
Contributor

Right now even the main branch fails on Windows; we'll need to look at that further as a separate issue.

@pboyd04
Copy link
Contributor

pboyd04 commented Mar 29, 2023

I'm able to build one open SUSE leap 15.4 without these changes. I'm still unclear on why this is needed.

@watologo1
Copy link
Author

watologo1 commented Apr 3, 2023

I'm able to build one open SUSE leap 15.4 without these changes.

Can you provide details on how you build, please.
You can reproduce the issues I see when you create an openSUSE build service account and then "branch checkout" (bco) the libredfish developement project:
osc bco systemsmangement
then remove the patch I added: add_crypto.patch
and try to build again, either local by:
osc build --jobs 5
or on the server by checking in the change:
osc rm add_crypto.patch
osc ci

@watologo1
Copy link
Author

Sigh..., so I try to explain this a bit more:
When using a buildsystem (redhat or openbuild service), one must not use absolute installation paths.
The installation directories bin and lib do not work. One has to use the generic CMAKE ones:
${CMAKE_INSTALL_BINDIR}
{CMAKE_INSTALL_LIBDIR}

Like that rpmbuild cmake macros will do the right thing. In my case the libredfish.spec file part looks like this:
%build
%cmake -DLIB_INSTALL_DIR=%{_libdir}
%cmake_build

%install
%cmake_install

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