Hi - > > +++ b/config/debuginfod.service > > +[Service] > > +Group=debuginfod > > +#CacheDirectory=debuginfod > > +ExecStart=/usr/bin/debuginfod -d /var/cache/debuginfod/debuginfod.sqlite > > -p $DEBUGINFOD_PORT $DEBUGINFOD_VERBOSE $DEBUGINFOD_PATHS
> Why is CacheDirectory commented out, I cannot find it anywhere else in > the code. It's a placeholder for a recently added systemd capability (post-rhel7) to avoid hardcoding /var/cache/$SUBDIR paths. > > +++ b/config/debuginfod.sysconfig > > +#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? Not sure how much it matters. Added a few of them. It's not a problem to include a path that includes non-elf/dwarf non-rpm files; they'll be checked only once. > 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. Yeah, some distros put the xz libraries into one vs the other name. > > +%if 0%{?rhel} >= 8 || 0%{?fedora} >= 20 > > +Recommends: elfutils-debuginfod-client > > +%endif > > + > > Should we add %else Requires: elfutils-debuginfod-client? Up to you. Remember, we made the debuginfod client such that it was dlopen'd into libdw(fl) because you didn't want all the debuginfod-client (libcurl) required solibs to be loaded into the program - or into the minimal elfutils installation footprint. This would undo the latter. > 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.) OK. > > +%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 OK. > > +%package debuginfod > > +Summary: HTTP ELF/DWARF file server addressed by build-id. > > +License: GPLv2+ > > GPLv3+ OK. > > +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? Correct. > > -%makeinstall > > +%make_install > > O. Why? What? > Probably fine, just different. Yeah, fedora things. > 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. OK, if you don't mind the subrpm proliferation. > Maybe there are some C++ specific warnings we could enable? Will look into it, but wouldn't be surprised if g++ -Wall / -Wextra includes some already. - FChE