Bug 802050 - Review request: love - A free 2D game engine which enables easy game creation in Lua
Summary: Review request: love - A free 2D game engine which enables easy game creation...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: 16
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Gregor Tätzner
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2012-03-10 14:49 UTC by Jeremy Newton
Modified: 2012-04-14 23:22 UTC (History)
7 users (show)

Fixed In Version: love-0.8.0-2.fc16
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-04-12 03:05:55 UTC
Type: ---
Embargoed:
gregor: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Jeremy Newton 2012-03-10 14:49:22 UTC
LOVE is an open source, cross platform 2D game engine which uses the
Lua scripting language. LOVE can be used to make games of any license
allowing it to be used for both free and non-free projects.

Note this uses zlib licensing, excluding one file that is public domain. Also, this is my 2nd Fedora package (3rd spec submitted to fedora) and I do require sponsorship. I am currently sponsored in RPM Fusion, so I am familiar with most of the rules and methods of packaging for Fedora.

SPEC:
http://dl.dropbox.com/u/42480493/love.spec
SRPM:
http://dl.dropbox.com/u/42480493/love-0.7.2-1.fc16.src.rpm

RPMLINIT OUPUT:

>love.x86_64: W: no-manual-page-for-binary love

This isn't critical, so ignored it. I can write a man page and send it upstream if needed.

>love.src: W: invalid-url Source1: love-extra.tar.xz

This source does not have a download link because I created it; a link above
the source provides the source for the files I included (from Ubuntu's Debian files). I can provide a script
to generate this if necessary.

>3 packages and 0 specfiles checked; 0 errors, 2 warnings.

Comment 1 Gregor Tätzner 2012-03-27 16:57:53 UTC
I'm going to take this review and help you out. If you package gets approved rdieter will sponsor you (I can't do that).

Comment 2 Gregor Tätzner 2012-03-27 17:56:06 UTC
just a quick shot:

- your package doesnt build in mock
->add desktop-file-utils and libtool as BuildRequires

- place your sed magic in the %install section :)

- please explain the optional patch. Why did you include it? Why isn't it in upstream?

(In reply to comment #0)
> >love.x86_64: W: no-manual-page-for-binary love
> 
> This isn't critical, so ignored it. I can write a man page and send it upstream
> if needed.
yeah non critical, but it would be really *cool* if you can do it :)

> >love.src: W: invalid-url Source1: love-extra.tar.xz
> 
> This source does not have a download link because I created it; a link above
> the source provides the source for the files I included (from Ubuntu's Debian
> files). I can provide a script
> to generate this if necessary.
is there some reason why you didn't send this to upstream?

