Bug 453016 - Review Request: un-core-fonts - Korean TrueType fonts
Review Request: un-core-fonts - Korean TrueType fonts
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Jens Petersen
Fedora Extras Quality Assurance
:
Depends On:
Blocks: 453017
  Show dependency treegraph
 
Reported: 2008-06-26 13:49 EDT by Dennis Jang
Modified: 2008-11-06 21:58 EST (History)
6 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
: 651990 (view as bug list)
Environment:
Last Closed: 2008-09-04 23:10:16 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
petersen: fedora_requires_release_note+
petersen: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)
un-fonts-core.spec-1.patch (1.97 KB, patch)
2008-07-01 03:19 EDT, Jens Petersen
no flags Details | Diff
un-fonts-core.spec-3.patch (1.31 KB, patch)
2008-07-04 04:04 EDT, Jens Petersen
no flags Details | Diff
un-core-fonts.spec-5.patch (2.40 KB, patch)
2008-07-16 03:35 EDT, Jens Petersen
no flags Details | Diff
updated spec file (3.25 KB, text/plain)
2008-07-21 01:17 EDT, Dennis Jang
no flags Details

  None (edit)
Description Dennis Jang 2008-06-26 13:49:27 EDT
Spec URL: http://get9.net/rpm/un-fonts-core.spec
SRPM URL: http://get9.net/rpm/un-fonts-core-1.0-1.src.rpm
Description: This package provides more improved free Korean Truetype fonts.

Dennis Jang
Comment 1 Jens Petersen 2008-06-27 03:46:36 EDT
rpmlint says:

 un-fonts-core.noarch: W: incoherent-version-in-changelog 1.0.2-080608-1 1.0-1.fc9

How about using the latest released 1.0.2-080608 tarball?
(Also since you note it in the changelog.:)

I think the version can be just 1.0.2 and then the datestamp can go into the release
field following http://fedoraproject.org/wiki/Packaging/NamingGuidelines.

Probably 
 %define fontdir     %{_datadir}/fonts/%{name}
 %define archivename   un-fonts

 Name:           un-fonts-core

would be ok for this package.

 un-fonts-core.src: W: invalid-license GPL v2

The license url http://korea.gnu.org/copyleft/gpl.ko.html embedded in the ttf fonts
is broken.  Could you please contact upstream to clarify their intention on the GPL
version.  It would be better if the fonts files just stated the GPL version
(eg "version 2 or later") than referring to a url which might change or disappear.
Anyway currently the License tag should be GPLv2+ or maybe GPLv2.

(There seems to be a translation at
http://korea.gnu.org/documents/copyleft/gpl.ko.html
but again Fedora, like the FSF, would not recognise the unofficial translation.
 So it
does not really help.)

You don't use xorg-x11-font-utils or ttmkfdir so those buildrequires can be dropped.

It would be good to have a bit more information in the description.
There can also be summary and description in Korean added.
Comment 2 Dennis Jang 2008-06-28 07:19:48 EDT
Summary: Un series Korean TrueType fonts

Spec URL: http://get9.net/rpm/un-fonts-core.spec
SRPM URL: http://get9.net/rpm/un-fonts-core-1.0.2.080608-1.fc9.src.rpm
Description: 
This is a set of Korean TrueType fonts. Un-fonts is comes from the HLaTeX as 
type1 fonts in 1998 by Koaunghi Un
It converted to TrueType with the FontForge(PfaEdit) by Won-kyu Park in 2003.
This package has only the most common font families.
Install un-fonts-extra for additional fonts.


#1 Rebuild and repackaging for rpmlint
#2 Changed package name to Un series Korean TrueType fonts
#3 Summary and description of the changes, added to Korean
#4 Added %define archiveversion 080608, because 080608 is snapshot version
#5 I have a problem in korean spacing words for rpmlint. I don't fix it
#6 fixed License: GPLv2+

-- Problem #5
# rpmlint -i un-fonts-core.spec 
un-fonts-core.spec:25: W: non-break-space line 25
The spec file contains a non-break space, which looks like a regular space in
some editors but can lead to obscure errors. It should be replaced by a
regular space.

un-fonts-core.spec:27: W: non-break-space line 27
The spec file contains a non-break space, which looks like a regular space in
some editors but can lead to obscure errors. It should be replaced by a
regular space.

