[Bug 1953254] New: Review Request: SLiMgui - an evolutionary simulation framework

https://bugzilla.redhat.com/show_bug.cgi?id=1953254

Bug ID: 1953254
           Summary: Review Request: SLiMgui - an evolutionary simulation
                    framework
           Product: Fedora
           Version: rawhide
          Hardware: All
                OS: Linux
            Status: NEW
         Component: Package Review
          Severity: medium
          Assignee: [e-mail]
          Reporter: [e-mail]
        QA Contact: [e-mail]
                CC: [e-mail]
  Target Milestone: ---
    Classification: Fedora

Spec URL:
https://download.copr.fedorainfracloud.org/results/bacarson/SLiM-Selection_o...
SRPM URL:
https://download.copr.fedorainfracloud.org/results/bacarson/SLiM-Selection_o...
Description: SLiM is an evolutionary simulation framework that combines a
powerful engine for population genetic simulations with the capability of
modeling arbitrarily complex evolutionary scenarios. Simulations are configured
via the integrated Eidos scripting language that allows interactive control
over practically every aspect of the simulated evolutionary scenarios. The
underlying individual-based simulation engine is highly optimized to enable
modeling of entire chromosomes in large populations. We also provide a
graphical user interface on macOS and Linux for easy simulation set-up,
interactive runtime control, and dynamical visualization of simulation output.
Fedora Account System Username: bacarson

This is my first package and I need a sponsor. I have maintained this package
for several months, for EPEL, CentOS, Fedora, and openSUSE, on Copr as `SLiM`.
I also maintain a build and installation script for Debian & Ubuntu, while I am
in the process of creating a native Debian package.

The package name "SLiM", which is used by users from my Copr, conflicts with
Simple Login Manager (which is packaged as "slim"), as does a binary file name
(`/usr/bin/slim`). This package, `SLiMgui`, addresses those conflicts and
prepares the SLiMgui package for submission to the official Fedora repository,
hence this review request.

The conflicting binary file name has been renamed, temporarily, to `SLiMcli`.
This conflict has been explained to the upstream developer here:
https://github.com/MesserLab/SLiM/issues/175. Until Ben Haller, the software
developer, provides feedback on his preference (to rename the binary or not),
or we hear from https://github.com/iwamatsu, or https://github.com/Hubbitus,
this rename is temporary and I consider it blocking.

The new package, with all necessary changes (and the temporarily renamed
binary), has been uploaded as a new build to my Copr here:
https://copr.fedorainfracloud.org/coprs/bacarson/SLiM-Selection_on_Linked_Mu....

[Bug 1953254] Review Request: SLiMgui - an evolutionary simulation framework

https://bugzilla.redhat.com/show_bug.cgi?id=1953254

--- Comment #1 from [e-mail] ---
The new build, SLiMgui, failed on openSUSE because of this RPM macro:
%{_metainfodir}. I will switch back to using `/usr/share/metainfo` as that
directory is the same across all distributions I package for. If this is a
blocker, which I doubt, then I'll need to make installation of the metainfo
file conditional on the distribution and determine what macro is usable on
openSUSE, or only use a hardcoded directory for openSUSE.

In my self-introduction to the devel mailing list, I stated that I had not
submitted the package to Bugzilla yet. This is now false. :)

I will update the upstream developer, informing them that I have uploaded the
package to Copr as a test build (and end-users will be unawares) and that I
have made a review request here. :)

Regards,

Bryce Carson.

[Bug 1953254] Review Request: SLiMgui - an evolutionary simulation framework

https://bugzilla.redhat.com/show_bug.cgi?id=1953254

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |177841 (FE-NEEDSPONSOR)

Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=177841
[Bug 177841] Tracker: Review requests from new Fedora packagers who need a
sponsor

[Bug 1953254] Review Request: SLiMgui - an evolutionary simulation framework

https://bugzilla.redhat.com/show_bug.cgi?id=1953254

--- Comment #2 from [e-mail] ---
Successful Fedora 35, Rawhide, build here:
https://koji.fedoraproject.org/koji/taskinfo?taskID=66624993.

