Bug 483250 - Review Request: chordii - Print songbooks (lyrics + chords)
Summary: Review Request: chordii - Print songbooks (lyrics + chords)
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Hans de Goede
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
: 483108 (view as bug list)
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-01-30 14:36 UTC by Johan Vromans
Modified: 2010-01-07 21:46 UTC (History)
3 users (show)

Fixed In Version: 4.3-1.fc12
Clone Of:
Environment:
Last Closed: 2010-01-04 11:26:50 UTC
Type: ---
Embargoed:
hdegoede: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Johan Vromans 2009-01-30 14:36:44 UTC
Spec URL: http://www.squirrel.nl/pub/xfer/chordii.spec
SRPM URL: http://www.squirrel.nl/pub/xfer/chordii-4.2-2.src.rpm
Description: 
chordii provides guitar players and other musicians with a tool to
produce good looking, self-descriptive music sheets from text files.

chordii reads text files in chordpro format, containing the lyrics of
songs, the chords to be played, their descriptions and some other
optional data. It produces a PostScript document suitable for viewing
and printing.

chordii features include:
 - centered titles,
 - chord names above the words,
 - chord diagrams at the end of the songs,
 - multiple columns on a page,
 - multiple logical pages per physical pages (1, 2 or 4),
 - configurable fonts and sizes,
 - the complete ISO 8859-1 character set,
 - chorus marking,
 - automated chord transposition,
 - songbook creation: typeset multiple songs with page numbers and a
   table of contents.

This is my first package, so I'm looking for a sponsor.
The spec and srpm are rpmlint free of warnings and errors.

Comment 1 Fabian Affolter 2009-01-30 15:04:04 UTC
After a quick look at your spec file, some comments.  

- Use '%defattr(-,root,root,-)' instead of '%defattr(-, root, root, 0755)'
- The 'URL:' should point to the upstream website.  The 'Source0' to the upstream tarball.  Move 'URL:' to 'Source'
  https://fedoraproject.org/wiki/Packaging/SourceURL
- It would be nice if you are going to use the dist tag -> 'Release:  2%{?dist}'
- Change 'examples' to 'examples/' to add all example files.

To get sponsored -> https://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored

Comment 2 Parag AN(पराग) 2009-01-30 15:24:53 UTC
Make sure which bug to mark duplicate I see this package submitted by submitter twice. see bug483108

Comment 3 Johan Vromans 2009-01-30 15:32:49 UTC
*** Bug 483108 has been marked as a duplicate of this bug. ***

Comment 4 Johan Vromans 2009-01-30 16:03:05 UTC
@Fabian: Thanks for your kind and constructive feedback.
I've made the suggested changes to the spec.

I documented in %changelog that I changed %defattr, and now rpmlint complains about 'macro-in-%changelog'. I assume this can be safely ignored.

New files have been uploaded:
Spec URL: http://www.squirrel.nl/pub/xfer/chordii.spec
SRPM URL: http://www.squirrel.nl/pub/xfer/chordii-4.2-3.fc11.src.rpm

Last build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1094024

Thanks for your patience.

Comment 5 manuel wolfshant 2009-01-30 16:10:26 UTC
Johan, use %% in the changelog in order to silence rpmlint. Otherwise the entry is expanded as a macro.

Comment 6 Johan Vromans 2009-01-30 17:33:55 UTC
@manuel: thanks! Fixed in a future version.

Comment 7 Fabian Affolter 2009-02-10 11:00:37 UTC
I will make a full review soon but I can't sponsor you.  You should make some informal reviews and/or submit more packages and add a note here ti find a sponsor.

Comment 8 Fabian Affolter 2009-03-19 08:07:09 UTC
Did you make some informal review on other packages?

https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group

Comment 9 Fabian Affolter 2009-03-25 21:08:41 UTC
Have you made an other package?

Comment 10 Johan Vromans 2009-03-25 21:53:28 UTC
I have two more packages under review:
https://bugzilla.redhat.com/show_bug.cgi?id=483364
https://bugzilla.redhat.com/show_bug.cgi?id=483406

