This service will be undergoing maintenance at 00:00 UTC, 2016-09-28. It is expected to last about 1 hours
Bug 225302 - Merge Review: automake
Merge Review: automake
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Paul Howarth
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-01-29 16:09 EST by Nobody's working on this, feel free to take it
Modified: 2010-08-06 15:49 EDT (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-08-06 15:49:14 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
paul: fedora‑review+


Attachments (Terms of Use)

  None (edit)
Description Nobody's working on this, feel free to take it 2007-01-29 16:09:09 EST
Fedora Merge Review: automake

http://cvs.fedora.redhat.com/viewcvs/devel/automake/
Comment 1 Ralf Corsepius 2007-02-15 22:30:00 EST
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 07:43:20 EST
fixed in automake-1.10-4
Comment 3 Karsten Hopp 2007-03-13 11:34:45 EDT
same here, please update the status
Comment 4 Robert Scheck 2009-01-13 17:15:55 EST
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 17:22:25 EST
(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 08:38:45 EST
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 09:01:28 EDT
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 19:30:22 EDT
- 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 07:25:09 EST
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 10:14:20 EST
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 10:38:56 EST
(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 06:22:32 EST
(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 06:29:34 EST
(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 06:17:48 EST
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 05:49:59 EDT
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 07:16:54 EDT
I've fixed the three issues from comment #15 in automake-1.11.1-5.fc14
Comment 17 Paul Howarth 2010-03-30 09:34:42 EDT
APPROVED.

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