search

poelzi / ulatencyd

daemon to minimize latency on a linux system using cgroups
GNU General Public License v3.0
243 stars 30 forks source link

Unbundle libraries #46

Closed Hubbitus closed 10 years ago

Hubbitus commented 11 years ago

Hello. I'm Fedora Linux maintainer and try package ulatencyd. However it is bundled part of bc, coreutils and proc from GNU project. It is strongly prohibited by our guidelines: https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries

I've found you comment about libproc: # FIXME: libproc should export more symbols. Is it solvable? Could you please provide ability to link with system libproc and work withit public API?

What with bc and coreutils?

With best whishes, Pavel Alexeev.

gajdusek commented 10 years ago

Hello, sorry for answering so late.

I am closing this issue as ulatencyd now contains no bundled libraries. I have opened bug #48 to track issues with dynamically linked procps.

I wish this will help ulatencyd on its way to Fedora.

Cheers, Petr

Hubbitus commented 10 years ago

Thank you very much and sorry for the late response.

Unfortunately this commit failed to build after remote bundled directories libs which should not be used now (rm -rf src/{bc,coreutils,proc}):

+ cmake -DLIBCGROUPS=0 -DCMAKE_INSTALL_PREFIX=/usr -DENABLE_DBUS=1 -DLUA_JIT=0 -DPROCPS_STATIC:BOOL=OFF .
CMake Error at /usr/share/cmake/Modules/FindPkgConfig.cmake:279 (message):
  A required package was not found
Call Stack (most recent call first):
  /usr/share/cmake/Modules/FindPkgConfig.cmake:333 (_pkg_check_modules_internal)
  CMakeLists.txt:45 (pkg_check_modules)

CMake Error at CMakeLists.txt:111 (add_subdirectory):
  The source directory

    /home/pasha/SOFT/rpmbuild/BUILD/ulatencyd.b30c5ef/src/bc

  does not contain a CMakeLists.txt file.
Hubbitus commented 10 years ago

"add_subdirectory(src/bc)" line still present in CMakeLists.txt

gajdusek commented 10 years ago

Hi,

sorry I was not specific enough. The mentioned commit removed building procps and coreutils, not the bc. Coreutils stayed in the tree, renamed to coreutils_delete, and unused. bc is removed since 758e602093 and coreutils_delete directory is removed since 759b9cb6ac. But you should use master branch HEAD, which contains more fixes and better handling of external static procps. Note that you need external static procps or patch procps to export more symbols, master HEAD will inform you about missing symbols if you try to build against shared procps, so at least you must not search the code. If you wish I will tag the last commit. Still I think it's not ready for stable release (e.g. missing logind support), but it should fix many issues. Currently I am working on other things I feel are needed, and want to merge them to master before I will propose @poelzi new release.

Hubbitus commented 10 years ago

Ok, thank you. I've decide packaging master HEAD branch and indeed it found many not exported symbols from current shared procps. And we have not static. So now only two ways: to wait until procps redesign happened (do you known any planned date?), or request exception for bundling.

gajdusek commented 10 years ago

IIRC plans are to refactor the whole API, get rid of global exported variables etc. As the code is not easily readable (due strong optimizations), no public headers matching the exported symbols, code had several maintainers over last 10 maybe 20 years and was occasionally abandon / unmaintained and because it seems progress is targeted on fixing / improving ps utils, I fear it may take a long time.

It is also possible that ulatencyd will not use procps in the far future.

Hubbitus commented 10 years ago

Thank you for the answer. Sad to heard. Request exception with arguments what coming implementation is not ready yet have much more chances to be approved (possible temporary). Arguments like very poor design, mesh of code and lack of developers make it near impossible unfortunately.

gajdusek commented 10 years ago

I didn't want to deem its design, I am not competent - not familiar with the code and not an extraordinary good C coder. True is, it is not easily readable for me due strong optimizations it uses.

Ad lack of developers: It is collaborative project (fork) of Debian, Fedora and OpenSUSE, and if I briefly look at https://gitorious.org/procps I see active development, just their current priorities seems to be other than making it fully usable as shared library for foreign projects, which like to continue use all symbols already exported or even more of them.

I looked again at https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=685962 which I filled some time ago, and see there are mentioned plans to remove vminfo() (and meminfo() ?) and all variables exported by them: vm* and kb*

I tried to build against newest procps availale in Debian and I got following missing symbols:

group_from_gid kb_inact_laundry kb_inact_dirty kb_inact_clean kb_inact_target kb_swap_cached kb_writeback kb_slab kb_committed_as kb_dirty kb_mapped kb_pagetables vminfo vm_nr_dirty vm_nr_writeback vm_nr_pagecache vm_nr_page_table_pages vm_nr_reverse_maps vm_nr_mapped vm_nr_slab vm_pgpgin vm_pgpgout vm_pswpin vm_pswpout vm_pgalloc vm_pgfree vm_pgactivate vm_pgdeactivate vm_pgfault vm_pgmajfault vm_pgscan vm_pgrefill vm_pgsteal vm_kswapd_steal vm_pageoutrun vm_allocstall

So if ulatencyd will implement its own vminfo(), meminfo() and group_from_gid(), it would be possible to link against current procps. group_from_gid() could be probably removed entirely. I will do that on weekend.

Note that ulatencyd does not currently use all of these, but more will be used in future. And it has no sense to export only some of them to LUA, moreover users may already use them in their custom rules.

Hubbitus commented 10 years ago

Thank you very much for work and effort to resolve this issue.