Bug 486760 - Review Request: mscore - Music Composition & Notation Software
Review Request: mscore - Music Composition & Notation Software
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Richard W.M. Jones
Fedora Extras Quality Assurance
:
Depends On: 483376
Blocks:
  Show dependency treegraph
 
Reported: 2009-02-21 16:58 EST by Orcan Ogetbil
Modified: 2014-02-04 07:53 EST (History)
4 users (show)

See Also:
Fixed In Version: 0.9.4-3.fc10.1
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-04-09 12:11:49 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
rjones: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Orcan Ogetbil 2009-02-21 16:58:43 EST
Spec URL: http://oget.fedorapeople.org/review/mscore.spec
SRPM URL: http://oget.fedorapeople.org/review/mscore-0.9.4-1.fc10.src.rpm
Description: 
MuseScore is a free cross platform WYSIWYG music notation program. Some
highlights:

    * WYSIWYG, notes are entered on a "virtual note sheet"
    * Unlimited number of staves
    * Up to four voices per staff
    * Easy and fast note entry with mouse, keyboard or MIDI
    * Integrated sequencer and FluidSynth software synthesizer
    * Import and export of MusicXML and Standard MIDI Files (SMF)
    * Available in 12 languages

Rpmlint:
mscore-mscore-fonts.x86_64: W: no-documentation                                               
mscore-mscore-fonts.x86_64: W: invalid-license GPL+ with exceptions

There's not much I can do for the first one. The second one is false alarm, because "GPL+ with exceptions" is listed in 
   http://fedoraproject.org/wiki/Licensing#Good_Licenses


This is a very nice piece of software.
Comment 1 Richard W.M. Jones 2009-03-06 10:46:13 EST
Taking for review.  Note dependency (bug 483376) still needs to
be reviewed by someone else.
Comment 2 Richard W.M. Jones 2009-03-06 11:39:58 EST
auto-buildrequires output:
BuildRequires: alsa-lib-devel = 1.0.19.3.fc11.x86_64
BuildRequires: baekmuk-ttf-batang-fonts = 2.2.20.fc11.noarch
BuildRequires: baekmuk-ttf-dotum-fonts = 2.2.20.fc11.noarch
BuildRequires: baekmuk-ttf-gulim-fonts = 2.2.20.fc11.noarch
BuildRequires: baekmuk-ttf-hline-fonts = 2.2.20.fc11.noarch
BuildRequires: binutils = 2.19.51.0.2.14.fc11.x86_64
BuildRequires: bzip2 = 1.0.5.4.fc11.x86_64
BuildRequires: ccache = 2.4.14.fc11.x86_64
BuildRequires: cjkuni-uming-fonts = 0.2.20080216.1.21.fc11.noarch
BuildRequires: cmake = 2.6.3.1.fc11.x86_64
BuildRequires: coreutils = 7.1.6.fc11.x86_64
BuildRequires: cpio = 2.9.90.4.fc11.x86_64
BuildRequires: dejavu-sans-fonts = 2.28.6.fc11.noarch
BuildRequires: dejavu-sans-mono-fonts = 2.28.6.fc11.noarch
BuildRequires: dejavu-serif-fonts = 2.28.6.fc11.noarch
BuildRequires: desktop-file-utils = 0.15.7.fc11.x86_64
BuildRequires: diffutils = 2.8.1.23.fc11.x86_64
BuildRequires: elfutils = 0.140.2.fc11.x86_64
BuildRequires: file = 5.00.4.fc11.x86_64
BuildRequires: findutils = 1:4.4.0.2.fc11.x86_64
BuildRequires: fluidsynth-devel = 1.0.8.3.fc11.x86_64
BuildRequires: fontconfig = 2.6.97.5.g945d6a4.fc11.x86_64
BuildRequires: fontforge = 20090224.1.fc11.x86_64
BuildRequires: gcc = 4.4.0.0.22.x86_64
BuildRequires: glibc-devel = 2.9.90.7.x86_64
BuildRequires: glibc-headers = 2.9.90.7.x86_64
BuildRequires: grep = 2.5.3.4.fc11.x86_64
BuildRequires: gzip = 1.3.12.8.fc11.x86_64
BuildRequires: jack-audio-connection-kit-devel = 0.116.1.4.fc11.x86_64
BuildRequires: kernel-headers = 2.6.29.0.203.rc7.fc11.x86_64
BuildRequires: libstdc++-devel = 4.4.0.0.22.x86_64
BuildRequires: make = 1:3.81.15.fc11.x86_64
BuildRequires: patch = 2.5.4.38.fc11.x86_64
BuildRequires: perl = 4:5.10.0.58.fc11.x86_64
BuildRequires: pkgconfig = 1:0.23.8.fc11.x86_64
BuildRequires: portaudio-devel = 19.8.fc11.x86_64
BuildRequires: qt-devel = 1:4.5.0.3.fc11.x86_64
BuildRequires: sazanami-gothic-fonts = 0.20040629.7.20061016.fc11.noarch
BuildRequires: sazanami-mincho-fonts = 0.20040629.7.20061016.fc11.noarch
BuildRequires: sed = 4.1.5.12.fc11.x86_64
BuildRequires: stix-fonts = 0.9.12.fc11.noarch
BuildRequires: tar = 2:1.21.2.fc11.x86_64
BuildRequires: texlive = 2007.41.fc11.x86_64
BuildRequires: thai-scalable-garuda-fonts = 0.4.11.2.fc11.noarch
BuildRequires: thai-scalable-kinnari-fonts = 0.4.11.2.fc11.noarch
BuildRequires: thai-scalable-umpush-fonts = 0.4.11.2.fc11.noarch
BuildRequires: vlgothic-fonts = 20090204.3.fc11.noarch
BuildRequires: zlib-devel = 1.2.3.20.fc11.x86_64
Comment 3 Richard W.M. Jones 2009-03-06 11:40:39 EST
rpmlint:

