Bug 225302

Summary: Merge Review: automake
Product: [Fedora] Fedora Reporter: Nobody's working on this, feel free to take it <nobody>
Component: Package ReviewAssignee: Paul Howarth <paul>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: 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
Fedora Merge Review: automake

http://cvs.fedora.redhat.com/viewcvs/devel/automake/

Comment 1 Ralf Corsepius 2007-02-16 03:30:00 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.



Comment 2 Karsten Hopp 2007-02-20 12:43:20 UTC
fixed in automake-1.10-4

Comment 3 Karsten Hopp 2007-03-13 15:34:45 UTC
same here, please update the status

Comment 4 Robert Scheck 2009-01-13 22:15:55 UTC
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?

Comment 5 Ralf Corsepius 2009-01-13 22:22:25 UTC
(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.

Comment 6 Karsten Hopp 2009-01-21 13:38:45 UTC
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.

Comment 7 Paul Howarth 2009-09-09 13:01:28 UTC
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?

Comment 8 Till Maas 2009-09-16 23:30:22 UTC
- 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).

Comment 9 Karsten Hopp 2009-12-02 12:25:09 UTC
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

Comment 10 Paul Howarth 2010-02-02 15:14:20 UTC
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?

Comment 11 Ralf Corsepius 2010-02-02 15:38:56 UTC
(In reply to comment #10)
> - have the manpages been submitted upstream? (Comment 8)

Automake-1.11 ships pregenerated manpages.

Comment 12 Paul Howarth 2010-02-05 11:22:32 UTC
(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.

Comment 13 Ralf Corsepius 2010-02-05 11:29:34 UTC
(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).

Comment 14 Karsten Hopp 2010-03-02 11:17:48 UTC
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

Comment 15 Paul Howarth 2010-03-29 09:49:59 UTC
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.

Comment 16 Karsten Hopp 2010-03-29 11:16:54 UTC
I've fixed the three issues from comment #15 in automake-1.11.1-5.fc14

Comment 17 Paul Howarth 2010-03-30 13:34:42 UTC
APPROVED.