Bug 5015

*** Bug 3111 has been marked as a duplicate of this bug. ***

This package is ready for fedora 28+ and el7

SPEC file: https://sagitter.fedorapeople.org/ppsspp/ppsspp.spec
SRPM file: https://sagitter.fedorapeople.org/ppsspp/ppsspp-1.6.3-2.fc28.src.rpm

- Wayland on epel7 uses an unversioned libwayland-egl library

- Please use one BR per line:

- Drop uneeded libglvnd-devel,mesa-libwayland-egl you don't want that as a application.

- Please use libGL-devel instead of mesa (that might move to libglvnd-libGL-devel or else in the future, so using libGL-devel is the way forward).
Same for libGLES-devel and libEGL-devel

- Requires: hicolor-icon-theme, is this still needed nowadays ?

- You fix on armv7 looks a bit strong. Specially:
"-mtune=cortex-a15.cortex-a7 -mcpu=cortex-a15 -mfpu=neon-vfpv4"
Isn't using -mfpu=neon enough ? Also it overrides fedora march flags because fedora uses: -march=armv7-a instead of -march=arm (we don't care about armv6hl).

So to me theses flags should be acceptable
"-mfpu=neon -fomit-frame-pointer -ftree-vectorize -mvectorize-with-neon-quad -ffast-math -DARM_NEON"
But it will discard non-neon armv7 hardware (such a tegra2 Trimslice or others). But I guess these devices might not be capable to handle the emulator well.
If it's the case, best is to verify that the device has the needed capability before running the binary.

- Why vulkan is kept disabled in cmake ?
- Why GLES2 is disabled in cmake but added in configure flags and BR ?
FYI, I'm splitted with GLES2, fedora seems to prefer to rely on "desktop GL", but many drivers might not expose full desktop opengl 2.0 support, so here a gles2 rendering path might looks more appropriate.
This is not only the case on arm where some FLOSS drivers are still limited, but also on x86 (Intel Pinetrail was downgraded to only advertise OpenGL 1.4 because of too much missing capabilities but still advertise GLES 2.0).

- Why a special case here ?
%if 0%{?rhel}
 -DWAYLAND_EGL_LIBRARIES:FILEPATH=%{_libdir}/libwayland-egl.so.1 \
%endif

Please remind that Wayland on RHEL is a technical preview. I don't think much people


- About you find loop in %prep
you can use "-exec  chmod -x {} ';'" at the end of the find command instead of using a loop

- Using git snapshot is documented here:
https://fedoraproject.org/wiki/Packaging:SourceURL#Git_Hosting_Services
I don't understand why you would need to use theses sub-modules. To me, there is  a need to unbundle them. Unless the ones available in fedora aren't new enough ?