Comment 11 Johan Vromans 2009-03-25 21:54:18 UTC
Make that three :)
https://bugzilla.redhat.com/show_bug.cgi?id=483286

Comment 12 Fabian Affolter 2009-03-25 22:40:38 UTC
Package Review
==============

Package: 

Key:
 - = N/A
 x = Check
 ! = Problem
 ? = Not evaluated

=== REQUIRED ITEMS ===
 [x] Package is named according to the Package Naming Guidelines.
 [x] Spec file name must match the base package %{name}, in the format
%{name}.spec.
 [x] Package meets the Packaging Guidelines
 [x] Package successfully compiles and builds into binary RPMs on at least one
supported architecture.
     Tested on: F10/i386
 [!] Rpmlint output:
     Source RPM: [1]
     [fab@laptop24 SRPMS]$ rpmlint chordii-4.2-3.fc11.src.rpm 
     chordii.src:61: W: macro-in-%changelog defattr
     1 packages and 0 specfiles checked; 0 errors, 1 warnings.
     Binary RPM(s):
     [fab@laptop24 i386]$ rpmlint chordii*
     2 packages and 0 specfiles checked; 0 errors, 0 warnings.
 [x] Package is not relocatable.
 [x] Buildroot is correct
     master   : %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
     spec file: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
 [x] Package is licensed with an open-source compatible license and meets other
legal requirements as defined in the legal section of Packaging Guidelines.
 [x] License field in the package spec file matches the actual license.
     License type: GPLv3+
 [x] If (and only if) the source package includes the text of the license(s) in
its own file, then that file, containing the text of the license(s) for the
package is included in %doc.

 [x] Spec file is legible and written in American English.
 [x] Sources used to build the package matches the upstream source, as provided
in the spec URL.
     Upstream source: 397894d724dbf7beebee05d8cacd93a6
     Build source:    397894d724dbf7beebee05d8cacd93a6
 [x] Package is not known to require ExcludeArch.
 [x] All build dependencies are listed in BuildRequires, except for any that
are listed in the exceptions section of Packaging Guidelines.
 [-] The spec file handles locales properly.  %find_lang used for locales.
 [x] %{optflags} or RPM_OPT_FLAGS are honoured.
 [-] ldconfig called in %post and %postun if required.
 [x] %install starts with rm -rf %{buildroot} or $RPM_BUILD_ROOT.
 [x] Package must own all directories that it creates.
 [x] Package requires other packages for directories it uses.
 [x] Package does not own files or directories owned by other packages.
 [x] Package does not contain duplicates in %files.
 [x] Permissions on files are set properly. %defattr(-,root,root,-) is in every
%files section.
 [x] Package uses nothing in %doc for runtime.
 [x] Package has a %clean section, which contains rm -rf %{buildroot} or
$RPM_BUILD_ROOT.
 [x] Package consistently uses macros.
 [x] Package contains code, or permissable content.

 [-] Large documentation files are in a -doc subpackage, if required.
 [-] Header files in -devel subpackage, if present.
 [-] Fully versioned dependency in subpackage, if present.
 [-] Static libraries in -devel subpackage, if present.
 [-] Package requires pkgconfig, if .pc files are present.
 [-] Development .so files in -devel subpackage, if present.
 [-] Package does not contain any libtool archives (.la).
 [x] -debuginfo subpackage is present and looks complete.
 [-] Package contains a properly installed %{name}.desktop file if it is a GUI
application.

=== SUGGESTED ITEMS ===
 [x] Timestamps preserved with cp and install.
 [x] Uses parallel make (%{?_smp_mflags})
 [x] Latest version is packaged.
 [-] Package does not include license text files separate from upstream.
 [-] Description and summary sections in the package spec file contains
translations for supported Non-English languages, if available.
 [x] Reviewer should test that the package builds in mock.
     Tested on: F10/i386
 [x] Package should compile and build into binary RPMs on all supported
