Spec URL: http://arand.fedorapeople.org/redeclipse.spec SRPM URL: http://arand.fedorapeople.org/redeclipse-1.2-1.fc17.src.rpm (mind, ~0.5G) Description: Red Eclipse is an actively developed FPS game based on the cube2 engine (of Sauerbraten fame). One of the main developers of sauerbraten are the co-main-developer of RE. Red Eclipse is currently packaged for Debian (by me), and I figured it would be nice to package it for Fedora as well. Some comments and questions: 1. Enet 1.3 is not yet in Fedora, I don't intent to upload RE before it is, however I wanted to have the package reviewed still so that other issues may be fixed in the meantime. 2. I am using my own debian packaging (metatdata) tarballs as Source#s both for the exhaustive copyright info, and for patches. Is this ok? should I be separating those files out as Source#s and Patch#s instead? 3. I am ignoring some rpmlint spelling errors in the Summary and description. How important is spelling vs common usage in this case? Thanks.
The rpmlint spell warnings (see 3. above) are, for reference: [arand@fed rpmbuild]$ rpmlint redeclipse redeclipse-data redeclipse-debuginfo cube2font redeclipse.x86_64: W: spelling-error Summary(en_US) Multiplayer -> Multiplier, Multiplexer redeclipse.x86_64: W: spelling-error %description -l en_US multi -> mulch, mufti redeclipse.x86_64: W: spelling-error %description -l en_US gameplay -> game play, game-play, nameplate redeclipse.x86_64: W: spelling-error %description -l en_US gamemodes -> game modes, game-modes, gametes redeclipse.x86_64: W: spelling-error %description -l en_US mutators -> commutators, mutilators, commutator 4 packages and 0 specfiles checked; 0 errors, 5 warnings. I've also done scratch builds on f17 and f18 with koji, see http://koji.fedoraproject.org/koji/tasks?owner=arand&state=all (They seem to have gone through without a hitch).
Er... running it on the srpms as well: redeclipse.src: W: strange-permission generate-tarball.sh 0775L redeclipse.src:13: W: macro-in-comment %{name} redeclipse.src:13: W: macro-in-comment %{name} redeclipse.src:13: W: macro-in-comment %{version} redeclipse.src: W: invalid-url Source0: redeclipse-1.2-fs.tar.xz I've tried to make the repackaging clear by keeping a Source# string as it would be if not repacking, and the generate script should be executable, no?
Great game this one. Use %%{name} in your comments to get rid of those errors. Remove the executable permissions from your flags. USually you put a comment abouce the script "Sources obtained by "sh redeclipse-generate-tarball.sh <revison>" You should be using upstream sources and patches, or svn/git snapshot if that is more appropriate see http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Package_Versioning
Hello, and thanks for reviewing! :) I'm in the process of re-uploading the files, but the srpm will obviously take a while... Changed: 1. Macros in the #Source0 comment to "%%" 2. -x and renamed generate-tarball.sh -> redeclipse-generate-tarball.sh 3. Added redeclipse-server package, in order that the server may be installed without pulling in all of redeclipse-data (which are not required for the server). > You should be using upstream sources and patches, or svn/git snapshot if that > is more appropriate see > http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Package_Versioning I'm not quite sure I follow you here, are you saying some of these patches are inappropriate, if so, which? Or are you saying the patches should be separated out and not extracted from the debian sources? I figured it was the right thing to do since these are patches that won't get applied upstream (I have asked), and since I wrote the patches for Debian initially, it is the upstream for these patches, no? The icon-fix patch is an exception, that one is applied upstream, the header should indicate that, should I be including this a a separate Patch# instead? (I was pulling it in since it already exists in the debian sources) I am not using svn snapshots since the SVN version will become (is?) incompatible with the released version. Maybe there would be reason to do a redeclipse-svn package sometime, but at the moment I do not think there is. Thanks.
(In reply to comment #4) > Hello, and thanks for reviewing! :) Hi, as this is your first package I can't sponsor you but I can at least help you get it into shape before someone does. > > I'm in the process of re-uploading the files, but the srpm will obviously take > a while... Don't upload a new SRPM until you've got your sources correct - its too big. Just list them here for now. > Or are you saying the patches should be separated out and not extracted from > the debian sources? I figured it was the right thing to do since these are > patches that won't get applied upstream (I have asked), and since I wrote the > patches for Debian initially, it is the upstream for these patches, no? Correct, just include the unedited source tarball along with the required patches as diffs (%Patch0, %Patch1 .. etc). Part of the review process is to checksum the upstream tarball against what is in the SRPM. Also, don't include any of the debian license files. You also shouldn't be building any bundled libraries (enet for example), you'll have to remove those sources in your %prep section and ad a BuildRequires: enet >= 1.3. I've added a blocker on the bug you've raised for this. > > The icon-fix patch is an exception, that one is applied upstream, the header > should indicate that, should I be including this a a separate Patch# instead? > (I was pulling it in since it already exists in the debian sources) Your patches should be split as you deem appropriate. This is a good candidate for a separate patch. If upstream fix/apply something in a subsequent release it is much easier to remove a single %patch, than it is to selectively edit/recreate a large one. > I am not using svn snapshots since the SVN version will become (is?) > incompatible with the released version. Maybe there would be reason to do a > redeclipse-svn package sometime, but at the moment I do not think there is. No problem I'll have a closer look at your spec tomorrow.
Spec URL: http://arand.fedorapeople.org/2/redeclipse.spec (In reply to comment #5) > (In reply to comment #4) > (...) > > Or are you saying the patches should be separated out and not extracted from > > the debian sources? I figured it was the right thing to do since these are > > patches that won't get applied upstream (I have asked), and since I wrote the > > patches for Debian initially, it is the upstream for these patches, no? > > Correct, just include the unedited source tarball along with the required > patches as diffs (%Patch0, %Patch1 .. etc). Part of the review process is to > checksum the upstream tarball against what is in the SRPM. Also, don't include > any of the debian license files. Upstream tarball has embedded libs without source code (sdl, freetype, ...), hence I take it repacking is required, And I'm taking the opportunity to remove the associated headers for these libs (no need to document a slew of copyrights), along with the osx/win-specific content. Should any of this be left alone instead? https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Multiple_Licensing_Scenarios : "Since this is a multiple licensing scenario, the package must contain a comment explaining the multiple licensing breakdown. The actual implementation of this is left to the maintainer." Since the license breakdown is humongous, I consider using the Debian copyright files are my best bet. > > You also shouldn't be building any bundled libraries (enet for example), you'll > have to remove those sources in your %prep section and ad a BuildRequires: enet > >= 1.3. I've added a blocker on the bug you've raised for this. Yeah, I'm keeping it for the time being though in order to test builds. Hmm, maybe it should be the Enet bug at https://bugzilla.redhat.com/show_bug.cgi?id=739313 ..blocking egoboo and redeclipse individually? > > > > The icon-fix patch is an exception, that one is applied upstream, the header > > should indicate that, should I be including this a a separate Patch# instead? > > (I was pulling it in since it already exists in the debian sources) > > Your patches should be split as you deem appropriate. This is a good candidate > for a separate patch. If upstream fix/apply something in a subsequent release > it is much easier to remove a single %patch, than it is to selectively > edit/recreate a large one. But the only thing that would need removing in my case is the %prep line patch -p1 < debian/patches/backported-fix-icon-sizes.patch ..so it would be really simple to remove it, no?
(In reply to comment #6) > > Upstream tarball has embedded libs without source code (sdl, freetype, ...), > hence I take it repacking is required, And I'm taking the opportunity to remove > the associated headers for these libs (no need to document a slew of > copyrights), along with the osx/win-specific content. > Should any of this be left alone instead? Just remove the offending libraries / directories in the %prep section. No need to remove the osx/win stuff as long as you don't build against it, nor include any of the files in your %file section > > https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Multiple_Licensing_Scenarios > : > "Since this is a multiple licensing scenario, the package must contain a > comment explaining the multiple licensing breakdown. The actual implementation > of this is left to the maintainer." > > Since the license breakdown is humongous, I consider using the Debian copyright > files are my best bet. You need to include all of the upstream license files with a summary. Because the content licenses are different in the data subpackage you can add a separate License tag for this sub-package. You can't include the Debian file as a license as they are not upstream and have no authority here. > > > > > You also shouldn't be building any bundled libraries (enet for example), you'll > > have to remove those sources in your %prep section and ad a BuildRequires: enet > > >= 1.3. I've added a blocker on the bug you've raised for this. > > Yeah, I'm keeping it for the time being though in order to test builds. > > Hmm, maybe it should be the Enet bug at > https://bugzilla.redhat.com/show_bug.cgi?id=739313 > ..blocking egoboo and redeclipse individually? Sure, change blockers/depends as you see fit. > But the only thing that would need removing in my case is the %prep line > patch -p1 < debian/patches/backported-fix-icon-sizes.patch > ..so it would be really simple to remove it, no? Right, but you would have Patch0: redeclipse-1.2-fix-icon-sizes.patch and in you %prep section: %patch0 -p1 for example. Do this for all of your patches.
(In reply to comment #7) > > Right, but you would have Patch0: redeclipse-1.2-fix-icon-sizes.patch > Patch0: %{name}-%{version}-fix-icon-sizes.patch sorry, is better > and in you %prep section: > > %patch0 -p1 > > for example. Do this for all of your patches.
(In reply to comment #7) > (In reply to comment #6) > > > > Upstream tarball has embedded libs without source code (sdl, freetype, ...), > > hence I take it repacking is required, And I'm taking the opportunity to remove > > the associated headers for these libs (no need to document a slew of > > copyrights), along with the osx/win-specific content. > > Should any of this be left alone instead? > > Just remove the offending libraries / directories in the %prep section. No need > to remove the osx/win stuff as long as you don't build against it, nor include > any of the files in your %file section Ah, I assumed the srpms was required to be "clean" and "open-source" (in addition to distributable).. Since not, then I agree, no repack needed. > > > > https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Multiple_Licensing_Scenarios > > : > > "Since this is a multiple licensing scenario, the package must contain a > > comment explaining the multiple licensing breakdown. The actual implementation > > of this is left to the maintainer." > > > > Since the license breakdown is humongous, I consider using the Debian copyright > > files are my best bet. > > You need to include all of the upstream license files with a summary. Because > the content licenses are different in the data subpackage you can add a > separate License tag for this sub-package. You can't include the Debian file as > a license as they are not upstream and have no authority here. So in effect, you are asking me to disregard the work already done by me in Debian to create a clear license breakdown, and to rewrite this information in a crappy non-standardised format? I'll see if I can commit this info upstream and and pull that commit blob in as a patch instead. That would make the information "authoritative", right? I've switched to using Patch#s and skipped the debug flag patch, instead using CXXFLAGS+=-g in the make invocation. Latest spec URL: http://arand.fedorapeople.org/3/redeclipse.spec
(In reply to comment #9) > > > > You need to include all of the upstream license files with a summary. Because > > the content licenses are different in the data subpackage you can add a > > separate License tag for this sub-package. You can't include the Debian file as > > a license as they are not upstream and have no authority here. > > So in effect, you are asking me to disregard the work already done by me in > Debian to create a clear license breakdown, and to rewrite this information in > a crappy non-standardised format? Here's the guidelines regarding the situation and your options. http://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Multiple_Licensing_Scenarios Apologies, I stand corrected, you can include a file in the %doc section which outlines the breakdown, but you don't need to pull in a debian tarball. Just copy the breakdown into a new file, removing debian references, and include it as a Source and move it into %doc in our install section. > > I'll see if I can commit this info upstream and and pull that commit blob in as > a patch instead. That would make the information "authoritative", right? > > > I've switched to using Patch#s and skipped the debug flag patch, instead using > CXXFLAGS+=-g in the make invocation. You should be using CXXFLAGS=%{optflags} http://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags IF this still results in the use of non standard compiler flags, you may need to sed/patch the makefile in you %prep section. > > Latest spec URL: http://arand.fedorapeople.org/3/redeclipse.spec
(In reply to comment #10) > (In reply to comment #9) > (...) > Apologies, I stand corrected, you can include a file in the %doc section which > outlines the breakdown, but you don't need to pull in a debian tarball. Just > copy the breakdown into a new file, removing debian references, and include it > as a Source and move it into %doc in our install section. Done > (..) > You should be using CXXFLAGS=%{optflags} > http://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags > > IF this still results in the use of non standard compiler flags, you may need > to sed/patch the makefile in you %prep section. Done Updated both: spec URL: http://arand.fedorapeople.org/4/redeclipse.spec srpm URL: http://arand.fedorapeople.org/4/redeclipse-1.2-1.fc17.src.rpm
You need a %doc entry in the %files section of the main package. Read this: http://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Subpackage_Licensing In the license.txt upstream has been very clear about the source and the content, your comments before your all license source is enough as long as you make sure the sub-packages require the main package. And seriously, lose all of the references to debian for your patches unless you are referring to a specific bug report. The why is much more important than the where. Better yet, refer to upstreams response to said bug report (I understand the debian maintainer is upstream). Whatever is going to help maintain the package longterm
(In reply to comment #12) > You need a %doc entry in the %files section of the main package. Read this: > http://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Subpackage_Licensing > Ah, right, but I will then need to duplicate most of the license info in the -data package since it shouldn't depend on the engine package, right? > In the license.txt upstream has been very clear about the source and the > content, your comments before your all license source is enough as long as you > make sure the sub-packages require the main package. If this is enough, I'd be happy to remove the all-licenses file, however, I get the impression that (paraphrasing license.txt) "There are a whole bunch of individual licenses (some custom-written), dig through the subfolders in order to find them" ..is not really good enough, or is it? Note that there are a few home-made licenses there as well, my comments in the spec file only cover those which have a known shortname in Fedora. > > And seriously, lose all of the references to debian for your patches unless you > are referring to a specific bug report. The why is much more important than the > where. Better yet, refer to upstreams response to said bug report (I understand > the debian maintainer is upstream). Whatever is going to help maintain the > package longterm The Debian maintainer is me, who is semi-upstream (doc, etc.) as well. I discussed those changes only via IRC (with main RE devs), so I'm afraid there's no response-reference for them. I tried to add some my "why" in the patch comments, hope it is reasonable. I've also tweaked the patch file headers to be a bit more Fedora-centric spec URL: http://arand.fedorapeople.org/5/redeclipse.spec
Getting there! When you make changes to your spec its customary to bump the release number and annotate in the %changlog like so: * Wed Feb 29 2012 Martin Erik Werner <martinerikwerner> 1.2-3 - blah * Wed Feb 29 2012 Martin Erik Werner <martinerikwerner> 1.2-2 - blah Also keep you spec file column with under 72 for readability (except where its not feasible for scripts), I'd also suggest having separate lines for each of your BuildRequires And Its also customary to order your tags like so: %package -n blah1 %description -n blah1 %package -n blah2 %description -n blah2 You can also drop rm -rf %{buildroot} from your %install section Change %{_mandir}/man6/%{name}-server.6.gz to %{_mandir}/man6/%{name}-server.6.* to allow for changes in compression format in subsequent releases Have a look at http://fedoraproject.org/wiki/Packaging:Guidelines#Libexecdir. Move the executables to %_bindir if these executables are to be run directly by users and not via the main executable.
(In reply to comment #14) > Getting there! > > When you make changes to your spec its customary to bump the release number and > annotate in the %changlog like so: > > * Wed Feb 29 2012 Martin Erik Werner <martinerikwerner> 1.2-3 > - blah > > * Wed Feb 29 2012 Martin Erik Werner <martinerikwerner> 1.2-2 > - blah Even during the review procss? I though that was only for changes that are made between actual Fedora releases? Or in the case of a first release, major things which differ compared to upstream (i.e. the patches). > > Also keep you spec file column with under 72 for readability (except where its > not feasible for scripts), I'd also suggest having separate lines for each of > your BuildRequires Ok, fixed, I was doing 80, since that was mentioned for %description > > And Its also customary to order your tags like so: > > %package -n blah1 > > %description -n blah1 > > %package -n blah2 > > %description -n blah2 Ok, done > > You can also drop rm -rf %{buildroot} from your %install section > Ah, true, targeting EPEL 5 might be a bit excessive, done > Change %{_mandir}/man6/%{name}-server.6.gz to > %{_mandir}/man6/%{name}-server.6.* to allow for changes in compression format > in subsequent releases Done > > Have a look at http://fedoraproject.org/wiki/Packaging:Guidelines#Libexecdir. > Move the executables to %_bindir if these executables are to be run directly by > users and not via the main executable. The game needs to be executed via wrapper scripts since it looks in the working dir for the data files.
- Start doing minor releases + changelog for review - Some indentation fixes - Drop "in the -data package" since licesene.txt is included in -server spec URL: http://arand.fedorapeople.org/6/redeclipse.spec
(In reply to comment #13) > (In reply to comment #12) > > You need a %doc entry in the %files section of the main package. Read this: > > http://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Subpackage_Licensing > > If this is enough, I'd be happy to remove the all-licenses file, however, I get > the impression that (paraphrasing license.txt) > "There are a whole bunch of individual licenses (some custom-written), dig > through the subfolders in order to find them" > ..is not really good enough, or is it? > > Note that there are a few home-made licenses there as well, my comments in the > spec file only cover those which have a known shortname in Fedora. > Its really your call, if some of the maps/content etc are packaged under differing licenses from the main package you should consider sub-packaging those files if feasible and they can be contained. It could also set a precedent for future maps You should also uncomment the requires and remove the bundled enet. Write your spec as if the enet you require is available (looks like someone will be looking at this soon). if you have already an enet spec patch please attach it to the blocker if you haven't already
(In reply to comment #17) > (In reply to comment #13) > > (In reply to comment #12) > > > You need a %doc entry in the %files section of the main package. Read this: > > > http://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Subpackage_Licensing > > > > If this is enough, I'd be happy to remove the all-licenses file, however, I get > > the impression that (paraphrasing license.txt) > > "There are a whole bunch of individual licenses (some custom-written), dig > > through the subfolders in order to find them" > > ..is not really good enough, or is it? > > > > Note that there are a few home-made licenses there as well, my comments in the > > spec file only cover those which have a known shortname in Fedora. > > > > Its really your call, if some of the maps/content etc are packaged under > differing licenses from the main package you should consider sub-packaging > those files if feasible and they can be contained. It could also set a > precedent for future maps I will not do this at this point in time at least, I think it would complicate the packaging greatly, and make updating it a chore (content changing -> new packages). I mean, in that case I would package two images on one package, a sound file in another, one map in one package and the rest in another package, if going just based on licensing... > > You should also uncomment the requires and remove the bundled enet. Write your > spec as if the enet you require is available (looks like someone will be > looking at this soon). if you have already an enet spec patch please attach it > to the blocker if you haven't already Ok, done I do have a spec patch: -Version: 1.2.1 -Release: 3%{?dist} +Version: 1.3.3 +Release: 1%{?dist} I figured it was not worth attaching. spec URL: http://arand.fedorapeople.org/7/redeclipse.spec
(In reply to comment #18) > I will not do this at this point in time at least, I think it would complicate > the packaging greatly, and make updating it a chore (content changing -> new > packages). > I mean, in that case I would package two images on one package, a sound file in > another, one map in one package and the rest in another package, if going just > based on licensing... > not feasible then > > > > You should also uncomment the requires and remove the bundled enet. Write your > > spec as if the enet you require is available (looks like someone will be > > looking at this soon). if you have already an enet spec patch please attach it > > to the blocker if you haven't already > > Ok, done > > I do have a spec patch: > -Version: 1.2.1 > -Release: 3%{?dist} > +Version: 1.3.3 > +Release: 1%{?dist} > I figured it was not worth attaching. > > spec URL: http://arand.fedorapeople.org/7/redeclipse.spec ha ha - I see your point
Once again Martin it seems I have misled you. It looks like some repacking of the source is required (as you quite rightly suspected at the beginning). My apologies Have a look at this link, I think it explains it really well. Your debian package is doing the same thing as far as I can tell. http://fedoraproject.org/wiki/Packaging:SourceURL#When_Upstream_uses_Prohibited_Code
Yeah, I saw the discussion on the -devel list, and I was the one who pointed out the fonts in the new egoboo being freeware-ish over at https://bugzilla.redhat.com/show_bug.cgi?id=799778 ;) But I'm wondering to what extent does this apply? The wiki article only mentions if the content is legally non-*distributable* as far as I can tell, and I get the impression that *freeware* (or e.g. CC-BY-NC) would not fall under the prohibited code umbrella? In addition, there is this notice regarding game *data* http://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Shareware In general the documentation is really flaky regarding the separation of the source rpm and the binary rpm, or if there would be any difference there at all? The license *info* seems to only apply to the binary rpms... I will be re-repacking the source, anyhow, since it's unclear, for the time being, better safe than sorry. I'm keeping the Enet lib code (not binary lib) in there though (it is clearly MIT-free), since upstream has warned that Enet shouldn't be treated as a separate lib (and asked for it to be used embedded, which I'm not doing), so I'm guessing that at some point it might be necessary to make use of it, if we're unlucky, better be prepared.
I had to check the koji build above, lucky that was using the initial fully-repacked tarball, so that should be ok :) With repacking again: spec URL: http://arand.fedorapeople.org/8/redeclipse.spec Are there anything else at the moment worth tweaking? Or is it getting to the point of being ready (and waiting for Enet)?
All looking pretty good. As this is your first package whoever sponsors you will want to have seen at least one informal review done by yourself of another package. You can "yum install fedora-review" and use that as your template - or choose one from here: http://fedoraproject.org/wiki/Category:Package_Maintainers/Review_Template. Choose any review request you find interesting - just make sure its one that doesn't track the FE-NEEDSPONSER bug. Just list them here so your sponsor can find them easily. You are also free to adhoc comment on any package review you find. All in all I think you've done good work here - Fedora and Debian aren't too different when it comes to policy so you shouldn't have any issues grasping the way things are done around Fedora.
Also have just seen Jon's reply to your question regarding the content license. You will need to remove those as well. There's also a tool included with rpmdevtools you can run on your source directory. It's not perfect but it may provide some help. licensecheck <build-dir> -r
There are no NonCommercial content in the RE archive, I was asking that on the account of egoboo. Yes, I'm well aware of that tool :)
I've picked up another game for review, also using enet. This could be a good candidate for your informal review. Let me know though as I'm planning on finishing it off in the next day or so (bug 801092).
Yeah, I'm up for having a look at it, will do so now. I made a warm-up review over at https://bugzilla.redhat.com/show_bug.cgi?id=769794 too
Review done for Sumwars over at bug 801092.
- 1.2-5 - Add Icon Cache scriptlet snippet - Add BuildArch: noarch for -data subpackage - Use %%{version} for Source0 and <version> in gen-tarball comment spec URL: http://arand.fedorapeople.org/9/redeclipse.spec
Brendan, should I at this point be looking for a sponsor? Or is there more to be done before that step?
Can you upload the patches for this? The SRPM I downloaded today seems to be missing them.
Ah, indeed, sorry about that, I've skimped a bit too much on the SRPM uploads. I also spotted some additional things, font miss - facepalm! - 1.2-6 - Remove fonts in %_prep - Corrections in all-licenses + Adapt comment about font generation (source URL & cube2font) + Remove superfluous overview "License:" field (copy of license.txt) spec URL: http://arand.fedorapeople.org/10/redeclipse.spec patch0 : http://arand.fedorapeople.org/10/redeclipse-1.2-windowed-by-default.patch patch1 : http://arand.fedorapeople.org/10/redeclipse-1.2-backported-fix-icon-sizes.patch patch2 : http://arand.fedorapeople.org/10/redeclipse-1.2-build-with-system-enet.patch Source1 : http://arand.fedorapeople.org/10/all-licenses Gen-tar : http://arand.fedorapeople.org/10/redeclipse-generate-tarball.sh Say if you want the full SRPM.
I've had a look through my gen-tar repacking, and I'm getting unsure again.. None of the following things are used, but how zealous should I be in removing them? (The CGTextures item is an obvious removal, legally, so repacking is required no matter what.) There are a bunch of binary blobs which are LGPL, they have no source in this tarball, however their source can be found in other Fedora packages (though likely the wrong version), and online, e.g. the SDL libs. Yet more blobs are permissively licensed, without source, e.g. freetype.lib. There's also a bunch of build/install items for win/osx. So what I'm wondering is, which of these needs to be taken care of in gen-tar, and which in %prep? And if the case is that I'm already doing repacking, is it wrong to remove more cruft whilst "already at it"? (Size-wise the gain is ~50MB for the SRPM...)
So, the rule is this: * If something is not used, but it is licensed so that it is redistributable without restrictions, it should be deleted in %prep, but can stay in the tarball. However, you are welcome to remove it from the tarball if you are already generating a modified tarball to make the SRPM smaller. * Items under licenses which are not redistributable without restrictions must not be included in the Sources. As to your question about cruft, I think it would be nice to scrape off as much as you can, but I will not block on review for it.
Ah, excellent, as per that I'll keep my current cruft removal (which is currently zealous) pretty much as-is. I'll also do: - 1.2-7 - Move removal of Enet & fonts from %%prep to generate-tarball script - Re-add generate-tarball script as Source1 Should I be versioning my repacking by the way? Like redeclipse-1.2-fs3.tar.xz, in order to distinguish different repackings, or is that unnecessary since it's already contained in a versioned SRPM? spec URL: http://arand.fedorapeople.org/11/redeclipse.spec srpm URL: http://arand.fedorapeople.org/11/redeclipse-1.2-7.fc17.src.rpm Gen-tar : http://arand.fedorapeople.org/11/redeclipse-generate-tarball.sh
That's really up to you. I would not recommend you use the same tarball name as the original, just for the purposes of sanity, but there is no real need to sub-version it, unless you anticipate it will be a frequent occurrence (and even then, probably unnecessary because the lookaside cache that Fedora uses to hold the source tarballs uses md5sums to differentiate files, not just file names).
Ah, ok, then I'll stick with what I have (-1.2-fs). Also noticed the client/server bins was getting stripped in the %install phase: - 1.2-8 - Set flags for make install to avoid strip and get a useful -debuginfo spec URL: http://arand.fedorapeople.org/12/redeclipse.spec
I haven't forgotten about this, I've just been super super busy this week.
No worries, I kind of guessed that :)
Reverting spurious Bug 553189 - mis-assignment to 0xFFFF
=== REVIEW === Needs Fixes: * You should run desktop-file-verify over the .desktop file. See: https://fedoraproject.org/wiki/Packaging/Guidelines#desktop-file-install_usage and be sure to add BuildRequires: desktop-file-utils * You have an explicit Requires: redeclipse-data, should that be a versioned requires to ensure clean upgrades? I think Requires: redeclipse-data = %{version}-%{release} even makes sense here. * rpmlint: cube2font.x86_64: W: spelling-error %description -l en_US utiliy -> utility, utilize This one is a legitimate typo. Please fix. Good: - rpmlint checks return: redeclipse.src: W: spelling-error Summary(en_US) Multiplayer -> Multiplier, Multiplexer redeclipse.src: W: spelling-error %description -l en_US multi -> mulch, mufti redeclipse.src: W: spelling-error %description -l en_US gameplay -> game play, game-play, nameplate redeclipse.src: W: spelling-error %description -l en_US gamemodes -> game modes, game-modes, gametes redeclipse.src: W: spelling-error %description -l en_US mutators -> commutators, mutilators, commutator redeclipse.src: W: invalid-url Source0: redeclipse-1.2-fs.tar.xz redeclipse.x86_64: W: spelling-error Summary(en_US) Multiplayer -> Multiplier, Multiplexer redeclipse.x86_64: W: spelling-error %description -l en_US multi -> mulch, mufti redeclipse.x86_64: W: spelling-error %description -l en_US gameplay -> game play, game-play, nameplate redeclipse.x86_64: W: spelling-error %description -l en_US gamemodes -> game modes, game-modes, gametes redeclipse.x86_64: W: spelling-error %description -l en_US mutators -> commutators, mutilators, commutator All of the above are safe to ignore. - package meets naming guidelines - package meets packaging guidelines - license (zlib and CC-BY-SA) OK, text in %doc, matches source - spec file legible, in am. english - source matches upstream - package compiles on devel (x86_64) - no missing BR - no unnecessary BR - no locales - not relocatable - owns all directories that it creates - no duplicate files - permissions ok - macro use consistent - content is okay - no need for -docs - nothing in %doc affects runtime Fix those three items and I'll approve this.
Hopefully they are addressed correctly: * Wed May 02 2012 Martin Erik Werner <martinerikwerner> - 1.2-9 - Use desktop-file-validate in %%install step - Typo fix in cube2font %%description: utiliy -> utility - Added versioned Requires: redeclipse-data = %%{version}-%%{release} spec URL: http://arand.fedorapeople.org/13/redeclipse.spec
Looks great, this package is APPROVED. Let me know what your Fedora Account username is and I will sponsor you.
It's "arand"
You're sponsored (and the review is done, so you don't need to change that flag).
New Package SCM Request ======================= Package Name: redeclipse Short Description: Multiplayer FPS game based on Cube2 Owners: arand Branches: f16 f17 InitialCC:
Git done (by process-git-requests).
Were you going to add redeclipse to the games group in comps? That's normal for games.
Ah, thanks for the hint, added in rawhide, was told to wait until package was in stable repos to add it to f17 though.
I have another question. I see that there is an f17 build of redeclipse, but I am not finding the package in bohdi. Did you make a request to move the package to f17 updates-testing?
redeclipse-1.2-9.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/redeclipse-1.2-9.fc17
Ah, I read that but thought updates meant... updates, and wasn't relevant in the case of new packages, submitted now.
redeclipse-1.2-9.fc17 has been pushed to the Fedora 17 testing repository.
redeclipse-1.2-10.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/redeclipse-1.2-10.fc17
This now having passed testing-verification, so I figured I'd say a somewhat overdue thank you to everyone who helped me along in packaging RE, especially for reviewing and polishing it through a rather lengthy process :) Thank you!
redeclipse-1.2-10.fc17 has been pushed to the Fedora 17 stable repository.