mscore-mscore-fonts.x86_64: W: no-documentation
mscore-mscore-fonts.x86_64: W: invalid-license GPL+ with exceptions
Comment 4 Richard W.M. Jones 2009-03-06 11:58:20 EST
Koji scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1227091
Comment 5 Richard W.M. Jones 2009-03-06 12:02:51 EST
You might want to have a look at this build log, because
there is some problem building one of the fonts:
http://koji.fedoraproject.org/koji/getfile?taskID=1227093&name=build.log
Comment 6 Richard W.M. Jones 2009-03-06 12:07:24 EST
As a general comment, these are the packages that auto-
buildrequires found, which don't seem to be mentioned in
your BuildRequires.  (But that doesn't necessarily mean they
all need to be listed explicitly, because they might be in the
Koji core packages, or found indirectly through other
packages - nevertheless, worth considering).

BuildRequires: baekmuk-ttf-batang-fonts = 2.2.20.fc11.noarch                    
BuildRequires: baekmuk-ttf-dotum-fonts = 2.2.20.fc11.noarch                     
BuildRequires: baekmuk-ttf-gulim-fonts = 2.2.20.fc11.noarch                     
BuildRequires: baekmuk-ttf-hline-fonts = 2.2.20.fc11.noarch                     
BuildRequires: cjkuni-uming-fonts = 0.2.20080216.1.21.fc11.noarch               
BuildRequires: dejavu-sans-fonts = 2.28.6.fc11.noarch                           
BuildRequires: dejavu-sans-mono-fonts = 2.28.6.fc11.noarch                      
BuildRequires: dejavu-serif-fonts = 2.28.6.fc11.noarch                          
BuildRequires: fontconfig = 2.6.97.5.g945d6a4.fc11.x86_64                       
BuildRequires: libstdc++-devel = 4.4.0.0.22.x86_64                              
BuildRequires: perl = 4:5.10.0.58.fc11.x86_64                                   
BuildRequires: pkgconfig = 1:0.23.8.fc11.x86_64                                 
BuildRequires: sazanami-gothic-fonts = 0.20040629.7.20061016.fc11.noarch        
BuildRequires: sazanami-mincho-fonts = 0.20040629.7.20061016.fc11.noarch        
BuildRequires: stix-fonts = 0.9.12.fc11.noarch                                  
BuildRequires: thai-scalable-garuda-fonts = 0.4.11.2.fc11.noarch                
BuildRequires: thai-scalable-kinnari-fonts = 0.4.11.2.fc11.noarch               
BuildRequires: thai-scalable-umpush-fonts = 0.4.11.2.fc11.noarch                
BuildRequires: vlgothic-fonts = 20090204.3.fc11.noarch
Comment 7 Richard W.M. Jones 2009-03-06 12:20:21 EST
Here is my partial review:

- rpmlint output

See comment 2.  These can both be fixed.

? package name satisfies the packaging naming guidelines

I may be missing something, but why is this package not called
"musescore" to match upstream?  The problem with the current
name is that it's confusing with names like "mscorefonts" (the
MS Core Fonts package found in some distros like Debian).

+ specfile name matches the package base name
+ package should satisfy packaging guidelines
+ license meets guidelines and is acceptable to Fedora
+ license matches the actual package license

Very complex licensing situation, but the packager seems to have
done a good job resolving the different licenses involved.

+ %doc includes license file
+ spec file written in American English
+ spec file is legible
+ upstream sources match sources in the srpm

I unpacked the sources and the upstream tarball and compared them
with 'diff -urN' and the only difference is the files that the
packager has removed because of licensing problems.

+ package successfully builds on at least one architecture

On x86_64.

n/a ExcludeArch bugs filed
- BuildRequires list all build dependencies

Fails to build in Koji.

