Spec URL: http://tagoh.fedorapeople.org/reviews/mozc/mozc.spec SRPM URL: http://tagoh.fedorapeople.org/reviews/mozc/mozc-0.12.422.102-0.1.20100729svn.fc13.src.rpm Description: Mozc is a Japanese Input Method Editor (IME) designed for multi-platform such as Chromium OS, Windows, Mac and Linux.
First of all would you clarify the following? ./data/dictionary/README.txt - Well, mozc says the overall license is BSD, however - this file (./data/dictionary/README.txt) says that the volaburaly set is taken from ipadic, and the license of ipadic is not the same as BSD. ! Fedora admits that the license of ipadic is free, however is different from BSD at least in that the compatibility with GPL is currently unclear: http://fedoraproject.org/wiki/Licensing - Also some other words seems added to the dictionary in the tarball. Maybe newly added words are licensed under BSD, however it seems unclear to me. Would you check under what license the dictionaries in mozc are actually licensed? ./third_party/rx/v1_0rc2/README - This is under ASL 2.0. ! By the way, there are two third-party products included in mozc tarball (gyp, rx). Generally using bundled libraries is discouraged on Fedora and it is recommended to seperate such bundled libraries https://fedoraproject.org/wiki/Packaging/Guidelines#Bundling_of_multiple_projects Would you create seperated review request for these (if these are really needed)?
By the way I don't know if you want to import this also into F-12, however compile on F-12 ppc64 met with segv: http://koji.fedoraproject.org/koji/taskinfo?taskID=2369262
(In reply to comment #1) > First of all would you clarify the following? > > ./data/dictionary/README.txt > - Well, mozc says the overall license is BSD, however > - this file (./data/dictionary/README.txt) says that > the volaburaly set is taken from ipadic, and > the license of ipadic is not the same as BSD. > ! Fedora admits that the license of ipadic is free, > however is different from BSD at least in that the > compatibility with GPL is currently unclear: > http://fedoraproject.org/wiki/Licensing > > - Also some other words seems added to the dictionary in the > tarball. Maybe newly added words are licensed under BSD, > however it seems unclear to me. > > Would you check under what license the dictionaries in mozc are > actually licensed? Sure. will check it with upstream though, I don't see any issues combining ipadic's license with BSD. > ./third_party/rx/v1_0rc2/README > - This is under ASL 2.0. > ! By the way, there are two third-party products included in mozc > tarball (gyp, rx). Generally using bundled libraries is discouraged > on Fedora and it is recommended to seperate such bundled libraries > > https://fedoraproject.org/wiki/Packaging/Guidelines#Bundling_of_multiple_projects > Would you create seperated review request for these (if these > are really needed)? I've submitted a package review for gyp though, there are no upstream for rx anymore. apparently it may be not supposed to be shipped live for library and a trivial code though, can't we just have a comment about the license for rx in the spec file?
(In reply to comment #3) > (In reply to comment #1) > > First of all would you clarify the following? > > > > ./data/dictionary/README.txt > > Would you check under what license the dictionaries in mozc are > > actually licensed? > > Sure. will check it with upstream though, I don't see any issues combining > ipadic's license with BSD. Yes, the combination of BSD and mecab-ipadic is okay, I just want to make it clarified what license mozc's license is under. > > ./third_party/rx/v1_0rc2/README > > - This is under ASL 2.0. > > ! By the way, there are two third-party products included in mozc > > tarball (gyp, rx). Generally using bundled libraries is discouraged > > on Fedora and it is recommended to seperate such bundled libraries > > > > https://fedoraproject.org/wiki/Packaging/Guidelines#Bundling_of_multiple_projects > > Would you create seperated review request for these (if these > > are really needed)? > > I've submitted a package review for gyp though, there are no upstream for rx > anymore. apparently it may be not supposed to be shipped live for library and a > trivial code though, can't we just have a comment about the license for rx in > the spec file? gyp taken. I guess rx can be shipped in current style (however the license tag of mozc needs fixing, after clarifying dictionary's license).
Updated. Spec URL: http://tagoh.fedorapeople.org/reviews/mozc/mozc.spec SRPM URL: http://tagoh.fedorapeople.org/reviews/mozc/mozc-0.12.434.102-0.1.20100817svn.fc13.src.rpm
Some notes for 0.12.434.102-0.1.20100817svn * License - So the license for dictionary/ directory is * mecab-ipadic (for mecab-ipadic) * BSD (for mecab-naist-jdic) ? If so, "mecab-ipadic" should also be added into license tag (note that "mecab-ipadic" is already registered in http://fedoraproject.org/wiki/Licensing ) * BuildRoot - BuildRoot tag is no longer needed (no longer used) on Fedora 10+ and EPEL6: https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag ? Obsoletes - I don't understand why "Obsoletes: ibus-mozc < 0.11.383.102" is added to main package although mozc has not existed on Fedora. * optflags / make build.log more verbose / debuginfo rpm - Currently build.log shows the output like: --------------------------------------------------------------- 208 + python build_mozc.py build_tools -c Release 209 ACTION base_gen_version_def out_linux/Release/obj/gen/base/version_def.cc 210 ACTION base_gen_character_set out_linux/Release/obj/gen/base/character_set.h 211 CXX(target) out_linux/Release/obj.target/storage/storage/existence_filter.o 212 CXX(target) out_linux/Release/obj.target/storage/storage/lru_storage.o 213 CXX(target) out_linux/Release/obj.target/storage/storage/registry.o 214 CXX(target) out_linux/Release/obj.target/storage/storage/tiny_storage.o --------------------------------------------------------------- With these messages we cannot check if optflags are correctly honored during build. - Also currently debuginfo rpm does not contain needed source files: -------------------------------------------------------------- mozc-debuginfo.i686: E: debuginfo-without-sources -------------------------------------------------------------- This usually means that optflags are not correctly honored (especially "-g" flag), or binaries are stripped during build or install. - So please make build.log more verbose so that we can check if Fedora specific compilation flags are correctly honored. ? %clean - Why is it needed to call "python build_mozc.py clean" after build? (Note that %_builddir%{?buildsubdir} is deleted after build anyway) * %defattr - Please set %defattr also for subpackages. ! ppc64 build failure - By the way your srpm fails to build on F-12 ppc64. It seems that segfault is occuring. ---------------------------------------------------------------- /bin/sh: line 1: 8168 Segmentation fault ../mozc_build_tools/linux/gen_connection_data_main --logtostderr "--input=../data/dictionary/connection.txt" --make_header "--output=/builddir/build/BUILD/mozc-0.12.434.102/out_linux/Release/obj/gen/converter/embedded_connection_data.h" make: *** [out_linux/Release/obj/gen/converter/embedded_connection_data.h] Error 139 make: *** Waiting for unfinished jobs.... ---------------------------------------------------------------- http://koji.fedoraproject.org/koji/taskinfo?taskID=2412449
(In reply to comment #6) > * License > - So the license for dictionary/ directory is > * mecab-ipadic (for mecab-ipadic) > * BSD (for mecab-naist-jdic) > ? If so, "mecab-ipadic" should also be added into license tag > (note that "mecab-ipadic" is already registered in > http://fedoraproject.org/wiki/Licensing ) I should added that. fixed. > * BuildRoot > - BuildRoot tag is no longer needed (no longer used) on > Fedora 10+ and EPEL6: > https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag fixed. > ? Obsoletes > - I don't understand why "Obsoletes: ibus-mozc < 0.11.383.102" is added > to main package although mozc has not existed on Fedora. not for official one but to fix the package changes on local repo for testing. > > * optflags / make build.log more verbose / debuginfo rpm Okay, fixed in gyp. > ? %clean > - Why is it needed to call "python build_mozc.py clean" after build? > (Note that %_builddir%{?buildsubdir} is deleted after build anyway) fixed. > * %defattr > - Please set %defattr also for subpackages. Doh. fixed. > ! ppc64 build failure > - By the way your srpm fails to build on F-12 ppc64. It seems that segfault > is > occuring. > ---------------------------------------------------------------- > /bin/sh: line 1: 8168 Segmentation fault > ../mozc_build_tools/linux/gen_connection_data_main --logtostderr > "--input=../data/dictionary/connection.txt" --make_header > "--output=/builddir/build/BUILD/mozc-0.12.434.102/out_linux/Release/obj/gen/converter/embedded_connection_data.h" > make: *** [out_linux/Release/obj/gen/converter/embedded_connection_data.h] > Error 139 > make: *** Waiting for unfinished jobs.... > ---------------------------------------------------------------- > http://koji.fedoraproject.org/koji/taskinfo?taskID=2412449 Sure. will have a look though, that may be a bit hard to investigate a kinda issue if there are no debugging machine.. just for snapshot of fixes: Spec URL: http://tagoh.fedorapeople.org/reviews/mozc/mozc.spec SRPM URL: http://tagoh.fedorapeople.org/reviews/mozc/mozc-0.12.434.102-0.1.20100820svn.fc13.src.rpm
Created attachment 440023 [details] make build.log verbose Okay. I tried 0.12.434.102-0.1.20100820svn and at least ibus-mozc seems to work on F-14. Then: ? Obsoletes: - I don't see the need of "Obsoletes: ibus-mozc < 0.11.383.102" for another reason. As this srpm creates ibus-mozc subpackage, even if this Obsoletes does not exist the upgrade path shouldn't be broken. * build.log / Fedora specific compilation flags - Still we cannot check if Fedora specific compilation flags are honored or not from build.log. Would you consider to apply the patch attached to show the actual command line on build.log? The result with the attached patch applied is: http://koji.fedoraproject.org/koji/taskinfo?taskID=2414933 * Directory ownership issue - The directory %{_datadir}/ibus-mozc/ is not owned by any packages. * Documents - At least some documents indicating license information should be added to %doc for main (mozc) binary rpm. Would you at least consider to add data/installer/credits_??.html to %doc?
(In reply to comment #8) > Then: > ? Obsoletes: > - I don't see the need of "Obsoletes: ibus-mozc < 0.11.383.102" for another > reason. As this srpm creates ibus-mozc subpackage, even if this Obsoletes > does not exist the upgrade path shouldn't be broken. Okay, I may be confused. let's drop that line then. > * build.log / Fedora specific compilation flags > - Still we cannot check if Fedora specific compilation flags are honored > or not from build.log. Would you consider to apply the patch attached > to show the actual command line on build.log? > The result with the attached patch applied is: > http://koji.fedoraproject.org/koji/taskinfo?taskID=2414933 Thanks. that looks nice. > > * Directory ownership issue > - The directory %{_datadir}/ibus-mozc/ is not owned by any packages. Indeed. fixed. > * Documents > - At least some documents indicating license information should be added > to %doc for main (mozc) binary rpm. > Would you at least consider to add data/installer/credits_??.html to > %doc? Sure. I was thinking of doing that though, it contains the unnecessary license informations too. I'm not sure if it's good to ship it as is or get rid of the unnecessary thing or add another one.
(In reply to comment #9) > > * Documents > > - At least some documents indicating license information should be added > > to %doc for main (mozc) binary rpm. > > Would you at least consider to add data/installer/credits_??.html to > > %doc? > > Sure. I was thinking of doing that though, it contains the unnecessary license > informations too. I'm not sure if it's good to ship it as is or get rid of the > unnecessary thing or add another one. I think shipping data/installer/credits_??.html as it is is (currently) enough for license information.
Okay. the above suggestion should be applied to, except ppc build issue: Spec URL: http://tagoh.fedorapeople.org/reviews/mozc/mozc.spec SRPM URL: http://tagoh.fedorapeople.org/reviews/mozc/mozc-0.12.434.102-0.2.20100820svn.fc13.src.rpm
Okay. One new issue and one issue I forgot to mention... ----------------------------------------------- scim-mozc.i686: W: spurious-executable-perm /usr/share/doc/scim-mozc-0.12.434.102/credits_ja.html scim-mozc.i686: W: spurious-executable-perm /usr/share/doc/scim-mozc-0.12.434.102/credits_en.html ----------------------------------------------- - Please modify the permission ----------------------------------------------- mozc.src:20: W: macro-in-comment %Y mozc.src:20: W: macro-in-comment %m mozc.src:20: W: macro-in-comment %d) ----------------------------------------------- - It is better that % in comments are escaped (by using %%) Please modify the issue above when importing this package into Fedora SCM. ----------------------------------------------- This package (mozc) is APPROVED by mtasaka -----------------------------------------------
Sure. thank you for review! New Package SCM Request ======================= Package Name: mozc Short Description: Opensourced Google Japanese Input Owners: tagoh Branches: f13 f14 InitialCC: i18n-team
Git done (by process-git-requests).
The package has been built for devel only so far, will build for f13 and f14 once the updated gyp is in stable. thanks!
Package Change Request ====================== Package Name: mozc New Branches: el5 el6 Owners: tagoh InitialCC: i18n-team requesting to port mozc packages to epel5 and epel6.
Package Change Request ====================== Package Name: mozc New Branches: epel7 Owners: tagoh InitialCC: i18n-team