Bug 1978947

Taking this review

Looks mostly fine, apart from the two issues below.

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

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


Issues:
=======
- Upstream consistently calls this NsCDE, so that should probably win over
  the Fedora preference for lowercased names; see
    https://docs.fedoraproject.org/en-US/packaging-guidelines/Naming/#_case_sensitivity
- Large documentation must go in a -doc subpackage. Large could be size
  (~1MB) or number of files.
  Note: Documentation size is 1628160 bytes in 117 files.
  See: https://docs.fedoraproject.org/en-US/packaging-
  guidelines/#_documentation


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

C/C++:
[x]: Package does not contain kernel modules.
[x]: 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]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "Unknown or generated", "Expat License", "GNU General Public
     License, Version 2", "GNU General Public License v2.0 or later
     [obsolete FSF postal address (Temple Place)]", "GNU General Public
     License v2.0 or later", "*No copyright* [generated file]". 6193 files
     have unknown license. Detailed output of licensecheck in
     /home/michel/src/fedora/reviews/1978947-nscde/licensecheck.txt
[x]: License file installed when any subpackage combination is installed.
[x]: %build honors applicable compiler flags or justifies otherwise.
[!]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: The spec file handles locales properly.
[x]: Package consistently uses macros (instead of hard-coded directory
     names).
[!]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Useful -debuginfo package or justification otherwise.
[x]: Package is not known to require an ExcludeArch tag.
[x]: 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]: 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]: %config files are marked noreplace or the reason is justified.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Package contains desktop file if it is a GUI application.
[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]: No %config files under /usr.
[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]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

Perl:
[-]: Package contains the mandatory BuildRequires and Requires:.
     Note: Requires: perl(:MODULE_COMPAT_%(eval "`%{__perl} -V:version`";
     echo $version)) missing?

===== 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.
[x]: Final provides and requires are sane (see attachments).
[-]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in nscde-
     data
[?]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[x]: Patches link to upstream bugs/comments/lists or are otherwise
     justified.
[-]: Sources are verified with gpgverify first in %prep if upstream
     publishes signatures.
     Note: gpgverify is not used.
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[!]: %check is present and all tests pass.
[x]: 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]: Sources can be downloaded from URI in Source: tag
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

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

Generic:
[!]: Spec file according to URL is the same as in SRPM.
     Note: Spec file as given by url is not the same as in SRPM (see
     attached diff).
     See: (this test has no URL)
[!]: Large data in /usr/share should live in a noarch subpackage if package
     is arched.
     Note: Arch-ed rpms have a total of 1751040 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: nscde-1.2-1.fc35.x86_64.rpm
          nscde-data-1.2-1.fc35.noarch.rpm
          nscde-debuginfo-1.2-1.fc35.x86_64.rpm
          nscde-debugsource-1.2-1.fc35.x86_64.rpm
          nscde-1.2-1.fc35.src.rpm
nscde.x86_64: W: no-soname /usr/lib64/nscde/lib/XOverrideFontCursor.so
nscde.x86_64: W: dangling-relative-symlink /usr/lib64/nscde/share ../../../usr/share/nscde
nscde-data.noarch: W: no-documentation
nscde-data.noarch: W: dangling-relative-symlink /usr/share/nscde/doc ../../../usr/share/doc/nscde
5 packages and 0 specfiles checked; 0 errors, 4 warnings.




Rpmlint (debuginfo)
-------------------
Checking: nscde-debuginfo-1.2-1.fc35.x86_64.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.





Rpmlint (installed packages)
----------------------------
Cannot parse rpmlint output:


Unversioned so-files
--------------------
nscde: /usr/lib64/nscde/lib/XOverrideFontCursor.so

Source checksums
----------------
https://github.com/NsCDE/NsCDE/releases/download/1.2/NsCDE-1.2.tar.gz :
  CHECKSUM(SHA256) this package     : 6a26623816de11719eff98f85d519a0b8acff8628975c83289acc023c824b5c4
  CHECKSUM(SHA256) upstream package : 6a26623816de11719eff98f85d519a0b8acff8628975c83289acc023c824b5c4


