Spec URL: http://dox.eap.su/rpms/screengrab/screengrab.spec SRPM URL: http://dox.eap.su/rpms/screengrab/screengrab-0.9.1-1.fc18.src.rpm Description: ScreenGrab - program geting screenshots working in Linux and Windows. The program uses Qt and is independent of any desktop environment. Main features: * grab screenshot of desktop * working on Window and Linux operating systems * save screenshots in PNG and JPEG format * grab screenshot with delay (1 - 90 sec) * hide his window * minimize to system tray and work from at (tray menu) This is my first package and I need a sponsor. Application is successfully packagable in Koji (http://koji.fedoraproject.org/koji/taskinfo?taskID=5090636) and OBS (https://build.opensuse.org/package/show?package=screengrab&project=X11%3AQtDesktop) Fedora Account System Username: tieugene
Hi Eugene. Some initial comments. - Source link is wrog, it's a https. - gcc-c++ can be omitted http://fedoraproject.org/wiki/Packaging:Guidelines#Exceptions_2 - Description line is too long, it must not exceed 80 characters - %{__rm} -rf %{buildroot} and %clean section are not necessary http://fedoraproject.org/wiki/Packaging:Guidelines#.25clean - %defattr(-,root,root,-) is no necessary http://fedoraproject.org/wiki/Packaging:Guidelines#File_Permissions - License should be a GPLv2. - Package includes a .desktop file, add a desktop-file-validation to .spec file http://fedoraproject.org/wiki/Packaging:Guidelines#desktop-file-install_usage - Changelog should regard the RPM history in Fedora starting from initial package created for Fedora review. The number includes also the release so it is 0.9.1-1. - Vendor tag should not be used http://fedoraproject.org/wiki/Packaging:Guidelines#Tags
> - License should be a GPLv2. What makes you say that? Looking at the source files, I see a mixture of BSD and GPLv2+ , which do effectively combine to GPLv2+
(In reply to comment #2) > > - License should be a GPLv2. > > What makes you say that? Looking at the source files, I see a mixture of > BSD and GPLv2+ , which do effectively combine to GPLv2+ LICENSE.txt and https://code.google.com/p/screengrab-qt/ . But licensecheck shows a GPLv2+ license indeed. I'll do a cross check henceforth. :)
(In reply to comment #1) > - Source link is wrog, it's a https. Real current link: http://screengrab.doomer.org/download/screengrab-0_9_1_/ (http://screengrab.doomer.org/download/) - but it is not spec-compatible. I don't know what to do. > - gcc-c++ can be omitted Fixed > - Description line is too long, it must not exceed 80 characters Fixed > - %{__rm} -rf %{buildroot} and %clean section are not necessary Fixed > - %defattr(-,root,root,-) is no necessary Fixed > - License should be a GPLv2. I used src/main.cpp head > - Package includes a .desktop file, add a desktop-file-validation to .spec file Fixed > - Changelog should regard the RPM history in Fedora starting from initial > package created for Fedora review. The number includes also the release so > it is 0.9.1-1. Fixed > - Vendor tag should not be used Fixed
(In reply to comment #4) > (In reply to comment #1) > > - Source link is wrog, it's a https. > Real current link: http://screengrab.doomer.org/download/screengrab-0_9_1_/ > (http://screengrab.doomer.org/download/) - but it is not spec-compatible. I > don't know what to do. You can follow this guide lines: https://fedoraproject.org/wiki/Packaging:SourceURL?rd=Packaging/SourceURL#Troublesome_URLs Something like that should be sufficient: # Screengrab has a mirror redirector for its downloads # You can get this tarball by following a link from: # http://screengrab.doomer.org/download/screengrab-0_9_1_/ Source0: %{name}-%{version}.tar.gz Regarding to the license, also GitHub reports a GPLv2 (https://github.com/DOOMer/ScreenGrab) and this contrasts with the licenses of files in the source (as Rex said). If I were you, I would ask clarification to the upstream. Increase release number for every change on .spec file. Now the release is 2 so from 0.9.1-1 to 0.9.1-2: ... %changelog * Thu Mar 07 2013 TI_Eugene <ti.eugene> 0.9.1-2 - ... - ... * Thu Mar 07 2013 TI_Eugene <ti.eugene> 0.9.1-1 - initial packaging for Fedora
(In reply to comment #5) > Something like that should be sufficient: Fixed > Regarding to the license, also GitHub reports a GPLv2 Fixed > Increase release number for every change on .spec file. Now the release is 2 > so > from 0.9.1-1 to 0.9.1-2: Fixed Thank you for comments - Fedora rules are stricter then OBS' ones (OpenSUSE Build Service).
(In reply to comment #6) > (In reply to comment #5) > > > Regarding to the license, also GitHub reports a GPLv2 > Fixed > Not yet. Source contain files with GPLv2+, we don't know why upstream releases the software with a GPLv2 licence.txt file. > > Increase release number for every change on .spec file. Now the release is 2 > > so > > from 0.9.1-1 to 0.9.1-2: > Fixed Not yet. > * Thu Mar 07 2013 TI_Eugene <ti.eugene> 0.9.1-2 > - spec fixes Not yet. Which are they ? See this example: http://pcpa.fedorapeople.org/coin-or/coin-or-Ipopt.spec Post again the links to SPEC and SRPM file when you end.
(In reply to comment #7) > Source contain files with GPLv2+, we don't know why upstream releases the > software with a GPLv2 licence.txt file. Ok - will wait author's reply to me. Seems that he has problems with Internet. > Not yet. Which are they ? > See this example: http://pcpa.fedorapeople.org/coin-or/coin-or-Ipopt.spec So - spec must have all of changes described in changelog. Next uploaded spec and src.rpm must have release == 3?
> cmake .. -DCMAKE_INSTALL_PREFIX=/usr -DCMAKE_BUILD_TYPE=release Please use the %cmake macro (which also takes care of setting CMAKE_INSTALL_PREFIX and CMAKE_BUILD_TYPE for you).
(In reply to comment #9) > Please use the %cmake macro Compile error. Maybe - because of built-in libraries (?).
How are you using the %cmake macro? It should be at least: %cmake .. (You can add -D options to that to add to or override the ones %cmake sets.) And what error are you getting? As for the bundled libraries, they are usually not allowed in Fedora and need to be packaged separately. See: https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries
In particular, src/3rdparty/qxt is from http://pkgs.fedoraproject.org/cgit/libqxt.git/ and should use the system libqxt package. Some of the stuff in src/common is also being built as a library for some reason, even though it is from the same author (and therefore it can probably stay bundled). The stuff in src/common/singleapp is different from qtsingleapplication which we have in Fedora. It's quite sad to have 3 implementations of essentially the same thing (kdelibs also has one), but that's not our problem. CMake itself defaults to building libraries static, our %cmake macro defaults to building them shared. So if you want those libraries to be static (because you don't want to install headers and provide -devel and -libs packages allowing their use with other packages, because they're internal to the application) and/or if they don't compile at all as shared (e.g. due to symbol visibility issues), add an explicit STATIC to the relevant add_library lines in the relevant CMakeLists.txt files and send that patch upstream. (You can also add -DBUILD_SHARED_LIBS:BOOL=OFF to the %cmake line instead, but CMakeLists.txt files really shouldn't assume that the default is static: add_library with nothing specified only makes sense where it makes sense to build the library as either static or shared at the user's request (BUILD_SHARED_LIBS).)
(In reply to comment #7) > Source contain files with GPLv2+, we don't know why upstream releases the > software with a GPLv2 licence.txt file. Author not reply yet. So - I found GPLv2 (main code), LGPL (qtx) and BSD (qkeysequencewidget) license. GPLv2+ appeared from "or (at your option) any later version" phrase. My proposition is to declare all of 3 licenses in License tag (https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Multiple_Licensing_Scenarios)
(In reply to comment #11) > How are you using the %cmake macro? It should be at least: > %cmake .. It is > And what error are you getting? Something like "... first defined here" It tries to link it's inner libraries as shared. > As for the bundled libraries, they are usually not allowed in Fedora and > need to be packaged separately. See: > https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries Seems that author preferes to build parts of his application as static libs and to link statically on finish. I'll try to patch CmakeLists
(In reply to comment #12) > In particular, src/3rdparty/qxt is from > http://pkgs.fedoraproject.org/cgit/libqxt.git/ and should use the system > libqxt package. I know. Current "unstable" screengrab version solve this. Maybe - to pack fresh "unstable" instead of "stable"? This give posibility to patch sources in upstream and to solve many other problems (like static vs shared libs, licenses etc).
(In reply to comment #7) > Post again the links to SPEC and SRPM file when you end. Current SPEC and SRPM: http://dox.eap.su/rpms/screengrab/screengrab.spec http://dox.eap.su/rpms/screengrab/screengrab-0.9.1-3.fc18.src.rpm
Re: comment #13, multiple licensing I disagree that multiple licensing is applicable or required here. Note: per, https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License:_field "The License: field refers to the licenses of the contents of the binary rpm. When in doubt, ask." for example, BSD and GPLv2+ sources when combined to create a binary, that binary effectively is License: GPLv2+ . If a package shipped a binary generated from BSD sources only too, then multiple licensing would be appropriate: License: BSD and GPLv2+ However, doing it either way is often left to the discretion of the packager in question, but the guideline does encourage keeping things simpler by default.
(In reply to comment #17) > "The License: field refers to the licenses of the contents of the binary > rpm. When in doubt, ask." Ok - I started screengrab and clicked About: "Copyright © 2009-2010, Artem 'DOOMer' Galichkin Licensed under the GPL v2" So - GPLv2 ?..
License statements in source headers take precedence really, so, still GPLv2+ though, I can't find any statement in our licensing guidelines that says that right now. I *did* find more reading on the topic of multiple licenses that has a more thorough explanation that what I gave above, https://fedoraproject.org/wiki/Licensing:FAQ?rd=Licensing/FAQ#Multiple_licensing_situations :)
(In reply to comment #19) > License statements in source headers take precedence really, so, still GPLv2+ "...(at your option)..." I prefere to have no option. So - I _can_ leave strict v2. Not? > I *did* find more reading on the topic of multiple licenses that has a more > thorough explanation that what I gave above, > https://fedoraproject.org/wiki/Licensing:FAQ?rd=Licensing/ > FAQ#Multiple_licensing_situations What I'm to do at the end of ends?
That the actual license preamble in source files may take precedence is a combination of these three: * https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#.22or_later_version.22_licenses * https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License:_field * https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Clarification
thanks Michael. :) I may as well start a formal review. 1. SHOULD saying License: GPLv2 here is justifiable too, given the About box text. It's not *wrong*, I just consider GPLv2+ to be more accurate. Ideally, could you contact your upstream and ask them for clarification? $ rpmlint *.spec 0 packages and 1 specfiles checked; 0 errors, 0 warnings. 2. SHOULD: add -DCMAKE_BUILD_TYPE=Release back , the %cmake macro doesn't set this. not setting this means you miss out on: add_definitions(-DNDEBUG) add_definitions(-DQT_NO_DEBUG_OUTPUT) 3. SHOULD remove deprecated items in .spec like Group: tag, and in %install %{__rm} -rf %{buildroot} (rpm does this by default now, for awhile) 4. SHOULD remove %description references "working in Linux and Windows.". This package is build on/for fedora (linux), so that's just extraneous. scriptlets: ok (none) macro usage: ok (especially if item 3 is implemented, otherwise you have a mixture of using 'rm' and '%{__rm}' in different places) sources: ok 21bffd96f8c9f68fb3ad04dfa76d249f screengrab-0.9.1.tar.gz .desktop validation: ok 5. MUST: address bundled libraries, try to use system copy of qxt instead. If you need help implementing this, I can take a closer look.
(In reply to comment #22) > 5. MUST: address bundled libraries, try to use system copy of qxt instead. > If you need help implementing this, I can take a closer look. This is solved in current "unstable" screengrab (0.9.96) that has proper cmake options. I propose leave 0.9.1 as is and wait for next stable release. Or I'll try to pack unstable.
(In reply to comment #22) > Ideally, could you contact your upstream and ask them for clarification? I'm trying for last 3 days. No effect.
We can't ship using the bundled library, period. So, working on packaging 0.9.96+ may indeed be the best strategy. Hopefully the this unstable development version will become "stable" relatively soon. Do you know or can you find out when the next stable version (0.10 or 1.0 or whatever) coming from this 0.9.9x development is expected to land?
(In reply to comment #25) > We can't ship using the bundled library, period. Screengrab rpm has no binary libs inside - it linked with it's libs staticaly (by design).
That distinction does not matter. https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries
(In reply to comment #25) > So, working on packaging 0.9.96+ may indeed be the best strategy. Yes. Current "unstable" can upload screenshots to external image hostings. > Do you know or can you find out when the next stable > version (0.10 or 1.0 or whatever) coming from this 0.9.9x development is > expected to land? No. As I wrote above - he is not achieved not via email nor XMPP.
(In reply to comment #22) > 1. SHOULD saying License: GPLv2 Done. > 2. SHOULD: add -DCMAKE_BUILD_TYPE=Release back Fixed > 3. SHOULD remove deprecated items in .spec Fixed > 4. SHOULD remove %description references "working in Linux and Windows.". Fixed > 5. MUST: address bundled libraries, try to use system copy of qxt instead. Fixed Fresh URLs: SPEC: http://dox.eap.su/rpms/screengrab/screengrab.spec SRPM: http://dox.eap.su/rpms/screengrab/screengrab-0.9.96-1.fc18.src.rpm
great, nice work. APPROVED. One more small thing (but can be done post-import), in %setup section do something like:src/3rdparty/qxt mv src/3rdparty/qxt src/3rdparty/qxt.BAK or rm -rf src/3rdparty/qxt so we can be assured none of this bundled code is used during a normal build. I've sponsored you. Next steps: https://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Add_Package_to_Source_Code_Management_.28SCM.29_system_and_Set_Owner
> 2. SHOULD: add -DCMAKE_BUILD_TYPE=Release back , the %cmake macro doesn't set > this. not setting this means you miss out on: Ugh, you're right, we don't set this in %cmake, only in %cmake_kde4. (I didn't realize this because I'm used to packaging KDE stuff.) This sucks.
You may recall, I lobbied to set a default in %cmake too , but the conclusion of discussion on -devel list was that there was opposition to implementing that.
Yeah, the problem is that projects do different things with different settings. :-( For KDE projects (and some others such as this one), Release is best (it only misses -g, but that gets picked up from %{optflags} anyway), for some others, RelWithDebInfo works better, some might even work best with Debug. So the thread died on bikeshedding between Release or RelWithDebInfo, and of course both have the potential to introduce unwanted flags in some projects.
(In reply to comment #25) > We can't ship using the bundled library, period. It's time to git. I want to pack for el6 too. It's possible (http://build.opensuse.org/package/show?package=screengrab&project=X11%3AQtDesktop) - but using built-in qxt only. What to do?
New Package SCM Request ======================= Package Name: screengrab Short Description: Screen grabber Owners: tieugene Branches: f17 f18 el6 InitialCC: rdieter
Git done (by process-git-requests).
For el6, we can ask for libqxt to be branched and built for epel-6 too (i can help maintain it, if it's primary maintainer isn't interested in supporting epel)
(In reply to comment #37) > For el6, we can ask for libqxt to be branched and built for epel-6 too (i > can help maintain it, if it's primary maintainer isn't interested in > supporting epel) Ok, will wait. Now I am in "Submit Package as Update in Bodhi" step. Seems that it is not thing that I need now... That's all, folks?
(In reply to comment #38) > Seems that it is not thing that I need now... I need NOT now, sorry.
screengrab-0.9.96-2.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/screengrab-0.9.96-2.fc17
screengrab-0.9.96-2.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/screengrab-0.9.96-2.fc18
screengrab-0.9.96-2.fc17 has been pushed to the Fedora 17 testing repository.
Package is not built with $RPM_OPT_FLAGS: http://kojipkgs.fedoraproject.org//packages/screengrab/0.9.96/2.fc19/data/logs/x86_64/build.log Fix upstreamed, but should be added to the Fedora package already now: https://github.com/DOOMer/ScreenGrab/issues/14 https://gist.github.com/scop/5165864
screengrab-0.9.96-3.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/screengrab-0.9.96-3.fc17
screengrab-0.9.96-3.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/screengrab-0.9.96-3.fc18
(In reply to comment #43) > Package is not built with $RPM_OPT_FLAGS: > http://kojipkgs.fedoraproject.org//packages/screengrab/0.9.96/2.fc19/data/ > logs/x86_64/build.log > > Fix upstreamed, but should be added to the Fedora package already now: > https://github.com/DOOMer/ScreenGrab/issues/14 > https://gist.github.com/scop/5165864 Fixed in 0.9.96-3
(In reply to comment #46) > Fixed in 0.9.96-3 Good, just don't forget the f19 build (only f17, f18, and f20 ones seem to be done at the moment).
(In reply to comment #47) > (In reply to comment #46) > > Fixed in 0.9.96-3 > > Good, just don't forget the f19 build (only f17, f18, and f20 ones seem to > be done at the moment). "bash-4.2$ fedpkg update Creating a new update for screengrab-0.9.96-3.fc19 screengrab-0.9.96-3.fc19 not tagged as an update candidate" Sorry - I can't to google what to do.
I believe it's fine to just do the f19 build for now, no need (nor possible) to submit an update.
(In reply to comment #49) > I believe it's fine to just do the f19 build for now, no need (nor possible) > to submit an update. No problem: http://koji.fedoraproject.org/koji/taskinfo?taskID=5130422
Package screengrab-0.9.96-3.fc17: * should fix your issue, * was pushed to the Fedora 17 testing repository, * should be available at your local mirror within two days. Update it with: # su -c 'yum update --enablerepo=updates-testing screengrab-0.9.96-3.fc17' as soon as you are able to. Please go to the following url: https://admin.fedoraproject.org/updates/FEDORA-2013-3959/screengrab-0.9.96-3.fc17 then log in and leave karma (feedback).
> Not yet. > Source contain files with GPLv2+, we don't know why upstream releases the > software with a GPLv2 licence.txt file. Where I can find GPLv2+ license.txt file?
There isn't any such thing. what is packaged now is sufficient.
(In reply to comment #53) > There isn't any such thing. what is packaged now is sufficient. This question is not exactly about this package. I'm starting another package (https://bugzilla.redhat.com/show_bug.cgi?id=926062) and have same problem - part of sources has not-GPLv2 license. And developer announce (by email) GPLv2+. I'm afraid that show will go on :-(
I've added a different comment to bug 926062. If upstream wants "GPLv2 only", ask for license clarification and a change of all source files that currently claim "GPLv2 or (at your option) any later version". If, however, upstream confirms GPLv2+, including the GNU GPL v2 "COPYING" file is correct. That license file explains the "or later" licensing option, btw.
screengrab-0.9.96-3.fc18 has been pushed to the Fedora 18 stable repository. If problems still persist, please make note of it in this bug report.
screengrab-0.9.96-3.fc17 has been pushed to the Fedora 17 stable repository. If problems still persist, please make note of it in this bug report.
qt-reviews block added