un-fonts-core.spec:28: W: non-break-space line 28
The spec file contains a non-break space, which looks like a regular space in
some editors but can lead to obscure errors. It should be replaced by a
regular space.

0 packages and 1 specfiles checked; 0 errors, 3 warnings.
Comment 3 Jens Petersen 2008-07-01 03:08:00 EDT
Thanks, Dennis for the clear update.

> #1 Rebuild and repackaging for rpmlint
> #2 Changed package name to Un series Korean TrueType fonts
> #3 Summary and description of the changes, added to Korean

Thanks

> #4 Added %define archiveversion 080608, because 080608 is snapshot version

OK - I am still a little unclear on the meaning of the snapshot date in relation to
the version 1.0.2.  Does it mean prerelease or postrelease or is it just a label?

In any case I don't think it should go into the version field but the release:

> #5 I have a problem in korean spacing words for rpmlint. I don't fix it

Okay - maybe we should file a bug for rpmlint for it.
Comment 4 Jens Petersen 2008-07-01 03:16:28 EDT
BTW just a note that if upstream released the fontforge .sfd files
it would be better to build those than shipping the .ttf files but that is not
blocker.
Comment 5 Jens Petersen 2008-07-01 03:18:17 EDT
(In reply to comment #2)
> #6 fixed License: GPLv2+

Did you check upstream about the GPL version and ask if they will update the license
text in the fonts themselves?
Comment 6 Jens Petersen 2008-07-01 03:19:04 EDT
Created attachment 310643 [details]
un-fonts-core.spec-1.patch

some suggested changes
Comment 7 Jens Petersen 2008-07-01 03:26:14 EDT
Also please update the changelog in the spec file next time you update.
Comment 8 Jens Petersen 2008-07-01 03:32:26 EDT
One more thing: since the fonts are big and probably most non-Korean users won't
need all of them installed I would like to subpackage the less used fonts so that
we can install just main ones by default and the rest for Korean users.

Default font for Korean should be Batang?
Comment 9 Jens Petersen 2008-07-01 03:46:01 EDT
> Default font for Korean should be Batang?

I tested now - well I guess the default Un Dotum should be ok?
Comment 10 Dennis Jang 2008-07-01 06:15:40 EDT
(In reply to comment #9)
> > Default font for Korean should be Batang?
> I tested now - well I guess the default Un Dotum should be ok?

Debin and ubuntu at default korean font is Un Dotum ^^

Comment 11 Dennis Jang 2008-07-01 06:19:39 EDT
(In reply to comment #5)
> (In reply to comment #2)
> > #6 fixed License: GPLv2+
> Did you check upstream about the GPL version and ask if they will update the 
license
> text in the fonts themselves?

I asked them to check again. They gave answers to a GPLv2
Comment 12 Jens Petersen 2008-07-02 05:45:27 EDT
> I asked them to check again. They gave answers to a GPLv2

Thanks.  So not GPLv2+ (ie "version 2 or later")?
Comment 13 Dennis Jang 2008-07-03 01:35:35 EDT
(In reply to comment #3)
> > #4 Added %define archiveversion 080608, because 080608 is snapshot version
> OK - I am still a little unclear on the meaning of the snapshot date in 
relation to
> the version 1.0.2.  Does it mean prerelease or postrelease or is it just a 
label?
> In any case I don't think it should go into the version field but the release:

this version is prerelease and  1.0 is stable release
how about you packaging version is 1.0 or 1.0.2-prerelease?

I think a 1.0.2-stable release will take a long time

Comment 14 Dennis Jang 2008-07-03 02:05:45 EDT
Spec URL: http://get9.net/rpm/un-fonts-core.spec
SRPM URL: http://get9.net/rpm/un-fonts-core-1.0.2-2.080608.fc9.src.rpm
diff URL: http://get9.net/rpm/un-fonts-core.spec-2.patch

- refined .spec literal, license, versioning contents.
#1 Changes to licenes GPLv2+ to GPLv2.
#2 Changes to versioning 1.0.2.080608-2 to 1.0.2-2.080608
Comment 15 sangu 2008-07-04 00:25:50 EDT
(In reply to comment #9)
> > Default font for Korean should be Batang?
> 
> I tested now - well I guess the default Un Dotum should be ok?

Default font is UnDotum in fontconfig-2.6.0-2.fc10<
http://koji.fedoraproject.org/koji/buildinfo?buildID=51207 >.

See Also : http://bugs.freedesktop.org/show_bug.cgi?id=13569 : Korean font in
the default config - replacing baekmuk with un
Comment 16 Jens Petersen 2008-07-04 03:46:34 EDT
(In reply to comment #12)
> Thanks.  So not GPLv2+ (ie "version 2 or later")?

About the license please see also http://fedoraproject.org/wiki/UN_fonts#Caveats
Comment 17 Jens Petersen 2008-07-04 03:53:55 EDT
(In reply to comment #13)
> this version is prerelease and  1.0 is stable release
> how about you packaging version is 1.0 or 1.0.2-prerelease?
> 
> I think a 1.0.2-stable release will take a long time

Oh I see. So I had misunderstood the upstream versioning intention.

Then I am sorry but better we revert to 1.0 unless there are
strong reasons for shipping a snapshot.
Comment 18 Jens Petersen 2008-07-04 04:04:19 EDT
Created attachment 311012 [details]
un-fonts-core.spec-3.patch

simple patch just to revert to the stable release
Comment 19 Nicolas Mailhot 2008-07-04 06:15:13 EDT
(In reply to comment #16)

> About the license please see also http://fedoraproject.org/wiki/UN_fonts#Caveats

Yes, please update the wiki page (change its status)
http://fedoraproject.org/wiki/UN_fonts

and follow our fonts packaging instructions
http://fedoraproject.org/wiki/Category:Fonts_packaging
Comment 20 Nicolas Mailhot 2008-07-04 17:40:27 EDT
(In reply to comment #17)
 
> Then I am sorry but better we revert to 1.0 unless there are
> strong reasons for shipping a snapshot.

There are 4 more MiBs of content with 4 years of work since 1.0. Is that strong
enough?

However the package should use pre-release conventions
http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Pre-Release_packages
Comment 21 Dennis Jang 2008-07-06 08:31:58 EDT
Summary: Un Core families Korean TrueType fonts

Spec URL: http://get9.net/rpm/1.0/un-core-fonts.spec
SRPM URL: http://get9.net/rpm/1.0/un-core-fonts-1.0-3.fc9.src.rpm
Description: 
This is a set of Korean TrueType fonts. Un-fonts come from the HLaTeX type1
fonts made by Koaunghi Un in 1998. They were converted to TrueType with
FontForge(PfaEdit) by Won-kyu Park in 2003.

Core families (9 fonts)
 * UnBatang, UnBatangBold: serif
 * UnDotum, UnDotumBold: sans-serif
 * UnGraphic, UnGraphicBold: sans-serif style
 * UnPilgi, UnPilgiBold: script
 * UnGungseo: cursive, brush-stroke

Install un-extra-fonts for additional fonts.

and 1.0.2-pre repackaging
http://get9.net/rpm/1.0.2-080608/un-core-fonts-1.0.2-0.3.080608.fc9.src.rpm
http://get9.net/rpm/1.0.2-080608/un-core-fonts.spec
Comment 22 Dennis Jang 2008-07-06 08:33:14 EDT
(In reply to comment #17)
> (In reply to comment #13)
> > this version is prerelease and  1.0 is stable release
> > how about you packaging version is 1.0 or 1.0.2-prerelease?
> > 
> > I think a 1.0.2-stable release will take a long time
> Oh I see. So I had misunderstood the upstream versioning intention.
> Then I am sorry but better we revert to 1.0 unless there are
> strong reasons for shipping a snapshot.

revert to the 1.0 stable release
Comment 24 Jens Petersen 2008-07-07 01:06:20 EDT
> There are 4 more MiBs of content with 4 years of work since 1.0. Is that
strong enough?

Ok, agreed let's go with the new pre-release if Dennis does not have any
concerns. :)
(Sorry to keep jumping back and forth with the version....)

Personally I would rather keep the upstream name un-fonts-core.
We do that with other fonts packages too.
IMHO upstream naming should take precedence AFAP.
Nicolas, how can we get consensus on this?
Comment 25 Nicolas Mailhot 2008-07-07 04:22:46 EDT
(In reply to comment #24)

> Personally I would rather keep the upstream name un-fonts-core.
> We do that with other fonts packages too.

My preference would be un-core-fonts/un-extra-fonts to minimize confusion with
subpackages. Multiple slightly different conventions are user-unfriendly. But
I'll let you have the last call on this one.
Comment 26 Jens Petersen 2008-07-10 04:12:33 EDT
Nicolas, ok I sent a mail to fedora-fonts-list proposing your idea of renaming
fonts deviating from
*-fonts.

Sorry, Dennis, starting this discussion here, and thanks for your patience.

So I agree let's proceed with un-core-fonts-1.0.2. :)
Comment 27 Dennis Jang 2008-07-10 10:45:18 EDT
(In reply to comment #26)
> Nicolas, ok I sent a mail to fedora-fonts-list proposing your idea of 
renaming
> fonts deviating from
> *-fonts.
> Sorry, Dennis, starting this discussion here, and thanks for your patience.
> So I agree let's proceed with un-core-fonts-1.0.2. :)

thanks Jens ^^
Comment 28 Jens Petersen 2008-07-11 02:29:12 EDT
So my last request I hope is to have subpackages for un-core-fonts.

I would suggest: un-core-fonts-{batang,dinaru,dotum,graphic,gungseo,pilgi}
and the base fonts can require all those for users that want to install all the
families.
Comment 29 Jens Petersen 2008-07-11 04:01:28 EDT
> [...] base fonts [...]

Sorry I meant base package.
Comment 30 Jens Petersen 2008-07-11 04:55:41 EDT
I talked to Nicolas on ##fonts and he persuaded me a base meta-package is a bad
idea.
(Maybe we need to think how to organize fonts better inside comps.)
Comment 31 Jens Petersen 2008-07-16 01:36:56 EDT
I am going to sponsor Dennis, so removing needsponsor for this bug.
Comment 32 Jens Petersen 2008-07-16 03:35:27 EDT
Created attachment 311926 [details]
un-core-fonts.spec-5.patch

Here is a patch which implements the subpackaging with a macro.

I put in a couple of FIXME for the ko text in the macro - please have a look
and fix them up as appropriate. :)
Comment 33 Dennis Jang 2008-07-21 01:17:49 EDT
Created attachment 312229 [details]
updated spec file

Spec file - http://get9.net/rpm/5/un-fonts-core.spec

this patch is a little modification, But main description disappear and short
length of spec file  but will untidy
Comment 34 Jens Petersen 2008-07-25 02:28:08 EDT
Sorry for the slow response.

Those two files are quite different.  Which one are you intending for the review? :)
Comment 35 Jens Petersen 2008-07-25 02:33:13 EDT
I am assuming attachment 312229 [details].
Comment 36 Jens Petersen 2008-07-25 03:35:15 EDT
Here is my review:

 +:ok, =:needs attention, -:needs fixing

MUST Items:
[=] MUST: rpmlint must be run on every package. The output should be posted in
the review.
[+] MUST: The package must be named according to the Package Naming Guidelines.
[+] MUST: The spec file name must match the base package %{name}
[+] MUST: The package must meet the Packaging Guidelines.
[+] MUST: The package must be licensed with a Fedora approved license and meet
the Licensing Guidelines.
[+] MUST: The License field in the package spec file must match the actual license.
[+] MUST: If (and only if) the source package includes the text of the
license(s) in its own file, then that file, containing the text of the
license(s) for the package must be included in %doc.
[+] MUST: The spec file must be written in American English.
[+] MUST: The spec file for the package MUST be legible.
[=] MUST: The sources used to build the package must match the upstream source,
as provided in the spec URL. Reviewers should use md5sum for this task.

87edeb65586b85d4ce10c1fab4f1e1aa  un-fonts-core-1.0.2-080608.tar.gz

[+] MUST: The package must successfully compile and build into binary rpms on at
least one supported architecture.
[+] MUST: All build dependencies must be listed in BuildRequires
[+] MUST: A package must own all directories that it creates. If it does not
create a directory that it uses, then it should require a package which does
create that directory.
[+] MUST: A package must not contain any duplicate files in the %files listing.
[+] MUST: Permissions on files must be set properly. Executables should be set
with executable permissions, for example. Every %files section must include a
%defattr(...) line.
[+] MUST: Each package must have a %clean section, which contains rm -rf
%{buildroot} (or $RPM_BUILD_ROOT).
[+] MUST: Each package must consistently use macros, as described in the macros
section of Packaging Guidelines.
[+] MUST: The package must contain code, or permissable content. This is
described in detail in the code vs. content section of Packaging Guidelines.
[+] MUST: If a package includes something as %doc, it must not affect the
runtime of the application.
[+] MUST: Packages must not own files or directories already owned by other
packages.
[+] MUST: At the beginning of %install, each package MUST run rm -rf
%{buildroot} (or $RPM_BUILD_ROOT).
[+] MUST: All filenames in rpm packages must be valid UTF-8.

SHOULD Items:
[+] SHOULD: The description and summary sections in the package spec file should
contain translations for supported Non-English languages, if available.
[+] SHOULD: The reviewer should test that the package builds in mock.
[+] SHOULD: The reviewer should test that the package functions as described.
[+] SHOULD: If scriptlets are used, those scriptlets must be sane.


Please provide the final srpm package and I am happy to approve this package.
Comment 37 Jens Petersen 2008-07-25 03:36:46 EDT
Just to clarify, these two points:

> [=] MUST: rpmlint must be run on every package. The output should be posted in
> the review.
> [=] MUST: The sources used to build the package must match the upstream source,
> as provided in the spec URL. Reviewers should use md5sum for this task.
> 
> 87edeb65586b85d4ce10c1fab4f1e1aa  un-fonts-core-1.0.2-080608.tar.gz

are just referring to this:

> Please provide the final srpm package and I am happy to approve this package.

Thanks
Comment 38 Jens Petersen 2008-08-04 01:59:00 EDT
Dennis communicated to me that he is happy with the .spec file in attachment 312229 [details].

I am sponsoring Dennis - thank you for the review!

This package is APPROVED for inclusion in Fedora.


Please go ahead and follow the process from step 7 on 
http://fedoraproject.org/wiki/PackageMaintainers/NewPackageProcess

http://fedoraproject.org/wiki/PackageMaintainers/Join also explains the steps in more detail.
Comment 39 Jens Petersen 2008-08-18 03:56:02 EDT
Dennis, ping :)  Can you please follow
http://fedoraproject.org/wiki/PackageMaintainers/CVSAdminProcedure
so we can get this package added to Fedora soon.
Comment 40 Dennis Jang 2008-08-19 09:15:30 EDT
(In reply to comment #39)
> Dennis, ping :)  Can you please follow
> http://fedoraproject.org/wiki/PackageMaintainers/CVSAdminProcedure
> so we can get this package added to Fedora soon.

Jens, sorry for slow response

ok I'll follow cvsadmin procdeure
Comment 41 Dennis Jang 2008-08-21 02:02:29 EDT
New Package CVS Request
=======================
Package Name: un-core-fonts
Short Description: Un Core families Korean TrueType fonts
Owners: smallvil
Branches: devel F-9 F-8
InitialCC: petersen,fonts-sig
Comment 42 Kevin Fenzi 2008-08-23 14:09:52 EDT
cvs done.
Comment 43 Jens Petersen 2008-09-04 23:10:16 EDT
Thanks, package has been built and is now in rawhide. :)

Added to f10 comps and relnotes under i18n.
Comment 44 Fedora Update System 2008-10-15 13:02:55 EDT
un-core-fonts-1.0.2-0.6.080608.fc8 has been submitted as an update for Fedora 8.
http://admin.fedoraproject.org/updates/un-core-fonts-1.0.2-0.6.080608.fc8
Comment 45 Fedora Update System 2008-10-15 13:04:02 EDT
un-core-fonts-1.0.2-0.6.080608.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/un-core-fonts-1.0.2-0.6.080608.fc9
Comment 46 Fedora Update System 2008-11-06 21:54:01 EST
un-core-fonts-1.0.2-0.6.080608.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 47 Fedora Update System 2008-11-06 21:58:33 EST
un-core-fonts-1.0.2-0.6.080608.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.

Note You need to log in before you can comment on or make changes to this bug.