Bug 1250833

Scratch builds available on cop https://copr.fedoraproject.org/coprs/baggypants/helm-stable/

This is my first request and I need a sponsor.

I'm not a sponsor, but I will give you some notes.

-> Source0: https://github.com/mtytel/helm/archive/v0.4.1.zip
   Source0: https://github.com/mtytel/helm/archive/v%{version}.tar.gz#/%{name}-%{version}.tar.gz
   Look at: https://fedoraproject.org/wiki/Packaging:SourceURL#Git_Tags
   You also can use %{url} instead of first part in Source0 tag

-> Requires:	mesa-libGL, alsa-lib, jack-audio-connection-kit, freetype, libXrandr, libXinerama, libXcursor, helm-common
   Please make it separate (I mean in some lines, not it one.
   I don't think that most of them is needed (I'm sure that RPM will pick up 90% of them automatically)

-> Requires: helm-common
   you should specify version at least. Example:
   Requires: helm-common = %{version}

-> For common subpkg please build noarch version.

-> $RPM_BUILD_ROOT
   replace with %{buildroot}

-> make %{?_smp_mflags}
   you could use %make_build

-> "%_libdir/lv2/helm.lv2/helm.so"
   don't need to use quotes there

-> Also i think you just want to have there just: %{_libdir}/lv2/helm.lv2/
   don't need to specify files manually
   The same for common subpackage

-> %doc COPYING
   should be %license COPYING

-> "%{_bindir}/helm"
   again, no quotes and try to use %{name} where it's possible

-> %setup -q 
   You could use %autosetup

-> License: GPL3
   GPL3 is not valid license. You'd want GPLv3 or GPLv3+ (haven't checked sources)

-> I've not checked desktop file, but looks like no icons in hicolor theme, only in /usr/share/helm. So if you want to use icons there -- use %{_datadir}/icons/hicolor/* as place for icons

Excellent review, really helpful. I'm pretty sure I did everything. Regarding the icon, it was in pixmaps but now it's in icons/hicolor/

New version
http://therobinsonfamily.net/SRPMS/helm-0.4.1-4.fc22.src.rpm

http://therobinsonfamily.net/SPECS/helm.spec

New version
http://therobinsonfamily.net/SRPMS/helm-0.4.1-6.fc22.src.rpm
http://therobinsonfamily.net/SPECS/helm.spec

Updated due to failed armv7hl builds

Koji build
http://koji.fedoraproject.org/koji/taskinfo?taskID=10784820

Please fill in your full real name in your bugzilla account preferences.


Consider pointing the fedora-review tool at this ticket:

  fedora-review -b 1250833

It downloads the latest spec file and src.rpm from the "Spec URL:" and "SRPM URL:" lines (or additional URLs it finds) and performs many checks that are relevant during review and should be most interesting to the package maintainer.


> -> Requires: helm-common
>    you should specify version at least. Example:
>    Requires: helm-common = %{version}

Omitting -%{release} serves no purpose. Actually, -common subpackages are disguised base packages, and this one applies except the %{?_isa}:

  https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package


> %doc

%doc is not a section but an attribute. Empty %doc lines are no-op. It's more cleaner to delete them.


> %package lv2
> Summary:	Helm lv2 plugin
> Group:		Applications/Multimedia
> Requires: 	%{name}-common = %{version}
> %description lv2
> Helm is a polyphonic software synth with lots of modulation and and easy to use UT

The %description ought to explain what this particular package does. What's the relevance of LV2, for example? Why is the %description longer than the %description of the base package?

> %description
> Helm is a software synth designed to be easy to use

The subpackage is missing an explicit dependency on "lv2". Note that since LV2 is a library-less API, there is no automatic dependency on it, but package "lv2" is the one that provides the ownership of %_libdir/lv2.


> %files lv2
> %{_libdir}/lv2/helm.lv2/

> %package lv2

That creates a subpackage helm-lv2. Instead, I think the general add-on package naming guidelines apply:

  https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Addon_Packages_.28General.29

