| Message ID | 1443729035-22348-1-git-send-email-rbryant@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
On Thu, Oct 01, 2015 at 03:50:35PM -0400, Russell Bryant wrote: > Commit e91b927d8966bfcb9768225392324dde4fd7d7f6 added optional usage of > the libcap-ng library. It's packaged in Fedora, so go ahead and added > it to the Fedora spec file. > > Our default systemd unit files don't make use of the --user option that > requires this library, but conceivably someone may want to customize > them and use this option. > > Signed-off-by: Russell Bryant <rbryant@redhat.com> I'd consider also adding --with-libcapng=yes to the configure command line. That way, a build without libcap-ng installed will fail. That will both root out problems in whatever RPM autobuilders happen to be running against OVS and make sure that every RPM build supports libcap-ng.
On 10/01/2015 03:54 PM, Ben Pfaff wrote: > On Thu, Oct 01, 2015 at 03:50:35PM -0400, Russell Bryant wrote: >> Commit e91b927d8966bfcb9768225392324dde4fd7d7f6 added optional usage of >> the libcap-ng library. It's packaged in Fedora, so go ahead and added >> it to the Fedora spec file. >> >> Our default systemd unit files don't make use of the --user option that >> requires this library, but conceivably someone may want to customize >> them and use this option. >> >> Signed-off-by: Russell Bryant <rbryant@redhat.com> > > I'd consider also adding --with-libcapng=yes to the configure command > line. That way, a build without libcap-ng installed will fail. That > will both root out problems in whatever RPM autobuilders happen to be > running against OVS and make sure that every RPM build supports > libcap-ng. > The rpm build would fail anyway if it's not installed at all. I tested that. It would not catch possible failures due to distro or package version differences. For example, if the header file was installed was installed in a different location and the configure script didn't find it, the package would still happily build now, but without libcap-ng support. --with-libcapng=yes would catch that, at least. Thanks for the suggestion. I'll send out a v2.
On Thu, Oct 01, 2015 at 04:00:09PM -0400, Russell Bryant wrote: > On 10/01/2015 03:54 PM, Ben Pfaff wrote: > > On Thu, Oct 01, 2015 at 03:50:35PM -0400, Russell Bryant wrote: > >> Commit e91b927d8966bfcb9768225392324dde4fd7d7f6 added optional usage of > >> the libcap-ng library. It's packaged in Fedora, so go ahead and added > >> it to the Fedora spec file. > >> > >> Our default systemd unit files don't make use of the --user option that > >> requires this library, but conceivably someone may want to customize > >> them and use this option. > >> > >> Signed-off-by: Russell Bryant <rbryant@redhat.com> > > > > I'd consider also adding --with-libcapng=yes to the configure command > > line. That way, a build without libcap-ng installed will fail. That > > will both root out problems in whatever RPM autobuilders happen to be > > running against OVS and make sure that every RPM build supports > > libcap-ng. > > > > The rpm build would fail anyway if it's not installed at all. I tested > that. That's because you are adding Requires and BuildRequires. The BuildRequires enforces that the libpcap-ng is installed during the RPM build. > It would not catch possible failures due to distro or package version > differences. For example, if the header file was installed was > installed in a different location and the configure script didn't find > it, the package would still happily build now, but without libcap-ng > support. --with-libcapng=yes would catch that, at least. The only issue I see is to have one more dependency to carry on which for some use cases might be a problem. Since this is an optional feature, worth to look at the %bcond_with and %bcond_without macros. We already use that to enable/disable running the testsuite. We could use the same approach to build with (default) or without libpcap-ng. Does that make sense? fbl > Thanks for the suggestion. I'll send out a v2.
On Thu, Oct 01, 2015 at 05:22:39PM -0300, Flavio Leitner wrote: > On Thu, Oct 01, 2015 at 04:00:09PM -0400, Russell Bryant wrote: > > On 10/01/2015 03:54 PM, Ben Pfaff wrote: > > > On Thu, Oct 01, 2015 at 03:50:35PM -0400, Russell Bryant wrote: > > >> Commit e91b927d8966bfcb9768225392324dde4fd7d7f6 added optional usage of > > >> the libcap-ng library. It's packaged in Fedora, so go ahead and added > > >> it to the Fedora spec file. > > >> > > >> Our default systemd unit files don't make use of the --user option that > > >> requires this library, but conceivably someone may want to customize > > >> them and use this option. > > >> > > >> Signed-off-by: Russell Bryant <rbryant@redhat.com> > > > > > > I'd consider also adding --with-libcapng=yes to the configure command > > > line. That way, a build without libcap-ng installed will fail. That > > > will both root out problems in whatever RPM autobuilders happen to be > > > running against OVS and make sure that every RPM build supports > > > libcap-ng. > > > It would not catch possible failures due to distro or package version > > differences. For example, if the header file was installed was > > installed in a different location and the configure script didn't find > > it, the package would still happily build now, but without libcap-ng > > support. --with-libcapng=yes would catch that, at least. > > The only issue I see is to have one more dependency to carry on which > for some use cases might be a problem. > > Since this is an optional feature, worth to look at the %bcond_with > and %bcond_without macros. We already use that to enable/disable > running the testsuite. We could use the same approach to build > with (default) or without libpcap-ng. > > Does that make sense? I sent out a patch to show what I am saying. Russel, I based my patch on yours, so if you like the patch, feel free to signed-off-by too, thanks! :) BTW, I haven't tested the binaries, just the rpm stuff. Ah, I removed the Requires to the library since RPM will catch that if needed. You can use 'rpm -qpR <rpm file>' to see the requirement list. Thanks! fbl
diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in index 695f1d7..4e46909 100644 --- a/rhel/openvswitch-fedora.spec.in +++ b/rhel/openvswitch-fedora.spec.in @@ -37,13 +37,14 @@ Source: http://openvswitch.org/releases/%{name}-%{version}.tar.gz BuildRequires: autoconf automake libtool BuildRequires: systemd-units openssl openssl-devel +BuildRequires: libcap-ng libcap-ng-devel BuildRequires: python python-twisted-core python-zope-interface PyQt4 BuildRequires: desktop-file-utils BuildRequires: groff graphviz # make check dependencies BuildRequires: procps-ng -Requires: openssl iproute module-init-tools +Requires: openssl iproute module-init-tools libcap-ng #Upstream kernel commit 4f647e0a3c37b8d5086214128614a136064110c3 #Requires: kernel >= 3.15.0-0
Commit e91b927d8966bfcb9768225392324dde4fd7d7f6 added optional usage of the libcap-ng library. It's packaged in Fedora, so go ahead and added it to the Fedora spec file. Our default systemd unit files don't make use of the --user option that requires this library, but conceivably someone may want to customize them and use this option. Signed-off-by: Russell Bryant <rbryant@redhat.com> --- rhel/openvswitch-fedora.spec.in | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)