Bug 1085612 - Review Request: voxelands - The Fun-Focused Free Software Voxel World Game (was: minetest-classic)
Review Request: voxelands - The Fun-Focused Free Software Voxel World Game (w...
Status: NEW
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
unspecified Severity medium
: ---
: ---
Assigned To: Dennis Chen
Fedora Extras Quality Assurance
:
Depends On:
Blocks: FE-NEEDSPONSOR FE-GAMESIG
  Show dependency treegraph
 
Reported: 2014-04-08 22:08 EDT by Ye Myat Kaung
Modified: 2017-03-06 17:21 EST (History)
7 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed:
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
barracks510: fedora‑review?


Attachments (Terms of Use)

  None (edit)
Description Ye Myat Kaung 2014-04-08 22:08:54 EDT
Spec URL: http://mavjs.fedorapeople.org/minetest-classic/minetest-classic.spec
SRPM URL: http://mavjs.fedorapeople.org/minetest-classic/minetest-classic-1403.00-1.fc20.src.rpm
Description: Game of mining, crafting and building in the infinite world of cubic blocks.
Features both single and networked multiplayer mode. A fork of minetest (http://www.minetest.net)
Fedora Account System Username: mavjs

This is my first package and I need a sponsor. Currently, I'm providing rpms for minetest-classic using copr: http://www.minetest-classic.com/download.html

Successfully built rpm(s) can be found here: http://copr.fedoraproject.org/coprs/mavjs/minetest-classic/
Comment 1 Michael Schwendt 2014-04-10 14:47:27 EDT
> http://copr-be.cloud.fedoraproject.org/results/mavjs/minetest-classic/fedora-20-x86_64/minetest-classic-1403.00-1.fc20/build.log

Build output is non-verbose. One cannot verify the compiler settings that are used (e.g. compiler flags, preprocessor definitions, paths).
https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags


> #warning "Disabling unit tests"

Any comment on this?


> warning: File listed twice: /usr/local/share/locale/da/LC_MESSAGES/minetest-classic.mo
> warning: File listed twice: /usr/local/share/locale/de/LC_MESSAGES/minetest-classic.mo
> warning: File listed twice: /usr/local/share/locale/fr/LC_MESSAGES/minetest-classic.mo
> warning: File listed twice: /usr/local/share/locale/it/LC_MESSAGES/minetest-classic.mo

Reason to worry, because /usr/local must not be used by the package.


> cp -p %{buildroot}%{_prefix}/local/bin/%{name} %{buildroot}%{_bindir}

> *** WARNING: identical binaries are copied, not linked:
>         /usr/bin/minetest-classic
>    and  /usr/local/bin/minetest-classic

/usr/local must not be used by Fedora's packages.

Have you ever before listed the package's files?

$ rpmls -p minetest-classic-1403.00-1.fc20.x86_64.rpm |grep /usr/local|wc -l
426


Please point the fedora-review tool at this ticket. It evaluates the "SRPM URL:" and "Spec URL:" lines and performs many helpful checks. Run "fedora-review -b 1085612".
Comment 2 Michael Schwendt 2014-04-10 15:06:46 EDT
[...]
Running transaction
  Erasing    : minetest-classic-1403.00-1.fc20.x86_64                       1/4 
/var/tmp/rpm-tmp.l7dpj4: line 1: [0: command not found
  Erasing    : irrlicht-1.8.1-2.fc20.x86_64                                 2/4 
[...]
Comment 3 Matthew Miller 2014-09-17 15:49:28 EDT
Looks like upstream has been renamed to "voxelands".
Comment 4 Matthew Miller 2014-09-17 15:51:18 EDT
AND there's a Copr of that at: http://copr.fedoraproject.org/coprs/mavjs/voxelands/
Comment 5 Ye Myat Kaung 2014-09-17 16:13:33 EDT
yes, after having not following it for awhile, I've been meaning to get back into fixing the errors mentioned above and submitting the name change.

Should I just go ahead and change the title and link to the new spec/src.rpms here? Or do I close this and file a new package review.
Comment 7 Mihkel Vain 2014-09-18 15:16:45 EDT
Hi. This is not a formal review... just some remarks.

1. voxelands.src:14: W: mixed-use-of-spaces-and-tabs (spaces: line 14, tab: line 3)

Use on only tabs or spaces in spec file.


2. Group:		Amusements/Games

You don't need that, unless to plan to target EPEL 5

3. URL:		http://wwww.voxelands.com

Loose one w

4. cmake -DBUILD_SERVER=0 -DRUN_IN_PLACE=0 -DCMAKE_INSTALL_PREFIX=/usr

Use %cmake macro eg:
%cmake -DBUILD_SERVER=0 -DRUN_IN_PLACE=0 -DCMAKE_INSTALL_PREFIX=/usr

5. %clean
rm -rf %{buildroot}

You don't need that, unless to plan to target EPEL 5 

5. %defattr(-,root,root)

You don't need that.


Note. This list may not be complete and I don't intend to do formal review, because I think I don't have enough right for this.
Comment 8 Christopher Meng 2014-09-18 19:41:36 EDT
%{_prefix}/local/


This is NOT allowed.
Comment 9 Ye Myat Kaung 2014-09-19 12:58:59 EDT
Thanks for the (informal) review(s). I've change the suggested remarks.

URL: https://mavjs.fedorapeople.org/voxelands/voxelands.spec


@Christopher Meng - I don't think I have %{_prefix}/local/ anymore in the voxelands.spec, I believe it was in the minetest-classic.spec (which was the old project/spec file)
Comment 10 Christopher Meng 2014-09-20 01:37:36 EDT
Yes, by after using %cmake macro the usr/local should be nuked.


%cmake -DBUILD_SERVER=0 -DRUN_IN_PLACE=0 -DCMAKE_INSTALL_PREFIX=/usr

$ rpm -E %cmake

  CFLAGS="${CFLAGS:--O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches  -m64 -mtune=generic}" ; export CFLAGS ; 
  CXXFLAGS="${CXXFLAGS:--O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches  -m64 -mtune=generic}" ; export CXXFLAGS ; 
  FFLAGS="${FFLAGS:--O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches  -m64 -mtune=generic -I/usr/lib64/gfortran/modules}" ; export FFLAGS ; 
  FCFLAGS="${FCFLAGS:--O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches  -m64 -mtune=generic -I/usr/lib64/gfortran/modules}" ; export FCFLAGS ; 
  LDFLAGS="${LDFLAGS:--Wl,-z,relro }" ; export LDFLAGS ; 
  /usr/bin/cmake \
        -DCMAKE_C_FLAGS_RELEASE:STRING="-DNDEBUG" \
        -DCMAKE_CXX_FLAGS_RELEASE:STRING="-DNDEBUG" \
        -DCMAKE_Fortran_FLAGS_RELEASE:STRING="-DNDEBUG" \
        -DCMAKE_VERBOSE_MAKEFILE:BOOL=ON \
        -DCMAKE_INSTALL_PREFIX:PATH=/usr \
        -DINCLUDE_INSTALL_DIR:PATH=/usr/include \
        -DLIB_INSTALL_DIR:PATH=/usr/lib64 \
        -DSYSCONF_INSTALL_DIR:PATH=/etc \
        -DSHARE_INSTALL_PREFIX:PATH=/usr/share \
%if "lib64" == "lib64" 
        -DLIB_SUFFIX=64 \
%endif 
        -DBUILD_SHARED_LIBS:BOOL=ON

ALready defined in the macro:

-DCMAKE_INSTALL_PREFIX=/usr

And, it'd be better to have a server "-DBUILD_SERVER=0", or other explanations?
Comment 11 Christopher Meng 2014-09-20 01:40:57 EDT
* Thu Sep 11 2014 Ye Myat Kaung <mavjs01@gmail.com> - 1408.00

The version in the end is not correct, you can use rpmdev-bumpspec to bump it and see the standard.
Comment 12 Matthew Miller 2014-09-20 11:38:20 EDT
(In reply to Christopher Meng from comment #11)
> * Thu Sep 11 2014 Ye Myat Kaung <mavjs01@gmail.com> - 1408.00
> 
> The version in the end is not correct, you can use rpmdev-bumpspec to bump
> it and see the standard.

I assume you mean this part:

  %global verrel 1408.00

  Name: 	voxelands
  Version:	%{verrel}
  Release:	1%{?dist}
  Summary:	The Fun-Focused Free Software Voxel World Game

  License:	GPLv3
  URL:		http://www.voxelands.com
  Source0:	http://www.voxelands.com/downloads/%{name}-%{verrel}-src.tar.bz2

This could be more simply done just like this:


  Name:         voxelands
  Version:      1408.00
  Release:      1%{?dist}
  Summary:       The Fun-Focused Free Software Voxel World Game

  License:      GPLv3
  URL:          http://www.voxelands.com
  Source0:      http://www.voxelands.com/downloads/%{name}-%{version}-src.tar.bz2


No need for a special global, because %{version} already works.
Comment 13 Ye Myat Kaung 2014-09-20 13:10:30 EDT
Thanks for the reviews. I have made a few changes as suggested.

Got rid of:
%{verrel}
-DCMAKE_INSTALL_PREFIX=/usr
1408.00 (and changed to 1408.00-1)

-DBUILD_SERVER=0 is passed to cmake, because, if not the project also builds the server binary at the same time.

-DRUN_IN_PLACE=0 is passed so that the client binary reads configuration and map data from ~/.voxelands and not from the parent directory the binary was launched from.
Comment 14 Ye Myat Kaung 2014-10-07 11:01:29 EDT
I've made a newer spec file that is similar to minetest - which is already in the fedora repos, and since voxelands is a fork of that.

WIth this new spec file there is no need to pass the -DBUILD_SERVER=0, and it also builds the voxelands-server package. My only concern is that voxelands, like minetest bundles jthread, so, I an not sure I'd need to apply "Bundled Library Exception" with FPC. The issue is the same as the original Exception: https://fedorahosted.org/fpc/ticket/347

Do I still need to apply for "Bundled Library Exception" with FPC for voxelands, as well?

A new stable version of voxelands (1409.00) was released some weekend ago.

SPEC URL: https://mavjs.fedorapeople.org/voxelands/1409.00/voxelands.spec (This spec is used to build the voxelands packages on mavjs/voxelands on copr)
SRPM URL: https://mavjs.fedorapeople.org/voxelands/1409.00/voxelands-1409.00-4.fc20.src.rpm
SOURCE FILE URLs: https://mavjs.fedorapeople.org/voxelands/1409.00/
Comment 15 Matthew Miller 2014-10-07 11:18:05 EDT
(In reply to Ye Myat Kaung from comment #14)
> Do I still need to apply for "Bundled Library Exception" with FPC for
> voxelands, as well?

'Fraid so. Note the other ticket and exception when doing so.
Comment 16 Dennis Chen 2016-03-11 15:20:29 EST
I'll take this under review. I am under the impression that Voxelands no longer bundles jthread. Do you still need a sponsor?
Comment 17 Andrew Toskin 2017-03-06 17:02:25 EST
What's the status on this package?

I liked Minetest, but felt like it was sorely missing better-developed creeps, which Voxelands seems to have worked on. So I'd be interested in seeing it packaged for Fedora. I'm also looking to do package reviews, as part of joining the official packagers group, so I could help get this moving again, if you're still interested in maintaining it.
Comment 18 Matthew Miller 2017-03-06 17:21:47 EST
Note that Fedora rules around bundling have changed, possibly making this easier.

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