dftlibs / xcfun

XCFun: A library of exchange-correlation functionals with arbitrary-order derivatives
https://dftlibs.org/xcfun/
Mozilla Public License 2.0
57 stars 32 forks source link

[openSUSE] W: shared-lib-calls-exit from RPMLINT #139

Open kevinsmia1939 opened 4 years ago

kevinsmia1939 commented 4 years ago

Hello,

I am trying to package xcfun to openSUSE. https://build.opensuse.org/package/show/home:andythe_great/xcfun

I got a warning from RPMLINT as shown below.

libxcfun2.x86_64: W: shared-lib-calls-exit /usr/lib64/libxcfun.so.2 exit@GLIBC_2.2.5
This library package calls exit() or _exit(), probably in a non-fork()
context. Doing so from a library is strongly discouraged - when a library
function calls exit(), it prevents the calling program from handling the
error, reporting it to the user, closing files properly, and cleaning up any
state that the program has. It is preferred for the library to return an
actual error code and let the calling program decide how to handle the
situation.

There is probably exit use somewhere in a library?

Thank you.

bast commented 4 years ago

There are indeed some exit calls in tests/testall.cpp. But I am not sure these are the ones that are found. How can I reproduce this locally? How do you execute RPMLINT? (I have never used this before)

robertodr commented 4 years ago

This is probably the offending function: https://github.com/dftlibs/xcfun/blob/4614fe51c07890e06ec4f7d25f2b3395c4500a5e/src/config.hpp#L24-L29

kevinsmia1939 commented 4 years ago

There are indeed some exit calls in tests/testall.cpp. But I am not sure these are the ones that are found. How can I reproduce this locally? How do you execute RPMLINT? (I have never used this before)

RPMLINT was invoke by openSUSE build service, I' not sure how to run it separately myself, but the comment above might already found the issue.

bast commented 4 years ago

So if I understand correctly we should avoid calling the die() function in the code? It is called in a number of places.

robertodr commented 4 years ago

I understand that it's common practice for libraries to not call exit(), but rather return error codes and let the caller handle the failure. However, that is very rarely done in practice in the community that uses XCFun and the API partially reflects this. We would need to break the API and I don't think the resulting library would be used properly afterwards.

bast commented 4 years ago

I agree that not letting the code stop and giving the caller the decision would be better. But maybe we indeed need to package this with an API update. For scripted runs it is typically OK if the calculations stop if something unexpected happens (this is the situation now). Not crashing would be good for web-based portals that are now getting more and more fashionable. There we would not like the browser to crash just because we made a typo in the functional.

robertodr commented 4 years ago

Thanks for letting us know about this warning message from the packaging linter. As mentioned, the changes required would mean breaking the API. This is a massive investment in redesigning how XCFun would look and, most importantly, educating a community to a different programming style. This is currently low priority for us and I've labeled the issue accordingly. However, I'd like to keep the issue open as it raises a point that will be important to keep in mind if and when a version 3 happens.

kevinsmia1939 commented 4 years ago

Thanks