Successful builds for other versions of Fedora previous (32 through 34), EPEL
8, CentOS 7, and openSUSE are visible on the Copr previously linked.

[Bug 1953254] Review Request: SLiMgui - an evolutionary simulation framework

https://bugzilla.redhat.com/show_bug.cgi?id=1953254

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |needinfo?(package-review@li
                   |                            |sts.fedoraproject.org)

--- Comment #3 from [e-mail] ---
@

needinfo requested: [Bug 1953254] Review Request: SLiMgui - an evolutionary simulation framework

Product: Fedora
Version: rawhide
Component: Package Review

[e-mail] has asked Package Review
 for needinfo:
Bug 1953254: Review Request: SLiMgui - an evolutionary simulation framework
https://bugzilla.redhat.com/show_bug.cgi?id=1953254

--- Comment #3 from [e-mail] ---
@

[Bug 1953254] Review Request: SLiMgui - an evolutionary simulation framework

https://bugzilla.redhat.com/show_bug.cgi?id=1953254

--- Comment #4 from [e-mail] ---
I should note here that the upstream developer does not wish to rename the
conflicting binary.

[Bug 1953254] Review Request: SLiMgui - an evolutionary simulation framework

https://bugzilla.redhat.com/show_bug.cgi?id=1953254

Kevin Fenzi  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Doc Type|---                         |If docs needed, set a value
              Flags|needinfo?(package-review@li |
                   |sts.fedoraproject.org)      |

--- Comment #5 from Kevin Fenzi  ---
No need to needinfo the review list. :)

[Bug 1953254] Review Request: SLiMgui - an evolutionary simulation framework

https://bugzilla.redhat.com/show_bug.cgi?id=1953254

--- Comment #6 from [e-mail] ---
Okay, thanks for the edit. Let me know if you're reviewing the package or if
there is anything I can provide.

[Bug 1953254] Review Request: SLiMgui - an evolutionary simulation framework

https://bugzilla.redhat.com/show_bug.cgi?id=1953254

Robert-André Mauchin 🐧  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #7 from Robert-André Mauchin 🐧  ---
 - Capitalize the Summary:

Summary:        An evolutionary simulation framework

- Use a better name for your archive:

Source0:       
https://github.com/MesserLab/SLiM/archive/v%%7Bversion%7D/%%7Bname%7D-%%7Bve...

- The patch in the SRC.RPM seems broken, you could use a direct link for that:

# Fixes a compilation issue on Fedora 34; fixed upstream for next release.
#
https://github.com/MesserLab/SLiM/commit/7964f7911e1665a1ba0783d9136b3d3bff0...
Patch0:        
https://github.com/MesserLab/SLiM/commit/7964f7911e1665a1ba0783d9136b3d3bff0...

- Don't mix Suse stuff in a Fedora SPEC

- Constrain the version here:

BuildRequires:  qt5-qtbase-devel  >= 5.9.5

- Are you sure the ExclusiveArch is needed, have you tested other arches?

- I don't see why this is necessary:

# Fedora: these package versions are those that COPR is building against, and
thus if
# they change because of point releases in Qt5, the RPMs need to be rebuilt and
deployed.
# Qt is weird and doesn't allow older software to be used if a newer point
release is
# installed on the system.
%if 0%{?fedora}
%if 0%{?fedora} == 31
Requires: qt5-qtbase == 5.13.2
%else
%if 0%{?fedora} == 32
Requires: qt5-qtbase == 5.14.2
%else
# Qt5 5.15.2 is the final version; Qt6 is not tested with SLiM.
%if 0%{?fedora} >= 33
Requires: qt5-qtbase == 5.15.2
%endif
%endif
%endif
%else
# If not installing on Fedora
# Conditonal requires for RHEL (and CentOS)
%if 0%{?rhel} == 8
Requires: qt5-qtbase == 5.12.5
%else
# Conditional requires for openSUSE and SLE.
# openSUSE Leap 15.2 and SLE 15 (and all service packs)
%if %{defined suse_version}
%if %{defined suse_version} == 1500
Requires: libqt5-qtbase == 5.12.7
%else
# openSUSE Tumbleweed
%if %{defined suse_version} >= 1550
Requires: libqt5-qtbase == 5.15.2
%endif
%endif
%else
# The minimum version of Qt5 required to run SLiMgui is 5.9.5, however this
does not actually assure that the system will install the same version of Qt5
that it was built against; an end-user may install a more recent Qt5 if, for
isntance, openSUSE Leap 15.3 builds against Qt 5.12.7 and the user upgrades to
5.15.2 when it is available. This is strange, but we can't protect against
every edge-case.
Requires: qt5-qtbase >= 5.9.5
%endif
%endif
%endif

