Bug 1844120

1. Remove Group tag.
   https://docs.fedoraproject.org/en-US/packaging-guidelines/#_tags_and_sections

2. Change
%setup -qn jamulus-%{commit0}
to
%autosetup -n jamulus-%{commit0}

3. Don't use macro for "install" command
   https://docs.fedoraproject.org/en-US/packaging-guidelines/#_macros

4. Correct desktop file install
   https://docs.fedoraproject.org/en-US/packaging-guidelines/#_desktop_file_install_usage

5. Remove %post, %postun and %posttrans sections. Scriptlets for desktop file and mime not needed now.


After that I will see at build flags and why you change them.

6. Why do you use commit if release available by direct URL?
https://github.com/corrados/jamulus/archive/r3_5_5.tar.gz

7. Change Summary: to something more usefull without repeating name.

8. Line length in description must be 80 symbols maximum. Correct it.

9. I think Obsoletes: Jamulus <= 3.5.3-3 should be removed because wi not contain it in repos before.

10. Add license and doc files to %files section.

Hello,

Thanks for the review.
I totally forgot to replace %setup by %autosetup, sorry.
I was focused on the fix wrt the debug package generation.

I removed the Group tag and I switch from %__install to install.

I corrected the desktop-file-install command.
I also removed the %post and other sections. I've got this sections by digging years ago in https://src.fedoraproject.org/. But I didn't notice that the usage changed.

I changed the Source0 line to download directly the release from the github page.

I changed the summary and descriptions. I also checked the length of lines so as to respect the 80 symbols limit.

The Obsolete line is removed.

And I added the files in %doc and %license (I totally forgot to add these file).

I also added a check on fedora version.
For Fedora 32, I use all the security flags.
For Fedora 31 and below, I remove the -fcf-protection flag because I still use this spec file to build jamulus for Fedora 30 and 31.
My main machine is still on Fedora 30.

The link to the src rpm file:
https://download.copr.fedorainfracloud.org/results/ycollet/linuxmao/fedora-32-x86_64/01429101-jamulus/jamulus-3.5.5-5.fc32.src.rpm

The link to the spec file:
https://raw.githubusercontent.com/ycollet/fedora-spec/master/jamulus/jamulus.spec

Remove INSTALL.md from %files. Install instructions from upstream should not be in package.

Check real license of source files. It may be need to describe what part of code what license use.

Created attachment 1695901 [details]
License check

I think you should remove opus directory from sources in %prep section to be sure that it will not bundled.

I removed INSTALL.md from doc files.
I removed the opus code too (with a comment in the spec file).

What script do you use to check the licence of the various files in the archive ?

The link to the src rpm file:
https://download.copr.fedorainfracloud.org/results/ycollet/linuxmao/fedora-32-x86_64/01431054-jamulus/jamulus-3.5.5-6.fc32.src.rpm

The link to the updated spec file:
https://raw.githubusercontent.com/ycollet/fedora-spec/master/jamulus/jamulus.spec

I filed an issue upstream related to the licenses:
https://github.com/corrados/jamulus/issues/341

I use fedora-review.

The command line I tested:

[SRPMS] $ fedora-review --verbose --mock-config fedora-32-x86_64 --rpm-spec --name jamulus

Do you use some other specific flags ?

fedora-review -b 1844120

All review must be on rawhide.

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

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



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

C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[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", "GNU Lesser General Public License",
     "GPL (v2 or later) (with incorrect FSF address)", "BSD 3-clause "New"
     or "Revised" License GPL (v2 or later) (with incorrect FSF address)",
     "GPL (v2 or later)", "*No copyright* GPL (v2 or later) (with incorrect
     FSF address)". 435 files have unknown license. Detailed output of
     licensecheck in /home/vascom/1844120-jamulus/licensecheck.txt
[x]: License file installed when any subpackage combination is installed.
[x]: %build honors applicable compiler flags or justifies otherwise.
[x]: 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]: Package consistently uses macros (instead of hard-coded directory
     names).
[x]: 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]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 30720 bytes in 2 files.
[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]: 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]: Package contains desktop file if it is a GUI application.
[x]: Package installs a %{name}.desktop using desktop-file-install or
     desktop-file-validate if there is such a file.
[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]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

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

Generic:
[x]: 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).
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[-]: Sources are verified with gpgverify first in %prep if upstream
     publishes signatures.
     Note: gpgverify is not used.
