Bug 951496 (gimp-lensfun)
Summary: | Review Request: gimp-lensfun - gimp plugin to correct lens distortion | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Graham White <graham_alton> |
Component: | Package Review | Assignee: | Rex Dieter <rdieter> |
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | anto.trande, graham_alton, gwhite, i, luya, nphilipp, package-review, rdieter |
Target Milestone: | --- | Flags: | rdieter:
fedora-review+
gwync: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
URL: | http://grahamwhiteuk.fedorapeople.org/pkgreviews/gimp-lensfun/ | ||
Whiteboard: | |||
Fixed In Version: | gimp-lensfun-0.2.3-1.fc21 | Doc Type: | Bug Fix |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2015-07-13 19:10:57 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | 1048570 | ||
Bug Blocks: | 1000885 |
Description
Graham White
2013-04-12 10:32:23 UTC
Successful Koji Build: http://koji.fedoraproject.org/koji/taskinfo?taskID=5253393 Hi Graham. Just some comments. - "Compilers used to build packages must honor the applicable compiler flags set in the system rpm configuration. For C, C++, and Fortran code, the %{optflags} macro contains these flags." http://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags See that Makefile uses right flags. - Source archive contains files released with GPLv3+ (see src/gimplensfun.c, src/LUT.h files). I think License tag must be GPLv3+. http://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Mixed_Source_Licensing_Scenario Thanks Antonio for the feedback. I have updated the spec and rebuilt the SRPM for the GPLv3+ license. The use of %{optflags} is problematic with this package. The makefile sets: CFLAGS = -O3 -Wall $(shell gimptool-2.0 --cflags && pkg-config --cflags lensfun exiv2) -fopenmp It's importing CFLAGS from gimptool and pkg-config that include links to libraries via the -I flag. Therefore, a straight use of CFLAGS="%{optflags}" breaks the build as the libraries cannot be found. I presume the gimp-lqr-plugin RPM had a similar issue as it also does not use %{optflags}. I could still include %{optflags} if I run gimptool and pkg-config in the spec file and pass the result into the call to make - would this be preferred to omitting %{optflags} altogether? I guess another approach would be for me to submit something upstream to alter their makefile so we can pass CFLAGS in more easily in the spec. The current git version of the makefile has changed from the release version I'm building here anyway but is still using gimptool and pkg-config in CFLAGS. If it's deemed best to leave %{optflags} out then I will add a comment to the spec to document why it's not there. (In reply to comment #3) > Thanks Antonio for the feedback. > > I have updated the spec and rebuilt the SRPM for the GPLv3+ license. > > The use of %{optflags} is problematic with this package. > > The makefile sets: > CFLAGS = -O3 -Wall $(shell gimptool-2.0 --cflags && pkg-config --cflags > lensfun exiv2) -fopenmp > > It's importing CFLAGS from gimptool and pkg-config that include links to > libraries via the -I flag. Therefore, a straight use of > CFLAGS="%{optflags}" breaks the build as the libraries cannot be found. > > I presume the gimp-lqr-plugin RPM had a similar issue as it also does not > use %{optflags}. > > I could still include %{optflags} if I run gimptool and pkg-config in the > spec file and pass the result into the call to make - would this be > preferred to omitting %{optflags} altogether? Try to: - Patch Makefile into %prep section with a simple replacement of "-O3 -Wall" with "@@OPTFLAGS@@" - In %build, add sed -e 's|@@OPTFLAGS@@|%{optflags}|g' -i Makefile' Correct. Note that http://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags is a MUST item, also with regard to what must be done when filtering/adding to the set of flags. That's because some of the flags are security related, and e.g. -g is required for automatic generation of -debuginfo packages: https://fedoraproject.org/wiki/Packaging:Guidelines#Debuginfo_packages $ rpm --eval %optflags -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic Spec URL: http://grahamwhiteuk.fedorapeople.org/pkgreviews/gimp-lensfun/gimp-lensfun.spec SRPM URL: http://grahamwhiteuk.fedorapeople.org/pkgreviews/gimp-lensfun/gimp-lensfun-0.2.2-2.20130423git6ab226c12a.fc18.src.rpm Successful Koji Build: http://koji.fedoraproject.org/koji/taskinfo?taskID=5301586 Change Summary: * %optflags added to the build * updated to postrelease code (changed package version) * changed package name The addition of %optflags has been done by a merged upstream pull request to fix the makefile. Hence, the package has been moved to a postrelease version from git and follows the Fedora Packaging Naming Guidelines for the updated version string. It would be nice to get a 0.2.3 release but upstream have not given me an outlook on when this might be available. I have also changed the package name from gimplensfun to gimp-lensfun. This has been done to fit in with the upstream name change following the same convention (i.e. they have introduced a hyphen into the name). Conveniently, this seems to fit better with Fedora's package naming and other gimp plugin names. The package name change has not been added to the changelog in the spec file since this change has not been seen by clients given this package isn't included in Fedora yet. Spec URL: http://grahamwhiteuk.fedorapeople.org/pkgreviews/gimp-lensfun/gimp-lensfun.spec SRPM URL: http://grahamwhiteuk.fedorapeople.org/pkgreviews/gimp-lensfun/gimp-lensfun-0.2.3-1.fc18.src.rpm Successful Koji Build: http://koji.fedoraproject.org/koji/taskinfo?taskID=5413262 Change Summary: * Bumped to official 0.2.3 build * Removed git build stuff Since building the git version (above) the 0.2.3 official release of gimp-lensfun has happened. This is an updated spec file and rebuilt srpm. Successful Koji build against rawhide instead of F18 this time. Successful F18 Koji Build: http://koji.fedoraproject.org/koji/taskinfo?taskID=6304317 Successful F19 Koji Build: http://koji.fedoraproject.org/koji/taskinfo?taskID=6304311 Successful F20 Koji Build: http://koji.fedoraproject.org/koji/taskinfo?taskID=6304277 Packages for F18, F19 and F20 updated and uploaded at http://grahamwhiteuk.fedorapeople.org/pkgreviews/gimp-lensfun/ and made available as a yum repository http://grahamwhiteuk.fedorapeople.org/pkgreviews/gimp-lensfun/gimp-lensfun.repo Christopher added a blocker to bug #1048570 , but the .spec here doesn't specifically require lensfun-0.2.8 (yet). So, does it really need the newer version (or not)? Oh, and if my question (comment #10) gets clarified, I'd be happy to help with the review/sponsorship process asap :) gimplensfun will build against lensfun 0.2.7 as well as 0.2.8 so 0.2.8 isn't a hard requirement, it's more of a nice-to-have since the later version includes updates that gimplensfun can take advantage of. There are no docs in gimplensfun to indicate which is the minimum version but I can probably find out what the minimum is from upstream if required. I've just submitted F-19, F-20 updates for lensfun-0.2.8: https://admin.fedoraproject.org/updates/lensfun-0.2.8-3.fc20 https://admin.fedoraproject.org/updates/lensfun-0.2.8-3.fc19 The sooner it gets karma, the sooner you can depend on it :o). I've tested lensfun-0.2.8-3 via gimplensfun version 0.2.3 and gimp 2.8.14 and all working OK for me on F20 - karma added. However, gimplensfun still doesn't *need* to depend on this particular level of lensfun or above, it works quite happily with 0.2.7 as well without any rebuilding required. Successful F21 Koji Build: http://koji.fedoraproject.org/koji/taskinfo?taskID=9470843 Successful F22 Koji Build: http://koji.fedoraproject.org/koji/taskinfo?taskID=10070503 Repo updated at https://grahamwhiteuk.fedorapeople.org/pkgreviews/gimp-lensfun/ Is someone ever going to sponsor this? Started over 2 years ago now! Fair enough, I threaten to help back in comment #11, so here we go. :) Naming: ok Sources: NOT ok 1. MUST: Source0 URL doesn't work, spectool -g *.spec Getting http://dl.bintray.com/content/seebk/GIMP-Lensfun/gimplensfun-0.2.3.tar.gz to ./gimplensfun-0.2.3.tar.gz % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0curl: (22) The requested URL returned error: 404 Not Found while we're at it, I'd encourage you to use the %{version} macro in the URL, instead of hard-coding 0.2.3 (to make future updates easier), so: Source0: https://github.com/seebk/GIMP-Lensfun/releases/download/%{version}/gimplensfun-%{version}.tar.gz confirmed that source has the same checksum, $ md5sum *.gz 2ac8cc61ab3672aa01311e47dfd911b0 gimplensfun-0.2.3.tar.gz 2. SHOULD document patches. Patch0: %{name}-remove-cci.patch What's the purpose of this patch? why is it needed? any bug references? 3. SHOULD omit deprecated rpm bits: Group:, BuildRoot:, %clean section, from %install: rm -rf $RPM_BUILD_ROOT 4. License: NOT ok. $ licensecheck * gimplensfun.cpp: GPL (v3 or later) LUT.hpp: GPL (v3 or later) MUST use License: GPLv3+ (unless you can clarify upstream intent to want only GPLv3, and and a .spec comment saying so) scriptlets: ok 5. dependencies: NOT ok These MUST be removed: Requires: lensfun Requires: exiv2 rpm should pull both of these library dependencies in via autorequires... unless does gimp-lensfun really need exiv2 (and not just the exiv2 library from exiv2-libs)? Fix these, and we should be close to approval. Thanks for the feedback. It's been a while since I've run the review tool on the spec file. I've taken the following actions: 1) fixed the issues identified above in #18 2) Run a couple of scratch builds against F21 and F22 http://koji.fedoraproject.org/koji/taskinfo?taskID=10071542 http://koji.fedoraproject.org/koji/taskinfo?taskID=10071546 3) Tested installation and functionality on F22 4) Run through fedora-review I'd welcome any further feedback if there are still issues. Got a new spec/srpm links for me? (hint: they shouldn't be the same as before, since you bumped Release, and added changelog entries, right? :) ) Spec URL: https://grahamwhiteuk.fedorapeople.org/pkgreviews/gimp-lensfun/gimp-lensfun.spec SRPM URL: https://grahamwhiteuk.fedorapeople.org/pkgreviews/gimp-lensfun/gimp-lensfun-0.2.3-1.fc22.src.rpm I've updated the spec file at the URL above and uploaded the SRPM too. I did add a changelog entry but didn't bump the release number. Since this has never been release I figured the release number means nothing at this stage. Damn and blast, I've just realised I left the "Requires" in the spec file... scratch that lot then, I'll try again! OK, I rebuilt scratch builds again and tested locally on F22... Spec URL: https://grahamwhiteuk.fedorapeople.org/pkgreviews/gimp-lensfun/gimp-lensfun.spec SRPM URL: https://grahamwhiteuk.fedorapeople.org/pkgreviews/gimp-lensfun/gimp-lensfun-0.2.3-1.fc22.src.rpm F22 http://koji.fedoraproject.org/koji/taskinfo?taskID=10072965 F21 http://koji.fedoraproject.org/koji/taskinfo?taskID=10072956 Thanks, APPROVED (and sponsored) For future reviews, make sure to increment Release: tag too whenever you make changes. Next steps, http://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Add_Package_to_Source_Code_Management_.28SCM.29_system_and_Set_Owner Let me know if you need anything else. New Package SCM Request ======================= Package Name: gimp-lensfun Short Description: Gimp plugin to correct lens distortion Upstream URL: http://seebk.github.io/GIMP-Lensfun/ Owners: grahamwhiteuk Branches: f21 f22 f23 InitialCC: Git done (by process-git-requests). gimp-lensfun-0.2.3-1.fc21 has been submitted as an update for Fedora 21. https://admin.fedoraproject.org/updates/gimp-lensfun-0.2.3-1.fc21 gimp-lensfun-0.2.3-1.fc22 has been submitted as an update for Fedora 22. https://admin.fedoraproject.org/updates/gimp-lensfun-0.2.3-1.fc22 gimp-lensfun-0.2.3-1.fc22 has been pushed to the Fedora 22 testing repository. gimp-lensfun-0.2.3-1.fc22 has been pushed to the Fedora 22 stable repository. gimp-lensfun-0.2.3-1.fc21 has been pushed to the Fedora 21 stable repository. |