Hi, On Mon, 2019-10-28 at 15:07 -0400, Frank Ch. Eigler wrote: > Add the server to the debuginfod/ subdirectory. This is a highly > multithreaded c++11 program (still buildable on rhel7's gcc 4.8, > which is only partly c++11 compliant). Includes an initial suite > of tests, man pages, and a sample systemd service.
Some feedback on the first part, the config directory additions. > diff --git a/config/ChangeLog b/config/ChangeLog > index b641d0d5b74e..73643f919406 100644 > --- a/config/ChangeLog > +++ b/config/ChangeLog > @@ -1,3 +1,10 @@ > +2019-10-28 Frank Ch. Eigler <f...@redhat.com> > + > + * eu.am (AM_CXXFLAGS): Clone & amend AM_CFLAGS for c++11 code. > + * debuginfod.service, debuginfod.sysconfig: New files: systemd. > + * Makefile.am: Install them. > + * elfutils.spec.in: Add debuginfod and debuginfod-client subrpms. > + > 2019-08-29 Mark Wielaard <m...@klomp.org> > > * elfutils.spec.in (%description devel): Remove libebl text. > diff --git a/config/Makefile.am b/config/Makefile.am > index 6425718efc54..7c75f59880d3 100644 > --- a/config/Makefile.am > +++ b/config/Makefile.am > @@ -28,8 +28,8 @@ > ## the GNU Lesser General Public License along with this program. If > ## not, see <http://www.gnu.org/licenses/>. > ## > -EXTRA_DIST = elfutils.spec.in known-dwarf.awk 10-default-yama-scope.conf > - libelf.pc.in libdw.pc.in > +EXTRA_DIST = elfutils.spec.in known-dwarf.awk 10-default-yama-scope.conf \ > + libelf.pc.in libdw.pc.in libdebuginfod.pc.in debuginfod.service > debuginfod.sysconfig > > pkgconfigdir = $(libdir)/pkgconfig > pkgconfig_DATA = libelf.pc libdw.pc libdebuginfod.pc OK. > diff --git a/config/debuginfod.service b/config/debuginfod.service > new file mode 100644 > index 000000000000..15a8808baf0c > --- /dev/null > +++ b/config/debuginfod.service > @@ -0,0 +1,15 @@ > +[Unit] > +Description=elfutils debuginfo-over-http server > +Documentation=http://elfutils.org/ > +After=network.target > + > +[Service] > +EnvironmentFile=/etc/sysconfig/debuginfod > +User=debuginfod > +Group=debuginfod > +#CacheDirectory=debuginfod > +ExecStart=/usr/bin/debuginfod -d /var/cache/debuginfod/debuginfod.sqlite -p > $DEBUGINFOD_PORT $DEBUGINFOD_VERBOSE $DEBUGINFOD_PATHS > +TimeoutStopSec=10 Why is CacheDirectory commented out, I cannot find it anywhere else in the code. > +[Install] > +WantedBy=multi-user.target > diff --git a/config/debuginfod.sysconfig b/config/debuginfod.sysconfig > new file mode 100644 > index 000000000000..bee42a4f443a > --- /dev/null > +++ b/config/debuginfod.sysconfig > @@ -0,0 +1,9 @@ > +# > +DEBUGINFOD_PORT="8002" > +#DEBUGINFOD_VERBOSE="-v" > +DEBUGINFOD_PATHS="/usr/lib/debug /usr/bin /usr/sbin /usr/lib /usr/lib64 > /usr/local" Should this also include /usr/libexec ? Isn't /usr/local too broad? Should it also include /opt and/or /srv? > +# upstream debuginfods > +#DEBUGINFOD_URLS="http://secondhost:8002 http://thirdhost:8002"; > +#DEBUGINFOD_TIMEOUT="5" > +#DEBUGINFOD_CACHE_DIR="" > diff --git a/config/elfutils.spec.in b/config/elfutils.spec.in > index 6771d13ba740..694f8fde2f48 100644 > --- a/config/elfutils.spec.in > +++ b/config/elfutils.spec.in > @@ -23,10 +23,22 @@ BuildRequires: flex >= 2.5.4a > BuildRequires: bzip2 > BuildRequires: m4 > BuildRequires: gettext > -BuildRequires: zlib-devel > +BuildRequires: pkgconfig(zlib) > +%if 0%{?rhel} == 7 > BuildRequires: bzip2-devel > -BuildRequires: xz-devel > +%else > +BuildRequires: pkgconfig(bzip2) > +%endif > +BuildRequires: pkgconfig(liblzma) So now we only need the -devel package for bzip2 because it might not have a pkgconfig file. Good. I am slightly confused about xz-devel vs liblzma. Could you remind me again what the relation was? I see we do check for liblzma with pkgconfig in configure.ac. So I assume the change is correct. Just confused about the naming. > BuildRequires: gcc-c++ > +BuildRequires: pkgconfig(libmicrohttpd) >= 0.9.33 > +BuildRequires: pkgconfig(libcurl) >= 7.29.0 > +BuildRequires: pkgconfig(sqlite3) >= 3.7.17 > +BuildRequires: pkgconfig(libarchive) >= 3.1.2 > +%if 0%{?rhel} >= 8 || 0%{?fedora} >= 20 > +Recommends: elfutils-debuginfod-client > +%endif > + Should we add %else Requires: elfutils-debuginfod-client? Also I think it would be more correct to move the Recommends/Requires up with the other Requires. (Side note, in fedora we actually have a separate libs subpackage, in fedora it should be moved there.) > %define _gnu %{nil} > %define _programprefix eu- > @@ -116,18 +128,45 @@ interprocess services, communication and introspection > (like synchronisation, signaling, debugging, tracing and > profiling) of processes. > > +%package debuginfod-client > +Summary: Libraries and command-line frontend for HTTP ELF/DWARF file server > addressed by build-id. > +License: GPLv2+ That should probably be: GPLv3+ and (GPLv2+ or LGPLv3+) ^ for the binary, ^ for the library > +%package debuginfod > +Summary: HTTP ELF/DWARF file server addressed by build-id. > +License: GPLv2+ GPLv3+ > +BuildRequires: systemd > +Requires(post): systemd > +Requires(preun): systemd > +Requires(postun): systemd > +Requires: shadow-utils > +Requires: /usr/bin/rpm2cpio Should that be Requires(pre): shadow-utils? Because you only need it it the pre-script? > +%description debuginfod-client > +The elfutils-debuginfod-client package contains shared libraries > +dynamically loaded from -ldw, which use a debuginfod service > +to look up debuginfo and associated data. Also includes a > +command-line frontend. > + > +%description debuginfod > +The elfutils-debuginfod package contains the debuginfod binary > +and control files for a service that can provide ELF/DWARF > +files to remote clients, based on build-id identification. > +The ELF/DWARF file searching functions in libdwfl can query > +such servers to download those files on demand. > + > %prep > %setup -q > > %build > -%configure --program-prefix=%{_programprefix} > +%configure --program-prefix=%{_programprefix} --enable-debuginfod > make > > %install > rm -rf ${RPM_BUILD_ROOT} > mkdir -p ${RPM_BUILD_ROOT}%{_prefix} > > -%makeinstall > +%make_install O. Why? What? Probably fine, just different. > chmod +x ${RPM_BUILD_ROOT}%{_prefix}/%{_lib}/lib*.so* > > @@ -140,6 +179,11 @@ chmod +x ${RPM_BUILD_ROOT}%{_prefix}/%{_lib}/lib*.so* > > install -Dm0644 config/10-default-yama-scope.conf > ${RPM_BUILD_ROOT}%{_sysctldir}/10-default-yama-scope.conf > > +install -Dm0644 config/debuginfod.service > ${RPM_BUILD_ROOT}%{_unitdir}/debuginfod.service > +install -Dm0644 config/debuginfod.sysconfig > ${RPM_BUILD_ROOT}%{_sysconfdir}/sysconfig/debuginfod > +mkdir -p ${RPM_BUILD_ROOT}%{_localstatedir}/cache/debuginfod > +touch ${RPM_BUILD_ROOT}%{_localstatedir}/cache/debuginfod/debuginfod.sqlite > + > %check > make check OK. > @@ -190,6 +234,7 @@ rm -rf ${RPM_BUILD_ROOT} > %dir %{_includedir}/elfutils > %{_includedir}/elfutils/elf-knowledge.h > %{_includedir}/elfutils/known-dwarf.h > +%{_includedir}/elfutils/debuginfod.h > #%{_includedir}/elfutils/libasm.h > %{_includedir}/elfutils/libdw.h > %{_includedir}/elfutils/libdwfl.h > @@ -197,6 +242,7 @@ rm -rf ${RPM_BUILD_ROOT} > %{_includedir}/elfutils/version.h > #%{_libdir}/libasm.so > %{_libdir}/libdw.so > +%{_libdir}/libdebuginfod.so > %{_libdir}/pkgconfig/libdw.pc Since the debuginfod client stuff is actually independent from the other devel libraries I think these should not go here, but... > %files devel-static > @@ -225,6 +271,43 @@ rm -rf ${RPM_BUILD_ROOT} > %files default-yama-scope > %{_sysctldir}/10-default-yama-scope.conf > > + > +%files debuginfod-client > +%defattr(-,root,root) > +%{_libdir}/libdebuginfod.so.* > +%{_libdir}/libdebuginfod-%{version}.so > +%{_libdir}/pkgconfig/libdebuginfod.pc > +%{_bindir}/debuginfod-find > +%{_mandir}/man1/debuginfod-find.1* > +%{_mandir}/man3/debuginfod_find_debuginfo.3* > +%{_mandir}/man3/debuginfod_find_executable.3* > +%{_mandir}/man3/debuginfod_find_source.3* ...here, but I think we should also split this into debuginfod-client with just the shared library and binary plus manpage. And debuginfod- client-devel with the header file, pkgconfig and other man-pages. > +%files debuginfod > +%defattr(-,root,root) > +%{_bindir}/debuginfod > +%config(noreplace) %verify(not md5 size mtime) > %{_sysconfdir}/sysconfig/debuginfod > +%{_unitdir}/debuginfod.service > +%{_sysconfdir}/sysconfig/debuginfod > +%{_mandir}/man8/debuginfod.8* > + > +%dir %attr(0700,debuginfod,debuginfod) %{_localstatedir}/cache/debuginfod > +%verify(not md5 size mtime) %attr(0600,debuginfod,debuginfod) > %{_localstatedir}/cache/debuginfod/debuginfod.sqlite > + > +%pre debuginfod > +getent group debuginfod >/dev/null || groupadd -r debuginfod > +getent passwd debuginfod >/dev/null || \ > + useradd -r -g debuginfod -d /var/cache/debuginfod -s /sbin/nologin \ > + -c "elfutils debuginfo server" debuginfod > +exit 0 OK, I see this follows https://docs.fedoraproject.org/en-US/packaging-guidelines/UsersAndGroups/#_dynamic_allocation > +%post debuginfod > +%systemd_post debuginfod.service > + > +%postun debuginfod > +%systemd_postun_with_restart debuginfod.service > + > %changelog > * Tue Aug 13 2019 Mark Wielaard <m...@klomp.org> 0.177-1 > - elfclassify: New tool to analyze ELF objects. > diff --git a/config/eu.am b/config/eu.am > index 82acda3ab2cd..6c3c444f143a 100644 > --- a/config/eu.am > +++ b/config/eu.am > @@ -79,6 +79,16 @@ AM_CFLAGS = -std=gnu99 -Wall -Wshadow -Wformat=2 \ > $(if $($(*F)_no_Wpacked_not_aligned),-Wno-packed-not-aligned,) \ > $($(*F)_CFLAGS) > > +AM_CXXFLAGS = -std=c++11 -Wall -Wshadow \ > + -Wtrampolines \ > + $(LOGICAL_OP_WARNING) $(DUPLICATED_COND_WARNING) \ > + $(NULL_DEREFERENCE_WARNING) $(IMPLICIT_FALLTHROUGH_WARNING) \ > + $(if $($(*F)_no_Werror),,-Werror) \ > + $(if $($(*F)_no_Wunused),,-Wunused -Wextra) \ > + $(if $($(*F)_no_Wstack_usage),,$(STACK_USAGE_WARNING)) \ > + $(if $($(*F)_no_Wpacked_not_aligned),-Wno-packed-not-aligned,) \ > + $($(*F)_CXXFLAGS) > + > COMPILE.os = $(filter-out -fprofile-arcs -ftest-coverage, $(COMPILE)) > > DEFS.os = -DPIC -DSHARED Looks good. Maybe there are some C++ specific warnings we could enable? Not a priority of course. Thanks, Mark