Point releases don't matter, what matters is the major soname version of the
libraries it was built against (5 for Qt 5). There is no need to rebuild
anything unless that soname is changed.

- No:

%prep
tar -xf ../SOURCES/v%{version}.tar.gz # Decompresses to 'SLiM-[version macro]'

Either use %autosetup -p1 or %setup -q:

%prep
%autosetup -p1 -n SLiM-%{version}

Then remove all the ./SLiM-%{version}/ prefix as it is now the base directory
for the next instructions.

- This isn't how you apply a patch:

# The patch is successfully applied, as demonstrated by successful builds on
Fedora 34 on Copr.
# rpmlint likes to complain because we're not using the patch macro.
patch ./SLiM-%{version}/eidos/robin_hood.h < ../SOURCES/fedora34.patch

If you use %autosetup -p1, the patching is automatically handled.
If you use %setup -q, then add:

%patch0 -p1

- Use the %cmake macro to use Fedora default build flags:

%cmake -DBUILD_SLIMGUI=ON

- Then use:

%cmake_build

instead of:

%make_build

- Slight change due to the use of %cmake:

# Binaries
pushd %{_vpath_builddir}
install -p --mode=0755 --target-directory %{buildroot}%{_bindir} eidos SLiMgui
install -p --mode=0755 --no-target-directory slim
%{buildroot}%{_bindir}/SLiMcli # Rename the binary so that it does not conflict
with Simple Login Manager.
popd

- Use install with -p to keep the files timestamps

- Use macros instead of hard-coded directories:

/usr/bin/ -> %{_bindir}/
/usr/share/ -> %{_datadir}/

- This is not needed:

%post
update-mime-database -n /usr/share/mime/
xdg-mime install --mode system
/usr/share/mime/packages/org.messerlab.slimgui-mime.xml

Proposed SPEC:

Name:           SLiMgui
Version:        3.6
Release:        5%{?dist}
Summary:        An evolutionary simulation framework

License:        GPLv3+
URL:            https://messerlab.org/slim/
Source0:       
https://github.com/MesserLab/SLiM/archive/v%%7Bversion%7D/%%7Bname%7D-%%7Bve...

# Fixes a compilation issue on Fedora 34; fixed upstream for next release.
#
https://github.com/MesserLab/SLiM/commit/7964f7911e1665a1ba0783d9136b3d3bff0...
Patch0:        
https://github.com/MesserLab/SLiM/commit/7964f7911e1665a1ba0783d9136b3d3bff0...

BuildRequires:  cmake
BuildRequires:  desktop-file-utils
BuildRequires:  qt5-qtbase-devel >= 5.9.5
BuildRequires:  libappstream-glib
Requires:       hicolor-icon-theme
# This is preferred to listing each architecture that SLiM cannot compile upon
and 
# filing Bugzilla tickets for each, as the Fedora Packaging Guidelines imply
should 
# be done. Upstream determines multiarch capabilities.
ExclusiveArch:  x86_64

%description
SLiM is an evolutionary simulation framework that combines a powerful engine
for
population genetic simulations with the capability of modeling arbitrarily
complex evolutionary scenarios. Simulations are configured via the integrated
Eidos scripting language that allows interactive control over practically every
aspect of the simulated evolutionary scenarios. The underlying individual-based
simulation engine is highly optimized to enable modeling of entire chromosomes
in large populations. We also provide a graphical user interface on macOS and
Linux for easy simulation set-up, interactive runtime control, and dynamical
visualization of simulation output.

