https://bugzilla.redhat.com/show_bug.cgi?id=1826439
Michal Schmidt <mschmidt(a)redhat.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Flags|needinfo?(mschmidt(a)redhat.c |
|om) |
--- Comment #21 from Michal Schmidt <mschmidt(a)redhat.com> ---
Hi Igor,
I have not added you to the packagers group yet. I have some questions and
comments about the package.
libvma.spec:
BuildRequires: libibverbs-devel
BuildRequires: rdma-core-devel
These days, libibverbs-devel is provided by rdma-core-devel.
BuildRequires: libnl3-devel
Your config/m4/nl.m4 uses PKG_CHECK_MODULES, so you should BR
pkgconfig(libnl-route-3.0) instead.
https://docs.fedoraproject.org/en-US/packaging-guidelines/PkgConfigBuildR...
%build
export revision=%{use_rel}
This seems unnecessary. I did not find anything referencing the "revision"
environment variable in your build system.
%install
%{make_build} DESTDIR=${RPM_BUILD_ROOT} install
You should use %{make_install} instead.
%files
%{_libdir}/%{name}.so*
So the package contains something like this:
libvma.so -> libvma.so.9.1.0
libvma.so.9 -> libvma.so.9.1.0
libvma.so.9.1.0
Since libvma.so is to be used only for LD_PRELOAD and no program should be
linked against it (and have DT_NEEDED for it),:
1. is there a point in versioning the library?
2. does it have to be installed in the linker's default path? Would it make
more sense to have it as %{_libdir}/libvma/libvma.so ?
Do you actually use configure_options for anything? I don't think it's common
for Fedora packages to have macros like that "just in case".
use_rel - This is fascinating. The macro not only determines the value of the
Release tag, it also influences the build where its value gets used for version
compatibility checks and encoded in version strings. To me the mechanism seems
needlessly complicated and I fail to see the benefit of it. Also, are you aware
that sometimes automatic scripts (e.g. release engineering mass rebuilds) bump
the Release fields in Fedora spec files? Normally in RPM, Version is the
upstream version and Release serves packaging needs.
30-libvma-limits.conf:
# Default limits that are needed for proper work of libvma
# Read more about this topic in the VMA's User Manual
Can you point me to where in the manual is this discussed?
* - memlock unlimited
* soft memlock unlimited
* hard memlock unlimited
Does having the package installed give every user on the system the permission
to mlock unlimited amounts of RAM? Is that really necessary? Could it be at
least limited to a user group?
vma.service:
[Unit]
Description=VMA Daemon. Version: 9.1.0-0
Please consider dropping the version from the Description. No other service has
that.
After=network.target syslog.target
syslog.target became irrelevant and was removed in 2013. Please remove that
dependency. I am aware some packages still have that, but it's just cargo-cult
now.
Requires=network.target
This does not make sense for your daemon. See man systemd.special(7) about the
intended use of network.target.
[Service]
Type=forking
Restart=on-failure
ExecStart=/usr/sbin/vma start
ExecStop=/usr/sbin/vma stop
/usr/sbin/vma is a SysV initscript. That's terrible.
Why not start the deamon binary directly?:
ExecStart=/usr/sbin/vmad
(And maybe set KillSignal if needed.)
ExecReload=/usr/sbin/vma restart
You must not fake the reload operation with restart. If the service cannot do a
reload, just do not define ExecReload.
RestartForceExitStatus=1 SIGTERM
It's unusual to need to use this setting. There may be a good reason for it,
but please double check.
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component