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 225683 - Merge Review: dev86
Summary: Merge Review: dev86
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Peter Lemenkov
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-01-31 17:57 UTC by Nobody's working on this, feel free to take it
Modified: 2010-07-12 17:00 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-02-19 12:18:16 UTC
lemenkov: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Nobody's working on this, feel free to take it 2007-01-31 17:57:01 UTC
Fedora Merge Review: dev86

http://cvs.fedora.redhat.com/viewcvs/devel/dev86/
Initial Owner: jnovy@redhat.com

Comment 1 Ville Skyttä 2007-02-20 22:24:43 UTC
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

Comment 2 Jindrich Novy 2007-02-21 15:23:42 UTC
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)


Comment 3 Peter Lemenkov 2009-02-16 20:32:45 UTC
I'll review it.

Comment 4 Peter Lemenkov 2009-02-18 10:48:01 UTC
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.

Comment 5 Jindrich Novy 2009-02-19 11:46:09 UTC
(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.

Comment 6 Peter Lemenkov 2009-02-19 12:18:16 UTC
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.

Comment 7 Jindrich Novy 2010-07-12 07:07:24 UTC
Package Change Request
======================
Package Name: dev86
New Branches: EL-6
Owners: jnovy

Comment 8 Kevin Fenzi 2010-07-12 17:00:52 UTC
CVS done (by process-cvs-requests.py).


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