patch 2/2 debuginfod server etc.
Mark Wielaard
mark@klomp.org
Wed Nov 13 17:22:00 GMT 2019
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 <fche@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 <mark@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 <mark@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
More information about the Elfutils-devel
mailing list