Comment 3 Jeremy Newton 2012-03-28 00:08:18 UTC
(In reply to comment #1)
> I'm going to take this review and help you out. If you package gets approved
> rdieter will sponsor you (I can't do that).

Awesome, thanks!

(In reply to comment #2)
> just a quick shot:
> 
> - your package doesnt build in mock
> ->add desktop-file-utils and libtool as BuildRequires

Ah, that's what I was missing, I knew I forgot something

> - place your sed magic in the %install section :)

As in the line that says: 
sed -i 's/\r//' license.txt readme.txt
In the install section? seems a little odd, shouldn't this be in the prep because its the equivalent of a patch?
 
> - please explain the optional patch. Why did you include it? Why isn't it in
> upstream?

Good point, I should ask upstream to include this.

> (In reply to comment #0)
> > >love.x86_64: W: no-manual-page-for-binary love
> > 
> > This isn't critical, so ignored it. I can write a man page and send it upstream
> > if needed.
> yeah non critical, but it would be really *cool* if you can do it :)

Haha yeah I'm planning to; I need to make one for a package I have in RPMFusion, so I will when I have some time to figure out how to make them.

> > >love.src: W: invalid-url Source1: love-extra.tar.xz
> > 
> > This source does not have a download link because I created it; a link above
> > the source provides the source for the files I included (from Ubuntu's Debian
> > files). I can provide a script
> > to generate this if necessary.
> is there some reason why you didn't send this to upstream?

Good point, this is another thing I can send upstream. Although knowing the current development, it probably won't be incorporated until 0.8 comes out.

Comment 4 Rex Dieter 2012-03-28 00:44:21 UTC
Re: the sed'ing

I'd have to agree with Jeremy, that this is akin to patching, and really does best belong in %prep

Comment 5 Jeremy Newton 2012-03-28 01:49:20 UTC
Alright, I added the missing deps and tested in mock; it builds just fine.

Here's the new files:

SPEC:
http://dl.dropbox.com/u/42480493/love.spec
SRPM:
http://dl.dropbox.com/u/42480493/love-0.7.2-2.fc16.src.rpm

Comment 6 Gregor Tätzner 2012-03-28 09:05:15 UTC
(In reply to comment #3)
> Good point, this is another thing I can send upstream. Although knowing the
> current development, it probably won't be incorporated until 0.8 comes out.

Good enough. Please keep informing us about the progress. I think there is some guideline about upstreaming patches. here it is http://fedoraproject.org/wiki/Staying_close_to_upstream_projects

round 2:

-please use the %name macro whenever possible throughout the spec file i.e. in the top section

-"make %{?_smp_mflags} install DESTDIR=%{buildroot}"
smp_mflags macro seems to be unnecessary since here no compilation is involved

-there is a typo in changelog 2

Comment 7 Jeremy Newton 2012-03-29 02:02:50 UTC
(In reply to comment #6)
> (In reply to comment #3)
> > Good point, this is another thing I can send upstream. Although knowing the
> > current development, it probably won't be incorporated until 0.8 comes out.
> 
> Good enough. Please keep informing us about the progress. I think there is some
> guideline about upstreaming patches. here it is
> http://fedoraproject.org/wiki/Staying_close_to_upstream_projects

Yep, I included the bug reports right into the new spec :)

> round 2:
> 
> -please use the %name macro whenever possible throughout the spec file i.e. in
> the top section
> 
> -"make %{?_smp_mflags} install DESTDIR=%{buildroot}"
> smp_mflags macro seems to be unnecessary since here no compilation is involved
> 
> -there is a typo in changelog 2

Done! Here's the new files and I also made a man page, which I sent upstream (issue 405, see spec):

SPEC:
http://dl.dropbox.com/u/42480493/love.spec
SRPM:
http://dl.dropbox.com/u/42480493/love-0.7.2-3.fc16.src.rpm

Comment 8 Gregor Tätzner 2012-03-29 16:33:16 UTC
you are magnificent! If our nearly invisible observer rdieter doesn't spot any issues I will do a formal review tomorrow.

Comment 9 Martin Airs 2012-03-29 16:45:27 UTC
I have tried to rpmbuild --rebuild love-0.7.2-3.fc16.src.rpm on F16 x86_64 and it fails with...

checking for library containing glLoadIdentity... no
configure: error: Can't LÖVE without OpenGL

I have installed mesa-libGL-devel and a ton of other -devel files in hopes of finding one that satisfies this.

which one am I missing??

Martin

Comment 10 Gregor Tätzner 2012-03-29 16:53:53 UTC
(In reply to comment #9)
> I have tried to rpmbuild --rebuild love-0.7.2-3.fc16.src.rpm on F16 x86_64 and
> it fails with...
> 
> checking for library containing glLoadIdentity... no
> configure: error: Can't LÖVE without OpenGL
> 
> I have installed mesa-libGL-devel and a ton of other -devel files in hopes of
> finding one that satisfies this.
> 
> which one am I missing??
> 
> Martin

try to run yum-builddep on the spec file

Comment 11 Martin Airs 2012-03-29 16:56:21 UTC
Getting requirements for ./love.spec
 --> Already installed : DevIL-devel-1.7.8-6.fc16.x86_64
 --> Already installed : libtiff-devel-3.9.5-1.fc16.x86_64
 --> Already installed : freetype-devel-2.4.6-4.fc16.x86_64
 --> Already installed : lua-devel-5.1.4-9.fc16.x86_64
 --> Already installed : physfs-devel-1.0.2-3.fc15.x86_64
 --> Already installed : SDL-devel-1.2.14-13.fc16.x86_64
 --> Already installed : openal-soft-devel-1.12.854-2.fc15.x86_64
 --> Already installed : 2:libogg-devel-1.2.2-3.fc15.x86_64
 --> Already installed : 1:libvorbis-devel-1.3.3-1.fc16.x86_64
 --> Already installed : flac-devel-1.2.1-6.fc12.x86_64
 --> Already installed : 1:libmodplug-devel-0.8.8.4-1.fc16.x86_64
 --> Already installed : libmng-devel-1.0.10-5.fc15.x86_64
 --> Already installed : desktop-file-utils-0.18-4.fc16.x86_64
 --> Already installed : libtool-2.4-9.fc16.x86_64
No uninstalled build requires

seems ok??

Comment 12 Gwyn Ciesla 2012-03-29 17:25:53 UTC
Don't set the cvs flag until the review is approved, thanks!

Comment 13 Martin Airs 2012-03-29 17:34:37 UTC
hmm ok, well it built in koji ok, http://koji.fedoraproject.org/koji/taskinfo?taskID=3943925

and I now have it installed, the funny thing is, the reason for all of this was to run something that I now discover requires 0.8.0 or higher, lol :)

Comment 14 Rex Dieter 2012-03-29 17:40:34 UTC
re: comment #12
oops, meant to reset the fedora-review flag, and botched it.  sorry.

Comment 15 Jeremy Newton 2012-03-29 18:19:06 UTC
(In reply to comment #13)
> hmm ok, well it built in koji ok,
> http://koji.fedoraproject.org/koji/taskinfo?taskID=3943925

I take it must be an issue with your system, as I've tested it in mock (i686 and x64). Let me know if you have an other troubles with this later on

> and I now have it installed, the funny thing is, the reason for all of this was
> to run something that I now discover requires 0.8.0 or higher, lol :)

I take you're trying to run mari0? I can make an unstable version too if there's interest, since I sort of have interest in using the unstable myself.

(In reply to comment #8)
> you are magnificent! If our nearly invisible observer rdieter doesn't spot any
> issues I will do a formal review tomorrow.

Thanks! Though I noticed I must be half asleep when I write these change logs haha, I'll fix it and re-upload it ;)

Comment 16 Martin Airs 2012-03-29 22:45:13 UTC
(In reply to comment #15)
> I take you're trying to run mari0? I can make an unstable version too if
> there's interest, since I sort of have interest in using the unstable myself.

Indeed I am, there's someone else trying to run mari0 too on the games mailing list.

Thanks
Martin

Comment 17 Jeremy Newton 2012-03-30 15:50:46 UTC
(In reply to comment #16)
> (In reply to comment #15)
> > I take you're trying to run mari0? I can make an unstable version too if
> > there's interest, since I sort of have interest in using the unstable myself.
> 
> Indeed I am, there's someone else trying to run mari0 too on the games mailing
> list.
> 
> Thanks
> Martin

Alright, I'll post another review request for love unstable

Comment 18 Gregor Tätzner 2012-03-30 17:52:49 UTC
Package Review
==============

Key:
- = N/A
x = Pass
! = Fail
? = Not evaluated



==== C/C++ ====
[x]: MUST Package does not contain any libtool archives (.la)
[x]: MUST Package does not contain kernel modules.
[x]: MUST Package contains no static executables.
[x]: MUST Rpath absent or only used for internal libs.
[x]: MUST Package is not relocatable.


==== Generic ====
[x]: MUST Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: MUST Package successfully compiles and builds into binary rpms on at
     least one supported primary architecture.
[x]: MUST %build honors applicable compiler flags or justifies otherwise.
[x]: MUST All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
[x]: MUST Buildroot is not present
     Note: Unless packager wants to package for EPEL5 this is fine
[x]: MUST Package contains no bundled libraries.
[x]: MUST Changelog in prescribed format.
[x]: MUST Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
     Note: Clean would be needed if support for EPEL is required
[x]: MUST Sources contain only permissible code or content.
[x]: MUST Each %files section contains %defattr if rpm < 4.4
     Note: Note: defattr macros not found. They would be needed for EPEL5
[x]: MUST Macros in Summary, %description expandable at SRPM build time.
[x]: MUST Package requires other packages for directories it uses.
[x]: MUST Package uses nothing in %doc for runtime.
[x]: MUST Package is not known to require ExcludeArch.
[x]: MUST Permissions on files are set properly.
[x]: MUST Package does not contain duplicates in %files.
[x]: MUST Spec file lacks Packager, Vendor, PreReq tags.
[x]: MUST Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
     Note: rm -rf would be needed if support for EPEL5 is required
[x]: MUST Large documentation files are in a -doc subpackage, if required.
[x]: MUST If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %doc.
[x]: MUST License field in the package spec file matches the actual license.
[x]: MUST Package consistently uses macros (instead of hard-coded directory
     names).
[x]: MUST Package is named according to the Package Naming Guidelines.
[x]: MUST Package does not generate any conflict.
[x]: MUST Package obeys FHS, except libexecdir and /usr/target.
[x]: MUST Package must own all directories that it creates.
[x]: MUST Package does not own files or directories owned by other packages.
[x]: MUST Package installs properly.
[x]: MUST Requires correct, justified where necessary.
[x]: MUST Rpmlint output is silent.
[x]: MUST Sources used to build the package match the upstream source, as
     provided in the spec URL.
/home/greg/projects/Review/802050/love-0.7.2-linux-src.tar.gz :
  MD5SUM this package     : c3e678606bb9a870c31168e85b269e7e
  MD5SUM upstream package : c3e678606bb9a870c31168e85b269e7e

[x]: MUST Spec file is legible and written in American English.
[x]: MUST Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[ ]: MUST Package contains a SysV-style init script if in need of one.
[x]: MUST File names are valid UTF-8.
[x]: MUST Useful -debuginfo package or justification otherwise.
[x]: SHOULD Reviewer should test that the package builds in mock.

Generated by fedora-review 0.2.0git
External plugins:

>>>>>APPROVED<<<<<<
well done

minor issue:
- before you check in please pass %{?_smp_mflags} to the make command
i.e. make %{?_smp_mflags}

don't remove it...we want a fast build, don't we? :)

Comment 19 Gregor Tätzner 2012-03-30 17:58:57 UTC
(In reply to comment #17)
> (In reply to comment #16)
> > (In reply to comment #15)
> > > I take you're trying to run mari0? I can make an unstable version too if
> > > there's interest, since I sort of have interest in using the unstable myself.
> > 
> > Indeed I am, there's someone else trying to run mari0 too on the games mailing
> > list.
> > 
> > Thanks
> > Martin
> 
> Alright, I'll post another review request for love unstable

If you don't mind a suggestion: Don't put a unstable package into the official repo. It's not discouraged or something like that but imho it's not worth the trouble to go through the review process. When you get sponsored you get access to fedorapeople webspace. There you could host your "unofficial" unstable package and prepare it for the next stable release.

just my 2 cents ;)

Comment 20 Rex Dieter 2012-03-30 18:12:09 UTC
Jeremy, if you could share your Fedora Account System (FAS) username, I can get you sponsored into the packager group

Comment 21 Jeremy Newton 2012-03-30 20:57:20 UTC
(In reply to comment #18)
> minor issue:
> - before you check in please pass %{?_smp_mflags} to the make command
> i.e. make %{?_smp_mflags}
> 
> don't remove it...we want a fast build, don't we? :)

Sure, will do. :)

(In reply to comment #19)
> If you don't mind a suggestion: Don't put a unstable package into the official
> repo. It's not discouraged or something like that but imho it's not worth the
> trouble to go through the review process. When you get sponsored you get access
> to fedorapeople webspace. There you could host your "unofficial" unstable
> package and prepare it for the next stable release.
> 
> just my 2 cents ;)

You're right, that does make more sense, I'll look into making a wiki page on https://fedoraproject.org/wiki/Fedora_Project_Wiki

(In reply to comment #20)
> Jeremy, if you could share your Fedora Account System (FAS) username, I can get
> you sponsored into the packager group

My FAS username is mystro256; is there anything else I need to do? Such as join a group on FAS that I'm not already in? Also should I post a CVS request or do you do that?

Comment 22 Jeremy Newton 2012-03-30 21:23:36 UTC
(In reply to comment #16)
> (In reply to comment #15)
> > I take you're trying to run mari0? I can make an unstable version too if
> > there's interest, since I sort of have interest in using the unstable myself.
> 
> Indeed I am, there's someone else trying to run mari0 too on the games mailing
> list.
> 
> Thanks
> Martin

I'll get the unstable up and going as soon as I can; I made a wiki page to post information on love in regards to Fedora. Please check up on it in a few days to grab the unstable build: https://fedoraproject.org/wiki/L%C3%96VE/

Comment 23 Jeremy Newton 2012-03-30 21:24:32 UTC
Sorry the link should be https://fedoraproject.org/wiki/L%C3%96VE

Comment 24 Rex Dieter 2012-03-31 03:33:05 UTC
sponsored, next step is for you to request SCM access:

http://fedoraproject.org/wiki/PackageMaintainers/Join#Add_Package_to_Source_Code_Management_.28SCM.29_system_and_Set_Owner

Comment 25 Jeremy Newton 2012-04-04 01:27:23 UTC
New Package SCM Request
=======================
Package Name: love
Short Description: A free 2D game engine which enables easy game creation in Lua
Owners: mystro256
Branches: F15 F16 F17 devel
InitialCC:

Comment 26 Gwyn Ciesla 2012-04-04 02:02:22 UTC
Git done (by process-git-requests).

Comment 27 Jeremy Newton 2012-04-04 03:23:39 UTC
It seems version 0.8.0 was tagged yesterday, so I'm going to import 0.8.0 as soon as I confirm this is a new sample release and not a mistake.

Though at the moment, it seems like I won't need to make an unstable after all.

Anyway, for Martin Airs and anyone who is interested in rebuilding in the mean time, here's the Updated SRPM:
http://dl.dropbox.com/u/42480493/love-0.8.0-1.fc16.src.rpm

(In reply to comment #26)
> Git done (by process-git-requests).

Thanks :)

Comment 28 Fedora Update System 2012-04-04 04:08:46 UTC
love-0.8.0-1.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/love-0.8.0-1.fc16

Comment 29 Fedora Update System 2012-04-04 04:11:32 UTC
love-0.8.0-1.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/love-0.8.0-1.fc17

Comment 30 Martin Airs 2012-04-04 17:06:55 UTC
Thanks Jeremy, love-0.8.0 installed and mari0 running sweet.

Martin

Comment 31 Jeremy Newton 2012-04-04 17:37:01 UTC
(In reply to comment #30)
> Thanks Jeremy, love-0.8.0 installed and mari0 running sweet.
> 
> Martin

No problem!

But please give my packages some Karma if you have time: https://admin.fedoraproject.org/updates/love?_csrf_token=a283ef2bc3a259b342ebdbe0f3b1da891c1412f7

Comment 32 Fedora Update System 2012-04-04 19:52:04 UTC
love-0.8.0-2.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/love-0.8.0-2.fc17

Comment 33 Fedora Update System 2012-04-04 19:53:52 UTC
love-0.8.0-2.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/love-0.8.0-2.fc16

Comment 34 Fedora Update System 2012-04-04 19:54:50 UTC
love-0.8.0-2.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/love-0.8.0-2.fc15

Comment 35 Fedora Update System 2012-04-05 18:24:24 UTC
love-0.8.0-1.fc17 has been pushed to the Fedora 17 testing repository.

Comment 36 Fedora Update System 2012-04-12 03:05:55 UTC
love-0.8.0-2.fc17 has been pushed to the Fedora 17 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 37 Fedora Update System 2012-04-14 23:21:50 UTC
love-0.8.0-2.fc15 has been pushed to the Fedora 15 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 38 Fedora Update System 2012-04-14 23:22:04 UTC
love-0.8.0-2.fc16 has been pushed to the Fedora 16 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.