Bug 225683

Just a note, not a review: package appears to be compiled without
$RPM_OPT_FLAGS.  Using 'make CFLAGS="$RPM_OPT_FLAGS"' seems to fix some of it,
but causes also the build to fail as options not recognized by ncc are passed to
it during build.
http://www.redhat.com/archives/fedora-maintainers/2007-January/msg00339.html

I patched the makefile to pass the RPM_OPT_FLAGS to the dev86 part compiled by
gcc, and here we go:

(seems like ncc needs additional fixing...)

ncc -c -Mn -O -D__LIBC__ -D__LIBC_VER__='"0.16.17"' -o crt0.o crt0.c
*** buffer overflow detected ***: ncc terminated
======= Backtrace: =========
/lib/libc.so.6(__chk_fail+0x41)[0x245361]
/lib/libc.so.6[0x245aa8]
ncc[0x8049047]
ncc[0x804ae62]
/lib/libc.so.6(__libc_start_main+0xdc)[0x179f2c]
ncc[0x8048901]
======= Memory map: ========
00147000-00160000 r-xp 00000000 03:02 3820099    /lib/ld-2.5.so
00160000-00161000 r-xp 00018000 03:02 3820099    /lib/ld-2.5.so
00161000-00162000 rwxp 00019000 03:02 3820099    /lib/ld-2.5.so
00164000-0029b000 r-xp 00000000 03:02 3820100    /lib/libc-2.5.so
0029b000-0029d000 r-xp 00137000 03:02 3820100    /lib/libc-2.5.so
0029d000-0029e000 rwxp 00139000 03:02 3820100    /lib/libc-2.5.so
0029e000-002a1000 rwxp 0029e000 00:00 0 
04225000-04230000 r-xp 00000000 03:02 3819945    /lib/libgcc_s-4.1.1-20070105.so.1
04230000-04231000 rwxp 0000a000 03:02 3819945    /lib/libgcc_s-4.1.1-20070105.so.1
08048000-0804d000 r-xp 00000000 03:02 4285554   
/home/jnovy/CVS/dev86/devel/dev86-0.16.17/bin/ncc
0804d000-0804e000 rwxp 00004000 03:02 4285554   
/home/jnovy/CVS/dev86/devel/dev86-0.16.17/bin/ncc
09b79000-09b9a000 rwxp 09b79000 00:00 0 
40000000-40001000 r-xp 40000000 00:00 0          [vdso]
40001000-40002000 rw-p 40001000 00:00 0 
4001a000-4001b000 rw-p 4001a000 00:00 0 
bfe8d000-bfea2000 rw-p bfe8d000 00:00 0          [stack]
make[4]: *** [crt0.o] Aborted
make[4]: Leaving directory `/home/jnovy/CVS/dev86/devel/dev86-0.16.17/libc'
make[3]: *** [library] Error 2
make[3]: Leaving directory `/home/jnovy/CVS/dev86/devel/dev86-0.16.17'
make[2]: *** [all] Error 2
make[2]: Leaving directory `/home/jnovy/CVS/dev86/devel/dev86-0.16.17'
make[1]: *** [all] Error 2
make[1]: Leaving directory `/home/jnovy/CVS/dev86/devel/dev86-0.16.17'
error: Bad exit status from /var/tmp/rpm-tmp.64192 (%build)

I'll review it.

Notes:

* "BuildRequires: gawk" is redundant (gawk is in Exceptions list https://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2 ). Not an issue, though.

* Looks like this package disallows parallel builds. You should add note about it.

* It's a good idea to add notes about patch status - upstreamed (with bz# or with maillist's link), specific for fedora and therefore shouldn't be upstreamed, etc

* What the purpose of expression at line 16? 

Other things (except this sorrow situation with RPM_OPT_FLAGS, described above) looks sane. So this is a formal review:

