[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #106 from Fedora Update System  ---
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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #105 from Fedora Update System  ---
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

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #104 from Fedora Update System  ---
kronosnet-1.8-1.el7 has been submitted as an update to Fedora EPEL 7.
https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2019-4780c5a01a

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #103 from Fedora Update System  ---
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

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #102 from Fedora Update System  ---
kronosnet-1.5-1.el7 has been submitted as an update to Fedora EPEL 7.
https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2018-8e2a9e3dbe

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #101 from Fedora Update System  ---
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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org/message/KRY4RBF4IYIGBRP3DZ5X3KCPZ7Q6AO7N/

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #100 from Fedora Update System  ---
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

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org/message/KOOZSFKJUY3IAH3IJCZIOJNFX5HFC2XG/

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #99 from Fedora Update System  ---
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

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org/message/OAC5T5HTXQIKARNDZPLKE4BQVJXHNAAK/

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #98 from Fedora Update System  ---
kronosnet-1.4-1.el7 has been submitted as an update to Fedora EPEL 7.
https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2018-6280026399

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org/message/BE4GJJ6CPOHHYHX56ON2DUSRK6Q6LBIK/

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #97 from Fedora Update System  ---
kronosnet-1.4-1.fc27 has been submitted as an update to Fedora 27.
https://bodhi.fedoraproject.org/updates/FEDORA-2018-b4cad12543

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org/message/UGJQASCYHLEKLTIJNTP56VLR23ZIHFIX/

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103

Fedora Update System  changed:

   What|Removed |Added

 Status|ON_QA   |CLOSED
 Resolution|--- |ERRATA
Last Closed||2018-03-30 09:13:08



--- Comment #96 from Fedora Update System  ---
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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #95 from Fedora Update System  ---
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

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103

Fedora Update System  changed:

   What|Removed |Added

 Status|MODIFIED|ON_QA



--- Comment #94 from Fedora Update System  ---
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

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #93 from Fedora Update System  ---
kronosnet-1.1-8.el7 has been submitted as an update to Fedora EPEL 7.
https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2018-603c1c93cf

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #92 from Fedora Update System  ---
kronosnet-1.1-8.fc28 has been submitted as an update to Fedora 28.
https://bodhi.fedoraproject.org/updates/FEDORA-2018-94bdc0ccfb

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103

Fedora Update System  changed:

   What|Removed |Added

 Status|POST|MODIFIED



-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #91 from Fedora Update System  ---
kronosnet-1.1-8.fc27 has been submitted as an update to Fedora 27.
https://bodhi.fedoraproject.org/updates/FEDORA-2018-6eae0c4a51

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #90 from Gwyn Ciesla  ---
(fedrepo-req-admin):  The Pagure repository was created at
https://src.fedoraproject.org/rpms/kronosnet

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #89 from digimer  ---
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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #88 from digimer  ---
I've been away for work this last week, but I am returning later today.
Finishing this process is my first ToDo.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #87 from Jan Friesse  ---
@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).

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103

Jan Friesse  changed:

   What|Removed |Added

 Status|ASSIGNED|POST
  Flags|fedora-review?  |fedora-review+



--- Comment #86 from Jan Friesse  ---
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 

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #85 from digimer  ---
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-72018-03-07 01:50:40.831722937 -0500
+++ kronosnet.spec.1.1-82018-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  - 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  - 1.1-7
 - Moved the comment back above '%%files -n libknet1-devel'.
 - Added comment to '%%debug_package'.


