This service will be undergoing maintenance at 00:00 UTC, 2017-10-23 It is expected to last about 30 minutes
Bug 473367 - Review Request: autotrust - DNSKEY trust anchor update utility that uses RFC-5011
Review Request: autotrust - DNSKEY trust anchor update utility that uses RFC-...
Status: CLOSED CURRENTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Adam Tkac
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-11-27 21:20 EST by Paul Wouters
Modified: 2013-04-30 19:42 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-03-04 18:40:36 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
atkac: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Paul Wouters 2008-11-27 21:20:33 EST
Spec URL: ftp://ftp.xelerance.com/autotrust/autotrust.spec
SRPM URL: ftp://ftp.xelerance.com/autotrust/autotrust-0.2.0-1.fc10.src.rpm
Description: autotrust takes care of keeping your DNSSEC trust anchors up to date. It
is RFC5011 compliant, except for the query intervals and the retry timers.
autotrust is meant to run from commandline or in a cron job. If we want to
follow the values recommended by RFC5011, autotrust should run as daemon.
Comment 1 Adam Tkac 2008-11-28 06:06:11 EST
I'm going to review this package
Comment 2 Paul Wouters 2008-11-28 17:20:24 EST
Thanks Adam!

I fixed a typo in the config file.

Spec URL: ftp://ftp.xelerance.com/autotrust/autotrust.spec
SRPM URL: ftp://ftp.xelerance.com/autotrust/autotrust-0.2.0-2.fc10.src.rpm
Comment 3 Adam Tkac 2008-12-01 09:12:47 EST
- specfile is properly named, is cleanly written and uses macros consistently: NO
Distribution configure flags --sysconfdir and --libdir are overriden. It doesn't
look right for me.

What is the reason? Why you can't put configuration file directly into /etc?

Libdir flag is unusable at all, package contains no libraries.

--------------------------
- package builds in mock (Rawhide/x86_64): NO
Source is distributed with config.{guess,sub} as symlinks. It is pretty bad
because rawhide has new libtool and those symlinks are no longer valid. I recommend to add config.{guess,sub} to package and use it and run "autoreconf -fiv" before build to get ltmain.sh fixed

--------------------------
- rpmlint is silent: NO
$ rpmlint autotrust-0.2.0-2.fc10.src.rpm
autotrust.src: W: strange-permission autotrust.cron 0755
1 packages and 0 specfiles checked; 0 errors, 1 warnings.

I recommend to use 644 and then install autotrust.cron with 755 perms.

--------------------------
- final provides and requires look sane: NO
I think package should Requires: anacron
Comment 4 Paul Wouters 2008-12-02 18:38:43 EST
* Tue Dec  2 2008 Paul Wouters <paul@xelerance.com> - 0.2.0-3
- Use upstream svn patch for multiple resolvers and defaults
- Don't override unused libdir
- Cleanup and run autoreconf
- Added missing dependancies (anacron, automake, libtool)
- Fix permission on cron job.


I Added a require for anacron (but is that right? shouldnt it be any cron daemon?)
 
Spec URL: ftp://ftp.xelerance.com/autotrust/autotrust.spec
SRPM URL: ftp://ftp.xelerance.com/autotrust/autotrust-0.2.0-3.fc10.src.rpm
Comment 5 Adam Tkac 2008-12-03 08:32:07 EST
"Formal" review:

source files match upstream: YES
package meets naming and versioning guidelines: YES
specfile is properly named, is cleanly written and uses macros consistently: NO
dist tag is present: YES
build root is correct: YES
license field matches the actual license: YES
license is open source-compatible: YES
latest version is being packaged: YES
BuildRequires are proper: YES
compiler flags are appropriate: YES
%clean is present: YES
package builds in mock (Rawhide/x86_64): NO
debuginfo package looks complete: YES
rpmlint is silent: YES
final provides and requires look sane: YES
owns the directories it creates: YES
doesn't own any directories it shouldn't: YES
no duplicates in %files: YES
file permissions are appropriate: YES
code, not content: YES
-------------

Problems:
- no configure flags are needed on rawhide, especially overriding --sysconfdir (see configure.ac, DEFAULT_CONFIGFILE macro definition)

- package is not buildable on rawhide. New libtool no longer copies config.{guess,sub}. You have to copy it manually (for example "cp /usr/share/automake-1.10/config.{guess,sub} ." after you remove them)

