Bug 235925 - (dnssec-tools) Review Request: dnssec-tools - Is a tool set for use with signed DNS zones
Review Request: dnssec-tools - Is a tool set for use with signed DNS zones
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Paul Wouters
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-04-10 16:24 EDT by Wes Hardaker
Modified: 2015-04-20 11:46 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-08-09 11:00:14 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
pwouters: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Wes Hardaker 2007-04-10 16:24:06 EDT
Spec URL: http://www.hardakers.net/dnssec-tools/dnssec-tools.spec
SRPM URL: http://www.hardakers.net/dnssec-tools/dnssec-tools-1.1.1-1.src.rpm
Description: The goal of the DNSSEC-Tools project is to create a set of tools, patches, applications, wrappers, extensions, and plugins that will help ease the deployment of DNSSEC-related technologies.
Comment 1 Wes Hardaker 2007-04-10 16:34:38 EDT
This is a first-time-package and I need a sponsor as well.
Comment 2 Paul Wouters 2007-04-18 17:22:29 EDT
I haven't taken on a person for sponsorship yet. I'll talk to Spot about it. But
in any case, here is the review. Please let me know when all issues have been
fixed and I'll do a new review.

It fails to compile on 64bit:

RPM build errors:
    File not found:
/var/tmp/dnssec-tools-1.1.1-1-root-root/usr/lib64/debug/usr/bin/validate.debug
    File not found:
/var/tmp/dnssec-tools-1.1.1-1-root-root/usr/lib64/debug/usr/lib64/libsres.so.3.0.0.debug
    File not found:
/var/tmp/dnssec-tools-1.1.1-1-root-root/usr/lib64/debug/usr/lib64/libval-threads.so.3.0.0.debug

These seem to get mistakenly installed in
/var/tmp/dnssec-tools-1.1.1-1-root-root/usr/lib/debug/usr/lib64/ instead

I am not sure why you are defining the debug package, since they are built
automatically. When your debug package "cut" from the spec file, at the end I get:
Wrote: /usr/src/redhat/SRPMS/dnssec-tools-1.1.1-1.src.rpm
Wrote: /usr/src/redhat/RPMS/x86_64/dnssec-tools-1.1.1-1.x86_64.rpm
Wrote: /usr/src/redhat/RPMS/x86_64/dnssec-tools-perlmods-1.1.1-1.x86_64.rpm
Wrote: /usr/src/redhat/RPMS/x86_64/dnssec-tools-libs-1.1.1-1.x86_64.rpm
Wrote: /usr/src/redhat/RPMS/x86_64/dnssec-tools-libs-devel-1.1.1-1.x86_64.rpm
Wrote: /usr/src/redhat/RPMS/x86_64/dnssec-tools-debuginfo-1.1.1-1.x86_64.rpm

Second:

        perl(Net::DNS::SEC) is needed by dnssec-tools-1.1.1-1.x86_64

It depends ona non-existing package, so this package would have to be added
first before Fedora can accept this package

Apart from that:

FIXME: rpmlint warnings:
 W: dnssec-tools incoherent-version-in-changelog 1.1-2 1.1.1-1
 W: dnssec-tools-libs no-documentation
