Bug 1822971 - Review Request: notcurses - character graphics and TUI library
Summary: Review Request: notcurses - character graphics and TUI library
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: David Cantrell
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: FE-NEEDSPONSOR
TreeView+ depends on / blocked
 
Reported: 2020-04-10 17:48 UTC by Nick Black
Modified: 2020-05-22 17:16 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2020-05-13 18:38:44 UTC
Type: ---
dcantrell: fedora-review+


Attachments (Terms of Use)
python module with SONAME, per request (52.25 KB, image/png)
2020-04-24 19:08 UTC, Nick Black
no flags Details

Description Nick Black 2020-04-10 17:48:03 UTC
Spec URL: https://github.com/dankamongmen/notcurses/blob/master/tools/notcurses.spec
SRPM URL: https://www.dsscaw.com/repos/dnf/notcurses-1.2.7-1.fc32.src.rpm
Description: notcurses facilitates the creation of modern TUI programs, making full use of Unicode and 24-bit TrueColor. It presents an API similar to that of Curses, and rides atop Terminfo.
Fedora Account System Username: nickblack

This is my first Fedora package. David Cantrell has agreed to be a sponsor (thanks, David!). I am the upstream author, and prepared the Fedora spec with Mssr. Cantrell's assistance. I am also the maintainer of the Arch and Debian packages of notcurses.

Comment 1 Nick Black 2020-04-10 20:29:06 UTC
Spec remains the same, but SRPM is now: https://www.dsscaw.com/repos/dnf/notcurses-1.2.8-1.fc32.src.rpm

See https://github.com/dankamongmen/notcurses/issues/465 for more information.

Comment 2 Artur Frenszek-Iwicki 2020-04-11 10:07:16 UTC
>Spec URL: https://github.com/dankamongmen/notcurses/blob/master/tools/notcurses.spec
This link points to a syntax-highlighted HTML view of the file. Please use a link to the raw file next time.

>%package devel
>Requires:      %{name} = %{version}-%{release}
This should be an archful dependency - "%{name}%{?_isa} = %{version}-%{release}"
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_requiring_base_package

>%build
>%cmake -DUSE_FFMPEG=off -DUSE_TESTS=off .
Why are the tests disabled? Add a comment to the spec.