? %find_lang instead of %{_datadir}/locale/*

This doesn't use ordinary locale files, but some sort of Qt thing.
Can the packager point to any guidance on how to package these?

n/a binary RPM with shared library files must call ldconfig in %post and %postun
n/a does not use Prefix: /usr
+ package owns all directories it creates
+ no duplicate files in %files
+ %defattr line
+ %clean contains rm -rf $RPM_BUILD_ROOT
+ consistent use of macros
+ package must contain code or permissible content
+ large documentation files should go in -doc subpackage
+ files marked %doc should not affect package
n/a header files should be in -devel
n/a static libraries should be in -static
n/a packages containing pkgconfig (.pc) files need 'Requires: pkgconfig'
n/a libfoo.so must go in -devel
n/a -devel must require the fully versioned base
n/a packages should not contain libtool .la files
+ packages containing GUI apps must include %{name}.desktop file
+ packages must not own files or directories owned by other packages
+ %install must start with rm -rf %{buildroot} etc.
+ filenames must be valid UTF-8

Optional:

n/a if there is no license file, packager should query upstream
? translations of description and summary for non-English languages, if available
- reviewer should build the package in mock
? the package should build into binary RPMs on all supported architectures
- review should test the package functions as described
+ scriptlets should be sane
n/a pkgconfig files should go in -devel
+ shouldn't have file dependencies outside /etc /bin /sbin /usr/bin or /usr/sbin
Comment 8 Orcan Ogetbil 2009-03-06 12:55:15 EST
Many thanks for the comments.

The build failure is due to some changes in the TeX system of rawhide. It may be a TeX bug. I'm currently investigating the situation.

The package builds on mock and runs fine on F-10.

As for the AutoBuildRequires output: Those fonts are definitely not all required. I believe that the other ones are optional checks of the mscore configuration script.

This package is named mscore in Debian, Mandriva and Ubuntu. It is based on the tarball name I suppose.

I'll go over the other questions/comments you made. But let me solve this rawhide build issue first.
Comment 9 Orcan Ogetbil 2009-03-06 16:43:37 EST
I found the problem. Since the fonts are packaged separately according to the new guidelines, I had to BR: tex-cm-lgc on Fedora 11+. Now the package builds in rawhide:
   http://koji.fedoraproject.org/koji/taskinfo?taskID=1227743

Pure Qt applications have their own way of handling locale files. AFAIK there is no guideline specific for them. Off the top of my head, I remember that muse and esperanza packages doing it the same way. esperanza even gets the translations compiled into the final executable binary. 

By the way, I also updated the icon scriptlets according to the new guidelines:
   https://www.redhat.com/archives/fedora-devel-list/2009-February/msg01604.html
   https://fedoraproject.org/wiki/PackagingDrafts/Icon_Cache  

Here are the updated files:

Spec URL: http://oget.fedorapeople.org/review/mscore.spec
SRPM URL: http://oget.fedorapeople.org/review/mscore-0.9.4-1.fc10.src.rpm

Changelog: 0.9.4-2
- Add extra BR:tex-cm-lgc for F-11+. This is necessary to build the fonts from source
- Update icon scriptlets according to the new guidelines


Please let me know if there's anything I missed.
Comment 10 Orcan Ogetbil 2009-03-06 16:44:18 EST
Sorry, the SRPM should be:

SRPM URL: http://oget.fedorapeople.org/review/mscore-0.9.4-2.fc10.src.rpm
Comment 11 Richard W.M. Jones 2009-03-06 19:05:41 EST
This continues the review from comment 7:

+ rpmlint output

I'll let the rpmlint warnings go because this uses the %_font_pkg
macro, so I assume that rpmlint is wrong and the macro is right.

+ package name satisfies the packaging naming guidelines

Consistent with Debian package name.

+ BuildRequires list all build dependencies

Fixed, now builds in Koji.

n/a %find_lang instead of %{_datadir}/locale/*

Qt locale, guidelines doesn't apply.

+ reviewer should build the package in mock

Package now built in Koji.

------------------------------------------------

This package is APPROVED by rjones.
Comment 12 Orcan Ogetbil 2009-03-06 20:14:41 EST
Thanks Rich for the review. If you want, you can also review the autoBR package.

If you have no objections, I am going to add a 
Provides: musescore = %{name}-%{version} to this package.

New Package CVS Request
=======================
Package Name: mscore
Short Description: Music Composition & Notation Software
Owners: oget
Branches: F-10
InitialCC:
Comment 13 Orcan Ogetbil 2009-03-06 20:16:16 EST
note that I'm not going to commit this until fluid-soundfont is approved.
Comment 14 Richard W.M. Jones 2009-03-07 09:19:47 EST
Adding the provides is fine.
Comment 15 Kevin Fenzi 2009-03-07 12:22:30 EST
cvs done.
Comment 16 Fedora Update System 2009-03-23 23:45:27 EDT
mscore-0.9.4-3.fc10.1 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/mscore-0.9.4-3.fc10.1
Comment 17 Fedora Update System 2009-03-25 12:03:33 EDT
mscore-0.9.4-3.fc10.1 has been pushed to the Fedora 10 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update mscore'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-3000
Comment 18 Fedora Update System 2009-04-09 12:11:44 EDT
mscore-0.9.4-3.fc10.1 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 19 Christopher Meng 2014-02-03 21:56:22 EST
Approved by the current maintainer Brendon Jones.

Package Change Request
======================
Package Name: mscore
New Branches: el6 epel7
Owners: cicku
Comment 20 Gwyn Ciesla 2014-02-04 07:53:29 EST
Git done (by process-git-requests).

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