Bug 321411 - Review Request: ski - IA-64 user and system mode simulator
Review Request: ski - IA-64 user and system mode simulator
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jason Tibbitts
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-10-06 12:30 EDT by Dan Horák
Modified: 2008-04-13 04:34 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-04-13 04:34:18 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
tibbs: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Dan Horák 2007-10-06 12:30:23 EDT
Spec URL: http://fedora.danny.cz/ski.spec
SRPM URL: http://fedora.danny.cz/ski-1.2.6-1.src.rpm
Description:
The Ski IA-64 user and system mode simulator originally developed by HP.
Comment 1 Dan Horák 2007-10-06 14:23:50 EDT
Result of rpmlint:
I: ski-debuginfo checking
I: ski-devel checking
W: ski-devel no-documentation
The package contains no documentation (README, doc, etc).
You have to include documentation files.

I: ski checking
I: ski checking

Result of scratch build in Koji -
http://koji.fedoraproject.org/koji/taskinfo?taskID=185594 . There is a failure
on ppc64, but it will probably require access to such machine.
Comment 2 Jason Tibbitts 2007-11-07 11:02:02 EST
You missed a bunch of rpmlint complaints:

ski.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libski-1.2.so.6.0.0
/usr/lib64/libelf.so.1
ski.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libski-1.2.so.6.0.0
/lib64/libncurses.so.5

The libski-1.2.so is linked against those two libraries even though it doesn't
call any functions in them.

ski.x86_64: W: undefined-non-weak-symbol /usr/lib64/libski-1.2.so.6.0.0 force_user
ski.x86_64: W: undefined-non-weak-symbol /usr/lib64/libski-1.2.so.6.0.0 userint
ski.x86_64: W: undefined-non-weak-symbol /usr/lib64/libski-1.2.so.6.0.0 sim_root_len
ski.x86_64: W: undefined-non-weak-symbol /usr/lib64/libski-1.2.so.6.0.0 sim_root

and over 300 more similar messages.

The library calls those functions, but isn't linked against any library which
provides them.

