Bug 1514274 - Review Request: twitter-twemoji-fonts - Twitter Emoji for everyone
Summary: Review Request: twitter-twemoji-fonts - Twitter Emoji for everyone
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Neal Gompa
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 1527289
Blocks:
TreeView+ depends on / blocked
 
Reported: 2017-11-17 00:05 UTC by Peter Oliver
Modified: 2018-02-20 17:12 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2018-02-20 17:12:57 UTC
Type: ---
Embargoed:
ngompa13: fedora-review+


Attachments (Terms of Use)

Description Peter Oliver 2017-11-17 00:05:37 UTC
Spec URL: https://pagure.io/twitter-twemoji-fonts/raw/master/f/twitter-twemoji-fonts.spec
SRPM URL: https://copr-be.cloud.fedoraproject.org/results/mavit/twitter-twemoji-fonts/fedora-rawhide-x86_64/00663056-twitter-twemoji-fonts/twitter-twemoji-fonts-2.3.1-2.fc28.src.rpm
Description: A color emoji font with a flat visual style, designed and used by Twitter.
Fedora Account System Username: mavit

Comment 1 Neal Gompa 2017-11-17 00:47:50 UTC
Taking this review.

Comment 3 Neal Gompa 2017-12-09 10:04:06 UTC
General question: Why is Noto part of Twemoji? My understanding is that these are different fonts altogether?

And what's exactly going on with the metadata mutation going on in the prep stage?


> # Work around UTF-8
> export LANG=zh_CN.UTF-8

Please use "C.UTF-8". In Fedora, that is the generic locale that offers full UTF-8 support.

