Bug 890772

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]: ldconfig called in %post and %postun if required.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.
[x]: Development (unversioned) .so files in -devel subpackage, if present.

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]: %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.
[x]: Each %files section contains %defattr if rpm < 4.4
     Note: %defattr present but not needed
[-]: Package contains desktop file if it is a GUI application.
[x]: Development files must be in a -devel package
[-]: Package requires other packages for directories it uses.
[x]: Package uses nothing in %doc for runtime.
[x]: Package is not known to require ExcludeArch.
[!]: Fully versioned dependency in subpackages, if present.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in tvlsim-doc
[x]: Package complies to the Packaging Guidelines
[!]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses found:
     "GPL (v2 or later) (with incorrect FSF address)", "BSL (v1.0)", "Unknown
     or generated". 158 files have unknown license. Detailed output of
     licensecheck in /home/felix/Downloads/890772-tvlsim/licensecheck.txt
[x]: License file installed when any subpackage combination is installed.
[x]: Package consistently uses macro is (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]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[-]: 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]: Large documentation must go in a -doc subpackage.
     Note: Documentation size is 40960 bytes in 5 files.
[x]: All build dependencies are listed in BuildRequires, except for any that
     are listed in the exceptions section of Packaging Guidelines.
[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 does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[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 %doc.
[x]: Package use %makeinstall only when make install' ' DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package do not use a name that already exist
[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
[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).

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

Generic:
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[!]: Buildroot is not present
     Note: Buildroot: present but not needed
[!]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
     Note: %clean present but not required
[-]: 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).
[!]: Package functions as described.
     Note: Segfaults on start
     terminate called after throwing an instance of 'stdair::SQLDatabaseConnectionImpossibleException'
       what():  Error while opening a connection to database: Can't connect to local MySQL server through socket '/var/lib/mysql/mysql.sock' (2)
     Database parameters used: db=sim_tvlsim user=tvlsim port=3306 host=localhost
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[x]: Scriptlets must be sane, if used.
[-]: 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.
     Note: http://koji.fedoraproject.org/koji/taskinfo?taskID=5722841
           looks like some packages need rebuilding against new boost in rawhide
[!]: %check is present and all tests pass.
     Note: Package ships tests, consider running them.
[x]: Packages should try to preserve timestamps of original installed files.
[x]: Sources can be downloaded from URI in Source: tag
[x]: Reviewer should test that the package builds in mock.
[x]: Dist tag is present.
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Uses parallel make.
[x]: The placement of pkgconfig(.pc) files are correct.
[x]: SourceX tarball generation or download is documented.
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define.

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

Generic:
[x]: Large data in /usr/share should live in a noarch subpackage if package is
     arched.
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: tvlsim-1.00.0-1.fc19.x86_64.rpm
          tvlsim-devel-1.00.0-1.fc19.x86_64.rpm
          tvlsim-doc-1.00.0-1.fc19.noarch.rpm
tvlsim.x86_64: W: spelling-error %description -l en_US traveller -> traveler, traveled, traversal
tvlsim.x86_64: W: spelling-error %description -l en_US modelling -> modeling, model ling, model-ling
3 packages and 0 specfiles checked; 0 errors, 2 warnings.




Rpmlint (installed packages)
----------------------------
# rpmlint tvlsim-doc tvlsim tvlsim-devel
tvlsim.x86_64: W: spelling-error %description -l en_US traveller -> traveler, traveled, traversal
tvlsim.x86_64: W: spelling-error %description -l en_US modelling -> modeling, model ling, model-ling
tvlsim.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libtvlsim.so.1.00.0 /lib64/libpython2.7.so.1.0
tvlsim.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libtvlsim.so.1.00.0 /lib64/libboost_regex-mt.so.1.53.0
tvlsim.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libtvlsim.so.1.00.0 /lib64/libboost_iostreams-mt.so.1.53.0
tvlsim.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libtvlsim.so.1.00.0 /lib64/libboost_serialization-mt.so.1.53.0
tvlsim.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libtvlsim.so.1.00.0 /lib64/libboost_filesystem-mt.so.1.53.0
tvlsim.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libtvlsim.so.1.00.0 /lib64/libboost_python-mt.so.1.53.0
tvlsim.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libtvlsim.so.1.00.0 /usr/lib64/mysql/libmysqlclient.so.18
tvlsim.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libtvlsim.so.1.00.0 /lib64/libsoci_mysql.so.3.2
tvlsim.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libtvlsim.so.1.00.0 /lib64/libzmq.so.1
tvlsim.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libtvlsim.so.1.00.0 /lib64/libreadline.so.6
tvlsim.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libtvlsim.so.1.00.0 /lib64/libncurses.so.5
tvlsim.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libtvlsim.so.1.00.0 /lib64/libtinfo.so.5
tvlsim.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libtvlsim.so.1.00.0 /lib64/libstdairuicl.so.1.00
tvlsim.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libtvlsim.so.1.00.0 /lib64/libairsched.so.1.00
tvlsim.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libtvlsim.so.1.00.0 /lib64/libairrac.so.1.00
tvlsim.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libtvlsim.so.1.00.0 /lib64/librmol.so.1.00
tvlsim.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libtvlsim.so.1.00.0 /lib64/libairinv.so.1.00
tvlsim.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libtvlsim.so.1.00.0 /lib64/libsimfqt.so.1.00
3 packages and 0 specfiles checked; 0 errors, 20 warnings.
# echo 'rpmlint-done:'



Requires
--------
tvlsim-doc (rpmlib, GLIBC filtered):

tvlsim (rpmlib, GLIBC filtered):
    /sbin/ldconfig
    libairinv.so.1.00()(64bit)
    libairrac.so.1.00()(64bit)
    libairsched.so.1.00()(64bit)
    libboost_date_time-mt.so.1.53.0()(64bit)
    libboost_filesystem-mt.so.1.53.0()(64bit)
    libboost_iostreams-mt.so.1.53.0()(64bit)
    libboost_program_options-mt.so.1.53.0()(64bit)
    libboost_python-mt.so.1.53.0()(64bit)
    libboost_regex-mt.so.1.53.0()(64bit)
    libboost_serialization-mt.so.1.53.0()(64bit)
    libc.so.6()(64bit)
    libgcc_s.so.1()(64bit)
    libgcc_s.so.1(GCC_3.0)(64bit)
    libm.so.6()(64bit)
    libmysqlclient.so.18()(64bit)
    libncurses.so.5()(64bit)
    libpthread.so.0()(64bit)
    libpython2.7.so.1.0()(64bit)
    libreadline.so.6()(64bit)
    librmol.so.1.00()(64bit)
    libsevmgr.so.1.00()(64bit)
    libsimcrs.so.1.00()(64bit)
    libsimfqt.so.1.00()(64bit)
    libsoci_core.so.3.2()(64bit)
    libsoci_mysql.so.3.2()(64bit)
    libstdair.so.1.00()(64bit)
    libstdairuicl.so.1.00()(64bit)
    libstdc++.so.6()(64bit)
    libstdc++.so.6(CXXABI_1.3)(64bit)
    libstdc++.so.6(CXXABI_1.3.1)(64bit)
    libtinfo.so.5()(64bit)
    libtrademgen.so.1.00()(64bit)
    libtravelccm.so.1.00()(64bit)
    libtvlsim.so.1.00()(64bit)
    libzmq.so.1()(64bit)
    rtld(GNU_HASH)

tvlsim-devel (rpmlib, GLIBC filtered):
    /bin/sh
    /usr/bin/pkg-config
    libtvlsim.so.1.00()(64bit)
    pkgconfig
    tvlsim(x86-64)



Provides
--------
tvlsim-doc:
    tvlsim-doc

tvlsim:
    libtvlsim.so.1.00()(64bit)
    tvlsim
    tvlsim(x86-64)

tvlsim-devel:
    pkgconfig(tvlsim)
    tvlsim-devel
    tvlsim-devel(x86-64)



Source checksums
----------------
http://downloads.sourceforge.net/tvlsim/tvlsim-1.00.0.tar.bz2 :
  CHECKSUM(SHA256) this package     : e2eec481359c2ce8780e27f8ba4a8ea5af31db3245c1138a49057b7e57ae42ec
  CHECKSUM(SHA256) upstream package : e2eec481359c2ce8780e27f8ba4a8ea5af31db3245c1138a49057b7e57ae42ec


Generated by fedora-review 0.4.1 (b2e211f) last change: 2013-04-29
Buildroot used: fedora-19-x86_64
Command line :/usr/bin/fedora-review -b 890772

Many thanks for that review!

Following are the updated RPM specification file and source RPM:
------
Spec URL: http://denisarnaud.fedorapeople.org/sim/tvlsim/tvlsim.spec
SRPM URL: http://denisarnaud.fedorapeople.org/sim/tvlsim/tvlsim-1.00.0-2.fc19.src.rpm
=====


Specific answers to the issues spotted by the review
----------------------------------------------------
1. Documentation sub-package
> [!]: Fully versioned dependency in subpackages, if present.
>      Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in tvlsim-doc

Note that the absence of "Requires:" was intentional, as the documentation (-doc) sub-package may be installed independently of the main package. Hence, someome may first install the -doc sub-package, check that it corresponds to his/her expectations, and only then installing the main package if deemed necessary.
=> It should be a [x]


2. License
> [!]: License field in the package spec file matches the actual license.
>      Note: Checking patched sources after %prep for licenses. Licenses found:
>      "GPL (v2 or later) (with incorrect FSF address)", "BSL (v1.0)", "Unknown
>      or generated". 158 files have unknown license. Detailed output of
>      licensecheck in /home/felix/Downloads/890772-tvlsim/licensecheck.txt

The license of TvlSim is LGPL v2.1, as stated by the COPYING file on the GitHub repository: http://github.com/airsim/tvlsim/blob/trunk/COPYING
That file is the verbatim of the LGPL v2.1 text: http://www.gnu.org/licenses/lgpl-2.1.html#SEC1
Since the source files do not mention any license, the license of the whole project applies for each of the source files.
=> It should be a [x]


3. BuildRoot
> [!]: Buildroot is not present
>      Note: Buildroot: present but not needed
> [!]: Package has no %clean section with rm -rf %{buildroot} (or
>      $RPM_BUILD_ROOT)
>      Note: %clean present but not required

BuildRoot is present because I intend to package TvlSim for EPEL (5 and 6).


4. Segfault on start
> [!]: Package functions as described.
>      Note: Segfaults on start
>      terminate called after throwing an instance of 'stdair::SQLDatabaseConnectionImpossibleException'
>        what():  Error while opening a connection to database: Can't connect to local MySQL server through socket '/var/lib/mysql/mysql.sock' (2)
>      Database parameters used: db=sim_tvlsim user=tvlsim port=3306 host=localhost

4.1. 'tvlsim' executable
That executable works out of the box, without any further configuration. For instance:
$ tvlsim
tvlsim > run
tvlsim > quit
$ less tvlsim.log 

4.2. 'simulate' executable
That executable does not work out of the box. It needs a MySQL database server. Then, the StdAir package contains helper scripts to create a database user, namely 'tvlsim', as well as a database, namely 'sim_tvlsim'. Some data need to be loaded into a specific table, namely 'airlines'. Following is the sequence of commands allowing to initialise the MySQL database correctly:
$ /usr/libexec/stdair/create_tvlsim_user.sh # Then, enter the credentials of an admin MySQL account. Launch with the '-h' option to see how to change the hostname and/or port.
$ /usr/libexec/stdair/create_tvlsim_db.sh # Then, enter the credentials of an admin MySQL account. Launch with the '-h' option to see how to change the hostname and/or port.
$ /usr/libexec/stdair/load_tvlsim_data.sh # Launch with the '-h' option to see how to change the hostname and/or port.
$ simulate
$ less simulate.log

4.3. 'TvlSimServer' executable
Same as for the 'simulate' executable.
A command-line (test) client exists in Python (https://github.com/airsim/tvlsim/blob/trunk/appserver/django/tvlsimClient.py), but it is not installed by the upstream installation scripts.
$ TvlSimServer &
$ wget https://raw.github.com/airsim/tvlsim/trunk/appserver/django/tvlsimClient.py
$ python tvlsimClient.py
$ kill %1
$ less tvlsimServer.log
Note that a Django-based Web application server exists, but it has not been packaged yet.

=> It should be a [x]


5. Successful build on Koji for Rawhide
> [!]: Package should compile and build into binary rpms on all supported
     architectures.
>      Note: http://koji.fedoraproject.org/koji/taskinfo?taskID=5722841
           looks like some packages need rebuilding against new boost in rawhide

The mass rebuild (https://fedoraproject.org/wiki/Fedora_20_Mass_Rebuild) is still on-going. Almost all the components of the simulator (TvlSim) have been successfully rebuilt on the 'f20-rebuild' side tag (for instance, AirInv: http://koji.fedoraproject.org/koji/taskinfo?taskID=5696380), and are now relying on Boost-1.54. The build of tvlsim on the 'f20-rebuild' side tag (and, soon, on Rawhide) should therefore be successful, as it is on Fedora 18 and 19.

=> It should be a [x]


6. %check
> [!]: %check is present and all tests pass.
>      Note: Package ships tests, consider running them.

Due to the MySQL configuration dependency, some tests fail when the MySQL has not been properly configured (which is the case in the chroot environment set up by mock). Hence, the tests are de-activated.

Sorry for the delay.

Your explanations of the various issues are reasonable.
I rebuilt the package on koji to confirm it now builds correctly: http://koji.fedoraproject.org/koji/taskinfo?taskID=5854028

Some hints which are probably nice to have but not mandatory:
Try contacting upstream to add header information to the source files (i.e. add something like "this source file belongs to the tvlsim project and is licensed under GPLv2+".
Also segfaulting, in my opinion, is not a nice way to tell the user to set up MySQL correctly ;) Maybe upstream should add some better error handling here? 

One thing I noticed only now is that your spec file uses British English spelling (see rpmlint warnings). It doesn't bother me, however the Review Guidelines have a MUST on American English spelling. See https://fedoraproject.org/wiki/Packaging:Guidelines#summary

If you fix the spelling I will approve the package.

Many thanks for that review!

Following are the updated RPM specification file and source RPM:
------
Spec URL: http://denisarnaud.fedorapeople.org/sim/tvlsim/tvlsim.spec
SRPM URL: http://denisarnaud.fedorapeople.org/sim/tvlsim/tvlsim-1.00.0-3.fc19.src.rpm
=====

(In reply to Felix Kaechele from comment #3)
> One thing I noticed only now is that your spec file uses British English
> spelling (see rpmlint warnings). It doesn't bother me, however the Review
> Guidelines have a MUST on American English spelling. See
> https://fedoraproject.org/wiki/Packaging:Guidelines#summary
> 
> If you fix the spelling I will approve the package.

Note that I had already corrected the spelling in the second version of the specification file (http://denisarnaud.fedorapeople.org/sim/tvlsim/tvlsim-1.00.0-2.spec). However, cross-reading all the texts carefully, I noticed some typos, which I fixed, hence the newer version above.

As for your other comments (e.g., regarding the license), I will tell upstream for sure (as I am upstream!) :)

I was thinking about the rpmlint warning regarding unused direct library dependencies.
Those can be fixed by passing "-Wl,--as-needed" to the LDFLAGS from cmake, I guess.
Maybe this is something you want to consider? I haven't checked if this breaks anything (as the output actually looks like false positives).

Still interested?

Yes, for sure, I am still very much interested in having that package finding its way to Fedora/CentOS/RedHat, as it is the main entry point for the whole Travel Market Simulator project (http://www.travel-market-simulator.com). Without that last package, the project is a lot less interesting...
Thanks in advance!
Kind regards
Denis

(In reply to Felix Kaechele from comment #5)
> I was thinking about the rpmlint warning regarding unused direct library
> dependencies.
> Those can be fixed by passing "-Wl,--as-needed" to the LDFLAGS from cmake, I
> guess.
> Maybe this is something you want to consider? I haven't checked if this
> breaks anything (as the output actually looks like false positives).

In cmake world it's pretty common:

http://www.cmake.org/Wiki/CMake_FAQ#Why_are_libraries_linked_to_my_shared_library_included_when_something_links_to_it.3F

Spec URL: http://denisarnaud.fedorapeople.org/sim/tvlsim/tvlsim.spec
SRPM URL: http://denisarnaud.fedorapeople.org/sim/tvlsim/tvlsim-1.00.0-4.fc20.src.rpm
-----------------
Thanks Christopher for your suggestion!

Well, I (as upstream) wanted to give that LDFLAGS configuration a try... but the head/trunk of source code now relies on AirTSP, rather than on AirSched; so, http://bugzilla.redhat.com/show_bug.cgi?id=972431 is in the way.

Another option, in the meantime, would be to alter the LDFLAGS directly from the tvlsim.spec file. But it is safer to do it upstream; an issue has been created upstream: http://github.com/airsim/tvlsim/issues/1.

----------
Successful builds:
Fedora 22: http://koji.fedoraproject.org/koji/taskinfo?taskID=9508938
Fedora 21: http://koji.fedoraproject.org/koji/taskinfo?taskID=9508865
------------

So, could you approve it?

------
Spec URL: http://denisarnaud.fedorapeople.org/sim/tvlsim/tvlsim.spec
SRPM URL: http://denisarnaud.fedorapeople.org/sim/tvlsim/tvlsim-1.01.1-1.fc30.src.rpm
------
Successful build on Rawhide (F30): https://koji.fedoraproject.org/koji/taskinfo?taskID=32099763
------

* The RPM specification file has been fully cleaned according to the latest standards, as of January 2019 (see for instance https://bugzilla.redhat.com/show_bug.cgi?id=1630837).
* rpmlint does not report any error or warning.

This is an automatic check from review-stats script.

This review request ticket hasn't been updated for some time, but it seems
that the review is still being working out by you. If this is right, please
respond to this comment clearing the NEEDINFO flag and try to reach out the
submitter to proceed with the review.

If you're not interested in reviewing this ticket anymore, please clear the
fedora-review flag and reset the assignee, so that a new reviewer can take
this ticket.

Without any reply, this request will shortly be resetted.

Yes, I'm still very much interested in that review to be completed (normally, it should now have become a no-brainer, as the latest iteration cleared the concerns; see #10).

This is an automatic check from review-stats script.

This review request ticket hasn't been updated for some time, but it seems
that the review is still being working out by you. If this is right, please
respond to this comment clearing the NEEDINFO flag and try to reach out the
submitter to proceed with the review.

If you're not interested in reviewing this ticket anymore, please clear the
fedora-review flag and reset the assignee, so that a new reviewer can take
this ticket.

Without any reply, this request will shortly be resetted.

(In reply to Package Review from comment #13)
> This is an automatic check from review-stats script.
> 
> This review request ticket hasn't been updated for some time, but it seems
> that the review is still being working out by you. If this is right, please
> respond to this comment clearing the NEEDINFO flag and try to reach out the
> submitter to proceed with the review.
> 
> If you're not interested in reviewing this ticket anymore, please clear the
> fedora-review flag and reset the assignee, so that a new reviewer can take
> this ticket.
> 
> Without any reply, this request will shortly be resetted.

Yes, I'm still very much interested in that review to be completed (normally, it should now have become a no-brainer, as the latest iteration cleared the concerns; see #10).

This is an automatic check from review-stats script.

This review request ticket hasn't been updated for some time, but it seems
that the review is still being working out by you. If this is right, please
respond to this comment clearing the NEEDINFO flag and try to reach out the
submitter to proceed with the review.

If you're not interested in reviewing this ticket anymore, please clear the
fedora-review flag and reset the assignee, so that a new reviewer can take
this ticket.

Without any reply, this request will shortly be resetted.

This is embarrasing. I am sorry.

I think I'm ready to finish this now ;-)
It's only been 10 years in the making.

(In reply to Felix Kaechele from comment #18)
> I think I'm ready to finish this now ;-)

Thanks!

A few notes before we can continue.

The package as linked here previously currently does not build. It looks like the cmake macros need updating (e.g. %cmake instead of %cmake .; %cmake_build and %cmake_install instead of %make_build and %make_install): https://docs.fedoraproject.org/en-US/packaging-guidelines/CMake/.

Other observations, going through the spec file top to bottom:

1. L5: Please update to the latest upstream version.
2. L6: Consider using %autorelease for the Release tag: https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/#_release_tag
3. L14-37: Consider expressing build dependencies in pkgconfig(libname) format if the CMake backend uses pkg-config to find the library: https://docs.fedoraproject.org/en-US/packaging-guidelines/PkgConfigBuildRequires/
4. L30: Using file based BuildRequires is slow (due to needing to download and traverse the file list metadata). The package that provides /usr/bin/epstopdf also has an extra Provides: texlive-epstopdf which could be used instead.
5. L40-L69, L88-92: I recommend using https for URLs listed in the description, where applicable
6. L96: the -n parameter could be removed from the %autosetup macro if you used a different Source0 URL (using the GitHub anchor URL trick): Source0:        %{url}/archive/refs/tags/%{version}.tar.gz#/%{name}-%{version}.tar.gz
7. L114 %check section can be removed if not used/not working.
8. L120,L141: COPYING file should be in %license not %doc
9. L122: %{_bindir}/simulate is a very generic binary name and may lead to future conflicts (see https://docs.fedoraproject.org/en-US/packaging-guidelines/Conflicts/#_potential_conflicting_files). I couldn't find any other packages or projects online using this path, so this is just an FYI.
10. L146: Consider using %autochangelog (https://docs.pagure.org/fedora-infra.rpmautospec/autochangelog.html)

After these things are addressed I will give it a run with fedora-review (feel free to run it against the package yourself as I will be going through all warnings and errors it produces in the review).

Many thanks, Felix, for that excellent piece of work!

Yes, you are absolutely right, it is time to upgrade that package to the latest upstream version and to the latest standard in packaging. Many thanks for all those suggestions, which I will integrate whenever possible (probably in the next few weeks, as I'm currently quite busy at work).

And you are right about the binary name, simulate is too generic. I guess something like %{name}-simulate will be better.