(In reply to Nicolas Chauvet from comment #4)
> - Please use one BR per line:
> 
> - Drop uneeded libglvnd-devel,mesa-libwayland-egl you don't want that as a
> application.
> 
> - Please use libGL-devel instead of mesa (that might move to
> libglvnd-libGL-devel or else in the future, so using libGL-devel is the way
> forward).
> Same for libGLES-devel and libEGL-devel
> 
> - Requires: hicolor-icon-theme, is this still needed nowadays ?

Yes, unless it is installed by default.

> 
> - You fix on armv7 looks a bit strong. Specially:
> "-mtune=cortex-a15.cortex-a7 -mcpu=cortex-a15 -mfpu=neon-vfpv4"
> Isn't using -mfpu=neon enough ? Also it overrides fedora march flags because
> fedora uses: -march=armv7-a instead of -march=arm (we don't care about
> armv6hl).
> 
> So to me theses flags should be acceptable
> "-mfpu=neon -fomit-frame-pointer -ftree-vectorize -mvectorize-with-neon-quad
> -ffast-math -DARM_NEON"
> But it will discard non-neon armv7 hardware (such a tegra2 Trimslice or
> others). But I guess these devices might not be capable to handle the
> emulator well.
> If it's the case, best is to verify that the device has the needed
> capability before running the binary.

Those flags come from upstream fix. I'm using yours now.

> 
> - Why vulkan is kept disabled in cmake ?

Enabled. An old build test.

> - Why GLES2 is disabled in cmake but added in configure flags and BR ?
> FYI, I'm splitted with GLES2, fedora seems to prefer to rely on "desktop
> GL", but many drivers might not expose full desktop opengl 2.0 support, so
> here a gles2 rendering path might looks more appropriate.
> This is not only the case on arm where some FLOSS drivers are still limited,
> but also on x86 (Intel Pinetrail was downgraded to only advertise OpenGL 1.4
> because of too much missing capabilities but still advertise GLES 2.0).

GLES support is not currently compiled fine

> 
> - Why a special case here ?
> %if 0%{?rhel}
>  -DWAYLAND_EGL_LIBRARIES:FILEPATH=%{_libdir}/libwayland-egl.so.1 \
> %endif
> 
> Please remind that Wayland on RHEL is a technical preview. I don't think
> much people

I've corrected BR packages, that line is useless now.

> 
> - Using git snapshot is documented here:
> https://fedoraproject.org/wiki/Packaging:SourceURL#Git_Hosting_Services
> I don't understand why you would need to use theses sub-modules. To me,
> there is  a need to unbundle them. Unless the ones available in fedora
> aren't new enough ?

CMake wants them, but they are not included in the source code of PPSSPP.
Commit 9a610c8a012e73b7d2c6677f48d24f3e185e61c7 coincides with the release 1.6.3.

SPEC file: https://sagitter.fedorapeople.org/ppsspp/ppsspp.spec
SRPM file: https://sagitter.fedorapeople.org/ppsspp/ppsspp-1.6.3-3.fc28.src.rpm

el7 scratch build: http://koji.rpmfusion.org/koji/taskinfo?taskID=257413

There is no need to use cmake and cmake3.
Fedora and RHEL can both use cmake3 

BuildRequires: cmake3


ad the same applies for the %cmake3 macro

(In reply to leigh scott from comment #6)
> There is no need to use cmake and cmake3.
> Fedora and RHEL can both use cmake3 
> 
> BuildRequires: cmake3
> 
> 
> ad the same applies for the %cmake3 macro

Ok.

I'm testing a recent commit. Please, do not review yet.

Ready to go on.

SPEC file: https://sagitter.fedorapeople.org/ppsspp/ppsspp.spec
SRPM file: https://sagitter.fedorapeople.org/ppsspp/ppsspp-1.6.3-4.20180912git6d0ed4a.fc28.src.rpm

(In reply to Antonio Trande from comment #8)
> Ready to go on.
> 
> SPEC file: https://sagitter.fedorapeople.org/ppsspp/ppsspp.spec
> SRPM file:
> https://sagitter.fedorapeople.org/ppsspp/ppsspp-1.6.3-4.20180912git6d0ed4a.
> fc28.src.rpm

I've a cosmetic change I would like to suggest:
Please consider adding two lines of spaces between RPM sections (%prep,%build,%install,%files,%changelog) and only one within a section.
This will improve readability for your spec files.

I'm still not sure about having a %{rhel} condition.
If using libEGL-devel doesn't work, please use mesa-libEGL-devel everywhere
But I might investigate this. (same for GLES)

Please drop  epel-rpm-macros mesa-libwayland-egl-devel libglvnd-egl
Theses should not be used directly by the package (but either bring by the infra or internal dependencies of others dependencies).

Please avoid to condition the arm patch to the architecture.

Also avoid to redefine cmake3 ctest3 as I expect the same %cmake3 can be used everywhere once you have the appropriate cmake3 dependency.

Modified (without packaging release bump):

SPEC file: https://sagitter.fedorapeople.org/ppsspp/ppsspp.spec
SRPM file: https://sagitter.fedorapeople.org/ppsspp/ppsspp-1.6.3-4.20180912git6d0ed4a.fc28.src.rpm

Good.

This package is APPROVED by me.

Thank you Nicolas.