Spec URL: http://pavlix.fedorapeople.org//netresolve.spec SRPM URL: http://pavlix.fedorapeople.org//netresolve-0.0.1-0.1.20140422git.fc21.src.rpm 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.
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: > URL: https://sourceware.org/netresolve/ 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_pkg-config > 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.
Created attachment 889255 [details] changes (In reply to Michael Schwendt from comment #1) > > URL: https://sourceware.org/netresolve/ > > 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=371bf5d950a579625d474c8526e6a4cf3688f73c Will attach new spec and srpm later.
*** Bug 1099435 has been marked as a duplicate of this bug. ***
Spec URL: http://pavlix.fedorapeople.org//netresolve.spec SRPM URL: http://pavlix.fedorapeople.org//netresolve-0.0.1-0.2.20140422git.fc21.src.rpm 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.
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.
(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.
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.
Spec URL: http://pavlix.fedorapeople.org//netresolve.spec SRPM URL: http://pavlix.fedorapeople.org//netresolve-0.0.1-0.3.20140422git.fc21.src.rpm 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.
This package built on koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=6870289
I just had to remove the call to tests. I don't know exactly why the test freezes during the build.
Please update the SRPM with to the latest git HEAD.
Spec URL: http://pavlix.fedorapeople.org//netresolve.spec SRPM URL: http://pavlix.fedorapeople.org//netresolve-0.0.1-0.5.20150923git.fc24.src.rpm 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.
Created attachment 1076550 [details] Build log Mock build fails. Some tests seem to be failing. Please resolve the issue.
I tried also COPR: http://copr-fe.cloud.fedoraproject.org/coprs/thozza/netresolve/build/118060/
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
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.
%define snapshot_suffix .20150923git https://fedoraproject.org/wiki/Packaging:Guidelines#.25global_preferred_over_.25define
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
Spec URL: http://pavlix.fedorapeople.org//netresolve.spec SRPM URL: http://pavlix.fedorapeople.org//netresolve-0.0.1-0.6.20151015git.fc24.src.rpm 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.
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.
(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. > [!]: 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 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.
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
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
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
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
Spec URL: http://pavlix.fedorapeople.org//netresolve.spec SRPM URL: http://pavlix.fedorapeople.org//netresolve-0.0.1-0.7.20151111git.fc24.src.rpm 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.
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
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} ?
Looks good to me, package approved.