Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Review Request: mecab - Yet Another Part-of-Speech and Morphological Analyzer https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=229927 j.w.r.degoede(a)hhs.nl changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED AssignedTo|nobody(a)fedoraproject.org |j.w.r.degoede(a)hhs.nl Flag| |fedora-review? ------- Additional Comments From j.w.r.degoede(a)hhs.nl 2007-02-26 06:59 EST ------- Some initial Should Fix's: * Better document / explain why you've got an empty base package and then a -base package. I think I get it, mecab-dict will buildrequire mecab-devel (and thus mecab-base) if it would actually require mecab plain, then there would be a problem as mecab plain requires macab-dict, which in turn buildrequires mecab plain, chicken and egg problem. Right? * mecab, not mecab base will go into comps, and thus the %description of mecab is what most end users will see. So the decription of mecab should be what you currently have for mecab-base and then the description of mecab-base would be something like: This package contains the actual mecab-files, the mecab package is just a dummy package things are done this way because ...... * Can't we just avoid all this together by not requiring mecab-dict? I understand that mecab is of no use without dicts, but I think there are multiple dictionaries possbile right? Now if a user installs mecab through yum, he will get the dictionary with the shortest name (as that is what yum will choose). Wouldn't it be better to thus let he user install the dict himself instead of doing this through deps? I think its safe to assume that people who want to use mecab no a bit about it as its very specific, and thus know they should install a dict. * Coding Style, as said above this is all SHOULD not MUST. I notice that you use %{__cmd} everywhere instead of just plain "cmd" the Fedora standard sort of is to use just cmd, so use rm instead of %{__rm} I also find this much easier to read. Notice that this also is what the spec templates used throughout fedora contain. But if you're uses to doing things this way and don't want to change it, thats fine too. Once I'm done reviewing this I'll probably never look at that .spec file again. * I also have my questions about the configfile handling. Does mecab-base not work (as a buildrequire) without a config file? Having one file owned and in 2 different packages is very ugly. Also what will happen if i install multiple dicitonaries? Again wouldn't it be better to just let the user add any dictionaries. Or the best would be I think to have a %post in dict packages where they add themselves to the config file through sed magic, or something like that. This is how its handled for fonts and XFs for example. Related to this I also think that you're getting the noreplace flag wrong, noreplace doesn't meant that it cannot be replaced by another package noreplace (AFAIK) means that if the file is modified and you update to a newer mecab-base, that the upgrade then won't replace the conffile when its modified, IOW AFAIK noreplace is exactly what you want. -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug, or are watching the QA contact.

Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Review Request: mecab - Yet Another Part-of-Speech and Morphological Analyzer https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=229927 ------- Additional Comments From mtasaka(a)ioa.s.u-tokyo.ac.jp 2007-02-26 09:24 EST ------- (In reply to comment #2) Actually removing rpath completely simply breaks the test because the binary (not installed) actually requres rpath, and your fix (also seemingly recommended on Fedora) seems just a workaround. Rather, does the following work for you? ----------------------------------------------------- %configure %{__sed} -i -e '/dlsearch/s|\(/[a-z/]*\)lib\([a-z/]*\)|\1lib\2 \1lib64\2|g' libtool %{__make} %{?_smp_mflags} ----------------------------------------------------- -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug, or are watching the QA contact.

Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Review Request: mecab - Yet Another Part-of-Speech and Morphological Analyzer https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=229927 ------- Additional Comments From mtasaka(a)ioa.s.u-tokyo.ac.jp 2007-02-26 10:06 EST ------- Well, here what we are required is simply to remove rpath, so here I include your advice. http://www.ioa.s.u-tokyo.ac.jp/~mtasaka/dist/extras/development/SPECS/mec... http://www.ioa.s.u-tokyo.ac.jp/~mtasaka/dist/extras/development/SRPMS/mec... (well, for normal rpmbuild, I set dist as "fc6_LC"...) --------------------------------------------------------- * Mon Feb 26 2007 Mamoru Tasaka <mtasaka(a)ioa.s.u-tokyo.ac.jp&gt; - 0.94-0.2.pre2 - Remove rpath on 64bits. -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug, or are watching the QA contact.

Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Review Request: mecab - Yet Another Part-of-Speech and Morphological Analyzer https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=229927 ------- Additional Comments From mtasaka(a)ioa.s.u-tokyo.ac.jp 2007-02-26 11:54 EST ------- (In reply to comment #6) Just I didn't look them... Well, your suggestion seems preferable. However I am always annoyed when I review packages * main package requires some data * and the data is provided by several package and only one is required and the views are different between the packages... http://www.ioa.s.u-tokyo.ac.jp/~mtasaka/dist/extras/development/SPECS/mec... http://www.ioa.s.u-tokyo.ac.jp/~mtasaka/dist/extras/development/SRPMS/mec... (for now also write jumandic here) http://www.ioa.s.u-tokyo.ac.jp/~mtasaka/dist/extras/development/SPECS/mec... http://www.ioa.s.u-tokyo.ac.jp/~mtasaka/dist/extras/development/SRPMS/mec... * Tue Feb 27 2007 Mamoru Tasaka <mtasaka(a)ioa.s.u-tokyo.ac.jp&gt; - 0.94-0.3.pre2 - Package requirement deps reconstruct * Tue Feb 27 2007 Mamoru Tasaka <mtasaka(a)ioa.s.u-tokyo.ac.jp&gt; - 5.1.20051121-2 - Package requirement deps reconstruct -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug, or are watching the QA contact.

Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Review Request: mecab - Yet Another Part-of-Speech and Morphological Analyzer https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=229927 mtasaka(a)ioa.s.u-tokyo.ac.jp changed: What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs? ------- Additional Comments From mtasaka(a)ioa.s.u-tokyo.ac.jp 2007-02-26 13:41 EST ------- Thank you!! Request to CVS admin about new package: --------------------------------------------- New Package CVS Request ======================= Package Name: mecab Short Description: Yet Another Part-of-Speech and Morphological Analyzer Owners: mtasaka(a)ioa.s.u-tokyo.ac.jp Branches: FC-devel FC-6 FC-5 InitialCC: (nobody) --------------------------------------------- -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug, or are watching the QA contact.

Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Review Request: mecab - Yet Another Part-of-Speech and Morphological Analyzer https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=229927 ------- Additional Comments From mtasaka(a)ioa.s.u-tokyo.ac.jp 2007-03-02 08:18 EST ------- Hans, will you have a time to review mecab-jumandic again? -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug, or are watching the QA contact.