Spec URL: http://ankursinha.fedorapeople.org/xmedcon/xmedcon.spec SRPM URL: http://ankursinha.fedorapeople.org/xmedcon/xmedcon-0.10.7-1.fc16.src.rpm Description: This project stands for Medical Image Conversion and is released under the GNU's (L)GPL license. It bundles the C sourcecode, a library, a flexible command-line utility and a graphical front-end based on the amazing Gtk+ toolkit. Its main purpose is image conversion while preserving valuable medical study information. The currently supported formats are: Acr/Nema 2.0, Analyze (SPM), Concorde/uPET, DICOM 3.0, CTI ECAT 6/7, InterFile 3.3 and PNG or Gif87a/89a towards desktop applications. ============================================================================ [ankur@ankur SRPMS]$ rpmlint /var/lib/mock/fedora-rawhide-i386/result/*.rpm ../SPECS/xmedcon.spec xmedcon.i686: W: spelling-error %description -l en_US Acr -> Ac, Ar, Apr xmedcon.i686: W: spelling-error %description -l en_US uPET -> u Pet, PET, u PET xmedcon.i686: W: invalid-license BSD with attribution xmedcon.i686: E: shlib-with-non-pic-code /usr/lib/libmdc.so.2.0.1 ^^^^^ I need to look into this :/ Any clues will be helpful. xmedcon.i686: W: shared-lib-calls-exit /usr/lib/libmdc.so.2.0.1 exit xmedcon.i686: W: devel-file-in-non-devel-package /usr/bin/xmedcon-config xmedcon.i686: E: incorrect-fsf-address /usr/share/doc/xmedcon-0.10.7/COPYING xmedcon.i686: E: incorrect-fsf-address /usr/share/doc/xmedcon-0.10.7/COPYING.LIB xmedcon.src: W: spelling-error %description -l en_US Acr -> Ac, Ar, Apr xmedcon.src: W: spelling-error %description -l en_US uPET -> u Pet, PET, u PET xmedcon.src: W: invalid-license BSD with attribution xmedcon-debuginfo.i686: W: invalid-license BSD with attribution xmedcon-devel.i686: W: spelling-error %description -l en_US libmdc -> libido xmedcon-devel.i686: W: invalid-license BSD with attribution xmedcon-devel.i686: E: incorrect-fsf-address /usr/share/doc/xmedcon-devel-0.10.7/COPYING.LIB xmedcon-devel.i686: E: incorrect-fsf-address /usr/share/doc/xmedcon-devel-0.10.7/COPYING 4 packages and 1 specfiles checked; 5 errors, 11 warnings.
I'll take a pass at this. One obvious fix: Instead of doing: %ifarch x86_64 sed -i \ -e "s|tpc_prefix/lib|tpc_prefix/lib64|" \ -e "s|nifti_prefix/lib|nifti_prefix/lib64|" configure %endif Just do: sed -i \ -e "s|tpc_prefix/lib|tpc_prefix/%{_lib}|" \ -e "s|nifti_prefix/lib|nifti_prefix/%{_lib}|" configure Also, I could not get this code to build at all, because of errors like this: /usr/bin/ld: /usr/lib64/libtpcimgio.a(ecat7w.o): relocation R_X86_64_32 against `.rodata.str1.1' can not be used when making a shared object; recompile with -fPIC /usr/lib64/libtpcimgio.a: could not read symbols: Bad value Sure enough, the libtpcimgio and libtpcmisc packages only spits out non-PIC static libs. Which is fine if you never need to make a shared object from them later, but we definitely do in xmedcon.
The obvious fix is to have libtpcimgio and libtpcmisc spit out proper PIC enabled shared libraries. I'm working on patches for you.
I'm building updates for libtpcimgio and libtpcmisc now. Here are some other items to resolve: Here's the correct license tag: License: LGPLv2+ and Copyright only and MIT and BSD and libtiff You will also need to remove rpath: https://fedoraproject.org/wiki/Packaging/Guidelines#Removing_Rpath I tested the libtool method and it worked fine.
Hi spot, That was *really really quick*. I thought I'd go work on generating shared objects but I just got notification emails for your git pushes. Thank you! btw, I just rebuilt xmedcon in mock, and somehow xmedcon built just fine... I guess this is why I didn't think of generating the shared objects for libtpc* earlier. Not sure why it failed for you.. Regards, Ankur
Perhaps you only built for i386? It is more tolerant of PIC issues than x86_64.
Ah! Aye, my default mock configuration is only for rawhide-i386. I'll do a scratch build and confirm this time. Thanks. Ankur
Okay, I've made updates for the fixed libtpcmisc and libtpcimgio. I had to delete your updates to work around a bodhi issue where it is possible that they would get tagged in the mash, not the newer ones (normally, my updates would just obsolete yours, but they hadn't been pushed to testing yet). I had it close out the respective Review Request bugs, so you probably already got bug spammed about it. :) Show me a fixed spec file for xmedcon and I'll finish this review.
Hello, I did indeed get the email(s). I'm just waiting on the new packages to hit the repos. I still run into the static error with libtpc* here because mock still finds the older package. As soon as I can build them correctly on x86_64, I shall submit a new spec. :) Thanks! Ankur
Hello, http://ankursinha.fedorapeople.org/xmedcon/xmedcon.spec http://ankursinha.fedorapeople.org/xmedcon/xmedcon-0.10.7-3.fc15.src.rpm * Tue Aug 09 2011 Ankur Sinha <ankursinha AT fedoraproject DOT org> - 0.10.7-3 - Fix license tag - remove rpath - https://bugzilla.redhat.com/show_bug.cgi?id=714328#c3 - Fix sed Thanks! Ankur
http://koji.fedoraproject.org/koji/taskinfo?taskID=3261863
== Review == Here are the must fix items: * The xmedcon-config binary belongs in the -devel subpackage. * The -devel package must require the main subpackage with %{?_isa}: Requires: %{name}%{?_isa} = %{version}-%{release} https://fedoraproject.org/wiki/Packaging/Guidelines#Requires This prevents mismatch in multilib scenarios. * There must be a desktop file (and an icon) for xmedcon. https://fedoraproject.org/wiki/Packaging/Guidelines#Desktop_files You might ask upstream for an icon, or use one of the generic icons for an image tool. Here are the optional fixes: * You're using %defattr(-,root,root,-) in %files sections, but this is now the default in all active Fedora branches. Consider removing it, although, this is not a blocker. == Details == - rpmlint checks return: xmedcon.src: W: spelling-error %description -l en_US Acr -> Ac, Ar, Apr xmedcon.src: W: spelling-error %description -l en_US uPET -> u Pet, PET, u PET xmedcon.x86_64: W: spelling-error %description -l en_US Acr -> Ac, Ar, Apr xmedcon.x86_64: W: spelling-error %description -l en_US uPET -> u Pet, PET, u PET xmedcon-devel.x86_64: W: spelling-error %description -l en_US libmdc -> libido All spelling errors safe to ignore. xmedcon.x86_64: W: shared-lib-calls-exit /usr/lib64/libmdc.so.2.0.1 exit.5 Safe to ignore. xmedcon.x86_64: W: devel-file-in-non-devel-package /usr/bin/xmedcon-config The xmedcon-config binary belongs in the -devel subpackage, this is a must-fix. xmedcon.x86_64: E: incorrect-fsf-address /usr/share/doc/xmedcon-0.10.7/COPYING.LIB xmedcon.x86_64: E: incorrect-fsf-address /usr/share/doc/xmedcon-0.10.7/COPYING xmedcon-devel.x86_64: E: incorrect-fsf-address /usr/share/doc/xmedcon-devel-0.10.7/COPYING.LIB xmedcon-devel.x86_64: E: incorrect-fsf-address /usr/share/doc/xmedcon-devel-0.10.7/COPYING Please inform the xmedcon upstream that they are using an outdated copy of the FSF license texts with the old FSF address, and ask them to please update this in their next release. - package meets naming guidelines - package meets packaging guidelines - license (LGPLv2+ and Copyright only and MIT and BSD and libtiff) OK, text in %doc, matches source - spec file legible, in am. english - source matches upstream: bc76d1edbe8e65bbea8afeca8a1d44a7d5e286a1befb5d42a743d1bfc6fe5016 - package compiles on devel (koji scratch OK) - no missing BR - no unnecessary BR - no locales - not relocatable - owns all directories that it creates - no duplicate files - permissions ok - macro use consistent - code, not content - no need for -docs - nothing in %doc affects runtime - desktop file missing - devel package ok (except for misplaced xmedcon-config binary) - no .la files - post/postun ldconfig ok - devel requires base package n-v-r, but is missing %{_isa}
Hi spot, Updated spec/srpm with issues corrected are here: http://ankursinha.fedorapeople.org/xmedcon/xmedcon.spec http://ankursinha.fedorapeople.org/xmedcon/xmedcon-0.10.7-4.fc15.src.rpm * Tue Aug 09 2011 Ankur Sinha <ankursinha AT fedoraproject DOT org> - 0.10.7-4 - Move xmedcon-config to -devel - Correct requires for -devel - Add icon, scriptlets - Add desktop file, scriptlets - Add a xmedconrc.linux file in docs - Remove defattr Thank you, Ankur
Nice work. Approved.
New Package SCM Request ======================= Package Name: xmedcon Short Description: A medical image conversion utility and library Owners: ankursinha Branches: f14 f15 f16 InitialCC: susmit mrceresa
Git done (by process-git-requests). Please take ownership of review BZs. Thanks!
xmedcon-0.10.7-4.fc15 has been submitted as an update for Fedora 15. https://admin.fedoraproject.org/updates/xmedcon-0.10.7-4.fc15
xmedcon-0.10.7-4.fc14 has been submitted as an update for Fedora 14. https://admin.fedoraproject.org/updates/xmedcon-0.10.7-4.fc14
xmedcon-0.10.7-4.fc16 has been submitted as an update for Fedora 16. https://admin.fedoraproject.org/updates/xmedcon-0.10.7-4.fc16
Hello, Build for f14,15,16 and rawhide. Closing. Thank you for the review spot :) Ankur
xmedcon-0.10.7-4.fc14 has been pushed to the Fedora 14 stable repository.
xmedcon-0.10.7-4.fc15 has been pushed to the Fedora 15 stable repository.
xmedcon-0.10.7-4.fc16 has been pushed to the Fedora 16 stable repository.