Otherwise I think this package looks OK, but I don't know enough about the
software to know to know if the above issues are problematic.  I suspect that
the unused-direct-shlib-dependency warnings are merely inefficiencies (libraries
pulled in when they're not needed) and that the undefined-non-weak-symbols are
provided by libraries which are linked into the executables which also link
against libski-1.2.so.  Still, this package included a -devel subpackage
indicating that libski might be used separately from its executables, and so the
library should actually be linked against the libraries it needs if possible.
Comment 3 Dan Horák 2007-11-07 15:43:37 EST
The "undefined-non-weak-symbol" issues are probably caused by the fact that
libski needs symbols from libskiui and vice versa. So you always need to link
with both of them and the solution is to create only one library. Also there is
no need to link the ski application with libelf and libncurses, they are linked
into libski. So I will prepare some patches to clean the situation a bit.
Comment 4 Dan Horák 2007-11-10 06:42:05 EST
* Sat Nov 10 2007 Dan Horak <dan[at]danny.cz> 1.2.6-2
- merge libski and libskiui

Spec URL: http://fedora.danny.cz/ski.spec
SRPM URL: http://fedora.danny.cz/ski-1.2.6-2.fc8.src.rpm

Remaining issues are some "unused-direct-shlib-dependency" for gnome libraries,
which are taken in by pkg-config.
Comment 5 Dan Horák 2008-02-19 15:17:31 EST
* Tue Feb 19 2008 Dan Horak <dan[at]danny.cz> 1.3.2-1
- update to version 1.3.2
- remove patches integrated into upstream codebase
- create a lib subpackage to be multi-lib aware

Spec URL: http://fedora.danny.cz/ski.spec
SRPM URL: http://fedora.danny.cz/ski-1.3.2-1.fc8.src.rpm
Comment 6 Jason Tibbitts 2008-04-04 16:08:37 EDT
This failed to build for me in mock (x86_64, rawhide).  The build log is nasty
due to parallel make but I see this:

mock.Root.build: linux/syscall-linux.c:75:22: error: asm/page.h: No such file or
directory

This header doesn't seem to exist in rawhide, although it exists in F8.  It
seems that it's been removed intentionally.
Comment 7 Dan Horák 2008-04-04 16:50:35 EDT
Google says that user visible asm/page.h was removed in 2.6.25-rc5. But the
syscall-linux.c file doesn't need to include it. I will prepare an update.
Comment 8 Dan Horák 2008-04-05 04:24:40 EDT
* Sat Apr  5 2008 Dan Horak <dan[at]danny.cz> 1.3.2-2
- fix compile in rawhide (kernel >= 2.6.25-rc5)

Spec URL: http://fedora.danny.cz/ski.spec
SRPM URL: http://fedora.danny.cz/ski-1.3.2-2.fc9.src.rpm
Comment 9 Jason Tibbitts 2008-04-07 19:21:54 EDT
This is looking good.

You can get dir of the unused-direct-shlib-dependency complaints with another libtool hack:
  sed -i -e 's! -shared ! -Wl,--as-needed\0!g' libtool
Since you're already hacking up libtool a bit, there's probably no reason not to do that as well (unless it breaks something, I guess).

The remaining rpmlint complaints are:

  ski-devel.x86_64: W: no-documentation
Not a problem.

  ski-devel.x86_64: W: no-dependency-on ski
The -devel package depends on the -lib package, but not the main package.  This also is not a problem.

Most packages seem to use "-libs" for the library subpackage, but there are a
few who use "-lib" and we don't seem to have any guidelines about it, so
that's OK.

There's a test suite in the "testsuite" directory; is this something which
could be run at build time?

Note that there's no need to include the COPYING twice.  Just having it in the
main package is sufficient, even if it's possible to install the software
without installing that package.  This isn't a blocker, though.

* source files match upstream:
  34b2a1b2575d6c8703df8f1f3980f7b668e744c4a03f20ed4ed91d40cf40c076  
  ski-1.3.2.tar.gz
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
* description is OK.
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text included in package.
* latest version is being packaged.
* BuildRequires are proper.
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly.
* debuginfo package looks complete.
? rpmlint has acceptable complaints, but a few are trivially fixed if you care
  to.
* final provides and requires are sane:
  ski-1.3.2-2.fc9.x86_64.rpm
   config(ski) = 1.3.2-2.fc9
   ski = 1.3.2-2.fc9
  =
   config(ski) = 1.3.2-2.fc9
   libICE.so.6()(64bit)
   libORBit-2.so.0()(64bit)
   libSM.so.6()(64bit)
   libX11.so.6()(64bit)
   libXext.so.6()(64bit)
   libXm.so.2()(64bit)
   libXp.so.6()(64bit)
   libXt.so.6()(64bit)
   libart_lgpl_2.so.2()(64bit)
   libatk-1.0.so.0()(64bit)
   libbonobo-2.so.0()(64bit)
   libbonobo-activation.so.4()(64bit)
   libbonoboui-2.so.0()(64bit)
   libcairo.so.2()(64bit)
   libelf.so.1()(64bit)
   libgconf-2.so.4()(64bit)
   libgdk-x11-2.0.so.0()(64bit)
   libgdk_pixbuf-2.0.so.0()(64bit)
   libglade-2.0.so.0()(64bit)
   libglib-2.0.so.0()(64bit)
   libgmodule-2.0.so.0()(64bit)
   libgnome-2.so.0()(64bit)
   libgnomecanvas-2.so.0()(64bit)
   libgnomeui-2.so.0()(64bit)
   libgnomevfs-2.so.0()(64bit)
   libgobject-2.0.so.0()(64bit)
   libgthread-2.0.so.0()(64bit)
   libgtk-x11-2.0.so.0()(64bit)
   libncurses.so.5()(64bit)
   libpango-1.0.so.0()(64bit)
   libpangocairo-1.0.so.0()(64bit)
   libpopt.so.0()(64bit)
   libski-1.3.so.2()(64bit)
   libxml2.so.2()(64bit)
   ski-lib = 1.3.2-2.fc9

  ski-devel-1.3.2-2.fc9.x86_64.rpm
   ski-devel = 1.3.2-2.fc9
  =
   /bin/sh
   libski-1.3.so.2()(64bit)
      ski-lib = 1.3.2-2.fc9

  ski-lib-1.3.2-2.fc9.x86_64.rpm
   libski-1.3.so.2()(64bit)
   ski-lib = 1.3.2-2.fc9
  =
   /sbin/ldconfig
   libXm.so.2()(64bit)
   libXt.so.6()(64bit)
   libelf.so.1()(64bit)
   libelf.so.1(ELFUTILS_1.0)(64bit)
   libglade-2.0.so.0()(64bit)
   libglib-2.0.so.0()(64bit)
   libgobject-2.0.so.0()(64bit)
   libgtk-x11-2.0.so.0()(64bit)
   libncurses.so.5()(64bit)
   libski-1.3.so.2()(64bit)

? %check is not present, but there's some sort of test suite included.
* Manual testing shows that things work at least partially, although I don't
  really know how to test this.
* shared libraries added; ldconfig is called properly.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files (besides the COPYING file)
* file permissions are appropriate.
* scriptlets are OK (ldconfig).
* code, not content.
* documentation is small, so no -doc subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* headers are in the -devel package.
* no pkgconfig files.
* no static libraries.
* no libtool .la files.
Comment 10 Dan Horák 2008-04-09 05:26:34 EDT
(In reply to comment #9)
> This is looking good.
> 
> You can get dir of the unused-direct-shlib-dependency complaints with another
libtool hack:
>   sed -i -e 's! -shared ! -Wl,--as-needed\0!g' libtool
> Since you're already hacking up libtool a bit, there's probably no reason not
to do that as well (unless it breaks something, I guess).
> 

Thanks for the libtool trick, it is included and nothing looks broken.

> The remaining rpmlint complaints are:
> 
>   ski-devel.x86_64: W: no-documentation
> Not a problem.
> 
>   ski-devel.x86_64: W: no-dependency-on ski
> The -devel package depends on the -lib package, but not the main package. 
This also is not a problem.
> 
> Most packages seem to use "-libs" for the library subpackage, but there are a
> few who use "-lib" and we don't seem to have any guidelines about it, so
> that's OK.
> 

Oh, I surely wanted to use -libs ;-)

> There's a test suite in the "testsuite" directory; is this something which
> could be run at build time?

It doesn't  look like it is easily runnable and even it doesn't test something
much useful.

> 
> Note that there's no need to include the COPYING twice.  Just having it in the
> main package is sufficient, even if it's possible to install the software
> without installing that package.  This isn't a blocker, though.

I would like to include both of them.


* Wed Apr  9 2008 Dan Horak <dan[at]danny.cz> 1.3.2-3
- fix linking issues
- use -libs for the subpackage

Spec URL: http://fedora.danny.cz/ski.spec
SRPM URL: http://fedora.danny.cz/ski-1.3.2-3.fc9.src.rpm
Comment 11 Dan Horák 2008-04-10 05:07:06 EDT
* Thu Apr 10 2008 Dan Horak <dan[at]danny.cz> 1.3.2-4
- fix build on ppc

Spec URL: http://fedora.danny.cz/ski.spec
SRPM URL: http://fedora.danny.cz/ski-1.3.2-4.fc9.src.rpm
Comment 12 Jason Tibbitts 2008-04-10 11:35:29 EDT
Sorry, for some reason I thought I had already approved this.

APPROVED
Comment 13 Dan Horák 2008-04-11 03:01:57 EDT
New Package CVS Request
=======================
Package Name: ski
Short Description: IA-64 user and system mode simulator
Owners: sharkcz
Branches: F-8
InitialCC: 
Cvsextras Commits: yes
Comment 14 Kevin Fenzi 2008-04-12 17:45:20 EDT
cvs done.
Comment 15 Dan Horák 2008-04-13 04:34:18 EDT
imported and built

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