Bug 1344261 (warsow) - Review Request: warsow - Fast paced 3D first person shooter
Summary: Review Request: warsow - Fast paced 3D first person shooter
Status: CLOSED ERRATA
Alias: warsow
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Chris Sandler
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Keywords:
Depends On: warsow-data
Blocks: FE-GAMESIG
TreeView+ depends on / blocked
 
Reported: 2016-06-09 09:39 UTC by Pete Walter
Modified: 2017-02-03 21:49 UTC (History)
5 users (show)

(edit)
Clone Of:
(edit)
Last Closed: 2017-02-03 19:18:14 UTC
chr77: fedora-review+


Attachments (Terms of Use)

Description Pete Walter 2016-06-09 09:39:26 UTC
Spec URL: https://pwalter.fedorapeople.org/warsow.spec
SRPM URL: https://pwalter.fedorapeople.org/warsow-2.1-1.fc24.src.rpm
Description:
Warsow is a fast paced first person shooter consisting of cel-shaded
cartoon-like graphics with dark, flashy and dirty textures. Warsow is based on
the E-novel "Chasseur de bots" ("Bots hunter" in English) by Fabrice Demurger.
Warsow's codebase is built upon Qfusion, an advanced modification of the Quake
II engine.

Fedora Account System Username: pwalter

Comment 1 Pete Walter 2016-06-09 10:57:32 UTC
Note that this goes together with the warsaw-data package from https://bugzilla.redhat.com/show_bug.cgi?id=1344294

Comment 2 Pete Walter 2016-06-09 11:59:58 UTC
Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=14434169

