Spec URL: http://tuanta.fedorapeople.org/ibus-unikey.spec SRPM URL: http://tuanta.fedorapeople.org/ibus-unikey-0.5.1-1.fc14.src.rpm Description: A Vietnamese engine for IBus input platform that uses Unikey.
Review for 194d2fae199c9f49dead1e45172c3e00 ibus-unikey-0.5.1-1.fc14.src.rpm OK - MUST: $ rpmlint /var/lib/mock/fedora-14-x86_64/result/ibus-unikey-* ibus-unikey.src: W: invalid-url Source0: http://ibus-unikey.googlecode.com/files/ibus-unikey-0.5.1.tar.gz HTTP Error 404: Not Found 3 packages and 0 specfiles checked; 0 errors, 1 warnings. Warning can be ignored, happens with all files on Google Code. I have verified the URL with spectool. OK - MUST: named according to the Package Naming Guidelines OK - MUST: spec file name matches the base package %{name} FIX - MUST: package does not meet the Packaging Guidelines, see below OK - MUST: Fedora approved license and meets the Licensing Guidelines (GPLv3) OK - MUST: License field in spec file matches the actual license OK - MUST: license file included in %doc OK - MUST: spec is in American English OK - MUST: spec is legible OK - MUST: sources match the upstream source by MD5 0b8f79941dc3e9a4744d52e88e4401dc OK - MUST: successfully compiles and builds into binary rpms on F14 x86_64 N/A - MUST: If the package does not successfully compile, build or work on an architecture, then those architectures should be listed in the spec in ExcludeArch. OK - MUST: all build dependencies are listed in BuildRequires. FIX - MUST: does not handle locales properly with %find_lang, see below N/A - MUST: Every binary RPM package (or subpackage) which stores shared library files (not just symlinks) in any of the dynamic linker's default paths, must call ldconfig in %post and %postun. OK - MUST: Package does not bundle copies of system libraries. N/A - MUST: If the package is designed to be relocatable, the packager must state this fact in the request for review, along with the rationalization for relocation of that specific package. FIX - MUST: does not own all directories that it creates, see below OK - MUST: no duplicate files in the %files listing OK - MUST: Permissions on files are set properly, includes %defattr(...) OK - MUST: consistently uses macros OK - MUST: package contains code, or permissable content N/A - MUST: Large documentation files should go in a -doc subpackage OK - MUST: Files included as %doc do not affect the runtime of the application N/A - MUST: Header files must be in a -devel package N/A - MUST: Static libraries must be in a -static package N/A - MUST: library files that end in .so are in the -devel package. N/A - MUST: devel packages must require the base package using a fully versioned dependency OK - MUST: The package does not contain any .la libtool archives. N/A - MUST: Packages containing GUI applications must include a %{name}.desktop file. FIX - MUST: package does owns own files or directories already owned by other packages, seel below OK - Should: at the beginning of %install, the package runs rm -rf $RPM_BUILD_ROOT. OK - MUST: all filenames valid UTF-8 SHOULD Items: OK - SHOULD: Source package includes license text(s) as a separate file. N/A - SHOULD: The description and summary sections in the package spec file should contain translations for supported Non-English languages, if available. OK - SHOULD: builds in mock. OK - SHOULD: compiles and builds into binary rpms on all supported architectures. OK - SHOULD: functions as described. OK - SHOULD: Scriptlets are sane. N/A - SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency. N/A - SHOULD: pkgconfig(.pc) files should be placed in a -devel pkg OK - SHOULD: no file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin N/A - SHOULD: package should contain man pages for binaries/scripts. Other items: OK - latest stable version OK - SourceURL valid OK - Compiler flags ok OK - Debuginfo complete OK - SHOULD: package has a %clean section, which contains rm -rf %{buildroot} ( or $RPM_BUILD_ROOT ). N/A - SHOULD: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'. Issues: Use of %{_datadir}/locale/... is forbidden, use %find_lang instead. See https://fedoraproject.org/wiki/Packaging:Guidelines#Handling_Locale_Files The package does not own all folders that is creates: %{_datadir}/%{name}/* will only own the files inside that folder, use %{_datadir}/%{name}/ instead. See https://fedoraproject.org/wiki/Packaging:UnownedDirectories The package owns the folder %{_datadir}/ibus/component/, which is already owned by the ibus package. Since ibus is a dependency of this package it should only own %{_datadir}/ibus/component/unikey.xml. See https://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership The package does not build on F15, attaching a build.log. You might want to contact upstream about this.
Created attachment 468849 [details] build.log of F15 x86_64
I updated all "MUST FIX" issues into the spec file; rebuilt the source package and then re-uploaded both onto the same location on FedoraPeople space. Please review the updates for me. Tell me anything I should do more. Regarding to the building issues on the upcoming release F15, I am contacting to upstream. Thanks, Tuan
I made some small changes in %changelog to meet the Guidelines at https://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs Btw, do I need to process "N/A" issues?
I got response from upstream developer. It could not be compiled for F15 because of some compatibility issues (with ibus 1.3.99) those would be fixed next month. At this time, we should go ahead with F14, then I will follow up with upstream for the next releases.
(In reply to comment #3) > I updated all "MUST FIX" issues into the spec file; rebuilt the source package > and then re-uploaded both onto the same location on FedoraPeople space. Whenever you make changes to a package, you must increase the release, even durign review. You should also note the changes you did in a changelog entry. > Please review the updates for me. Tell me anything I should do more. When you give me a -2 package, I will review the remaining points. (In reply to comment #4) > Btw, do I need to process "N/A" issues? Not Applicable = the guideline is does not meet this package, so you don't need to care about it here. (In reply to comment #5) > At this time, we should go ahead with F14, then I will follow up with upstream > for the next releases. I'm sorry, but this is not possible because we need a clean upgrade path. rawhide always needs to have the highest version, otherwise people cannot upgrade from F14 to rawhide. When you do an update or a new package, you first to it in rawhide, then in F14 and then F13. Maybe you can pick changes from SVN when they are available, but without rawhide I cannot accept the package.
(In reply to comment #6) > I'm sorry, but this is not possible because we need a clean upgrade path. > rawhide always needs to have the highest version, otherwise people cannot > upgrade from F14 to rawhide. When you do an update or a new package, you first > to it in rawhide, then in F14 and then F13. > > Maybe you can pick changes from SVN when they are available, but without > rawhide I cannot accept the package. This made sense to me. So I should wait for changes from upstream to rebuild the package on f15-dist; then the review process would be resumed. Thanks for all. Tuan
We can still finish the review but you cannot yet import the package. Please give me a proper ibus-unikey-0.5.1-2.fc14.src.rpm so I can check the last points.
I updated .spec file, rebuilt the .src file and uploaded both. Please review those new ones.
The .spec file is still on: http://tuanta.fedorapeople.org/ibus-unikey.spec The new .src file is on: http://tuanta.fedorapeople.org/ibus-unikey-0.5.1-2.fc14.src.rpm
I'm incredibly sorry, this completely dropped of my radar. :( From a packaging point of view, the package is fine now: OK - MUST: package meets the Packaging Guidelines OK - MUST: handles locales properly with %find_lang OK - MUST: owns all directories that it creates OK - MUST: package does not own files or directories already owned by other packages Some minor notes on the changelog: - mention version and release - mention your changes - use one blank line between al changelog entries A proper changelog would look like this: %changelog * Mon Dec 20 2010 Truong Anh Tuan <tuanta> - 0.5.1-2 - Fix file and directory ownership (#662604) - Use %%find_lang macro for locales * Mon Dec 13 2010 Truong Anh Tuan <tuanta> - 0.5.1-1 - Initial release 0.5.1 getting from upstream If you use a macro in a comment/changelog, you need to escape it with a second %. If you fix a but, add the number at the More about changelogs can be found at https://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs I am really sorry I cannot approve this package because it still doesn't build in Fedora 15. Did you contact the developers and showed them the build.log?
(In reply to comment #11) > I am really sorry I cannot approve this package because it still doesn't build > in Fedora 15. Did you contact the developers and showed them the build.log? It's a compatibility issue. I am working with upstream developer to get it built fine on Fedora 15. I will also update all others. Rgds, Tuan
(In reply to comment #12) > It's a compatibility issue. I am working with upstream developer to get it > built fine on Fedora 15. FWIW, I created a patch for ibus-unikey to use the ibus-1.4 API, which is in Fedora 15: http://ueno.fedorapeople.org/ibus-unikey/ibus-unikey-ibus-1.4.patch You may also find SRPM and spec there: http://ueno.fedorapeople.org/ibus-unikey/ Since actually ibus-1.4 has not released in upstream yet, I maintain internal patches for ibus-hangul and ibus-m17n for Fedora.
Hi there, Thanks. You patch is great. I updated the .spec file to get the patch and recompiled the package successfully. So, please review the updated src and spec file for me. Hope it is good enough now. P.S. the files have been moved to sub-folder so their locations are: http://tuanta.fedorapeople.org/ibus-unikey/ibus-unikey.spec http://tuanta.fedorapeople.org/ibus-unikey/ibus-unikey-0.5.1-4.fc15.src.rpm Rgds, Tuan
With the patch applied the package builds for F15 and rawhide but no longer for F13 and F14. You will either have to maintain two versions of the spec or apply the patch conditionally: %if 0%{?fedora} >= 15 %patch0 -p1 -b .ibus-1.4 %endif For more info have a look at http://fedoraproject.org/wiki/Packaging/DistTag#Conditionals I am now approving this package but please don't forget to add the conditional.
Thanks Christoph. Updated .spec and .src files have been uploaded: http://tuanta.fedorapeople.org/ibus-unikey/ibus-unikey.spec http://tuanta.fedorapeople.org/ibus-unikey/ibus-unikey-0.5.1-5.fc15.src.rpm Please review them. Rgds, Tuan
No need for further review, the package is APPROVED. You can now continue with the SCM admin request as described in http://fedoraproject.org/wiki/Package_SCM_admin_requests
New Package SCM Request ======================= Package Name: ibus-unikey Short Description: A Vietnamese engine for IBus input platform that uses Unikey. Owners: tuanta Branches: f14 f15 InitialCC:
Git done (by process-git-requests).
Thanks for your work, Truong and Christoph! BTW, how about adding "i18n-team" to the InitialCC of the package, so that Fedora I18N people can track future bugs?
Thanks for your helps, Daiki Ueno. My submitted SCM request has been approved. However, I will submit a change request soon.
ibus-unikey-0.5.1-5.fc15 has been submitted as an update for Fedora 15. https://admin.fedoraproject.org/updates/ibus-unikey-0.5.1-5.fc15
ibus-unikey-0.5.1-5.fc14 has been submitted as an update for Fedora 14. https://admin.fedoraproject.org/updates/ibus-unikey-0.5.1-5.fc14
ibus-unikey-0.5.1-5.fc14 has been pushed to the Fedora 14 testing repository.
ibus-unikey-0.5.1-5.fc14 has been pushed to the Fedora 14 stable repository.
ibus-unikey-0.5.1-5.fc15 has been pushed to the Fedora 15 stable repository.