[Bug 1297821] Review Request: qlcplus - Q Light Controller Plus

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1297821



--- Comment #3 from Roman Tsisyk <roman@xxxxxxxxxx> ---
Your spec is in the very good state.

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

> Summary:          Q Light Controller Plus

Please try to fit some basic information about "Q Light Controller Plus"
purpose rather than expand %{name}.

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

> License:          ASL 2.0 and GPLv2+ and BSD and GPLv2 and MIT
> the whole project is licensed under Apache-2.0 execept for

Disclaimer: I'm not IP lawyer, please consider this part of my review as an
advice. I hope somebody more experienced in licensing practices will be able to
take a look on this package too.

> qmlui/qml/FontAwesomeVariables.qml which is MIT/X11
> ui/src/flowlayout.cpp and ui/src/flowlayout.h which are BSD

It is ok to distribute MIT/X11 sources under Apache-2.0, see
http://apache.org/legal/resolved.html

> plugins/udmx/src/libusb_dyn.c and plugins/udmx/src/libusb_dyn.h which are GPLv2+

GPLv2 source can't be re-licensed under Apache 2.0
Anyway, "LIBUSB-WIN32, Generic Windows USB Library" - do you actually use this
file on Fedora? I think you can remove this file from a tarball or from a build
tree during %prep stage.

> webaccess/src/mongoose.c and webaccess/src/mongoose.h which are GPLv2

It is OK to make a subpackage with different license if this file is used by a
plugin. 
It seems that GPLv2 mongoose.c is used as a part of libqlcplus:
Binary file
./libqlcplus-4.10.2b-1.fc24.x86_64.rpm/usr/lib64/libqlcpluswebaccess.so.1.0.0
matches
Binary file
./libqlcplus-4.10.2b-1.fc24.x86_64.rpm/usr/lib64/libqlcpluswebaccess.so.1.0
matches
Binary file
./libqlcplus-4.10.2b-1.fc24.x86_64.rpm/usr/lib64/libqlcpluswebaccess.so.1
matches
Is it possible to split this library into a plugin?

More GPLv2+ files:

+qlcplus-QLC-_4.10.2b/plugins/hid/linux/hidapi.cpp
+qlcplus-QLC-_4.10.2b/plugins/peperoni/win32/peperoni/usbdmx-dynamic.cpp
+qlcplus-QLC-_4.10.2b/plugins/peperoni/win32/peperoni/usbdmx-dynamic.h

Upstream claims to be licensed under ASL 2.0. I hope you will be able to deal
with libusb and mongoose and license the main package as ASL 2.0, as it was
desired by upstream. If in doubt, please ask upstream to resolve this confusion
[1].

[1]:
https://fedoraproject.org/wiki/Packaging:LicensingGuidelines?rd=Packaging/LicensingGuidelines#License_Clarification

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

Please ensure that license file is present when any subpackage combination is
installed.

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

> %package plugin-artnet
> %package plugin-dmx4linux
> %package plugin-dmxusb
> %package plugin-e131
> %package plugin-enttecwing
> %package plugin-hid
> %package plugin-loopback
> %package plugin-midi
> %package plugin-osc
> %package plugin-peperoni
> %package plugin-spi
> %package plugin-udmx
> %package plugin-ola

Do you actually want to split all plugins into separate packages?
How it is supposed to be installed by the end user?
Shouldn't main package Recommends [3] for plugins needed for 99% of users?

> Typical use cases for weak dependencies are:
> Plug-ins or add-ons
> Support for file formats
> Support for protocols
> ...

[3]: https://fedoraproject.org/wiki/Packaging:WeakDependencies

The packaging scheme is very good, though.

--------------------------------------------------------------------------------
># fedora specific patch 
>Patch0:           qlcplus.ola-pkgconfig.patch
># fixed upstream: https://github.com/mcallegari/qlcplus/commit/16b6728e5066a041297bc4d424ada515e85a1e75
>Patch1:           qlcplus.install-paths.patch
># fixed upstream: https://github.com/mcallegari/qlcplus/commit/e29f34f7e89db4db3fce3bcb8449f991ef3b9a1f
>Patch2:           qlcplus.udev-rules-path.patch
># under review: https://github.com/mcallegari/qlcplus/pull/735
>Patch3:           qlcplus.fix-translate.patch
># fixed upstream:
># https://github.com/mcallegari/qlcplus/commit/7dcc3cc006a6b03b659be99b72f9a23ce8b277c2
># https://github.com/mcallegari/qlcplus/commit/81308651bab16a50eb35e8a0431c62de9f94942c
># https://github.com/mcallegari/qlcplus/commit/c6eff5b6b4d3852d922e7c966c21b33faf74299f
>Patch4:           qlcplus.fix-licenses.patch
># fedora specific patch
>Patch5:           qlcplus.fedora-revision.patch