- rpmlint is not silent - see output (except numerous messages about devel-file-in-non-devel-package, which may be safely ignored, and binaryinfo-readelf-failed due to my powerpc arch):

[petro@Sulaco SPECS]$ rpmlint ../RPMS/ppc/dev86-*|grep -v devel-file-in-non-devel-package | grep -v binaryinfo-readelf-failed
dev86.ppc: E: zero-length /usr/lib/bcc/include/math.h
dev86.ppc: E: zero-length /usr/lib/bcc/include/linux/ioctl.h
dev86.ppc: W: obsolete-not-provided bin86
2 packages and 0 specfiles checked; 2 errors, 105 warnings.
[petro@Sulaco SPECS]$ rpmlint ../SRPMS/dev86-0.16.17-12.fc10.src.rpm 
dev86.src:13: W: unversioned-explicit-obsoletes bin86
dev86.src: W: mixed-use-of-spaces-and-tabs (spaces: line 16, tab: line 44)
1 packages and 0 specfiles checked; 0 errors, 2 warnings.
[petro@Sulaco SPECS]$

I think that these messages are safe to ignore too.

+ The package is named according to the Package Naming Guidelines .
+ The spec file name matches the base package %{name}, in the format %{name}.spec.
+ The package meets the Packaging Guidelines.
+ The package is licensed with a Fedora approved license and meets the Licensing Guidelines .
+ The License field in the package spec file matches the actual license.

- File, containing the text of the license(s), MUST be included in %doc. 

+ The spec file is written in American English.
+ The spec file for the package is legible.
+ The package successfully compiles and builds into binary rpms on at least one primary architecture.
+ All build dependencies are listed in BuildRequires.
+ No need to handle locales.
+ The package owns all directories that it creates.
+ The package doesn't contain any duplicate files in the %files listing.
+ Permissions on files are set properly.
+ The package has a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
+ The package consistently uses macros.
+ The package contains code, or permissible content.
+ No large documentation files.
+ All files, that the package includes as %doc, does not affect the runtime of the application.

+/- Header files must be in a -devel package, but I'm in doubts whether this rule can or cannot be applied in this case. And the next one.
+/- Static libraries must be in a -static package. See note above.

+ No pkgconfig(.pc) files
+ No .la libtool archives
+ Not a GUI application
+ The package does not own files or directories already owned by other packages.
+ At the beginning of %install, the package runs rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
+ All filenames in rpm packages must be valid UTF-8.

(In reply to comment #4)
> Notes:
> 
> * "BuildRequires: gawk" is redundant (gawk is in Exceptions list
> https://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2 ). Not an
> issue, though.

Removed.

> * Looks like this package disallows parallel builds. You should add note about
> it.

Yup, commented.

> * It's a good idea to add notes about patch status - upstreamed (with bz# or
> with maillist's link), specific for fedora and therefore shouldn't be
> upstreamed, etc

Upstream is dead for couple of years AFAIK.

> * What the purpose of expression at line 16? 

/usr/bin/strip tries to strip binaries generated by dev86. This is bad as strip doesn't know their format and fails so it is needed to be removed from __os_install_post.

> Other things (except this sorrow situation with RPM_OPT_FLAGS, described above)

Fixed. Shipped binaries should now be compiled with RPM_OPT_FLAGS.

> - File, containing the text of the license(s), MUST be included in %doc. 

Added both GPL and LGPL.

> +/- Header files must be in a -devel package, but I'm in doubts whether this
> rule can or cannot be applied in this case. And the next one.
> +/- Static libraries must be in a -static package. See note above.

Better not trying to fix this. This package is in many cases special and doesn't match the ordinary -devel and -static like packaging scheme.

OK, this package is APPROVED.

What should I do next? I guess, that I should simply close this ticket, since the package already in Fedora.

Package Change Request
======================
Package Name: dev86
New Branches: EL-6
Owners: jnovy

CVS done (by process-cvs-requests.py).