Bug 225296 - Merge Review: autoconf
Summary: Merge Review: autoconf
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
(Show other bugs)
Version: rawhide
Hardware: All Linux
medium
medium
Target Milestone: ---
Assignee: Patrice Dumas
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Keywords:
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:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-03-13 15:40:05 UTC
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
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.