-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #84 from Fabio Massimo Di Nitto  ---
(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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #83 from Jan Pokorný  ---
[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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #82 from digimer  ---
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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103

Remi Collet  changed:

   What|Removed |Added

 CC||fed...@famillecollet.com



--- Comment #81 from Remi Collet  ---
(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).

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #80 from digimer  ---
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-62018-03-07 01:19:35.321125418 -0500
+++ kronosnet.spec.1.1-72018-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  - 1.1-7
+- Moved the comment back above '%%files -n libknet1-devel'.
+- Added comment to '%%debug_package'.
+
 * Wed Mar 07 2018 Madison Kelly  - 1.1-6
 - Added a version requirement to lz4 to deal with koji pulling in the
   wrong package.


-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #79 from digimer  ---
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-42018-03-04 02:00:23.290918796 -0500
+++ kronosnet.spec.1.1-62018-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  - 1.1-6
+- Added a version requirement to lz4 to deal with koji pulling in the
+  wrong package.
+
+* Tue Mar 06 2018 Madison Kelly  - 1.1-5
+- Updated ldconfig scriptlet calls.
+- Moved the debug_package leading comment.
+
 * Sun Mar 04 2018 Madison Kelly  - 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  - 1.0-10
-- Added missing change log for 1.0-9.
-
-* Thu Feb 22 2018 Madison Kelly  - 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  - 1.0-8
-- Reverted to 'BuildRequires: bzip2-devel' to fix EPEL7 builds.
-
-* Thu Feb 15 2018 Madison Kelly  - 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  - 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  - 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 

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103

Chris Feist  changed:

   What|Removed |Added

   Assignee|jpoko...@redhat.com |jfrie...@redhat.com



-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #78 from digimer  ---
> 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?

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #77 from Jan Pokorný  ---
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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #76 from Jan Pokorný  ---
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/de...@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.)

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #75 from digimer  ---
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-32018-02-26 10:03:13.304992918 -0500
+++ kronosnet.spec.1.1-42018-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. 

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #74 from Fabio Massimo Di Nitto  ---
(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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #73 from Jan Pokorný  ---
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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #72 from Fabio Massimo Di Nitto  ---
(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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #71 from Jan Pokorný  ---
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...

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #70 from Fabio Massimo Di Nitto  ---
(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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #69 from Jan Pokorný  ---
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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #68 from Fabio Massimo Di Nitto  ---
> 
> 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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #67 from Jan Pokorný  ---
> 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).

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #66 from Fabio Massimo Di Nitto  ---
(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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #65 from Jan Pokorný  ---
[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

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #64 from Fabio Massimo Di Nitto  ---
(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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103

Jan Pokorný  changed:

   What|Removed |Added

  Flags|needinfo?(jpokorny@redhat.c |
   |om) |



--- Comment #63 from Jan Pokorný  ---
> 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], 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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103

Fabio Massimo Di Nitto  changed:

   What|Removed |Added

  Flags||needinfo?(jpokorny@redhat.c
   ||om)



--- Comment #62 from Fabio Massimo Di Nitto  ---
(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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103

Jan Pokorný  changed:

   What|Removed |Added

  Flags|needinfo?(jpokorny@redhat.c |
   |om) |



--- Comment #61 from Jan Pokorný  ---
Created attachment 1402543
  --> https://bugzilla.redhat.com/attachment.cgi?id=1402543=edit
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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #60 from Fabio Massimo Di Nitto  ---

> > 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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103

Fabio Massimo Di Nitto  changed:

   What|Removed |Added

  Flags||needinfo?(jpokorny@redhat.c
   ||om)



--- Comment #59 from Fabio Massimo Di Nitto  ---
(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 

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103

Jan Pokorný  changed:

   What|Removed |Added

  Flags|needinfo?(jpokorny@redhat.c |
   |om) |



--- Comment #58 from Jan Pokorný  ---
... see
https://fedoraproject.org/wiki/Packaging:WeakDependencies

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #57 from Jan Pokorný  ---
>> 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:

> 

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103

Fabio Massimo Di Nitto  changed:

   What|Removed |Added

  Flags||needinfo?(jpokorny@redhat.c
   ||om)



--- Comment #56 from Fabio Massimo Di Nitto  ---
(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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103

Jan Pokorný  changed:

   What|Removed |Added

 Status|POST|ASSIGNED
   Assignee|jfrie...@redhat.com |jpoko...@redhat.com
  Flags|fedora-review+  |fedora-review?



--- Comment #55 from Jan Pokorný  ---
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...")

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #54 from Jan Pokorný  ---
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...")

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103

Jan Friesse  changed:

   What|Removed |Added

 Status|ASSIGNED|POST



-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103

Jan Friesse  changed:

   What|Removed |Added

  Flags|fedora-review?  |fedora-review+



-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #53 from Jan Friesse  ---
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]: 

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103

Fabio Massimo Di Nitto  changed:

   What|Removed |Added

   Assignee|jpoko...@redhat.com |jfrie...@redhat.com



-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #52 from digimer  ---
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-22018-02-25 23:17:33.268040008 -0500
+++ kronosnet.spec.1.1-32018-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  - 1.1-3
+- Fixed the changelog to not have the full macro names.
+
 * Sun Feb 25 2018 Madison Kelly  - 1.1-2
 - Moved the 'BuildRequires: systemd' to be conditional with kronostnetd.

@@ -474,7 +477,7 @@
 * Thu Feb 22 2018 Madison Kelly  - 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  - 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  - 1.0-5
 - All changes related to this spec;


-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #51 from Jan Friesse  ---
@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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #50 from digimer  ---
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-12018-02-25 13:17:20.497971906 -0500
+++ kronosnet.spec.1.1-22018-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  - 1.1-2
+- Moved the 'BuildRequires: systemd' to be conditional with kronostnetd.
+
 * Sun Feb 25 2018 Madison Kelly  - 1.1-1
 - Rerolled for 1.1 upstream release.
 - Removed the (no longer needed) gcc8-fixes.patch


-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #49 from digimer  ---
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-102018-02-23 14:47:17.586627823 -0500
+++ kronosnet.spec.1.1-12018-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  - 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  - 1.0-10
 - Added missing change log for 1.0-9.


-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #48 from digimer  ---
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-92018-02-22 14:34:54.588904329 -0500
+++ kronosnet.spec.1.0-102018-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  - 1.0-10
+- Added missing change log for 1.0-9.
+
+* Thu Feb 22 2018 Madison Kelly  - 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  - 1.0-8
 - Reverted to 'BuildRequires: bzip2-devel' to fix EPEL7 builds.


-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #47 from digimer  ---
That's the second time I missed that... >_<.

I'll roll a final RPM today/tonight with the change log updated.

Madi

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #46 from Jan Friesse  ---
@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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #45 from digimer  ---
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-82018-02-16 00:04:45.452029189 -0500
+++ kronosnet.spec.1.0-92018-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


-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #44 from Jan Friesse  ---
@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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #43 from Jan Friesse  ---
@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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #42 from digimer  ---
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-72018-02-15 19:18:05.854232031 -0500
+++ kronosnet.spec.1.0-82018-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  - 1.0-8
+- Reverted to 'BuildRequires: bzip2-devel' to fix EPEL7 builds.
+
 * Thu Feb 15 2018 Madison Kelly  - 1.0-7
 - Added missing 1.0-6 changelog.
 - Added kronosnetd postun.


-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #41 from digimer  ---
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?

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #40 from digimer  ---
> 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-62018-02-15 00:25:25.797016016 -0500
+++ kronosnet.spec.1.0-72018-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+

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #39 from Jan Friesse  ---
@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 are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #38 from digimer  ---
> - 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-52018-02-12 14:20:52.122179273 -0500
+++ kronosnet.spec.1.0-62018-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.*

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #37 from Jan Friesse  ---
@digimer:
Forgot one more thing. We need more %{_isa}. Just add them basically for every

Requires: libtap1

and

Requires: libknet1

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #36 from Jan Friesse  ---
@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
  ```

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #35 from digimer  ---
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-42018-01-31 23:32:53.661198886 -0500
+++ kronosnet.spec.1.0-52018-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/de...@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 

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #34 from Jan Friesse  ---
@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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103

Jan Friesse  changed:

   What|Removed |Added

 CC||jfrie...@redhat.com



--- Comment #33 from Jan Friesse  ---
@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
 - 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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #32 from digimer  ---
Any comments from upstream on comment #30 or the latest RPMs?

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #31 from digimer  ---
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

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #30 from Jan Pokorný  ---
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

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #29 from digimer  ---
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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #28 from Jan Pokorný  ---
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

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103

Jan Pokorný  changed:

   What|Removed |Added

 Status|NEW |ASSIGNED
   Assignee|nob...@fedoraproject.org|jpoko...@redhat.com



-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #27 from digimer  ---
Upstream is fixing the rawhide issue (gcc8 v gcc7)

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1507103

Fabio Massimo Di Nitto  changed:

   What|Removed |Added

Summary|Review Request: kronosnet - |Review Request: kronosnet -
   |Multipoint-to-Multipoint|Multipoint-to-Multipoint
   |network abstraction layer   |network abstraction layer
   |for High Availability   |for High Availability
   |usecases|applications



-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org