Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 1514274
| Summary: | Review Request: twitter-twemoji-fonts - Twitter Emoji for everyone | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Peter Oliver <mavit> |
| Component: | Package Review | Assignee: | Neal Gompa <ngompa13> |
| Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | fonts-bugs, jbicha, mavit, ngompa13, package-review, pwu |
| Target Milestone: | --- | Flags: | ngompa13:
fedora-review+
|
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | If docs needed, set a value | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2018-02-20 17:12:57 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: | 1527289 | ||
| Bug Blocks: | |||
|
Description
Peter Oliver
2017-11-17 00:05:37 UTC
Taking this review. 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.
(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. (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? (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 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.) (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. (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. :) (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) Thanks. I've updated the spec to use C.UTF-8, too. 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 (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. Okay, I am trying to package nototools separately. (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. Should we open a separate bug to discuss nototools packaging? I filed a package review request: https://bugzilla.redhat.com/show_bug.cgi?id=1527289 It seems nototools can install scripts and data now. I will check if we can build noto-emoji with installed nototools. This builds fine with the unbundled nototools. SRPM: https://copr-be.cloud.fedoraproject.org/results/mavit/twitter-twemoji-fonts/fedora-rawhide-x86_64/00689222-twitter-twemoji-fonts/twitter-twemoji-fonts-2.3.1-6.fc28.src.rpm 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? (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. Now that the new nototools is in Rawhide, can you please post updated spec and source package for review using it? @Peter, I need specifically links laid out specifically as they are in the original post, so that fedora-review can process it. (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) (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 (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. (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. @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? (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. 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. (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. 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 Thanks for your help, everyone. 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 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. |