OK: Package Naming Guidelines followed
OK: spec file base name matches base package
OK: Packaging Guidelines followed (isn't this a recursive loop anyways :)
OK: Package complies to an opensource licence
FIX: License states "BSD" but it also includes a requirement to include a
     disclaimer for the vendor SPARTA. So license should state "BSD-like"
OK: %doc includes license file
OK: package is in american english
OK: spec file is readable.
FIX: source field using local reference instead of full URL, eg:
     Source:
http://downloads.sourceforge.net/sourceforge/%{name}/%{name}-%{version}.tar.gz
     See:
http://fedoraproject.org/wiki/PackagingDrafts/SourceUrl#head-27442167fe28eb345470e8db56716d62b508978c

OK: md5sum of source matches that of upstream
OK: package compiles into binary rpms (but see note at start or review)
OK: all buildrequires listed
OK: no bad locale handling
FIX: calling ldconfig in %post/%postun in subpackage that installs lib files
     does not call %post/%postun for sub package libs-devel, which installs
     files in _libdir. in other words, add:
%post libs-devel -p /sbin/ldconfig

%postun libs-devel -p /sbin/ldconfig
OK: no relocatable

FIX: package creates directories it does not own: /usr/include/validator
     add it to the %files section
     Same for /usr/share/dnssec-tools and /etc/dnssec and the perl dirs
OK: no duplicate entries in %files
OK: every subpackage/package contains %defattr
OK: package contains %clean section and rm's buildroot
OK: macros used consistently
OK: contains contain code, or permissable content.
OK: no large documentation files in package
OK: %doc items contain no running dependancies
OK: includes files are in devel package only
OK: no static libs
OK: no .pc files without pkconfig
OK: .so$ files in devel package
OK: devel package requires package
   (though you could rewrite it to: Requires: %{name} = %{version}-%{release} 
    instead of using hardcoded name)
OK: *.la files removed correctly
OK: no .desktop file requires (no GUI is build)
OK: package owns no dir other package owns already
OK: install calls rm -rf buildroot
OK: filenames are valid UTF-8
OK: scriptlets are sane
OK: basic functinality tested and works

Comment 3 Wes Hardaker 2007-04-18 20:11:21 EDT
Are you by chance using the SRPM available from the dnssec-tools website instead
of the one in the comment field above from my submission?  Because the one above
does *not* build it's own debug package and is much cleaner than the one on the
dnssec-tools website (eventually they should become the same, but they're not at
the moment).  I suspect you've actually grabbed the wrong one?  Your other
warnings also look like they're from the SRPM on the main dnssec-tools website
as they've been corrected in the one at hardakers.net as well.
Comment 4 Wes Hardaker 2007-04-18 20:14:00 EDT
Oh, one more comment: I'm not sure whether the dnssec-tools package has been
fully tested on 64bit architectures.  I believe it should be ok as that was
taken into consideration during development, but...

After installing, run "validate -s" to run a bunch of self-tests and see if they
succeeds.
Comment 5 Wes Hardaker 2007-04-18 20:39:48 EDT
New versions of the SRPM and spec file to fix some of the bugs that were not
fixed by the original:

Spec URL: http://www.hardakers.net/dnssec-tools/dnssec-tools.spec
SRPM URL: http://www.hardakers.net/dnssec-tools/dnssec-tools-1.1.1-2.src.rpm

* Tue Apr 10 2007  Wes Hardaker <wjhns174@hardakers.net> - 1.1.1-2
- Pointed Source0 at the sourceforge server instead of a local file
- Set License to BSD-like
- Took ownership of %{_includedir}/validator
Comment 6 Wes Hardaker 2007-04-18 23:41:41 EDT
Spec URL: http://www.hardakers.net/dnssec-tools/dnssec-tools.spec
SRPM URL: http://www.hardakers.net/dnssec-tools/dnssec-tools-1.1.1-3.src.rpm

* Wed Apr 18 2007  Wes Hardaker <wjhns174@hardakers.net> - 1.1.1-3
- Add patch to make Net::DNS::SEC optional
- fix date in previous log

This makes the Net::DNS::SEC module optional (needed only for certain
functions).  Ideally I would like to see that part of FE as well and I'll work
on making that spec as well but it starts a long dependency chain of missing FE
components (Crypto::OpenSSL::* for one).
Comment 7 Paul Wouters 2007-04-19 00:04:39 EDT
I'll check this latest rpm.

Yes, I know the problem of NET::DNS::SEC. That's why I haven't done it either,
though I'm requiring it as well. Maybe we should divide the load and tackle them
together?
Comment 8 Paul Wouters 2007-04-19 01:54:14 EDT
rpmlint found a few things. I still need to go over things manually....

W: dnssec-tools macro-in-%changelog _includedir
W: dnssec-tools-libs no-documentation
Comment 9 Wes Hardaker 2007-04-19 02:34:21 EDT
I'll take the macro out of the changelog (it was just documenting something)

The no-documentation error can't go away because there is none!
Comment 10 Wes Hardaker 2007-04-19 03:07:21 EDT
* Wed Apr 18 2007  Wes Hardaker <wjhns174@hardakers.net> - 1.1.1-4
- Fix changelog so it doesn't have a macro in the documentation
- Added a dnsval.conf starting file.
- Remove include subdir wildcard expansion since the entire directory
  is owned.

Spec URL: http://www.hardakers.net/dnssec-tools/dnssec-tools.spec
SRPM URL: http://www.hardakers.net/dnssec-tools/dnssec-tools-1.1.1-4.src.rpm
Comment 11 Wes Hardaker 2007-05-22 19:26:04 EDT
Spec URL: http://www.hardakers.net/dnssec-tools/dnssec-tools.spec
SRPM URL: http://www.hardakers.net/dnssec-tools/dnssec-tools-1.2-1.src.rpm

This updates the code to the parent released 1.2 which fixes a number of minor
issues here too.
Comment 12 Wes Hardaker 2007-05-24 15:04:40 EDT
Spec URL: http://www.hardakers.net/FE/dnssec-tools.spec
SRPM URL: http://www.hardakers.net/FE/dnssec-tools-1.2-1.src.rpm

fix url typos.
Comment 13 Paul Wouters 2007-05-24 16:13:52 EDT
Three issues left:

1) There seems to be a missing DESTDIR or buildroot argument, as mock building
reveals attempts to install in the real /

