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 1527289
Summary: | Review Request: nototools - Noto fonts support tools and scripts plus web site generation | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Peng Wu <pwu> |
Component: | Package Review | Assignee: | Parag AN(पराग) <panemade> |
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | jbicha, mavit, ngompa13, package-review |
Target Milestone: | --- | Flags: | panemade:
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-06 15:35:08 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: | 1514274 |
Description
Peng Wu
2017-12-19 06:28:36 UTC
Thanks for working on this. I noticed the following: > Version: 20170926 https://fedoraproject.org/wiki/Packaging:Versioning#Upstream_has_never_chosen_a_version says: “When upstream has never chosen a version, you MUST use `Version: 0`” > License: ASL 2.0 The files packaged into `/usr/lib/python2.7/site-packages/third_party/ucd` are under the Unicode licence. > Source0: nototools-0c99dff.tar.gz Shouldn’t this be `https://github.com/googlei18n/%{name}/archive/%{commit0}.tar.gz#/%{name}-%{commit0}.tar.gz`, or similar? Description is empty for python2-nototools. There’s missing Requires and BuildRequires of `python2dist(fonttools)`. Are we sure it’s okay to put files in /usr/lib/python2.7/site-packages/third_party? File third_party/ucd/README.third_party describes where this content comes from. I think we should probably add these as separate sources and pull them down ourselves. Some scripts get installed into both /usr/bin and /usr/lib/python2.7/site-packages/nototools, but the copies in /usr/lib/python2.7/site-packages/nototools are not executable and have an incorrect #! line. Two things: 1. site-packages/third_party should not be done in any circumstance. That's just Google's vendoring thing going on, and you should replace that with proper modules. 2. Please default to Python 3 for binaries. (In reply to Neal Gompa from comment #2) > > 1. site-packages/third_party should not be done in any circumstance. That's > just Google's vendoring thing going on, and you should replace that with > proper modules. > So apparently all this stuff in "third_party" aside from spiro and fontcrunch are data files. The data files should be put in /usr/share/nototools/third_party and the scripts referencing them should be adjusted. I'm not sure what you should do with spiro and fontcrunch. (In reply to Neal Gompa from comment #3) > Please default to Python 3 for binaries. These scripts don't (yet) work with Python 3. > I'm not sure what you should do with spiro and fontcrunch. If it turned out that we needed them, we could try to package their upstream versions directly. However, I don't think we do need them. They're not referenced elsewhere in nototools' source. Hi, I helped package nototools for Debian. I thought I'd leave some comments, although some of them are just confirming what you already figured out. nototools explicitly does not support Python 3 yet. In Debian I excluded all of third_party/ except for ucd/ (I see that's the same as is being done in the current .spec). The problem is that Google has made it clear that it will patch the Unicode data and so it's not really the same as the generic Unicode data that may already be in the distro. The only real build dependency besides python I added was fonttools which was only used for build tests. nototools wants to install several scripts to /usr/bin/ . So far, the only one I bother installing is add_vs_cmap (renamed from add_vs_cmap.py to comply with Debian recommendations. Note that renaming means you will need to patch the Noto Emoji build system and any other fonts that copy the build system for there.) If you do install the other scripts, be aware that some of them have dependencies on other font related Python packages which might not be packaged in your distro yet. Okay, I updated the spec file a bit. Please help on how to improve the rest part of the spec file, thanks! Spec URL: https://pwu.fedorapeople.org/fonts/nototools/nototools.spec SRPM URL: https://pwu.fedorapeople.org/fonts/nototools/nototools-0-20170926.0c99dff.fc27.src.rpm (In reply to Peter Oliver from comment #1) > Some scripts get installed into both /usr/bin and > /usr/lib/python2.7/site-packages/nototools, but the copies in > /usr/lib/python2.7/site-packages/nototools are not executable and have an > incorrect #! line. I think the python scripts in /usr/lib/python2.7/site-packages/nototools with "#!/usr/bin/env python" is okay. sorry I am getting late here. I will review this by Friday. If you follow the current guidelines of Python packaging then you can use the py2_* and py3_* macros. Use snapshot release guideline for release tag which should be Release: 0.20170926.git%{shortcommit0}%{?dist} You may want to use this modified spec file here -> https://paste.fedoraproject.org/paste/dtiP95sW0puc46~R4z8e9w Thanks for the help on the spec file! Please review the package again! Spec URL: https://pwu.fedorapeople.org/fonts/nototools/nototools.spec SRPM URL: https://pwu.fedorapeople.org/fonts/nototools/nototools-0-0.20170926.git0c99dff.fc27.src.rpm Thanks but one issue still remained. We want this package to support python2 as well as python3? I will say good to provide only python3 package with same name binaries instead to go for renaming binaries for py2 as well as py3 variants. If package is not supposed to be used on RHEL7/EPEL7 or its previous releases then just target python3 subpackage. (In reply to Parag AN(पराग) from comment #11) > Thanks but one issue still remained. We want this package to support python2 > as well as python3? These scripts don't (yet) work with Python 3. Hence, only Python 2 packages will be built. I really don't see any use of packaging only for python2 version whereas we are moving to python3 soon totally. Let me see if this can be made compatible with python3 version. (In reply to Parag AN(पराग) from comment #13) > I really don't see any use of packaging only for python2 version whereas we > are moving to python3 soon totally. > > Let me see if this can be made compatible with python3 version. Well, the use would be that we could get this into Fedora sooner. If you can port it to Python 3 that's great, but if you don't have time to work on it now, that's okay, Python 2 is going to be around for a couple more years yet. I had already given my attempt to convert it to python3 compatible code but I could not make "third_party/spiro/*" code compatible with python3. Its very old and mathematical code. Also once this conversion completes we need to make sure code runs in python3 environment without error. I am sorry I took some time here to work on this but could not actually make some progress. Another thing, If I am not wrong then Fedora is moving to use python3 packages and soon in next few releases it will become default. I think currently F28 Live installation also pulling python3 modules. I am approving the SRPM submitted in comment#10 Let's first have this (python2) package in Fedora. APPROVED. (fedrepo-req-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/nototools. You may commit to the branch "f27" in about 10 minutes. google-noto-emoji-fonts-20170928-3.fc27 nototools-0-0.20170926.git0c99dff.fc27 has been submitted as an update to Fedora 27. https://bodhi.fedoraproject.org/updates/FEDORA-2018-4791ed67e5 google-noto-emoji-fonts-20170928-3.fc27, nototools-0-0.20170926.git0c99dff.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-4791ed67e5 google-noto-emoji-fonts-20170928-3.fc27, nototools-0-0.20170926.git0c99dff.fc27 has been pushed to the Fedora 27 stable repository. If problems still persist, please make note of it in this bug report. |