>%{python3_sitelib}/*egg-info/
This should probably go in a separate package, like "python3-notcurses". Not everyone needs the python bindings.

>%{_mandir}/man1/notcurses-demo.1.gz
>%{_mandir}/man1/notcurses-input.1.gz
>...
>%{_mandir}/man3/notcurses.3.gz
>%{_mandir}/man3/notcurses_cell.3.gz
>...
Do not assume that man pages will be gzipped. Use a wildcard that can match any compression method (including no compression at all).
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_manpages

Also, you list every file in the resulting package individually, but you do not list the directories. This will cause RPM to consider the directories as unowned (not belonging to any package). You should add directories to the %files list. You can then also replace the list of files with a wildcard that matches all of them.

Comment 3 Nick Black 2020-04-11 16:17:54 UTC
Thanks for the review, Artur! I've made the requested changes; the updated spec file (in raw, non-rich format) can be found here:

https://raw.githubusercontent.com/dankamongmen/notcurses/master/tools/notcurses.spec>%{python3_sitelib}/*egg-info/


> This should probably go in a separate package, like "python3-notcurses". Not everyone needs the python bindings.

Done

> Why are the tests disabled? Add a comment to the spec.

Done. doctest is not present in F32, so far as I can tell. I considered downloading it at runtime or packaging it with notcurses, but given its presence in other targeted distributions, I decided to wait on it appearing in Fedora (or to package it for Fedora myself).

> This should be an archful dependency - "%{name}%{?_isa} = %{version}-%{release}"

Done. I've added a similar dependency to the new Python package.

> Do not assume that man pages will be gzipped. Use a wildcard that can match any compression method

Done.

> Also, you list every file in the resulting package individually, but you do not list the directories. 

Done. That cleans up the file nicely, thanks. I've commited this to my git, and will test it now.

Comment 4 Nick Black 2020-04-11 16:33:31 UTC
OK, I've tested this up. I need to cut a new 1.2.9 release and put up a new SRPM to include some Python installation fixes (right now `rpmbuild -ba` fails due to a missing egg-info directory). Other than that, your suggested changes all seem to work. Thanks for the review, Artur!

Comment 5 Nick Black 2020-04-11 18:09:25 UTC
I have cut the 1.2.9 release as required. I've likewise amended the specfile to properly install python into the new python3-notcurses package. I did a local install on a fresh F32 box, and verified that all binaries ran correctly.

Comment 6 Nick Black 2020-04-11 19:04:14 UTC
The new RPMs are available from https://www.dsscaw.com/repos/dnf/, with the source RPM at https://www.dsscaw.com/repos/dnf/notcurses-1.2.9-1.fc32.src.rpm.

Comment 7 Nick Black 2020-04-14 07:59:32 UTC
@artur, are you my official reviewer? If so, could you PTAL (and update the fedora-review flag)? thanks muchly!

SRPM: https://www.dsscaw.com/repos/dnf/notcurses-1.3.0-1.fc32.src.rpm
Spec: https://github.com/dankamongmen/notcurses/blob/master/tools/notcurses.spec

Comment 8 Nick Black 2020-04-14 16:23:47 UTC
Sorry, ugh, raw spec is here: https://raw.githubusercontent.com/dankamongmen/notcurses/master/tools/notcurses.spec (previous was marked up)

Comment 9 David Cantrell 2020-04-15 15:58:39 UTC
This package fails to build as-is.  I made some changes to the spec file to get it to build:

--- notcurses-1.3.0-1.fc32.src/notcurses.spec   2020-04-12 01:26:33.000000000 -0400
+++ rpmbuild/SPECS/notcurses.spec       2020-04-15 11:55:45.080414210 -0400
@@ -13,7 +13,7 @@
 BuildRequires: gcc-c++
 BuildRequires: pandoc
 BuildRequires: python3-devel
-BuildRequires: python3-setuptools
+BuildRequires: python3-cffi
 BuildRequires: pkgconfig(ncurses)
 
 %description
@@ -56,12 +56,14 @@
 cd build
 %cmake -DUSE_FFMPEG=off -DUSE_TESTS=off ..
 %make_build
+cd python
+%py3_build
 
 %install
 cd build
 %make_install
 cd python
-python setup.py install --root=%{buildroot} --optimize=1
+%py3_install
 
 %files
 %doc CHANGELOG.md README.md


Python packaging macros are explained in more detail here:
    https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/

Python RPM packaging in Fedora has gotten a lot easier in recent years.

I am testing building locally using mock(1) like this:
    mock -r fedora-rawhide-x86_64 --rebuild notcurses-1.3.0-1.src.rpm

Please update the spec file and SRPM so I can continue the review.

Comment 10 Nick Black 2020-04-20 10:28:25 UTC
nice catch on the cffi, thanks! i've merged your changes, David, and as soon as I've built and verified new packages, I'll have them staged for you.

With that said, I'm pretty certain things were working for me before. I can understand the cffi miss, but I'm surprised your other changes are needed. Or are they just better idioms? If the latter, thanks for making me aware of them. If the former, please don't feel compelled to worry about it.

Comment 11 Nick Black 2020-04-20 10:52:48 UTC
Same locations.

[vps](0) $ sha256sum *
c8cdb0ada6580bf8d9b13ebb4e20a83499b0c4014c0b226d6f21b1f53cdd202c  notcurses-1.3.2-1.fc32.src.rpm
ed9e2ac283e3465999e93fe6f9df787aba861fb954d3e49646eb6cfcd4ff2282  notcurses-1.3.2-1.fc32.x86_64.rpm
1a9f75f848d7b59118917ab18286a1efdd1f24e8d4eafa5e83dd57ecdfd4245c  notcurses-debuginfo-1.3.2-1.fc32.x86_64.rpm
af5346a99a166e7fc68ccca416d17676a9d9ebc19c487660450cdfa4cf039363  notcurses-debugsource-1.3.2-1.fc32.x86_64.rpm
15a4aa66c12061a6b907335deec9932c2c324a2c8259970ed5ff4d18031908c8  notcurses-devel-1.3.2-1.fc32.x86_64.rpm
04a9d5065a9d36587cdd10b211674620df53478c688bcd9072f4ae54e196f440  notcurses-static-1.3.2-1.fc32.x86_64.rpm
9a60b57d6de4db4b8192276d97208d0d2fabcbf751c36d32046441f3105471a9  python3-notcurses-1.3.2-1.fc32.x86_64.rpm
9d7512addfa4cb8ed6d5f2fcd4128bf851627aca3bfd3353cca07accde2babc7  python3-notcurses-debuginfo-1.3.2-1.fc32.x86_64.rpm
[vps](0) $

Comment 12 David Cantrell 2020-04-20 16:10:10 UTC
(In reply to Nick Black from comment #10)
> nice catch on the cffi, thanks! i've merged your changes, David, and as soon
> as I've built and verified new packages, I'll have them staged for you.
> 
> With that said, I'm pretty certain things were working for me before. I can
> understand the cffi miss, but I'm surprised your other changes are needed.
> Or are they just better idioms? If the latter, thanks for making me aware of
> them. If the former, please don't feel compelled to worry about it.

If you just build locally using rpmbuild, then things will work fine if you have a build requirement installed but not explicitly stated in the spec file.  All Fedora builds are done using mock in clean chroots so the spec file has to contain enough information to set up the build environment from scratch.

The Python build macros are preferred over direct calls to setup.py because it will ensure you pick up the Fedora packaging standards for Python stuff and not have to chase that on your own.  That said, the Fedora macros also change and you're chasing stuff anyway.  But without %py3_build, the install doesn't work right in a mock chroot.

I just did a 'mock -r fedora-rawhide-x86_64 --rebuild notcurses-1.3.2-1.fc32.src.rpm' and it worked fine.  Continuing the review.

Comment 13 David Cantrell 2020-04-20 16:30:38 UTC
Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed



===== MUST items =====

C/C++:
[ ]: Package does not contain kernel modules.
[ ]: Package contains no static executables.
[ ]: Development (unversioned) .so files in -devel subpackage, if present.
     Note: Unversioned so-files in private %_libdir subdirectory (see
     attachment). Verify they are not in ld path.
[x]: If your application is a C or C++ application you must list a
     BuildRequires against gcc, gcc-c++ or clang.
[x]: Header files in -devel subpackage, if present.
[x]: ldconfig not called in %post and %postun for Fedora 28 and later.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.

Generic:
[ ]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[ ]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "Unknown or generated", "Apache License (v2.0)", "*No
     copyright* Apache License (v2.0)". 260 files have unknown license.
     Detailed output of licensecheck in
     /home/dcantrell/notcurses/licensecheck.txt
[ ]: License file installed when any subpackage combination is installed.
[ ]: %build honors applicable compiler flags or justifies otherwise.
[ ]: Package contains no bundled libraries without FPC exception.
[ ]: Changelog in prescribed format.
[ ]: Sources contain only permissible code or content.
[ ]: Package contains desktop file if it is a GUI application.
[ ]: Development files must be in a -devel package
[ ]: Package uses nothing in %doc for runtime.
[ ]: Package consistently uses macros (instead of hard-coded directory
     names).
[ ]: Package is named according to the Package Naming Guidelines.
[ ]: Package does not generate any conflict.
[ ]: Package obeys FHS, except libexecdir and /usr/target.
[ ]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[ ]: Requires correct, justified where necessary.
[ ]: Spec file is legible and written in American English.
[ ]: Package contains systemd file(s) if in need.
[ ]: Useful -debuginfo package or justification otherwise.
[ ]: Package is not known to require an ExcludeArch tag.
[ ]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 133120 bytes in 2 files.
[ ]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: 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 is included in %license.
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package must not depend on deprecated() packages.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: Static libraries in -static or -devel subpackage, providing -devel if
     present.
     Note: Package has .a files: notcurses-static.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

Python:
[ ]: Python eggs must not download any dependencies during the build
     process.
[ ]: A package which is used by another package via an egg interface should
     provide egg info.
[ ]: Package meets the Packaging Guidelines::Python
[x]: Package contains BR: python2-devel or python3-devel
[x]: Packages MUST NOT have dependencies (either build-time or runtime) on
     packages named with the unversioned python- prefix unless no properly
     versioned package exists. Dependencies on Python packages instead MUST
     use names beginning with python2- or python3- as appropriate.
[x]: Python packages must not contain %{pythonX_site(lib|arch)}/* in %files
[x]: Binary eggs must be removed in %prep

===== SHOULD items =====

Generic:
[ ]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[ ]: Final provides and requires are sane (see attachments).
[ ]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in
     notcurses-static
[ ]: Package functions as described.
[ ]: Latest version is packaged.
[ ]: Package does not include license text files separate from upstream.
[ ]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[ ]: Package should compile and build into binary rpms on all supported
     architectures.
[ ]: %check is present and all tests pass.
[ ]: Packages should try to preserve timestamps of original installed
     files.
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: The placement of pkgconfig(.pc) files are correct.
[x]: Sources can be downloaded from URI in Source: tag
[x]: SourceX is a working URL.
[x]: Sources are verified with gpgverify first in %prep if upstream
     publishes signatures.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on debuginfo package(s).
     Note: No rpmlint messages.
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Large data in /usr/share should live in a noarch subpackage if package
     is arched.


Rpmlint
-------
Checking: notcurses-1.3.2-1.fc33.x86_64.rpm
          notcurses-devel-1.3.2-1.fc33.x86_64.rpm
          notcurses-static-1.3.2-1.fc33.x86_64.rpm
          python3-notcurses-1.3.2-1.fc33.x86_64.rpm
          notcurses-debuginfo-1.3.2-1.fc33.x86_64.rpm
          notcurses-debugsource-1.3.2-1.fc33.x86_64.rpm
          notcurses-1.3.2-1.fc33.src.rpm
notcurses-static.x86_64: W: no-documentation
python3-notcurses.x86_64: E: useless-provides python-notcurses
python3-notcurses.x86_64: E: useless-provides python38-notcurses
python3-notcurses.x86_64: E: non-executable-script /usr/lib64/python3.8/site-packages/notcurses/notcurses.py 644 /usr/bin/python3 
7 packages and 0 specfiles checked; 3 errors, 1 warnings.




Rpmlint (debuginfo)
-------------------
Checking: notcurses-debuginfo-1.3.2-1.fc33.x86_64.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.





Rpmlint (installed packages)
----------------------------
perl: warning: Setting locale failed.
perl: warning: Please check that your locale settings:
        LANGUAGE = (unset),
        LC_ALL = (unset),
        LC_CTYPE = "C.UTF-8",
        LANG = "en_US.utf8"
    are supported and installed on your system.
perl: warning: Falling back to the standard locale ("C").
perl: warning: Setting locale failed.
perl: warning: Please check that your locale settings:
        LANGUAGE = (unset),
        LC_ALL = (unset),
        LC_CTYPE = "C.UTF-8",
        LANG = "en_US.utf8"
    are supported and installed on your system.
perl: warning: Falling back to the standard locale ("C").
notcurses-static.x86_64: W: invalid-url URL: https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno -2] Name or service not known>
notcurses-static.x86_64: W: no-documentation
python3-notcurses.x86_64: W: invalid-url URL: https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno -2] Name or service not known>
python3-notcurses.x86_64: E: useless-provides python-notcurses
python3-notcurses.x86_64: E: useless-provides python38-notcurses
python3-notcurses.x86_64: E: non-executable-script /usr/lib64/python3.8/site-packages/notcurses/notcurses.py 644 /usr/bin/python3 
notcurses-debuginfo.x86_64: W: invalid-url URL: https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno -2] Name or service not known>
notcurses.x86_64: W: invalid-url URL: https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno -2] Name or service not known>
notcurses-debugsource.x86_64: W: invalid-url URL: https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno -2] Name or service not known>
notcurses-devel.x86_64: W: invalid-url URL: https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno -2] Name or service not known>
6 packages and 0 specfiles checked; 3 errors, 7 warnings.



Unversioned so-files
--------------------
python3-notcurses: /usr/lib64/python3.8/site-packages/_notcurses.abi3.so

Source checksums
----------------
https://dank.qemfd.net/dankamongmen.gpg :
  CHECKSUM(SHA256) this package     : 0a6f979c9d88b0d72310b8462d2066a60b657a2f188a74c3cb19cac0064616ce
  CHECKSUM(SHA256) upstream package : 0a6f979c9d88b0d72310b8462d2066a60b657a2f188a74c3cb19cac0064616ce
https://github.com/dankamongmen/notcurses/releases/download/v1.3.2/v1.3.2.tar.gz.asc :
  CHECKSUM(SHA256) this package     : a9db45a35ffec9b621a5ea053a44e3861fc420e6b518ca93ba0d74cf6fcc1e71
  CHECKSUM(SHA256) upstream package : a9db45a35ffec9b621a5ea053a44e3861fc420e6b518ca93ba0d74cf6fcc1e71
https://github.com/dankamongmen/notcurses/archive/v1.3.2.tar.gz :
  CHECKSUM(SHA256) this package     : 9f432090714d1e086c0046b9be07877ce937d0f6456f0298395a549f3c552ff5
  CHECKSUM(SHA256) upstream package : 9f432090714d1e086c0046b9be07877ce937d0f6456f0298395a549f3c552ff5


Requires
--------
notcurses (rpmlib, GLIBC filtered):
    libc.so.6()(64bit)
    libgcc_s.so.1()(64bit)
    libgcc_s.so.1(GCC_3.0)(64bit)
    libnotcurses++.so.1()(64bit)
    libnotcurses.so.1()(64bit)
    libpthread.so.0()(64bit)
    libstdc++.so.6()(64bit)
    libstdc++.so.6(CXXABI_1.3)(64bit)
    libstdc++.so.6(CXXABI_1.3.9)(64bit)
    libtinfo.so.6()(64bit)
    rtld(GNU_HASH)

notcurses-devel (rpmlib, GLIBC filtered):
    /usr/bin/pkg-config
    cmake-filesystem(x86-64)
    libnotcurses++.so.1()(64bit)
    libnotcurses.so.1()(64bit)
    notcurses(x86-64)
    pkgconfig(notcurses)

notcurses-static (rpmlib, GLIBC filtered):
    notcurses-devel

python3-notcurses (rpmlib, GLIBC filtered):
    /usr/bin/python3
    libc.so.6()(64bit)
    libnotcurses.so.1()(64bit)
    notcurses(x86-64)
    python(abi)
    python3.8dist(cffi)
    rtld(GNU_HASH)

notcurses-debuginfo (rpmlib, GLIBC filtered):

notcurses-debugsource (rpmlib, GLIBC filtered):



Provides
--------
notcurses:
    libnotcurses++.so.1()(64bit)
    libnotcurses.so.1()(64bit)
    notcurses
    notcurses(x86-64)

notcurses-devel:
    cmake(notcurses)
    notcurses-devel
    notcurses-devel(x86-64)
    pkgconfig(notcurses)
    pkgconfig(notcurses++)

notcurses-static:
    notcurses-static
    notcurses-static(x86-64)

python3-notcurses:
    python-notcurses
    python3-notcurses
    python3-notcurses(x86-64)
    python3.8dist(notcurses)
    python38-notcurses
    python3dist(notcurses)

notcurses-debuginfo:
    debuginfo(build-id)
    notcurses-debuginfo
    notcurses-debuginfo(x86-64)

notcurses-debugsource:
    notcurses-debugsource
    notcurses-debugsource(x86-64)



Generated by fedora-review 0.7.5 (5fa5b7e) last change: 2020-02-16
Command line :/usr/bin/fedora-review --rpm-spec -n notcurses-1.3.2-1.fc32.src.rpm
Buildroot used: fedora-rawhide-x86_64
Active plugins: Python, Generic, Shell-api, C/C++
Disabled plugins: fonts, Ocaml, Haskell, PHP, Perl, SugarActivity, R, Java
Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH

Comment 14 David Cantrell 2020-04-20 16:49:23 UTC
Comments and questions on items requiring manual review.

(In reply to David Cantrell from comment #13)
> Package Review
> ==============
> 
> Legend:
> [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
> [ ] = Manual review needed
> 
> 
> 
> ===== MUST items =====
> 
> C/C++:
> [ ]: Package does not contain kernel modules.

No kernel modules.

> [ ]: Package contains no static executables.

No static executables, but there are static libraries in notcurses-static.

> Generic:
> [ ]: Package is licensed with an open-source compatible license and meets
>      other legal requirements as defined in the legal section of Packaging
>      Guidelines.

The software is licensed under the Apache License 2.0, which is approved by Fedora.  The short name is "ASL 2.0" as noted in the spec file License tag.

> [ ]: License field in the package spec file matches the actual license.
>      Note: Checking patched sources after %prep for licenses. Licenses
>      found: "Unknown or generated", "Apache License (v2.0)", "*No
>      copyright* Apache License (v2.0)". 260 files have unknown license.
>      Detailed output of licensecheck in
>      /home/dcantrell/notcurses/licensecheck.txt

All files lack license boilerplate information, but the entire project says it is licensed under the Apache License 2.0 as noted in the LICENSE file.  Copyright information is in the COPYRIGHT file.

Upstream is advised to consider adding license information to each source file either as indicated by the Apache License 2.0 or an SPDX identifier.

> [ ]: License file installed when any subpackage combination is installed.

The license is installed as /usr/share/licenses/notcurses/LICENSE in the notcurses package.

> [ ]: %build honors applicable compiler flags or justifies otherwise.

The %build section uses the %cmake, %make_build, and %py3_build macros to pull in applicable compiler flags.

> [ ]: Package contains no bundled libraries without FPC exception.

There are no bundled libraries.

> [ ]: Changelog in prescribed format.

It is.

> [ ]: Sources contain only permissible code or content.

All code and docs are licensed under the Apache License 2.0.  The files in data/ are unknown.  Need clarification from upstream.

> [ ]: Package contains desktop file if it is a GUI application.

There is no desktop content.

> [ ]: Development files must be in a -devel package

They are.

> [ ]: Package uses nothing in %doc for runtime.

It doesn't.

> [ ]: Package consistently uses macros (instead of hard-coded directory
>      names).

Oh yes, it does.

> [ ]: Package is named according to the Package Naming Guidelines.

It is.

> [ ]: Package does not generate any conflict.

Only awesomeness.

> [ ]: Package obeys FHS, except libexecdir and /usr/target.

It does.

> [ ]: If the package is a rename of another package, proper Obsoletes and
>      Provides are present.

N/A

> [ ]: Requires correct, justified where necessary.

N/A

> [ ]: Spec file is legible and written in American English.

Yep.

> [ ]: Package contains systemd file(s) if in need.

N/A

> [ ]: Useful -debuginfo package or justification otherwise.

There is a useful debuginfo package.

> [ ]: Package is not known to require an ExcludeArch tag.

Correct.

> [ ]: Large documentation must go in a -doc subpackage. Large could be size
>      (~1MB) or number of files.
>      Note: Documentation size is 133120 bytes in 2 files.

/usr/share/doc is 136K, do not need a subpackage

> [ ]: Package complies to the Packaging Guidelines

That's what this review is confirming.

> Python:
> [ ]: Python eggs must not download any dependencies during the build
>      process.

None are.

> [ ]: A package which is used by another package via an egg interface should
>      provide egg info.

There is an egg provided in /usr/lib[64]/pythonX.Y/site-packages/notcurses

> [ ]: Package meets the Packaging Guidelines::Python

It does.

> ===== SHOULD items =====
> 
> Generic:
> [ ]: If the source package does not include license text(s) as a separate
>      file from upstream, the packager SHOULD query upstream to include it.

Need clarification on the files in data/.
Recommend adding license information to each source and doc file.

> [ ]: Final provides and requires are sane (see attachments).

They are.

> [ ]: Fully versioned dependency in subpackages if applicable.
>      Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in
>      notcurses-static

Need to add:

    Requires: %{name}%{?_isa} = %{version}-%{release}

To the notcurses-static package.

> [ ]: Package functions as described.

Yep.

> [ ]: Latest version is packaged.

Yep.

> [ ]: Package does not include license text files separate from upstream.

It does not.

> [ ]: Description and summary sections in the package spec file contains
>      translations for supported Non-English languages, if available.

Not available.

> [ ]: Package should compile and build into binary rpms on all supported
>      architectures.

It does.

> [ ]: %check is present and all tests pass.

%check is not present due to -DUSE_TESTS=off.  Another BuildRequire is necessary to enable tests, which I understand is coming in a later package review.

> [ ]: Packages should try to preserve timestamps of original installed
>      files.

They do.

> 
> 
> Rpmlint
> -------
> Checking: notcurses-1.3.2-1.fc33.x86_64.rpm
>           notcurses-devel-1.3.2-1.fc33.x86_64.rpm
>           notcurses-static-1.3.2-1.fc33.x86_64.rpm
>           python3-notcurses-1.3.2-1.fc33.x86_64.rpm
>           notcurses-debuginfo-1.3.2-1.fc33.x86_64.rpm
>           notcurses-debugsource-1.3.2-1.fc33.x86_64.rpm
>           notcurses-1.3.2-1.fc33.src.rpm
> notcurses-static.x86_64: W: no-documentation

Documentation is in the main notcurses package.  The static package needs a requires on the main package.

> python3-notcurses.x86_64: E: useless-provides python-notcurses
> python3-notcurses.x86_64: E: useless-provides python38-notcurses

I guess the python_provide macro can be dropped?

> python3-notcurses.x86_64: E: non-executable-script
> /usr/lib64/python3.8/site-packages/notcurses/notcurses.py 644
> /usr/bin/python3 

Make it executable unless it shouldn't be.

> Rpmlint (installed packages)
> ----------------------------
> perl: warning: Setting locale failed.
> perl: warning: Please check that your locale settings:
>         LANGUAGE = (unset),
>         LC_ALL = (unset),
>         LC_CTYPE = "C.UTF-8",
>         LANG = "en_US.utf8"
>     are supported and installed on your system.
> perl: warning: Falling back to the standard locale ("C").
> perl: warning: Setting locale failed.
> perl: warning: Please check that your locale settings:
>         LANGUAGE = (unset),
>         LC_ALL = (unset),
>         LC_CTYPE = "C.UTF-8",
>         LANG = "en_US.utf8"
>     are supported and installed on your system.
> perl: warning: Falling back to the standard locale ("C").
> notcurses-static.x86_64: W: invalid-url URL:
> https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno
> -2] Name or service not known>
> notcurses-static.x86_64: W: no-documentation
> python3-notcurses.x86_64: W: invalid-url URL:
> https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno
> -2] Name or service not known>

I think this is a problem with the environment I ran fedora-review in.  These URLs are valid.

> python3-notcurses.x86_64: E: useless-provides python-notcurses
> python3-notcurses.x86_64: E: useless-provides python38-notcurses
> python3-notcurses.x86_64: E: non-executable-script
> /usr/lib64/python3.8/site-packages/notcurses/notcurses.py 644
> /usr/bin/python3 

See above.

> notcurses-debuginfo.x86_64: W: invalid-url URL:
> https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno
> -2] Name or service not known>
> notcurses.x86_64: W: invalid-url URL:
> https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno
> -2] Name or service not known>
> notcurses-debugsource.x86_64: W: invalid-url URL:
> https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno
> -2] Name or service not known>
> notcurses-devel.x86_64: W: invalid-url URL:
> https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno
> -2] Name or service not known>
> 6 packages and 0 specfiles checked; 3 errors, 7 warnings.

See above, I think this is a bogus warning.

> Unversioned so-files
> --------------------
> python3-notcurses: /usr/lib64/python3.8/site-packages/_notcurses.abi3.so

Not sure if these should be versioned like regular shared libraries.  Check the Fedora packaging guidelines for Python modules.


Please see the comments above for the remaining changes needed.

Comment 15 Nick Black 2020-04-21 08:58:08 UTC
Regarding clarification of the multimedia in data/ (I just did this for the Debian package):

In the upstream tarball, there's a lot of material which doesn't belong in a Linux distribution. There are sprites borrowed from NES games, etc. I do not intend to include these in either the SRPM or RPMs. I thus need to change the specfile to reference the DFSG tarball. For those unaware, this stands for Debian Free Software Guidelines, and I believe the set of files excluded for DFSG-compatibility to be the same set of files that ought be excluded on Fedora. I will change the specfile to reflect this. Thanks for the catch!

Regarding the few files that will remain, the README.source from our Debian package explains things best:

-----------------------------------------------------------------------
The upstream tarball, as automatically put together by GitHub upon a tagging
event, is unsuitable for distribution in Debian due to several DFSG-unfree
multimedia, and one DFSG-unfree source file (src/demo/jungle.c includes an
unfree image as a blob). Generating the DFSG-compliant source file can be
performed via e.g.:

 uscan --repack --compression xz -v

uscan gets its list of excluded files from debian/copyright.

The multimedia which *does* remain is all Free media created for this project
by the project authors, and is licensed under Apache 2.0 like the rest of the
project. "Source" for these multimedia is included in the source tarball's
data/ directory as .xcf (GIMP) and .osp (OpenShot) files. The former were made
using GIMP 2.10 as packaged by Debian, the latter using OpenShot 2.50 as built
from upstream source, and Blender 2.82 as packaged by Debian. These are the
preferred forms for editing the included media. The final media remains
included, and installed in the binary packages, because building/rendering is
computationally intensive and somewhat brittle.
-----------------------------------------------------------------------

So I'd like to do the same thing for Fedora. The SRPM will contain both the rendered media, and the source necessary to generate them using Free tools. The binary RPMs will only install the rendered versions. Rendering the media will not be part of a typical build, and indeed is not currently automated.

Fedora doesn't, so far as I know, *require* "preferred editable form" for multimedia, just that it can be redistributed (please correct me if I'm wrong). We could thus leave the "sources" out of the SRPM entirely. I'd rather not because they're (a) less than 10MB total and (b) I've already got this automated this way.

Hopefully that answers your question about data/. I'll update the specfile, which ought be sufficient, since I can just change the upstream tarball location.

Comment 16 Nick Black 2020-04-21 09:12:42 UTC
> Not sure if these should be versioned like regular shared libraries.  Check the Fedora packaging guidelines for Python modules.

Taking a step back, I'm having a difficult time imaging a use case that relies on python cffi modules having a particular SONAME, but I'm no Python expert.

Taking another step back, setting an SONAME seems a polite thing to do in all cases.

Tightening back in, I'm certain I can weave the necessary incantations through to get an SONAME set...but I'm surprised that this isn't already being done by some setuptools layer, or the Fedora Python packaging macros. I've been unable to find firm Fedora guidance yet, save the lint complaint, but surely I'm not the first person to be building a Python cffi module in Fedora, so I'm proceeding under the assumption that the complaint is valid. I'll have this resolved today, one way or the other.

> I guess the python_provide macro can be dropped?

Done.

> %check is not present due to -DUSE_TESTS=off.  Another BuildRequire is necessary to enable tests, which I understand is coming in a later package review.

Correct. I'll be filing a spec+SRPM for the header-only C++ interface "doctest"

> Make it executable unless it shouldn't be.

Done (it should be). Thanks for the catch!

> Requires: %{name}%{?_isa} = %{version}-%{release}
 
Added. I removed the preexisting line, which lacked the %{?_isa} substitution. Thanks for the catch!

> Need clarification on the files in data/.

See previous comment and upcoming specfile change.

> Recommend adding license information to each source and doc file.

Duly noted.

Comment 17 Nick Black 2020-04-21 09:29:46 UTC
Actually, right now we build without any multimedia capability, and thus *none* of the contents of data/ are installed. We just need to ensure they're not in the SRPM. That'll be satisfied by just grabbing the DFSG tarball. I should have a new spec+SRPM for you within the hour.

Comment 18 Nick Black 2020-04-21 09:51:46 UTC
[vps](0) $ sha256sum *
9f8dc946b25ebd2aa9213c1572d45e9c0764bf478a8d4ace336364a5ea00c431  build.log
a58c41334e01cdeb4ce257649131dfbbe5b3dc58b915c6b29a7127ff7ba3492b  hw_info.log
b643b204abb1d82179dfa465274ad94eadebbf25c28472a041e4d4d8fe95e0f4  installed_pkgs.log
ebb65500e37ad3cb89c2ef8f9774958d3c19f6d263dd659a78a2746939e3577f  notcurses-1.3.2-2.fc33.src.rpm
f9ce8a8151e491e4c39485c9c2f4195e1ada4ddb9dcff8cbb3dab9a010a6954f  notcurses-1.3.2-2.fc33.x86_64.rpm
997584fe6d39e53e2bea37b5c9871b4595be9dd95a9fc42dd7102419f02083fd  notcurses-debuginfo-1.3.2-2.fc33.x86_64.rpm
96365a593f0c7b3563a6ec7abc3adc683b48e27a51c0c9c3140b6c4c2b2e2006  notcurses-debugsource-1.3.2-2.fc33.x86_64.rpm
fcde8eef5650847c1769f560733e415b285556a666f61fcb547107a9e188748d  notcurses-devel-1.3.2-2.fc33.x86_64.rpm
189fc9968f0d24e4cf299533defc3a85cdfd665834b75e8589f7714c93f5a8db  notcurses-static-1.3.2-2.fc33.x86_64.rpm
8f5025258f48842a510d3bb9e54d8ff7830a165fba7eee0822b3863c1eca34e6  python3-notcurses-1.3.2-2.fc33.x86_64.rpm
c7d60859d954caa62f77eca599b31093579224f3ac798d5931f6bed5571ab645  python3-notcurses-debuginfo-1.3.2-2.fc33.x86_64.rpm
41f36b148e0d36b686430bfd35be58362a0b705a1ce422dd47f55e5801ca7ea1  root.log
de713ba48b7416b43399dc54f855dedc2cf5f23c0bbbfba6cc0b67d3a593f2d7  state.log
[vps](0) $

Comment 19 Nick Black 2020-04-21 10:55:51 UTC
Note: i used "mock" instead of "rpmbuild" this time, and am now getting "fc33" packages as opposed to the "fc32" i was seeing before. Makes sense, just pointing it out.

Comment 20 David Cantrell 2020-04-24 14:39:12 UTC
(In reply to Nick Black from comment #15)
> Regarding clarification of the multimedia in data/ (I just did this for the
> Debian package):
> 
> In the upstream tarball, there's a lot of material which doesn't belong in a
> Linux distribution. There are sprites borrowed from NES games, etc. I do not
> intend to include these in either the SRPM or RPMs. I thus need to change
> the specfile to reference the DFSG tarball. For those unaware, this stands
> for Debian Free Software Guidelines, and I believe the set of files excluded
> for DFSG-compatibility to be the same set of files that ought be excluded on
> Fedora. I will change the specfile to reflect this. Thanks for the catch!
> 
> Regarding the few files that will remain, the README.source from our Debian
> package explains things best:
> 
> -----------------------------------------------------------------------
> The upstream tarball, as automatically put together by GitHub upon a tagging
> event, is unsuitable for distribution in Debian due to several DFSG-unfree
> multimedia, and one DFSG-unfree source file (src/demo/jungle.c includes an
> unfree image as a blob). Generating the DFSG-compliant source file can be
> performed via e.g.:
> 
>  uscan --repack --compression xz -v
> 
> uscan gets its list of excluded files from debian/copyright.
> 
> The multimedia which *does* remain is all Free media created for this project
> by the project authors, and is licensed under Apache 2.0 like the rest of the
> project. "Source" for these multimedia is included in the source tarball's
> data/ directory as .xcf (GIMP) and .osp (OpenShot) files. The former were
> made
> using GIMP 2.10 as packaged by Debian, the latter using OpenShot 2.50 as
> built
> from upstream source, and Blender 2.82 as packaged by Debian. These are the
> preferred forms for editing the included media. The final media remains
> included, and installed in the binary packages, because building/rendering is
> computationally intensive and somewhat brittle.
> -----------------------------------------------------------------------
> 
> So I'd like to do the same thing for Fedora. The SRPM will contain both the
> rendered media, and the source necessary to generate them using Free tools.
> The binary RPMs will only install the rendered versions. Rendering the media
> will not be part of a typical build, and indeed is not currently automated.
> 
> Fedora doesn't, so far as I know, *require* "preferred editable form" for
> multimedia, just that it can be redistributed (please correct me if I'm
> wrong). We could thus leave the "sources" out of the SRPM entirely. I'd
> rather not because they're (a) less than 10MB total and (b) I've already got
> this automated this way.

That is correct for these files types.

> Hopefully that answers your question about data/. I'll update the specfile,
> which ought be sufficient, since I can just change the upstream tarball
> location.

Yeah, this should be fine.  The Fedora package using the DFSG tarball as the source will comply with Fedora guidelines too.  That should keep things a little easier on your end.

I am currently running a new review now on the latest SRPM.

Comment 21 David Cantrell 2020-04-24 14:48:54 UTC
Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed


Issues:
=======
- Sources used to build the package match the upstream source, as provided
  in the spec URL.
  Note: Upstream MD5sum check error, diff is in
  /home/dcantrell/notcurses/diff.txt
  See: https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/


===== MUST items =====

C/C++:
[ ]: Package does not contain kernel modules.
[ ]: Package contains no static executables.
[ ]: Development (unversioned) .so files in -devel subpackage, if present.
     Note: Unversioned so-files in private %_libdir subdirectory (see
     attachment). Verify they are not in ld path.
[x]: If your application is a C or C++ application you must list a
     BuildRequires against gcc, gcc-c++ or clang.
[x]: Header files in -devel subpackage, if present.
[x]: ldconfig not called in %post and %postun for Fedora 28 and later.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.

Generic:
[ ]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[ ]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "Unknown or generated", "Apache License (v2.0)", "*No
     copyright* Apache License (v2.0)". 260 files have unknown license.
     Detailed output of licensecheck in
     /home/dcantrell/notcurses/licensecheck.txt
[ ]: License file installed when any subpackage combination is installed.
[ ]: %build honors applicable compiler flags or justifies otherwise.
[ ]: Package contains no bundled libraries without FPC exception.
[ ]: Changelog in prescribed format.
[ ]: Sources contain only permissible code or content.
[ ]: Package contains desktop file if it is a GUI application.
[ ]: Development files must be in a -devel package
[ ]: Package uses nothing in %doc for runtime.
[ ]: Package consistently uses macros (instead of hard-coded directory
     names).
[ ]: Package is named according to the Package Naming Guidelines.
[ ]: Package does not generate any conflict.
[ ]: Package obeys FHS, except libexecdir and /usr/target.
[ ]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[ ]: Requires correct, justified where necessary.
[ ]: Spec file is legible and written in American English.
[ ]: Package contains systemd file(s) if in need.
[ ]: Useful -debuginfo package or justification otherwise.
[ ]: Package is not known to require an ExcludeArch tag.
[ ]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 133120 bytes in 2 files.
[ ]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: 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 is included in %license.
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package must not depend on deprecated() packages.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: Static libraries in -static or -devel subpackage, providing -devel if
     present.
     Note: Package has .a files: notcurses-static.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

Python:
[ ]: Python eggs must not download any dependencies during the build
     process.
[ ]: A package which is used by another package via an egg interface should
     provide egg info.
[ ]: Package meets the Packaging Guidelines::Python
[x]: Package contains BR: python2-devel or python3-devel
[x]: Packages MUST NOT have dependencies (either build-time or runtime) on
     packages named with the unversioned python- prefix unless no properly
     versioned package exists. Dependencies on Python packages instead MUST
     use names beginning with python2- or python3- as appropriate.
[x]: Python packages must not contain %{pythonX_site(lib|arch)}/* in %files
[x]: Binary eggs must be removed in %prep

===== SHOULD items =====

Generic:
[ ]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[ ]: Final provides and requires are sane (see attachments).
[ ]: Package functions as described.
[ ]: Latest version is packaged.
[ ]: Package does not include license text files separate from upstream.
[ ]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[ ]: Package should compile and build into binary rpms on all supported
     architectures.
[ ]: %check is present and all tests pass.
[ ]: Packages should try to preserve timestamps of original installed
     files.
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Fully versioned dependency in subpackages if applicable.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: The placement of pkgconfig(.pc) files are correct.
[x]: Sources can be downloaded from URI in Source: tag
[x]: SourceX is a working URL.
[x]: Sources are verified with gpgverify first in %prep if upstream
     publishes signatures.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[!]: Spec file according to URL is the same as in SRPM.
     Note: Bad spec filename: /home/dcantrell/notcurses/srpm-
     unpacked/notcurses.spec
     See: (this test has no URL)
[x]: Rpmlint is run on debuginfo package(s).
     Note: No rpmlint messages.
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Large data in /usr/share should live in a noarch subpackage if package
     is arched.


Rpmlint
-------
Checking: notcurses-1.3.2-2.fc33.x86_64.rpm
          notcurses-devel-1.3.2-2.fc33.x86_64.rpm
          notcurses-static-1.3.2-2.fc33.x86_64.rpm
          python3-notcurses-1.3.2-2.fc33.x86_64.rpm
          notcurses-debuginfo-1.3.2-2.fc33.x86_64.rpm
          notcurses-debugsource-1.3.2-2.fc33.x86_64.rpm
          notcurses-1.3.2-2.fc33.src.rpm
notcurses.x86_64: W: incoherent-version-in-changelog 1.3.2-1 ['1.3.2-2.fc33', '1.3.2-2']
notcurses-static.x86_64: W: no-documentation
python3-notcurses.x86_64: E: non-executable-script /usr/lib64/python3.8/site-packages/notcurses/notcurses.py 644 /usr/bin/python3 
notcurses.src: W: inconsistent-file-extension notcurses_1.3.2+dfsg.1.orig.tar.xz
7 packages and 0 specfiles checked; 1 errors, 3 warnings.




Rpmlint (debuginfo)
-------------------
Checking: notcurses-debuginfo-1.3.2-2.fc33.x86_64.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.





Rpmlint (installed packages)
----------------------------
notcurses-static.x86_64: W: invalid-url URL: https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno -2] Name or service not known>
notcurses-static.x86_64: W: no-documentation
notcurses-devel.x86_64: W: invalid-url URL: https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno -2] Name or service not known>
notcurses-debuginfo.x86_64: W: invalid-url URL: https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno -2] Name or service not known>
notcurses.x86_64: W: incoherent-version-in-changelog 1.3.2-1 ['1.3.2-2.fc33', '1.3.2-2']
notcurses.x86_64: W: invalid-url URL: https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno -2] Name or service not known>
notcurses-debugsource.x86_64: W: invalid-url URL: https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno -2] Name or service not known>
python3-notcurses.x86_64: W: invalid-url URL: https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno -2] Name or service not known>
python3-notcurses.x86_64: E: non-executable-script /usr/lib64/python3.8/site-packages/notcurses/notcurses.py 644 /usr/bin/python3 
6 packages and 0 specfiles checked; 1 errors, 8 warnings.



Unversioned so-files
--------------------
python3-notcurses: /usr/lib64/python3.8/site-packages/_notcurses.abi3.so

Source checksums
----------------
https://dank.qemfd.net/dankamongmen.gpg :
  CHECKSUM(SHA256) this package     : 0a6f979c9d88b0d72310b8462d2066a60b657a2f188a74c3cb19cac0064616ce
  CHECKSUM(SHA256) upstream package : 0a6f979c9d88b0d72310b8462d2066a60b657a2f188a74c3cb19cac0064616ce
https://github.com/dankamongmen/notcurses/releases/download/v1.3.2/v1.3.2.tar.gz.asc :
  CHECKSUM(SHA256) this package     : a9db45a35ffec9b621a5ea053a44e3861fc420e6b518ca93ba0d74cf6fcc1e71
  CHECKSUM(SHA256) upstream package : a9db45a35ffec9b621a5ea053a44e3861fc420e6b518ca93ba0d74cf6fcc1e71
https://github.com/dankamongmen/notcurses/releases/download/v1.3.2/notcurses_1.3.2+dfsg.1.orig.tar.xz :
  CHECKSUM(SHA256) this package     : 9f432090714d1e086c0046b9be07877ce937d0f6456f0298395a549f3c552ff5
  CHECKSUM(SHA256) upstream package : 0fa147d16334498ea80c25169574e13a37ab4b66a242e6131221ba8afb9b6389
diff -r also reports differences


Requires
--------
notcurses (rpmlib, GLIBC filtered):
    libc.so.6()(64bit)
    libgcc_s.so.1()(64bit)
    libgcc_s.so.1(GCC_3.0)(64bit)
    libnotcurses++.so.1()(64bit)
    libnotcurses.so.1()(64bit)
    libpthread.so.0()(64bit)
    libstdc++.so.6()(64bit)
    libstdc++.so.6(CXXABI_1.3)(64bit)
    libstdc++.so.6(CXXABI_1.3.9)(64bit)
    libtinfo.so.6()(64bit)
    rtld(GNU_HASH)

notcurses-devel (rpmlib, GLIBC filtered):
    /usr/bin/pkg-config
    cmake-filesystem(x86-64)
    libnotcurses++.so.1()(64bit)
    libnotcurses.so.1()(64bit)
    notcurses(x86-64)
    pkgconfig(notcurses)

notcurses-static (rpmlib, GLIBC filtered):
    notcurses(x86-64)

python3-notcurses (rpmlib, GLIBC filtered):
    /usr/bin/python3
    libc.so.6()(64bit)
    libnotcurses.so.1()(64bit)
    notcurses(x86-64)
    python(abi)
    python3.8dist(cffi)
    rtld(GNU_HASH)

notcurses-debuginfo (rpmlib, GLIBC filtered):

notcurses-debugsource (rpmlib, GLIBC filtered):



Provides
--------
notcurses:
    libnotcurses++.so.1()(64bit)
    libnotcurses.so.1()(64bit)
    notcurses
    notcurses(x86-64)

notcurses-devel:
    cmake(notcurses)
    notcurses-devel
    notcurses-devel(x86-64)
    pkgconfig(notcurses)
    pkgconfig(notcurses++)

notcurses-static:
    notcurses-static
    notcurses-static(x86-64)

python3-notcurses:
    python-notcurses
    python3-notcurses
    python3-notcurses(x86-64)
    python3.8dist(notcurses)
    python38-notcurses
    python3dist(notcurses)

notcurses-debuginfo:
    debuginfo(build-id)
    notcurses-debuginfo
    notcurses-debuginfo(x86-64)

notcurses-debugsource:
    notcurses-debugsource
    notcurses-debugsource(x86-64)



Generated by fedora-review 0.7.5 (5fa5b7e) last change: 2020-02-16
Command line :/usr/bin/fedora-review --rpm-spec -n notcurses-1.3.2-2.fc33.src.rpm
Buildroot used: fedora-rawhide-x86_64
Active plugins: C/C++, Python, Shell-api, Generic
Disabled plugins: Perl, SugarActivity, fonts, Ocaml, PHP, R, Java, Haskell
Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH

Comment 22 David Cantrell 2020-04-24 15:07:13 UTC
Many things fixed since the last review, adding comments for the manual review items and other issues:

(In reply to David Cantrell from comment #21)
> Package Review
> ==============
> 
> Legend:
> [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
> [ ] = Manual review needed
> 
> 
> Issues:
> =======
> - Sources used to build the package match the upstream source, as provided
>   in the spec URL.
>   Note: Upstream MD5sum check error, diff is in
>   /home/dcantrell/notcurses/diff.txt
>   See: https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/

This seems bogus.  I think this is a bug in the review tool.

> ===== MUST items =====
> 
> C/C++:
> [ ]: Package does not contain kernel modules.

It does not.

> [ ]: Package contains no static executables.

It does not.

> [ ]: Development (unversioned) .so files in -devel subpackage, if present.
>      Note: Unversioned so-files in private %_libdir subdirectory (see
>      attachment). Verify they are not in ld path.

The .so files are in notcurses-devel.

> Generic:
> [ ]: Package is licensed with an open-source compatible license and meets
>      other legal requirements as defined in the legal section of Packaging
>      Guidelines.

It is.  The project is licensed under the Apache License 2.0.

> [ ]: License field in the package spec file matches the actual license.
>      Note: Checking patched sources after %prep for licenses. Licenses
>      found: "Unknown or generated", "Apache License (v2.0)", "*No
>      copyright* Apache License (v2.0)". 260 files have unknown license.
>      Detailed output of licensecheck in
>      /home/dcantrell/notcurses/licensecheck.txt

It does.  ASL 2.0 is the Fedora short name.

> [ ]: License file installed when any subpackage combination is installed.

The main package uses %license to install COPYRIGHT and LICENSE.  All subpackages require the main package, so the license files are installed to the system under any subpackage combination.

> [ ]: %build honors applicable compiler flags or justifies otherwise.

It does.

> [ ]: Package contains no bundled libraries without FPC exception.

It does not.

> [ ]: Changelog in prescribed format.

It is.

> [ ]: Sources contain only permissible code or content.

They do.  Upstream provides a release tarball named %{name}_${version}+dfsg.1.orig.tar.xz to comply with the Debian Free Software Guidelines.  This also complies with Fedora packaging policy, so the spec file uses the DFSG source tarball/

> [ ]: Package contains desktop file if it is a GUI application.

It is not a GUI application.

> [ ]: Development files must be in a -devel package

They are.

> [ ]: Package uses nothing in %doc for runtime.

It does not.

> [ ]: Package consistently uses macros (instead of hard-coded directory
>      names).

It does.

> [ ]: Package is named according to the Package Naming Guidelines.

It is.

> [ ]: Package does not generate any conflict.

It does not.

> [ ]: Package obeys FHS, except libexecdir and /usr/target.

It does.

> [ ]: If the package is a rename of another package, proper Obsoletes and
>      Provides are present.

Not a rename.

> [ ]: Requires correct, justified where necessary.

They are.

> [ ]: Spec file is legible and written in American English.

Yep, totes.

> [ ]: Package contains systemd file(s) if in need.

systemd not needed.

> [ ]: Useful -debuginfo package or justification otherwise.

So useful.

> [ ]: Package is not known to require an ExcludeArch tag.

It does not require an ExcludeArch tag.

> [ ]: Large documentation must go in a -doc subpackage. Large could be size
>      (~1MB) or number of files.
>      Note: Documentation size is 133120 bytes in 2 files.

Documentation included is small and a subpackage is not necessary.

> [ ]: Package complies to the Packaging Guidelines

What do you think I'm doing here?

> Python:
> [ ]: Python eggs must not download any dependencies during the build
>      process.

They do not.

> [ ]: A package which is used by another package via an egg interface should
>      provide egg info.

egg is provided in the python3-notcurses package

> [ ]: Package meets the Packaging Guidelines::Python

It does.

> ===== SHOULD items =====
> 
> Generic:
> [ ]: If the source package does not include license text(s) as a separate
>      file from upstream, the packager SHOULD query upstream to include it.

Upstream includes license text.

> [ ]: Final provides and requires are sane (see attachments).

They are.

> [ ]: Package functions as described.

It does.

> [ ]: Latest version is packaged.

It is.

> [ ]: Package does not include license text files separate from upstream.

It does not.

> [ ]: Description and summary sections in the package spec file contains
>      translations for supported Non-English languages, if available.

Translations unavailable.

> [ ]: Package should compile and build into binary rpms on all supported
>      architectures.

It does.

> [ ]: %check is present and all tests pass.

Unable to use this now until additional BR available to run the test suite.

> [ ]: Packages should try to preserve timestamps of original installed
>      files.

It does.

> ===== EXTRA items =====
> 
> Generic:
> [!]: Spec file according to URL is the same as in SRPM.
>      Note: Bad spec filename: /home/dcantrell/notcurses/srpm-
>      unpacked/notcurses.spec
>      See: (this test has no URL)

This is bogus.  I think this is a bug in the fedora-review tool.  It did not show up until the DFSG source tarball was used.  I think it doesn't know how to tokenize the DFSG basename.

> Rpmlint
> -------
> Checking: notcurses-1.3.2-2.fc33.x86_64.rpm
>           notcurses-devel-1.3.2-2.fc33.x86_64.rpm
>           notcurses-static-1.3.2-2.fc33.x86_64.rpm
>           python3-notcurses-1.3.2-2.fc33.x86_64.rpm
>           notcurses-debuginfo-1.3.2-2.fc33.x86_64.rpm
>           notcurses-debugsource-1.3.2-2.fc33.x86_64.rpm
>           notcurses-1.3.2-2.fc33.src.rpm
> notcurses.x86_64: W: incoherent-version-in-changelog 1.3.2-1
> ['1.3.2-2.fc33', '1.3.2-2']

Change the Release: back to '1'.  You changed it to 2, which fine, but since this will be the initial import to Fedora, no reason to not start at 1.

Or an another entry to the %changelog for 1.3.2-2.

> notcurses-static.x86_64: W: no-documentation

The static package requires the main package which provides the documentation.

> python3-notcurses.x86_64: E: non-executable-script
> /usr/lib64/python3.8/site-packages/notcurses/notcurses.py 644
> /usr/bin/python3 

Should this be executable?  I think fedora-review is picking this up because it has #!/usr/bin/python3 at the top and an if __name__ == "__main__" block.

> notcurses.src: W: inconsistent-file-extension
> notcurses_1.3.2+dfsg.1.orig.tar.xz

Disregard, this is bogus.

> Rpmlint (installed packages)
> ----------------------------
> notcurses-static.x86_64: W: invalid-url URL:
> https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno
> -2] Name or service not known>
> notcurses-static.x86_64: W: no-documentation
> notcurses-devel.x86_64: W: invalid-url URL:
> https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno
> -2] Name or service not known>
> notcurses-debuginfo.x86_64: W: invalid-url URL:
> https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno
> -2] Name or service not known>
> notcurses.x86_64: W: incoherent-version-in-changelog 1.3.2-1
> ['1.3.2-2.fc33', '1.3.2-2']
> notcurses.x86_64: W: invalid-url URL:
> https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno
> -2] Name or service not known>
> notcurses-debugsource.x86_64: W: invalid-url URL:
> https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno
> -2] Name or service not known>
> python3-notcurses.x86_64: W: invalid-url URL:
> https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno
> -2] Name or service not known>

Disregard, this is bogus.  I don't know why fedora-review doesn't have network support at this point in its run.  But this URL is entirely valid.

> python3-notcurses.x86_64: E: non-executable-script
> /usr/lib64/python3.8/site-packages/notcurses/notcurses.py 644
> /usr/bin/python3 

See above.

> Unversioned so-files
> --------------------
> python3-notcurses: /usr/lib64/python3.8/site-packages/_notcurses.abi3.so

This again?  What was the final plan on this?

Comment 23 David Cantrell 2020-04-24 15:09:02 UTC
Condensed remaining items:

* Either change the Release back to 1%{?dist} or add another %changelog entry for 1.3.2-2
* Should /usr/lib64/python3.8/site-packages/notcurses/notcurses.py be 0755?  If so, set it.
* /usr/lib64/python3.8/site-packages/_notcurses.abi3.so lacks a version.

Comment 24 Nick Black 2020-04-24 17:58:02 UTC
> Either change the Release back to 1%{?dist} or add another %changelog entry for 1.3.2-2

by the power of the Delorian, 1.3.2-1 it is.

> Should /usr/lib64/python3.8/site-packages/notcurses/notcurses.py be 0755? 

As opposed to 0775? Aye, not sure how that came to be. Fixed.

> /usr/lib64/python3.8/site-packages/_notcurses.abi3.so lacks a version.

I'm having a ridiculous time getting this to happen, and am not convinced that it needs to. It certainly doesn't if this is only ever dlopen()ed, which is the only way I know it to be used. Most of the Python modules I see in site-packages on Fedora Core 32, as noted, seem to be missing an SONAME. I'll take another hour's worth of stabs at it. How I loathe Python.

Comment 25 Nick Black 2020-04-24 18:49:56 UTC
https://gitlab.kitware.com/cmake/cmake/issues/17430 Finally figured out my SONAME problems. It'll require a new upstream release to fix this, unfortunately, but I'm planning on cutting 1.3.3 this weekend anyway.

For anyone interested, CMake escapes spaces in quoted text. Sometimes. Hence LFLAGS="-soname -L" failing to link. Ugh.

Comment 26 David Cantrell 2020-04-24 18:57:47 UTC
OK, sounds good.  Honestly, I don't know what the preferred Python packaging _way_ is right now in Fedora for .so libs.  I've never explicitly set a version for any Python modules I build from C, so maybe it was handled by a combination of setuptools and the RPM macros in Fedora?  Or maybe it was never set and no one cares?

The cmake issue makes sense and since fedora-review is reporting this now, I suppose it's easy enough to fix.  The fedora-review tool does get maintained as packaging policy changes, so I'm inclined to mostly believe it.

I'll wait for a new SRPM to complete this.

Comment 27 Nick Black 2020-04-24 19:08:17 UTC
This was so much more difficult than it deserved to be, but I now have an SONAME embedded in the _notcurses.abi3.so python cffi module.

Comment 28 Nick Black 2020-04-24 19:08:50 UTC
Created attachment 1681610 [details]
python module with SONAME, per request

There it is!

Comment 29 Nick Black 2020-04-25 23:48:30 UTC
I've rebuilt RPMs and verified that the installed _notcurses.abi3.so has an SONAME of _notcurses.abi3.so.1.

In the meantime, I've also added OpenImageIO support to notcurses upstream. Since OpenImageIO is in Fedora Core (unlike FFmpeg), I've updated the spec file to invoke CMake such that notcurses is built against OpenImageIO. As a result of further changes, this means that notcurses-view is now installed, and that notcurses-demo is built with (Free) multimedia support (i.e. the non-Free multimedia remains uninstalled). This is a total of less than 10 megabytes, but I went ahead and split it out into a noarch notcurses-data package. So we're now at eight binary packages and one SRPM. The base package depends on notcurses-data because the binaries are installed as part of that package. If i moved them out into notcurses-utils, there's no longer a dep on those data files from the shared library package. Just a thought. It's trivial to make that change, so feel free to ask for it.

I should have a new release and SRPM up late tonight.

Comment 30 Nick Black 2020-04-27 02:09:18 UTC
New version 1.3.3 has been released upstream.
Verified spec file via `spectool -g`.
Built source RPM with `rpmbuild -bs`.
Built binary RPMs with `mock -r fedora-rawhide-x86_64 --rebuild notcurses-1.3.0-1.src.rpm`.

I've uploaded all relevant files to https://www.dsscaw.com/repos/dnf/.

This release uses OpenImageIO as a multimedia backend. This support is experimental, but expected to solidify rapidly. There are currently some known issues with video, but images seem to be working fine. I have listed OpenEXR-devel as a dependency; this is necessary until https://bugzilla.redhat.com/show_bug.cgi?id=1827577 is fixed and pushed. At that point, this dependency will be removed. As a result of enabling this functionality, `notcurses-view` is now installed.

This release adds the `notcurses-data` no-arch package, containing Free multimedia used by binaries in the `notcurses` package. I can split these binaries out into their own package to eliminate the dependency if desired, as noted above.

a4959165ea13a42c203decfc38d2fc60a627b140d8bb8ee5459c409cc2160cb5  build.log
7ae8b7da06c4b1d692317254a8d1b94f93b035338742ef1245c0f5bc48e0f403  hw_info.log
339d19192466ceaa701687180d301ed71f20382dc7c905d32c1cda52163e2d15  installed_pkgs.log
ede4e0287341c6c9d528b83c73987680c8276bb2afeec3bfe158e207e132e7a4  notcurses-1.3.3-1.fc33.src.rpm
0985f02ec18265cc3771b26c34e83ce652269bcff1003f37de80ae404d923f11  notcurses-1.3.3-1.fc33.x86_64.rpm
af26b327c590ce4af14f42164a556af0e9893735adda29eb088212bb84b38780  notcurses-data-1.3.3-1.fc33.noarch.rpm
f1096639a2af1b7ae17d3a3a4ddcd8e18950e26068a90341383419420ce1bafd  notcurses-debuginfo-1.3.3-1.fc33.x86_64.rpm
f53e0d877a1d270151d644789f8a592d116b619f0077b82ecb5ce1e50d6f55a8  notcurses-debugsource-1.3.3-1.fc33.x86_64.rpm
86f35747c74a36cc97cb3bbfca1a90d4b0e9300b12dfc7370350618308bc9e11  notcurses-devel-1.3.3-1.fc33.x86_64.rpm
df1810597c0a60cc60723e136e4dd116c4bdde8cf40cd18de1619eeba8f0746b  notcurses-static-1.3.3-1.fc33.x86_64.rpm
15be81548166ffc984fb3269df443e7910ebe8c97638e4a9246b54878fc152bc  python3-notcurses-1.3.3-1.fc33.x86_64.rpm
7a6a0c295ee61caea15bf3e132b34cf3fe95bcdcf6a46a4c9027156f839ddbe5  python3-notcurses-debuginfo-1.3.3-1.fc33.x86_64.rpm
0f70ba49836b2c946463d349762bc979f13f5d2b14f33e6b30e9098f1b91a808  root.log
135dbbce03227f84d0e811f1351df8c48ee88c6623677c9613269033a44face6  state.log

PTAL, thanks!

Comment 31 David Cantrell 2020-04-27 13:02:18 UTC
Latest package reviewed:

Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed



===== MUST items =====

C/C++:
[ ]: Package does not contain kernel modules.
[ ]: Package contains no static executables.
[ ]: Development (unversioned) .so files in -devel subpackage, if present.
     Note: Unversioned so-files in private %_libdir subdirectory (see
     attachment). Verify they are not in ld path.
[x]: If your application is a C or C++ application you must list a
     BuildRequires against gcc, gcc-c++ or clang.
[x]: Header files in -devel subpackage, if present.
[x]: ldconfig not called in %post and %postun for Fedora 28 and later.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.

Generic:
[ ]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[ ]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "Unknown or generated", "Apache License (v2.0)", "*No
     copyright* Apache License (v2.0)". 218 files have unknown license.
     Detailed output of licensecheck in
     /home/dcantrell/notcurses/licensecheck.txt
[ ]: License file installed when any subpackage combination is installed.
[ ]: Package requires other packages for directories it uses.
     Note: No known owner of /usr/share/notcurses
[ ]: Package must own all directories that it creates.
     Note: Directories without known owners: /usr/share/notcurses
[ ]: %build honors applicable compiler flags or justifies otherwise.
[ ]: Package contains no bundled libraries without FPC exception.
[ ]: Changelog in prescribed format.
[ ]: Sources contain only permissible code or content.
[ ]: Package contains desktop file if it is a GUI application.
[ ]: Development files must be in a -devel package
[ ]: Package uses nothing in %doc for runtime.
[ ]: Package consistently uses macros (instead of hard-coded directory
     names).
[ ]: Package is named according to the Package Naming Guidelines.
[ ]: Package does not generate any conflict.
[ ]: Package obeys FHS, except libexecdir and /usr/target.
[ ]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[ ]: Requires correct, justified where necessary.
[ ]: Spec file is legible and written in American English.
[ ]: Package contains systemd file(s) if in need.
[ ]: Useful -debuginfo package or justification otherwise.
[ ]: Package is not known to require an ExcludeArch tag.
[ ]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 143360 bytes in 4 files.
[ ]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: 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 is included in %license.
[x]: Package does not own files or directories owned by other packages.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package must not depend on deprecated() packages.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: Static libraries in -static or -devel subpackage, providing -devel if
     present.
     Note: Package has .a files: notcurses-static.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

Python:
[ ]: Python eggs must not download any dependencies during the build
     process.
[ ]: A package which is used by another package via an egg interface should
     provide egg info.
[ ]: Package meets the Packaging Guidelines::Python
[x]: Package contains BR: python2-devel or python3-devel
[x]: Packages MUST NOT have dependencies (either build-time or runtime) on
     packages named with the unversioned python- prefix unless no properly
     versioned package exists. Dependencies on Python packages instead MUST
     use names beginning with python2- or python3- as appropriate.
[x]: Python packages must not contain %{pythonX_site(lib|arch)}/* in %files
[x]: Binary eggs must be removed in %prep

===== SHOULD items =====

Generic:
[ ]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[ ]: Final provides and requires are sane (see attachments).
[ ]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in
     notcurses-data
[ ]: Package functions as described.
[ ]: Latest version is packaged.
[ ]: Package does not include license text files separate from upstream.
[ ]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[ ]: Package should compile and build into binary rpms on all supported
     architectures.
[ ]: %check is present and all tests pass.
[ ]: Packages should try to preserve timestamps of original installed
     files.
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: The placement of pkgconfig(.pc) files are correct.
[x]: Sources can be downloaded from URI in Source: tag
[x]: SourceX is a working URL.
[x]: Sources are verified with gpgverify first in %prep if upstream
     publishes signatures.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on debuginfo package(s).
     Note: No rpmlint messages.
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Large data in /usr/share should live in a noarch subpackage if package
     is arched.


Rpmlint
-------
Checking: notcurses-1.3.3-1.fc33.x86_64.rpm
          notcurses-devel-1.3.3-1.fc33.x86_64.rpm
          notcurses-static-1.3.3-1.fc33.x86_64.rpm
          notcurses-data-1.3.3-1.fc33.noarch.rpm
          python3-notcurses-1.3.3-1.fc33.x86_64.rpm
          notcurses-debuginfo-1.3.3-1.fc33.x86_64.rpm
          notcurses-debugsource-1.3.3-1.fc33.x86_64.rpm
          notcurses-1.3.3-1.fc33.src.rpm
notcurses.x86_64: W: shared-lib-calls-exit /usr/lib64/libnotcurses.so.1.3.3 exit@GLIBC_2.2.5
notcurses-static.x86_64: W: no-documentation
notcurses-data.noarch: W: no-documentation
python3-notcurses.x86_64: E: non-executable-script /usr/lib64/python3.8/site-packages/notcurses/notcurses.py 644 /usr/bin/python3 
8 packages and 0 specfiles checked; 1 errors, 3 warnings.




Rpmlint (debuginfo)
-------------------
Checking: notcurses-debuginfo-1.3.3-1.fc33.x86_64.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.





Rpmlint (installed packages)
----------------------------
notcurses-debugsource.x86_64: W: invalid-url URL: https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno -2] Name or service not known>
notcurses-debuginfo.x86_64: W: invalid-url URL: https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno -2] Name or service not known>
notcurses-devel.x86_64: W: invalid-url URL: https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno -2] Name or service not known>
python3-notcurses.x86_64: W: invalid-url URL: https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno -2] Name or service not known>
python3-notcurses.x86_64: E: non-executable-script /usr/lib64/python3.8/site-packages/notcurses/notcurses.py 644 /usr/bin/python3 
notcurses.x86_64: W: invalid-url URL: https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno -2] Name or service not known>
notcurses.x86_64: W: shared-lib-calls-exit /usr/lib64/libnotcurses.so.1.3.3 exit@GLIBC_2.2.5
notcurses-data.noarch: W: invalid-url URL: https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno -2] Name or service not known>
notcurses-data.noarch: W: no-documentation
notcurses-static.x86_64: W: invalid-url URL: https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno -2] Name or service not known>
notcurses-static.x86_64: W: no-documentation
7 packages and 0 specfiles checked; 1 errors, 10 warnings.



Unversioned so-files
--------------------
python3-notcurses: /usr/lib64/python3.8/site-packages/_notcurses.abi3.so

Source checksums
----------------
https://nick-black.com/dankamongmen.gpg :
  CHECKSUM(SHA256) this package     : 0a6f979c9d88b0d72310b8462d2066a60b657a2f188a74c3cb19cac0064616ce
  CHECKSUM(SHA256) upstream package : 0a6f979c9d88b0d72310b8462d2066a60b657a2f188a74c3cb19cac0064616ce
https://github.com/dankamongmen/notcurses/releases/download/v1.3.3/notcurses_1.3.3+dfsg.1.orig.tar.xz.asc :
  CHECKSUM(SHA256) this package     : 59f711f6544aacac2b7390cdb4b1c43983ef3a286667d6db651a89055fb362f6
  CHECKSUM(SHA256) upstream package : 59f711f6544aacac2b7390cdb4b1c43983ef3a286667d6db651a89055fb362f6
https://github.com/dankamongmen/notcurses/releases/download/v1.3.3/notcurses_1.3.3+dfsg.1.orig.tar.xz :
  CHECKSUM(SHA256) this package     : b81692696eddda717a0214dc28bb0a4ec3a00a95795b985f0b29240695deebb1
  CHECKSUM(SHA256) upstream package : b81692696eddda717a0214dc28bb0a4ec3a00a95795b985f0b29240695deebb1


Requires
--------
notcurses (rpmlib, GLIBC filtered):
    ld-linux-x86-64.so.2()(64bit)
    libOpenImageIO.so.2.1()(64bit)
    libc.so.6()(64bit)
    libgcc_s.so.1()(64bit)
    libgcc_s.so.1(GCC_3.0)(64bit)
    libm.so.6()(64bit)
    libnotcurses++.so.1()(64bit)
    libnotcurses.so.1()(64bit)
    libpthread.so.0()(64bit)
    libqrcodegen.so.1()(64bit)
    libstdc++.so.6()(64bit)
    libstdc++.so.6(CXXABI_1.3)(64bit)
    libstdc++.so.6(CXXABI_1.3.11)(64bit)
    libstdc++.so.6(CXXABI_1.3.3)(64bit)
    libstdc++.so.6(CXXABI_1.3.5)(64bit)
    libstdc++.so.6(CXXABI_1.3.8)(64bit)
    libstdc++.so.6(CXXABI_1.3.9)(64bit)
    libtinfo.so.6()(64bit)
    notcurses-data
    rtld(GNU_HASH)

notcurses-devel (rpmlib, GLIBC filtered):
    /usr/bin/pkg-config
    cmake-filesystem(x86-64)
    libnotcurses++.so.1()(64bit)
    libnotcurses.so.1()(64bit)
    notcurses(x86-64)
    pkgconfig(notcurses)

notcurses-static (rpmlib, GLIBC filtered):
    notcurses(x86-64)

notcurses-data (rpmlib, GLIBC filtered):

python3-notcurses (rpmlib, GLIBC filtered):
    /usr/bin/python3
    libc.so.6()(64bit)
    libnotcurses.so.1()(64bit)
    notcurses(x86-64)
    python(abi)
    python3.8dist(cffi)
    rtld(GNU_HASH)

notcurses-debuginfo (rpmlib, GLIBC filtered):

notcurses-debugsource (rpmlib, GLIBC filtered):



Provides
--------
notcurses:
    libnotcurses++.so.1()(64bit)
    libnotcurses.so.1()(64bit)
    notcurses
    notcurses(x86-64)

notcurses-devel:
    cmake(notcurses)
    notcurses-devel
    notcurses-devel(x86-64)
    pkgconfig(notcurses)
    pkgconfig(notcurses++)

notcurses-static:
    notcurses-static
    notcurses-static(x86-64)

notcurses-data:
    notcurses-data

python3-notcurses:
    python-notcurses
    python3-notcurses
    python3-notcurses(x86-64)
    python3.8dist(notcurses)
    python38-notcurses
    python3dist(notcurses)

notcurses-debuginfo:
    debuginfo(build-id)
    notcurses-debuginfo
    notcurses-debuginfo(x86-64)

notcurses-debugsource:
    notcurses-debugsource
    notcurses-debugsource(x86-64)



Generated by fedora-review 0.7.5 (5fa5b7e) last change: 2020-02-16
Command line :/usr/bin/fedora-review --rpm-spec -n notcurses-1.3.3-1.fc33.src.rpm
Buildroot used: fedora-rawhide-x86_64
Active plugins: Generic, Python, C/C++, Shell-api
Disabled plugins: Perl, SugarActivity, R, PHP, Haskell, Ocaml, fonts, Java
Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH

Comment 32 David Cantrell 2020-04-27 13:15:22 UTC
For completeness, comments on the manual items:

(In reply to David Cantrell from comment #31)
> [ ]: Package does not contain kernel modules.

It does not.

> [ ]: Package contains no static executables.

It does not.

> [ ]: Development (unversioned) .so files in -devel subpackage, if present.
>      Note: Unversioned so-files in private %_libdir subdirectory (see
>      attachment). Verify they are not in ld path.

They are.

> Generic:
> [ ]: Package is licensed with an open-source compatible license and meets
>      other legal requirements as defined in the legal section of Packaging
>      Guidelines.

Apache Linux 2.0, ASL 2.0 noted in License tag.

> [ ]: License field in the package spec file matches the actual license.
>      Note: Checking patched sources after %prep for licenses. Licenses
>      found: "Unknown or generated", "Apache License (v2.0)", "*No
>      copyright* Apache License (v2.0)". 218 files have unknown license.
>      Detailed output of licensecheck in
>      /home/dcantrell/notcurses/licensecheck.txt

Apache Linux 2.0, ASL 2.0 noted in License tag.

> [ ]: License file installed when any subpackage combination is installed.

It is.

> [ ]: Package requires other packages for directories it uses.
>      Note: No known owner of /usr/share/notcurses

It does.

> [ ]: Package must own all directories that it creates.
>      Note: Directories without known owners: /usr/share/notcurses

The -data package should own %{_datadir}/%{name} in %files.  It's likely sufficient to just do:

%files data
%{_datadir}/%{name}

And that will pick up the contents of the directory as well as the directory itself.

> [ ]: %build honors applicable compiler flags or justifies otherwise.

It does.

> [ ]: Package contains no bundled libraries without FPC exception.

It does not.

> [ ]: Changelog in prescribed format.

Need to skip a line between changelog entries.

> [ ]: Sources contain only permissible code or content.

Correct, using the DFSG release tarball for notcurses.

> [ ]: Package contains desktop file if it is a GUI application.

N/A

> [ ]: Development files must be in a -devel package

They are.

> [ ]: Package uses nothing in %doc for runtime.

It does not.

> [ ]: Package consistently uses macros (instead of hard-coded directory
>      names).

It does.

> [ ]: Package is named according to the Package Naming Guidelines.

It is.

> [ ]: Package does not generate any conflict.

No conflicts.

> [ ]: Package obeys FHS, except libexecdir and /usr/target.

It does.

> [ ]: If the package is a rename of another package, proper Obsoletes and
>      Provides are present.

N/A

> [ ]: Requires correct, justified where necessary.

They are.

> [ ]: Spec file is legible and written in American English.

It is.

> [ ]: Package contains systemd file(s) if in need.

N/A

> [ ]: Useful -debuginfo package or justification otherwise.

Yes

> [ ]: Package is not known to require an ExcludeArch tag.

Not required.

> [ ]: Large documentation must go in a -doc subpackage. Large could be size
>      (~1MB) or number of files.
>      Note: Documentation size is 143360 bytes in 4 files.

Not necessary.

> [ ]: Package complies to the Packaging Guidelines

That's what this review is doing.

> Python:
> [ ]: Python eggs must not download any dependencies during the build
>      process.

It does not.

> [ ]: A package which is used by another package via an egg interface should
>      provide egg info.

It does.

> [ ]: Package meets the Packaging Guidelines::Python

Yes.

> Generic:
> [ ]: If the source package does not include license text(s) as a separate
>      file from upstream, the packager SHOULD query upstream to include it.

License already included in the source.

> [ ]: Final provides and requires are sane (see attachments).

They are.

> [ ]: Fully versioned dependency in subpackages if applicable.
>      Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in
>      notcurses-data

notcurses-data needs:

Requires: %{name}%{?_isa} = %{version}-%{release}

> [ ]: Package functions as described.

It does.

> [ ]: Latest version is packaged.

It is.

> [ ]: Package does not include license text files separate from upstream.

It does not.

> [ ]: Description and summary sections in the package spec file contains
>      translations for supported Non-English languages, if available.

Yes.

> [ ]: Package should compile and build into binary rpms on all supported
>      architectures.

It does.

> [ ]: %check is present and all tests pass.

N/A

> [ ]: Packages should try to preserve timestamps of original installed
>      files.

It does.

> Rpmlint
> -------
> Checking: notcurses-1.3.3-1.fc33.x86_64.rpm
>           notcurses-devel-1.3.3-1.fc33.x86_64.rpm
>           notcurses-static-1.3.3-1.fc33.x86_64.rpm
>           notcurses-data-1.3.3-1.fc33.noarch.rpm
>           python3-notcurses-1.3.3-1.fc33.x86_64.rpm
>           notcurses-debuginfo-1.3.3-1.fc33.x86_64.rpm
>           notcurses-debugsource-1.3.3-1.fc33.x86_64.rpm
>           notcurses-1.3.3-1.fc33.src.rpm
> notcurses.x86_64: W: shared-lib-calls-exit /usr/lib64/libnotcurses.so.1.3.3
> exit@GLIBC_2.2.5

It does, really?  Necessary?

> notcurses-static.x86_64: W: no-documentation
> notcurses-data.noarch: W: no-documentation

Documentation is provided by the main package which these packages require.

> python3-notcurses.x86_64: E: non-executable-script
> /usr/lib64/python3.8/site-packages/notcurses/notcurses.py 644
> /usr/bin/python3 

This again.

> Rpmlint (installed packages)
> ----------------------------
> notcurses-debugsource.x86_64: W: invalid-url URL:
> https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno
> -2] Name or service not known>
> notcurses-debuginfo.x86_64: W: invalid-url URL:
> https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno
> -2] Name or service not known>
> notcurses-devel.x86_64: W: invalid-url URL:
> https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno
> -2] Name or service not known>
> python3-notcurses.x86_64: W: invalid-url URL:
> https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno
> -2] Name or service not known>
> notcurses.x86_64: W: invalid-url URL:
> https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno
> -2] Name or service not known>
> notcurses-data.noarch: W: invalid-url URL:
> https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno
> -2] Name or service not known>
> notcurses-data.noarch: W: no-documentation
> notcurses-static.x86_64: W: invalid-url URL:
> https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno
> -2] Name or service not known>
> notcurses-static.x86_64: W: no-documentation

This is bogus, disregard.  URL is valid.

> Unversioned so-files
> --------------------
> python3-notcurses: /usr/lib64/python3.8/site-packages/_notcurses.abi3.so

This again?


Summary after this comment.  I'm still in the middle of the review, so wait for my condensed summary.

Comment 33 David Cantrell 2020-04-27 13:36:28 UTC
Condensed summary:

* The notcurses-data package does not really make sense to me.  The main package requires it, so there's never an instance where you wouldn't have the notcurses package installed and not the data subpackage.  To me what would make more sense is having a notcurses-demo subpackage that contains the demo programs and the data files used by those demos.  Reduce the notcurses main package to just the shared libraries needed for runtime use.  This is just my opinion and the split up is your decision, but having a dedicated demo subpackage would make more sense to me.

* If you stick with the notcurses-data package, then it needs an explicit Requires on the main package in the format "Requires: %{name}%{?_isa} = %{version}-%{release}"

* The notcurses-data package needs to own /usr/share/notcurses.  I would reduce the files section to:

%files data
%{_datadir}/%{name}

(Of course, if you go with the demo subpackage idea then change accordingly.)

* Entries in the %changelog should have a blank line between them.

* /usr/lib64/libnotcurses.so.1.3.3 calls exit(), which I found in ncsubproc_createv() and friends.  If this is deliberate for the library, that's fine.  Without digging in to it more, I'm assuming 'ncsubproc' is spawning processes for planes, but I haven't gotten that far in my playing with notcurses.

* /usr/lib64/python3.8/site-packages/notcurses/notcurses.py is still 0644.  Proposed fixes:
      * Add "chmod 0755 %{buildroot}%{python3_sitearch}/%{name}/%{name}.py" to the %install block
      * Add "%attr(root, root, 0755) %{python3_sitearch}/%{name}/%{name}.py" to the %files block for the python package
      * Figure out how to modify setup.py to install notcurses.py with 0755 permissions.

BOGUS THINGS THAT CAN BE IGNORED:

* The unversioned /usr/lib64/python3.8/site-packages/_notcurses.abi3.so file is misleading in the fedora-review tool.  This is reporting that _notcurses.abi3.so is a ".so" file without a corresponding ".so.1.2.3" file alongside it like you see in /usr/lib or something.  It thingks this is a devel symlink.  This fedora-review test should be skipped for Python .so files, so I think I'll track that down today.

Comment 34 Nick Black 2020-04-27 19:15:47 UTC
(1) You are correct in all analysis of the dependencies and deployment of the -data package--it would just be part of the main package. The only reason why I made it distinct was due to wanting to avoid replication across multiple architecture-dependent binary packages. I.e. if the package is provided for 5 architectures, without this no-arch package, you're replicating the same 5MB video across all five architectures, consuming 25MB on FTP servers etc. If this is not a big concern for Fedora (it's a cause for global apocalypse in Debian thinking), it can happily be subsumed into an existing binary-specific package.

So I think what I'll do is *almost* what you allude to, and *almost* what I mentioned earlier:

 - split out the binaries from the shared libraries, into notcurses-utils
 - fold notcurses-data into notcurses-utils

This way, there's no extra package, people who are only installing notcurses as a dependency for some other program needn't install stuff they don't need, and everything is still shipped. Sound good?

(2) Done.

(3) exit(2) is called only in the following structure:


  ret->pid = launch_pipe_process(&fd, &ret->pidfd);
  if(ret->pid == 0){
    execv(bin, arg);
    fprintf(stderr, "Error execv()ing %s\n", bin);
    exit(EXIT_FAILURE);
  }else if(ret->pid < 0){
    free(ret);
    return NULL;
  }

 so I think that's safe.

(4) Got it, will test the installed rpm for the proper perms this time, how embarrassing.

Thankfully, all issues you've raised can be fixed with specfile changes, and thus do not require a new upstream release. I ought have all issues fixed by EoD.

Comment 35 Nick Black 2020-04-29 03:57:42 UTC
I've rebuilt with a new spec file.

* Verified that notcurses.py is installed 0755 (achieved via %attr directive)
* Verified that /usr/share/notcurses gets cleaned up on removal (indeed, find /usr -iname \*notcurses\* is empty upon removal)
* Verified that the -data package is gone, and the binaries/man pages are in -utils. The base package installs only shared libraries.
* Blank line between specfile changelog entries.

I think this answers everything you raised in this most recent round of review. In particular, /usr/share/notcurses is properly owned (and thus removed).

As I noted above, the exit() is in the child path of a fork(), following a failed exec(). I believe this to be acceptable library behavior. Please note that all child processes are managed with Linux 5 pidfds, eliminating some typical worries there.

I've uploaded the new SRPM and RPMs:

https://www.dsscaw.com/repos/dnf/

[vps](0) $ sha256sum *
12a220effbbb6674880ad5da448a167905ab22611ea3569d6eaf97ed46121891  build.log
3b361c0d38f2fd17ab6b0a220d35557c1c27581e86ac216f6c6680a538ff4125  hw_info.log
b6671f523a9f27223e777be3a23c8850bae755709e1ed75fa732419c2b81a35b  installed_pkgs.log
e73cbfefa06fb4288f7f0e160417108bce953009b11929d4a5c23696e162ba9c  notcurses-1.3.3-1.fc33.src.rpm
677adf06441b7be9ef0ebe2795a3ac67332e11a360b5999f38c385e10361b351  notcurses-1.3.3-1.fc33.x86_64.rpm
58b7cfb64a1e0000738b966bda7d4adb6fc84cb113d81a386954e42b797e6439  notcurses-debuginfo-1.3.3-1.fc33.x86_64.rpm
579e4c10d69808455332cf7cf1422a187466ed59f73013640282781458d9137f  notcurses-debugsource-1.3.3-1.fc33.x86_64.rpm
666d9ec54d33e8accea13325cd71d033e0b96a859a3e83b47e610692c1bd59d8  notcurses-devel-1.3.3-1.fc33.x86_64.rpm
a51497a0596ba57a0281824266408df7cecf517cf9031b19c1262e4aaea094f4  notcurses-static-1.3.3-1.fc33.x86_64.rpm
10e57a4dcb1f88af632e7c540647c5fcf2a26a96e3bfbbbfb53048fed05a972c  notcurses-utils-1.3.3-1.fc33.x86_64.rpm
9bd852283ef92bbaa3702d45dcbf7cedba2cc4e274f75dca392cf18faefee235  notcurses-utils-debuginfo-1.3.3-1.fc33.x86_64.rpm
d4612c2fe0dd9ffa7decbbe763e03f1cdee30d68e5bc47af7eb7b9bd5d3cfb24  python3-notcurses-1.3.3-1.fc33.x86_64.rpm
9e855b0455e899b70df7ed59591e97d9f5c64e0d6b560c0eee2a70e72113decf  python3-notcurses-debuginfo-1.3.3-1.fc33.x86_64.rpm
54949abce9bf6343969eac5a9169a79fc6dc2194441fe2ddeecbe96c4b22b22e  root.log
46ff4b0254c09f75d9fa68710ff30035e559a596bf459b4c88b3a00b368a337f  state.log
[vps](0) $ 


Specfile: https://raw.githubusercontent.com/dankamongmen/notcurses/master/tools/notcurses.spec

PTAL.

Comment 36 David Cantrell 2020-04-29 20:31:21 UTC
Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed


Issues:
=======
- Package does not contain duplicates in %files.
  Note: warning: File listed twice: /usr/lib64/python3.8/site-
  packages/notcurses/notcurses.py
  See: https://docs.fedoraproject.org/en-US/packaging-
  guidelines/#_duplicate_files


===== MUST items =====

C/C++:
[ ]: Package does not contain kernel modules.
[ ]: Package contains no static executables.
[ ]: Development (unversioned) .so files in -devel subpackage, if present.
     Note: Unversioned so-files in private %_libdir subdirectory (see
     attachment). Verify they are not in ld path.
[x]: If your application is a C or C++ application you must list a
     BuildRequires against gcc, gcc-c++ or clang.
[x]: Header files in -devel subpackage, if present.
[x]: ldconfig not called in %post and %postun for Fedora 28 and later.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.

Generic:
[ ]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[ ]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "Unknown or generated", "Apache License (v2.0)", "*No
     copyright* Apache License (v2.0)". 218 files have unknown license.
     Detailed output of licensecheck in
     /home/dcantrell/notcurses/licensecheck.txt
[ ]: License file installed when any subpackage combination is installed.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "Unknown or generated", "Apache License (v2.0)", "*No
     copyright* Apache License (v2.0)". 218 files have unknown license.
     Detailed output of licensecheck in
     /home/dcantrell/notcurses/licensecheck.txt
[ ]: License file installed when any subpackage combination is installed.
[ ]: %build honors applicable compiler flags or justifies otherwise.
[ ]: Package contains no bundled libraries without FPC exception.
[ ]: Changelog in prescribed format.
[ ]: Sources contain only permissible code or content.
[ ]: Package contains desktop file if it is a GUI application.
[ ]: Development files must be in a -devel package
[ ]: Package uses nothing in %doc for runtime.
[ ]: Package consistently uses macros (instead of hard-coded directory
     names).
[ ]: Package is named according to the Package Naming Guidelines.
[ ]: Package does not generate any conflict.
[ ]: Package obeys FHS, except libexecdir and /usr/target.
[ ]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[ ]: Requires correct, justified where necessary.
[ ]: Spec file is legible and written in American English.
[ ]: Package contains systemd file(s) if in need.
[ ]: Useful -debuginfo package or justification otherwise.
[ ]: Package is not known to require an ExcludeArch tag.
[ ]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 143360 bytes in 4 files.
[ ]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: 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 is included in %license.
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[x]: Permissions on files are set properly.
[x]: Package must not depend on deprecated() packages.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: Static libraries in -static or -devel subpackage, providing -devel if
     present.
     Note: Package has .a files: notcurses-static.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

Python:
[ ]: Python eggs must not download any dependencies during the build
     process.
[ ]: A package which is used by another package via an egg interface should
     provide egg info.
[ ]: Package meets the Packaging Guidelines::Python
[x]: Package contains BR: python2-devel or python3-devel
[x]: Packages MUST NOT have dependencies (either build-time or runtime) on
     packages named with the unversioned python- prefix unless no properly
     versioned package exists. Dependencies on Python packages instead MUST
     use names beginning with python2- or python3- as appropriate.
[x]: Python packages must not contain %{pythonX_site(lib|arch)}/* in %files
[x]: Binary eggs must be removed in %prep

===== SHOULD items =====

Generic:
[ ]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[ ]: Final provides and requires are sane (see attachments).
[ ]: Package functions as described.
[ ]: Latest version is packaged.
[ ]: Package does not include license text files separate from upstream.
[ ]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[ ]: Package should compile and build into binary rpms on all supported
     architectures.
[ ]: %check is present and all tests pass.
[ ]: Packages should try to preserve timestamps of original installed
     files.
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Fully versioned dependency in subpackages if applicable.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: The placement of pkgconfig(.pc) files are correct.
[x]: Sources can be downloaded from URI in Source: tag
[x]: SourceX is a working URL.
[x]: Sources are verified with gpgverify first in %prep if upstream
     publishes signatures.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[ ]: Large data in /usr/share should live in a noarch subpackage if package
     is arched.
     Note: Arch-ed rpms have a total of 4044800 bytes in /usr/share
[x]: Rpmlint is run on debuginfo package(s).
     Note: No rpmlint messages.
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).


Rpmlint
-------
Checking: notcurses-1.3.3-1.fc33.x86_64.rpm
          notcurses-devel-1.3.3-1.fc33.x86_64.rpm
          notcurses-static-1.3.3-1.fc33.x86_64.rpm
          notcurses-utils-1.3.3-1.fc33.x86_64.rpm
          python3-notcurses-1.3.3-1.fc33.x86_64.rpm
          notcurses-debuginfo-1.3.3-1.fc33.x86_64.rpm
          notcurses-debugsource-1.3.3-1.fc33.x86_64.rpm
          notcurses-1.3.3-1.fc33.src.rpm
notcurses.x86_64: W: shared-lib-calls-exit /usr/lib64/libnotcurses.so.1.3.3 exit@GLIBC_2.2.5
notcurses-static.x86_64: W: no-documentation
8 packages and 0 specfiles checked; 0 errors, 2 warnings.




Rpmlint (debuginfo)
-------------------
Checking: notcurses-debuginfo-1.3.3-1.fc33.x86_64.rpm
          notcurses-utils-debuginfo-1.3.3-1.fc33.x86_64.rpm
2 packages and 0 specfiles checked; 0 errors, 0 warnings.





Rpmlint (installed packages)
----------------------------
notcurses-utils-debuginfo.x86_64: W: invalid-url URL: https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno -2] Name or service not known>
notcurses-debuginfo.x86_64: W: invalid-url URL: https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno -2] Name or service not known>
notcurses-devel.x86_64: W: invalid-url URL: https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno -2] Name or service not known>
python3-notcurses.x86_64: W: invalid-url URL: https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno -2] Name or service not known>
notcurses-utils.x86_64: W: invalid-url URL: https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno -2] Name or service not known>
notcurses-debugsource.x86_64: W: invalid-url URL: https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno -2] Name or service not known>
notcurses-static.x86_64: W: invalid-url URL: https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno -2] Name or service not known>
notcurses-static.x86_64: W: no-documentation
notcurses.x86_64: W: invalid-url URL: https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno -2] Name or service not known>
notcurses.x86_64: W: shared-lib-calls-exit /usr/lib64/libnotcurses.so.1.3.3 exit@GLIBC_2.2.5
8 packages and 0 specfiles checked; 0 errors, 10 warnings.



Unversioned so-files
--------------------
python3-notcurses: /usr/lib64/python3.8/site-packages/_notcurses.abi3.so

Source checksums
----------------
https://nick-black.com/dankamongmen.gpg :
  CHECKSUM(SHA256) this package     : 0a6f979c9d88b0d72310b8462d2066a60b657a2f188a74c3cb19cac0064616ce
  CHECKSUM(SHA256) upstream package : 0a6f979c9d88b0d72310b8462d2066a60b657a2f188a74c3cb19cac0064616ce
https://github.com/dankamongmen/notcurses/releases/download/v1.3.3/notcurses_1.3.3+dfsg.1.orig.tar.xz.asc :
  CHECKSUM(SHA256) this package     : 59f711f6544aacac2b7390cdb4b1c43983ef3a286667d6db651a89055fb362f6
  CHECKSUM(SHA256) upstream package : 59f711f6544aacac2b7390cdb4b1c43983ef3a286667d6db651a89055fb362f6
https://github.com/dankamongmen/notcurses/releases/download/v1.3.3/notcurses_1.3.3+dfsg.1.orig.tar.xz :
  CHECKSUM(SHA256) this package     : b81692696eddda717a0214dc28bb0a4ec3a00a95795b985f0b29240695deebb1
  CHECKSUM(SHA256) upstream package : b81692696eddda717a0214dc28bb0a4ec3a00a95795b985f0b29240695deebb1


Requires
--------
notcurses (rpmlib, GLIBC filtered):
    ld-linux-x86-64.so.2()(64bit)
    libOpenImageIO.so.2.1()(64bit)
    libc.so.6()(64bit)
    libgcc_s.so.1()(64bit)
    libgcc_s.so.1(GCC_3.0)(64bit)
    libm.so.6()(64bit)
    libnotcurses.so.1()(64bit)
    libpthread.so.0()(64bit)
    libqrcodegen.so.1()(64bit)
    libstdc++.so.6()(64bit)
    libstdc++.so.6(CXXABI_1.3)(64bit)
    libstdc++.so.6(CXXABI_1.3.11)(64bit)
    libstdc++.so.6(CXXABI_1.3.3)(64bit)
    libstdc++.so.6(CXXABI_1.3.5)(64bit)
    libstdc++.so.6(CXXABI_1.3.8)(64bit)
    libstdc++.so.6(CXXABI_1.3.9)(64bit)
    libtinfo.so.6()(64bit)
    rtld(GNU_HASH)
notcurses-devel (rpmlib, GLIBC filtered):
    /usr/bin/pkg-config
    cmake-filesystem(x86-64)
    libnotcurses++.so.1()(64bit)
    libnotcurses.so.1()(64bit)
    notcurses(x86-64)
    pkgconfig(notcurses)

notcurses-static (rpmlib, GLIBC filtered):
    notcurses(x86-64)

notcurses-utils (rpmlib, GLIBC filtered):
    libc.so.6()(64bit)
    libgcc_s.so.1()(64bit)
    libgcc_s.so.1(GCC_3.0)(64bit)
    libnotcurses++.so.1()(64bit)
    libnotcurses.so.1()(64bit)
    libpthread.so.0()(64bit)
    libstdc++.so.6()(64bit)
    libstdc++.so.6(CXXABI_1.3)(64bit)
    libstdc++.so.6(CXXABI_1.3.9)(64bit)
    notcurses(x86-64)
    rtld(GNU_HASH)

python3-notcurses (rpmlib, GLIBC filtered):
    /usr/bin/python3
    libc.so.6()(64bit)
    libnotcurses.so.1()(64bit)
    notcurses(x86-64)
    python(abi)
    python3.8dist(cffi)
    rtld(GNU_HASH)

notcurses-debuginfo (rpmlib, GLIBC filtered):

notcurses-debugsource (rpmlib, GLIBC filtered):



Provides
--------
notcurses:
    libnotcurses++.so.1()(64bit)
    libnotcurses.so.1()(64bit)
    notcurses
    notcurses(x86-64)

notcurses-devel:
    cmake(notcurses)
    notcurses-devel
    notcurses-devel(x86-64)
    pkgconfig(notcurses)
    pkgconfig(notcurses++)

notcurses-static:
    notcurses-static
    notcurses-static(x86-64)

notcurses-utils:
    notcurses-utils
    notcurses-utils(x86-64)

python3-notcurses:
    python-notcurses
    python3-notcurses
    python3-notcurses(x86-64)
    python3.8dist(notcurses)
    python38-notcurses
    python3dist(notcurses)

notcurses-debuginfo:
    debuginfo(build-id)
    notcurses-debuginfo
    notcurses-debuginfo(x86-64)

notcurses-debugsource:
    notcurses-debugsource
    notcurses-debugsource(x86-64)



Generated by fedora-review 0.7.5 (5fa5b7e) last change: 2020-02-16
Command line :/usr/bin/fedora-review --rpm-spec -n notcurses-1.3.3-1.fc33.src.rpm
Buildroot used: fedora-rawhide-x86_64
Active plugins: Python, Generic, C/C++, Shell-api
Disabled plugins: Ocaml, R, Haskell, SugarActivity, PHP, fonts, Perl, Java
Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH

Comment 37 David Cantrell 2020-04-29 20:38:38 UTC
Let's make this the last one...

(In reply to David Cantrell from comment #36)
> [ ]: Package does not contain kernel modules.

It does not.

> [ ]: Package contains no static executables.

It does not.

> [ ]: Development (unversioned) .so files in -devel subpackage, if present.
>      Note: Unversioned so-files in private %_libdir subdirectory (see
>      attachment). Verify they are not in ld path.

They are.

> [ ]: Package is licensed with an open-source compatible license and meets
>      other legal requirements as defined in the legal section of Packaging
>      Guidelines.

Yes, Apache License 2.0

> [ ]: License field in the package spec file matches the actual license.
>      Note: Checking patched sources after %prep for licenses. Licenses
>      found: "Unknown or generated", "Apache License (v2.0)", "*No
>      copyright* Apache License (v2.0)". 218 files have unknown license.
>      Detailed output of licensecheck in
>      /home/dcantrell/notcurses/licensecheck.txt

Yep, ASL 2.0 which is how Fedora spells "Apache License 2.0".

> [ ]: License file installed when any subpackage combination is installed.
>      Note: Checking patched sources after %prep for licenses. Licenses
>      found: "Unknown or generated", "Apache License (v2.0)", "*No
>      copyright* Apache License (v2.0)". 218 files have unknown license.
>      Detailed output of licensecheck in
>      /home/dcantrell/notcurses/licensecheck.txt

Yes.

> [ ]: License file installed when any subpackage combination is installed.

Yes.

> [ ]: %build honors applicable compiler flags or justifies otherwise.

Yes.

> [ ]: Package contains no bundled libraries without FPC exception.

It does not.

> [ ]: Changelog in prescribed format.

Yes.  rpm does not complain.

> [ ]: Sources contain only permissible code or content.

Correct, using the DFSG source release.

> [ ]: Package contains desktop file if it is a GUI application.

It is not a desktop application.

> [ ]: Development files must be in a -devel package

They are.

> [ ]: Package uses nothing in %doc for runtime.

It does not.

> [ ]: Package consistently uses macros (instead of hard-coded directory
>      names).

Yes.

> [ ]: Package is named according to the Package Naming Guidelines.

It is.

> [ ]: Package does not generate any conflict.

Nope.

> [ ]: Package obeys FHS, except libexecdir and /usr/target.

It does.

> [ ]: If the package is a rename of another package, proper Obsoletes and
>      Provides are present.

N/A

> [ ]: Requires correct, justified where necessary.

Yes.

> [ ]: Spec file is legible and written in American English.

Yes.

> [ ]: Package contains systemd file(s) if in need.

N/A

> [ ]: Useful -debuginfo package or justification otherwise.

Yes.

> [ ]: Package is not known to require an ExcludeArch tag.

N/A

> [ ]: Large documentation must go in a -doc subpackage. Large could be size
>      (~1MB) or number of files.
>      Note: Documentation size is 143360 bytes in 4 files.

Not needed here.

> [ ]: Package complies to the Packaging Guidelines

Again, this is what I'm doing.

> [ ]: Python eggs must not download any dependencies during the build
>      process.

It does not.

> [ ]: A package which is used by another package via an egg interface should
>      provide egg info.

It does.

> [ ]: Package meets the Packaging Guidelines::Python

Yes, very much so.

> [ ]: If the source package does not include license text(s) as a separate
>      file from upstream, the packager SHOULD query upstream to include it.

It includes the license.

> [ ]: Final provides and requires are sane (see attachments).

Yes.

> [ ]: Package functions as described.

Yes.

> [ ]: Latest version is packaged.

Yes.

> [ ]: Package does not include license text files separate from upstream.

It does not.

> [ ]: Description and summary sections in the package spec file contains
>      translations for supported Non-English languages, if available.

N/A

> [ ]: Package should compile and build into binary rpms on all supported
>      architectures.

It does.

> [ ]: %check is present and all tests pass.

N/A until doctest passes package review.

> [ ]: Packages should try to preserve timestamps of original installed
>      files.

It does.  But really, what is time?

> [ ]: Large data in /usr/share should live in a noarch subpackage if package
>      is arched.
>      Note: Arch-ed rpms have a total of 4044800 bytes in /usr/share

We're ok with this.

> notcurses.x86_64: W: shared-lib-calls-exit /usr/lib64/libnotcurses.so.1.3.3
> exit@GLIBC_2.2.5

This is ok per comment #34 and comment #35.

> notcurses-static.x86_64: W: no-documentation

notcurses-static requires the main package which brings in documentation.

> notcurses-utils-debuginfo.x86_64: W: invalid-url URL:
> https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno
> -2] Name or service not known>
> notcurses-debuginfo.x86_64: W: invalid-url URL:
> https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno
> -2] Name or service not known>
> notcurses-devel.x86_64: W: invalid-url URL:
> https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno
> -2] Name or service not known>
> python3-notcurses.x86_64: W: invalid-url URL:
> https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno
> -2] Name or service not known>
> notcurses-utils.x86_64: W: invalid-url URL:
> https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno
> -2] Name or service not known>
> notcurses-debugsource.x86_64: W: invalid-url URL:
> https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno
> -2] Name or service not known>
> notcurses-static.x86_64: W: invalid-url URL:
> https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno
> -2] Name or service not known>
> notcurses-static.x86_64: W: no-documentation
> notcurses.x86_64: W: invalid-url URL:
> https://nick-black.com/dankwiki/index.php/Notcurses <urlopen error [Errno
> -2] Name or service not known>

Lies.  This is bogus, so disregard.  URL is valid.

> notcurses.x86_64: W: shared-lib-calls-exit /usr/lib64/libnotcurses.so.1.3.3
> exit@GLIBC_2.2.5

This is ok per comment #34 and comment #35.

> python3-notcurses: /usr/lib64/python3.8/site-packages/_notcurses.abi3.so

This is a bug in fedora-review.  See, I filed it:
https://pagure.io/FedoraReview/issue/389

Comment 38 David Cantrell 2020-04-29 20:40:07 UTC
Everything looks good.  My only suggestion is to put a blank line between %changelog entries in the spec file, so it would look like:


%changelog
* Sat Apr 25 2020 Nick Black <dankamongmen@gmail.com> - 1.3.3-1
- New upstream version, incorporate review feedback
- Build against OpenImageIO, install notcurses-view and data.

* Tue Apr 07 2020 Nick Black <dankamongmen@gmail.com> - 1.3.3-1
- Initial Fedora packaging


But that's only for readability in the spec file.  RPM breaks all that out and stores it in the header differently so when you do "rpm -q --changelog" it's generating that output and not just dumping a text file.

APPROVED.

Comment 39 Nick Black 2020-04-30 02:46:49 UTC
Wait are you not seeing the blank lines in the spec file? I totally do:

[schwarzgerat](0) $ tail tools/*spec
%attr(0755, -, -) %{python3_sitearch}/notcurses/notcurses.py
%{python3_sitearch}/*.so

%changelog
* Sat Apr 25 2020 Nick Black <dankamongmen@gmail.com> - 1.3.3-1
- New upstream version, incorporate review feedback
- Build against OpenImageIO, install notcurses-view and data.

* Tue Apr 07 2020 Nick Black <dankamongmen@gmail.com> - 1.3.3-1
- Initial Fedora packaging
[schwarzgerat](0) $

Comment 40 David Cantrell 2020-04-30 16:06:43 UTC
I do not see blank lines.  I am looking at the spec file in the SRPM.  Here's what I see:

Name:          notcurses
Version:       1.3.3
Release:       1%{?dist}
Summary:       Character graphics and TUI library
License:       ASL 2.0
URL:           https://nick-black.com/dankwiki/index.php/Notcurses
Source0:       https://github.com/dankamongmen/notcurses/releases/download/v%{version}/notcurses_%{version}+dfsg.1.orig.tar.xz
Source1:       https://github.com/dankamongmen/%{name}/releases/download/v%{version}/notcurses_%{version}+dfsg.1.orig.tar.xz.asc
Source2:       https://nick-black.com/dankamongmen.gpg

BuildRequires: gnupg2
BuildRequires: cmake
BuildRequires: gcc-c++
BuildRequires: libqrcodegen-devel
BuildRequires: OpenEXR-devel
BuildRequires: OpenImageIO-devel
BuildRequires: pandoc
BuildRequires: python3-devel
BuildRequires: python3-cffi
BuildRequires: pkgconfig(ncurses)

%description
Notcurses facilitates the creation of modern TUI programs,
making full use of Unicode and 24-bit TrueColor. It presents
an API similar to that of Curses, and rides atop Terminfo.
This package includes C and C++ shared libraries.

%package devel
Summary:       Development files for the Notcurses library
License:       ASL 2.0
Requires:      %{name}%{?_isa} = %{version}-%{release}

%description devel
Development files for the notcurses library.

%package static
Summary:       Static library for the Notcurses library
License:       ASL 2.0
Requires:      %{name}%{?_isa} = %{version}-%{release}

%description static
A statically-linked version of the notcurses library.

%package utils
Summary:       Binaries from the Notcurses project
License:       ASL 2.0
Requires:      %{name}%{?_isa} = %{version}-%{release}

%description utils
Binaries from Notcurses, and multimedia content used thereby.

%package -n python3-%{name}
Summary:       Python wrappers for notcurses
License:       ASL 2.0
Requires:      %{name}%{?_isa} = %{version}-%{release}

%description -n python3-%{name}
Python wrappers and a demonstration script for the notcurses library.

%prep
%{gpgverify} --keyring='%{SOURCE2}' --signature='%{SOURCE1}' --data='%{SOURCE0}'
%autosetup

# Tests have been disabled due to absence of doctest in Fedora (as of F32)
%build
mkdir build
cd build
%cmake -DUSE_MULTIMEDIA=oiio -DUSE_TESTS=off -DDFSG_BUILD=on ..
%make_build
cd python
%py3_build

%install
cd build
%make_install
cd python
%py3_install

%files
%doc CHANGELOG.md OTHERS.md README.md USAGE.md
%license COPYRIGHT LICENSE
%{_libdir}/libnotcurses.so.%{version}
%{_libdir}/libnotcurses.so.1
%{_libdir}/libnotcurses++.so.1
%{_libdir}/libnotcurses++.so.%{version}

%files devel
%{_includedir}/notcurses/
%{_includedir}/ncpp/
%{_libdir}/libnotcurses.so
%{_libdir}/libnotcurses++.so
%{_libdir}/cmake/notcurses
%{_libdir}/pkgconfig/notcurses.pc
%{_libdir}/pkgconfig/notcurses++.pc
%{_mandir}/man3/*.3*

%files static
%{_libdir}/libnotcurses.a
%{_libdir}/libnotcurses++.a

%files utils
# Don't use a wildcard, lest we pull in notcurses-pydemo.1. We install the man
# pages for notcurses-tester, which we're not yet installing, because we intend
# to install it Real Soon and it's IMHO not worth mucking with the CMake in the
# meantime FIXME.
%{_bindir}/notcurses-demo
%{_bindir}/notcurses-input
%{_bindir}/notcurses-ncreel
%{_bindir}/notcurses-tetris
%{_bindir}/notcurses-view
%{_mandir}/man1/notcurses-demo.1*
%{_mandir}/man1/notcurses-input.1*
%{_mandir}/man1/notcurses-ncreel.1*
%{_mandir}/man1/notcurses-tester.1*
%{_mandir}/man1/notcurses-tetris.1*
%{_mandir}/man1/notcurses-view.1*
%{_datadir}/%{name}

%files -n python3-%{name}
%{_bindir}/notcurses-pydemo
%{_mandir}/man1/notcurses-pydemo.1*
%{python3_sitearch}/*egg-info/
%{python3_sitearch}/notcurses/
%attr(0755, -, -) %{python3_sitearch}/notcurses/notcurses.py
%{python3_sitearch}/*.so

%changelog
* Sat Apr 25 2020 Nick Black <dankamongmen@gmail.com> - 1.3.3-1
- New upstream version, incorporate review feedback
- Build against OpenImageIO, install notcurses-view and data.
* Tue Apr 07 2020 Nick Black <dankamongmen@gmail.com> - 1.3.3-1
- Initial Fedora packaging

Comment 41 Nick Black 2020-04-30 17:29:14 UTC
Ahhhh, I had thought the spec file was distinct from the SRPM. Your explanation makes perfect sense. I'll put up a SRPM with the current spec shortly. Thanks!

Comment 42 David Cantrell 2020-04-30 18:43:39 UTC
Yeah, sorry about that.  The review process states that you need to post both a spec file and an SRPM file.  Personally, I just need the SRPM file.  I think posting the spec file just makes it easier for reviewers to start by looking at that.

When the git repo is created for the package on src.fedoraproject.org, you will import the SRPM using fedpkg.  For example:

    fedpkg co notcurses
    cd notcurses
    fedpkg import NOTCURSES.src.rpm

Then git status and make sure the files are queued for a commit.  Then your first commit on master (which == rawhide) can be done with:

    fedpkg ci -c -p -s

The -c option will generate a "clog", which takes the diff of the %changelog in the spec file since the last commit and uses that for the git commit message.  The package git repo commit history is mostly garbage, but I do this since it's mostly consistent with what other people do.  I view the package git repo as purely downstream especially if it's for something I maintain.

The -p option does a "fedpkg push" after the commit.

The -s option adds -s to the git commit to get a signed-off by.  We didn't do that when wee migrated pkg-cvs to pkg-git, but it's there now and I try to use it.

I do recommend checking your git user.name and user.email and make sure those are set to what you want to use in Fedora.

After the ci, you can build:

    fedpkg build

If the upstream notcurses project can generate an SRPM on release, then you can automate the process of doing Fedora builds using the steps above.  If you want to go further and set up automatic builds each time you commit to your upstream repo, you can do that with a Copr repository.  I do that for my project (rpminspect) so that there's a repo with the latest available development build for people to install.  When I make a versioned release, I drop that in the main Fedora repos.

See https://github.com/rpminspect/rpminspect for how that integration works.  I bundled all of my garbage scripts together that help me automate this stuff at https://github.com/dcantrell/continuous-goodies.  Steal, share, etc.

Comment 43 David Cantrell 2020-04-30 18:47:17 UTC
Oh, and my Copr repo for rpminspect:

https://copr.fedorainfracloud.org/coprs/dcantrell/rpminspect/

These I find really handy because it gives bug reporters an easy way to "try the latest build" and not have to compile and install themselves.  They can enable Copr repos with dnf directly and away it goes.  Because rpm is deficient with versioning and release numbers, it's best to munge the release value to get what amounts to a timestamp so that "dnf upgrade" will pick up newer builds.  Unless you're always revving the version number in your project, which I assume you're not.

My continuous-goodies project has scripts to do that sort of junk.  You'll see.  And also, if you have improvements for any of that junk, please share.

Comment 44 Nick Black 2020-05-04 10:12:49 UTC
fedpkg request issued, https://pagure.io/releng/fedora-scm-requests/issue/24741

Comment 45 Nick Black 2020-05-04 13:41:39 UTC
Hrmm. @dcantrell, you're in the packager group, right? I thought you were (and https://admin.fedoraproject.org/accounts/user/view/dcantrell?_csrf_token=d323ae14bb4130fa5ae8456ee1aa5c6fa055d492 agrees, AFAICT), but I'm getting pushback: https://pagure.io/releng/fedora-scm-requests/issue/24741

Comment 46 David Cantrell 2020-05-04 13:50:10 UTC
Yes, I am.  I have been for some time.

Comment 47 Nick Black 2020-05-04 13:53:06 UTC
That's what I thought, but https://admin.fedoraproject.org/accounts/user/view/dcantrell indicates that you're at "User" level in Fedora Packager GIT Commit Group, and you need to be "Sponsor" there, right? If I'm misunderstanding, could you defend your sacred honor at https://pagure.io/releng/fedora-scm-requests/issue/24741?

Comment 48 Nick Black 2020-05-06 05:35:02 UTC
Just refreshing this bug -- @dcantrell still appears to be "User" rather than "Sponsor", but I understand that he's initiated the promo process.

Comment 49 David Cantrell 2020-05-06 16:46:34 UTC
The ticket is opened for my account to move to sponsor and per policy should be resolved by the end of the week.  If you want to follow that ticket:

https://pagure.io/packager-sponsors/issue/420

Comment 50 Gwyn Ciesla 2020-05-13 18:00:43 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/notcurses

Comment 51 Nick Black 2020-05-13 18:38:44 UTC
https://src.fedoraproject.org/rpms/notcurses/c/665f391c097e8106769f3caa46adee02e9b2b50f?branch=master

Checked in. Closing. Thanks everybody!

Comment 52 Miro Hrončok 2020-05-15 10:24:54 UTC
Nick, I see you have committed and pushed the source tarball to git.

We put tarballs to the "lookaside cache". See "fedpkg new-sources" in https://fedoraproject.org/wiki/Package_maintenance_guide#Common_fedpkg_commands

Comment 53 Miro Hrončok 2020-05-15 10:31:43 UTC
(In reply to David Cantrell from comment #9)
> This package fails to build as-is.  I made some changes to the spec file to
> get it to build:
> 
> --- notcurses-1.3.0-1.fc32.src/notcurses.spec   2020-04-12
> 01:26:33.000000000 -0400
> +++ rpmbuild/SPECS/notcurses.spec       2020-04-15 11:55:45.080414210 -0400
> @@ -13,7 +13,7 @@
>  BuildRequires: gcc-c++
>  BuildRequires: pandoc
>  BuildRequires: python3-devel
> -BuildRequires: python3-setuptools
> +BuildRequires: python3-cffi


David, I don't understand why you removed setuptools here. It is used and should be specified explicitly.

  notcurses-1.3.3/python/setup.py
  1:from setuptools import setup

The fact that setuptools is dragged in as a transitive dependency of other packages is not to be relied upon.

Comment 54 Miro Hrončok 2020-05-15 10:56:45 UTC
(BTW I've only noticed this by chance and wanted to make sure you know about it.)

Comment 55 David Cantrell 2020-05-15 14:02:47 UTC
(In reply to Miro Hrončok from comment #52)
> Nick, I see you have committed and pushed the source tarball to git.
> 
> We put tarballs to the "lookaside cache". See "fedpkg new-sources" in
> https://fedoraproject.org/wiki/
> Package_maintenance_guide#Common_fedpkg_commands

I noticed this too and pointed it out yesterday.  He said he was fixing it up.

Comment 56 David Cantrell 2020-05-15 14:05:25 UTC
(In reply to Miro Hrončok from comment #53)
> (In reply to David Cantrell from comment #9)
> > This package fails to build as-is.  I made some changes to the spec file to
> > get it to build:
> > 
> > --- notcurses-1.3.0-1.fc32.src/notcurses.spec   2020-04-12
> > 01:26:33.000000000 -0400
> > +++ rpmbuild/SPECS/notcurses.spec       2020-04-15 11:55:45.080414210 -0400
> > @@ -13,7 +13,7 @@
> >  BuildRequires: gcc-c++
> >  BuildRequires: pandoc
> >  BuildRequires: python3-devel
> > -BuildRequires: python3-setuptools
> > +BuildRequires: python3-cffi
> 
> 
> David, I don't understand why you removed setuptools here. It is used and
> should be specified explicitly.
> 
>   notcurses-1.3.3/python/setup.py
>   1:from setuptools import setup
> 
> The fact that setuptools is dragged in as a transitive dependency of other
> packages is not to be relied upon.

My default has been to always minimize the BuildRequires listings for easier maintenance.  Since he needed python3-cffi he could get two for one there.  It doesn't really matter to me whether it's listed explicitly or not, but I've always liked the shorter BuildRequires listings.

Comment 57 Miro Hrončok 2020-05-15 14:19:25 UTC
> My default has been to always minimize the BuildRequires listings for easier maintenance.  Since he needed python3-cffi he could get two for one there.  It doesn't really matter to me whether it's listed explicitly or not, but I've always liked the shorter BuildRequires listings.

I consider this approach error prone: in the future, cffi or Python might no longer need setuptools and the dependency will be removed, suddenly, this package will fail to build. Since this package uses setuptools directly (and not trough cffi), I think that listing the build dependency explicitly is the right thing to do.

In fact, we want to get rid of the current "python3-devel requires python3-setuptools" thing, but many packages don't list setuptools explicitly, that makes it hard for us.

Comment 58 David Cantrell 2020-05-15 14:34:41 UTC
I think this comes down to style.  What you're describing is just the nature of package maintenance.  No amount of futureproofing here is going to avoid having to continue to maintain the spec file.

But, I would say this is just a matter of style.  List it or don't.  The main objective here is that the build completes and is reproducible and the built packages comply with policy.  The maintainer will pick up ways that make that easier as time goes on and learn various tips as time goes on.

Comment 59 Miro Hrončok 2020-05-15 14:42:21 UTC
> No amount of futureproofing here is going to avoid having to continue to maintain the spec file.

Fair. I don't want to argue about this, I just assumed the dependency was removed in error.

Comment 60 David Cantrell 2020-05-15 14:52:13 UTC
(In reply to Miro Hrončok from comment #59)
> > No amount of futureproofing here is going to avoid having to continue to maintain the spec file.
> 
> Fair. I don't want to argue about this, I just assumed the dependency was
> removed in error.

Sorry, I'm not trying to argue.  Was not removed in error, just my standard practice of keeping BuildRequires lists short.

Regarding the hypothetical you described... I would actually rather see the builds fail that did not explicitly list python3-setuptools.  For several reasons.  First, it keeps package maintainers aware of overall dependency changes (i.e., people building Python packages should probably be generally aware of the Python packaging practices in effect).  Second, it keeps the project aware of what is actually still in use by users.  A bunch of build failures may cause some things to show up as just things we can safely remove.  Third, it presents package maintainers or other contributors with opportunities to go and clean things up in spec files from time to time as we continually revise and refine recommendations.

Comment 61 Miro Hrončok 2020-05-15 15:02:34 UTC
(In reply to David Cantrell from comment #60)
> Regarding the hypothetical you described... I would actually rather see the
> builds fail that did not explicitly list python3-setuptools.  For several
> reasons.  First, it keeps package maintainers aware of overall dependency
> changes (i.e., people building Python packages should probably be generally
> aware of the Python packaging practices in effect).  Second, it keeps the
> project aware of what is actually still in use by users.  A bunch of build
> failures may cause some things to show up as just things we can safely
> remove.  Third, it presents package maintainers or other contributors with
> opportunities to go and clean things up in spec files from time to time as
> we continually revise and refine recommendations.

That is very correct for situations where the missing dependency makes the package fail to build. In the setuptools case, we cannot do this, some Python projects have:

try:
    from setuptools import setup
except ImportError:
    from distutils.core import setup

And as a result, when setuptools is present, the build succeeds and .egg-info is created as a directory (that's what setuptools does). When setuptools is not present, the build still succeeds, but .egg-info is created as a text file (that's what distutils does). When users upgrade to the newly built package, RPM fails with https://docs.fedoraproject.org/en-US/packaging-guidelines/Directory_Replacement/

So we will actually need to go and add the setuptools requirement to all packages that use setuptools in this way. Might be easier to add it to all packages that use setuptools in any way -- including this one.

Comment 62 David Cantrell 2020-05-15 15:21:59 UTC
In that situation isn't the .egg-info not being a directory caught by this line in %files:

    %{python3_sitearch}/*egg-info/

The trailing slash tells RPM that it expects that glob to be a directory.  I just tried it out locally where I did:

    echo "testing file" > %{buildroot}%{python3_sitearch}/blurfle

And put this in %files:

    %{python3_sitearch}/blurfle/

An 'rpmbuild -ba' gave me:

    RPM build errors:
        Not a directory: /home/dcantrell/rpmbuild/BUILDROOT/notcurses-1.3.0-1.fc31.x86_64/usr/lib64/python3.7/site-packages/blurfle

With an exit code of 1.

We have both the recommendation for "BuildRequires: python3-setuptools" as well as the %files entry for the egg-info directory in the Fedora packaging guide.  Even without setuptools as a BR, the %files entry still guards the build.

Good point and worth noting for other language ecosystems that have package managers and module systems of their own.

Comment 63 Miro Hrončok 2020-05-15 15:48:25 UTC
> In that situation isn't the .egg-info not being a directory caught by this line in %files...

Yes it is. But not all packages use it :(

Comment 64 Nick Black 2020-05-17 04:57:29 UTC
I've removed the upstream tarball from the fedora repository, and pushed a new version to the lookaside cache (along with updating the specfile). I now have a populated `sources` file, which I have also committed (along with .gitignore). Sorry for the mistake! I'm about to kick off a koji build, and believe it will work this time without having the tarball in the repo.

Comment 65 Nick Black 2020-05-17 05:12:22 UTC
Regarding the BuildsRequires, I would personally look to the precedent of cpp #includes and ld linking. There, if <stdio.h> #includes <math.h> I'm still expected to #include <math.h> if I'm using a symbol on the API boundary exported by <math.h>, and I'm still expected to link any library where I'm using a symbol at the ABI boundary, even if I would get it transitively.

Thus my opinion is to go with the explicit dependency, if only because my UNIX forefathers did.

In that same vein, I wasn't going to make an issue of it in my first package review.

Comment 66 David Cantrell 2020-05-18 17:03:57 UTC
(In reply to Miro Hrončok from comment #63)
> > In that situation isn't the .egg-info not being a directory caught by this line in %files...
> 
> Yes it is. But not all packages use it :(

Fair.  We should expand fedora-review so it checks for this.  :)

I know fedora-review isn't law, but it's helpful to have something to go with.

Comment 67 David Cantrell 2020-05-18 17:05:31 UTC
(In reply to Nick Black from comment #64)
> I've removed the upstream tarball from the fedora repository, and pushed a
> new version to the lookaside cache (along with updating the specfile). I now
> have a populated `sources` file, which I have also committed (along with
> .gitignore). Sorry for the mistake! I'm about to kick off a koji build, and
> believe it will work this time without having the tarball in the repo.

The package repo looks good now.  Thanks for fixing that up.

Comment 68 David Cantrell 2020-05-18 17:09:13 UTC
(In reply to Nick Black from comment #65)
> Regarding the BuildsRequires, I would personally look to the precedent of
> cpp #includes and ld linking. There, if <stdio.h> #includes <math.h> I'm
> still expected to #include <math.h> if I'm using a symbol on the API
> boundary exported by <math.h>, and I'm still expected to link any library
> where I'm using a symbol at the ABI boundary, even if I would get it
> transitively.
> 
> Thus my opinion is to go with the explicit dependency, if only because my
> UNIX forefathers did.
> 
> In that same vein, I wasn't going to make an issue of it in my first package
> review.

Eiher way is now your choice.  Miro's points are entirely valid.  I stand by my tendency to keep the BuildRequires list as small as possible because I think about SRPMs less as standalone source files and more part of a single Borg hive where the only thing we do is topological sorting.

Comment 69 Nick Black 2020-05-18 23:42:43 UTC
I have made the change locally, but don't see any reason to cut a new release of yesterday's 1.4.2-1. Miro, thanks for your feedback. David, I might take a swing at the recommended fedora-review change, to show some appreciation for shepherding this package through.

Comment 70 Nick Black 2020-05-22 14:04:36 UTC
Just to close the loop on this, the python3-setuptools entry has been live since 2020-05-20's 1.4.2.3-1.

Comment 71 Miro Hrončok 2020-05-22 17:16:01 UTC
Thank you.


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