Bug 225296 - Merge Review: autoconf
Merge Review: autoconf
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Patrice Dumas
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-01-29 16:08 EST by Nobody's working on this, feel free to take it
Modified: 2007-11-30 17:11 EST (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-03-13 11:40:05 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
pertusus: fedora‑review+


Attachments (Terms of Use)

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

http://cvs.fedora.redhat.com/viewcvs/devel/autoconf/
Comment 1 Patrice Dumas 2007-02-14 15:20:00 EST
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 06:37:02 EST
- 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-15 21:51:04 EST
(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-15 22:28:34 EST
- 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 06:40:45 EST
fixed in -5
Comment 6 Patrice Dumas 2007-02-21 18:22:13 EST
* 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 06:12:48 EST
all done except the first suggestion in -6. I'll keep the filenames.
Comment 8 Patrice Dumas 2007-02-23 16:41:14 EST
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 07:58:23 EST
Source match upstream, but the tarball timestamp isn't the same.
Comment 10 Karsten Hopp 2007-02-26 05:29:10 EST
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 17:34:50 EST
(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 17:37:46 EST
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 06:38:44 EST
sounds reasonable, fixed in -8
Comment 14 Patrice Dumas 2007-02-28 18:15:00 EST
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 11:40:05 EDT
Closing as suggested by Warren in his mail to fedora-maintainers 'Review with
Flags (Version 6)'
Comment 16 Patrice Dumas 2007-03-29 04:28:35 EDT
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 07:50:38 EDT
(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.