Bug 300651
Summary: | Review Request: baekmuk-ttf-fonts - Korean truetype fonts (from fonts-korean) | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Jens Petersen <petersen> |
Component: | Package Review | Assignee: | Mamoru TASAKA <mtasaka> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | high | ||
Version: | rawhide | CC: | eng-i18n-bugs, fedora-package-review, K9, mtasaka, notting, panemade |
Target Milestone: | --- | Keywords: | i18n |
Target Release: | --- | Flags: | mtasaka:
fedora-review+
petersen: fedora-cvs+ |
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2007-09-24 08:31:10 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | |||
Bug Blocks: | 235704, 253155, 302451 |
Description
Jens Petersen
2007-09-21 15:53:29 UTC
oops please use these corrects urls: Spec URL: http://petersen.fedorapeople.org/baekmuk-ttf/baekmuk-ttf-fonts.spec SRPM URL: http://petersen.fedorapeople.org/baekmuk-ttf/baekmuk-ttf-fonts-2.2-1.fc7.src.rpm [28MB] If possible I would really like to get this included in f8test3 since fonts-korean is a very large package (actually the largest font package in the distro iirc). I just sent the license file to fedora-legal for classification. Removed tabs from .spec file (only): http://petersen.fedorapeople.org/baekmuk-ttf/baekmuk-ttf-fonts.spec The license is now on the Licensing page as Baekmuk: Spec URL: http://fedorapeople.org/petersen/baekmuk-ttf/baekmuk-ttf-fonts.spec SRPM URL: http://fedorapeople.org/petersen/baekmuk-ttf/baekmuk-ttf-fonts-2.2-2.fc7.src.rpm [28MB] Some remarks and questions * Use "-p" option for "cp" or "install" to keep timestamp ? So FAPIcidfmap.ko and cidfmap.ko, COPYRIGHT* README are instaled in each subpackage? ? Why can each subpackage have "Provides: ttfonts-ko = %{ttfontverhead}"? I guess only when all subpackages are installed, they can provide "ttfonts-ko = %{ttfontverhead}" for consistency with the state before kkfonts related packages are seperated? * /etc should be %_sysconfdir ? What happens if /usr/bin/fc-cache fails by some reason? (In reply to comment #5) > * Use "-p" option for "cp" or "install" to keep timestamp Not sure what you mean here: I don't see any cp usage currently in .spec. > ? So FAPIcidfmap.ko and cidfmap.ko, COPYRIGHT* README are instaled > in each subpackage? Currently, yes. Installing the CID fmap files in each package is not ideal, but I can't see a better way without introducing some messy scripts say and it should not have any particular adverse affects I think. The %doc files are all small so I don't see a big problem with duplicating them in each subpackage. > ? Why can each subpackage have "Provides: ttfonts-ko = %{ttfontverhead}"? > I guess only when all subpackages are installed, they can provide > "ttfonts-ko = %{ttfontverhead}" for consistency with the state before > kkfonts related packages are seperated? Ok, let me remove the Provides. > * /etc should be %_sysconfdir ah yes, thanks > ? What happens if /usr/bin/fc-cache fails by some reason? Shrug, I am just using the standard Font scriptlets, but if you want I can protect the fc-cache invocations in %post and %postun. Spec URL: http://fedorapeople.org/petersen/baekmuk-ttf/baekmuk-ttf-fonts.spec SRPM URL: http://fedorapeople.org/petersen/baekmuk-ttf/baekmuk-ttf-fonts-2.2-3.fc7.src.rpm [28MB] (In reply to comment #6) > (In reply to comment #5) > > * Use "-p" option for "cp" or "install" to keep timestamp > > Not sure what you mean here: I don't see any cp usage currently in .spec. * For example, -------------------------------------------------------- install -m 0644 ttf/$i.ttf $RPM_BUILD_ROOT%{ttfontdir}-$i -------------------------------------------------------- Here "install -m 0644" should be "install -p -m 0644". > > > ? So FAPIcidfmap.ko and cidfmap.ko, COPYRIGHT* README are instaled > > in each subpackage? > > Currently, yes. Installing the CID fmap files in each package is not ideal, > but I can't see a better way without introducing some messy scripts say > and it should not have any particular adverse affects I think. * I think you should - introduce a base package (say: "baekmuk-ttf-fonts-base"), - and have CIP fmap, copying file and other files owned by -base package - and have other subpackages require -base package. > > ? Why can each subpackage have "Provides: ttfonts-ko = %{ttfontverhead}"? > > I guess only when all subpackages are installed, they can provide > > "ttfonts-ko = %{ttfontverhead}" for consistency with the state before > > kkfonts related packages are seperated? > > Ok, let me remove the Provides. ? This means you will write "Provides: ttfonts-ko = <somever>" in original fonts-korean package (and make fonts-korean require all baekmuk fonts subpackages)? > Spec URL: http://fedorapeople.org/petersen/baekmuk-ttf/baekmuk-ttf-fonts.spec ? By the way the URL above is not accessible from me. Instead http://petersen.fedorapeople.org/baekmuk-ttf/baekmuk-ttf-fonts.spec works. (In reply to comment #7) > Here "install -m 0644" should be "install -p -m 0644". Ok, thanks - fixing. :) > - introduce a base package (say: "baekmuk-ttf-fonts-base"), > - and have CIP fmap, copying file and other files owned by -base > package > - and have other subpackages require -base package. Ok, thanks for that - good idea. I called it -common for now. > > Ok, let me remove the Provides. > ? This means you will write "Provides: ttfonts-ko = <somever>" in > original fonts-korean package (and make fonts-korean require > all baekmuk fonts subpackages)? fonts-korean will require all the baekmuk packages for the time being yes (certainly for F8). I am kind of tempted to just drop the ttfonts-ko provides though, since we don't really want to provide it forever, but if necessary we could still provide it I suppose. > > Spec URL: http://fedorapeople.org/petersen/baekmuk-ttf/baekmuk-ttf-fonts.spec > ? By the way the URL above is not accessible from me. Instead > http://petersen.fedorapeople.org/baekmuk-ttf/baekmuk-ttf-fonts.spec > works. Ugh, sorry about that: thought I was editing the fixed url. Spec URL: http://petersen.fedorapeople.org/baekmuk-ttf/baekmuk-ttf-fonts.spec SRPM URL: http://petersen.fedorapeople.org/baekmuk-ttf/baekmuk-ttf-fonts-2.2-4.fc7.src.rpm [28MB] Well, * Still CID fmap is owned by every subpackage. ? By the way, how do we treat the ownership of %_datadir/ghostscript/conf.d ? - On my system, currently the following packages owns %_datadir/ghostscript/conf.d ------------------------------------------------------------- ghostscript-8.60-2.fc8 fonts-korean-2.2-4.fc8 fonts-japanese-0.20061016-11.fc8 ------------------------------------------------------------- - Now as fonts-korean will require baekmuk fonts subpackages, baekmuk-ttf-fonts-common should own the directory? - IMO, in the future the ownership of the directory (%_datadir/ghostscript/fonts.d) should be moved to filesystem as well as %_datadir/ghostscript? One more thing * Please update the contents of CID fmap files. Currently They referes to /usr/share/fonts/korean/TrueType/batang.ttf or so. One more thing ? Maybe it is better that -common package is marked as in conflict with "fonts-korean < 2.2-5" ? Thanks, Tasaka-san for all your good comments. :) (In reply to comment #9) > * Still CID fmap is owned by every subpackage. Oops, fixing > ? By the way, how do we treat the ownership of > %_datadir/ghostscript/conf.d ? > - Now as fonts-korean will require baekmuk fonts subpackages, > baekmuk-ttf-fonts-common should own the directory? Agreed. We don't want to require ghostscript. > - IMO, in the future the ownership of the directory > (%_datadir/ghostscript/fonts.d) should be moved to > filesystem as well as %_datadir/ghostscript? Yes, that would probably be good, though then %_datadir/ghostscript would have to be there too I guess. > * Please update the contents of CID fmap files. > Currently They refer to /usr/share/fonts/korean/TrueType/batang.ttf ... Thanks for catching that - I had forgotten about it. > One more thing > ? Maybe it is better that -common package is marked as in conflict > with "fonts-korean < 2.2-5" ? Ok, thanks added that too. Spec URL: http://petersen.fedorapeople.org/baekmuk-ttf/baekmuk-ttf-fonts.spec SRPM URL: http://petersen.fedorapeople.org/baekmuk-ttf/baekmuk-ttf-fonts-2.2-5.fc7.src.rpm [28MB] > - IMO, in the future the ownership of the directory > (%_datadir/ghostscript/fonts.d) should be moved to > filesystem as well as %_datadir/ghostscript? I filed bug 302521 for this. Okay. ------------------------------------------------------- This package (baekmuk-ttk-fonts) is APPROVED by me ------------------------------------------------------- Note: tarball in -5 srpm seems broken (tarball in -4 srpm is okay) Question: Maybe it is better COPYRIGHT.ks is converted to UTF-8 (I don't know with what encoding this file is encoded) mtasaka, Thanks for your quick review here. I have request to you. Sorry to ask here. But I can't see you on IRC. Can you also please review following Japanese font packages please? https://bugzilla.redhat.com/show_bug.cgi?id=266261 https://bugzilla.redhat.com/show_bug.cgi?id=253175 Thanks for the quick thorough review. :) (In reply to comment #14) > Note: tarball in -5 srpm seems broken (tarball in -4 srpm is okay) Sorry, I think it is because I went over my quota on fedorapeople. :-/ Just to confirm the correct source file is: a6f4349179cbe3557641782cefba4d70 baekmuk-ttf-2.2.tar.gz It is a bit worrying though if rpm doesn't know that the srpm is incomplete... > Maybe it is better COPYRIGHT.ks is converted to UTF-8 > (I don't know with what encoding this file is encoded) Thanks, I will convert it. New Package CVS Request ======================= Package Name: baekmuk-ttf-fonts Short Description: Korean TrueType fonts Owners: cchance Branches: devel InitialCC: petersen Cvsextras Commits: yes (In reply to comment #15) > mtasaka, > Thanks for your quick review here. I have request to you. Sorry to ask here. > But I can't see you on IRC. Can you also please review following Japanese font > packages please? > https://bugzilla.redhat.com/show_bug.cgi?id=266261 > https://bugzilla.redhat.com/show_bug.cgi?id=253175 Well, I first try bug 302451 (IMO this can be accepted soon), then try them By the way, until what time are you checking review requests comments? (In reply to comment #16) > (In reply to comment #14) > > Note: tarball in -5 srpm seems broken (tarball in -4 srpm is okay) > Just to confirm the correct source file is: > a6f4349179cbe3557641782cefba4d70 baekmuk-ttf-2.2.tar.gz This tarball is okay. (In reply to comment #18) > (In reply to comment #15) > > mtasaka, > > Thanks for your quick review here. I have request to you. Sorry to ask here. > > But I can't see you on IRC. Can you also please review following Japanese font > > packages please? > > https://bugzilla.redhat.com/show_bug.cgi?id=266261 > > https://bugzilla.redhat.com/show_bug.cgi?id=253175 > > Well, I first try bug 302451 (IMO this can be accepted soon), > then try them Sure. Please. > By the way, until what time are you checking review requests comments? > If I understood you correctly I am working in IST timezone 9.30 am to 6.30 pm cvsadmin done (except owner-sync-pkgdb, which is failing currently) imported package to cvs baekmuk-ttf-fonts-2.2-6.fc8 built in koji now :) Thanks for the review. |