Bug 836840 - Review Request: gtkradiant - level design program for videogames
Summary: Review Request: gtkradiant - level design program for videogames
Keywords:
Status: CLOSED CANTFIX
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard: NotReady
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2012-07-01 22:13 UTC by Alberto Chiusole
Modified: 2013-10-19 14:42 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2013-08-03 15:47:07 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Alberto Chiusole 2012-07-01 22:13:54 UTC
Spec URL: https://www.dropbox.com/sh/rrgfwzc8bcjg74h/vnkCK3Ahof/source/gtkradiant.spec
SRPM URL: https://www.dropbox.com/sh/rrgfwzc8bcjg74h/5ahKh147GD/source/gtkradiant-1.6.3-1.fc16.src.rpm

Description:
Hello everybody! I tried to make an rpm of gtkradiant, this is my first rpm and I need a sponsor to help me.
I've also already done an rpm for fedora16, uploaded on my dropbox.
All stuff available here: https://www.dropbox.com/sh/rrgfwzc8bcjg74h/RxjiNIJ4_S

GtkRadiant is a powerful level editor used for various egoshooter games.
It stores level data in text format and uses compilers (q3map2 for Quake III for instance) to create the binary map files.

For more detailed information see
http://icculus.org/gtkradiant/

Fedora Account System Username: bebosudo

Comment 1 Alberto Chiusole 2012-07-03 15:57:31 UTC
I made some mistakes with the rpm's requires so I upload the new version of the spec here (https://www.dropbox.com/s/4b29fbxjr8uemtn/gtkradiant.spec), the new SRPM here (https://www.dropbox.com/s/tv04tx692r6sa44/gtkradiant-1.6.3-2.fc16.src.rpm) and here the RPM for f16 (https://www.dropbox.com/s/s6kde0k5snzal2p/gtkradiant-1.6.3-2.fc16.noarch.rpm)

Other related stuff available here (https://www.dropbox.com/sh/rrgfwzc8bcjg74h/RxjiNIJ4_S).

Repeat that I need a sponsor, thx for your patience.

Comment 2 Jason Tibbitts 2012-07-03 23:56:02 UTC
A couple of comments:

Using dropbox for your package hosting makes it difficult to get things.  For example, I don't get a valid source rpm when I do 
  wget https://www.dropbox.com/s/tv04tx692r6sa44/gtkradiant-1.6.3-2.fc16.src.rpm
Not sure how I'm supposed to download the package to my build host.

How on earth is your package noarch yet it requires a compiler?  And it requires all of those C headers and libraries yet the spec has an empty %build section.  I'm going to assume there's some C or C++ source that needs to be compiled, but I can't figure out where that happens.

Without being able to download the source package I can't say much more, but something looks pretty odd.

Comment 3 Alberto Chiusole 2012-07-04 22:25:47 UTC
Hi,
I uploaded all on my new github's account.
It's all available here: https://github.com/bebosudo/GtkRadiant_RPM

thank you again :)
bebo_sudo

Comment 4 Jason Tibbitts 2012-07-05 18:48:28 UTC
So, are you going to answer my questions about how this package gets built?

Comment 5 Alberto Chiusole 2012-07-05 20:27:26 UTC
Sure, first of all i have to say that i inspired my package on the "urbanterror" launcher on fedora, which at the first launch gets the data from the net and set up the game in the user's home.
My launcher takes data from the system folder (/usr/share/gtkradiant) and copies them in the .gtkradiant-src folder in the home, then it run the real set up with scons (scons target=radiant,q3map2 config=release). That's why the rpm needs so much dependencies.
I inserted a pair of lines in the spec to check the version of the data in the home:
touch $RPM_BUILD_ROOT%{_datadir}/%{name}/install/rpm_version
echo "%{version}-%{release}" > $RPM_BUILD_ROOT%{_datadir}/%{name}/install/rpm_version
Here there is the script: https://github.com/bebosudo/GtkRadiant_RPM/blob/master/source/gtkradiant.sh

