Bug 787364

Summary: Review Request: clipgrab - Streaming videos platforms grabber
Product: [Fedora] Fedora Reporter: Pierre Dorbais <pierre.dorbais>
Component: Package ReviewAssignee: Nobody's working on this, feel free to take it <nobody>
Status: CLOSED NOTABUG QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: alexjnewt, notting, oschreib, package-review, volker27
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2013-05-01 15:10:13 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Bug Depends On:    
Bug Blocks: 201449    

Description Pierre Dorbais 2012-02-04 07:44:35 EST
Spec URL: http://pierre.dorbais.free.fr/rpms/clipgrab.spec
SRPM URL: http://pierre.dorbais.free.fr/rpms/clipgrab-3.1.3.0-1.fc16.src.rpm
Description: Download and convert videos from streaming plate-forms like Youtube and Dailymotion.

rpmlint:
[chdorb@chdorb-desktop rpmbuild]$ rpmlint SPECS/clipgrab.spec RPMS/i686/clipgrab-* SRPMS/clipgrab-3.1.3.0-1.fc16.src.rpm 
clipgrab.i686: W: no-manual-page-for-binary clipgrab
3 packages and 1 specfiles checked; 0 errors, 1 warnings.

mock OK
Comment 1 Ofer Schreiber 2012-02-07 05:59:02 EST
I'm not a packager, so I can't be a sponsor yet.
Some comments though:

1. LICENSE is missing, you should contact upstream  so a LICENSE text will be
included in source tarball

2. The %defattr(-,root,root,-) entry is not needed in Fedora, unless you're packaging for EPEL5.

3. Same for the "rm -rf RPM_BUILD_ROOT" in the beginning of %install

