Bug 225296

Many comments done for the autoconf213 are also valid here,
please fix those items.

* Additionally, -n autoconf-%{version} is not useful on the %setup
line.

* Could you please explain why you don't use config.sub/guess from
autoconf?

* Prereq(post,preun): should be 
Requires(post): ....
Requires(preun): ....

- I don't use autoconf's config.sub/guess as they require help2man. This is in
extras, but our buildsystem doesn't pull in packages from there atm. help2man is
only required if you modified a dependency of a manual page, which is not the
case here.

autoconf-2.61-4.fc7 has the following changes:
 - add disttag
 - replace  tabs with spaces
 - fix buildroot
 - use Requires(post), Requires(preun)
 - use make install DESTDIR=....
 - drop perl requirement as it gets pulled it automatically

(In reply to comment #2)
> - I don't use autoconf's config.sub/guess as they require help2man.
Sorry, what you say here does not apply. It's a side-effect of a questionable
feature in %configure:

%configure replaces config.guess/config.sub
=> Timestamps are being altered => rebuilding the corresponding man-pages

With your spec:

# rpmbuild autoconf.spec
...
++ find . -name config.guess -o -name config.sub
+ for i in '$(find . -name config.guess -o -name config.sub)'
++ basename ./build-aux/config.guess
+ '[' -f /usr/lib/rpm/redhat/config.guess ']'
+ /bin/rm -f ./build-aux/config.guess
++ basename ./build-aux/config.guess 
+ /bin/cp -fv /usr/lib/rpm/redhat/config.guess ./build-aux/config.guess
`/usr/lib/rpm/redhat/config.guess' -> `./build-aux/config.guess'
+ for i in '$(find . -name config.guess -o -name config.sub)'   
++ basename ./build-aux/config.sub
+ '[' -f /usr/lib/rpm/redhat/config.sub ']'
+ /bin/rm -f ./build-aux/config.sub
++ basename ./build-aux/config.sub 
+ /bin/cp -fv /usr/lib/rpm/redhat/config.sub ./build-aux/config.sub
`/usr/lib/rpm/redhat/config.sub' -> `./build-aux/config.sub'
...
=> %configure messed up the sources


I.e. if using plain ./configure instead of %configure, this issue goes away.

Try this patch and you will see:

Index: autoconf.spec
===================================================================
RCS file: /cvs/dist/devel/autoconf/autoconf.spec,v
retrieving revision 1.41
diff -u -r1.41 autoconf.spec
--- autoconf.spec       15 Feb 2007 11:34:43 -0000      1.41
+++ autoconf.spec       16 Feb 2007 02:47:02 -0000
@@ -33,13 +33,8 @@
 %setup -q

 %build
-# move config files out of they way as they require help2man from extras:
-cp -p build-aux/config{.,-}guess
-cp -p build-aux/config{.,-}sub
-%configure
-# ... and restore them:
-mv build-aux/config{-,.}guess
-mv build-aux/config{-,.}sub
+./configure --prefix=%{_prefix} --mandir=%{_mandir} --infodir=%{_infodir} \
+  --bindir=%{_bindir} --datadir=%{_datadir}
 make #  %{?_smp_mflags}  Makefile not smp save

 #check

- CONSIDER: I'd recommend to filter the 'perl(Autom4te::*)' provides/requires.
IMO, they are bogus, because these modules are not in perl's module search path,
but are autoconf internal perl-modules.

fixed in -5

* there should be a comment explaining why there are the %define
for the find_provides (something along
# filter out bogus perl(Autom4te*) dependencies

And for ./configure instead of %configure a comment is needed too.

* sed in BuildRequires is unneeded

* gawk doesn't seem to be needed as BuildRequires nor as Requires


Suggestions:

* call the dependency generator files along
autoconf-filter-provides.sh

* remove .gz in install-info scriptlets

all done except the first suggestion in -6. I'll keep the filenames.

grep is used in autom4te in a system call, so there is a missing
Requires.

Otherwise it seems right to me. Ralf, do you see any other issue?

Source match upstream, but the tarball timestamp isn't the same.

grep requirement fixed in autoconf-2.61-7.fc7, but I have no idea why the
timestamp is wrong. As the source matches upstream I'd think we can leave it as
it is.

(In reply to comment #10)
> grep requirement fixed in autoconf-2.61-7.fc7, but I have no idea why the
> timestamp is wrong. As the source matches upstream I'd think we can leave it as
> it is.

Ok. Just remember to keep it next time. In case you don't already know,
youo can use wget -N for that, or spectool -g.

Just noticed that %{_datadir}/emacs/ is unowned. Since you
already own %{_datadir}/emacs/site-lisp/ it shouldn't be
a problem to own %{_datadir}/emacs/ too.

I think that the way you do is the best way, it seems better to me 
to own those directories than to depend on emacs-common.

sounds reasonable, fixed in -8

I don't see any other issues, I hope I didn't miss anything, I will 
assign to me, and set that package to accepted; Ralf you can change 
or comment if you are not happy with that.

Closing as suggested by Warren in his mail to fedora-maintainers 'Review with
Flags (Version 6)'

Another issue was raised on the devel list: what is the use
of the imake Requires? If it is needed by tests it should be 
BuildRequires, if in a macro it may be a dependency for the 
program configure script, but not for autoconf.

(In reply to comment #16)
> Another issue was raised on the devel list: what is the use
> of the imake Requires? If it is needed by tests it should be 
> BuildRequires, if in a macro it may be a dependency for the 
> program configure script, but not for autoconf.

imake is neither required to run the tests nor is it needed at run-time.
=> "Requires: imake" is wrong and should be removed.

To provide better test coverage BR: libtool would make sense.