- as you wrote above you can "Requires: /etc/cron.daily" instead of "Requires: anacron". But AFAIK it is not needed because only anacron provides that directory, no other cron daemon.

Otherwise no problems.
Comment 6 Paul Wouters 2008-12-03 17:47:15 EST
I checked anacron and cronie with rpm -qi and now understand how they work together (and not like I thought before were a replacement). So Requires: anacron is fine with me.

with my last patch, it also fixed the default conf file from usptream to /etc/autotrust/autotrust.conf, so indeed, override is no longer needed. I've removed it.

I am still not sure how to deal with libtool. How am I supposed to know the latest version of libtool. Lookig in /usr/shake/automake-1.XX/ looks like an awful ugly hack. there must be a better way of doing this properly?
Comment 7 Adam Tkac 2008-12-04 08:48:45 EST
(In reply to comment #6)
> I am still not sure how to deal with libtool. How am I supposed to know the
> latest version of libtool. Lookig in /usr/shake/automake-1.XX/ looks like an
> awful ugly hack. there must be a better way of doing this properly?

Well, the best way is to report bug in upstream that they should not ship config.{sub,guess} as symlinks. Before new upstream release/tarball there are only two ways how deal with config.* problem:
- copy them from automake
- git clone git://git.sv.gnu.org/config.git and then add them as SourceX to srpm

You can choose.
Comment 8 Paul Wouters 2009-01-17 17:55:29 EST
Okay, upstream has fixed the issue. I've created a package with their rc release, though I'll redo the package in a day or two when the final release is made by upstream.

Spec URL: ftp://ftp.xelerance.com/autotrust/autotrust.spec
SRPM URL: ftp://ftp.xelerance.com/autotrust/autotrust-0.2.1rc1-1.fc10.src.rpm
Comment 9 Paul Wouters 2009-01-21 11:16:07 EST
Thanks for the addition comments Adam, here is the new release:

Spec URL: ftp://ftp.xelerance.com/autotrust/autotrust.spec
SRPM URL: ftp://ftp.xelerance.com/autotrust/autotrust-0.2.1-0.1.rc1.fc10.src.rpm
Comment 10 Adam Tkac 2009-01-21 11:35:59 EST
(In reply to comment #9)
> Thanks for the addition comments Adam, here is the new release:
> 

Well, package looks better but I still can't give you review, two problems are remaining:

autotrust.x86_64: W: incoherent-version-in-changelog 0.2.1-0
- please use 0.2.1-0.1.rc1 instead

Drop --sysconfdir configure parameter, it is not needed (as written in comment #5)
Comment 11 Paul Wouters 2009-01-21 23:24:03 EST
done

Spec URL: ftp://ftp.xelerance.com/autotrust/autotrust.spec
SRPM URL:
ftp://ftp.xelerance.com/autotrust/autotrust-0.2.1-0.2.rc1.fc10.src.rpm

rpmlint autotrust-0.2.1-0.2.rc1.fc10.src.rpm /autotrust-0.2.1-0.2.rc1.fc10.x86_64.rpm /autotrust-debuginfo-0.2.1-0.2.rc1.fc10.x86_64.rpm
3 packages and 0 specfiles checked; 0 errors, 0 warnings.
Comment 12 Adam Tkac 2009-01-22 07:55:49 EST
Ok, all issues written in comment #5 are fixed => package is reviewed.
Comment 13 Paul Wouters 2009-01-23 10:35:52 EST
New Package CVS Request
=======================
Package Name: autotrust
Short Description: DNSKEY trust anchor update utility that uses RFC-5011
Owners: pwouters
Branches: F-10, F-9, EL-5
InitialCC:
Comment 14 Kevin Fenzi 2009-01-23 18:13:50 EST
cvs done.
Comment 15 Paul Wouters 2009-01-23 18:28:15 EST
Looks like an ACL is still missing?

cvs commit...
**** Access denied: pwouters is not in ACL for rpms/autotrust/devel
cvs commit: Pre-commit check failed
cvs [commit aborted]: correct above errors first!
cvs commit: saving log message in /tmp/cvsVuHdcH
Comment 16 Kevin Fenzi 2009-01-23 19:01:54 EST
The job that generates the acl runs every 30min. Please wait a while and try again?
Comment 17 Paul Wouters 2009-01-23 19:30:46 EST
it worked. thanks!

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