Bug 473367

I'm going to review this package

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

- 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

* Tue Dec  2 2008 Paul Wouters <paul> - 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

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

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?

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

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

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

(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)

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.

Ok, all issues written in comment #5 are fixed => package is reviewed.

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:

cvs done.

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

The job that generates the acl runs every 30min. Please wait a while and try again?

it worked. thanks!