Comment 4 Peter Oliver 2017-12-10 13:04:11 UTC
(In reply to Neal Gompa from comment #3)
> General question: Why is Noto part of Twemoji? My understanding is that
> these are different fonts altogether?

See https://github.com/googlei18n/noto-emoji/issues/9:
“The noto-emoji project is two things at the same time:

    • an open-source toolchain for converting SVG assets to TrueType with colored emoji glyphs;
    • freely licensed SVG assets for the Noto Emoji glyphs.

After a few minor changes, our toolchain could also consume other free assets and generate optimized fonts for Twitter Emoji and Emoji One.”

The trouble is, this work to separate the two parts hasn’t yet been done.  Meanwhile, the Twemoji and Emoji Two projects are web-focused, producing PNGs for use on web pages but not traditional TTF fonts.

This is my attempt to use the Noto tooling to build Twemoji into a TTF.  I’m open to discussion about the approach.  An alternative might be to modify the existing google-noto-emoji-fonts package to distribute the tooling for use by other packages, but, to be honest, I’m not sure exactly which files would need to be included.

See also this email I sent to the Fonts SIG (https://lists.fedoraproject.org/archives/list/fonts@lists.fedoraproject.org/thread/O6C2BFTPST4YWCOVKVLI6PBSDXQ5FF7C/), and a similar attempt for the Emoji Two font (https://pagure.io/emojitwo-fonts).

> > # Work around UTF-8
> > export LANG=zh_CN.UTF-8
> 
> Please use "C.UTF-8". In Fedora, that is the generic locale that offers full UTF-8 support.

I based the spec on that of google-noto-emoji-fonts, which does this too.  It wasn’t clear to me exactly what UTF-8 issue it was supposed to be working around, so I left it in for now.

Comment 5 Neal Gompa 2017-12-10 13:09:33 UTC
(In reply to Peter Oliver from comment #4)
> (In reply to Neal Gompa from comment #3)
> > General question: Why is Noto part of Twemoji? My understanding is that
> > these are different fonts altogether?
> 
> See https://github.com/googlei18n/noto-emoji/issues/9:
> “The noto-emoji project is two things at the same time:
> 
>     • an open-source toolchain for converting SVG assets to TrueType with
> colored emoji glyphs;
>     • freely licensed SVG assets for the Noto Emoji glyphs.
> 
> After a few minor changes, our toolchain could also consume other free
> assets and generate optimized fonts for Twitter Emoji and Emoji One.”
> 
> The trouble is, this work to separate the two parts hasn’t yet been done. 
> Meanwhile, the Twemoji and Emoji Two projects are web-focused, producing
> PNGs for use on web pages but not traditional TTF fonts.
> 
> This is my attempt to use the Noto tooling to build Twemoji into a TTF.  I’m
> open to discussion about the approach.  An alternative might be to modify
> the existing google-noto-emoji-fonts package to distribute the tooling for
> use by other packages, but, to be honest, I’m not sure exactly which files
> would need to be included.
> 
> See also this email I sent to the Fonts SIG
> (https://lists.fedoraproject.org/archives/list/fonts@lists.fedoraproject.org/
> thread/O6C2BFTPST4YWCOVKVLI6PBSDXQ5FF7C/), and a similar attempt for the
> Emoji Two font (https://pagure.io/emojitwo-fonts).
> 

Okay, at least this explains what's going on.

> > > # Work around UTF-8
> > > export LANG=zh_CN.UTF-8
> > 
> > Please use "C.UTF-8". In Fedora, that is the generic locale that offers full UTF-8 support.
> 
> I based the spec on that of google-noto-emoji-fonts, which does this too. 
> It wasn’t clear to me exactly what UTF-8 issue it was supposed to be working
> around, so I left it in for now.

I suspect the fact that C.UTF-8 is in Fedora isn't that well known. Does it work correctly when you use C.UTF-8 instead of a Chinese locale?

Comment 6 Peter Oliver 2017-12-13 13:16:57 UTC
(In reply to Neal Gompa from comment #5)

> I suspect the fact that C.UTF-8 is in Fedora isn't that well known. Does it
> work correctly when you use C.UTF-8 instead of a Chinese locale?

It appears to work for me even if I leave out setting LANG.  

Peng Wu: Do you please happen to remember why the spec file for google-noto-emoji-fonts includes the following lines?

> # Work around UTF-8
> export LANG=zh_CN.UTF-8

Comment 7 Jeremy Bicha 2017-12-13 19:19:02 UTC
Why don't you package nototools separately? You are using an embedded copy of nototools to build google-noto-emoji-fonts and emojitwo-fonts (in COPR, not yet in Fedora).

See https://fedoraproject.org/wiki/Bundled_Libraries

(I'm not a Fedora uploader or reviewer.)

Comment 8 Peng Wu 2017-12-14 07:14:10 UTC
(In reply to Jeremy Bicha from comment #7)
> Why don't you package nototools separately? You are using an embedded copy
> of nototools to build google-noto-emoji-fonts and emojitwo-fonts (in COPR,
> not yet in Fedora).


The problem is that nototools can't by installed when we package,
the upstream didn't tell when the tools can be installed.

If we package nototools separately, it will only install
some symbolic links of the scripts and can't run when we package it.

Comment 9 Peng Wu 2017-12-14 08:32:17 UTC
(In reply to Peter Oliver from comment #6)
> Peng Wu: Do you please happen to remember why the spec file for
> google-noto-emoji-fonts includes the following lines?
> 
> > # Work around UTF-8
> > export LANG=zh_CN.UTF-8

Because the python script of build process will print Unicode character,
by default python 2 will crash with Unicode character in koji.

By setting non-English locale will work around the crash.

Thanks for the comments, I changed it to "C.UTF-8" now. :)

Comment 10 Jeremy Bicha 2017-12-14 12:50:49 UTC
(In reply to Peng Wu from comment #8)
> The problem is that nototools can't by installed when we package,
> the upstream didn't tell when the tools can be installed.

I don't think that's true. It works fine in Debian (but Debian has a lot of tools to automatically do the right thing for Python packages).

In Debian, nototools ships all the stuff from
/usr/lib/python2.7/dist-packages/nototools/

but for /usr/bin, only add_vs_cmap is installed (with the .py suffix dropped. If you do that, you'll need to patch the Noto Color Emoji build script to look for add_vs_cmap without the .py suffix too)

Comment 12 Neal Gompa 2017-12-15 16:04:44 UTC
(In reply to Peng Wu from comment #8)
> (In reply to Jeremy Bicha from comment #7)
> > Why don't you package nototools separately? You are using an embedded copy
> > of nototools to build google-noto-emoji-fonts and emojitwo-fonts (in COPR,
> > not yet in Fedora).
> 
> 
> The problem is that nototools can't by installed when we package,
> the upstream didn't tell when the tools can be installed.
> 
> If we package nototools separately, it will only install
> some symbolic links of the scripts and can't run when we package it.

That sounds like the upstream build system is broken. If this is the case, we should fix it.

Comment 13 Peng Wu 2017-12-18 08:08:51 UTC
Okay, I am trying to package nototools separately.

Comment 14 Peter Oliver 2017-12-18 16:37:04 UTC
(In reply to Peng Wu from comment #13)
> Okay, I am trying to package nototools separately.

I started to look at this myself.  I saw that there's a copy of fontcrunch bundled in third_party/fontcrunch, but it doesn't seem to be used by nototools so far as I can see, so perhaps we can discard it.

Comment 15 Jeremy Bicha 2017-12-18 17:32:21 UTC
Should we open a separate bug to discuss nototools packaging?

Comment 16 Peng Wu 2017-12-19 06:32:40 UTC
I filed a package review request:
https://bugzilla.redhat.com/show_bug.cgi?id=1527289

Comment 17 Peng Wu 2017-12-19 06:39:31 UTC
It seems nototools can install scripts and data now.

I will check if we can build noto-emoji with installed nototools.

Comment 19 Peter Oliver 2018-01-17 11:28:47 UTC
So, can I confirm the status of this review?  Are we waiting for nototools to be split out into its own package?  Is there anything else blocking review?

Comment 20 Neal Gompa 2018-01-17 14:27:15 UTC
(In reply to Peter Oliver from comment #19)
> So, can I confirm the status of this review?  Are we waiting for nototools
> to be split out into its own package?  Is there anything else blocking
> review?

No, that's pretty much it. Once the split out nototools is approved, then I can proceed with the review.

Comment 21 Neal Gompa 2018-02-02 16:46:27 UTC
Now that the new nototools is in Rawhide, can you please post updated spec and source package for review using it?

Comment 23 Neal Gompa 2018-02-03 14:46:43 UTC
@Peter, I need specifically links laid out specifically as they are in the original post, so that fedora-review can process it.

Comment 24 Peter Oliver 2018-02-05 13:11:03 UTC
(In reply to Neal Gompa from comment #23)
> I need specifically links laid out specifically as they are in the
> original post, so that fedora-review can process it.

Are you sure that that’s the cause of whatever problem you’re experiencing?  I had success finding the spec and SRPM with:


```
cd `mktemp -d`
curl -O https://kojipkgs.fedoraproject.org//packages/nototools/0/0.20170926.git0c99dff.fc28/noarch/nototools-0-0.20170926.git0c99dff.fc28.noarch.rpm -O https://kojipkgs.fedoraproject.org//packages/nototools/0/0.20170926.git0c99dff.fc28/noarch/python2-nototools-0-0.20170926.git0c99dff.fc28.noarch.rpm
fedora-review -b 1514274 -L .
```

(Here I’m downloading the nototools packages manually only because they haven’t yet reached my nearest Rawhide mirror)

Comment 25 Neal Gompa 2018-02-05 18:10:08 UTC
(In reply to Peter Oliver from comment #24)
> (In reply to Neal Gompa from comment #23)
> > I need specifically links laid out specifically as they are in the
> > original post, so that fedora-review can process it.
> 
> Are you sure that that’s the cause of whatever problem you’re experiencing? 
> I had success finding the spec and SRPM with:
> 
> 
> ```
> cd `mktemp -d`
> curl -O
> https://kojipkgs.fedoraproject.org//packages/nototools/0/0.20170926.
> git0c99dff.fc28/noarch/nototools-0-0.20170926.git0c99dff.fc28.noarch.rpm -O
> https://kojipkgs.fedoraproject.org//packages/nototools/0/0.20170926.
> git0c99dff.fc28/noarch/python2-nototools-0-0.20170926.git0c99dff.fc28.noarch.
> rpm
> fedora-review -b 1514274 -L .
> ```
> 
> (Here I’m downloading the nototools packages manually only because they
> haven’t yet reached my nearest Rawhide mirror)

Doing that means I'm overriding fedora-review on locating the spec and srpm, which I can do, of course.

And I have the Koji internal repo enabled on my fedora-review.

But fedora-review will automatically use posts that have the following construction:

Spec URL: https://pagure.io/twitter-twemoji-fonts/raw/master/f/twitter-twemoji-fonts.spec

SRPM URL: https://copr-be.cloud.fedoraproject.org/results/mavit/twitter-twemoji-fonts/fedora-rawhide-x86_64/00705303-twitter-twemoji-fonts/twitter-twemoji-fonts-2.4.0-1.fc28.src.rpm

Comment 26 Peter Oliver 2018-02-05 23:33:14 UTC
(In reply to Neal Gompa from comment #25)

> But fedora-review will automatically use posts that have the following
> construction:
> 
> Spec URL:
> https://pagure.io/twitter-twemoji-fonts/raw/master/f/twitter-twemoji-fonts.
> spec
> 
> SRPM URL:
> https://copr-be.cloud.fedoraproject.org/results/mavit/twitter-twemoji-fonts/
> fedora-rawhide-x86_64/00705303-twitter-twemoji-fonts/twitter-twemoji-fonts-2.
> 4.0-1.fc28.src.rpm

If I understand correctly, what you're telling me is that I need to include both URLs in a single comment, and that I have to label them "Spec URL" and "SRPM URL".  However, my testing indicates that fedora-review copes fine with the URLs being in different comments, and with the string " URL" being omitted from the label, so I'm a bit confused.

Comment 27 Neal Gompa 2018-02-06 11:54:16 UTC
(In reply to Peter Oliver from comment #26)
> 
> If I understand correctly, what you're telling me is that I need to include
> both URLs in a single comment, and that I have to label them "Spec URL" and
> "SRPM URL".  However, my testing indicates that fedora-review copes fine
> with the URLs being in different comments, and with the string " URL" being
> omitted from the label, so I'm a bit confused.

Don't worry about it. Everything is fine now.

Comment 28 Neal Gompa 2018-02-06 12:03:27 UTC
@Peter, can we drop the whole Noto Emoji source requirement from this font package now? It looks like the only file you use from it is a single tmpl file.

Can you instead submit a version of that to the Twemoji GitHub repository and use that locally in this package?

Comment 29 Peter Oliver 2018-02-06 13:28:17 UTC
(In reply to Neal Gompa from comment #28)
> @Peter, can we drop the whole Noto Emoji source requirement from this font
> package now? It looks like the only file you use from it is a single tmpl
> file.

This would be true if https://github.com/googlei18n/noto-emoji/issues/9 was fixed, but I don’t think anyone is working on that.

For now, also required from Noto Emoji are the Makefile and many of the scripts the Makefile calls.  I haven’t looked into it in detail myself, but Jeremy Bicha of Ubuntu has recently done the equivalent integration work for the Emoji Two font, and the patch touches 32 files.  It hasn’t yet been accepted by Emoji Two, because of, as I understand it, its complexity (https://github.com/EmojiTwo/emojitwo/pull/167).

> Can you instead submit a version of that to the Twemoji GitHub repository
> and use that locally in this package?

We could, yes, but I suspect that Twitter may be reluctant to accept it, since it would require maintaining this fork of the build system long term.  It seems like too much code to copy and paste from one project to another, to me.  Let me know what you think.

Comment 30 Neal Gompa 2018-02-06 13:54:28 UTC
Review notes:

* Package follows packaging guidelines
* Package denotes licensing correctly and includes license files as appropriate
* Package builds and installs

I see no other issues remaining.

PACKAGE APPROVED.

Comment 31 Gwyn Ciesla 2018-02-06 15:52:40 UTC
(fedrepo-req-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/twitter-twemoji-fonts. You may commit to the branch "f27" in about 10 minutes.

Comment 32 Fedora Update System 2018-02-06 18:04:26 UTC
twitter-twemoji-fonts-2.4.0-1.fc27 has been submitted as an update to Fedora 27. https://bodhi.fedoraproject.org/updates/FEDORA-2018-86ff582bf9

Comment 33 Peter Oliver 2018-02-06 18:06:16 UTC
Thanks for your help, everyone.

Comment 34 Fedora Update System 2018-02-07 14:09:44 UTC
twitter-twemoji-fonts-2.4.0-1.fc27 has been pushed to the Fedora 27 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2018-86ff582bf9

Comment 35 Fedora Update System 2018-02-20 17:12:57 UTC
twitter-twemoji-fonts-2.4.0-1.fc27 has been pushed to the Fedora 27 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.