patch 2/2 debuginfod server etc.

[Frank Ch. Eigler]

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