%prep
%autosetup -p1 -n SLiM-%{version}

# The packaging guidelines recommend LUA or Python, but sed is POSIX and SUS,
# so the dependency chain needn't really BuildRequires: sed. Moreover, this
# is extremely basic 'scripting'.
# Create a symbolic icon from the provided SVG
# SVG object data structure:
#    <rect
#       y="0"
#       x="0"
#       height="16"
#       width="16"
#       id="rect31"
#       style="fill:#000000" />
sed -i '69,166 s/"fill:#[[:alnum:]]*"/"fill:#ffffff"/'
QtSLiM/icons/AppIcon16.svg
sed -i '62,68 d' QtSLiM/icons/AppIcon16.svg

%build
%cmake -DBUILD_SLIMGUI=ON
%cmake_build

%install
mkdir -p %{buildroot}%{_bindir}
mkdir -p
%{buildroot}%{_datadir}/icons/hicolor/{scalable/apps,scalable/mimetypes,symbolic/apps}
mkdir -p %{buildroot}%{_datadir}/{applications/,metainfo/,mime/packages/}

# Binaries
pushd %{_vpath_builddir}
install -p --mode=0755 --target-directory %{buildroot}%{_bindir} eidos SLiMgui
install -p --mode=0755 --no-target-directory slim
%{buildroot}%{_bindir}/SLiMcli # Rename the binary so that it does not conflict
with Simple Login Manager.
popd

# Colourful and Symbolic Application Icons
install -p --mode=0644 --no-target-directory QtSLiM/icons/AppIcon64.svg
%{buildroot}%{_datadir}/icons/hicolor/scalable/apps/org.messerlab.slimgui.svg
install -p --mode=0644 --no-target-directory QtSLiM/icons/AppIcon16.svg
%{buildroot}%{_datadir}/icons/hicolor/symbolic/apps/org.messerlab.slimgui-symbolic.svg

# slim Mimetype Icon
install -p --mode=0644 --no-target-directory QtSLiM/icons/DocIcon.svg
%{buildroot}%{_datadir}/icons/hicolor/scalable/mimetypes/text-slim.svg

# Desktop Entry and Appstream XMLs
install -p --mode=0644 org.messerlab.slimgui-mime.xml --target-directory
%{buildroot}%{_datadir}/mime/packages/
desktop-file-install --mode=0644 --dir=%{buildroot}%{_datadir}/applications/
org.messerlab.slimgui.desktop
install -p --mode=0644 org.messerlab.slimgui.appdata.xml --target-directory
%{buildroot}%{_metainfodir}/

%check
# Annoyingly, Blender's appstream XML points to https and will always produce
output if installed.
appstream-util validate-relax --nonet
%{buildroot}%{_metainfodir}/org.messerlab.slimgui.appdata.xml

%files
%{_bindir}/eidos
%{_bindir}/SLiMcli
%{_bindir}/SLiMgui
%{_datadir}/applications/org.messerlab.slimgui.desktop
%{_datadir}/icons/hicolor/scalable/apps/org.messerlab.slimgui.svg
%{_datadir}/icons/hicolor/scalable/mimetypes/text-slim.svg
%{_datadir}/icons/hicolor/symbolic/apps/org.messerlab.slimgui-symbolic.svg
%{_datadir}/metainfo/org.messerlab.slimgui.appdata.xml
%{_datadir}/mime/packages/org.messerlab.slimgui-mime.xml

%changelog
* Sat Apr 24 2021 Bryce Carson  - 3.6-5
- Preparing the package for submission to Bugzilla for Fedora's official
repository.
- Changed from using `cp` to `install` with proper mode bits for files.
- Changed from using `cp` to `desktop-file-install` for .desktop file, as per
Fedora packaging guidelines.
- Changed from hardcoded source directories, such as "SLiM-3.6", to
"SLiM-[version macro]" in prep, build, and install. The change is also made in
Source0.
- Changed from hardcoding the metainfo directory to using the preferred
[_metainfodir] macro.
- General grammar and spelling improvements throughout the spec file.
- Fixed contributor URI (email) in the changelog entry for 3.6-4, changing from
localhost.localdomain to my email address. I don't know why that changelog
entry was wrong in that manner.
- Included additional comments to clarify decisions in packaging.
- Renamed the slim binary to SLiMcli, to prevent conflict with Simple Login
Manager; the capitalization is the preferred 'typography' of upstream, and
appending 'cli' seems a sensible change to complement 'SLiMgui'. Upstream has
not been consulted as of this changelog entry. Will consult before package
submission.
- Requires: for Fedora now uses >= 33, as all versions should use Qt 5.15.2,
the highest version of Qt5 available from the Qt Company, as far as I know.

