Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 225296 - Merge Review: autoconf
Summary: Merge Review: autoconf
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Patrice Dumas
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-01-29 21:08 UTC by Nobody's working on this, feel free to take it
Modified: 2007-11-30 22:11 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-03-13 15:40:05 UTC
pertusus: fedora-review+


Attachments (Terms of Use)

Description Nobody's working on this, feel free to take it 2007-01-29 21:08:16 UTC
Fedora Merge Review: autoconf

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

Comment 1 Patrice Dumas 2007-02-14 20:20:00 UTC
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): ....

Comment 2 Karsten Hopp 2007-02-15 11:37:02 UTC
- 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

Comment 3 Ralf Corsepius 2007-02-16 02:51:04 UTC
(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

Comment 4 Ralf Corsepius 2007-02-16 03:28:34 UTC
- 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.


Comment 5 Karsten Hopp 2007-02-19 11:40:45 UTC
fixed in -5

Comment 6 Patrice Dumas 2007-02-21 23:22:13 UTC
* 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

Comment 7 Karsten Hopp 2007-02-22 11:12:48 UTC
all done except the first suggestion in -6. I'll keep the filenames.

Comment 8 Patrice Dumas 2007-02-23 21:41:14 UTC
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?

Comment 9 Patrice Dumas 2007-02-24 12:58:23 UTC
Source match upstream, but the tarball timestamp isn't the same.

Comment 10 Karsten Hopp 2007-02-26 10:29:10 UTC
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.

Comment 11 Patrice Dumas 2007-02-26 22:34:50 UTC
(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.

Comment 12 Patrice Dumas 2007-02-26 22:37:46 UTC
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.


Comment 13 Karsten Hopp 2007-02-27 11:38:44 UTC
sounds reasonable, fixed in -8

Comment 14 Patrice Dumas 2007-02-28 23:15:00 UTC
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.

Comment 15 Karsten Hopp 2007-03-13 15:40:05 UTC
Closing as suggested by Warren in his mail to fedora-maintainers 'Review with
Flags (Version 6)'

Comment 16 Patrice Dumas 2007-03-29 08:28:35 UTC
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.

Comment 17 Ralf Corsepius 2007-03-29 11:50:38 UTC
(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.


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