Requires
--------
nscde (rpmlib, GLIBC filtered):
    /usr/bin/ksh93
    /usr/bin/python3
    ImageMagick
    config(nscde)
    cpp
    fvwm
    gettext
    glibc
    ksh
    libX11.so.6()(64bit)
    libXext.so.6()(64bit)
    libXpm.so.4()(64bit)
    libc.so.6()(64bit)
    libm.so.6()(64bit)
    nscde-data
    python3-psutil
    python3-qt5
    python3-yaml
    redhat-menus
    rtld(GNU_HASH)
    xdotool
    xdpyinfo
    xprop
    xrandr
    xrdb
    xrefresh
    xset
    xsettingsd

nscde-data (rpmlib, GLIBC filtered):
    /usr/bin/ksh93

nscde-debuginfo (rpmlib, GLIBC filtered):

nscde-debugsource (rpmlib, GLIBC filtered):



Provides
--------
nscde:
    bundled(XOverrideFontCursor)
    bundled(colorpicker)
    bundled(pclock)
    config(nscde)
    nscde
    nscde(x86-64)

nscde-data:
    nscde-data

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

nscde-debugsource:
    nscde-debugsource
    nscde-debugsource(x86-64)



Diff spec file in url and in SRPM
---------------------------------
--- /home/michel/src/fedora/reviews/1978947-nscde/srpm/nscde.spec	2021-07-03 20:15:53.191442932 -0700
+++ /home/michel/src/fedora/reviews/1978947-nscde/srpm-unpacked/nscde.spec	2021-07-03 08:34:05.000000000 -0700
@@ -7,6 +7,5 @@
 URL:            https://github.com/NsCDE/NsCDE
 Source0:        %{url}/releases/download/%{version}/NsCDE-%{version}.tar.gz
-# Pass build flags from the environment if available
-Patch0:         https://github.com/NsCDE/NsCDE/pull/61.patch
+Patch0:         nscde-cflags.patch
 
 # For the installer
@@ -66,5 +65,4 @@
 Recommends:     xscreensaver
 
-# These are an integral part of NsCDE and have been specifically modified
 Provides:       bundled(colorpicker) = 0
 Provides:       bundled(pclock) = 0.13.1
@@ -172,4 +170,4 @@
 
 %changelog
-* Sat Jul 03 2021 Davide Cavalca <dcavalca> - 1.2-1
+* Fri Jul 02 2021 Davide Cavalca <dcavalca> - 1.2-1
 - Initial package


Generated by fedora-review 0.7.6 (b083f91) last change: 2020-11-10
Command line :/usr/bin/fedora-review -b 1978947 -L /home/michel/Downloads/
Buildroot used: fedora-rawhide-x86_64
Active plugins: Generic, C/C++, Shell-api, Perl
Disabled plugins: R, Java, Ocaml, Python, Haskell, SugarActivity, PHP, fonts
Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH

Built with local dependencies:
    /home/michel/Downloads/xrefresh-1.0.6-3.fc35.x86_64.rpm
    /home/michel/Downloads/xsettingsd-1.0.2-1.fc35.x86_64.rpm

Spec URL: https://dcavalca.fedorapeople.org/review/NsCDE/NsCDE.spec
SRPM URL: https://dcavalca.fedorapeople.org/review/NsCDE/NsCDE-1.2-1.fc35.src.rpm

Addressed feedback.

LGTM, approved

Thanks!

$ fedpkg request-repo NsCDE 1978947
https://pagure.io/releng/fedora-scm-requests/issue/35468

(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/NsCDE

FEDORA-2021-a57670ae9a has been submitted as an update to Fedora 34. https://bodhi.fedoraproject.org/updates/FEDORA-2021-a57670ae9a

FEDORA-2021-a57670ae9a has been pushed to the Fedora 34 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf install --enablerepo=updates-testing --advisory=FEDORA-2021-a57670ae9a \*`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2021-a57670ae9a

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

FEDORA-2021-a57670ae9a has been pushed to the Fedora 34 stable repository.
If problem still persists, please make note of it in this bug report.