[Swan-dev] commit war over spec file
Paul Wouters
paul at nohats.ca
Thu Feb 13 19:49:01 UTC 2020
I'm not very happy to see a commit/revert fight.
I was already not happy with the libreswan-testing.spec file. I believe
it serves no purpose, and since it is not actively maintained, it will
start to differentiate from the real fedora spec file. Previously, I
left it alone to avoid a fight. But now that the fight is hitting git,
let us re-evaluate this scenario.
Look at the diff between the "real" and the "testing" spec file:
--- libreswan-testing.spec 2020-02-13 14:08:55.306432457 -0500
+++ libreswan.spec 2020-02-12 15:25:00.614531238 -0500
@@ -1,42 +1,41 @@
%global _hardened_build 1
# These are rpm macros and are 0 or 1
-%global with_efence 1
-%global with_development 1
+%global with_efence 0
+%global with_development 0
+%global with_cavstests 1
%global nss_version 3.41
%global unbound_version 1.6.6
-%global with_cavstests 0
These globals are meant to come in via the rpmbuild command as
arguments. It should never be needed to manually set these. If
that does not work with the curent scheme, we should redo this
so it does work like that.
-%global _exec_prefix %{_prefix}/local
rpm installs should never live in /usr/local. They should live in
the regular root or in /opt. The directory /usr/local is meant for
local installs that are unmanaged by a package manager.
-%global initsystem @INITSYSTEM@
As fedora comes with only one init system, I'm not sure why this
line is here.
# Libreswan config options
%global libreswan_config \\\
- FINALDOCDIR=%{_pkgdocdir} \\\
- FINALEXAMPLECONFDIR=%{_pkgdocdir} \\\
FINALLIBEXECDIR=%{_libexecdir}/ipsec \\\
FINALMANDIR=%{_mandir} \\\
- FINALRUNDIR=%{_rundir}/pluto \\\
FIPSPRODUCTCHECK=%{_sysconfdir}/system-fips \\\
INC_RCDEFAULT=%{_initrddir} \\\
- INC_USRLOCAL=%{_exec_prefix} \\\
- IPSECVERSION=%{IPSECVERSION} \\\
- INITSYSTEM=%{initsystem} \\\
- USE_NSS_IPSEC_PROFILE=true \\\
+ INC_USRLOCAL=%{_prefix} \\\
+ INITSYSTEM=systemd \\\
PYTHON_BINARY=%{__python3} \\\
- SHELL_BINARY=%{_prefix}/bin/sh \\\
Here we see some new stuff that hasn't been ported to the test version
yet. We also see IPSECVERSION=%{IPSECVERSION} which I believe is the
sole reason for this file. As I explained in the past, an rpm version
is NOT an upstream version. It also can lead to the version reported
being different than the version from the source used. This override
is dangerous and should not be used. I argued this before when I
explained how I build "make rpm" targets. You set the version in the
tar ball. then you let rpm make its own version based on the proper
naming for pre-releases so that the upgrade path remains valid.
-%{nil}
-
+ SHELL_BINARY=%{_bindir}/sh \\\
+ USE_DNSSEC=true \\\
+ USE_FIPSCHECK=true \\\
+ USE_KLIPS=false \\\
+ USE_LABELED_IPSEC=true \\\
+ USE_LDAP=true \\\
+ USE_LIBCAP_NG=true \\\
+ USE_LIBCURL=true \\\
+ USE_LINUX_AUDIT=true \\\
+ USE_NM=true \\\
+ USE_NSS_IPSEC_PROFILE=true \\\
+ USE_SECCOMP=true \\\
+ USE_XAUTHPAM=true \\\
%{nil}
These are all missing in the testing version. I do not know why. I see
no reason why we would build a testing rpm without these features. Also,
the rpm global vars are ued to switch features on/off, like cavs testing
or development or efence builds. Anything using the USE_* libreswan
options is expected to never require changing when we build rpms. So
omitting these in test versions seem a rather custom hack for one
person, and not a valid generic way to build "testing" rpms. And again,
"testing" rpms should be build using the regular fedora spec file,
and changing the supported options for developing/testing, being:
%global with_efence 1
%global with_development 1
%global with_cavstests 1
#global prever rc1
-%global rel %{?prever:0.}1%{?prever:.%{prever}}
-# for pluto --version
-%global IPSECVERSION %{version}-%{rel}
Again, this is wrong as it is mixing up rpm versioning and upstream
versioning.
Name: libreswan
-Summary: IPsec implementation with IKEv1 and IKEv2 keying protocols
-# version is replaced in make target
-Version: 3.30
-Release: %{rel}%{?dist}
+Summary: Internet Key Exchange (IKEv1 and IKEv2) implementation for IPsec
+# version is generated in the release script
+Version: IPSECBASEVERSION
+Release: %{?prever:0.}1%{?prever:.%{prever}}%{?dist}
It's pretty bad that 3.30 here is hardcoded when we haven't released
that. It would create builds that mistakenly call themselves 3.30.
License: GPLv2
Url: https://libreswan.org/
Source0: https://download.libreswan.org/%{?prever:development/}%{name}-%{version}%{?prever}.tar.gz
@@ -45,40 +44,45 @@
Source2: https://download.libreswan.org/cavs/ikev1_psk.fax.bz2
Source3: https://download.libreswan.org/cavs/ikev2.fax.bz2
%endif
-BuildRequires: gcc
+
+BuildRequires: audit-libs-devel
BuildRequires: bison
+BuildRequires: curl-devel
+BuildRequires: fipscheck-devel
BuildRequires: flex
-BuildRequires: pkgconfig
-BuildRequires: systemd-devel
These seems due to re-ordering.
-Conflicts: openswan < %{version}-%{release}
-Obsoletes: openswan < %{version}-%{release}
-Provides: openswan = %{version}-%{release}
-Provides: openswan-doc = %{version}-%{release}
Same here.
-BuildRequires: pkgconfig hostname
-BuildRequires: nss-devel >= 3.16.1
-BuildRequires: nspr-devel
-BuildRequires: pam-devel
-BuildRequires: libevent-devel
-BuildRequires: unbound-devel >= 1.5.0-1
+BuildRequires: gcc
+BuildRequires: hostname
BuildRequires: ldns-devel
+BuildRequires: libcap-ng-devel
+BuildRequires: libevent-devel
BuildRequires: libseccomp-devel
BuildRequires: libselinux-devel
-BuildRequires: fipscheck-devel
-Requires: fipscheck%{_isa}
-Buildrequires: audit-libs-devel
-BuildRequires: libcap-ng-devel
+BuildRequires: nspr-devel
+BuildRequires: nss-devel >= %{nss_version}
BuildRequires: openldap-devel
-BuildRequires: curl-devel
+BuildRequires: pam-devel
+BuildRequires: pkgconfig
+BuildRequires: systemd-devel
+BuildRequires: unbound-devel >= %{unbound_version}
+BuildRequires: xmlto
%if 0%{with_efence}
BuildRequires: ElectricFence
%endif
-BuildRequires: xmlto
-
-Requires: nss-tools
-Requires: nss-softokn
+Requires: fipscheck%{_isa}
Requires: iproute >= 2.6.8
+Requires: nss >= %{nss_version}
+Requires: nss-softokn
+Requires: nss-tools
+Requires: unbound-libs >= %{unbound_version}
+Requires(post): bash
+Requires(post): coreutils
+Requires(post): systemd
+Requires(preun): systemd
+Requires(postun): systemd
+Conflicts: openswan < %{version}-%{release}
+Obsoletes: openswan < %{version}-%{release}
+Provides: openswan = %{version}-%{release}
+Provides: openswan-doc = %{version}-%{release}
Same here, but there is additional mismatch of nss version. The "real"
spec uses >= 3.41 and the testing spec uses nss-devel >= 3.16.1 instead.
%description
Libreswan is a free implementation of IPsec & IKE for Linux. IPsec is
@@ -90,15 +94,18 @@
tunnel is a virtual private network or VPN.
This package contains the daemons and userland tools for setting up
-Libreswan. To build KLIPS, see the kmod-libreswan.spec file.
+Libreswan.
-Libreswan also supports IKEv2 (RFC4309) and Secure Labeling
+Libreswan also supports IKEv2 (RFC7296) and Secure Labeling
More scars from libreswan-testing.spec not updating to updates to the
real libreswan.spec file.
Libreswan is based on Openswan-2.6.38 which in turn is based on FreeS/WAN-2.04
%prep
%setup -q -n libreswan-%{version}%{?prever}
+# enable crypto-policies support
sed -i "s:#[ ]*include \(.*\)\(/crypto-policies/back-ends/libreswan.config\)$:include \1\2:" programs/configs/ipsec.conf.in
+# linking to freebl is no longer needed
+sed -i "s/-lfreebl //" mk/config.mk
Also fixes to the "real" spec file not represented in the "testing" spec file.
%build
make %{?_smp_mflags} \
@@ -110,6 +117,7 @@
%if 0%{with_efence}
USE_EFENCE=true \
%endif
+ USERLINK="%{?__global_ldflags}" \
%{libreswan_config} \
programs
FS=$(pwd)
Missing an update again.
@@ -128,10 +136,10 @@
%{libreswan_config} \
install
FS=$(pwd)
-# Work around for FINALEXAMPLECONFDIR not working properly
-rm -rf %{buildroot}%{_prefix}/share/doc
+rm -rf %{buildroot}/usr/share/doc/libreswan
A hack needed in the testing spec file only.
+rm -rf %{buildroot}%{_libexecdir}/ipsec/*check
testing shipping some check binaries - might be useful or not. I am not
convinced this was a conscious decision and not just a missed update to
the real spec file.
-install -d -m 0700 %{buildroot}%{_rundir}/pluto
+install -d -m 0755 %{buildroot}%{_rundir}/pluto
This does not matter since the %files section has %attr() anyway.....
# used when setting --perpeerlog without --perpeerlogbase
install -d -m 0700 %{buildroot}%{_localstatedir}/log/pluto/peer
install -d %{buildroot}%{_sbindir}
@@ -151,8 +159,8 @@
# There is an elaborate upstream testing infrastructure which we do not
# run here - it takes hours and uses kvm
# We only run the CAVS tests.
-# cp %{SOURCE1} %{SOURCE2} %{SOURCE3} .
-# bunzip2 *.fax.bz2
+cp %{SOURCE1} %{SOURCE2} %{SOURCE3} .
+bunzip2 *.fax.bz2
Seems testing avoids CAVS testing - undocumented why that is.
# work around for older xen based machines
export NSS_DISABLE_HW_GCM=1
@@ -161,7 +169,7 @@
%{buildroot}%{_libexecdir}/ipsec/cavp -v2 ikev2.fax | \
diff -u ikev2.fax - > /dev/null
: starting CAVS test for IKEv1 RSASIG
-%{buildroot}%{_libexecdir}/ipsec/cavp -v1sig ikev1_dsa.fax | \
+%{buildroot}%{_libexecdir}/ipsec/cavp -v1dsa ikev1_dsa.fax | \
Looks like a bug - possibly an update not pulled in ?
diff -u ikev1_dsa.fax - > /dev/null
: starting CAVS test for IKEv1 PSK
%{buildroot}%{_libexecdir}/ipsec/cavp -v1psk ikev1_psk.fax | \
@@ -169,7 +177,6 @@
: CAVS tests passed
%endif
-%if "%{initsystem}" == "systemd"
%post
%systemd_post ipsec.service
@@ -178,7 +185,6 @@
%postun
%systemd_postun_with_restart ipsec.service
-%endif
weirdness done due to setting the INITSYSTEM differently from the real
spec file ?
%files
%doc CHANGES COPYING CREDITS README* LICENSE
@@ -191,22 +197,15 @@
%attr(0644,root,root) %config(noreplace) %{_sysconfdir}/sysctl.d/50-libreswan.conf
%attr(0700,root,root) %dir %{_localstatedir}/log/pluto
%attr(0700,root,root) %dir %{_localstatedir}/log/pluto/peer
-%attr(0700,root,root) %dir %{_rundir}/pluto
-%if "%{initsystem}" == "systemd"
+%attr(0755,root,root) %dir %{_rundir}/pluto
Same as what I mentiobed before
%attr(0644,root,root) %{_tmpfilesdir}/libreswan.conf
%attr(0644,root,root) %{_unitdir}/ipsec.service
-%endif
-%if "%{initsystem}" == "docker" || "%{initsystem}" == "sysvinit"
-%attr(0755,root,root) %{_initddir}/ipsec
-%config(noreplace) %{_sysconfdir}/sysconfig/pluto
-%endif
So this seems a specific change. Apparently the rpm has to somehow work
with docker too. I still have an open unanswered question about why the
change I did for "ipsec" with detecting namespace testing is not working
for Antony, but I never received a reply after asking twice.
%attr(0644,root,root) %config(noreplace) %{_sysconfdir}/pam.d/pluto
%{_sbindir}/ipsec
%{_libexecdir}/ipsec
-%{_mandir}/*/*
+%doc %{_mandir}/*/*
Another missed update in the testing version.
%{_libdir}/fipscheck/pluto.hmac
%changelog
-* Wed Aug 9 2017 Team Libreswan <team at libreswan.org> - @IPSECBASEVERSION@
-- Automated build for testing from git tree.
-- All compile time options are set in Makefile.inc.local not here.
+* Sun Oct 7 2018 Team Libreswan <team at libreswan.org> - IPSECBASEVERSION-1
+- Automated build from release tar ball
If the only reason for this file is to do some docker based testing,
it would be good to just see how to support this in the regular fedora
spec file. That way, we can go back to one proper spec file that is
maintained and won't age and use bad/old options.
As things stand now, I still feel this file does not belong in our
repository. I left it alone as an unimportant item and not worth putting
more energy in, but I guess some action is now warranted to stop the
commit war. I would appreciate an explanation of the docker use case
so we can support Antony's docket setup as well supporting "make rpm"
and using stock rpms with namespace based testing for fedora/rhel CI/TDD
using 1 regular spec file.
Paul
Paul
Paul
More information about the Swan-dev
mailing list