It is a plugin to LV2. It extends LV2.

%package -n lv2-helm


> https://kojipkgs.fedoraproject.org//work/tasks/4821/10784821/build.log

Build output is non-verbose. One cannot see which compiler and linker flags have been used actually.

https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags

new versions

Spec URL: http://therobinsonfamily.net/SPECS/helm.spec
SRPM URL: http://therobinsonfamily.net/SRPMS/helm-0.4.1-7.fc22.src.rpm

Build info: http://koji.fedoraproject.org/koji/taskinfo?taskID=10898667

Everything done apart from the verbose build output. The linked output regarding compiler flags and the optflags meant little to me as I'm not a programmer. I tried adding it to %make_build but the build failed. I'm guessing I may need to patch the Makefile to add the flags, or raise a bug upstream.

Spec URL: http://therobinsonfamily.net/SPECS/helm.spec
SRPM URL: https://therobinsonfamily.net/SRPMS/helm-0.5.0-2.fc22.src.rpm

Build info: http://koji.fedoraproject.org/koji/taskinfo?taskID=11128243

Patched Makefiles to make buildoutput verbose.

SRPM URL: http://therobinsonfamily.net/SRPMS/helm-0.5.0-2.fc22.src.rpm

An informational note that the next upstream version will not require patching to make the build verbose.

https://github.com/mtytel/helm/issues/30#issuecomment-141542740

Good.

Some more advanced topics:


> License:	GPLv3

The source files say "or later", which would be "GPLv3+":

https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#.22or_later_version.22_licenses


> ExcludeArch:	armv7hl

https://fedoraproject.org/wiki/Packaging:Guidelines#Architecture_Build_Failures


> Requires:	lv2

# dnf list lv2|grep ^lv
lv2.i686                           1.10.0-2.fc22                          fedora
lv2.x86_64                         1.10.0-2.fc22                          fedora

As you're shipping files in %{_libdir}/lv2/helm.lv2/ you want to make this explicit dependency arch-specific via %{?_isa}:
https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires


> %{_datadir}/applications/helm.desktop

/builddir/build/SOURCES/helm.desktop: warning: key "Categories" is a list and does not have a semicolon as trailing character, fixing

/builddir/build/BUILDROOT/helm-0.5.0-2.fc24.x86_64/usr/share/applications/helm.desktop: warning: value "Software Synthesizer" for key "Comment" in group "Desktop Entry" looks redundant with value "Software Synthesizer" of key "GenericName"


> %{_datadir}/applications/helm.desktop

https://fedoraproject.org/wiki/Packaging:Guidelines#AppData_files


> helm-0.5.0/JUCE/

That is a bundled library from juce.com and makes the package non-trivial. The No_Bundled_Libraries policy has been changed recently, and the controversial change is still being discussed:

https://fedoraproject.org/w/index.php?title=Packaging:No_Bundled_Libraries&oldid=406058


> helm-0.5.0/mopo/

Dunno whether the author has released this independently yet
(apart from https://github.com/mtytel/mopo ) or only together with Helm. Not an issue yet.


> helm-0.5.0/fonts/

$ grep ttf build.log 
$

Also don't appear in the packages.


> /usr/share/helm/patches/LICENSE

That's not GPLv3, and this file in the helm-common package is not marked %license either. This is some Creative Commons Attribution 4.0 license.

Note that each (sub-)package can have its own Group tag:
https://fedoraproject.org/wiki/Packaging:Guidelines#Licensing


> rpmlint

It finds some things when running it on all the built packages.


> Unknown

It GNOME Shell (Fedora 22), the application /usr/bin/helm creates a shell menu entry with the name "Unknown".

ping?

The developer has stopped tagging builds and is now bundling more libraries like libpng. 

https://github.com/mtytel/helm/commit/468f26b7dc7b26b0fcc122391502c105c6f1d61b

I think at the moment this project and my skills are to immature for me to package at this present time.

*** This bug has been marked as a duplicate of bug 1661657 ***