Comment 3 Raphael Groner 2016-06-12 20:54:34 UTC
Can you put both packages warsow (this bug) and warsaw-data (bug #1344294) in one spec file? warsaw-data could be a subpackage and is not much useful for its own alone. We could handle all in one single review.

Are you interested in a review swap? Maybe with bug #1344115.

> %global warsow_libdir %{_prefix}/lib/warsow> # Filter private libraries from provides
> %global __provides_exclude_from ^%{warsow_libdir}/.*\\.so$> # Install private libraries to a private directory
> install -d $RPM_BUILD_ROOT%{warsow_libdir}/libs
> install -m 755 libs/*.so $RPM_BUILD_ROOT%{warsow_libdir}/libs/
…
%files
%{warsow_libdir}/

This does not look right to me. Please explain why this private(?) folder must be used. You should use %{_libdir}/%{name} instead to also honor 64 bits with /usr/lib64/warsaw.


> BuildRequires:  /usr/bin/convert
> BuildRequires:  /usr/bin/desktop-file-install
> BuildRequires:  /usr/bin/dos2unix

Better is to use concrete package names:
BuildRequires:  ImageMagick
BuildRequires:  desktop-file-utils
BuildRequires:  dos2unix

You can verify with dnf whatprovides.

Comment 4 Pete Walter 2016-06-14 14:49:59 UTC
(In reply to Raphael Groner from comment #3)
> Can you put both packages warsow (this bug) and warsaw-data (bug #1344294)
> in one spec file? warsaw-data could be a subpackage and is not much useful
> for its own alone. We could handle all in one single review.

I think two separate packages makes things simpler. They are built from separate tarballs and it makes packaging easier of they are separate SRPMs as well.


> Are you interested in a review swap? Maybe with bug #1344115.

Sorry, not really interested in wine stuff


> > %global warsow_libdir %{_prefix}/lib/warsow
> …
> > # Filter private libraries from provides
> > %global __provides_exclude_from ^%{warsow_libdir}/.*\\.so$
> …
> > # Install private libraries to a private directory
> > install -d $RPM_BUILD_ROOT%{warsow_libdir}/libs
> > install -m 755 libs/*.so $RPM_BUILD_ROOT%{warsow_libdir}/libs/
> …
> %files
> %{warsow_libdir}/
> 
> This does not look right to me. Please explain why this private(?) folder
> must be used. You should use %{_libdir}/%{name} instead to also honor 64
> bits with /usr/lib64/warsaw.

Private folder is used because it installs private libraries that aren't meant for consumption by other apps.


> > BuildRequires:  /usr/bin/convert
> > BuildRequires:  /usr/bin/desktop-file-install
> > BuildRequires:  /usr/bin/dos2unix
> 
> Better is to use concrete package names:
> BuildRequires:  ImageMagick
> BuildRequires:  desktop-file-utils
> BuildRequires:  dos2unix
> 
> You can verify with dnf whatprovides.

I disagree here. Spelling out what executable we need makes packaging much easier to understand.

Comment 5 Raphael Groner 2016-08-07 18:01:12 UTC
Review swap with bug #1364805?

Comment 6 Raphael Groner 2016-10-17 20:19:47 UTC
Are you interested in a review swap? Maybe you can take a look into bug #1385856?

Comment 7 Pete Walter 2016-10-18 12:17:35 UTC
Sure, let's do that. Thanks.

Comment 8 Raphael Groner 2016-11-11 20:37:08 UTC
FTBFS

You may need 'BuildRequires: pkgconfig(gl)' that installs mesa-libGL-devel including the missing %{_includedir}/GL/gl.h file.

[  8%] Building C object ref_gl/CMakeFiles/ref_gl.dir/r_alias.c.o
cd /builddir/build/BUILD/warsow_21_sdk/source/source/cmake_build/ref_gl && /usr/bin/cc  -DAPPLICATION_VERSION_HEADER=\"version.warsow.h\" -DFREETYPELIB_RUNTIME -DLIBCURL_RUNTIME -DLIBJPEG_RUNTIME -DLIBPNG_RUNTIME -DOGGLIB_RUNTIME -DTHEORALIB_RUNTIME -DUSE_SDL2 -DVORBISLIB_RUNTIME -DZLIB_RUNTIME -D_LARGEFILE64_SOURCE -D_LARGEFILE_SOURCE -Dref_gl_EXPORTS -I/usr/include/SDL2  -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic -pipe -Wall -Wno-unused-function -fvisibility=hidden -Winvalid-pch -std=gnu99 -D_POSIX_C_SOURCE=200112L -D_XOPEN_SOURCE=500 -DNDEBUG -fno-strict-aliasing -ffast-math -funroll-loops -fPIC   -o CMakeFiles/ref_gl.dir/r_alias.c.o   -c /builddir/build/BUILD/warsow_21_sdk/source/source/ref_gl/r_alias.c
In file included from /builddir/build/BUILD/warsow_21_sdk/source/source/ref_gl/r_glimp.h:70:0,
                 from /builddir/build/BUILD/warsow_21_sdk/source/source/ref_gl/r_local.h:72,
                 from /builddir/build/BUILD/warsow_21_sdk/source/source/ref_gl/r_alias.c:23:
/builddir/build/BUILD/warsow_21_sdk/source/source/ref_gl/qgl.h:69:19: fatal error: GL/gl.h: No such file or directory
 #include <GL/gl.h>
                   ^
compilation terminated.

Comment 9 Raphael Groner 2016-12-09 21:06:27 UTC
Ping? Are you still interested in this package?

Comment 10 Pete Walter 2017-01-11 09:30:55 UTC
Right, thanks. I've added mesa-libGL-devel locally. Do you need a new SRPM? It's rather huge, I'd avoid uploading a new one unless you really need it.

Comment 11 Raphael Groner 2017-01-11 18:04:07 UTC
Yes, some new SPEC and SRPM files are needed to do another run of fedora-review. f-r tries to install the package in a mock environment, please provide warsow-data (rhbz#1344294) for completeness of runtime dependencies.

FYI I'm currently moving and stuck with a limited internet connection, so the next run may not be expected in the next couple of weeks.

Comment 12 Raphael Groner 2017-01-31 23:46:58 UTC
No response so far. :(

Comment 13 Chris Sandler 2017-02-01 11:21:23 UTC
Raphael, I do not understand why you wanted to have warsow-data built to continue the review. Pete has a completely legitimate reason for not building it first (it would create broken dependencies in rawhide).

Packaging here looks solid to me. Approving the review based on the earlier rpmfusion package review (https://bugzilla.rpmfusion.org/show_bug.cgi?id=2762) and the review comments here.

Thanks Pete, great work here!

Comment 14 Gwyn Ciesla 2017-02-01 13:16:40 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/warsow

Comment 15 Raphael Groner 2017-02-01 18:23:20 UTC
Thanks!

Comment 16 Fedora Update System 2017-02-01 19:31:44 UTC
warsow-data-2.1-2.el7 warsow-2.1-3.el7 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2017-56ee1f45bb

Comment 17 Fedora Update System 2017-02-01 19:32:01 UTC
warsow-data-2.1-2.fc24 warsow-2.1-3.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2017-9f2970d1d7

Comment 18 Fedora Update System 2017-02-01 19:32:10 UTC
warsow-data-2.1-2.fc25 warsow-2.1-3.fc25 has been submitted as an update to Fedora 25. https://bodhi.fedoraproject.org/updates/FEDORA-2017-ff35e19cfe

Comment 19 Fedora Update System 2017-02-02 21:49:22 UTC
warsow-2.1-3.fc24, warsow-data-2.1-2.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-9f2970d1d7

Comment 20 Fedora Update System 2017-02-02 22:49:46 UTC
warsow-2.1-3.fc25, warsow-data-2.1-2.fc25 has been pushed to the Fedora 25 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-ff35e19cfe

Comment 21 Fedora Update System 2017-02-02 23:18:43 UTC
warsow-2.1-3.el7, warsow-data-2.1-2.el7 has been pushed to the Fedora EPEL 7 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2017-56ee1f45bb

Comment 22 Fedora Update System 2017-02-03 19:18:14 UTC
warsow-2.1-3.el7, warsow-data-2.1-2.el7 has been pushed to the Fedora EPEL 7 stable repository. If problems still persist, please make note of it in this bug report.

Comment 23 Fedora Update System 2017-02-03 20:49:38 UTC
warsow-2.1-3.fc25, warsow-data-2.1-2.fc25 has been pushed to the Fedora 25 stable repository. If problems still persist, please make note of it in this bug report.

Comment 24 Fedora Update System 2017-02-03 21:49:04 UTC
warsow-2.1-3.fc24, warsow-data-2.1-2.fc24 has been pushed to the Fedora 24 stable repository. If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.