4. Why do you need the .desktop file as a source?
Comment 2 Pierre Dorbais 2012-02-07 12:57:44 EST
(In reply to comment #1)
> I'm not a packager, so I can't be a sponsor yet.
> Some comments though:
> 
> 1. LICENSE is missing, you should contact upstream  so a LICENSE text will be
> included in source tarball

There is a "licence.odt" file, do the package must have plain text file ?

> 2. The %defattr(-,root,root,-) entry is not needed in Fedora, unless you're
> packaging for EPEL5.
> 
> 3. Same for the "rm -rf RPM_BUILD_ROOT" in the beginning of %install

OK I'll delete these lines.

> 4. Why do you need the .desktop file as a source?

I've written and added the .desktop file because ->
MUST: Packages containing GUI applications must include a %{name}.desktop file, and that file must be properly installed with desktop-file-install in the %install section. If you feel that your packaged GUI application does not need a .desktop file, you must put a comment in the spec file with your explanation.

Thank you.
Comment 3 Volker Fröhlich 2012-02-13 16:51:01 EST
According to the code, license is GPLv3+.

Throw away the defattr line, it is not necessary. "rm -rf $RPM_BUILD_ROOT" is outdated too.

Should that really be "plate-forms" and not "platform"?

As you're not defining or using MIME types, neither installing icons in the hicolor directory, all of the scriptlets are useless.

Please preserve the timestamps when installing, where it makes sense. For instance, use cp -p icon.png ...

Commonly _datadir is used instead of _datarootdir.

Please use the name macro consistently.
Comment 4 Volker Fröhlich 2012-02-13 16:54:16 EST
I can't find your name in the packagers group. Do you have a sponsor yet? If not, please follow point 3:

http://fedoraproject.org/wiki/Package_Review_Process
Comment 5 Volker Fröhlich 2012-02-13 17:06:55 EST
http://fedoraproject.org/wiki/Packaging:ScriptletSnippets
Comment 6 Pierre Dorbais 2012-02-14 13:48:14 EST
(In reply to comment #3)
> According to the code, license is GPLv3+.
> 
> Throw away the defattr line, it is not necessary. "rm -rf $RPM_BUILD_ROOT" is
> outdated too.
> 
> Should that really be "plate-forms" and not "platform"?
> 
> As you're not defining or using MIME types, neither installing icons in the
> hicolor directory, all of the scriptlets are useless.
> 
> Please preserve the timestamps when installing, where it makes sense. For
> instance, use cp -p icon.png ...
> 
> Commonly _datadir is used instead of _datarootdir.
> 
> Please use the name macro consistently.

OK, fixed

(In reply to comment #4)
> I can't find your name in the packagers group. Do you have a sponsor yet? If
> not, please follow point 3:
> 
> http://fedoraproject.org/wiki/Package_Review_Process

Strangely I'm not in Packagers groups (my screenname is chdorb) however my sponsor is mtasaka and I've already created two packages (ciso and vifm). I don't understand...

------
[chdorb@chdorb-desktop rpmbuild]$ rpmlint SPECS/clipgrab.spec RPMS/i686/clipgrab-3.1.3.0-2.fc16.i686.rpm RPMS/i686/clipgrab-debuginfo-3.1.3.0-2.fc16.i686.rpm SRPMS/clipgrab-3.1.3.0-2.fc16.src.rpm 
clipgrab.i686: W: no-manual-page-for-binary clipgrab
3 packages and 1 specfiles checked; 0 errors, 1 warnings.

Spec URL: http://pierre.dorbais.free.fr/rpms/clipgrab.spec
SRPM URL: http://pierre.dorbais.free.fr/rpms/clipgrab-3.1.3.0-2.fc16.src.rpm
Comment 7 Volker Fröhlich 2012-02-21 17:12:39 EST
Hm, weird. Have you asked somebody about it?

Oh, look at that:

"""
[makerpm@desktop clipgrab-3.1.3.0]$ diff COPYING{,~}
1c1
< ClipGrab™ and its components are free software. By installing and using ClipGrab™, you agree to be bound to the following licence and terms.
---
> ClipGrab and its components are free software. By installing ClipGrab, you agree to be bound to the following licence and terms.
3c3
< ClipGrab™:    © 2011 Philipp Schmieder
---
> ClipGrab:     © 2011 Philipp Schmieder
186,187c186,187
< While ClipGrab™ itself is free software, its artwork and name are not. Thus, you are not allowed to redistribute this compiled setup binary file or any other compiled version of ClipGrab™ which includes the ClipGrab™ artwork or the ClipGrab™ trademark commercially without prior written permission of the author.
< However, you are free to redistribute any compiled version of ClipGrab within non-commercial or private bounds.
---
> While ClipGrab itself is free software, its artwork and name are not. Thus, you are not allowed to redistribute this compiled setup binary file or any other compiled version of ClipGrab which includes the ClipGrab artwort commercially without prior written permission of the author.
> However, you are free to redistribute any compiled version of ClipGrab within non-commercial or private bounds. 
"""

Besides leaving you in limbo about which version is the proper one, that doesn't sound very free to me.
Comment 8 Jeremy Newton 2012-02-24 18:37:44 EST
I do believe Firefox is along the same idea as ClipGrab. All code is free but Artwork and Name are Trademarked. I would figure asking the developer would help clarify this. Although unfortunately, a higher concern would be that Clipgrab includes/uses patented codecs (ie MPEG) and will not be able to be included in Fedora period.

If you wish for this to be packaged, you can include this in RPM Fusion instead. If you do not wish to do so, I would gladly do it for you, as I am an RPM Fusion packager.
Comment 9 Volker Fröhlich 2012-04-10 17:56:18 EDT
Jeremy, if you're sure about the legal situation, we could close this ticket.
Comment 10 Jeremy Newton 2012-04-10 20:13:03 EDT
On second thought, the distributing rights of the artwork are not technically "free" therefore it cannot be included in Fedora unless forked or patched with new artwork. Even if this was accepted into RPMFusion as is, it would still need to be included in non-free as it has non-commercial distributed artwork. You could ask permission from the author but I would doubt he would give free use to the artwork.

As for the codec issue, I can't confirm that it uses a MPEG codec, but I know it endorses the use of MPEG and WMV which are not accepted codecs in Fedora due to legal reasons. Though it has OGG support, the other codec support in Clipgrab may cause legal issues. I would need to ask someone who is more knowledgeable on the legal know how of these software patents to know if this is a legal blocking issue.
Comment 11 Volker Fröhlich 2012-08-03 02:32:19 EDT
Did you find the time to ask yet?