Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 219653 - Review Request: solfege - music education software
Summary: Review Request: solfege - music education software
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Mamoru TASAKA
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-12-14 17:07 UTC by Sindre Pedersen Bjørdal
Modified: 2007-11-30 22:11 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-12-26 08:59:04 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Sindre Pedersen Bjørdal 2006-12-14 17:07:50 UTC
Spec URL: http://folk.ntnu.no/sindrb/packages/solfege.spec
SRPM URL: http://folk.ntnu.no/sindrb/packages/solfege-3.6.4-1.src.rpm

Description:

Solfege is free music education software. Use it to train your rhythm, 
interval, scale and chord skills. Solfege - Smarten your ears!

Note: Solfege relies on esound for large parts of it's functionality. To ensure that esd is running I've created a simple wrapper script. The wrapper script uses a pid file instead of the -terminate option of esd due to Solfege breaking the connection to esd and as such, esd would stop. The goal of the wrapper script is to ensure that Solfege "just works". This is a hack and if someone has a better solution to make Solfege "just work" please provide it.

Comment 1 Sindre Pedersen Bjørdal 2006-12-14 17:44:42 UTC
Changes:

- Use install to install wrapper script
- Improvements to wrapper script

Updated: 

Spec URL: http://folk.ntnu.no/sindrb/packages/solfege.spec
SRPM URL: http://folk.ntnu.no/sindrb/packages/solfege-3.6.4-2.src.rpm

Comment 2 Jima 2006-12-14 17:52:18 UTC
Not sure I should be doing the review, as you're implementing some of my ideas
already, but I'll clear up a few simple points.

rpmlint produces output for the RPMs.

SRPM:
W: solfege strange-permission solfege.sh.in 0755
W: solfege mixed-use-of-spaces-and-tabs (spaces: line 6, tab: line 4)

 For the first, `chmod -x olfege.sh.in` -- the script doesn't need to be
executable in the SRPM.
 The second isn't particularly critical, just a matter of presentation. Pretty
easy to fix so you don't have any warnings, though.

the main RPM:
W: solfege unstripped-binary-or-object /usr/lib/solfege/_solfege_c_midi.so
E: solfege non-executable-script /usr/share/solfege/mpd/testsetup.py 0644
E: solfege non-executable-script /usr/share/solfege/src/testsetup.py 0644

 No idea what's going on there.

debuginfo RPM:
E: solfege-debuginfo empty-debuginfo-package

 I'm a bit confused by this; it looks like the binaries may have gotten stripped
by the `make install` or something.

Comment 3 Mamoru TASAKA 2006-12-14 18:23:01 UTC
Just a comment:

W: solfege unstripped-binary-or-object /usr/lib/solfege/_solfege_c_midi.so
E: solfege non-executable-script /usr/share/solfege/mpd/testsetup.py 0644
E: solfege non-executable-script /usr/share/solfege/src/testsetup.py 0644
E: solfege-debuginfo empty-debuginfo-package

Perhaps all permission issues.
* *.py python script should have the permission 0755
* python module (_solfege_c_midi.so) should have the permission 0644.
  This is also related to debuginfo issue. Binaries are not stripped
  unless they have executable permission flags.