architectures.
     Tested:  http://koji.fedoraproject.org/koji/taskinfo?taskID=1258859
 [x] Package functions as described.
 [-] Scriptlets must be sane, if used.
 [-] The placement of pkgconfig(.pc) files is correct.
 [-] File based requires are sane.
 [x] Changelog in allowed format

=== ISSUES ===

[1] Comment #5 is already about the rpmlint warning

It would be nice to have one blank line between the changelog entries

Comment 13 Hans de Goede 2009-04-03 11:16:03 UTC
Hi Johan,

As discussed by mail already, I'll review your current 4 package submissions,
and once they are all approved, I'll sponsor you.

I've done a full review of this package and I fully agree with Fabian,
please change the %'s in the changelog to %% and add a whiteline between the different release entries, iow change this:

* Fri Jan 30 2009 Johan Vromans <jvromans> - 4.2-3
- Fixed URL and Source urls
- Added %{?dist} to Release
- Fixed missing (optional) argument to %defattr
- Changed examples to examples/* to include all examples
* Fri Jan 30 2009 Johan Vromans <jvromans> - 4.2-2
- Update description
- Add patch to fix Makefiles to avoid double install of manual pages
- use smp_mflags
- use install -p
* Thu Jan 29 2009 Johan Vromans <jvromans> - 4.2-1
- First Fedora version

To:

* Fri Jan 30 2009 Johan Vromans <jvromans> - 4.2-3
- Fixed URL and Source urls
- Added %%{?dist} to Release
- Fixed missing (optional) argument to %%defattr
- Changed examples to examples/* to include all examples

* Fri Jan 30 2009 Johan Vromans <jvromans> - 4.2-2
- Update description
- Add patch to fix Makefiles to avoid double install of manual pages
- use smp_mflags
- use install -p

* Thu Jan 29 2009 Johan Vromans <jvromans> - 4.2-1
- First Fedora version

Otherwise its fine. Fabian, re-assigning to me since I'm going to
sponsor Johan, and thanks for your review!

Comment 14 Hans de Goede 2009-04-03 12:00:53 UTC
Oops,

I'm afraid that I've not done my review properly, trusting to much on Fabain's initial review.

I just checked the AUTHORS file and the licensing discussed in there is a big red flag. Quoting from the AUTHORS file:

"CHORD is licensed following the conditions of the general GNU license.
You are authorized to use this program free of charge. You are
authorized to distribute this program freely as long as the full
source is included. You are not allowed to remove the `copyright'
notices from the authors nor are you allowed to pretend you wrote it.
You are not allowed to charge any money for CHORD. You are not allowed
to distribute a modified version of CHORD without written
authorizations from the authors. You are not allowed to use parts of
CHORD in any other commercial or public-domain software. Sorry for all
the negatives rules ... but we've been bitten once!"

The GPL of course is fine, however the additional non GPL compatible rules
like no selling or not acceptable for Fedora. I notice that you are the new upstream, and that you've written:

"CHORD is originally written by Martin Leclerc and Mario Dorion.
Unfortunately, they have given up interest and disappeared from the
internet."

I hope that that is not entirely true, and with some hunting you can still
contact them and get them to wave the "extra rules" they've added on top of
the GPL, otherwise this program cannot be part of Fedora.

---

As discussed already, I would review your current 4 package submissions,
and once they are all approved, I would sponsor you. However this package looks
like it will hit serious license troubles, so I would like to change
the deal (in your benefit) to the following: as soon as the other 3 packages are approved I'll sponsor you.

Comment 15 Hans de Goede 2009-04-03 12:04:29 UTC
Hmm,

I see in the README that you've already tried to contact the old upstream people
several times and failed. Let me run this past our legal stuff vetting person.

Comment 16 Fabian Affolter 2009-04-03 21:50:58 UTC
Hans, thanks for checking my review.  So far I wasn't aware of that there are developers out there who put copyright information in the AUTHORS files.

Comment 17 Johan Vromans 2009-04-04 19:03:40 UTC
It seems that both original authors are back on the net since a couple of months! I'll contact them and see what to do.

Comment 18 Tom "spot" Callaway 2009-04-21 17:32:47 UTC
As Hans says, all of those additional restrictions on top of the GPL are horribly non-free. If the old upstream (the original copyright holders) will waive all of those additional restrictions, you should be okay, but if not, this is not acceptable in Fedora as-is.

Comment 19 Tom "spot" Callaway 2009-12-01 01:05:50 UTC
Any update here?

Comment 20 Johan Vromans 2009-12-01 08:52:08 UTC
In a way, yes.
I managed to contact one of the original authors and he promised to make the necessary changes.
That was several weeks ago but I didn't get a response since.
I'll try again.

Comment 21 Johan Vromans 2009-12-04 22:05:21 UTC
I now have a decent and positive response:

----snip----
From: Mario Dorion <mdorion>
To: Johan Vromans <jvromans>
CC: Martin Leclerc <martin.a.leclerc>
Subject: Re: Chord license
Date: Thu, 03 Dec 2009 14:07:47 -0500
Content-Transfer-Encoding: 7BIT

Hi Johan,

I apologize for the long delay in responding. I guess I had issues with "letting Chord go".

Martin and I are happy to see the work you've been doing with Chordii and are enthusiastic about changing the original licensing so that Chordii can be bundled with major Linux distros.

Do you require anything special to make this official? Do you want a signed letter? Is this email good enough?
----snip---

The intent is clear. Now what do they have to do to make it official?

Comment 22 Tom "spot" Callaway 2009-12-05 15:07:27 UTC
Well, the best thing would be for them to do a new tarball release which has the following in their AUTHORS file:

"CHORD is licensed following the conditions of the GNU Public License, version 2 or later. See COPYING for the full license text."

(All of the other wording about licensing should be replaced with the above sentences.)

In addition, COPYING should be a copy of the GPL v2, and should be present in the tarball.

If they don't want to do that for some reason, we would need an email from both Martin and Mario which includes the new license sentences (as above). We should save copies of those emails as text files, and package them as %doc.

Comment 23 Johan Vromans 2009-12-30 22:03:54 UTC
A new Chord release has been issued. This version, 3.6.4, is GPL without constraints and blessed by the original authors.
Chordii has been rebased on this version, so there should be no legal problems anymore.

So I herewith reopen this review request.

The latest koji build can be inspected here (for a little while): http://koji.fedoraproject.org/koji/taskinfo?taskID=1896258

Thanks for your patience!

Comment 24 Tom "spot" Callaway 2009-12-30 22:10:11 UTC
Lifting FE-Legal.

Comment 25 Hans de Goede 2009-12-31 09:43:52 UTC
(In reply to comment #23)
> Thanks for your patience!  

Thanks for your perseverance! Sorting out licensing issues like this
always takes a long breath, so thank you for doing this.

Given that the Legal issue was the one and only blocker: Approved!

Comment 26 Johan Vromans 2009-12-31 13:25:47 UTC
New Package CVS Request
=======================
Package Name: chordii
Short Description: Utility to print songsbooks (lyrics + chords)
Owners: sciurius
Branches: F-11 F-12
InitialCC:

Comment 27 Kevin Fenzi 2010-01-02 20:16:06 UTC
cvs done.

Comment 28 Fedora Update System 2010-01-03 21:56:53 UTC
chordii-4.3-1.fc12 has been submitted as an update for Fedora 12.
http://admin.fedoraproject.org/updates/chordii-4.3-1.fc12

Comment 29 Fedora Update System 2010-01-03 21:57:01 UTC
chordii-4.3-1.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/chordii-4.3-1.fc11

Comment 30 Hans de Goede 2010-01-04 11:26:50 UTC
Closing this, as this has been imported and build now (just some bookkeeping so that this drops of my bugs needing attention list).

Comment 31 Fedora Update System 2010-01-07 21:43:53 UTC
chordii-4.3-1.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 32 Fedora Update System 2010-01-07 21:46:27 UTC
chordii-4.3-1.fc12 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.


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