Spec URL: https://www.alteeve.com/files/packages/kronosnet.spec SRPM URL: https://www.alteeve.com/files/packages/kronosnet-0.8-1.fc26.src.rpm Description: VPNs on steroids Fedora Account System Username: digimer
Koji builds; f26 - https://koji.fedoraproject.org/koji/taskinfo?taskID=22733286 f27 - https://koji.fedoraproject.org/koji/taskinfo?taskID=22733363 epel7 - https://koji.fedoraproject.org/koji/taskinfo?taskID=22733372
Thanks for starting this Madison :) Some quick drive-by comments: So you might want to run 'fedora-review -n kronosnet' in the folder where you have your .spec + .src.rpm kronosnet files. That will spit out a bunch of nits that we need to fix. For example: WARNING: Cannot download url: https://github.com/fabbione/kronosnet/archive/kronosnet-0.8.tar.gz WARNING: Package kronosnet-debuginfo-0.8-1.fc28 not built Once we fix the URLs: --- a/kronosnet.spec +++ b/kronosnet.spec @@ -72,8 +72,8 @@ Version: 0.8 Release: 1%{?numcomm:.%{numcomm}}%{?alphatag:.%{alphatag}}%{?dirty:.%{dirty}}%{?dist} License: GPLv2+ and LGPLv2+ Group: System Environment/Base -URL: https://github.com/fabbione/kronosnet/ -Source0: https://github.com/fabbione/kronosnet/archive/%{name}-%{version}%{?numcomm:.%{numcomm}}%{?alphatag:-%{alphatag}}%{?dirty:-%{dirty}}.tar.gz +URL: https://github.com/kronosnet/kronosnet/ +Source0: https://github.com/kronosnet/kronosnet/archive/v%{version}%{?numcomm:.%{numcomm}}%{?alphatag:-%{alphatag}}%{?dirty:-%{dirty}}.tar.gz We will need to call autogen.sh explicitely because ./configure is not shipped in the tarball. Also, I am not sure those if defined macros are working as expected, so we'll need to look into that as well.
Please use http://www.kronosnet.org/releases/ to download release tarballs.
f26: https://koji.fedoraproject.org/koji/taskinfo?taskID=22835875 f27: https://koji.fedoraproject.org/koji/taskinfo?taskID=22835903 epel7: https://koji.fedoraproject.org/koji/taskinfo?taskID=22835924 New .spec and srpm: https://www.alteeve.com/an-repo/files/packages/kronosnet.spec.0.8-3 https://www.alteeve.com/an-repo/files/packages/kronosnet-0.8-3.fc26.src.rpm I tried to use 'fedora-review', but it prompted me for a password and it wouldn't take the passwords I normally use and my google-fu was too weak to sort it out. I'll try to catch you on IRC, but in the meantime, I addressed (hopefully) your comments.
Hi, just took a quick glance. >Group: System Environment/Base >Group: System Environment/Libraries >Group: Development/Libraries The "Group" tag should not be used. >BuildRoot: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX) The same for "BuildRoot:". >%install >rm -rf %{buildroot} You shouldn't manually remove the buildroot during %install. https://fedoraproject.org/wiki/Packaging:Guidelines#Tags_and_Sections >%doc COPYING.* COPYRIGHT These should probably be marked as %license. As for fedora-review: you can try "fedora-review -rn PATH_TO_LOCAL_SRPM".
Updated to address your comments (and thanks for the package link). f26: https://koji.fedoraproject.org/koji/taskinfo?taskID=23123556 f27: https://koji.fedoraproject.org/koji/taskinfo?taskID=23123615 epel7: https://koji.fedoraproject.org/koji/taskinfo?taskID=23123672 New .spec and srpm: https://www.alteeve.com/an-repo/files/packages/kronosnet.spec.0.8-4 https://www.alteeve.com/an-repo/files/packages/kronosnet-0.8-4.fc26.src.rpm Note that I have not updated the source in a bit. I've wanted to focus on getting the spec right. Once OK, I'll build again with the latest from git.
Latest RPMs, using this morning's master from git; f26: https://koji.fedoraproject.org/koji/taskinfo?taskID=23656614 f27: https://koji.fedoraproject.org/koji/taskinfo?taskID=23656681 rawhide: https://koji.fedoraproject.org/koji/taskinfo?taskID=23656760 epel7: https://koji.fedoraproject.org/koji/taskinfo?taskID=23656730 New .spec and srpm: https://www.alteeve.com/an-repo/files/packages/kronosnet.spec.0.8-5 https://www.alteeve.com/an-repo/files/packages/kronosnet-0.8-5.fc26.src.rpm
Spec URL: http://acksyn.org/files/rpms/kronosnet/kronosnet.spec SRPM URL: http://acksyn.org/files/rpms/kronosnet/kronosnet-1.0-1.fc27.src.rpm
This is a rebuild using bandini's .spec and the SRPM source from master. f26: https://koji.fedoraproject.org/koji/taskinfo?taskID=24381668 f27: https://koji.fedoraproject.org/koji/taskinfo?taskID=24381821 rawhide: https://koji.fedoraproject.org/koji/taskinfo?taskID=24381926 epel7: https://koji.fedoraproject.org/koji/taskinfo?taskID=24382047 New .spec and srpm: https://www.alteeve.com/an-repo/files/packages/kronosnet.spec.1.0-2 https://www.alteeve.com/an-repo/files/packages/kronosnet-1.0-2.fc26.src.rpm
I'll need a sponsor to take the next step in packaging this RPM for fedora. Would anyone on this BZ be interested/able to do that for me? If not, I'll post in the main list. Cheers
I'll happily assign myself as a reviewer if there're no complaints. For a start, the spec is needlessly overcomplicated around conditionals: - instead of > %bcond_without sctp > [...] > %if %{with sctp} > %global buildsctp 1 > %endif > [...] > %if %{defined buildsctp} > BuildRequires: lksctp-tools-devel > %endif > > [...] > > %build > [...] > %{configure} \ > %if %{defined buildsctp} > --enable-libknet-sctp \ > %else > --disable-libknet-sctp \ > %endif please use: > %bcond_without sctp > [...] > %if %{with sctp} > BuildRequires: lksctp-tools-devel > %endif > > [...] > > %build > [...] > %{configure} \ > %{?with_sctp:--enable-libknet-sctp} \ > %{!?with_sctp:--disable-libknet-sctp} \ where the latter part can even be compacted as: > %build > [...] > %{configure} \ > --%{?with_sctp:en}%{!?with_sctp:dis}able-libknet-sctp} \ See also: http://rpm.org/user_doc/conditional_builds.html * * * Another spot from this first sight, there are various pieces I believe are already obsolete in Fedora, e.g.: > Requires(post): systemd-sysv > Requires(post): systemd-units > Requires(preun): systemd-units > Requires(postun): systemd-units See e.g.: https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/message/LLG4T53FW2BGVZLGLKNYTKPD5SQNBZ2Y/
Thanks!! I'll work on a new RPM shortly.
(In reply to Jan Pokorný from comment #11) > I'll happily assign myself as a reviewer if there're no complaints. > > For a start, the spec is needlessly overcomplicated around conditionals: can you please propose a patch upstream too? I am very rusty on spec files and stuff. Digimer probably used mine as template and there is some goodness in having them aligned. Just please make sure it builds also on opensuse.
re [comment 13]: Mind that this is in the context of Fedora, OpenSUSE can have conflicting guidelines and that's generally out of scope here.
(In reply to Jan Pokorný from comment #14) > re [comment 13]: > > Mind that this is in the context of Fedora, OpenSUSE can have > conflicting guidelines and that's generally out of scope here. A lot of the basic rpm functionalities are the same. While the fedora spec file needs/must match fedora guideline, it doesn´t prevent using some of those recommendations upstream and have them converged to be more sane on both side :-)
> %build > [...] > %{configure} \ > --%{?with_sctp:en}%{!?with_sctp:dis}able-libknet-sctp} \ I think that might be the ugliest thing I've seen this year
re [comment 16]: Hehe, with 10+ configure options, it actually becomes quite reasonable.
I am super new to this, so I am somewhat reluctant to stick my neck out, but then that's never stopped me before... :) Jan's way is more compact, but it took me longer to parse what the string was doing. Once I got the logic, it is cleaner and smaller. On the other hand, the original way is more verbose, but it is a bit easier to grasp (particularly for those like me who are still newish to RPM/spec). I've always *personally* been a fan of longer-if-easier-to-grasp. (This is a common debate in the perl community, where I've landed on the "no one liners!" side). For spec though, I'm on the fence because I just don't know enough.
> re [comment 16]: > > Hehe, with 10+ configure options, it actually becomes quite reasonable. No, it becomes MUCH worse. Lines are not a sparse commodity - we can use as many as we like to help make things readable. Please don't go about saving lines of code and sacrificing clarity like that. I'd rather scroll down a few lines than unscramble that mess.
Digimer, feel free to stick with middle-grounds, it's the best trade-off. Though I am not getting the backlash against the compact version; more lines means more redundancy (write more + read more = more error-prone, also some brains are happier with more coarse-grained copy-paste patterns instigating reasoning in a bit bigger picture), and clarity is not lost at all once you absorb the logic upon the first use as mentioned by Digimer.
Regarding licenses, we have the situation with multiple licenses involved. What do the guidelines[1] have to say: > The License: field refers to the licenses of the contents of the > binary rpm. [...] > If a source package generates multiple binary packages, the License: > field may differ between them if necessary. This implies that a > single spec may have multiple per-subpackage License: tags. README.license file makes the distinction clear: applications (executables) vs. libraries. Brief look suggest this pairing of subpackages: * libraries: - lib*.rpm * application/executable: - kronosnetd*.rpm Respective subpackages should receive the license tag individually per the above dichotomy ("LGPLv2+" and "GPLv2+", respectively). [1] https://fedoraproject.org/wiki/Packaging:LicensingGuidelines
We don't want kronosnetd packaging.
re [comment 22]: Sure, the point is, either drop kronosnetd completely (from the conditionals), or set the License tag properly also there.
I've rolled a new RPM with the comments from here worked in. A couple key changes; I added a License to all packages based on the license I found at the top of the relevant C files. Please someone confirm if this is wrong. I left the original; > %{configure} \ > %{?with_sctp:--enable-libknet-sctp} \ > %{!?with_sctp:--disable-libknet-sctp} \ I commented out, but didn't fully delete, the systemd-{units,sysv}. I'd like comment from Fabio or Chrissie before fully deleting. I added a reference to the thread though. New .spec and srpm: https://www.alteeve.com/an-repo/files/packages/kronosnet.spec.1.0-3 https://www.alteeve.com/an-repo/files/packages/kronosnet-1.0-3.fc26.src.rpm f26: https://koji.fedoraproject.org/koji/taskinfo?taskID=24578635 f27: https://koji.fedoraproject.org/koji/taskinfo?taskID=24578768 rawhide: https://koji.fedoraproject.org/koji/taskinfo?taskID=24578816 *** This is failing. x86_64 build.log -> https://kojipkgs.fedoraproject.org//work/tasks/8817/24578817/build.log epel7: https://koji.fedoraproject.org/koji/taskinfo?taskID=24578906
@Jan, when you are happy, can you do this so that I can take the next step in packaging? I got a sponsor so I am in the package group now. :D Thanks! ==== 20:37 < tibbs> The bug should be assigned to the person who did the review, and they should set the fedora-review flag to '+' when they're done with the review. 20:37 < tibbs> That ticket isn't assigned to anyone. ====
re: rawhide build failure; Might be a koji issue. Will try again when it appears to be fixed.
Upstream is fixing the rawhide issue (gcc8 v gcc7)
Looking at https://www.alteeve.com/an-repo/files/packages/kronosnet.spec.1.0-3 1. I don't see any change about the clumsy conditionals (is it what was meant with "I left the original"?) 2. you are right that source files appear dual-licensed, but as mentioned, the License tag describes license of shipped artifacts (built executables, libraries, etc.) not of the source files, and that seems refined with README.license file making it clear under which terms are which artefacts expected to be distributed (binary RPMs are a form of distribution); I think particular License tags should reflect that -- perhaps best checked with upstream 3. it's customary to specify BuildRequires dependencies that are sourced by using pkg-config utility (*.pc files, here through PKG_CHECK_MODULES() macro in configure.ac file) as pkgconfig(foo) -- guidelines state it as SHOULD item: https://fedoraproject.org/wiki/Packaging:PkgConfigBuildRequires - for a start: libqb-devel -> pkgconfig(libqb) xz-devel -> pkgconfig(liblzma) zlib-devel -> pkgconfig(zlib) - also, this is likely the first time I've seen dependency on *-devel packages expressed via direct header file dependency, though configure script also asks for pkg-config module explicitly at least in some instances, hence I suggest: /usr/include/bzlib.h -> pkgconfig(bzip2) /usr/include/lz4hc.h -> pkgconfig(liblz4) /usr/include/nss3/nss.h -> pkgconfig(nss) /usr/include/openssl/conf.h -> pkgconfig(openssl) 4. what's the purpose of fiddling with debug packages that has been added since last time? it's likely inappropriate here
1. Yes, I left it as it was based on Chrissie's (strong) comments, and that it is easier to read on first pass. 2. Fabio confirmed that the licenses I entered are OK with him. Does this address the license concerns, or should a README.license be created for the RPM? 3. This is a little beyond me at this point (though I will read the link shortly). Should Fabio/Chrissie/Others comment on this, as it sounds like an upstream comment. 4. Certainly a comment for upstream.
1. Ah, I see, there's a little misunderstanding here, we indeed polemized about "--%{?with_sctp:en}%{!?with_sctp:dis}able-libknet-sctp}", but the disagreement did not cover > %{?with_sctp:--enable-libknet-sctp} \ > %{!?with_sctp:--disable-libknet-sctp} \ variant from [comment 11] (with the surrounding changes), which is hardly disputable and still better than the overcombined original 2. I am talking about README.license included in the tarball that's included in the SRPM (quick tip: you can use Midnight Commander to enter RPM files, and subsequently CONTENTS.cpio and any nested tarball that's present there), i.e., file of kronosnet proper: https://github.com/kronosnet/kronosnet/blob/master/README.licence my take is that it provides a definitive answer what (and only what) should License tag for libraries vs. application/executable packages contain -- see also [comment 21]; you may want to check this very conclusion with upstream, though 3. certainly a matter of advanced compiled code packaging fu (but no need to stress about this as we are here to help), though the SHOULD recommendation has its merit -- beside being nicer, it also offers flexibility in terms of what particular package will deliver the functionality requested like that, making the dependency expressed the most descriptive way at our disposal in Fedora 4. I mean, it may make reasons for test builds, but it will be catching eyes of anyone working on downstream packages needlessly
I've rolled a new RPM to address the rawhide / gcc8 issue. I also updated the project description. New .spec and srpm: https://www.alteeve.com/an-repo/files/packages/kronosnet.spec.1.0-4 https://www.alteeve.com/an-repo/files/packages/kronosnet-1.0-4.fc26.src.rpm f26: https://koji.fedoraproject.org/koji/taskinfo?taskID=24608847 f27: https://koji.fedoraproject.org/koji/taskinfo?taskID=24608937 rawhide (now builds correctly): https://koji.fedoraproject.org/koji/taskinfo?taskID=24608749 epel7: https://koji.fedoraproject.org/koji/taskinfo?taskID=24608951
Any comments from upstream on comment #30 or the latest RPMs?
@digimer: I've quickly checked the https://www.alteeve.com/an-repo/files/packages/kronosnet-1.0-4.fc26.src.rpm and found following issues: - warning: bogus date in %changelog: Tue Jan 31 2018 Madison Kelly <mkelly> - 1.0-4 <-- Should be Wed - capitalize the summary (so for example convert "libknet1 crypto plugins meta package" ->> "Libknet crypto....", libknet1 openssl support -> "Libknet openssl support", ...) - Please try to follow https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires . We need explicit requires for plugins-all, but they should be properly versioned and some small comment why we need it would be nice to have. - Please remove old cruft: %clean rm -rf %{buildroot} %defattr(-,root,root,-) in %files - I know kronosnetd is not going to be distributed right now, but fixing groupadd as https://fedoraproject.org/wiki/Packaging:UsersAndGroups#Dynamic_allocation may be good idea.
@digimer: Few more comments to kronosned section. It's now safe to expect systemd_post to exists so it's better to follow https://fedoraproject.org/wiki/Packaging:Systemd#Packaging, so for knet it means: - Add buildrequire - Remove 0%{?systemd_post:1}, 0%{?systemd_preun:1}, else sections.
I think I have integrated the comments, save for the last one in comment #34 for "buildrequire". I'm not sure what you are referring to, specifically. If I can get clarification on that, or missed anything, please let me know and I'll roll a new RPM asap. New .spec and srpm: https://www.alteeve.com/an-repo/files/packages/kronosnet.spec.1.0-5 https://www.alteeve.com/an-repo/files/packages/kronosnet-1.0-5.fc27.src.rpm f26: https://koji.fedoraproject.org/koji/taskinfo?taskID=24971062 f27: https://koji.fedoraproject.org/koji/taskinfo?taskID=24971069 rawhide: https://koji.fedoraproject.org/koji/taskinfo?taskID=24971077 epel7: https://koji.fedoraproject.org/koji/taskinfo?taskID=24971089 Here is the diff in .spec from 1.0-4; ==== --- kronosnet.spec.1.0-4 2018-01-31 23:32:53.661198886 -0500 +++ kronosnet.spec.1.0-5 2018-02-12 14:20:52.122179273 -0500 @@ -70,10 +70,11 @@ Name: kronosnet Summary: Multipoint-to-Multipoint VPN daemon Version: 1.0 -Release: 3%{?dist} +Release: 5%{?dist} License: GPLv2+ and LGPLv2+ URL: http://www.kronosnet.org Source0: http://www.kronosnet.org/releases/kronosnet-%{version}.tar.gz +Patch0: gcc8-fixes.patch # Build dependencies BuildRequires: gcc @@ -83,16 +84,16 @@ BuildRequires: lksctp-tools-devel %endif %if %{defined buildcryptonss} -BuildRequires: /usr/include/nss3/nss.h /usr/include/nspr4/nspr.h +BuildRequires: nss-devel nspr-devel %endif %if %{defined buildcryptoopenssl} -BuildRequires: /usr/include/openssl/conf.h +BuildRequires: openssl-devel %endif %if %{defined buildcompresszlib} BuildRequires: zlib-devel %endif %if %{defined buildcompresslz4} -BuildRequires: /usr/include/lz4hc.h +BuildRequires: lz4-devel %endif %if %{defined buildcompresslzo2} BuildRequires: lzo-devel @@ -101,7 +102,7 @@ BuildRequires: xz-devel %endif %if %{defined buildcompressbzip2} -BuildRequires: /usr/include/bzlib.h +BuildRequires: bzip2-devel %endif %if %{defined buildkronosnetd} BuildRequires: pam-devel @@ -198,9 +199,6 @@ # remove docs rm -rf %{buildroot}/usr/share/doc/kronosnet -%clean -rm -rf %{buildroot} - # main empty package %description kronosnet source @@ -210,16 +208,6 @@ %package -n kronosnetd Summary: Multipoint-to-Multipoint VPN daemon License: GPLv2+ and LGPLv2+ -%if %{defined _unitdir} -## Needed for systemd unit - Removed, see: https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/message/LLG4T53FW2BGVZLGLKNYTKPD5SQNBZ2Y/ -#Requires(post): systemd-sysv -#Requires(post): systemd-units -#Requires(preun): systemd-units -#Requires(postun): systemd-units -%else -Requires(post): chkconfig -Requires(preun): chkconfig, initscripts -%endif Requires(post): shadow-utils Requires(preun): shadow-utils Requires: pam, /etc/pam.d/passwd @@ -236,16 +224,8 @@ or service disruption. %post -n kronosnetd -%if %{defined _unitdir} - %if 0%{?systemd_post:1} - %systemd_post kronosnetd.service - %else - /bin/systemctl daemon-reload >/dev/null 2>&1 || : - %endif -%else -/sbin/chkconfig --add kronosnetd -%endif -/usr/sbin/groupadd --force --system kronosnetadm +%systemd_post kronosnetd.service +getent group kronosnetadm >/dev/null || groupadd --force kronosnetadm %preun -n kronosnetd %if %{defined _unitdir} @@ -265,7 +245,6 @@ %endif %files -n kronosnetd -%defattr(-,root,root,-) %license COPYING.* COPYRIGHT %dir %{_sysconfdir}/kronosnet %dir %{_sysconfdir}/kronosnet/* @@ -363,7 +342,7 @@ %if %{defined buildcryptonss} %package -n libknet1-crypto-nss-plugin -Summary: libknet1 nss support +Summary: Libknet1 nss support License: GPLv2+ and LGPLv2+ Requires: libknet1 = %{version}-%{release} @@ -377,7 +356,7 @@ %if %{defined buildcryptoopenssl} %package -n libknet1-crypto-openssl-plugin -Summary: libknet1 openssl support +Summary: Libknet1 openssl support License: GPLv2+ and LGPLv2+ Requires: libknet1 = %{version}-%{release} @@ -391,7 +370,7 @@ %if %{defined buildcompresszlib} %package -n libknet1-compress-zlib-plugin -Summary: libknet1 zlib support +Summary: Libknet1 zlib support License: GPLv2+ and LGPLv2+ Requires: libknet1 = %{version}-%{release} @@ -404,7 +383,7 @@ %endif %if %{defined buildcompresslz4} %package -n libknet1-compress-lz4-plugin -Summary: libknet1 lz4 and lz4hc support +Summary: Libknet1 lz4 and lz4hc support License: GPLv2+ and LGPLv2+ Requires: libknet1 = %{version}-%{release} @@ -419,7 +398,7 @@ %if %{defined buildcompresslzo2} %package -n libknet1-compress-lzo2-plugin -Summary: libknet1 lzo2 support +Summary: Libknet1 lzo2 support License: GPLv2+ and LGPLv2+ Requires: libknet1 = %{version}-%{release} @@ -433,7 +412,7 @@ %if %{defined buildcompresslzma} %package -n libknet1-compress-lzma-plugin -Summary: libknet1 lzma support +Summary: Libknet1 lzma support License: GPLv2+ and LGPLv2+ Requires: libknet1 = %{version}-%{release} @@ -447,7 +426,7 @@ %if %{defined buildcompressbzip2} %package -n libknet1-compress-bzip2-plugin -Summary: libknet1 bzip2 support +Summary: Libknet1 bzip2 support License: GPLv2+ and LGPLv2+ Requires: libknet1 = %{version}-%{release} @@ -460,13 +439,13 @@ %endif %package -n libknet1-crypto-plugins-all -Summary: libknet1 crypto plugins meta package +Summary: Libknet1 crypto plugins meta package License: GPLv2+ and LGPLv2+ %if %{defined buildcryptonss} -Requires: libknet1-crypto-nss-plugin +Requires: libknet1-crypto-nss-plugin%{_isa} = %{version}-%{release} %endif %if %{defined buildcryptoopenssl} -Requires: libknet1-crypto-openssl-plugin +Requires: libknet1-crypto-openssl-plugin%{_isa} = %{version}-%{release} %endif %description -n libknet1-crypto-plugins-all @@ -475,22 +454,22 @@ %files -n libknet1-crypto-plugins-all %package -n libknet1-compress-plugins-all -Summary: libknet1 compress plugins meta package +Summary: Libknet1 compress plugins meta package License: GPLv2+ and LGPLv2+ %if %{defined buildcompresszlib} -Requires: libknet1-compress-zlib-plugin +Requires: libknet1-compress-zlib-plugin%{_isa} = %{version}-%{release} %endif %if %{defined buildcompresslz4} -Requires: libknet1-compress-lz4-plugin +Requires: libknet1-compress-lz4-plugin%{_isa} = %{version}-%{release} %endif %if %{defined buildcompresslzo2} -Requires: libknet1-compress-lzo2-plugin +Requires: libknet1-compress-lzo2-plugin%{_isa} = %{version}-%{release} %endif %if %{defined buildcompresslzma} -Requires: libknet1-compress-lzma-plugin +Requires: libknet1-compress-lzma-plugin%{_isa} = %{version}-%{release} %endif %if %{defined buildcompressbzip2} -Requires: libknet1-compress-bzip2-plugin +Requires: libknet1-compress-bzip2-plugin%{_isa} = %{version}-%{release} %endif %description -n libknet1-compress-plugins-all @@ -499,10 +478,10 @@ %files -n libknet1-compress-plugins-all %package -n libknet1-plugins-all -Summary: libknet1 plugins meta package +Summary: Libknet1 plugins meta package License: GPLv2+ and LGPLv2+ -Requires: libknet1-compress-plugins-all -Requires: libknet1-crypto-plugins-all +Requires: libknet1-compress-plugins-all%{_isa} = %{version}-%{release} +Requires: libknet1-crypto-plugins-all%{_isa} = %{version}-%{release} %description -n libknet1-plugins-all meta package to install all of libknet1 plugins @@ -514,7 +493,15 @@ %endif %changelog -* Tue Jan 31 2018 Madison Kelly <mkelly> - 1.0-4 +* Mon Feb 12 2018 Madison Kelly <mkelly> - 1.0-5 +- All changes related to this spec; +- Fixed typo in previous changelog date (Tue -> Wed). +- Capitalized summaries as needed. +- Changed Requires that were file paths to package names. +- Added arch and version/release requirements for knet packages. +- Changed groupadd to be a little smarter. + +* Wed Jan 31 2018 Madison Kelly <mkelly> - 1.0-4 - Added patch for gcc8 fix. - Cleaned up description. ====
@digimer: From my point of view, package is almost ready to pass all the required test, but there are still few left: - Please remove %defattr(-,root,root,-) everywhere - For kronosnetd: * Requires(preun): shadow-utils <-- not needed * groupadd --force kronosnetadm <-- kronosnetadm should be system account (and force is not needed), so result should be "groupadd -r kronosnetadm" * %systemd_post kronosnetd.service <-- I believe there was small misunderstanding. So the question is, if you want to keep compatibility with non-systemd systems (whole spec is written that way), then stanza should look like: ``` %if %{defined _unitdir} %systemd_post kronosnetd.service %else /sbin/chkconfig --add kronosnetd %endif ``` * Please fix the "%preun -n kronosnetd" section similar way * Add %{?systemd_requires} (conditionally) * Add postun: ``` %postun -n kronosnetd %if %{defined _unitdir} %systemd_postun kronosnetd.service %endif ``` - Missing "BuildRequires: systemd" (probably conditional) so something like ``` %if %{defined _unitdir} && %{defined buildkronosnetd} BuildRequires: systemd %endif ```
@digimer: Forgot one more thing. We need more %{_isa}. Just add them basically for every Requires: libtap1 and Requires: libknet1
> - Please remove %defattr(-,root,root,-) everywhere Done. > - For kronosnetd: > * Requires(preun): shadow-utils <-- not needed What about the (postrun) entry? I've left that for now. > * groupadd --force kronosnetadm <-- kronosnetadm should be system account (and force is not needed), so result should be "groupadd -r kronosnetadm" OK, but I used the more verbose '--system' just to be a little more self-documenting. > So the question is, if you want to keep compatibility with non-systemd systems No, I see no reason for this. Knet will never run on EL6. I do test against EPEL7 and modern Fedoras, which are all systemd now. I'd actually prefer to remove all sysvinit stuff down the road. I'm only not doing it now because I still have too much to learn and I didn't want to slow down the initial release. Now that it is an issue though (if minor), I will try to remove it in this release. I apologize if this introduces new issues at this stage. > - Missing "BuildRequires: systemd" (probably conditional) so something like Given the above comment, I added it without conditional. > Forgot one more thing. We need more %{_isa}. Just add them basically for every... Done. New .spec and srpm: https://www.alteeve.com/an-repo/files/packages/kronosnet.spec.1.0-6 https://www.alteeve.com/an-repo/files/packages/kronosnet-1.0-6.fc27.src.rpm f26: https://koji.fedoraproject.org/koji/taskinfo?taskID=25061282 f27: https://koji.fedoraproject.org/koji/taskinfo?taskID=25061295 rawhide: https://koji.fedoraproject.org/koji/taskinfo?taskID=25061303 epel7: https://koji.fedoraproject.org/koji/taskinfo?taskID=25061269 Here is the diff in .spec from 1.0-5; ==== --- kronosnet.spec.1.0-5 2018-02-12 14:20:52.122179273 -0500 +++ kronosnet.spec.1.0-6 2018-02-15 00:25:25.797016016 -0500 @@ -70,7 +70,7 @@ Name: kronosnet Summary: Multipoint-to-Multipoint VPN daemon Version: 1.0 -Release: 5%{?dist} +Release: 6%{?dist} License: GPLv2+ and LGPLv2+ URL: http://www.kronosnet.org Source0: http://www.kronosnet.org/releases/kronosnet-%{version}.tar.gz @@ -78,6 +78,7 @@ # Build dependencies BuildRequires: gcc +BuildRequires: systemd # required to build man pages BuildRequires: libqb-devel libxml2-devel doxygen %if %{defined buildsctp} @@ -170,11 +171,7 @@ --enable-libtap \ %endif --with-initdefaultdir=%{_sysconfdir}/sysconfig/ \ -%if %{defined _unitdir} --with-systemddir=%{_unitdir} -%else - --with-initddir=%{_sysconfdir}/rc.d/init.d/ -%endif make %{_smp_mflags} @@ -188,13 +185,8 @@ find %{buildroot} -name "*.la" -exec rm {} \; # handle systemd vs init script -%if %{defined _unitdir} # remove init scripts rm -rf %{buildroot}/etc/init.d -%else -# remove systemd specific bits -find %{buildroot} -name "*.service" -exec rm {} \; -%endif # remove docs rm -rf %{buildroot}/usr/share/doc/kronosnet @@ -209,7 +201,6 @@ Summary: Multipoint-to-Multipoint VPN daemon License: GPLv2+ and LGPLv2+ Requires(post): shadow-utils -Requires(preun): shadow-utils Requires: pam, /etc/pam.d/passwd %description -n kronosnetd @@ -228,21 +219,14 @@ getent group kronosnetadm >/dev/null || groupadd --force kronosnetadm %preun -n kronosnetd -%if %{defined _unitdir} - %if 0%{?systemd_preun:1} +%if 0%{?systemd_preun:1} %systemd_preun kronosnetd.service - %else +%else if [ "$1" -eq 0 ]; then /bin/systemctl --no-reload disable kronosnetd.service /bin/systemctl stop kronosnetd.service >/dev/null 2>&1 fi %endif -%else -if [ "$1" = 0 ]; then - /sbin/service kronosnetd stop >/dev/null 2>&1 - /sbin/chkconfig --del kronosnetd -fi -%endif %files -n kronosnetd %license COPYING.* COPYRIGHT @@ -251,11 +235,7 @@ %config(noreplace) %{_sysconfdir}/sysconfig/kronosnetd %config(noreplace) %{_sysconfdir}/pam.d/kronosnetd %config(noreplace) %{_sysconfdir}/logrotate.d/kronosnetd -%if %{defined _unitdir} %{_unitdir}/kronosnetd.service -%else -%config(noreplace) %{_sysconfdir}/rc.d/init.d/kronosnetd -%endif %{_sbindir}/* %{_mandir}/man8/* %endif @@ -271,7 +251,6 @@ pre-up.d/up.d/down.d/post-down.d infrastructure. %files -n libtap1 -%defattr(-,root,root,-) %license COPYING.* COPYRIGHT %{_libdir}/libtap.so.* @@ -282,7 +261,7 @@ %package -n libtap1-devel Summary: Simple userland wrapper around kernel tap devices (developer files) License: GPLv2+ and LGPLv2+ -Requires: libtap1 = %{version}-%{release} +Requires: libtap1%{_isa} = %{version}-%{release} Requires: pkgconfig %description -n libtap1-devel @@ -291,7 +270,6 @@ pre-up.d/up.d/down.d/post-down.d infrastructure. %files -n libtap1-devel -%defattr(-,root,root,-) %license COPYING.* COPYRIGHT %{_libdir}/libtap.so %{_includedir}/libtap.h @@ -312,7 +290,6 @@ Please refer to https://kronosnet.org/ for further information. %files -n libknet1 -%defattr(-,root,root,-) %license COPYING.* COPYRIGHT %{_libdir}/libknet.so.* %dir %{_libdir}/kronosnet @@ -324,7 +301,7 @@ %package -n libknet1-devel Summary: Kronosnet core switching implementation (developer files) License: GPLv2+ and LGPLv2+ -Requires: libknet1 = %{version}-%{release} +Requires: libknet1%{_isa} = %{version}-%{release} Requires: pkgconfig %description -n libknet1-devel @@ -333,7 +310,6 @@ information. %files -n libknet1-devel -%defattr(-,root,root,-) %license COPYING.* COPYRIGHT %{_libdir}/libknet.so %{_includedir}/libknet.h @@ -344,13 +320,12 @@ %package -n libknet1-crypto-nss-plugin Summary: Libknet1 nss support License: GPLv2+ and LGPLv2+ -Requires: libknet1 = %{version}-%{release} +Requires: libknet1%{_isa} = %{version}-%{release} %description -n libknet1-crypto-nss-plugin NSS crypto support for libknet1. %files -n libknet1-crypto-nss-plugin -%defattr(-,root,root,-) %{_libdir}/kronosnet/crypto_nss.so %endif @@ -358,13 +333,12 @@ %package -n libknet1-crypto-openssl-plugin Summary: Libknet1 openssl support License: GPLv2+ and LGPLv2+ -Requires: libknet1 = %{version}-%{release} +Requires: libknet1%{_isa} = %{version}-%{release} %description -n libknet1-crypto-openssl-plugin OpenSSL crypto support for libknet1. %files -n libknet1-crypto-openssl-plugin -%defattr(-,root,root,-) %{_libdir}/kronosnet/crypto_openssl.so %endif @@ -372,26 +346,24 @@ %package -n libknet1-compress-zlib-plugin Summary: Libknet1 zlib support License: GPLv2+ and LGPLv2+ -Requires: libknet1 = %{version}-%{release} +Requires: libknet1%{_isa} = %{version}-%{release} %description -n libknet1-compress-zlib-plugin zlib compression support for libknet1. %files -n libknet1-compress-zlib-plugin -%defattr(-,root,root,-) %{_libdir}/kronosnet/compress_zlib.so %endif %if %{defined buildcompresslz4} %package -n libknet1-compress-lz4-plugin Summary: Libknet1 lz4 and lz4hc support License: GPLv2+ and LGPLv2+ -Requires: libknet1 = %{version}-%{release} +Requires: libknet1%{_isa} = %{version}-%{release} %description -n libknet1-compress-lz4-plugin lz4 and lz4hc compression support for libknet1. %files -n libknet1-compress-lz4-plugin -%defattr(-,root,root,-) %{_libdir}/kronosnet/compress_lz4.so %{_libdir}/kronosnet/compress_lz4hc.so %endif @@ -400,13 +372,12 @@ %package -n libknet1-compress-lzo2-plugin Summary: Libknet1 lzo2 support License: GPLv2+ and LGPLv2+ -Requires: libknet1 = %{version}-%{release} +Requires: libknet1%{_isa} = %{version}-%{release} %description -n libknet1-compress-lzo2-plugin lzo2 compression support for libknet1. %files -n libknet1-compress-lzo2-plugin -%defattr(-,root,root,-) %{_libdir}/kronosnet/compress_lzo2.so %endif @@ -414,13 +385,12 @@ %package -n libknet1-compress-lzma-plugin Summary: Libknet1 lzma support License: GPLv2+ and LGPLv2+ -Requires: libknet1 = %{version}-%{release} +Requires: libknet1%{_isa} = %{version}-%{release} %description -n libknet1-compress-lzma-plugin lzma compression support for libknet1. %files -n libknet1-compress-lzma-plugin -%defattr(-,root,root,-) %{_libdir}/kronosnet/compress_lzma.so %endif @@ -428,13 +398,12 @@ %package -n libknet1-compress-bzip2-plugin Summary: Libknet1 bzip2 support License: GPLv2+ and LGPLv2+ -Requires: libknet1 = %{version}-%{release} +Requires: libknet1%{_isa} = %{version}-%{release} %description -n libknet1-compress-bzip2-plugin bzip2 compression support for libknet1. %files -n libknet1-compress-bzip2-plugin -%defattr(-,root,root,-) %{_libdir}/kronosnet/compress_bzip2.so %endif ====
@digimer: - You've forgot to update changelog - For kronosnetd: * As was noted in previous comment, please add postun ``` %postun -n kronosnetd %systemd_postun kronosnetd.service ``` * Please fix the "%preun -n kronosnetd" section similar way as postun (= no need to check 0%{?systemd_preun:1}) * Add %{?systemd_requires} as described in https://fedoraproject.org/wiki/Packaging:Scriptlets#Systemd That's all from me. Some of the @poki comments are still not solved and may be useful: - License - It took me a while to parse what poki meant, and I can agree with him that License field may be more specific. So main idea is: * License of kronosnetd should be GPLv2+, because it is application * License of libknet* should be LGPLv2+, simply because they are libraries But let's quickly check with Fabio what he thinks about it. - Idea of using pkgconfig(foo) seems to be quite nice, could you please give it a try? (just to recap): libqb-devel -> pkgconfig(libqb) xz-devel -> pkgconfig(liblzma) zlib-devel -> pkgconfig(zlib) bzip2-devel -> pkgconfig(bzip2) lz4-devel -> pkgconfig(liblz4) nss-devel -> pkgconfig(nss) # We don't need to require on nspr-devel then openssl-devel -> pkgconfig(openssl)
> You've forgot to update changelog Doh! Added (for 1.0-6 and -7) > As was noted in previous comment, please add postun Added, but please verify I added in the correct location. > Please fix the "%preun -n kronosnetd" section... Being an if/else, with 'if [ "$1" -eq 0 ]; ...' being in the 'else', I removed that as well. Was that correct? > Add %{?systemd_requires} as described in ... Done. > License - ... Done. > Idea of using pkgconfig(foo) seems to be quite nice, could you please give it a try? Done. Question though; I wasn't able to find a clear explanation about what this change does (see: https://fedoraproject.org/wiki/Packaging:Guidelines#Pkgconfig_Files_.28foo.pc.29). Why would we not also wrap, say, libxml2-devel, lksctp-tools-devel, etc in a similar manner? NOTE: It would seem that one of these changes breaks epel7 build. I will dig into this and post a new .spec if I can sort it out. For now, this one is up for you to review as it currently stands. New .spec and srpm: https://www.alteeve.com/an-repo/files/packages/kronosnet.spec.1.0-7 https://www.alteeve.com/an-repo/files/packages/kronosnet-1.0-7.fc27.src.rpm f26: https://koji.fedoraproject.org/koji/taskinfo?taskID=25080933 f27: https://koji.fedoraproject.org/koji/taskinfo?taskID=25080940 rawhide: https://koji.fedoraproject.org/koji/taskinfo?taskID=25080948 epel7: *Failed* https://koji.fedoraproject.org/koji/taskinfo?taskID=25080956 Here is the diff in .spec from 1.0-5; ==== --- kronosnet.spec.1.0-6 2018-02-15 00:25:25.797016016 -0500 +++ kronosnet.spec.1.0-7 2018-02-15 19:18:05.854232031 -0500 @@ -70,40 +70,42 @@ Name: kronosnet Summary: Multipoint-to-Multipoint VPN daemon Version: 1.0 -Release: 6%{?dist} -License: GPLv2+ and LGPLv2+ +Release: 7%{?dist} +License: GPLv2+ URL: http://www.kronosnet.org Source0: http://www.kronosnet.org/releases/kronosnet-%{version}.tar.gz Patch0: gcc8-fixes.patch # Build dependencies BuildRequires: gcc +%{?systemd_requires} BuildRequires: systemd # required to build man pages -BuildRequires: libqb-devel libxml2-devel doxygen +BuildRequires: libxml2-devel doxygen +BuildRequires: pkgconfig(libqb) %if %{defined buildsctp} BuildRequires: lksctp-tools-devel %endif %if %{defined buildcryptonss} -BuildRequires: nss-devel nspr-devel +BuildRequires: pkgconfig(nss) %endif %if %{defined buildcryptoopenssl} -BuildRequires: openssl-devel +BuildRequires: pkgconfig(openssl) %endif %if %{defined buildcompresszlib} -BuildRequires: zlib-devel +BuildRequires: pkgconfig(zlib) %endif %if %{defined buildcompresslz4} -BuildRequires: lz4-devel +BuildRequires: pkgconfig(liblz4) %endif %if %{defined buildcompresslzo2} BuildRequires: lzo-devel %endif %if %{defined buildcompresslzma} -BuildRequires: xz-devel +BuildRequires: pkgconfig(liblzma) %endif %if %{defined buildcompressbzip2} -BuildRequires: bzip2-devel +BuildRequires: pkgconfig(bzip2) %endif %if %{defined buildkronosnetd} BuildRequires: pam-devel @@ -199,7 +201,7 @@ ## Runtime and subpackages section %package -n kronosnetd Summary: Multipoint-to-Multipoint VPN daemon -License: GPLv2+ and LGPLv2+ +License: GPLv2+ Requires(post): shadow-utils Requires: pam, /etc/pam.d/passwd @@ -218,15 +220,11 @@ %systemd_post kronosnetd.service getent group kronosnetadm >/dev/null || groupadd --force kronosnetadm +%postun -n kronosnetd +%systemd_postun kronosnetd.service + %preun -n kronosnetd -%if 0%{?systemd_preun:1} - %systemd_preun kronosnetd.service -%else -if [ "$1" -eq 0 ]; then - /bin/systemctl --no-reload disable kronosnetd.service - /bin/systemctl stop kronosnetd.service >/dev/null 2>&1 -fi -%endif +%systemd_preun kronosnetd.service %files -n kronosnetd %license COPYING.* COPYRIGHT @@ -243,7 +241,7 @@ %if %{defined buildlibtap} %package -n libtap1 Summary: Simple userland wrapper around kernel tap devices -License: GPLv2+ and LGPLv2+ +License: LGPLv2+ %description -n libtap1 This is an over-engineered commodity library to manage a pool @@ -260,7 +258,7 @@ %package -n libtap1-devel Summary: Simple userland wrapper around kernel tap devices (developer files) -License: GPLv2+ and LGPLv2+ +License: LGPLv2+ Requires: libtap1%{_isa} = %{version}-%{release} Requires: pkgconfig @@ -278,7 +276,7 @@ %package -n libknet1 Summary: Kronosnet core switching implementation -License: GPLv2+ and LGPLv2+ +License: LGPLv2+ %description -n libknet1 Kronosnet, often referred to as knet, is a network abstraction layer @@ -300,7 +298,7 @@ %package -n libknet1-devel Summary: Kronosnet core switching implementation (developer files) -License: GPLv2+ and LGPLv2+ +License: LGPLv2+ Requires: libknet1%{_isa} = %{version}-%{release} Requires: pkgconfig @@ -319,7 +317,7 @@ %if %{defined buildcryptonss} %package -n libknet1-crypto-nss-plugin Summary: Libknet1 nss support -License: GPLv2+ and LGPLv2+ +License: LGPLv2+ Requires: libknet1%{_isa} = %{version}-%{release} %description -n libknet1-crypto-nss-plugin @@ -332,7 +330,7 @@ %if %{defined buildcryptoopenssl} %package -n libknet1-crypto-openssl-plugin Summary: Libknet1 openssl support -License: GPLv2+ and LGPLv2+ +License: LGPLv2+ Requires: libknet1%{_isa} = %{version}-%{release} %description -n libknet1-crypto-openssl-plugin @@ -345,7 +343,7 @@ %if %{defined buildcompresszlib} %package -n libknet1-compress-zlib-plugin Summary: Libknet1 zlib support -License: GPLv2+ and LGPLv2+ +License: LGPLv2+ Requires: libknet1%{_isa} = %{version}-%{release} %description -n libknet1-compress-zlib-plugin @@ -357,7 +355,7 @@ %if %{defined buildcompresslz4} %package -n libknet1-compress-lz4-plugin Summary: Libknet1 lz4 and lz4hc support -License: GPLv2+ and LGPLv2+ +License: LGPLv2+ Requires: libknet1%{_isa} = %{version}-%{release} %description -n libknet1-compress-lz4-plugin @@ -371,7 +369,7 @@ %if %{defined buildcompresslzo2} %package -n libknet1-compress-lzo2-plugin Summary: Libknet1 lzo2 support -License: GPLv2+ and LGPLv2+ +License: LGPLv2+ Requires: libknet1%{_isa} = %{version}-%{release} %description -n libknet1-compress-lzo2-plugin @@ -384,7 +382,7 @@ %if %{defined buildcompresslzma} %package -n libknet1-compress-lzma-plugin Summary: Libknet1 lzma support -License: GPLv2+ and LGPLv2+ +License: LGPLv2+ Requires: libknet1%{_isa} = %{version}-%{release} %description -n libknet1-compress-lzma-plugin @@ -397,7 +395,7 @@ %if %{defined buildcompressbzip2} %package -n libknet1-compress-bzip2-plugin Summary: Libknet1 bzip2 support -License: GPLv2+ and LGPLv2+ +License: LGPLv2+ Requires: libknet1%{_isa} = %{version}-%{release} %description -n libknet1-compress-bzip2-plugin @@ -409,7 +407,7 @@ %package -n libknet1-crypto-plugins-all Summary: Libknet1 crypto plugins meta package -License: GPLv2+ and LGPLv2+ +License: LGPLv2+ %if %{defined buildcryptonss} Requires: libknet1-crypto-nss-plugin%{_isa} = %{version}-%{release} %endif @@ -424,7 +422,7 @@ %package -n libknet1-compress-plugins-all Summary: Libknet1 compress plugins meta package -License: GPLv2+ and LGPLv2+ +License: LGPLv2+ %if %{defined buildcompresszlib} Requires: libknet1-compress-zlib-plugin%{_isa} = %{version}-%{release} %endif @@ -448,7 +446,7 @@ %package -n libknet1-plugins-all Summary: Libknet1 plugins meta package -License: GPLv2+ and LGPLv2+ +License: LGPLv2+ Requires: libknet1-compress-plugins-all%{_isa} = %{version}-%{release} Requires: libknet1-crypto-plugins-all%{_isa} = %{version}-%{release} @@ -462,6 +460,19 @@ %endif %changelog +* Thu Feb 15 2018 Madison Kelly <mkelly> - 1.0-7 +- Added missing 1.0-6 changelog. +- Added kronosnetd postun. +- Clarified licensing. +- (re)added systemd_requires. +- Wrapped several buildrequires with pkgconfig(). + +* Wed Feb 14 2018 Madison Kelly <mkelly> - 1.0-6 +- Removed sysvinit checks. +- Fixed the groupadd to add to system group. +- Added build requirement for systemd. +- Added %{_isa} to knet requirements in sub packages. + * Mon Feb 12 2018 Madison Kelly <mkelly> - 1.0-5 - All changes related to this spec; - Fixed typo in previous changelog date (Tue -> Wed). ====
This is the change that breaks EPEL7; -BuildRequires: bzip2-devel +BuildRequires: pkgconfig(bzip2) If I roll that back, it builds fine on RHEL 7. With the 'pkgconfig(bzip2)', I get this: ==== [root@el7-builder-t1 ~]# yum install bzip2-devel Loaded plugins: kabi, product-id, search-disabled-repos, subscription-manager Loading support for Red Hat kernel ABI Package bzip2-devel-1.0.6-13.el7.x86_64 already installed and latest version Nothing to do [root@el7-builder-t1 ~]# su - digimer Last login: Thu Feb 15 19:30:30 EST 2018 on pts/0 [digimer@el7-builder-t1 ~]$ cd rpmbuild/SPECS/ [digimer@el7-builder-t1 SPECS]$ rpmbuild -ba kronosnet.spec error: Failed build dependencies: pkgconfig(bzip2) is needed by kronosnet-1.0-7.el7.x86_64 ==== Any insight? Or shall I just revert that one BuildRequires?
Speaking to Fabio, and given the way the comment around 'pkgconfig()' was framed, I rolled back the BuildRequires for bzip2. New .spec and srpm: https://www.alteeve.com/an-repo/files/packages/kronosnet.spec.1.0-8 https://www.alteeve.com/an-repo/files/packages/kronosnet-1.0-8.fc27.src.rpm f26: https://koji.fedoraproject.org/koji/taskinfo?taskID=25084124 f27: https://koji.fedoraproject.org/koji/taskinfo?taskID=25084134 rawhide: https://koji.fedoraproject.org/koji/taskinfo?taskID=25084142 epel7: https://koji.fedoraproject.org/koji/taskinfo?taskID=25084066 Diff from 1.0-7: ==== --- kronosnet.spec.1.0-7 2018-02-15 19:18:05.854232031 -0500 +++ kronosnet.spec.1.0-8 2018-02-16 00:04:45.452029189 -0500 @@ -70,7 +70,7 @@ Name: kronosnet Summary: Multipoint-to-Multipoint VPN daemon Version: 1.0 -Release: 7%{?dist} +Release: 8%{?dist} License: GPLv2+ URL: http://www.kronosnet.org Source0: http://www.kronosnet.org/releases/kronosnet-%{version}.tar.gz @@ -105,7 +105,7 @@ BuildRequires: pkgconfig(liblzma) %endif %if %{defined buildcompressbzip2} -BuildRequires: pkgconfig(bzip2) +BuildRequires: bzip2-devel %endif %if %{defined buildkronosnetd} BuildRequires: pam-devel @@ -460,6 +460,9 @@ %endif %changelog +* Fri Feb 16 2018 Madison Kelly <mkelly> - 1.0-8 +- Reverted to 'BuildRequires: bzip2-devel' to fix EPEL7 builds. + * Thu Feb 15 2018 Madison Kelly <mkelly> - 1.0-7 - Added missing 1.0-6 changelog. - Added kronosnetd postun. ====
@digimer: - Postun is on correct location, no worries - "%preun -n kronosnetd" is also corrent - But %{?systemd_requires} is not on correct location. It basically expands to: ``` Requires(post): systemd Requires(preun): systemd Requires(postun): systemd ``` so it should be in the kronosnetd part. (do not move BuildRequires: systemd, just %{?systemd_requires} ). - About license, I would let kronosnet License (only main package, other should be as they are) field as it was (so GPL + LGPL), because this is correct license for SRPM. - pkgconfig was (at least from my side) optional so nice job. Please fix the two things (License + location of %{?systemd_requires}) and knet should be good to go.
@digimer: Also please use official release from http://www.kronosnet.org/releases in srpm, not something else (as is the case now). Checksum then differs.
Sorry for the delay. Changes made (tarball from release, license and systed_requires). New .spec and srpm: https://www.alteeve.com/an-repo/files/packages/kronosnet.spec.1.0-9 https://www.alteeve.com/an-repo/files/packages/kronosnet-1.0-9.fc27.src.rpm f26: https://koji.fedoraproject.org/koji/taskinfo?taskID=25239823 f27: https://koji.fedoraproject.org/koji/taskinfo?taskID=25239830 rawhide: https://koji.fedoraproject.org/koji/taskinfo?taskID=25239815 epel7: https://koji.fedoraproject.org/koji/taskinfo?taskID=25239839 Diff from 1.0-8: ==== --- kronosnet.spec.1.0-8 2018-02-16 00:04:45.452029189 -0500 +++ kronosnet.spec.1.0-9 2018-02-22 14:34:54.588904329 -0500 @@ -70,15 +70,14 @@ Name: kronosnet Summary: Multipoint-to-Multipoint VPN daemon Version: 1.0 -Release: 8%{?dist} -License: GPLv2+ +Release: 9%{?dist} +License: GPLv2+ + LGPLv2+ URL: http://www.kronosnet.org Source0: http://www.kronosnet.org/releases/kronosnet-%{version}.tar.gz Patch0: gcc8-fixes.patch # Build dependencies BuildRequires: gcc -%{?systemd_requires} BuildRequires: systemd # required to build man pages BuildRequires: libxml2-devel doxygen @@ -204,6 +203,7 @@ License: GPLv2+ Requires(post): shadow-utils Requires: pam, /etc/pam.d/passwd +%{?systemd_requires} %description -n kronosnetd The kronosnet daemon is a bridge between kronosnet switching engine ====
@digimer: Perfect. Spec file now looks really great with one small nitpick - changelog is not updated. Please keep that in mind, I believe you also find useful when package has good changelog.
That's the second time I missed that... >_<. I'll roll a final RPM today/tonight with the change log updated. Madi
Updated to add the missing change log. New .spec and srpm: https://www.alteeve.com/an-repo/files/packages/kronosnet.spec.1.0-10 https://www.alteeve.com/an-repo/files/packages/kronosnet-1.0-10.fc27.src.rpm f26: https://koji.fedoraproject.org/koji/taskinfo?taskID=25264181 f27: https://koji.fedoraproject.org/koji/taskinfo?taskID=25264214 rawhide: https://koji.fedoraproject.org/koji/taskinfo?taskID=25264225 epel7: https://koji.fedoraproject.org/koji/taskinfo?taskID=25264234 Diff from 1.0-9: ==== --- kronosnet.spec.1.0-9 2018-02-22 14:34:54.588904329 -0500 +++ kronosnet.spec.1.0-10 2018-02-23 14:47:17.586627823 -0500 @@ -70,7 +70,7 @@ Name: kronosnet Summary: Multipoint-to-Multipoint VPN daemon Version: 1.0 -Release: 9%{?dist} +Release: 10%{?dist} License: GPLv2+ + LGPLv2+ URL: http://www.kronosnet.org Source0: http://www.kronosnet.org/releases/kronosnet-%{version}.tar.gz @@ -460,6 +460,14 @@ %endif %changelog +* Fri Feb 23 2018 Madison Kelly <mkelly> - 1.0-10 +- Added missing change log for 1.0-9. + +* Thu Feb 22 2018 Madison Kelly <mkelly> - 1.0-9 +- Changed the source tarball to be one from the upstream source. +- Updated the main license. +- Moved down %{?systemd_requires}. + * Fri Feb 16 2018 Madison Kelly <mkelly> - 1.0-8 - Reverted to 'BuildRequires: bzip2-devel' to fix EPEL7 builds. ====
Rerolled for the new 1.1 release. New .spec and srpm: https://www.alteeve.com/an-repo/files/packages/kronosnet.spec.1.1-1 https://www.alteeve.com/an-repo/files/packages/kronosnet-1.1-1.fc27.src.rpm f26: https://koji.fedoraproject.org/koji/taskinfo?taskID=25302495 f27: https://koji.fedoraproject.org/koji/taskinfo?taskID=25302502 rawhide: https://koji.fedoraproject.org/koji/taskinfo?taskID=25302512 epel7: https://koji.fedoraproject.org/koji/taskinfo?taskID=25302490 Diff from 1.0-10: ==== --- kronosnet.spec.1.0-10 2018-02-23 14:47:17.586627823 -0500 +++ kronosnet.spec.1.1-1 2018-02-25 13:17:20.497971906 -0500 @@ -69,12 +69,11 @@ Name: kronosnet Summary: Multipoint-to-Multipoint VPN daemon -Version: 1.0 -Release: 10%{?dist} +Version: 1.1 +Release: 1%{?dist} License: GPLv2+ + LGPLv2+ URL: http://www.kronosnet.org Source0: http://www.kronosnet.org/releases/kronosnet-%{version}.tar.gz -Patch0: gcc8-fixes.patch # Build dependencies BuildRequires: gcc @@ -117,7 +116,6 @@ %prep %setup -q -n %{name}-%{version} -%patch0 -p1 -z gcc8-fixes.patch %build %if %{with runautogen} @@ -277,6 +275,8 @@ %package -n libknet1 Summary: Kronosnet core switching implementation License: LGPLv2+ +BuildRequires: libqb-devel +BuildRequires: doxygen %description -n libknet1 Kronosnet, often referred to as knet, is a network abstraction layer @@ -460,6 +460,11 @@ %endif %changelog +* Sun Feb 25 2018 Madison Kelly <mkelly> - 1.1-1 +- Rerolled for 1.1 upstream release. +- Removed the (no longer needed) gcc8-fixes.patch +- Added the new doxygen and libqb-devel buildrequires for libknetd. + * Fri Feb 23 2018 Madison Kelly <mkelly> - 1.0-10 - Added missing change log for 1.0-9. ====
Slight change as per upstream comment to make 'systemd' build dep in the kronosnetd conditional. New .spec and srpm: https://www.alteeve.com/an-repo/files/packages/kronosnet.spec.1.1-2 https://www.alteeve.com/an-repo/files/packages/kronosnet-1.1-2.fc27.src.rpm f26: https://koji.fedoraproject.org/koji/taskinfo?taskID=25311003 f27: https://koji.fedoraproject.org/koji/taskinfo?taskID=25310992 rawhide: https://koji.fedoraproject.org/koji/taskinfo?taskID=25310980 epel7: https://koji.fedoraproject.org/koji/taskinfo?taskID=25311010 Diff from 1.1-1: ==== --- kronosnet.spec.1.1-1 2018-02-25 13:17:20.497971906 -0500 +++ kronosnet.spec.1.1-2 2018-02-25 23:17:33.268040008 -0500 @@ -70,14 +70,13 @@ Name: kronosnet Summary: Multipoint-to-Multipoint VPN daemon Version: 1.1 -Release: 1%{?dist} +Release: 2%{?dist} License: GPLv2+ + LGPLv2+ URL: http://www.kronosnet.org Source0: http://www.kronosnet.org/releases/kronosnet-%{version}.tar.gz # Build dependencies BuildRequires: gcc -BuildRequires: systemd # required to build man pages BuildRequires: libxml2-devel doxygen BuildRequires: pkgconfig(libqb) @@ -106,6 +105,7 @@ BuildRequires: bzip2-devel %endif %if %{defined buildkronosnetd} +BuildRequires: systemd BuildRequires: pam-devel %endif %if %{defined buildautogen} @@ -460,6 +460,9 @@ %endif %changelog +* Sun Feb 25 2018 Madison Kelly <mkelly> - 1.1-2 +- Moved the 'BuildRequires: systemd' to be conditional with kronostnetd. + * Sun Feb 25 2018 Madison Kelly <mkelly> - 1.1-1 - Rerolled for 1.1 upstream release. - Removed the (no longer needed) gcc8-fixes.patch ====
@digimer: Perfect, looks really nice, and good work with packaging latest version. I've just noticed one small problem - again with changelog - and it's using of macros in the changelog. This is problem, because macros are expanded no matter that they are in the changelog section. So please remove %{_isa} and %{?systemd_requires} otherwise changelog gets screwed.
Fixed. New .spec and srpm: https://www.alteeve.com/an-repo/files/packages/kronosnet.spec.1.1-3 https://www.alteeve.com/an-repo/files/packages/kronosnet-1.1-3.fc27.src.rpm f26: https://koji.fedoraproject.org/koji/taskinfo?taskID=25322696 f27: https://koji.fedoraproject.org/koji/taskinfo?taskID=25322704 rawhide: https://koji.fedoraproject.org/koji/taskinfo?taskID=25322713 epel7: https://koji.fedoraproject.org/koji/taskinfo?taskID=25322722 Diff from 1.1-2: ==== --- kronosnet.spec.1.1-2 2018-02-25 23:17:33.268040008 -0500 +++ kronosnet.spec.1.1-3 2018-02-26 10:03:13.304992918 -0500 @@ -70,7 +70,7 @@ Name: kronosnet Summary: Multipoint-to-Multipoint VPN daemon Version: 1.1 -Release: 2%{?dist} +Release: 3%{?dist} License: GPLv2+ + LGPLv2+ URL: http://www.kronosnet.org Source0: http://www.kronosnet.org/releases/kronosnet-%{version}.tar.gz @@ -460,6 +460,9 @@ %endif %changelog +* Mon Feb 26 2018 Madison Kelly <mkelly> - 1.1-3 +- Fixed the changelog to not have the full macro names. + * Sun Feb 25 2018 Madison Kelly <mkelly> - 1.1-2 - Moved the 'BuildRequires: systemd' to be conditional with kronostnetd. @@ -474,7 +477,7 @@ * Thu Feb 22 2018 Madison Kelly <mkelly> - 1.0-9 - Changed the source tarball to be one from the upstream source. - Updated the main license. -- Moved down %{?systemd_requires}. +- Moved down the {?systemd_requires} macro. * Fri Feb 16 2018 Madison Kelly <mkelly> - 1.0-8 - Reverted to 'BuildRequires: bzip2-devel' to fix EPEL7 builds. @@ -490,7 +493,7 @@ - Removed sysvinit checks. - Fixed the groupadd to add to system group. - Added build requirement for systemd. -- Added %{_isa} to knet requirements in sub packages. +- Added {_isa} macro to knet requirements in sub packages. * Mon Feb 12 2018 Madison Kelly <mkelly> - 1.0-5 - All changes related to this spec; ====
Package is ok to go into Fedora. Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed ===== MUST items ===== C/C++: [x]: Package does not contain kernel modules. [x]: Package contains no static executables. [x]: Development (unversioned) .so files in -devel subpackage, if present. Note: Unversioned so-files in private %_libdir subdirectory (see attachment). Verify they are not in ld path. [x]: ldconfig called in %post and %postun if required. [x]: Package does not contain any libtool archives (.la) [x]: Rpath absent or only used for internal libs. Generic: [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x]: License field in the package spec file matches the actual license. [x]: License file installed when any subpackage combination is installed. [x]: Package does not own files or directories owned by other packages. [x]: %build honors applicable compiler flags or justifies otherwise. [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: Package contains desktop file if it is a GUI application. [x]: Development files must be in a -devel package [x]: Package uses nothing in %doc for runtime. [x]: Package consistently uses macros (instead of hard-coded directory names). [x]: Package is named according to the Package Naming Guidelines. [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [x]: Package contains systemd file(s) if in need. [x]: Useful -debuginfo package or justification otherwise. [x]: Package is not known to require an ExcludeArch tag. [x]: Package complies to the Packaging Guidelines [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). [x]: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %license. [x]: Package requires other packages for directories it uses. [x]: Package must own all directories that it creates. [x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Dist tag is present. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [x]: Package use %makeinstall only when make install DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: Package does not use a name that already exists. [x]: Package is not relocatable. [x]: Sources used to build the package match the upstream source, as provided in the spec URL. [x]: Spec file name must match the spec package %{name}, in the format %{name}.spec. [x]: File names are valid UTF-8. [x]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 0 bytes in 0 files. [x]: Packages must not store files under /srv, /opt or /usr/local ===== SHOULD items ===== Generic: [-]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x]: Final provides and requires are sane (see attachments). [x]: Fully versioned dependency in subpackages if applicable. [?]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [x]: Scriptlets must be sane, if used. [-]: Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [x]: Package should compile and build into binary rpms on all supported architectures. [-]: %check is present and all tests pass. [x]: Packages should try to preserve timestamps of original installed files. [x]: Reviewer should test that the package builds in mock. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: Uses parallel make %{?_smp_mflags} macro. [x]: The placement of pkgconfig(.pc) files are correct. [x]: Sources can be downloaded from URI in Source: tag [x]: SourceX is a working URL. [x]: Spec use %global instead of %define unless justified. ===== EXTRA items ===== Generic: [x]: Rpmlint is run on debuginfo package(s). Note: There are rpmlint messages (see attachment). [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. [x]: Spec file according to URL is the same as in SRPM. Rpmlint ------- Checking: kronosnet-debuginfo-1.1-3.fc28.i686.rpm kronosnet-debugsource-1.1-3.fc28.i686.rpm libknet1-1.1-3.fc28.i686.rpm libknet1-devel-1.1-3.fc28.i686.rpm libknet1-crypto-nss-plugin-1.1-3.fc28.i686.rpm libknet1-crypto-openssl-plugin-1.1-3.fc28.i686.rpm libknet1-compress-zlib-plugin-1.1-3.fc28.i686.rpm libknet1-compress-lz4-plugin-1.1-3.fc28.i686.rpm libknet1-compress-lzo2-plugin-1.1-3.fc28.i686.rpm libknet1-compress-lzma-plugin-1.1-3.fc28.i686.rpm libknet1-compress-bzip2-plugin-1.1-3.fc28.i686.rpm libknet1-crypto-plugins-all-1.1-3.fc28.i686.rpm libknet1-compress-plugins-all-1.1-3.fc28.i686.rpm libknet1-plugins-all-1.1-3.fc28.i686.rpm kronosnet-1.1-3.fc28.src.rpm kronosnet-debuginfo.i686: W: invalid-license GPLv2+ + LGPLv2+ kronosnet-debugsource.i686: W: invalid-license GPLv2+ + LGPLv2+ kronosnet-debugsource.i686: W: no-documentation libknet1.i686: W: spelling-error Summary(en_US) Kronosnet -> Kronecker libknet1.i686: W: spelling-error %description -l en_US Kronosnet -> Kronecker libknet1.i686: W: spelling-error %description -l en_US knet -> net, knelt, knee libknet1.i686: W: no-documentation libknet1-devel.i686: W: spelling-error Summary(en_US) Kronosnet -> Kronecker libknet1-devel.i686: W: spelling-error %description -l en_US kronosnet -> Kronecker libknet1-crypto-nss-plugin.i686: W: no-documentation libknet1-crypto-openssl-plugin.i686: W: no-documentation libknet1-compress-zlib-plugin.i686: W: no-documentation libknet1-compress-lz4-plugin.i686: W: no-documentation libknet1-compress-lzo2-plugin.i686: W: no-documentation libknet1-compress-lzma-plugin.i686: W: no-documentation libknet1-compress-bzip2-plugin.i686: W: no-documentation libknet1-crypto-plugins-all.i686: W: no-documentation libknet1-compress-plugins-all.i686: W: no-documentation libknet1-plugins-all.i686: W: no-documentation kronosnet.src: W: spelling-error Summary(en_US) Multipoint -> Multipurpose kronosnet.src: W: invalid-license GPLv2+ + LGPLv2+ 15 packages and 0 specfiles checked; 0 errors, 21 warnings. Rpmlint (debuginfo) ------------------- Checking: kronosnet-debuginfo-1.1-3.fc28.i686.rpm kronosnet-debuginfo.i686: W: invalid-license GPLv2+ + LGPLv2+ 1 packages and 0 specfiles checked; 0 errors, 1 warnings.
Re macros in %changelog: Common way of escaping that convey what was meant down the road (e.g. to rpm -q --changelog query) is to use doubled '%', i.e., %{_isa} -> %%{_isa}, not to drop the per cent character. Some concerns that remain: A. [Comment 28] 4.: no reason to mangle with debuginfo generation - one can always use command-line switches to achieve the same: http://lists.rpm.org/pipermail/rpm-list/2013-April/001416.html (definitely not a mainstream need, even less in Fedora context) B. [Comment 11]: I'd still suggest using %{configure} \ > %{?with_sctp:--enable-libknet-sctp} \ > %{!?with_sctp:--disable-libknet-sctp} \ > [...] Reason is also practical, e.g. the whole "configure" statement barely fits a single laptop screen for me currently, because the notation of choice is excessively line-hungry. Also, please: C. Refrain from initial spaces/indentation in %description-s. D. Check whether there are some tests that could be run as part of the build under %check scriptlet (to be added if that's the case). E. Because libknet1-devel requires (indirectly) libknet1, it may drop the %license files, as these will be present thanks to libknet1 installed in parallel, hence satisfying: https://fedoraproject.org/wiki/Packaging:LicensingGuidelines?rd=Packaging/LicensingGuidelines#Subpackage_Licensing Rather for future consideration: - if the documentation for the API functions will keep growing, it might be desirable to split those manpages to a separate subpackage: https://fedoraproject.org/wiki/Packaging:Guidelines#Documentation ("Or if there's a lot of documentation...")
(In reply to Jan Pokorný from comment #55) > Re macros in %changelog: > > Common way of escaping that convey what was meant down the road > (e.g. to rpm -q --changelog query) is to use doubled '%', i.e., > %{_isa} -> %%{_isa}, not to drop the per cent character. > > > Some concerns that remain: > > A. [Comment 28] 4.: no reason to mangle with debuginfo generation > - one can always use command-line switches to achieve the same: > http://lists.rpm.org/pipermail/rpm-list/2013-April/001416.html > (definitely not a mainstream need, even less in Fedora context) > > B. [Comment 11]: I'd still suggest using > > %{configure} \ > > %{?with_sctp:--enable-libknet-sctp} \ > > %{!?with_sctp:--disable-libknet-sctp} \ > > [...] > > Reason is also practical, e.g. the whole "configure" statement > barely fits a single laptop screen for me currently, because > the notation of choice is excessively line-hungry. This is only a matter of personal preference. It doesn´t interfere in any way with the review. > > > Also, please: > > C. Refrain from initial spaces/indentation in %description-s. rationale? > > D. Check whether there are some tests that could be run as part of > the build under %check scriptlet (to be added if that's the case). this is a good point, but FYI upstream already has an extensive CI/CD including different versions of Fedora. My only concern is that the testsuite does play with the network (loopback interface) and should be very safe, but in the unlucky event of bugs, we should probably avoid DoS´ing the fedora builders by generating unwanted traffic. I think that Digimer choice to avoid running the test suite is more of a safe precaution. > > E. Because libknet1-devel requires (indirectly) libknet1, it may > drop the %license files, as these will be present thanks to > libknet1 installed in parallel, hence satisfying: > > https://fedoraproject.org/wiki/Packaging:LicensingGuidelines?rd=Packaging/ > LicensingGuidelines#Subpackage_Licensing Is this a blocker for the review or a wishlist level? what are the consequences of not doing it? > > > Rather for future consideration: > - if the documentation for the API functions will keep growing, > it might be desirable to split those manpages to a separate > subpackage: > https://fedoraproject.org/wiki/Packaging:Guidelines#Documentation > ("Or if there's a lot of documentation...") Hardly, the API is pretty stable at this point.
>> Some concerns that remain: >> >> A. [Comment 28] 4.: no reason to mangle with debuginfo generation >> - one can always use command-line switches to achieve the same: >> http://lists.rpm.org/pipermail/rpm-list/2013-April/001416.html >> (definitely not a mainstream need, even less in Fedora context) >> >> B. [Comment 11]: I'd still suggest using >> >> %{configure} \ >>> %{?with_sctp:--enable-libknet-sctp} \ >>> %{!?with_sctp:--disable-libknet-sctp} \ >>> [...] >> >> Reason is also practical, e.g. the whole "configure" statement >> barely fits a single laptop screen for me currently, because >> the notation of choice is excessively line-hungry. > > This is only a matter of personal preference. It doesn´t interfere > in any way with the review. Looks like bigger picture is neglected: having packages in somewhat unsurprising state (goal of package review, afterall) is not to the benefit of selected people, but for overall community. https://fedoraproject.org/wiki/Packaging:Guidelines#Spec_Legibility Packages get routinely modified by proven packagers when new packaging/technical needs arise. And this general eligibility/predictability applies to both above points. Do not discount this by unsound personal preference claims, please. It takes effort on both sides to undergo the review if the result is not to become maintainance (and daily distro usage, e.g. in case of ldconfig point H. below) burden anytime soon. For instance, when a package attempts to juggle with debuginfo generation on its own, it attracts attention needlessly, and it's not clear at all why this is needed in Fedora context. If you think this is relevant for Fedora, the justification shall be provided in the form of a comment. Regarding using superfluous globals/non-terse conditionals, that is something killing said general eligibility. Instead of: > %bcond_without sctp > [...] > %if %{with sctp} > %global buildsctp 1 > %endif > [...] > %if %{defined buildsctp} > BuildRequires: lksctp-tools-devel > %endif > [...] > %{configure} \ > %if %{defined buildsctp} > --enable-libknet-sctp \ > %else > --disable-libknet-sctp \ > %endif please use: > %bcond_without sctp > [...] > %if %{with sctp} > BuildRequires: lksctp-tools-devel > %endif > [...] > %{configure} \ > %{?with_sctp:--enable-libknet-sctp} \ > %{!?with_sctp:--disable-libknet-sctp} \ [Voila, some bogus lines down, while eligibility raises.] >> Also, please: >> >> C. Refrain from initial spaces/indentation in %description-s. > > rationale? Not looking weird in comparison to other packages, e.g. in various output of console programs dealing with packages. (see the above note on unsurprising state) >> D. Check whether there are some tests that could be run as part of >> the build under %check scriptlet (to be added if that's the case). > > this is a good point, but FYI upstream already has an extensive CI/CD > including different versions of Fedora. This is indeed nice and respectable. But to provide other use cases, some preliminary toolchain rebuilds can be performed on the package base in sole discretion of the people involved, and having some kind of smoke test will help establish the confidence all work alright, before even hitting Rawhide. Also the number of architectures covered this way is rather impressive. And there are more benefits down the road AIUI, like test-rebuilding the inverse dependencies, IOW when some of kronosnet's dependency is receiving an update, for instance. > My only concern is that the testsuite does play with the network > (loopback interface) and should be very safe, but in the unlucky > event of bugs, we should probably avoid DoS´ing the fedora builders > by generating unwanted traffic. I think that Digimer choice to avoid > running the test suite is more of a safe precaution. Might be at least worth adding the %check scriptlet triggering some straightforward test in a commented-out form together with a brief explanation why it is deactivated per above. >> E. Because libknet1-devel requires (indirectly) libknet1, it may >> drop the %license files, as these will be present thanks to >> libknet1 installed in parallel, hence satisfying: >> >> https://fedoraproject.org/wiki/Packaging:LicensingGuidelines?rd=Packaging/ >> LicensingGuidelines#Subpackage_Licensing > > Is this a blocker for the review or a wishlist level? what are the > consequences of not doing it? Purpose of the package review is that at least at one well-defined point the packaging is known to be in order and in-line with the guidelines, otherwise it's mere relying on the eventual/hypothetical settlement (for which COPR repositories exist). In this light, let's just do it. * * * In addition, I have these (and no more, I swear) complaints: F. only and/or and parentheses are meta-connectives for the license tag: > GPLv2+ + LGPLv2+ should become > GPLv2+ and LGPLv2+ https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Multiple_Licensing_Scenarios G. regarding naming guidelines, explanation is missing why digit-ending packages (libknet1) are present, which is not customary in Fedora and only expected when multiple versions of a package can be installed simultaneously: https://fedoraproject.org/wiki/Packaging:Naming?rd=Packaging:NamingGuidelines#MultiplePackages Is this indeed the case with kronosnet? It doesn't look like that to me, otherwise the plugin dir (%{_libdir}/kronosnet) would also be versioned (e.g., %{_libdir}/kronosnet1). Then, please drop those trailing digits from package names at all places. H. instead of > %post -n SUBPKG -p /sbin/ldconfig use > %ldconfig_scriptlets SUBPKG the appropriate place: https://fedoraproject.org/wiki/Packaging:Scriptlets#Shared_Libraries * * * FYI only: I. possible overlinking reported by rpmlint: > libknet1.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libknet.so.1.1.0 /lib64/libm.so.6 J. "Suggests:" hints may be suitable for libknet on *-plugins-all ("Recommends:" will be understood in dnf case as "Requires:" by default, which may not be always desired, turning Suggests to Recommends looks less problematic than the opposite should the change become appealing)
... see https://fedoraproject.org/wiki/Packaging:WeakDependencies
(In reply to Jan Pokorný from comment #57) > >> Some concerns that remain: > >> > >> A. [Comment 28] 4.: no reason to mangle with debuginfo generation > >> - one can always use command-line switches to achieve the same: > >> http://lists.rpm.org/pipermail/rpm-list/2013-April/001416.html > >> (definitely not a mainstream need, even less in Fedora context) > >> > >> B. [Comment 11]: I'd still suggest using > >> > >> %{configure} \ > >>> %{?with_sctp:--enable-libknet-sctp} \ > >>> %{!?with_sctp:--disable-libknet-sctp} \ > >>> [...] > >> > >> Reason is also practical, e.g. the whole "configure" statement > >> barely fits a single laptop screen for me currently, because > >> the notation of choice is excessively line-hungry. > > > > This is only a matter of personal preference. It doesn´t interfere > > in any way with the review. > > Looks like bigger picture is neglected: having packages in somewhat > unsurprising state (goal of package review, afterall) is not to the > benefit of selected people, but for overall community. > > https://fedoraproject.org/wiki/Packaging:Guidelines#Spec_Legibility > > Packages get routinely modified by proven packagers when > new packaging/technical needs arise. And this general > eligibility/predictability applies to both above points. > Do not discount this by unsound personal preference claims, please. > It takes effort on both sides to undergo the review if the result > is not to become maintainance (and daily distro usage, e.g. in > case of ldconfig point H. below) burden anytime soon. > > For instance, when a package attempts to juggle with debuginfo > generation on its own, it attracts attention needlessly, and it's > not clear at all why this is needed in Fedora context. If you > think this is relevant for Fedora, the justification shall be > provided in the form of a comment. > > Regarding using superfluous globals/non-terse conditionals, that > is something killing said general eligibility. Instead of: > > > %bcond_without sctp > > [...] > > %if %{with sctp} > > %global buildsctp 1 > > %endif > > [...] > > %if %{defined buildsctp} > > BuildRequires: lksctp-tools-devel > > %endif > > [...] > > %{configure} \ > > %if %{defined buildsctp} > > --enable-libknet-sctp \ > > %else > > --disable-libknet-sctp \ > > %endif > > please use: > > > %bcond_without sctp > > [...] > > %if %{with sctp} > > BuildRequires: lksctp-tools-devel > > %endif > > [...] > > %{configure} \ > > %{?with_sctp:--enable-libknet-sctp} \ > > %{!?with_sctp:--disable-libknet-sctp} \ > > [Voila, some bogus lines down, while eligibility raises.] The current format still achieve the same technical goal. terse or verbose is still a matter of personal preference. The code is not obfuscated in either forms. > > >> Also, please: > >> > >> C. Refrain from initial spaces/indentation in %description-s. > > > > rationale? > > Not looking weird in comparison to other packages, e.g. in > various output of console programs dealing with packages. Please provide an example of which commands are you referring to. It´s hard to guess what you see without some data. > (see the above note on unsurprising state) > > >> D. Check whether there are some tests that could be run as part of > >> the build under %check scriptlet (to be added if that's the case). > > > > this is a good point, but FYI upstream already has an extensive CI/CD > > including different versions of Fedora. > > This is indeed nice and respectable. But to provide other use cases, > some preliminary toolchain rebuilds can be performed on the package > base in sole discretion of the people involved, and having some kind > of smoke test will help establish the confidence all work alright, > before even hitting Rawhide. Also the number of architectures covered > this way is rather impressive. And there are more benefits down the > road AIUI, like test-rebuilding the inverse dependencies, IOW when > some of kronosnet's dependency is receiving an update, for instance. the number of architectures is indeed a benefit, but we do test rebuild and check daily in CI to catch issues on latest of each distribution exactly for that use case. I am less interested in automatic rebuilds. Who does full rebuilds of fedora is never going to look at each single failure. > > > My only concern is that the testsuite does play with the network > > (loopback interface) and should be very safe, but in the unlucky > > event of bugs, we should probably avoid DoS´ing the fedora builders > > by generating unwanted traffic. I think that Digimer choice to avoid > > running the test suite is more of a safe precaution. > > Might be at least worth adding the %check scriptlet triggering some > straightforward test in a commented-out form together with a brief > explanation why it is deactivated per above. Reasonable. > > >> E. Because libknet1-devel requires (indirectly) libknet1, it may > >> drop the %license files, as these will be present thanks to > >> libknet1 installed in parallel, hence satisfying: > >> > >> https://fedoraproject.org/wiki/Packaging:LicensingGuidelines?rd=Packaging/ > >> LicensingGuidelines#Subpackage_Licensing > > > > Is this a blocker for the review or a wishlist level? what are the > > consequences of not doing it? > > Purpose of the package review is that at least at one well-defined > point the packaging is known to be in order and in-line with the > guidelines, otherwise it's mere relying on the eventual/hypothetical > settlement (for which COPR repositories exist). > In this light, let's just do it. You haven´t answered either of my questions. > > * * * > > In addition, I have these (and no more, I swear) complaints: > > F. only and/or and parentheses are meta-connectives for the license > tag: > > > GPLv2+ + LGPLv2+ > > should become > > > GPLv2+ and LGPLv2+ ok. > > > https://fedoraproject.org/wiki/Packaging: > LicensingGuidelines#Multiple_Licensing_Scenarios > > G. regarding naming guidelines, explanation is missing why digit-ending > packages (libknet1) are present, which is not customary in Fedora > and only expected when multiple versions of a package can be > installed simultaneously: > > https://fedoraproject.org/wiki/Packaging:Naming?rd=Packaging: > NamingGuidelines#MultiplePackages > > Is this indeed the case with kronosnet? It doesn't look like that > to me, otherwise the plugin dir (%{_libdir}/kronosnet) would also > be versioned (e.g., %{_libdir}/kronosnet1). Then, please drop > those trailing digits from package names at all places. It reflects the version of the onwire protocol. There are plenty libraries in Fedora that use similar convention. We can document it somewhere in the spec file. it´s not going to drop. > > H. instead of > > > %post -n SUBPKG -p /sbin/ldconfig > > use > > > %ldconfig_scriptlets SUBPKG > > the appropriate place: > https://fedoraproject.org/wiki/Packaging:Scriptlets#Shared_Libraries > > * * * > > FYI only: > > I. possible overlinking reported by rpmlint: > > > libknet1.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libknet.so.1.1.0 /lib64/libm.so.6 which version of the package did you test? we fixed that already upstream in 1.1. ./configure.ac does check if it is necessary to link to libm or not at build time to use ceil(). > > J. "Suggests:" hints may be suitable for libknet on *-plugins-all > ("Recommends:" will be understood in dnf case as "Requires:" > by default, which may not be always desired, turning Suggests > to Recommends looks less problematic than the opposite should > the change become appealing)
> > I. possible overlinking reported by rpmlint: > > > > > libknet1.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libknet.so.1.1.0 /lib64/libm.so.6 > > which version of the package did you test? we fixed that already upstream in > 1.1. ./configure.ac does check if it is necessary to link to libm or not at > build time to use ceil(). > Your rpm seems old. Neither Honza´s rpmlint, or mine show that error: [fabbione@lilith kronosnet]$ rpmlint libknet1-1.1-3.fc29.x86_64.rpm libknet1.x86_64: W: spelling-error Summary(en_US) Kronosnet -> Kronecker libknet1.x86_64: W: spelling-error %description -l en_US Kronosnet -> Kronecker libknet1.x86_64: W: spelling-error %description -l en_US knet -> net, knelt, knee libknet1.x86_64: W: no-documentation 1 packages and 0 specfiles checked; 0 errors, 4 warnings. tested with the latest brew build from Digimer and on all rpms generated by CI.
Created attachment 1402543 [details] Example simplification Slipped the recent comment, but mentioned in [comment 54]: - either do not refer to particular macros in the changelog at all, double '%' characters (producing single '%' after macro processing, preventing macro expansion as such when it happens during the build process), or put it in some other understandable way, e.g.: "_isa" macro * * * >> [re A. and B.[ > > The current format still achieve the same technical goal. terse or > verbose is still a matter of personal preference. The code is not > obfuscated in either forms. We still did not get to why you insist on avoidable (B.) or inappropriate (A.) complexities without any gain for Fedora ecosystem. Attached is the patch how it may look. >>>> Also, please: >>>> >>>> C. Refrain from initial spaces/indentation in %description-s. >>> >>> rationale? >> >> Not looking weird in comparison to other packages, e.g. in >> various output of console programs dealing with packages. > > Please provide an example of which commands are you referring to. > It´s hard to guess what you see without some data. "rpm -qi" is the first test of choice. > [%check scriptlet discussion] Thanks. >> [re E.] > > You haven´t answered either of my questions. My answer was: let's just do it. >> [re G.] > > It reflects the version of the onwire protocol. There are plenty > libraries in Fedora that use similar convention. We can document > it somewhere in the spec file. it´s not going to drop. True, and other library cases mainly boil down to "multiple versions installed simulatenously" scenario as shown. Explicitly designating on-wire compatible series may also make sense, then please make this apparent in package summaries, e.g.: -Summary: Kronosnet core switching implementation +Summary: Kronosnet core switching implementation (protocol v1) >> [re I.] > > which version of the package did you test? we fixed that already > upstream in 1.1. ./configure.ac does check if it is necessary to link > to libm or not at build time to use ceil(). > > Your rpm seems old. Used SRPM per [comment 52], using. Just rechecked. It can be found in "Rpmlint (installed packages)" of review.txt generated with "fedora-review -rn kronosnet-1.1-3.fc27.src.rpm" on Rawhide. As mentioned, this has no effect on the review itself, just a feedback about detections (allowing for false positives) observed.
(In reply to Jan Pokorný from comment #61) > Created attachment 1402543 [details] > Example simplification > > Slipped the recent comment, but mentioned in [comment 54]: > - either do not refer to particular macros in the changelog > at all, double '%' characters (producing single '%' after > macro processing, preventing macro expansion as such when > it happens during the build process), or put it in some other > understandable way, e.g.: "_isa" macro > > * * * > > >> [re A. and B.[ > > > > The current format still achieve the same technical goal. terse or > > verbose is still a matter of personal preference. The code is not > > obfuscated in either forms. > > We still did not get to why you insist on avoidable (B.) or > inappropriate (A.) complexities without any gain for Fedora > ecosystem. > Because it´s impossible to follow those jumps you do around between comments and we have no idea what you are talking about related to A. What is A? How about adding some context or spec file snippets to make it clear? If B is the configure switches, you need to learn to discern what is required for a review or nice to have. You mentioned different times that it is nice to have and since there are no fedora strict requirements to switch to your format of choice, then I don´t see why we have to change it. > Attached is the patch how it may look. > > > >>>> Also, please: > >>>> > >>>> C. Refrain from initial spaces/indentation in %description-s. > >>> > >>> rationale? > >> > >> Not looking weird in comparison to other packages, e.g. in > >> various output of console programs dealing with packages. > > > > Please provide an example of which commands are you referring to. > > It´s hard to guess what you see without some data. > > "rpm -qi" is the first test of choice. Ok now I can see it. > > > > [%check scriptlet discussion] > > Thanks. > > > >> [re E.] > > > > You haven´t answered either of my questions. > > My answer was: let's just do it. My questions are still unanswered. I asked if it is breaking policies or not. > > > >> [re G.] > > > > It reflects the version of the onwire protocol. There are plenty > > libraries in Fedora that use similar convention. We can document > > it somewhere in the spec file. it´s not going to drop. > > True, and other library cases mainly boil down to "multiple versions > installed simulatenously" scenario as shown. Explicitly designating > on-wire compatible series may also make sense, then please make this > apparent in package summaries, e.g.: > > -Summary: Kronosnet core switching implementation > +Summary: Kronosnet core switching implementation (protocol v1) ok that´s reasonable. > > > >> [re I.] > > > > which version of the package did you test? we fixed that already > > upstream in 1.1. ./configure.ac does check if it is necessary to link > > to libm or not at build time to use ceil(). > > > > Your rpm seems old. > > Used SRPM per [comment 52], using. Just rechecked. > It can be found in "Rpmlint (installed packages)" of review.txt > generated with "fedora-review -rn kronosnet-1.1-3.fc27.src.rpm" > on Rawhide. hmmm does fedora-review -rn rebuilds the package on the local system (aka f29?) or does it build in a chroot for f27 and test the end result.f27 binaries? > > As mentioned, this has no effect on the review itself, just > a feedback about detections (allowing for false positives) observed. Right, that´s clear. I am investigating because it annoys me. libknet uses ceil(). ceil() recently moved from libm to libc. The build system might detect that it needs to link to libm but then when you do rpmlint on a different system that has ceil() in libc, the symbol check would report the warning. So I prefer to understand if the upstream check is bogus on fedora or if the test environment (despite being just a minor warning) is being tricked to think it´s a problem.
> Because it´s impossible to follow those jumps Really? Enumerating separate points was to prevent any confusion. > you do around between comments and we have no idea what you are > talking about related to A. What is A? For whoever is "we", A. was defined in [comment 55]... > How about adding some context or spec file snippets to make > it clear? ...see [attachment 1402543 [details]], lines 25,26, 28-30, 458-460 in the original. Hopefully it clarifies it fully. >>>> [re E.] >>> >>> You haven´t answered either of my questions. >> >> My answer was: let's just do it. > > My questions are still unanswered. I asked if it is breaking > policies or not. Let me explain that the review is not a black-or-white mechanical process, which appears to be your current view. It's meant to combine experience and common sense to find near-optimal solutions, individually. In this case, the undisclosed reason behind my suggestion is the common sense one: to eliminate redundancy. To keep Fedora scalable regarding mirrors, minimal installations, etc., it's really good to consider whether the identical files need to be carried over twice per each architecture when not necessary. Common sense hence says, it's enough -- thanks to intra-srpm dependencies -- to carry just a single copy per arch. Packaging guidelines give the green light to this "optimization". I see I should have been more patient to provide this explanation first-hand, sorry about that. Would you be willing to make this change, i.e., to drop license files from libknet1-devel subpackage, now? * * * > hmmm does fedora-review -rn rebuilds the package on the local system > (aka f29?) or does it build in a chroot for f27 and test the end > result.f27 binaries? It built the package detached from host, using mock (systemd-nspawn containers), which in turn obtained buildroot with > /usr/bin/dnf builddep --installroot \ > /var/lib/mock/fedora-rawhide-x86_64/root/ --releasever 28 \ > /var/lib/mock/fedora-rawhide-x86_64/root//builddir/build/SRPMS/kronosnet-1.1-3.fc28.src.rpm with toolchain: > gcc-8.0.1-0.14.fc28.x86_6 > binutils-2.29.1-19.fc28.x86_64 > glibc-2.27-3.fc28.x86_64 Then > So I prefer to understand if the upstream check is bogus on fedora or > if the test environment (despite being just a minor warning) is being > tricked to think it´s a problem. in the respective logs, I can see: > checking for library containing ceil... -lm It corresponds to this observation: $ readelf -s /usr/lib64/libc.so.6 | grep ceil || echo none > none So the warning may be false positive, after all.
(In reply to Jan Pokorný from comment #63) > > Because it´s impossible to follow those jumps > > Really? Enumerating separate points was to prevent any confusion. Yes really. > > > you do around between comments and we have no idea what you are > > talking about related to A. What is A? > > For whoever is "we", A. was defined in [comment 55]... "we" are all the people currently involved in this review to move this package forward. > > > How about adding some context or spec file snippets to make > > it clear? > > ...see [attachment 1402543 [details]], lines 25,26, 28-30, 458-460 in the > original. Hopefully it clarifies it fully. > was it that hard to reference the original lines? The debuginfo generation is default: on in fedora. Those statements have no effect on fedora unless explicitly overridden. Those are coming from upstream spec file that requires tuning to build debuginfo on Opensuse. They create no harm and have no effect on Fedora. The end result is the same. Something you missed in the process is that as the review goes, we are merging all those bits back into upstream spec file so that it can build properly both for suse and fedora / rhel / centos and reduce the need to maintain multiple spec files around (after all as you somehow agreed below in another context we want to kill redundancy). Given that those are more useful on opensuse we can add a: %if 0%{?suse_version} somewhere later on to make them even more transparent for fedora. Let´s move on. > > >>>> [re E.] > >>> > >>> You haven´t answered either of my questions. > >> > >> My answer was: let's just do it. > > > > My questions are still unanswered. I asked if it is breaking > > policies or not. > > Let me explain that the review is not a black-or-white > mechanical process, which appears to be your current view. > > It's meant to combine experience and common sense to find > near-optimal solutions, individually. Oh finally you start talking like a real mentor that a good reviewer should be. > > In this case, the undisclosed reason behind my suggestion is the common > sense one: to eliminate redundancy. To keep Fedora scalable regarding > mirrors, minimal installations, etc., it's really good to consider > whether the identical files need to be carried over twice per each > architecture when not necessary. Common sense hence says, it's enough > -- thanks to intra-srpm dependencies -- to carry just a single copy > per arch. Packaging guidelines give the green light to this > "optimization". I see I should have been more patient to provide > this explanation first-hand, sorry about that. > > Would you be willing to make this change, i.e., to drop license files > from libknet1-devel subpackage, now? Yes of course, now that you provided a good rationale on your request, I have no objections to make changes. > > * * * > > > hmmm does fedora-review -rn rebuilds the package on the local system > > (aka f29?) or does it build in a chroot for f27 and test the end > > result.f27 binaries? > > It built the package detached from host, using mock (systemd-nspawn > containers), which in turn obtained buildroot with > > /usr/bin/dnf builddep --installroot \ > > /var/lib/mock/fedora-rawhide-x86_64/root/ --releasever 28 \ > > /var/lib/mock/fedora-rawhide-x86_64/root//builddir/build/SRPMS/kronosnet-1.1-3.fc28.src.rpm > > with toolchain: > > gcc-8.0.1-0.14.fc28.x86_6 > > binutils-2.29.1-19.fc28.x86_64 > > glibc-2.27-3.fc28.x86_64 > > Then > > > So I prefer to understand if the upstream check is bogus on fedora or > > if the test environment (despite being just a minor warning) is being > > tricked to think it´s a problem. > > in the respective logs, I can see: > > checking for library containing ceil... -lm > > It corresponds to this observation: > $ readelf -s /usr/lib64/libc.so.6 | grep ceil || echo none > > none > > So the warning may be false positive, after all. Ok, that makes sense.
[re A.] > The debuginfo generation is default: on in fedora. Those statements > have no effect on fedora unless explicitly overridden. Those are > coming from upstream spec file that requires tuning to build debuginfo > on Opensuse. Can you enlighten me, then, how OpenSUSE and their OBS perhaps relate to the need to specify "debug_package" macro explicitly? I was unable to find such instructions, and the spec-s I looked at did not need it, either, e.g.: https://build.opensuse.org/package/view_file/devel:gcc/gcc8/gcc8.spec?expand=1 > They create no harm and have no effect on Fedora. The end result is > the same. I am not disputing direct effects, just the spec file clarity important for comprehension by arbitrary Fedora maintainers, per the linked statement in the guidelines: https://fedoraproject.org/wiki/Packaging:Guidelines#Spec_Legibility Anything not a concern of Fedora doesn't belong to a dedicated specfile. > Something you missed in the process is that as the review goes, we are > merging all those bits back into upstream spec file so that it can > build properly both for suse and fedora / rhel / centos and reduce the > need to maintain multiple spec files around (after all as you somehow > agreed below in another context we want to kill redundancy). > > Given that those are more useful on opensuse we can add a: > %if 0%{?suse_version} > somewhere later on to make them even more transparent for fedora. This is expressly forbidden, see the link. Therein lies a clear conflict of interest: - upstream: one-size-fits-all should it be interested in high-level packaging at all - downstream: well-tailored solution, following special needs and conventions of the distribution for its greater good Solution may be a mix of: - use conditionalized spec in upstream, drop irrelevant conditionals when reflecting the upstream changes in downstream - maintain downstream-quality spec files in upstream as discrete files per distro - synthesis of the previous two can be to use a macro language like M4 to conditionalize without spoiling the results (for clufter, I abused the fact that spec language is in itself based on expanding macros, which can be selectively blinded with external preprocessing: https://pagure.io/distill-spec, hence can have upstream spec file almost arbitrarily complex, but that's just a source for further chewing: https://pagure.io/clufter/blob/master/f/misc/clufter.spec) - apply nifty tricks, which we did for instance in pacemaker to support %license tag also in EL6: https://github.com/ClusterLabs/pacemaker/commit/a592019fbe88144bc42131b0e20deea96acd6d45
(In reply to Jan Pokorný from comment #65) > [re A.] > > > The debuginfo generation is default: on in fedora. Those statements > > have no effect on fedora unless explicitly overridden. Those are > > coming from upstream spec file that requires tuning to build debuginfo > > on Opensuse. > > Can you enlighten me, then, how OpenSUSE and their OBS perhaps relate > to the need to specify "debug_package" macro explicitly? I was unable > to find such instructions, and the spec-s I looked at did not need it, > either, e.g.: > https://build.opensuse.org/package/view_file/devel:gcc/gcc8/gcc8. > spec?expand=1 Not sure, we received the patch from the Suse maintainers, asking explicitly to enable it. Given that it didn´t affect Fedora I didn´t feel the need to investigate further. Probably the OBS is not the same as internal OpenSUSE build system? > > > They create no harm and have no effect on Fedora. The end result is > > the same. > > I am not disputing direct effects, just the spec file clarity > important for comprehension by arbitrary Fedora maintainers, per the > linked statement in the guidelines: > https://fedoraproject.org/wiki/Packaging:Guidelines#Spec_Legibility > > Anything not a concern of Fedora doesn't belong to a dedicated specfile. see below. > > > Something you missed in the process is that as the review goes, we are > > merging all those bits back into upstream spec file so that it can > > build properly both for suse and fedora / rhel / centos and reduce the > > need to maintain multiple spec files around (after all as you somehow > > agreed below in another context we want to kill redundancy). > > > > Given that those are more useful on opensuse we can add a: > > %if 0%{?suse_version} > > somewhere later on to make them even more transparent for fedora. > > This is expressly forbidden, see the link. the statement can be somehow interpreted: "To help facilitate legibility, only macros and conditionals for Fedora and EPEL are allowed to be used in Fedora Packages. Use of macros and conditionals for other distributions, including Fedora derivatives, is not permitted in spec files of packages in the main Fedora repositories unless those macros and conditionals are also present in Fedora." Then we can just change them to %if 0%{?fedora*..} but before answering, please read more below first. > > Therein lies a clear conflict of interest: > > - upstream: one-size-fits-all should it be interested in high-level > packaging at all > > - downstream: well-tailored solution, following special needs and > conventions of the distribution for its greater good > > Solution may be a mix of: > > - use conditionalized spec in upstream, drop irrelevant conditionals > when reflecting the upstream changes in downstream > > - maintain downstream-quality spec files in upstream as discrete files > per distro > > - synthesis of the previous two can be to use a macro language like > M4 to conditionalize without spoiling the results (for clufter, > I abused the fact that spec language is in itself based on expanding > macros, which can be selectively blinded with external preprocessing: > https://pagure.io/distill-spec, hence can have upstream spec file > almost arbitrarily complex, but that's just a source for further > chewing: https://pagure.io/clufter/blob/master/f/misc/clufter.spec) > > - apply nifty tricks, which we did for instance in pacemaker to > support %license tag also in EL6: > > https://github.com/ClusterLabs/pacemaker/commit/ > a592019fbe88144bc42131b0e20deea96acd6d45 The technicality on how to get there is an upstream problem, but ideally we will get the make $distro-specfile upstream to generate a valid (and policy compliant) spec file. One step at a time tho, once this is done, we can start merging things upstream for downstream benefit.
> Not sure, we received the patch from the Suse maintainers, asking > explicitly to enable it. Given that it didn´t affect Fedora I didn´t > feel the need to investigate further. > > Probably the OBS is not the same as internal OpenSUSE build system? https://github.com/kronosnet/kronosnet/pull/98#issuecomment-369880485 Btw. I am still thinking how the future protocol bumps will work out, shouldn't the pkgconfig file contain the versioning in its name as well (see dbus, glib, etc.)? If so, it would be preferred to make that change prior to inclusion (so that no bogus pkgconfig virtual provides get spread).
> > Btw. I am still thinking how the future protocol bumps will work out, > shouldn't the pkgconfig file contain the versioning in its name > as well (see dbus, glib, etc.)? If so, it would be preferred to make > that change prior to inclusion (so that no bogus pkgconfig virtual > provides get spread). whoever is going to use libknet will have BR libknet1-devel or libknetX-devel and Requires the equivalent. the BR is explicit.
But the client programs will likely use the pkgconfig dependency in the build setup, and that has to be differentiated eventually in case it cares about the protocol version. > PKG_CHECK_MODULES(DBUS, libknet1, ...) would make the specification targeted per the least surprise principle. Otherwise versioning subpackages directly in the name is just half-baked anticipation of future progress. And then, RPM is automatically picking any pkgconfig files, turning them into virtual "pkgconfig(X)" provides, i.e., fixed point in the package dependencies graph (it was used to avoid file-based dependencies hinted in [comment 28]), which is a concern from packaging perspective.
(In reply to Jan Pokorný from comment #69) > But the client programs will likely use the pkgconfig dependency in the > build setup, and that has to be differentiated eventually in case it > cares about the protocol version. The protocol is completely transparent to the final application. The API doesn´t allow mingling of the onwire protocol at any level. All the application cares is the API (and features) related to a given release. the pkg-config file correctly exports a Version: already. If an application needs newer versions they can rely on that at configure time.
Ok, the same argument can be applied to implicit versioning of subpackages (BuildRequires: libknet-devel%{?_isa} < 2.0), why do you want to treat these two things (subpackages and respective pkgconfig files) differently, especially (to repeat it) if it's customary for the latter even when some packages (dbus) do explicit versioning for the former in addition (dbus-1.pc while avoiding dbus1-devel as the name of a subpackage)? Am I the only to see a conflict here? Will hypothetical libknet2 ship its standalone libknet2.pc? Why not to apply unified approach and rename libknet.pc to libknet1.pc. Or conversely, to stop the explicit versioning in the subpackage names in there's ever to be just a single pkgconfig file...
(In reply to Jan Pokorný from comment #71) > Ok, the same argument can be applied to implicit versioning of > subpackages (BuildRequires: libknet-devel%{?_isa} < 2.0), why > do you want to treat these two things (subpackages and respective > pkgconfig files) differently, especially (to repeat it) if it's > customary for the latter even when some packages (dbus) do > explicit versioning for the former in addition (dbus-1.pc while > avoiding dbus1-devel as the name of a subpackage)? > > Am I the only to see a conflict here? I honestly don´t see the problem. One is upstream way to express versioning and one is packaging. Each distro has its own similar but different ways to handle it. > > Will hypothetical libknet2 ship its standalone libknet2.pc? No, it will ship libknet.pc, I don´t want or expect that v1 or v2 can be co-installed or co-exist in the same system. > Why not to apply unified approach and rename libknet.pc to > libknet1.pc. Or conversely, to stop the explicit versioning > in the subpackage names in there's ever to be just a single > pkgconfig file... I already explain why the libknet1 should have the number there to express protocol version being installed/used in that build.
Ok, if the expectations are set like this, meaning that the future obstacles I was worried about -- mostly related to parallel pkgconfig files as their names form de facto inter-dependencies parallel of API in RPM world (hence something that should be established wisely since the beginning because once the client packages will start to pick this "pkgconfig(libknet)", Pandora's box is open and maintenance burden cannot be taken back) -- won't occur (all libknet clients will need to be rebuilt for/ported to libknet2 at once on the single system, and all the systems they want to communicate with unless compatibility is preserved), I will conclude this extension of point G. with a simple task to have in-spec comment above "%files -n libknet1-devel" to express this explicitly, e.g.: > # libknet.pc leading to pkgconfig(libknet) automatic virtual provides, > # like other files, is not explicitly versioned in the name like the > # subpackages are -- intention of doing so for subpackage names is > # to ease the cross-checking the compatibility of the remote clients > # interchanging data using this network communication library, as > # the number denotes the protocol version (providing multiple > # protocol versions in parallel is not planned). > %files -n libknet1-devel [I hope I picked the gist of the reasoning right, but really can't see any other justification, to version packaged libraries like this all the time may be common for Debian, but this is not Debian, hopefully this is clear.] This is in addition to summary tags per [comment 61]. * * * Regarding A., I can tolerate the situation as is provided there's: - clear promise to do something about that in the future (I will follow-up on that github question to see if the original use case cannot be handled by other means, which would be the simplest solution, otherwise there are these casing options etc. to be implemented if upstream-downstream sync is required) - comment above "%debug_package" that it is not relevant for Fedora and its removal is pending * * * Please, present the fixed spec file so I can recheck and approve the review.
(In reply to Jan Pokorný from comment #73) > Ok, if the expectations are set like this, meaning that the future > obstacles I was worried about -- mostly related to parallel pkgconfig > files as their names form de facto inter-dependencies parallel of > API in RPM world (hence something that should be established wisely > since the beginning because once the client packages will start to > pick this "pkgconfig(libknet)", Pandora's box is open and maintenance > burden cannot be taken back) -- won't occur (all libknet clients will > need to be rebuilt for/ported to libknet2 at once on the single system, > and all the systems they want to communicate with unless compatibility > is preserved), I will conclude this extension of point G. with a simple > task to have in-spec comment above "%files -n libknet1-devel" to > express this explicitly, e.g.: > > > # libknet.pc leading to pkgconfig(libknet) automatic virtual provides, > > # like other files, is not explicitly versioned in the name like the > > # subpackages are -- intention of doing so for subpackage names is > > # to ease the cross-checking the compatibility of the remote clients > > # interchanging data using this network communication library, as > > # the number denotes the protocol version (providing multiple > > # protocol versions in parallel is not planned). > > %files -n libknet1-devel Good enough explanation. > > [I hope I picked the gist of the reasoning right, but really can't see > any other justification, to version packaged libraries like this all > the time may be common for Debian, but this is not Debian, hopefully > this is clear.] Let´s not mix up things please. Debian uses the soname there. > > This is in addition to summary tags per [comment 61]. > > * * * > > Regarding A., I can tolerate the situation as is provided there's: > > - clear promise to do something about that in the future > (I will follow-up on that github question to see if the original > use case cannot be handled by other means, which would be the > simplest solution, otherwise there are these casing options etc. > to be implemented if upstream-downstream sync is required) > > - comment above "%debug_package" that it is not relevant for > Fedora and its removal is pending ack. > > * * * > > Please, present the fixed spec file so I can recheck and approve > the review.
There has been a lot of back and forth, but I think I got most of it. Please review and let me know if anything is still overlooked/outstanding. New .spec and srpm: https://www.alteeve.com/an-repo/files/packages/kronosnet.spec.1.1-4 https://www.alteeve.com/an-repo/files/packages/kronosnet-1.1-4.fc27.src.rpm f26: https://koji.fedoraproject.org/koji/taskinfo?taskID=25463902 f27: https://koji.fedoraproject.org/koji/taskinfo?taskID=25463917 f28: https://koji.fedoraproject.org/koji/taskinfo?taskID=25463942 rawhide: https://koji.fedoraproject.org/koji/taskinfo?taskID=25463975 epel7: https://koji.fedoraproject.org/koji/taskinfo?taskID=25463987 Diff from 1.1-3: ==== --- kronosnet.spec.1.1-3 2018-02-26 10:03:13.304992918 -0500 +++ kronosnet.spec.1.1-4 2018-03-04 02:00:23.290918796 -0500 @@ -70,8 +70,8 @@ Name: kronosnet Summary: Multipoint-to-Multipoint VPN daemon Version: 1.1 -Release: 3%{?dist} -License: GPLv2+ + LGPLv2+ +Release: 4%{?dist} +License: GPLv2+ and LGPLv2+ URL: http://www.kronosnet.org Source0: http://www.kronosnet.org/releases/kronosnet-%{version}.tar.gz @@ -190,6 +190,13 @@ # remove docs rm -rf %{buildroot}/usr/share/doc/kronosnet +# Disabled because of concern that the testsuite does not play nice with the +# network loopback interface. Upstream has a comprehensive CI/CD system which +# tests different versions of Fedora and should be very safe. In the unlikely +# event of bugs, we should probably avoid DoS´ing the fedora builders by +# generating unwanted traffic. +#%check + # main empty package %description kronosnet source @@ -205,14 +212,14 @@ %description -n kronosnetd The kronosnet daemon is a bridge between kronosnet switching engine - and kernel network tap devices, to create and administer a - distributed LAN over multipoint-to-multipoint VPNs. +and kernel network tap devices, to create and administer a +distributed LAN over multipoint-to-multipoint VPNs. - The daemon does a poor attempt to provide a configure UI similar - to other known network devices/tools (Cisco, quagga). - Beside looking horrific, it allows runtime changes and - reconfiguration of the kronosnet(s) without daemon reload - or service disruption. +The daemon does a poor attempt to provide a configure UI similar +to other known network devices/tools (Cisco, quagga). +Beside looking horrific, it allows runtime changes and +reconfiguration of the kronosnet(s) without daemon reload +or service disruption. %post -n kronosnetd %systemd_post kronosnetd.service @@ -242,15 +249,15 @@ License: LGPLv2+ %description -n libtap1 - This is an over-engineered commodity library to manage a pool - of tap devices and provides the basic - pre-up.d/up.d/down.d/post-down.d infrastructure. +This is an over-engineered commodity library to manage a pool +of tap devices and provides the basic +pre-up.d/up.d/down.d/post-down.d infrastructure. %files -n libtap1 %license COPYING.* COPYRIGHT %{_libdir}/libtap.so.* -%post -n libtap1 -p /sbin/ldconfig +%ldconfig_scriptlets libtap1 %postun -n libtap1 -p /sbin/ldconfig @@ -261,9 +268,9 @@ Requires: pkgconfig %description -n libtap1-devel - This is an over-engineered commodity library to manage a pool - of tap devices and provides the basic - pre-up.d/up.d/down.d/post-down.d infrastructure. +This is an over-engineered commodity library to manage a pool +of tap devices and provides the basic +pre-up.d/up.d/down.d/post-down.d infrastructure. %files -n libtap1-devel %license COPYING.* COPYRIGHT @@ -273,19 +280,19 @@ %endif %package -n libknet1 -Summary: Kronosnet core switching implementation +Summary: Kronosnet core switching implementation (protocol v1) License: LGPLv2+ BuildRequires: libqb-devel BuildRequires: doxygen %description -n libknet1 - Kronosnet, often referred to as knet, is a network abstraction layer - designed for High Availability use cases, where redundancy, security, - fault tolerance and fast fail-over are the core requirements of your - application. +Kronosnet, often referred to as knet, is a network abstraction layer +designed for High Availability use cases, where redundancy, security, +fault tolerance and fast fail-over are the core requirements of your +application. - The whole kronosnet core is implemented in this library. - Please refer to https://kronosnet.org/ for further information. +The whole kronosnet core is implemented in this library. +Please refer to https://kronosnet.org/ for further information. %files -n libknet1 %license COPYING.* COPYRIGHT @@ -303,12 +310,18 @@ Requires: pkgconfig %description -n libknet1-devel - The whole kronosnet core is implemented in this library. - Please refer to the not-yet-existing documentation for further - information. - +The whole kronosnet core is implemented in this library. +Please refer to the not-yet-existing documentation for further +information. + +# libknet.pc leading to pkgconfig(libknet) automatic virtual provides, +# like other files, is not explicitly versioned in the name like the +# subpackages are -- intention of doing so for subpackage names is +# to ease the cross-checking the compatibility of the remote clients +# interchanging data using this network communication library, as +# the number denotes the protocol version (providing multiple +# protocol versions in parallel is not planned). %files -n libknet1-devel -%license COPYING.* COPYRIGHT %{_libdir}/libknet.so %{_includedir}/libknet.h %{_libdir}/pkgconfig/libknet.pc @@ -321,7 +334,7 @@ Requires: libknet1%{_isa} = %{version}-%{release} %description -n libknet1-crypto-nss-plugin - NSS crypto support for libknet1. +NSS crypto support for libknet1. %files -n libknet1-crypto-nss-plugin %{_libdir}/kronosnet/crypto_nss.so @@ -334,7 +347,7 @@ Requires: libknet1%{_isa} = %{version}-%{release} %description -n libknet1-crypto-openssl-plugin - OpenSSL crypto support for libknet1. +OpenSSL crypto support for libknet1. %files -n libknet1-crypto-openssl-plugin %{_libdir}/kronosnet/crypto_openssl.so @@ -347,7 +360,7 @@ Requires: libknet1%{_isa} = %{version}-%{release} %description -n libknet1-compress-zlib-plugin - zlib compression support for libknet1. +zlib compression support for libknet1. %files -n libknet1-compress-zlib-plugin %{_libdir}/kronosnet/compress_zlib.so @@ -359,7 +372,7 @@ Requires: libknet1%{_isa} = %{version}-%{release} %description -n libknet1-compress-lz4-plugin - lz4 and lz4hc compression support for libknet1. +lz4 and lz4hc compression support for libknet1. %files -n libknet1-compress-lz4-plugin %{_libdir}/kronosnet/compress_lz4.so @@ -373,7 +386,7 @@ Requires: libknet1%{_isa} = %{version}-%{release} %description -n libknet1-compress-lzo2-plugin - lzo2 compression support for libknet1. +lzo2 compression support for libknet1. %files -n libknet1-compress-lzo2-plugin %{_libdir}/kronosnet/compress_lzo2.so @@ -386,7 +399,7 @@ Requires: libknet1%{_isa} = %{version}-%{release} %description -n libknet1-compress-lzma-plugin - lzma compression support for libknet1. +lzma compression support for libknet1. %files -n libknet1-compress-lzma-plugin %{_libdir}/kronosnet/compress_lzma.so @@ -399,7 +412,7 @@ Requires: libknet1%{_isa} = %{version}-%{release} %description -n libknet1-compress-bzip2-plugin - bzip2 compression support for libknet1. +bzip2 compression support for libknet1. %files -n libknet1-compress-bzip2-plugin %{_libdir}/kronosnet/compress_bzip2.so @@ -416,7 +429,7 @@ %endif %description -n libknet1-crypto-plugins-all - meta package to install all of libknet1 crypto plugins +meta package to install all of libknet1 crypto plugins %files -n libknet1-crypto-plugins-all @@ -440,7 +453,7 @@ %endif %description -n libknet1-compress-plugins-all - meta package to install all of libknet1 compress plugins +meta package to install all of libknet1 compress plugins %files -n libknet1-compress-plugins-all @@ -451,7 +464,7 @@ Requires: libknet1-crypto-plugins-all%{_isa} = %{version}-%{release} %description -n libknet1-plugins-all - meta package to install all of libknet1 plugins +meta package to install all of libknet1 plugins %files -n libknet1-plugins-all @@ -460,6 +473,15 @@ %endif %changelog +* Sun Mar 04 2018 Madison Kelly <mkelly> - 1.1-4 +- Removed leading spaces from descriptions. +- Added the (commented out) %%check tests. +- Updated the changelog macro references to have two percent signs. +- Dropped the redundant libknet1-devel license files. +- Changed 'GPLv2+ + LGPLv2+' to 'GPLv2+ and LGPLv2+'. +- Updated %%ldconfig_scriptlets call. +- Clarified the kronosnet protocol version in the summary. + * Mon Feb 26 2018 Madison Kelly <mkelly> - 1.1-3 - Fixed the changelog to not have the full macro names. @@ -477,7 +499,7 @@ * Thu Feb 22 2018 Madison Kelly <mkelly> - 1.0-9 - Changed the source tarball to be one from the upstream source. - Updated the main license. -- Moved down the {?systemd_requires} macro. +- Moved down the %%{?systemd_requires} macro. * Fri Feb 16 2018 Madison Kelly <mkelly> - 1.0-8 - Reverted to 'BuildRequires: bzip2-devel' to fix EPEL7 builds. @@ -493,7 +515,7 @@ - Removed sysvinit checks. - Fixed the groupadd to add to system group. - Added build requirement for systemd. -- Added {_isa} macro to knet requirements in sub packages. +- Added %%{_isa} macro to knet requirements in sub packages. * Mon Feb 12 2018 Madison Kelly <mkelly> - 1.0-5 - All changes related to this spec; ====
Thanks to everyone involved, also for patience; I understand that time as a main metric might be preferred to overall quality, but I tried to explain the significance of the latter for the wider "package collection". As this is one-off currently, it's believed that the reviewed version will be used as a gauge for future updates. That being said, I find just two points to be addressed prior to approval, and I am at least partially responsible for the first one: * * * re H.: I should have explained better the required change, as currently it's only partially satisfied (beside following bad instructions). Let me provide wider context: - until very recently, /sbin/ldconfig invocations were interspersed in individual spec files in %post and %postun scriptlets (execution unit triggered as the package is being installed/removed in this case), to keep the dynamic linker cached knowledge about the installed libraries current -- so far so good, this is what previous version of this spec was using - some time ago, rpm added generic support for scriptlets to be run only once per whole transaction (comprising, e.g., all your updates received in one go) - relatively recently, wise Fedora maintainers realized there can be cumulated overhead arising from running ldconfig per package, when in most cases it's enough to run it per said transaction, and introduced new "self contained" change: https://fedoraproject.org/wiki/Changes/Removing_ldconfig_scriptlets which asks either to drop ldconfig invocation from the spec altogether (Fedora 28+ only), or to use the compatibility macros (to cover also pre-28 cases, which I believe is the intention with kronosnet) The problem is that we need to address all "ldconfig" occurrences, not just the one I've mistakenly mentioned in isolation in [comment 57]. Beside the description at the above page, we can also investigate on our own: $ rpm --showrc | grep -A3 ldconfig_scriptlets > -13: ldconfig_scriptlets(n:) %{?ldconfig: > %ldconfig_post %{?*} %{-n:-n %{-n*}} > %ldconfig_postun %{?*} %{-n:-n %{-n*}} > } We can grok that %ldconfig_scriptlets will handle both %post and %postun sides on its own. Regarding usage with subpackages, there's a confirmation that currently adopted usage is OK, e.g., on devel ML: https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/message/JZNJEN5FG5SAVQKC5RTWEJDQDLBETNI4/ Hence what I ask for: for both libtap1 and libknet1 subpackages, make sure that these occurrences in the original: > %post -n SUBPACKAGE -p /sbin/ldconfig > %postun -n SUBPACKAGE -p /sbin/ldconfig are both mapped to a single, unique line: > %ldconfig_scriptlets -n SUBPACKAGE As you can see, I mistakenly provided a bad piece of advice, actually, as using "-n SUBPACKAGE" vs. just "SUBPACKAGE" is aligned with how other macros (namely %package, but consequently %description, %files, %postun, etc.), and it's the former case with both libknet1 and libtap1. These naming subtleties are best understood here: http://ftp.rpm.org/max-rpm/s1-rpm-subpack-spec-file-changes.html#S3-RPM-SUBPACK-PACKAGE-DIRECTIVE-N-OPTION * * * Free-form implementation of > - comment above "%debug_package" that it is not relevant for > Fedora and its removal is pending [there] per [comment 73] is still missing. * * * Digimer, you are also free to cut off the older changelog entries (like up to and including 1.0-10) to make the situation perhaps a bit easier to grok, since those steps are all prior to inclusion (and hence not a game changer for downstream users), and you already demonstrated your changelog entries are spot-on. But it's merely up to you, there was certainly an undeniable effort invested since the initial version... (And if you decide to do so, there's no reason to state that very change in the changelog.)
Oh, and one more nit, %check might have contained, commented out, something actually usable for said quick check of the basic functionality. In case it would be desired in spite of the mentioned concerns. This is a mere recommendation.
> Free-form implementation of > >> - comment above "%debug_package" that it is not relevant for >> Fedora and its removal is pending [there] > > per [comment 73] is still missing. Did I misunderstand? I added the comment above "%files -n libknet1-devel"; ==== > # libknet.pc leading to pkgconfig(libknet) automatic virtual provides, > # like other files, is not explicitly versioned in the name like the > # subpackages are -- intention of doing so for subpackage names is > # to ease the cross-checking the compatibility of the remote clients > # interchanging data using this network communication library, as > # the number denotes the protocol version (providing multiple > # protocol versions in parallel is not planned). > %files -n libknet1-devel ==== Anyway, I moved it. I'm sorry, I don't understand the question/comment/suggestion in comment #77. Can you please restate?
I applied the changes and hit an odd build error against just epel7 on x86_64 pulling in an unexpected RPM on koji. Bumped again from 1.1-5 to -6 to add a version restriction to deal with it. Diff against -4 to -6 below. New .spec and srpm: https://www.alteeve.com/an-repo/files/packages/kronosnet.spec.1.1-6 https://www.alteeve.com/an-repo/files/packages/kronosnet-1.1-6.fc27.src.rpm f26: https://koji.fedoraproject.org/koji/taskinfo?taskID=25534544 f27: https://koji.fedoraproject.org/koji/taskinfo?taskID=25534556 f28: https://koji.fedoraproject.org/koji/taskinfo?taskID=25534564 rawhide: https://koji.fedoraproject.org/koji/taskinfo?taskID=25534572 epel7: https://koji.fedoraproject.org/koji/taskinfo?taskID=25534493 Diff from 1.1-4: ==== --- kronosnet.spec.1.1-4 2018-03-04 02:00:23.290918796 -0500 +++ kronosnet.spec.1.1-6 2018-03-07 01:19:35.321125418 -0500 @@ -70,7 +70,7 @@ Name: kronosnet Summary: Multipoint-to-Multipoint VPN daemon Version: 1.1 -Release: 4%{?dist} +Release: 6%{?dist} License: GPLv2+ and LGPLv2+ URL: http://www.kronosnet.org Source0: http://www.kronosnet.org/releases/kronosnet-%{version}.tar.gz @@ -93,7 +93,7 @@ BuildRequires: pkgconfig(zlib) %endif %if %{defined buildcompresslz4} -BuildRequires: pkgconfig(liblz4) +BuildRequires: pkgconfig(liblz4) >= 1.7 %endif %if %{defined buildcompresslzo2} BuildRequires: lzo-devel @@ -257,9 +257,7 @@ %license COPYING.* COPYRIGHT %{_libdir}/libtap.so.* -%ldconfig_scriptlets libtap1 - -%postun -n libtap1 -p /sbin/ldconfig +%ldconfig_scriptlets -n libtap1 %package -n libtap1-devel Summary: Simple userland wrapper around kernel tap devices (developer files) @@ -299,9 +297,7 @@ %{_libdir}/libknet.so.* %dir %{_libdir}/kronosnet -%post -n libknet1 -p /sbin/ldconfig - -%postun -n libknet1 -p /sbin/ldconfig +%ldconfig_scriptlets -n libknet1 %package -n libknet1-devel Summary: Kronosnet core switching implementation (developer files) @@ -314,13 +310,6 @@ Please refer to the not-yet-existing documentation for further information. -# libknet.pc leading to pkgconfig(libknet) automatic virtual provides, -# like other files, is not explicitly versioned in the name like the -# subpackages are -- intention of doing so for subpackage names is -# to ease the cross-checking the compatibility of the remote clients -# interchanging data using this network communication library, as -# the number denotes the protocol version (providing multiple -# protocol versions in parallel is not planned). %files -n libknet1-devel %{_libdir}/libknet.so %{_includedir}/libknet.h @@ -469,10 +458,25 @@ %files -n libknet1-plugins-all %if %{with rpmdebuginfo} +# libknet.pc leading to pkgconfig(libknet) automatic virtual provides, +# like other files, is not explicitly versioned in the name like the +# subpackages are -- intention of doing so for subpackage names is +# to ease the cross-checking the compatibility of the remote clients +# interchanging data using this network communication library, as +# the number denotes the protocol version (providing multiple +# protocol versions in parallel is not planned). %debug_package %endif %changelog +* Wed Mar 07 2018 Madison Kelly <mkelly> - 1.1-6 +- Added a version requirement to lz4 to deal with koji pulling in the + wrong package. + +* Tue Mar 06 2018 Madison Kelly <mkelly> - 1.1-5 +- Updated ldconfig scriptlet calls. +- Moved the debug_package leading comment. + * Sun Mar 04 2018 Madison Kelly <mkelly> - 1.1-4 - Removed leading spaces from descriptions. - Added the (commented out) %%check tests. @@ -493,45 +497,3 @@ - Removed the (no longer needed) gcc8-fixes.patch - Added the new doxygen and libqb-devel buildrequires for libknetd. -* Fri Feb 23 2018 Madison Kelly <mkelly> - 1.0-10 -- Added missing change log for 1.0-9. - -* Thu Feb 22 2018 Madison Kelly <mkelly> - 1.0-9 -- Changed the source tarball to be one from the upstream source. -- Updated the main license. -- Moved down the %%{?systemd_requires} macro. - -* Fri Feb 16 2018 Madison Kelly <mkelly> - 1.0-8 -- Reverted to 'BuildRequires: bzip2-devel' to fix EPEL7 builds. - -* Thu Feb 15 2018 Madison Kelly <mkelly> - 1.0-7 -- Added missing 1.0-6 changelog. -- Added kronosnetd postun. -- Clarified licensing. -- (re)added systemd_requires. -- Wrapped several buildrequires with pkgconfig(). - -* Wed Feb 14 2018 Madison Kelly <mkelly> - 1.0-6 -- Removed sysvinit checks. -- Fixed the groupadd to add to system group. -- Added build requirement for systemd. -- Added %%{_isa} macro to knet requirements in sub packages. - -* Mon Feb 12 2018 Madison Kelly <mkelly> - 1.0-5 -- All changes related to this spec; -- Fixed typo in previous changelog date (Tue -> Wed). -- Capitalized summaries as needed. -- Changed Requires that were file paths to package names. -- Added arch and version/release requirements for knet packages. -- Changed groupadd to be a little smarter. - -* Wed Jan 31 2018 Madison Kelly <mkelly> - 1.0-4 -- Added patch for gcc8 fix. -- Cleaned up description. - -* Tue Jan 30 2018 Madison Kelly <mkelly> - 1.0-3 -- Rebuild for Fedora release. - -* Tue Jan 30 2018 Autotools generated version <nobody> - 1.0-1-48.d96b. -- These aren't the droids you're looking for. - ====
Fixed comments. As an aside for later; When talking to people on #fedora-devel, it was recommended *not* to use 'BuildRequires: pkgconfig(x)', despite the package guidelines recommending it, as it is more ambiguous. The example given was: ==== issue occurs when various providers exists, ex: pkgconfig(openssl) mays pull openssl-devel or compat-openssl10-devel (the second being a terrible choice) ==== I don't want to hold up the approval process, but I wanted to bring this up as a possible change later. Thoughts? New .spec and srpm: https://www.alteeve.com/an-repo/files/packages/kronosnet.spec.1.1-7 https://www.alteeve.com/an-repo/files/packages/kronosnet-1.1-7.fc27.src.rpm f26: https://koji.fedoraproject.org/koji/taskinfo?taskID=25535007 f27: https://koji.fedoraproject.org/koji/taskinfo?taskID=25535014 f28: https://koji.fedoraproject.org/koji/taskinfo?taskID=25535022 rawhide: https://koji.fedoraproject.org/koji/taskinfo?taskID=25535030 epel7: https://koji.fedoraproject.org/koji/taskinfo?taskID=25535038 Diff from 1.1-6: ==== --- kronosnet.spec.1.1-6 2018-03-07 01:19:35.321125418 -0500 +++ kronosnet.spec.1.1-7 2018-03-07 01:50:40.831722937 -0500 @@ -70,7 +70,7 @@ Name: kronosnet Summary: Multipoint-to-Multipoint VPN daemon Version: 1.1 -Release: 6%{?dist} +Release: 7%{?dist} License: GPLv2+ and LGPLv2+ URL: http://www.kronosnet.org Source0: http://www.kronosnet.org/releases/kronosnet-%{version}.tar.gz @@ -310,6 +310,13 @@ Please refer to the not-yet-existing documentation for further information. +# libknet.pc leading to pkgconfig(libknet) automatic virtual provides, +# like other files, is not explicitly versioned in the name like the +# subpackages are -- intention of doing so for subpackage names is +# to ease the cross-checking the compatibility of the remote clients +# interchanging data using this network communication library, as +# the number denotes the protocol version (providing multiple +# protocol versions in parallel is not planned). %files -n libknet1-devel %{_libdir}/libknet.so %{_includedir}/libknet.h @@ -458,17 +465,15 @@ %files -n libknet1-plugins-all %if %{with rpmdebuginfo} -# libknet.pc leading to pkgconfig(libknet) automatic virtual provides, -# like other files, is not explicitly versioned in the name like the -# subpackages are -- intention of doing so for subpackage names is -# to ease the cross-checking the compatibility of the remote clients -# interchanging data using this network communication library, as -# the number denotes the protocol version (providing multiple -# protocol versions in parallel is not planned). +# This is left over from upstream. %debug_package %endif %changelog +* Wed Mar 07 2018 Madison Kelly <mkelly> - 1.1-7 +- Moved the comment back above '%%files -n libknet1-devel'. +- Added comment to '%%debug_package'. + * Wed Mar 07 2018 Madison Kelly <mkelly> - 1.1-6 - Added a version requirement to lz4 to deal with koji pulling in the wrong package. ====
(In reply to digimer from comment #80) > As an aside for later; When talking to people on #fedora-devel, it was > recommended *not* to use 'BuildRequires: pkgconfig(x)', despite the package > guidelines recommending it, as it is more ambiguous. The example given was: To be clearer, I want to share that pkgconfig(xx) dependency have issues, probably the reason why Guidelines only only states a "SHOULD", and thus xx-devel is a usual workaround (why may have other issues).
Thanks, Remi. To add a reference; The rhbz related to the -6 change to 'BuildRequire' is https://bugzilla.redhat.com/show_bug.cgi?id=1552431.
[the review shifts behind my back, not keen on fighting the mills, I am not an unprofessional rational-processes-bending person, just my responses and thank you for your work so far] There's a misunderstanding, "%files -n libknet1-devel" comment should stay where it was in 1.1.4. I was asking for a new one to explain the interim character of extra treatment of debug packages that shouldn't have been introduced in Fedora context in the first place. * * * re [comment 77], I am not familiar with how the test suite is run for kronosnet, an example command would be "make check". Nice-to-have category, though, the comment already explains why it is not so straightforward in this case to run the tests. * * * Thanks for dealing with lz4 issues. Regarding "pkgconfig(openssl)" expression of dependencies, yes, they can be versioned as well and/or can be combined with "Suggests" to prioritize particular underlying package name should the conflict on such virtual provides arise: https://fedoraproject.org/wiki/Packaging:WeakDependencies#Real_life_example Depending on how compat packages are structured, the same "satisfied by more packages" situation could occur also with the previous cryptical select-by-header-file approach, so there's effectively no regression in this comparison.
(In reply to Jan Pokorný from comment #83) > There's a misunderstanding, "%files -n libknet1-devel" comment should > stay where it was in 1.1.4. > > I was asking for a new one to explain the interim character of extra > treatment of debug packages that shouldn't have been introduced in > Fedora context in the first place. this is already addressed in comment #80 > > * * * > > re [comment 77], I am not familiar with how the test suite is run > for kronosnet, an example command would be "make check". > Nice-to-have category, though, the comment already explains why it > is not so straightforward in this case to run the tests. executing the test is straight forward make check, but we comment it out for safety. > > * * * > > Thanks for dealing with lz4 issues. > > Regarding "pkgconfig(openssl)" expression of dependencies, yes, they can > be versioned as well and/or can be combined with "Suggests" to prioritize > particular underlying package name should the conflict on such virtual > provides arise: > > https://fedoraproject.org/wiki/Packaging:WeakDependencies#Real_life_example > > Depending on how compat packages are structured, the same "satisfied by > more packages" situation could occur also with the previous cryptical > select-by-header-file approach, so there's effectively no regression > in this comparison. We will just switch back to BuildRequires: package-name. In context, upstream is also moving away from file based dependencies.
Removed the 'pkgconfig()' method of handling BuildRequires. New .spec and srpm: https://www.alteeve.com/an-repo/files/packages/kronosnet.spec.1.1-8 https://www.alteeve.com/an-repo/files/packages/kronosnet-1.1-8.fc27.src.rpm f26: https://koji.fedoraproject.org/koji/taskinfo?taskID=25592451 f27: https://koji.fedoraproject.org/koji/taskinfo?taskID=25592465 f28: https://koji.fedoraproject.org/koji/taskinfo?taskID=25592473 rawhide: https://koji.fedoraproject.org/koji/taskinfo?taskID=25592491 epel7: https://koji.fedoraproject.org/koji/taskinfo?taskID=25592503 Diff from 1.1-7: ==== --- kronosnet.spec.1.1-7 2018-03-07 01:50:40.831722937 -0500 +++ kronosnet.spec.1.1-8 2018-03-09 19:48:42.630061443 -0500 @@ -70,7 +70,7 @@ Name: kronosnet Summary: Multipoint-to-Multipoint VPN daemon Version: 1.1 -Release: 7%{?dist} +Release: 8%{?dist} License: GPLv2+ and LGPLv2+ URL: http://www.kronosnet.org Source0: http://www.kronosnet.org/releases/kronosnet-%{version}.tar.gz @@ -79,27 +79,27 @@ BuildRequires: gcc # required to build man pages BuildRequires: libxml2-devel doxygen -BuildRequires: pkgconfig(libqb) +BuildRequires: libqb-devel %if %{defined buildsctp} BuildRequires: lksctp-tools-devel %endif %if %{defined buildcryptonss} -BuildRequires: pkgconfig(nss) +BuildRequires: nss-devel %endif %if %{defined buildcryptoopenssl} -BuildRequires: pkgconfig(openssl) +BuildRequires: openssl-devel %endif %if %{defined buildcompresszlib} -BuildRequires: pkgconfig(zlib) +BuildRequires: zlib-devel %endif %if %{defined buildcompresslz4} -BuildRequires: pkgconfig(liblz4) >= 1.7 +BuildRequires: lz4-devel %endif %if %{defined buildcompresslzo2} BuildRequires: lzo-devel %endif %if %{defined buildcompresslzma} -BuildRequires: pkgconfig(liblzma) +BuildRequires: xz-devel %endif %if %{defined buildcompressbzip2} BuildRequires: bzip2-devel @@ -470,6 +470,10 @@ %endif %changelog +* Fri Mar 09 2018 Madison Kelly <mkelly> - 1.1-8 +- Changed pkgconfig() to normal package names to help avoid the wrong + package being pulled in to satisfy dependencies. + * Wed Mar 07 2018 Madison Kelly <mkelly> - 1.1-7 - Moved the comment back above '%%files -n libknet1-devel'. - Added comment to '%%debug_package'. ====
Package is ok to go into Fedora. Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed ===== MUST items ===== C/C++: [x]: Package does not contain kernel modules. [x]: Package contains no static executables. [x]: Development (unversioned) .so files in -devel subpackage, if present. Note: Unversioned so-files in private %_libdir subdirectory (see attachment). Verify they are not in ld path. [x]: ldconfig called in %post and %postun if required. [x]: Package does not contain any libtool archives (.la) [x]: Rpath absent or only used for internal libs. Generic: [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x]: License field in the package spec file matches the actual license. [x]: License file installed when any subpackage combination is installed. [x]: Package does not own files or directories owned by other packages. [x]: %build honors applicable compiler flags or justifies otherwise. [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: Package contains desktop file if it is a GUI application. [x]: Development files must be in a -devel package [x]: Package uses nothing in %doc for runtime. [x]: Package consistently uses macros (instead of hard-coded directory names). [x]: Package is named according to the Package Naming Guidelines. [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [x]: Package contains systemd file(s) if in need. [x]: Useful -debuginfo package or justification otherwise. [x]: Package is not known to require an ExcludeArch tag. [x]: Package complies to the Packaging Guidelines [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). [x]: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %license. [x]: Package requires other packages for directories it uses. [x]: Package must own all directories that it creates. [x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Dist tag is present. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [x]: Package use %makeinstall only when make install DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: Package does not use a name that already exists. [x]: Package is not relocatable. [x]: Sources used to build the package match the upstream source, as provided in the spec URL. [x]: Spec file name must match the spec package %{name}, in the format %{name}.spec. [x]: File names are valid UTF-8. [x]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 0 bytes in 0 files. [x]: Packages must not store files under /srv, /opt or /usr/local ===== SHOULD items ===== Generic: [-]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x]: Final provides and requires are sane (see attachments). [x]: Fully versioned dependency in subpackages if applicable. [?]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [x]: Scriptlets must be sane, if used. [-]: Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [x]: Package should compile and build into binary rpms on all supported architectures. [-]: %check is present and all tests pass. [x]: Packages should try to preserve timestamps of original installed files. [x]: Reviewer should test that the package builds in mock. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: Uses parallel make %{?_smp_mflags} macro. [x]: The placement of pkgconfig(.pc) files are correct. [x]: Sources can be downloaded from URI in Source: tag [x]: SourceX is a working URL. [x]: Spec use %global instead of %define unless justified. ===== EXTRA items ===== Generic: [x]: Rpmlint is run on debuginfo package(s). Note: There are rpmlint messages (see attachment). [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. [x]: Spec file according to URL is the same as in SRPM. Rpmlint ------- Checking: kronosnet-debuginfo-1.1-8.fc28.i686.rpm kronosnet-debugsource-1.1-8.fc28.i686.rpm libknet1-1.1-8.fc28.i686.rpm libknet1-devel-1.1-8.fc28.i686.rpm libknet1-crypto-nss-plugin-1.1-8.fc28.i686.rpm libknet1-crypto-openssl-plugin-1.1-8.fc28.i686.rpm libknet1-compress-zlib-plugin-1.1-8.fc28.i686.rpm libknet1-compress-lz4-plugin-1.1-8.fc28.i686.rpm libknet1-compress-lzo2-plugin-1.1-8.fc28.i686.rpm libknet1-compress-lzma-plugin-1.1-8.fc28.i686.rpm libknet1-compress-bzip2-plugin-1.1-8.fc28.i686.rpm libknet1-crypto-plugins-all-1.1-8.fc28.i686.rpm libknet1-compress-plugins-all-1.1-8.fc28.i686.rpm libknet1-plugins-all-1.1-8.fc28.i686.rpm kronosnet-1.1-8.fc28.src.rpm kronosnet-debugsource.i686: W: no-documentation libknet1.i686: W: spelling-error Summary(en_US) Kronosnet -> Kronecker libknet1.i686: W: spelling-error %description -l en_US Kronosnet -> Kronecker libknet1.i686: W: spelling-error %description -l en_US knet -> net, knelt, knee libknet1.i686: W: no-documentation libknet1.i686: E: library-without-ldconfig-postin /usr/lib/libknet.so.1.1.0 libknet1.i686: E: library-without-ldconfig-postun /usr/lib/libknet.so.1.1.0 libknet1-devel.i686: W: spelling-error Summary(en_US) Kronosnet -> Kronecker libknet1-devel.i686: W: spelling-error %description -l en_US kronosnet -> Kronecker libknet1-crypto-nss-plugin.i686: W: no-documentation libknet1-crypto-openssl-plugin.i686: W: no-documentation libknet1-compress-zlib-plugin.i686: W: no-documentation libknet1-compress-lz4-plugin.i686: W: no-documentation libknet1-compress-lzo2-plugin.i686: W: no-documentation libknet1-compress-lzma-plugin.i686: W: no-documentation libknet1-compress-bzip2-plugin.i686: W: no-documentation libknet1-crypto-plugins-all.i686: W: no-documentation libknet1-compress-plugins-all.i686: W: no-documentation libknet1-plugins-all.i686: W: no-documentation kronosnet.src: W: spelling-error Summary(en_US) Multipoint -> Multipurpose kronosnet.src:198: W: macro-in-comment %check 15 packages and 0 specfiles checked; 2 errors, 19 warnings. Rpmlint (debuginfo) ------------------- Checking: kronosnet-debuginfo-1.1-8.fc28.i686.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings.
@digimer: Because fedora-review is now set to +, it should be possible to follow https://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Add_Package_to_Source_Code_Management_.28SCM.29_system_and_Set_Owner . It basically means to use https://pagure.io/fedrepo_req (or actually fedpkg request-repo) and then just wait till repo is created. It's also good idea to pass all required branches (of course it's possible to do it later).
I've been away for work this last week, but I am returning later today. Finishing this process is my first ToDo.
https://pagure.io/releng/fedora-scm-requests/issue/5210 Note that I had to use the deprecated call (fedrepo-req kronosnet -t 1507103) as this: fedpkg request-repo 1507103 Errored with: Could not execute request_repo: The Bugzilla bug provided is not the proper type I doubt it makes a difference, but just in case.
(fedrepo-req-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/kronosnet
kronosnet-1.1-8.fc27 has been submitted as an update to Fedora 27. https://bodhi.fedoraproject.org/updates/FEDORA-2018-6eae0c4a51
kronosnet-1.1-8.fc28 has been submitted as an update to Fedora 28. https://bodhi.fedoraproject.org/updates/FEDORA-2018-94bdc0ccfb
kronosnet-1.1-8.el7 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2018-603c1c93cf
kronosnet-1.1-8.fc27 has been pushed to the Fedora 27 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2018-6eae0c4a51
kronosnet-1.1-8.el7 has been pushed to the Fedora EPEL 7 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2018-603c1c93cf
kronosnet-1.1-8.fc28 has been pushed to the Fedora 28 stable repository. If problems still persist, please make note of it in this bug report.
kronosnet-1.4-1.fc27 has been submitted as an update to Fedora 27. https://bodhi.fedoraproject.org/updates/FEDORA-2018-b4cad12543
kronosnet-1.4-1.el7 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2018-6280026399
kronosnet-1.4-1.fc27 has been pushed to the Fedora 27 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2018-b4cad12543
kronosnet-1.4-1.el7 has been pushed to the Fedora EPEL 7 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2018-6280026399
kronosnet-1.4-1.fc27 has been pushed to the Fedora 27 stable repository. If problems still persist, please make note of it in this bug report.
kronosnet-1.5-1.el7 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2018-8e2a9e3dbe
kronosnet-1.5-1.el7 has been pushed to the Fedora EPEL 7 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2018-8e2a9e3dbe
kronosnet-1.8-1.el7 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2019-4780c5a01a
kronosnet-1.8-1.el7 has been pushed to the Fedora EPEL 7 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2019-4780c5a01a
kronosnet-1.8-1.el7 has been pushed to the Fedora EPEL 7 stable repository. If problems still persist, please make note of it in this bug report.