* Sat Mar 20 2021 Bryce Carson  - 3.6-4
- Added support for openSUSE (with SUSE Linux Enterprise users possibly able to
use the openSUSE RPM).
- Cleaned up the changelog.
- The `[<jobs>]` argument to the cmake `--parallel` option was removed, so that
Copr uses the default number of concurrent processes (and hopefully the maximum
number, rather than hardcoding eight processes).

* Wed Mar 3 2021 Bryce Carson  - 3.6-3
- Application of patch to allow building on Fedora 34 and Fedora Rawhide.

* Wed Mar 3 2021 Bryce Carson  - 3.6-2
- Specified required Qt 5.15.2 on Fedora 34.
- Added package version in previous changelog entry.

* Wed Mar 3 2021 Bryce Carson  - 3.6-1
- New package release.
- Removed source edits that were addressed upstream.

* Sun Jan 31 2021 Bryce Carson  - 3.5-6
- spec file improvements; brace expansion used and sorting performed.
- FreeDesktop compliance improvements; the organization domain and application
name are corrected and are now compliant.
- Source modifications allow Gnome Classic to display the proper application
name.
- The symbolic application icon is now created programmatically from upstream
icons in the source, rather than a second source file.

* Thu Jan 28 2021 Bryce Carson  - 3.5-5
- org.messerlab.slimgui.desktop changed in prep to correct Categories value;
fixes desktop integration on Fedora 33 when using Gnome Classic environment.
- New symbolic icon included; improves desktop integration on Fedora 33 when
using Gnome 3 with Wayland.
- Edited the changelog to not refer to the prep stage as a macro, simply
"prep", to fix rpmlint warnings.

* Thu Jan 14 2021 Bryce Carson  - 3.5-4
- org.messerlab.slimgui.desktop changed in prep to correct StartupWMClass and
icon; fixes desktop integration on Fedora 33.
- Sorted changelog in descending chronological order.

* Sun Dec 06 2020 Bryce Carson  - 3.5-3
- Updated the requires in the .spec file (and thus the package dependencies) to
reflect updates to Qt5 on Fedora 33.
- Qt5 5.15.2 now required on Fedora 33.

* Sun Dec 06 2020 Bryce Carson  - 3.5-2
- Changed the tar command in .spec file to address discrepancy between GitHub
archive URI and downloaded source archive.

* Sun Dec 06 2020 Bryce Carson  - 3.5-1
- Created new release package
- Differences from 3.4-8 include removal of necessary source modifications for
3.4
- The source modifications for 3.4 were addressed by the upstream

[Bug 1953254] Review Request: SLiMgui - an evolutionary simulation framework

https://bugzilla.redhat.com/show_bug.cgi?id=1953254

--- Comment #8 from Robert-André Mauchin 🐧  ---
Also explicitly add gcc-c++ as a BuildRequires

needinfo canceled: [Bug 1953254] Review Request: SLiMgui - an evolutionary simulation framework

Product: Fedora
Version: rawhide
Component: Package Review

Package Review  has canceled Package
Review 's request for
[e-mail]'s needinfo:
Bug 1953254: Review Request: SLiMgui - an evolutionary simulation framework
https://bugzilla.redhat.com/show_bug.cgi?id=1953254

--- Comment #10 from Package Review 
---
This is an automatic action taken by review-stats script.

The ticket submitter failed to clear the NEEDINFO flag in a month.
As per https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews
we consider this ticket as DEADREVIEW and proceed to close it.