make[1]: Entering directory `/builddir/build/BUILD/dnssec-tools-1.2/validator'
creating directory /etc/dnssec-tools/
mkdir /etc/dnssec-tools
mkdir: cannot create directory `/etc/dnssec-tools': Permission denied
creating directory /usr/include/validator/
mkdir /usr/include/validator
mkdir: cannot create directory `/usr/include/validator': Permission denied
/usr/bin/install -c libval-config /usr/bin
/usr/bin/install: cannot create regular file `/usr/bin/libval-config':
Permission denied
make[1]: *** [localinstall] Error 1
make[1]: Leaving directory `/builddir/build/BUILD/dnssec-tools-1.2/validator'
make: *** [subdirinstall] Error 1
error: Bad exit status from /var/tmp/rpm-tmp.88153 (%install)


RPM build errors:
    Bad exit status from /var/tmp/rpm-tmp.88153 (%install)

Error building package from dnssec-tools-1.2-1.fc6.src.rpm, See build log
ending
done

2) add OPTIMIZE="$RPM_OPT_FLAGS" to the --with-perl-build-args

3) Some cosmetics:
-find $RPM_BUILD_ROOT -type d -depth -exec rmdir {} 2>/dev/null ';'
+find $RPM_BUILD_ROOT -depth -type d -exec rmdir {} 2>/dev/null ';'

Other then that, rpmlint is quiet (apart from dnssec-libs not having a %doc
section, which is okay). Let me know when I can try an updated package again.
Comment 14 Wes Hardaker 2007-05-31 19:48:53 EDT
Spec URL: http://www.hardakers.net/FE/dnssec-tools.spec
SRPM URL: http://www.hardakers.net/FE/dnssec-tools-1.2-2.src.rpm

All 3 issues should be fixed!


Comment 15 Wes Hardaker 2007-07-11 17:47:02 EDT
Spec URL: http://www.hardakers.net/FE/dnssec-tools.spec
SRPM URL: http://www.hardakers.net/FE/dnssec-tools-1.2-3.src.rpm

A couple more clean-ups for newer pecl buildrequires needed...
Comment 16 Wes Hardaker 2007-07-11 18:24:46 EDT
setting to + as agreed above and over jabber.
Comment 17 Wes Hardaker 2007-07-11 18:25:27 EDT
ack.  wrong package.  setting back to ?
Comment 18 Wes Hardaker 2007-07-12 16:08:49 EDT
Spec URL: http://www.hardakers.net/FE/dnssec-tools.spec
SRPM URL: http://www.hardakers.net/FE/dnssec-tools-1.2-4.src.rpm

Recent changes in the perl-Net-DNS module caused some problems...  This new src
rpm contains patches to those.

* Thu Jul 12 2007 Wes Hardaker <wjhns174@hardakers.net> - 1.2-4
- patch to fix a donuts rule for newer perl-Net::DNS update
- patch for maketestzone to work around a bug in Net::DNS::RR::DS
Comment 19 Paul Wouters 2007-08-06 23:21:27 EDT
OK rpmlint warnings fixed
OK license updated
OK includes directories it creates (/usr/include/validator)
OK missing ldconfig calls added
OK source url fixed

OK builds in mock
OK rpmlint only gives one warning which may be ignored
   (dnssec-tools-libs no-documentation)

When trying to install, I still seem to be missing two dependancies (at least in
F7):

perl(Net::DNS::SEC)
perl(Mail::Send)

I do find perl-Net-DNS-SEC and perl-Mail-Sender in FC6, but not in F7.

Did you forget to request branches and/or fire of a few build requests?

Once these dependancies exist in F7, the package is approved. 
Comment 20 Wes Hardaker 2007-08-08 12:36:23 EDT
perl(Net::DNS::SEC) is satisfied by an rpm currently in testing (will be pushed
final in a week or so)

perl(Mail::Send) is provided by perl-MailTools so it already exists in the
packaging system.
Comment 21 Wes Hardaker 2007-08-08 12:41:43 EDT
New Package CVS Request
=======================
Package Name: dnssec-tools
Short Description: Tools for manipulating and maintaining DNSSEC zones
Owners: hardaker
Branches: FC-6 F-7
InitialCC: hardaker
Comment 22 Kevin Fenzi 2007-08-08 16:58:44 EDT
cvs done.
Comment 23 Wes Hardaker 2007-08-09 11:00:14 EDT
checked in.  thanks!

(make -j4 turned out to be a problem, so there is a new patch in the repo for that)
Comment 24 Wes Hardaker 2010-07-05 09:12:16 EDT
Package Change Request
======================
Package Name: dnssec-tools
New Branches: EL-5 EL-6
Owners: hardaker

just adding branches
Comment 25 Kevin Fenzi 2010-07-07 21:11:39 EDT
CVS done (by process-cvs-requests.py).
Comment 26 Petr Pisar 2011-11-16 12:08:23 EST
dnssec-tools-1.11-1.fc17 contains file validator/libsres/base64.c which starts with:

/*
 * Copyright (c) 2004 by Internet Systems Consortium, Inc. ("ISC")
 * Copyright (c) 1996-1999 by Internet Software Consortium.
 *
 * Permission to use, copy, modify, and distribute this software for any
 * purpose with or without fee is hereby granted, provided that the above
 * copyright notice and this permission notice appear in all copies.
 *
 * THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES
 * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
 * MERCHANTABILITY AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR
 * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
 * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
 * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT
 * OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 */

/*
 * Portions Copyright (c) 1995 by International Business Machines, Inc.
 *
 * International Business Machines, Inc. (hereinafter called IBM) grants
 * permission under its copyrights to use, copy, modify, and distribute this
 * Software with or without fee, provided that the above copyright notice and
 * all paragraphs of this notice appear in all copies, and that the name of IBM
 * not be used in connection with the marketing of any product incorporating
 * the Software or modifications thereof, without specific, written prior
 * permission.
 *
 * To the extent it has a right to do so, IBM grants an immunity from suit
 * under its patents, if any, for the use, sale or manufacture of products to
 * the extent that such products are used for performing Domain Name System
 * dynamic updates in TCP/IP networks by means of the Software.  No immunity is
 * granted for any product per se or for any other function of any product.
 *
 * THE SOFTWARE IS PROVIDED "AS IS", AND IBM DISCLAIMS ALL WARRANTIES,
 * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
 * PARTICULAR PURPOSE.  IN NO EVENT SHALL IBM BE LIABLE FOR ANY SPECIAL,
 * DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER ARISING
 * OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE, EVEN
 * IF IBM IS APPRISED OF THE POSSIBILITY OF SUCH DAMAGES.
 */

The first part is ICS license which is O.k. for Fedora, the second is some amendment by IBM which Fedora-suitability is not known to me.

I think these two licenses should be put into package meta-data. (Current is License: BSD.)
Comment 27 Wes Hardaker 2015-04-20 10:15:39 EDT
Package Change Request
======================
Package Name: dnssec-tools
New Branches: el7
Owners: hardaker
Comment 28 Jon Ciesla 2015-04-20 11:46:08 EDT
Git done (by process-git-requests).

Note You need to log in before you can comment on or make changes to this bug.