Bug 225302
Summary: | Merge Review: automake | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Nobody's working on this, feel free to take it <nobody> |
Component: | Package Review | Assignee: | Paul Howarth <paul> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | karsten, opensource, paul, rc040203, redhat-bugzilla |
Target Milestone: | --- | Flags: | paul:
fedora-review+
|
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2010-08-06 19:49:14 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: |
Description
Nobody's working on this, feel free to take it
2007-01-29 21:09:09 UTC
Most of the comments having been applied to the autoconf packages also apply here: - Use "make DESTDIR=... install" instead of %makeinstall - BuildRoot - BuildArchitectures - Use Requires(Pre,Preun) instead of Prereq ... Additionally: - CONSIDER: Add all %{_*dirs} you are using in %files to ./configure (cf. https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225296#c3) - CONSIDER: I'd recommend to filter the 'perl(Automake::*)' provides/requires. IMO, they are bogus, because these modules are not in perl's module search path, but are automake internal perl-modules. fixed in automake-1.10-4 same here, please update the status automake.noarch: W: file-not-utf8 /usr/share/doc/automake-1.10.1/NEWS Easily to solve. automake.noarch: W: devel-file-in-non-devel-package /usr/share/automake-1.10/ansi2knr.c Is that correct? (In reply to comment #4) > automake.noarch: W: devel-file-in-non-devel-package > /usr/share/automake-1.10/ansi2knr.c > > Is that correct? Yes. NEWS file fixed in automake-1.10.2-2. Can't fix the ansi2knr stuff as this is used by some m4 macros and will be compiled when needed. Here's a IMHO neater method of fixing the perl requires/provides that doesn't involve turning off the automatic dependency generator or requiring out-of-spec scripts:
Index: automake.spec
===================================================================
RCS file: /cvs/pkgs/rpms/automake/devel/automake.spec,v
retrieving revision 1.44
diff -r1.44 automake.spec
10,11d9
< Source1: filter-provides-automake.sh
< Source2: filter-requires-automake.sh
22,25d19
< %define _use_internal_dependency_generator 0
< %define __find_provides %{SOURCE1}
< %define __find_requires %{SOURCE2}
<
37a32,37
> # Kludge to remove bogus Automake perl dependencies and provides
> %global reqfilt /bin/sh -c "%{__perl_requires} | grep -Fv 'perl(Automake::'"
> %define __perl_requires %{reqfilt}
> %global provfilt /bin/sh -c "%{__perl_provides} | grep -Fv 'perl(Automake::'"
> %define __perl_provides %{provfilt}
>
It also has the beneficial side effect of fixing this rpmlint warning:
automake.src: W: strange-permission filter-requires-automake.sh 0775
automake.src: W: strange-permission filter-provides-automake.sh 0775
As for this:
automake.noarch: W: devel-file-in-non-devel-package /usr/share/automake-1.11/ansi2knr.c
Since automake is itself a developer tool, this can safely be ignored.
Which parts of automake are MIT licensed?
- The URL is outdated, the new one is: http://sourceware.org/automake/ - The installation of the manpages is missing -p to preserve the timestamp - Why are the manpages not submitted to upstream? Please remove the NotReady from the status Whiteboard once it is clear which part of automake is MIT licensed (see comment:7). I couldn't find any MIT licensed files in the package anymore, dropped MIT from the list. I've also fixed the man page timestamps: http://cvs.fedora.redhat.com/viewvc/devel/automake/automake.spec?revision=1.47&view=markup Review: - rpmlint output: automake.noarch: W: devel-file-in-non-devel-package /usr/share/automake-1.11/ansi2knr.c automake.src: W: strange-permission filter-requires-automake.sh 0775 automake.src: W: strange-permission filter-provides-automake.sh 0775 rpmlint is wrong about automake being a non-devel package so the first one can be ignored. the filtering could be done more cleanly as mentioned in comment 7, which would resolve the automake.src warnings but that's not a blocker. - package and spec file naming OK - package meets guidelines - license OK, matches sources - spec file written in English and is legible - sources match upstream - package builds OK in mock for Rawhide (x86_64) - buildreqs OK - no translations, shared or static libraries to worry about - no bundled libraries - package not relocatable - directory ownership OK - file permissions OK - %defattr and %clean present and correct - macro usage is consistent - only code and permissable content included - docs not excessively large and don't affect runtime - not a GUI app, no .desktop file needed - no libtool libraries present - buildroot properly cleaned at start of %install - filenames all US-ASCII Comments: The comment in %description about also needing to install autoconf is redundant since there is a package dependency on autoconf. Whilst macro usage is consistent, variable usage isn't, i.e. we have: $RPM_BUILD_ROOT ${RPM_BUILD_ROOT} $RPM_BUILD_ROOT/ Please stick to one of the first two forms; either would be OK. Hangovers from previous comments: - have the manpages been submitted upstream? (Comment 8) - URL needs updating; existing URL is a redirect to http://sourceware.org/automake/ (Comment 8) - requires/provides filters could be cleaner (Comment 7) None of the identified issues are blockers so I'm inclined to approve this package unless anyone else has anything they'd like to bring up? (In reply to comment #10) > - have the manpages been submitted upstream? (Comment 8) Automake-1.11 ships pregenerated manpages. (In reply to comment #11) > (In reply to comment #10) > > - have the manpages been submitted upstream? (Comment 8) > > Automake-1.11 ships pregenerated manpages. Indeed it does. They're slightly smaller than the Debian manpages but more likely to be in-sync with what automake/aclocal actually do, so I think the Debian manpages should be dropped. (In reply to comment #12) > Indeed it does. They're slightly smaller than the Debian manpages but more > likely to be in-sync with what automake/aclocal actually do, so I think the > Debian manpages should be dropped. Agreed. Upstream's manpages are help2man generated from automake's sources. => If you really want to add manpages to older automakes, my advise would be to utilize help2man (Other distros do so, I do so in my rtems.org automake packages). I've fixed the following in automake-1.11.1-2.fc14: - better method of fixing the perl requires/provides - fixed variable usage in spec file - use pregenerated manpages from automake-1.11 - updated URL A few minor nits: * Redundant text about installing Autoconf still present in %description (package dependency enforces installation of autoconf so there's no need to mention this) * Creation of empty directory ${RPM_BUILD_ROOT}%{_datadir}/aclocal in %install is no longer necessary as the package no longer owns this directory * Old man pages still in CVS and can be removed since they're no longer referenced from the spec I'll approve this package tomorrow unless anyone has any further objections. I've fixed the three issues from comment #15 in automake-1.11.1-5.fc14 APPROVED. |