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: 2016-08-06 19:11 EDT (History)
6 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:
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?

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