[x]: 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]: Fully versioned dependency in subpackages if applicable.
[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)
[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: jamulus-3.5.5-6.fc33.x86_64.rpm
          jamulus-debuginfo-3.5.5-6.fc33.x86_64.rpm
          jamulus-debugsource-3.5.5-6.fc33.x86_64.rpm
          jamulus-3.5.5-6.fc33.src.rpm
jamulus.x86_64: E: incorrect-fsf-address /usr/share/licenses/jamulus/COPYING
jamulus.x86_64: W: no-manual-page-for-binary jamulus
4 packages and 0 specfiles checked; 1 errors, 1 warnings.




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





Rpmlint (installed packages)
----------------------------
warning: Found bdb Packages database while attempting sqlite backend: using bdb backend.
warning: Found bdb Packages database while attempting sqlite backend: using bdb backend.
jamulus-debuginfo.x86_64: W: invalid-url URL: https://github.com/corrados/jamulus/ <urlopen error [Errno -2] Name or service not known>
warning: Found bdb Packages database while attempting sqlite backend: using bdb backend.
jamulus.x86_64: W: invalid-url URL: https://github.com/corrados/jamulus/ <urlopen error [Errno -2] Name or service not known>
jamulus.x86_64: E: incorrect-fsf-address /usr/share/licenses/jamulus/COPYING
jamulus.x86_64: W: no-manual-page-for-binary jamulus
warning: Found bdb Packages database while attempting sqlite backend: using bdb backend.
jamulus-debugsource.x86_64: W: invalid-url URL: https://github.com/corrados/jamulus/ <urlopen error [Errno -2] Name or service not known>
3 packages and 0 specfiles checked; 1 errors, 4 warnings.



Source checksums
----------------
https://github.com/corrados/jamulus/archive/r3_5_5.tar.gz#/jamulus-3.5.5.tar.gz :
  CHECKSUM(SHA256) this package     : 666e90f296d48b24dab47ac72f1d6f09964e1396f5cf625df967d13f74a6b164
  CHECKSUM(SHA256) upstream package : 666e90f296d48b24dab47ac72f1d6f09964e1396f5cf625df967d13f74a6b164


Requires
--------
jamulus (rpmlib, GLIBC filtered):
    libQt5Core.so.5()(64bit)
    libQt5Core.so.5(Qt_5)(64bit)
    libQt5Core.so.5(Qt_5.14)(64bit)
    libQt5Gui.so.5()(64bit)
    libQt5Gui.so.5(Qt_5)(64bit)
    libQt5Network.so.5()(64bit)
    libQt5Network.so.5(Qt_5)(64bit)
    libQt5Widgets.so.5()(64bit)
    libQt5Widgets.so.5(Qt_5)(64bit)
    libQt5Xml.so.5()(64bit)
    libQt5Xml.so.5(Qt_5)(64bit)
    libc.so.6()(64bit)
    libgcc_s.so.1()(64bit)
    libgcc_s.so.1(GCC_3.0)(64bit)
    libjack.so.0()(64bit)
    libm.so.6()(64bit)
    libopus.so.0()(64bit)
    libpthread.so.0()(64bit)
    libstdc++.so.6()(64bit)
    libstdc++.so.6(CXXABI_1.3)(64bit)
    libstdc++.so.6(CXXABI_1.3.1)(64bit)
    rtld(GNU_HASH)

jamulus-debuginfo (rpmlib, GLIBC filtered):

jamulus-debugsource (rpmlib, GLIBC filtered):



Provides
--------
jamulus:
    application()
    application(jamulus.desktop)
    jamulus
    jamulus(x86-64)

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

jamulus-debugsource:
    jamulus-debugsource
    jamulus-debugsource(x86-64)



Diff spec file in url and in SRPM
---------------------------------
--- /home/vascom/1844120-jamulus/srpm/jamulus.spec	2020-06-09 09:17:31.458179180 +0300
+++ /home/vascom/1844120-jamulus/srpm-unpacked/jamulus.spec	2020-06-08 11:11:40.000000000 +0300
@@ -20,5 +20,5 @@
 
 %description
-jamulus is a client / server software which allow to perform
+Jamulus is a client / server software which allow to perform
 real-time rehearsal over the internet. It uses Jack Audio Connection Kit
 and Opus audio codec to manage the audio session. 
@@ -32,11 +32,6 @@
 %build
 
-pushd .
-cd src/res/translation
-lrelease-qt5 *.ts
-popd
-
 %if 0%{?fedora} >= 32
-  %qmake_qt5 Jamulus.pro CONFIG+="noupcasename opus_shared_lib"
+  %qmake_qt5 Jamulus.pro CONFIG+=opus_shared_lib
 %else
   # -fcf-protection produce an error in an object generatoin ...
@@ -46,5 +41,5 @@
             QMAKE_CXXFLAGS_DEBUG="%{__global_compiler_flags} -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection" \
             QMAKE_CXXFLAGS_RELEASE="%{__global_compiler_flags} -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection" \
-            CONFIG+="noupcasename opus_shared_lib" 
+            CONFIG+=opus_shared_lib 
 %endif
 
@@ -54,11 +49,11 @@
 
 install -m 755 -d %{buildroot}/%{_bindir}/
-install -m 755 %{name} %{buildroot}%{_bindir}/%{name}
+install -m 755 Jamulus %{buildroot}%{_bindir}/jamulus
 
 install -m 755 -d %{buildroot}/%{_datadir}/applications/
-install -m 644 distributions/%{name}.desktop %{buildroot}%{_datadir}/applications/
+install -m 644 distributions/jamulus.desktop %{buildroot}%{_datadir}/applications/
 
 install -m 755 -d %{buildroot}/%{_datadir}/pixmaps/
-install -m 644 distributions/%{name}.png %{buildroot}%{_datadir}/pixmaps/
+install -m 644 distributions/jamulus.png %{buildroot}%{_datadir}/pixmaps/
 
 desktop-file-install                         \
@@ -66,5 +61,5 @@
   --delete-original                          \
   --dir=%{buildroot}%{_datadir}/applications \
-  %{buildroot}/%{_datadir}/applications/%{name}.desktop
+  %{buildroot}/%{_datadir}/applications/jamulus.desktop
 
 %files


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

Last thing you need to use "install -p" key to preserve timestamps.
You must change it but no need to rebuild at that moment.

Thanks for the install advice. I fixed the spec file.
I also updated to jamulus-3.5.6. The developpers have fixed most of the licences problem.
The link to the src rpm file: https://download.copr.fedorainfracloud.org/results/ycollet/linuxmao/fedora-32-x86_64/01434101-jamulus/jamulus-3.5.6-6.fc32.src.rpm
The link to ths spec file: https://raw.githubusercontent.com/ycollet/fedora-spec/master/jamulus/jamulus.spec

Hi, is this review alive? I was playing around with Jamulus and then I saw this review request. Here's my attempt:

Spec: https://iucar.fedorapeople.org/pkgs/jamulus.spec
SRPM: https://iucar.fedorapeople.org/pkgs/jamulus-3.6.0-1.fc32.src.rpm

Please, feel free to take any piece of that for your spec. I can help reviewing too if needed.

Thanks a lot for this spec file.

I am still waiting for some more news related to this review.
I am not an official packager but I am willing to become an official packager ... So, I wait ...

Ah, ok, you need a sponsor. I see now that you have several review requests and you are working with Vasily to get sponsored. Great.

Some comments about my spec with respect to yours. Note that:

- I package the service file that comes under 'distributions', so that the program can be launched as a server by systemd.
- I add the /var/log/jamulus folder for the log files that this service generates, including a logrotate file.
- I use the %qmake_qt5 macro, which automatically sets up the required flags.
- I use INSTALL_ROOT to be able to call %make_install without manually installing everything.
- I generate a manpage with help2man.
- I create some more icon sizes.

Hope it helps.

I forgot to add systemd file in my spec file because, when I use jamulus, I start the server manually to set the directory path for jamulus to save audio files for the session.

I also noticed that you add a %check section to verify the validityof the desktop file.

I have converted some of my spec files to %qmake_qt5 macro too, but I missed this one :)

And I noticed the use of help2man to generated a stub man page and convert to produced icons for missing resolutions.

Thanks for this spec file :)

Package review opened more than 5 months ago.

Nobody seems to be interested ...
So, I close the package review.
Somebody else will had back this if somebody at fedora is interested.

I am interested. I didn't review this because I thought @vascom was working on it. I can take it, but I cannot help you with the sponsoring process, because I'm not a sponsor.

Don't waste your time on this.
Personnaly, I give up ..

(In reply to Iñaki Ucar from comment #22)
> I am interested. I didn't review this because I thought @vascom was working
> on it. I can take it, but I cannot help you with the sponsoring process,
> because I'm not a sponsor.

As you can see review completed. But I can't sponsor ycollet because I see that he not ready to be maintainer. Every new Review Request contain spec with significant errors.