Comment 4 Mamoru TASAKA 2006-12-14 18:25:25 UTC
(In reply to comment #3)

> * python module (_solfege_c_midi.so) should have the permission 0644.
Oops, I wanted to say this should be 0755, not 0644.


Comment 5 Sindre Pedersen Bjørdal 2006-12-15 00:58:09 UTC
Tried to fix the permissions. There are still problems, solutions welcome.

SRPM: 
W: solfege strange-permission solfege.sh.in 0755
W: solfege mixed-use-of-spaces-and-tabs (spaces: line 6, tab: line 4)

RPM:
W: solfege unstripped-binary-or-object
/usr/lib/debug/usr/lib/solfege/_solfege_c_midi.so.debug
E: solfege shared-lib-without-dependency-information
/usr/lib/debug/usr/lib/solfege/_solfege_c_midi.so.debug

Updated: 

Spec URL: http://folk.ntnu.no/sindrb/packages/solfege.spec
SRPM URL: http://folk.ntnu.no/sindrb/packages/solfege-3.6.4-3.src.rpm

Comment 6 Mamoru TASAKA 2006-12-15 03:15:04 UTC
Again just a comment:

W: solfege strange-permission solfege.sh.in 0755
   - permission should be 0644
W: solfege mixed-use-of-spaces-and-tabs (spaces: line 6, tab: line 4)
   - Indentation in spec file should be spaces _or_ tabs and using
     _both_ is not considered good for cosmetic annoyance.

W: solfege unstripped-binary-or-object
/usr/lib/debug/usr/lib/solfege/_solfege_c_midi.so.debug
E: solfege shared-lib-without-dependency-information
/usr/lib/debug/usr/lib/solfege/_solfege_c_midi.so.debug
   - This is because normal binary rpm contain debug information
     binary.
     You should not write:
-----------------------------------------
%files
.......   
%{_libdir}/*
------------------------------------------
     because this includes %{_libdir}/debug, this is wrong.

Comment 7 Sindre Pedersen Bjørdal 2006-12-15 06:35:17 UTC
Got rid of all the rpmlint errors and warnings.

updated:

Spec URL: http://folk.ntnu.no/sindrb/packages/solfege.spec
SRPM URL: http://folk.ntnu.no/sindrb/packages/solfege-3.6.4-4.src.rpm

Comment 8 Gérard Milmeister 2006-12-15 20:34:25 UTC
Better use spaces instead of tabs, the formatting is messy.

Comment 9 Mamoru TASAKA 2006-12-16 16:29:04 UTC
Well, I will review this....

Comment 10 Mamoru TASAKA 2006-12-17 09:43:24 UTC
Well, this time I only checked for packaging issue and
not checked this package for its funtion.

A. From http://fedoraproject.org/wiki/Packaging/Guidelines :
* Filesystem Layout
  - Usually when a wrapper script for the original binary is
  needed, the original binary should be where user's path are
  not passed, which is usually %{_libexecdir}.

* Use rpmlint
  - rpmlint (for FC-devel) is not silent, for which please see
  below

* Requires
  - Well, python related dependencies are not checked
  automatically by rpmbuild process and then should be checked
  manually.

  -- For example, /usr/share/solfege/src/utils.py contails:
-----------------------------------------
import gtk
-----------------------------------------
     This means that this package (solfege) should need
     "Requires: pygtk2".
  -- And another example is that 
  /usr/share/solfege/soundcard/midifilesynth.py includes the line:
-----------------------------------------
    import win32api
-----------------------------------------
  However, what package provides win32api python module?
  (or can this file be ignored??)

* BuildRequires
  - libxslt-devel
  Please check whether this should be whether "libxslt-devel" or
  "libxslt", since it seems only /usr/bin/xsltproc is used and
  this package is included in libxslt, not -devel pacakge.

* Desktop files
--------------------------------------------
Categories=GNOME;Application;AudioVideo;Audio;Education;X-Fedora;
--------------------------------------------
  Categories "Application" "X-Fedora" is deprecated from
  desktop-file-utils >= 0.11 and these should be removed
  (this is also applied for FC5/6). For FC-devel, these
  causes rpmlint warnings/errors.

* Documentation
  - /usr/share/doc/solfege-3.6.4/{AUTHORS,README} contain
  non-UTF8 character(s). Please change the encodings of
  these files to UTF-8.

* Timestamps
  - This package (solfege) contains many image files and
  for these files timestamps should be kept to show correctly
  when these files are created. For these purpose:
1.
-----------------------------------------------
export INSTALL="%{__install} -c -p"
-----------------------------------------------
  is needed before
-----------------------------------------------
%configure --enable-docbook-stylesheet=%stylesheet
-----------------------------------------------
  line to pass INSTALL environment to Makefile.in

2. 
------------------------------------------------
%{__sed} -i.stamp -e 's|shutil\.copy|shutil.copy2|' tools/pcopy.py
------------------------------------------------
  is needed at %prep stage to keep timestamps on 
  image files.

B. From http://fedoraproject.org/wiki/Packaging/ReviewGuidelines :
   = this is okay.

Comment 11 Sindre Pedersen Bjørdal 2006-12-20 08:34:04 UTC
Changes:

- Move original binary to %{_libexecdir}
- Remove X-Fedora Category from meny entry
- Add pygtk2 Requires
- Replace libxlst-devel BuildRequires with libxlst
- Keep timestamps for image files 
- Convert AUTHORS and README from iso8859 to UTF-8 

Not sure if I've used the prefered method for converting from iso8859-15 to
UTF-8. Please correct me if I've messed this up.

Updated:

SPEC: http://folk.ntnu.no/sindrb/packages/solfege.spec
SRPM: http://folk.ntnu.no/sindrb/packages/solfege-3.6.4-5.src.rpm

Comment 12 Mamoru TASAKA 2006-12-20 18:33:45 UTC
Well,

* Use rpmlint
  - For src.rpm:
-----------------------------------------------------
W: solfege macro-in-%changelog _libexecdir
-----------------------------------------------------
   rpmbuild tries to expand the macro in %changelog and this
   should be avoilded. In %changelog, when you want to use percent,
   please use %%, i.e. 
-----------------------------------------------------
- Move original binary to %%{_libexecdir}
-----------------------------------------------------

* Desktop files
-----------------------------------------------------
Categories=GNOME;Application;AudioVideo;Audio;Education;
-----------------------------------------------------
  - Well, category "Application" is also deprecated and this
  should be removed. You can use:
-----------------------------------------------------
desktop-file-install --vendor fedora --delete-original	\
	--dir $RPM_BUILD_ROOT%{_datadir}/applications	\
	--remove-category Application \
	$RPM_BUILD_ROOT%{_datadir}/applications/%{name}.desktop
-----------------------------------------------------

* Documentation
-----------------------------------------------------
iconv -o $RPM_BUILD_DIR/%{name}-%{version}/AUTHORS -f ISO-8859-15 -t UTF-8
$RPM_BUILD_DIR/%{name}-%{version}/AUTHORS
iconv -o $RPM_BUILD_DIR/%{name}-%{version}/README -f ISO-8859-15 -t UTF-8
$RPM_BUILD_DIR/%{name}-%{version}/README
-----------------------------------------------------
  - Well, actually these remove AUTHORS and README. In fact rpmlint
  complaints as following.
-----------------------------------------------------
E: solfege zero-length /usr/share/doc/solfege-3.6.4/AUTHORS
E: solfege zero-length /usr/share/doc/solfege-3.6.4/README
-----------------------------------------------------
  And, $RPM_BUILD_DIR/%{name}-%{version} is not needed because the
  working directory at this stage is there. Usually:
-----------------------------------------------------
for f in AUTHORS README ; do
	iconv -f ISO-8859-15 -t UTF-8 $f > ${f}.tmp && \
		%{__mv} -f ${f}.tmp ${f} || \
		%{__rm} -f ${f}.tmp
done
-----------------------------------------------------

Comment 13 Sindre Pedersen Bjørdal 2006-12-20 21:38:30 UTC
Changes:

- Fix charset conversion
- Remove Application category from desktop file
- Fix changelog


Updated:

SPEC: http://folk.ntnu.no/sindrb/packages/solfege.spec
SRPM: http://folk.ntnu.no/sindrb/packages/solfege-3.6.4-6.src.rpm



Comment 14 Mamoru TASAKA 2006-12-21 05:33:59 UTC
Okay.

--------------------------------------------------
   This package (solfege) is APPROVED by me.


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