I followed this guide (http://daffy.nerius.com/radiant/#compile) to make a guide on the Italian wiki (http://doc.fedoraonline.it/Installazione_e_Configurazione_GtkRadiant), where can be seen how the compiling process works.

Regards,
bebo_sudo

Comment 6 Jason Tibbitts 2012-08-10 22:41:13 UTC
I can say with some certainty that this package won't be approved if you insist on doing things that way.  This isn't gentoo.  Sorry.

Comment 7 Alberto Chiusole 2012-10-02 13:32:56 UTC
Ok, I followed your tip and I made some changes. Now the rpm didn't need anymore so much dependencies. I transferred all on sourceforge, you can find all here:
https://sourceforge.net/projects/gtkradiant-rpms/files/

To get the spec do this:
$ wget "https://sourceforge.net/projects/gtkradiant-rpms/files/sources/gtkradiant.spec/download" -O gtkradiant.spec

Cheers, bebo_sudo

Comment 8 Alberto Chiusole 2012-10-09 19:18:23 UTC
Hi all, 
I'm trying to build an updated version but rpmbuild exit with those results:
....[cut]
+ desktop-file-install --dir /home/makerpm/rpmbuild/BUILDROOT/gtkradiant-1.6-20121007.1.fc17.x86_64/usr/share/applications /home/makerpm/rpmbuild/SOURCES/gtkradiant.desktop
+ install -p -m 644 /home/makerpm/rpmbuild/SOURCES/gtkradiant.png /home/makerpm/rpmbuild/BUILDROOT/gtkradiant-1.6-20121007.1.fc17.x86_64/usr/share/icons/hicolor/128x128/apps
+ /usr/lib/rpm/check-rpaths /usr/lib/rpm/check-buildroot
*******************************************************************************
*
* WARNING: 'check-rpaths' detected a broken RPATH and will cause 'rpmbuild'
*          to fail. To ignore these errors, you can set the '$QA_RPATHS'
*          environment variable which is a bitmask allowing the values
*          below. The current value of QA_RPATHS is 0x0000.
*
*    0x0001 ... standard RPATHs (e.g. /usr/lib); such RPATHs are a minor
*               issue but are introducing redundant searchpaths without
*               providing a benefit. They can also cause errors in multilib
*               environments.
*    0x0002 ... invalid RPATHs; these are RPATHs which are neither absolute
*               nor relative filenames and can therefore be a SECURITY risk
*    0x0004 ... insecure RPATHs; these are relative RPATHs which are a
*               SECURITY risk
*    0x0008 ... the special '$ORIGIN' RPATHs are appearing after other
*               RPATHs; this is just a minor issue but usually unwanted
*    0x0010 ... the RPATH is empty; there is no reason for such RPATHs
*               and they cause unneeded work while loading libraries
*    0x0020 ... an RPATH references '..' of an absolute path; this will break
*               the functionality when the path before '..' is a symlink
*          
*
* Examples:
* - to ignore standard and empty RPATHs, execute 'rpmbuild' like
*   $ QA_RPATHS=$[ 0x0001|0x0010 ] rpmbuild my-package.src.rpm
* - to check existing files, set $RPM_BUILD_ROOT and execute check-rpaths like
*   $ RPM_BUILD_ROOT=<top-dir> /usr/lib/rpm/check-rpaths
*  
*******************************************************************************
ERROR   0004: file '/usr/share/gtkradiant/q3data' contains an insecure rpath '.' in [.]
ERROR   0004: file '/usr/share/gtkradiant/q3map2' contains an insecure rpath '.' in [.]
ERROR   0004: file '/usr/share/gtkradiant/modules/bobtoolz.so' contains an insecure rpath '.' in [.]
ERROR   0004: file '/usr/share/gtkradiant/modules/model.so' contains an insecure rpath '.' in [.]
ERROR   0004: file '/usr/share/gtkradiant/modules/ufoai.so' contains an insecure rpath '.' in [.]
ERROR   0004: file '/usr/share/gtkradiant/modules/bkgrnd2d.so' contains an insecure rpath '.' in [.]
ERROR   0004: file '/usr/share/gtkradiant/modules/camera.so' contains an insecure rpath '.' in [.]
ERROR   0004: file '/usr/share/gtkradiant/modules/surface_heretic2.so' contains an insecure rpath '.' in [.]
ERROR   0004: file '/usr/share/gtkradiant/modules/imagewal.so' contains an insecure rpath '.' in [.]
ERROR   0004: file '/usr/share/gtkradiant/modules/map.so' contains an insecure rpath '.' in [.]
ERROR   0004: file '/usr/share/gtkradiant/modules/vfswad.so' contains an insecure rpath '.' in [.]
ERROR   0004: file '/usr/share/gtkradiant/modules/prtview.so' contains an insecure rpath '.' in [.]
ERROR   0004: file '/usr/share/gtkradiant/modules/mapxml.so' contains an insecure rpath '.' in [.]
..[cut]..
ERROR   0004: file '/usr/share/gtkradiant/modules/vfsqlpk3.so' contains an insecure rpath '.' in [.]
ERROR   0004: file '/usr/share/gtkradiant/radiant.bin' contains an insecure rpath '.' in [.]
ERROR   0004: file '/usr/share/gtkradiant/q3map2_urt' contains an insecure rpath '.' in [.]
errore: Stato d'uscita errato da /var/tmp/rpm-tmp.RfZ5zk (%install)


Errori di compilazione RPM:
    Stato d'uscita errato da /var/tmp/rpm-tmp.RfZ5zk (%install)
$

how could I solve it?

Thank's to anyone.

Comment 9 Alberto Chiusole 2012-10-24 14:45:31 UTC
Solved those errors, i commented the %_arch_install_post lines in my ~/.rpmmacros.

I built the new version of gtkradiant (1.6-20121007 http://icculus.org/pipermail/gtkradiant/2012-October/011825.html) and uploaded it.

Here the spec: http://sourceforge.net/projects/gtkradiant-rpms/files/sources/gtkradiant.spec/download
And here the srpm: http://sourceforge.net/projects/gtkradiant-rpms/files/1.6-20121007/1.6-20121007.2/gtkradiant-1.6-20121007.2.fc17.src.rpm/download

Is there someone who can revise this package?

Cheers, thanks for all.
bebo_sudo

Comment 10 Alberto Chiusole 2012-11-06 21:53:46 UTC
Hi all,
a new version has been released so I changed a little the spec and rebuilt the srpm.

SRPM: http://sourceforge.net/projects/gtkradiant-rpms/files/1.6-20121007/1.6-20121007.3/gtkradiant-1.6-20121007.3.fc17.src.rpm/download

SPEC: you can get it via wget

$ wget "http://sourceforge.net/projects/gtkradiant-rpms/files/specs/gtkradiant_1.6-20121007.3.spec/download" -O gtkradiant.spec


Thank's again for your time,
regards.

Comment 11 Michael Schwendt 2012-12-09 01:03:27 UTC
Had a brief look at

http://sourceforge.net/projects/gtkradiant-rpms/files/specs/gtkradiant_1.6-20121007.3.spec/download

but that spec file makes no sense. I strongly recommend that you restart

  https://fedoraproject.org/wiki/Packaging:ReviewGuidelines

and comment in this ticket on as many MUST/SHOULD items as you consider relevant to your package.

Further reading:
https://fedoraproject.org/wiki/Category:Package_Maintainers

Comment 12 Alberto Chiusole 2012-12-09 16:42:26 UTC
Hi Michael,
meanwhile thanks for your reply.
Could you say to me the mistakes i did in my spec?
It's something about the libraries links?

I will review my package next weeks since now i don't have time to spend on it.

Best regards,
bebo_sudo

Comment 13 Hans de Goede 2013-03-13 15:13:09 UTC
Hi All,

Bebo has send a mail about this package to the Fedora-games mailinglist I think it is useful to reproduce my reply to his mail here:

I've taken a quick look at your gtkradiant packages, but as already indicated by both Jason and Michael in this bug,  your package as is, is no good. I'm sorry to say so, but it is no good at all.

You cannot create package for a FOSS distro such as Fedora by using precompiled binaries from upstream and putting those in an rpm.

If you look at other spec files in Fedora you will see they all start with the C source code, then compile and install this, ie:

%build
%configure
make

%install
make install DESTDIR=$RPM_BUILD_ROOT

Rather then using prebuild binaries, using prebuild binaries simple is not acceptable within Fedora.

So what you need to do is get the sources and use those as Source0, since the sources are on github, and they're not tagging releases just take a git-snapshot .zip file as Source0 by clicking on the zip button here:

https://github.com/TTimo/GtkRadiant

This means that your Source0 won't have a url and rpmlint will complain, but that is ok. Simple add a comment above the Source0 tag to explain where the sources come from.

But I see that gtkradiant also uses scons as a buildsystem, which is a pain to work with. Since you're new to packaging it would likely be better to choose a different package as your first Fedora packages.

Sorry I could not be more helpful.

Regards,

Hans

Comment 14 Alberto Chiusole 2013-03-25 22:55:40 UTC
Hi Hans, thanks anyway for your answer.

I revised my spec and now I think is better than before.

spec: http://sourceforge.net/projects/bebosudo-rpms/files/gtkradiant/F18/SRPM/gtkradiant.spec/download
srpm: http://sourceforge.net/projects/bebosudo-rpms/files/gtkradiant/F18/SRPM/gtkradiant-1.6.3-20121007.6.fc18.src.rpm/download

I've followed your advice and now the source url points at https://github.com/TTimo/GtkRadiant/tarball/master/TTimo-GtkRadiant-6afda55.tar.gz (or whatever value has the shortcommit variable). Now in the %build I use this (http://icculus.org/gtkradiant/documentation/windows_compile_guide/#sources) instructions to build the rpm.


The build is made through scons, but, and this is the first doubt, every time I start the rpmbuild process, scons seek and download the whole games archive from the svn server of the project: svn.icculus.org/gtkradiant-gamepacks/
I think isn't normal during the creation of an rpm leave download data from the internet. I think I should make an archive of files and patch the sources to avoid download things from the net, shouldn't I?
Look at the config.py: https://github.com/TTimo/GtkRadiant/blob/master/config.py#L274 Here is when the process downloads data from internet.

I tried to comment out the two functions to patch the source, but don't think I've done a very clear and complete job. This is my patched version of config.py: http://www.fpaste.org/pOCE/ .. I didn't try it, but could it work?

To create the archive containing the games data I suggest to do this:
 $ mkdir installs/ && cd installs/
 $ for game in {Q3,UrT,ET,QL}Pack; do mkdir $game; cd $game; svn checkout svn://svn.icculus.org/gtkradiant-gamepacks/${game}/trunk; mv trunk/* .; rm -r trunk/; cd ../; done; cd ..
 $ tar -czf gtkradiant-1.6-gamepacks-`date +%Y%m%d`.tar.gz installs/

and add the final tar.gz in the spec like a source, so in the %prep section I can untar it and put in the right place. Is this a possible (and right) way to solve the issue?

Another problem.. I discovered that the scons process builds, besides gtkradiant itself, two libraries, libpng and libjpeg. As described in the fp wiki, I shouldn't include bundled libraries in my rpm, so in my spec I removed them in the %install section, but I think I also should remove their creation, to speed up the build.
Here is what the upstream said about the embedded libraries: http://icculus.org/pipermail/gtkradiant/2012-September/011823.html -->
".. The slightly less common deps (png and jpeg libraries, some old version) are bundled in to make it simple."

How can I avoid the build of unneeded libraries? I think I should edit config.py, around this point (https://github.com/TTimo/GtkRadiant/blob/master/config.py#L251), but I think should be made a deep work on it, which I am not able to do.
I've also added the two libraries needed in the Requires tags (libpng12 and libjpeg-turbo), obtained with 'yum provides libpng.so.? libjpeg.so.62', and rpmlint complains these are explicit-lib-dependencies. Is this an issue or should I simply leave
Requires:	libpng.so.3
Requires:	libjpeg.so.62
?

I've also found two right images of gtkradiant, taken from the official server, and I've added them to sources.

During the build I've noticed rpmbuild would create strange folders in /usr/lib/debug/blabla.. and so I've added
%global debug_package	%{nil}
at the top of my spec. Is this ok?

Besides these problems I tried to install my package in a virtual machine but the problem, which I've already noticed to the upstream (http://icculus.org/pipermail/gtkradiant/2013-February/011841.html) still remains.
GtkRadiant would create a folder in the same folder as the launcher, but it doesn't have, rightly, the proper permissions. This a screenshot: http://www.pikky.net/uploads/3f2efd1024371f768dc0c40cce5bbb51872dc5d3.png

As Hans noticed, scons and in general all gtkradiant package, are a nasty beast for rpm.


Thanks for your attention,
Best wishes

Alberto "bebo"

Comment 15 Hans de Goede 2013-03-26 08:21:44 UTC
Hi Alberto,

(In reply to comment #14)
> Hi Hans, thanks anyway for your answer.

Cool to see that you're pursuing getting gtkradiant into Fedora!

> I revised my spec and now I think is better than before.

Yes much better, but still a lot of work todo.

> The build is made through scons, but, and this is the first doubt, every
> time I start the rpmbuild process, scons seek and download the whole games
> archive from the svn server of the project:
> svn.icculus.org/gtkradiant-gamepacks/
> I think isn't normal during the creation of an rpm leave download data from
> the internet. I think I should make an archive of files and patch the
> sources to avoid download things from the net, shouldn't I?

There are a number of problems here:

1) It should not download those gamepacks during the build

2) I wonder what the license on these gamepacks are, chances are that these
packs are not licensed under a FOSS license, in which case they should not be
packaged at all. Instead you can provide the user with a download script to
download them for the user + a README.fedora explaning this. Or you could
do a wrapper script around the gtkradiant binary which will ask the user if it
is ok to do the download automatically when the user starts gtkradiant for
the first time. Try "yum install vavoom" and then run doom-shareware as an
example of this.

3) If I'm wrong and these gamepacks are under a FOSS license they should
really be packaged in a separate gtkradiant-data.src.rpm

As for how to stop scons from doing the download, have you tried removing the
setup target from the scons invocation ?

> Another problem.. I discovered that the scons process builds, besides
> gtkradiant itself, two libraries, libpng and libjpeg. As described in the fp
> wiki, I shouldn't include bundled libraries in my rpm, so in my spec I
> removed them in the %install section, but I think I also should remove their
> creation, to speed up the build.

Yes this is the 2nd big problem you will need to solve, you should remove
these libraries from the extracted source tree in %prep to make sure that
the system version of the libraries and their headers is used during the
build.

You will then of course also need to fix any scons errors from when it tries
to build / install them anyways.

> Here is what the upstream said about the embedded libraries:
> http://icculus.org/pipermail/gtkradiant/2012-September/011823.html -->
> ".. The slightly less common deps (png and jpeg libraries, some old version)
> are bundled in to make it simple."
> 
> How can I avoid the build of unneeded libraries? I think I should edit
> config.py, around this point
> (https://github.com/TTimo/GtkRadiant/blob/master/config.py#L251),

Yes you will probably need to do something like that.

> think should be made a deep work on it, which I am not able to do.
> I've also added the two libraries needed in the Requires tags (libpng12 and
> libjpeg-turbo), obtained with 'yum provides libpng.so.? libjpeg.so.62', and
> rpmlint complains these are explicit-lib-dependencies. Is this an issue or
> should I simply leave
> Requires:	libpng.so.3
> Requires:	libjpeg.so.62

There is no need to add these Requires when you've build the binary against
the system copies of the libraries by removing the included copies in %prep
then rpm will pick up the Requires automatically.

> During the build I've noticed rpmbuild would create strange folders in
> /usr/lib/debug/blabla.. and so I've added
> %global debug_package	%{nil}
> at the top of my spec. Is this ok?

No, rpm will automatically create a gtkradiant-debuginfo sub-package where
these files will get added too, this can then later be used to debug problems
with gtkradiant. So you should NOT have a "%global debug_package	%{nil}"
in your specfile.

One last small thing I noticed, currently you have:
%global releaseno       6

And:

Release:        %{version2}.%{releaseno}%{?dist}

You've the releaseno and version2 the wrong way around, see:
http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Snapshot_packages

Also I would not use a global for releaseno, people expect to find (and modify if they change your package) the releaseno in the Release field, So I would just use:

Release:        6.%{version2}%{?dist}

Regards,

Hans

Comment 16 Alberto Chiusole 2013-08-03 15:37:47 UTC
Here I am again,
sorry for my late reply but i had to study hard.

Just after the answer of Hans I wrote to the ML of gtkradiant, asking about the gamepacks licences and here is the answer of TTimo:

	They are not under any kind of FOSS license. Each of them comes with the particular EULA pf the game SDK they were released with.
	
	TTimo

http://icculus.org/pipermail/gtkradiant/2013-April/011855.html

So we can't include these gamepacks in the official repos due to the licences of quake3 & co.

I thought.. could we create two separate packages, one containing only the engine of gtkradiant and one other containing the gamepacks. The first should be included in the base repos of fedora, and the second in the rpmfusion free or non-free repos. Could it be a good idea?


Anyway thanks to Hans for the advices, I will take note of them.

Have a nice day,
Alberto

Comment 17 Hans de Goede 2013-08-03 15:47:07 UTC
Hi,

Thanks for your continued interest in this, and for figuring out the licensing. Fedora does not allow applications which can only work when combined with non-free data, which seems to be the case here. So it is best to just submit gtkradiant to rpmfusion in its entirety.

Regards,

Hans

Comment 18 Alberto Chiusole 2013-08-03 15:57:02 UTC
Ok, thanks Hans for your patience.
I'll open a bug on the rpmfusion bugzilla.

Greetings,

Alberto


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