https://bugzilla.redhat.com/show_bug.cgi?id=1090499
Bug ID: 1090499 Summary: Review Request: netresolve - Generic name resolution library Product: Fedora Version: rawhide Component: Package Review Assignee: nobody@fedoraproject.org Reporter: psimerda@redhat.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org
Spec URL: http://pavlix.fedorapeople.org//netresolve.spec SRPM URL: http://pavlix.fedorapeople.org//netresolve-0.0.1-0.1.20140422git.fc21.src.rp...
Description: Netresolve is a package for nonblocking network name resolution via backends intended as a replacement for name service switch based name resolution in glibc.
https://bugzilla.redhat.com/show_bug.cgi?id=1090499
--- Comment #1 from Michael Schwendt bugs.michael@gmx.net --- Consider pointing the fedora-review tool at this ticket. Run "fedora-review -b 1090499". It evaluates the "SRPM URL:" and "Spec URL:" lines and performs many helpful checks you ought to be interested in.
A brief look at the package:
Forbidden You don't have permission to access /netresolve/ on this server.
Source0: netresolve-0.0.1.tar.xz
https://fedoraproject.org/wiki/Packaging:SourceURL#Referencing_Source
%package devel Summary: Development files for getdns Group: Development/Libraries
If you set the optional Group tag for this subpackage, why is it missing in the base package? "Group: System Environment/Libraries" https://fedoraproject.org/wiki/Packaging:Guidelines#Group_tag
Requires: %{name} = %{version}-%{release}
https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package
Requires: pkgconfig
There are automatic pkgconfig dependencies for a long time. Query the built packages. You would only need this explicit dep for EL5. But the package does not include any .pc file, so the dependency is superfluous currently.
%post /sbin/ldconfig
%postun /sbin/ldconfig
If you don't to execute anything else, consider executing ldconfig directly instead of running it within a /bin/sh script:
%post -p /sbin/ldconfig
%postun -p /sbin/ldconfig
%doc NEWS COPYING
Why not include README and TODO?
Instead, the NEWS file contents are rather useless so far.
Btw, it declares this as "0.0.1", but if there is a 0.0.1 release, the RPM package ought not apply the pre-release snapshot versioning scheme, but apply the post-release versioning scheme: https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Package_Versioning
PKG_CHECK_MODULES([ARES], [libcares])
https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRequires_based_on_p...
build.log
Output is non-verbose. One cannot see whether Fedora's %optflags are used, for example, and one cannot verify the compiler/preprocessor settings.
Is the included "tests" directory suitable for running it at build-time in the spec %check section?
checking for ARES... yes checking for ub_ctx_create in -llibunbound... no
This check fails, but it linked with libunbound nevertheless. Suspicious.
https://bugzilla.redhat.com/show_bug.cgi?id=1090499
--- Comment #2 from Pavel Šimerda (pavlix) psimerda@redhat.com --- Created attachment 889255 --> https://bugzilla.redhat.com/attachment.cgi?id=889255&action=edit changes
(In reply to Michael Schwendt from comment #1)
Forbidden You don't have permission to access /netresolve/ on this server.
Yes, the upstream website hasn't been launched yet but I received numerous requests to come up with a Fedora package.
Source0: netresolve-0.0.1.tar.xz
https://fedoraproject.org/wiki/Packaging:SourceURL#Referencing_Source
It is a git snapshot and the sourceware git doesn't offer tarballs. I already contacted sourceware maintainers about it.
%package devel Summary: Development files for getdns Group: Development/Libraries
If you set the optional Group tag for this subpackage, why is it missing in the base package? "Group: System Environment/Libraries" https://fedoraproject.org/wiki/Packaging:Guidelines#Group_tag
Added.
Requires: %{name} = %{version}-%{release}
https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package
Fixed.
Requires: pkgconfig
There are automatic pkgconfig dependencies for a long time. Query the built packages. You would only need this explicit dep for EL5. But the package does not include any .pc file, so the dependency is superfluous currently.
Removed.
%post /sbin/ldconfig
%postun /sbin/ldconfig
If you don't to execute anything else, consider executing ldconfig directly instead of running it within a /bin/sh script:
%post -p /sbin/ldconfig
%postun -p /sbin/ldconfig
Done.
%doc NEWS COPYING
Why not include README and TODO?
Added.
Instead, the NEWS file contents are rather useless so far.
Let's get ready for the releases.
Btw, it declares this as "0.0.1", but if there is a 0.0.1 release, the RPM package ought not apply the pre-release snapshot versioning scheme, but apply the post-release versioning scheme:
This is a 0.0.1 pre-release package, no release exists, yet.
https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Package_Versioning
PKG_CHECK_MODULES([ARES], [libcares])
https://fedoraproject.org/wiki/Packaging: Guidelines#BuildRequires_based_on_pkg-config
Fixed.
build.log
Output is non-verbose. One cannot see whether Fedora's %optflags are used, for example, and one cannot verify the compiler/preprocessor settings.
Fixed.
Is the included "tests" directory suitable for running it at build-time in the spec %check section?
Definitely yes.
checking for ARES... yes checking for ub_ctx_create in -llibunbound... no
This check fails, but it linked with libunbound nevertheless. Suspicious.
Fixed upstream.
https://sourceware.org/git/?p=netresolve.git;a=commitdiff;h=371bf5d950a57962...
Will attach new spec and srpm later.
https://bugzilla.redhat.com/show_bug.cgi?id=1090499
Christopher Meng cickumqt@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |cickumqt@gmail.com Assignee|nobody@fedoraproject.org |cickumqt@gmail.com Flags| |fedora-review?
https://bugzilla.redhat.com/show_bug.cgi?id=1090499
--- Comment #3 from Pavel Šimerda (pavlix) psimerda@redhat.com --- *** Bug 1099435 has been marked as a duplicate of this bug. ***
https://bugzilla.redhat.com/show_bug.cgi?id=1090499
--- Comment #4 from Pavel Šimerda (pavlix) psimerda@redhat.com ---
Spec URL: http://pavlix.fedorapeople.org//netresolve.spec SRPM URL: http://pavlix.fedorapeople.org//netresolve-0.0.1-0.2.20140422git.fc21.src.rp...
Description: Netresolve is a package for nonblocking network name resolution via backends intended as a replacement for name service switch based name resolution in glibc.
https://bugzilla.redhat.com/show_bug.cgi?id=1090499
Christopher Meng i@cicku.me changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |NEW CC|i@cicku.me | Assignee|i@cicku.me |nobody@fedoraproject.org
--- Comment #5 from Christopher Meng i@cicku.me --- Sorry, after running the building process for almost 4 hrs it still couldn't get terminated, so I can't review this package on my computer. Please find other reviewers to help you step forward.
https://bugzilla.redhat.com/show_bug.cgi?id=1090499
--- Comment #6 from Pavel Šimerda (pavlix) psimerda@redhat.com --- (In reply to Christopher Meng from comment #5)
Sorry, after running the building process for almost 4 hrs it still couldn't get terminated, so I can't review this package on my computer. Please find other reviewers to help you step forward.
No problem. I'm just curious about the build process as the package is very small with few dependencies, so this is the last package I would expect to take a long compilation time.
https://bugzilla.redhat.com/show_bug.cgi?id=1090499
--- Comment #7 from Christopher Meng i@cicku.me --- Yes it's a bug IMO. I haven't saved the build log so I don't know what happened still, but if you want me to help I can run the build process again.
https://bugzilla.redhat.com/show_bug.cgi?id=1090499
--- Comment #8 from Pavel Šimerda (pavlix) psimerda@redhat.com ---
Spec URL: http://pavlix.fedorapeople.org//netresolve.spec SRPM URL: http://pavlix.fedorapeople.org//netresolve-0.0.1-0.3.20140422git.fc21.src.rp...
Description: Netresolve is a package for nonblocking network name resolution via backends intended as a replacement for name service switch based name resolution in glibc.
https://bugzilla.redhat.com/show_bug.cgi?id=1090499
--- Comment #9 from Pavel Šimerda (pavlix) psimerda@redhat.com --- This package built on koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=6870289
https://bugzilla.redhat.com/show_bug.cgi?id=1090499
--- Comment #10 from Pavel Šimerda (pavlix) psimerda@redhat.com --- I just had to remove the call to tests. I don't know exactly why the test freezes during the build.
https://bugzilla.redhat.com/show_bug.cgi?id=1090499
Tomas Hozza thozza@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |thozza@redhat.com Assignee|nobody@fedoraproject.org |thozza@redhat.com
--- Comment #11 from Tomas Hozza thozza@redhat.com --- Please update the SRPM with to the latest git HEAD.
https://bugzilla.redhat.com/show_bug.cgi?id=1090499
--- Comment #12 from Pavel Šimerda (pavlix) psimerda@redhat.com ---
Spec URL: http://pavlix.fedorapeople.org//netresolve.spec SRPM URL: http://pavlix.fedorapeople.org//netresolve-0.0.1-0.5.20150923git.fc24.src.rp...
Description: Netresolve is a package for non-blocking network name resolution via backends intended as a replacement for name service switch based name resolution in glibc as well as a testbed for future glibc improvements.
https://bugzilla.redhat.com/show_bug.cgi?id=1090499
Pavel Šimerda (pavlix) psimerda@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |883152 (dualstack)
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=883152 [Bug 883152] IPv6 and dual-stack networking sanity tracker
https://bugzilla.redhat.com/show_bug.cgi?id=1090499
--- Comment #13 from Tomas Hozza thozza@redhat.com --- Created attachment 1076550 --> https://bugzilla.redhat.com/attachment.cgi?id=1076550&action=edit Build log
Mock build fails. Some tests seem to be failing. Please resolve the issue.
https://bugzilla.redhat.com/show_bug.cgi?id=1090499
Tomas Hozza thozza@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |psimerda@redhat.com Flags| |needinfo?(psimerda@redhat.c | |om)
--- Comment #14 from Tomas Hozza thozza@redhat.com --- I tried also COPR: http://copr-fe.cloud.fedoraproject.org/coprs/thozza/netresolve/build/118060/
https://bugzilla.redhat.com/show_bug.cgi?id=1090499
--- Comment #15 from Upstream Release Monitoring upstream-release-monitoring@fedoraproject.org --- pavlix's scratch build of netresolve-0.0.1-0.5.20150930git.fc24.src.rpm for rawhide completed http://koji.fedoraproject.org/koji/taskinfo?taskID=11278160
https://bugzilla.redhat.com/show_bug.cgi?id=1090499
Pavel Šimerda (pavlix) psimerda@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(psimerda@redhat.c | |om) |
--- Comment #16 from Pavel Šimerda (pavlix) psimerda@redhat.com --- For completeness, my rawhide build ran without problem including the tests.
(In reply to Tomas Hozza from comment #14)
I tried also COPR: http://copr-fe.cloud.fedoraproject.org/coprs/thozza/netresolve/build/118060/
The coper build shows that the tests are sensitive to the existence of 'localhost4' in '/etc/hosts'. I will fix that.
(In reply to Tomas Hozza from comment #13)
Created attachment 1076550 [details] Build log
Mock build fails. Some tests seem to be failing. Please resolve the issue.
From the build log:
+ diff -u /dev/fd/63 ./tests/data/localhost ++ libtool execute valgrind --leak-check=full --error-exitcode=1 ./netresolve --backends hosts --node localhost
--- /dev/fd/63 2015-09-24 15:09:48.096146394 +0200 +++ ./tests/data/localhost 2015-09-17 22:47:15.000000000 +0200 @@ -1,5 +1,6 @@ response netresolve 0.0.1 name localhost +ip ::1 any any 0 0 0 0 ip 127.0.0.1 any any 0 0 0 0 secure
The diff above shows that '/etc/hosts' lacks IPv6 loopback address for 'localhost' name. That is IMO a bug in the build environment rather then the test. Some of the tests depend on the build environment but in most cases this is intentional. The objective is to also test that the system configuration allows netresolve to provide correct results.
https://bugzilla.redhat.com/show_bug.cgi?id=1090499
Jiri Popelka jpopelka@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |jpopelka@redhat.com
--- Comment #17 from Jiri Popelka jpopelka@redhat.com --- %define snapshot_suffix .20150923git
https://fedoraproject.org/wiki/Packaging:Guidelines#.25global_preferred_over...
https://bugzilla.redhat.com/show_bug.cgi?id=1090499
--- Comment #18 from Upstream Release Monitoring upstream-release-monitoring@fedoraproject.org --- pavlix's scratch build of netresolve-0.0.1-0.5.20150930git.fc24.src.rpm for rawhide completed http://koji.fedoraproject.org/koji/taskinfo?taskID=11412615
https://bugzilla.redhat.com/show_bug.cgi?id=1090499
--- Comment #19 from Pavel Šimerda (pavlix) psimerda@redhat.com ---
Spec URL: http://pavlix.fedorapeople.org//netresolve.spec SRPM URL: http://pavlix.fedorapeople.org//netresolve-0.0.1-0.6.20151015git.fc24.src.rp...
Description: Netresolve is a package for non-blocking network name resolution via backends intended as a replacement for name service switch based name resolution in glibc as well as a testbed for future glibc improvements.
https://bugzilla.redhat.com/show_bug.cgi?id=1090499
--- Comment #20 from Jiri Popelka jpopelka@redhat.com --- Package Review ==============
Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
Issues: ======= [!]: ldconfig called in %post and %postun if required. Note: /sbin/ldconfig not called in netresolve-core, netresolve-compat, netresolve-backends-compat, netresolve-backends-aresdns, netresolve- backends-avahi, netresolve-backends-ubdns See: http://fedoraproject.org/wiki/Packaging/Guidelines#Shared_Libraries
[!]: 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. Note: License file COPYING is marked as %doc instead of %license See: http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text
[!]: License field in the package spec file matches the actual license. Note: baskends/asyncns.c is LGPLv2+ licensed
[!]: License file installed when any subpackage combination is installed. Note: you need %license COPYING in each %files section (except the main package and -tools/-compat subpackages AFAICT)
[!]: Fully versioned dependency in subpackages if applicable. Note: It'd better to replace for example - Requires: netresolve-core + Requires: netresolve-core%{?_isa} = %{version}-%{release} (the same for all subpackages)
===== MUST items =====
C/C++: [x]: Package does not contain kernel modules. [x]: Package contains no static executables. [x]: Header files in -devel subpackage, if present. [x]: Package does not contain any libtool archives (.la) [x]: Rpath absent or only used for internal libs. [x]: Development (unversioned) .so files in -devel subpackage, if present.
Generic: [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [!]: License field in the package spec file matches the actual license. baskends/asyncns.c is LGPLv2+ licensed [!]: License file installed when any subpackage combination is installed. you need %license COPYING in each %files section (except the main package and -tools/-compat subpackages AFAICT) [x]: %build honors applicable compiler flags or justifies otherwise. [x]: All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [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. [x]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [x]: Useful -debuginfo package or justification otherwise. [x]: Package is not known to require an ExcludeArch tag. [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. [x]: Package requires other packages for directories it uses. [x]: Package must own all directories that it creates. [x]: Package does not own files or directories owned by other packages. [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]: Packages must not store files under /srv, /opt or /usr/local
===== SHOULD items =====
Generic: [x]: Reviewer should test that the package builds in mock. [x]: Final provides and requires are sane. [!]: Fully versioned dependency in subpackages if applicable. It'd better to replace for example - Requires: netresolve-core + Requires: netresolve-core%{?_isa} = %{version}-%{release} (for all subpackages) [x]: Latest version is packaged. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [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]: Uses parallel make %{?_smp_mflags} macro. [x]: Spec use %global instead of %define unless justified.
https://bugzilla.redhat.com/show_bug.cgi?id=1090499
--- Comment #21 from Pavel Šimerda (pavlix) psimerda@redhat.com --- (In reply to Jiri Popelka from comment #20)
[!]: ldconfig called in %post and %postun if required. Note: /sbin/ldconfig not called in netresolve-core, netresolve-compat, netresolve-backends-compat, netresolve-backends-aresdns, netresolve- backends-avahi, netresolve-backends-ubdns See: http://fedoraproject.org/wiki/Packaging/Guidelines#Shared_Libraries
OK.
in its own file, then that file, containing the text of the license(s) for the package is included in %license. Note: License file COPYING is marked as %doc instead of %license See: http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text
OK.
[!]: License field in the package spec file matches the actual license. Note: baskends/asyncns.c is LGPLv2+ licensed
TODO: Will fix upstream and import the new changes.
[!]: License file installed when any subpackage combination is installed. Note: you need %license COPYING in each %files section (except the main package and -tools/-compat subpackages AFAICT)
TODO: Will reconsider the dependencies and fix it.
[!]: Fully versioned dependency in subpackages if applicable. Note: It'd better to replace for example - Requires: netresolve-core + Requires: netresolve-core%{?_isa} = %{version}-%{release} (the same for all subpackages)
OK.
https://bugzilla.redhat.com/show_bug.cgi?id=1090499
--- Comment #22 from Upstream Release Monitoring upstream-release-monitoring@fedoraproject.org --- pavlix's scratch build of netresolve-0.0.1-0.6.20151015git.fc24.src.rpm for rawhide failed http://koji.fedoraproject.org/koji/taskinfo?taskID=11725080
https://bugzilla.redhat.com/show_bug.cgi?id=1090499
--- Comment #23 from Upstream Release Monitoring upstream-release-monitoring@fedoraproject.org --- pavlix's scratch build of netresolve-0.0.1-0.6.20151015git.fc24.src.rpm for rawhide failed http://koji.fedoraproject.org/koji/taskinfo?taskID=11728303
https://bugzilla.redhat.com/show_bug.cgi?id=1090499
--- Comment #24 from Upstream Release Monitoring upstream-release-monitoring@fedoraproject.org --- pavlix's scratch build of netresolve-0.0.1-0.6.20151015git.fc24.src.rpm for rawhide failed http://koji.fedoraproject.org/koji/taskinfo?taskID=11753550
https://bugzilla.redhat.com/show_bug.cgi?id=1090499
--- Comment #25 from Upstream Release Monitoring upstream-release-monitoring@fedoraproject.org --- pavlix's scratch build of netresolve-0.0.1-0.6.20151015git.fc24.src.rpm for rawhide completed http://koji.fedoraproject.org/koji/taskinfo?taskID=11754048
https://bugzilla.redhat.com/show_bug.cgi?id=1090499
--- Comment #26 from Pavel Šimerda (pavlix) psimerda@redhat.com ---
Spec URL: http://pavlix.fedorapeople.org//netresolve.spec SRPM URL: http://pavlix.fedorapeople.org//netresolve-0.0.1-0.7.20151111git.fc24.src.rp...
Description: Netresolve is a package for non-blocking network name resolution via backends intended as a replacement for name service switch based name resolution in glibc as well as a testbed for future glibc improvements.
https://bugzilla.redhat.com/show_bug.cgi?id=1090499
--- Comment #27 from Upstream Release Monitoring upstream-release-monitoring@fedoraproject.org --- pavlix's scratch build of netresolve-0.0.1-0.7.20151111git.fc24.src.rpm for rawhide completed http://koji.fedoraproject.org/koji/taskinfo?taskID=11792116
https://bugzilla.redhat.com/show_bug.cgi?id=1090499
--- Comment #28 from Jiri Popelka jpopelka@redhat.com --- Ok, all my objections seem to be resolved now.
But I have a new one ;-) The dependencies doesn't look correct to me. The main package require all subpackages and each subpackage requires main package.
I guess you wanted to use Requires: %{name}-core%{?_isa} = %{version}-%{release} instead of Requires: %{name}%{?_isa} = %{version}-%{release} ?
https://bugzilla.redhat.com/show_bug.cgi?id=1090499
Jiri Popelka jpopelka@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|thozza@redhat.com |jpopelka@redhat.com
https://bugzilla.redhat.com/show_bug.cgi?id=1090499
--- Comment #29 from Pavel Šimerda (pavlix) psimerda@redhat.com ---
Spec URL: http://pavlix.fedorapeople.org//netresolve.spec SRPM URL: http://pavlix.fedorapeople.org//netresolve-0.0.1-0.7.20151111git.fc24.src.rp...
Description: Netresolve is a package for non-blocking network name resolution via backends intended as a replacement for name service switch based name resolution in glibc as well as a testbed for future glibc improvements.
https://bugzilla.redhat.com/show_bug.cgi?id=1090499
Jiri Popelka jpopelka@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+
--- Comment #30 from Jiri Popelka jpopelka@redhat.com --- Looks good to me, package approved.
https://bugzilla.redhat.com/show_bug.cgi?id=1090499
Pavel Šimerda (pavlix) psimerda@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |CLOSED Resolution|--- |RAWHIDE Last Closed| |2016-01-05 06:42:21
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla