Bug 225295 - Merge Review: autoconf213
Merge Review: autoconf213
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:07 EST by Nobody's working on this, feel free to take it
Modified: 2007-11-30 17:11 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-03-13 11:39:13 EDT
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)
use autoconf213 info file and add a comment saying it's legacy (464 bytes, patch)
2007-02-14 15:08 EST, Patrice Dumas
no flags Details | Diff

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

http://cvs.fedora.redhat.com/viewcvs/devel/autoconf213/
Comment 1 Patrice Dumas 2007-02-13 18:34:05 EST
Issues:

* Builroot is not right

* In general textutils shouldn't be in Requires. Which program is 
  it for?

* info files should be compressed automatically

* remove summary end dot

* install-info scriptlets are missing

* gawk and perl seems to be BuildRequires. perl may be omitted.

suggestions:

* the indenting of the beginning of the spec looks bad because there 
  are spaces and tabs.

* I think that BuildArch is prefered over BuildArchitectures

* use %defattr(-,root,root,-) instead of %defattr(-,root,root) for 
  consistency

* having a / at the end of a directory in %files shows visually that
  it is indeed a directory:
%{_datadir}/autoconf-%{version}/

* maybe the testsuite could be run?

Remark:

* make install DESTDIR=.. is preferred over %makeinstall, but here
  it seems that there is no support for DESTDIR.
Comment 2 Karsten Hopp 2007-02-14 10:29:47 EST
in autoconf213-2.13-13:
- buildroot fixed
- removed textutils, rebuilt in mock, no errors, no lost files. looks like it
  doesn't need this requirement anymore
- dot removed from summary
- it requires gawk, but not perl (real requirements, not build requirements)
- use install-info
- use BuildArch
- replace tabs with spaces
- fix defattr
- use 'make install DESTDIR=...'

info files are compressed automatically by rpmbuild, it's not required to put
that into the spec file.
Comment 3 Patrice Dumas 2007-02-14 11:01:37 EST
* remove the .gz in scriptlets

* remove 
gzip -9nf ${RPM_BUILD_ROOT}%{_infodir}/*

* gawk and perl are also needed during the build, since their values
  are substituted using sed, in some scripts. Therefore they should
  be buildrequires. Same for m4 if I'm not wrong.
Comment 4 Karsten Hopp 2007-02-14 11:34:11 EST
done in -14
Comment 5 Patrice Dumas 2007-02-14 14:43:55 EST
This is approved. I assigned it to you.
Comment 6 Patrice Dumas 2007-02-14 15:07:19 EST
I still have 2 remarks. The rpmlint warning is ignorable
W: autoconf213 devel-file-in-non-devel-package /usr/share/autoconf-2.13/acconfig.h

The install-info scriptlet adds an entry for Autoconf in the Misc
section in dir. It points to the latest autoconf manual and not 
to the autoconf213 manual. I'll attach a patch.
Comment 7 Patrice Dumas 2007-02-14 15:08:54 EST
Created attachment 148084 [details]
use autoconf213 info file and add a comment saying it's legacy
Comment 8 Patrice Dumas 2007-02-14 15:15:18 EST
Hopefully a very last comment, you didn't used %dist, and I think 
it is a good choice. This package shouldn't ever need a rebuild
(except to check that it actually builds).
Comment 9 Karsten Hopp 2007-02-15 05:58:54 EST
-15 has:
- a disttag
- autoconf213 info file. I've done it a little bit different, so that the File:
tag inside info is correct.
Comment 10 Patrice Dumas 2007-02-16 01:53:27 EST
I haven't tested, but your changes look good and saner.
Comment 11 Patrice Dumas 2007-02-24 07:44:21 EST
I have checked that source is upstream source, but it doesn't
seems to be the case. The checksum, size, and timestamp are
different.

I reassign to me since it seems to be what should be done in the
new version of the review process.
Comment 12 Karsten Hopp 2007-02-26 05:42:02 EST
I don't know what the previous maintainer did, but I've downloaded the original
tarball and run a diff against our tarball. There are no changes at all.
I've built a new package with upstream sources to clean this up.
Comment 13 Patrice Dumas 2007-02-26 18:02:12 EST
Please consider keeping the source timestamp too in the
future. 

Anyway it is re-approved.
Comment 14 Karsten Hopp 2007-03-13 11:39:13 EDT
Closing as suggested by Warren in his mail to fedora-maintainers 'Review with
Flags (Version 6)'
Comment 15 Stepan Kasal 2007-07-18 12:14:58 EDT
(In reply to comment #3)
> * gawk and perl are also needed during the build, [...]
>   Therefore they should be buildrequires.

A nit (not worth fixing):

Both are redundant, as both are in the minimum build environment.

Perl is explicitly listed among the exceptions in Packaging/Guidelines.
gawk is dragged there indirectly, but it is hard to imagine minimal instal
without awk.
Comment 16 Patrice Dumas 2007-07-24 06:56:30 EDT
(In reply to comment #15)

> Both are redundant, as both are in the minimum build environment.
> 
> Perl is explicitly listed among the exceptions in Packaging/Guidelines.
> gawk is dragged there indirectly, but it is hard to imagine minimal instal
> without awk.

Personally, I think that they shouldn't be in the minimal 
install and hopefully won't be in the future. Moreover they are
really direct build dependencies and it was only a suggestion.
Comment 17 Ralf Corsepius 2007-07-24 08:35:32 EDT
(In reply to comment #16)
> (In reply to comment #15)

> Personally, I think that they shouldn't be in the minimal 
> install and hopefully won't be in the future.
awk is part of the POSIX standard run-time environment.

Unless POSIX changes, you will always find awk installed on any POSIX compliant
system.

> Moreover they are
> really direct build dependencies and it was only a suggestion.
Fedora not having them always installed is a bug in Fedora.


Comment 18 Patrice Dumas 2007-07-24 09:01:32 EDT
The minimal install and minimal build environment needs not 
be POSIX compliant. There could be a POSIX meta package (I
guess the LSB package already bring in everything needed
for POSIX). I am not saying that it should not be POSIX compliant
but it should only if it makes sense, we shouldn't tie ourselves
to standards -- while still giving the possibility to conform.

More precisely, I personally think that using a comps that has not 
been tuned should give a minimal install that is POSIX compliant, 
but in my opinion we should give room for minimal sets that are 
more minimal than POSIX, and in those minimal install dependencies
should still be correct.
Comment 19 Ralf Corsepius 2007-07-24 11:28:17 EDT
(In reply to comment #18)
> The minimal install and minimal build environment needs not 
> be POSIX compliant. 
Once again: POSIX is an official standard (IEEE Std 1003.1), not an arbitrary
vendor package set.

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