Bug 619395

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.

Git done (by process-git-requests).

Package Change Request
======================
Package Name: mozc
New Branches: epel7
Owners: tagoh
InitialCC: i18n-team

Git done (by process-git-requests).