I personally dislike idea of manually cherry-picking bugfixes from upstream's
git tree. It is like supporting a new fork and so requires too many efforts.
Ideally, it should be possible to bump `Version:` and then get spec for the
next release.

Your efforts to push all requires changes to the upstream is the right
direction.
It is possible to make a snapshot package directly from git [2] and then wait
for the next tagged release.

[2]:
https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Snapshot_packages

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

> Source1:          https://github.com/mcallegari/qlcplus/raw/c126fcaaf59622009db8e3dca1f72e47bd9e77bd/etc/qlcplus.appdata.xml
> Source2:          https://github.com/mcallegari/qlcplus/raw/c126fcaaf59622009db8e3dca1f72e47bd9e77bd/etc/qlcplus-fixtureeditor.appdata.xml
> # metainfo files from upstream
> Source3:          https://github.com/mcallegari/qlcplus/raw/c126fcaaf59622009db8e3dca1f72e47bd9e77bd/plugins/E1.31/qlcplus-e131.metainfo.xml
> Source4:          https://github.com/mcallegari/qlcplus/raw/c126fcaaf59622009db8e3dca1f72e47bd9e77bd/plugins/artnet/src/qlcplus-artnet.metainfo.xml
> Source5:          https://github.com/mcallegari/qlcplus/raw/c126fcaaf59622009db8e3dca1f72e47bd9e77bd/plugins/dmx4linux/qlcplus-dmx4linux.metainfo.xml
> Source6:          https://github.com/mcallegari/qlcplus/raw/c126fcaaf59622009db8e3dca1f72e47bd9e77bd/plugins/dmxusb/src/qlcplus-dmxusb.metainfo.xml
> Source7:          https://github.com/mcallegari/qlcplus/raw/c126fcaaf59622009db8e3dca1f72e47bd9e77bd/plugins/enttecwing/src/qlcplus-enttecwing.metainfo.xml
> Source8:          https://github.com/mcallegari/qlcplus/raw/c126fcaaf59622009db8e3dca1f72e47bd9e77bd/plugins/hid/linux/qlcplus-hid.metainfo.xml
> Source9:          https://github.com/mcallegari/qlcplus/raw/c126fcaaf59622009db8e3dca1f72e47bd9e77bd/plugins/loopback/src/qlcplus-loopback.metainfo.xml
> Source10:         https://github.com/mcallegari/qlcplus/raw/c126fcaaf59622009db8e3dca1f72e47bd9e77bd/plugins/midi/qlcplus-midi.metainfo.xml
> Source11:         https://github.com/mcallegari/qlcplus/raw/c126fcaaf59622009db8e3dca1f72e47bd9e77bd/plugins/ola/qlcplus-ola.metainfo.xml
> Source12:         https://github.com/mcallegari/qlcplus/raw/c126fcaaf59622009db8e3dca1f72e47bd9e77bd/plugins/osc/qlcplus-osc.metainfo.xml
> Source13:         https://github.com/mcallegari/qlcplus/raw/c126fcaaf59622009db8e3dca1f72e47bd9e77bd/plugins/peperoni/unix/qlcplus-peperoni.metainfo.xml
> Source14:         https://github.com/mcallegari/qlcplus/raw/c126fcaaf59622009db8e3dca1f72e47bd9e77bd/plugins/spi/qlcplus-spi.metainfo.xml
> Source15:         https://github.com/mcallegari/qlcplus/raw/c126fcaaf59622009db8e3dca1f72e47bd9e77bd/plugins/udmx/src/qlcplus-udmx.metainfo.xml

All these files already exist in the source tarball, generated by GitHub.

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

Minor issue, please report to upstream:

libqlcplus.x86_64: E: missing-call-to-setgroups-before-setuid
/usr/lib64/libqlcpluswebaccess.so.1.0.0

https://github.com/mcallegari/qlcplus/blob/49a0a6d29070bc17a632f84f0a18add8ba859573/webaccess/src/mongoose.c#L5096-L5101

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

Please open a ticket in the upstream to create simple man pages for
binaries un /usr/bin:

/usr/bin/qlcplus
/usr/bin/qlcplus-fixtureeditor

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

Probably -data should depend on lib, but I'm not sure about that.
--------------------------------------------------------------------------------

=> Licensing and tarball are main problems.
Great work. Thanks!

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review




[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]