SPEC: http://people.redhat.com/spoyarek/gource/gource.spec SRPM: http://people.redhat.com/spoyarek/gource/gource-0.18-1.fc12.src.rpm Description: OpenGL-based 3D visualisation tool for source control repositories. The repository is displayed as a tree where the root of the repository is the centre, directories are branches and files are leaves. Contributors to the source code appear and disappear as they contribute to specific files and directories.
Updated the SPEC and SRPM after rpmlint'ing the packages and specfile. Thanks Rahul Sundaram!
From last comment it seems you have done some update. If you change your spec or srpm, please do a release tag bump and write down changes in ChangeLog and post the new urls. Thanks,
While skimming I found an unowned directory /usr/share/gource/ https://fedoraproject.org/wiki/Packaging:UnownedDirectories
"" autoreconf %configure --prefix=/usr make "" needs to be replaced with "" %configure make %{?_smp_mflags} "" %configure macro already has --prefix=/usr and other options and a lone make is not advisable. moreover timestamp needs to be saved, while copying files into BUILDROOT in %install section. I would also suggest to try and get rid of patch for version correction, as running autoreconf is not advised. If you really care about package version, try sed in configure script. Thanks,
Running autoreconf is not a real problem. I assume, that patch makes it anyway upstream and won't be required in the next release of gource, yes? Keeping the timestamps is not a must, it's just suggested by the way...
(In reply to comment #5) > Running autoreconf is not a real problem. I disagree - People who do so are exposing themselves and their package's users to avoidable risks. > Keeping the timestamps is not a must, it's just suggested by the way... This suggestions is based on ill assumptions. Keeping timestamps is technically non-required eye-candy.
Siddhesh, I had a look to your spec file. A few thoughts and comments: - You are currently mixing $RPM_BUILD_ROOT and %{buildroot}, choose one and please keep it consistent (https://fedoraproject.org/wiki/PackagingGuidelines#Using_.25.7Bbuildroot.7D_and_.25.7Boptflags.7D_vs_.24RPM_BUILD_ROOT_and_.24RPM_OPT_FLAGS) - I can't see a reason to have the "Requires:" lines inside of the spec file. If your software links to a library, rpmbuild is catching up the dependency in the end of the build process and adds "Requires:" to the required libs. - Why do you do "%configure --prefix=/usr" rather "%configure"? Is there a real reason for? If you do "rpm --eval '%configure'", you see what %configure will be expanded to. - You want to use parallel make for making your package. If the software does not support it right now, please make a comment and otherwise please use it: https://fedoraproject.org/wiki/PackagingGuidelines#Parallel_make - You might want to preserve timestamps by appending INSTALL="install -p" to the "make install" command. As Ralf agreed with me, it is not required on a technically base, Guidelines just suggest it, see e.g. https://fedoraproject.org/wiki/PackagingGuidelines#Timestamps - As far as I can see, you're missing "BuildRequires: freetype-devel", otherwise I had trouble to rebuild the package - Can you please choose a valid BuildRoot tag from the available list? https://fedoraproject.org/wiki/PackagingGuidelines#BuildRoot_tag - You are missing the pareparation of BuildRoot in %install section, see https://fedoraproject.org/wiki/PackagingGuidelines#Prepping_BuildRoot_For_.25install - I'm wondering about the following lines in your spec file: %dir %{_datadir}/gource/* %dir %{_mandir}/man*/* %dir %{_datadir}/gource/fonts/* Have a look at: https://fedoraproject.org/wiki/Packaging:UnownedDirectories - Can you please communicate with upstream and ensure that your patch really makes it into the next release of gource? Thank you.
According to FAS, you're not yet a packager, so we need to block FE-NEEDSPONSOR bug report according to https://fedoraproject.org/wiki/Package_Review_Process. I'm resetting the fedora‑review flag to empty, as only the reviewer, not the packager should ever set and change it. Sorry, but I'm not a sponsor for packaging, so I'm now removing myself from assigned. Nevertheless you should have a look to my comments and work on it.
The upstream developer has accepted the patch, so we should see it in the next upstream version: http://code.google.com/p/gource/issues/detail?id=21 I'll work on the spec file and revert with an update (with a bumped release number this time :) ). Thanks Robert, Rakesh and Ralf!
Uploaded updated spec files based on feedback in all comments above: http://people.redhat.com/spoyarek/gource/gource.spec http://people.redhat.com/spoyarek/gource/gource-0.18-3.fc12.src.rpm I've kept the autoreconf in place for now to cater for any autotools file patches in the future. I've also dropped the configure.ac patch for now since it does not really break anything. Accordingly, I've also dropped autoreconf since it's not necessary anymore.
Sorry for the mixed up comment earlier, I'll rephrase that: I've dropped the configure.ac patch for now since it does not really break anything. Accordingly, I've also dropped autoreconf since it's not necessary anymore.
just a few comments: - Please use %global instead of %define - (already mentioned) If parallel make doesn't work, please mention that in a comment. Otherwise, the make line should just read make %{?_smp_mflags} I'm not a sponsor either... Stefan
Thanks Stefan. Implemented your suggestions. Updated spec and source package. Rebased to new upstream release too: http://www.siddhesh.in/hacks/gource.spec http://www.siddhesh.in/hacks/gource-0.23-1.fc13.src.rpm
Well, * Version specific BuildRequires - %sdl_image_version, %sdl_version are not needed because SDL{_image} packges in currently supported Fedora branch all satisfies this depedency ! For ftgl, F-11 ftgl is still 2.1.2, so you may want to write this version specific BuildRequires. * Virtual BuildRequires - For some reasons, we prefers to use - "BuildRequires: libGL-devel" instead of mesa-libGL-devel - and "libGLU-devel" instead of mesa-libGLU-devel * Preserving timestamps - Using alias won't work when using Makefile because the subprocess executed by make won't recognize such shell builtin commands. For this case you have to modify Makefile.in directly at %prep like; --------------------------------------------------- %prep %setup -q sed -i.cp -e 's|cp |cp -p |' Makefile.in --------------------------------------------------- * Enclosed fonts - Currently Fedora bans to use fonts enclosed in the non-font source tarball and you must use fonts provided from font related rpms shipped in Fedora. Fedora uses dejanu fonts by default. If you choose to use /usr/share/fonts/dejavu/DejaVuSans.ttf (in dejavu-sans-fonts rpm), you have to - Add "Requires: dejavu-sans-fonts" to this rpm - Replace FreeSans.ttf under %_datadir/gource/fonts/ to the symlink to DejaVuSans.ttf. %files entry - For example %files contains --------------------------------------------------- %files %dir %{_datadir}/gource/fonts/* --------------------------------------------------- However the installed objects under %{_datadir}/gource/fonts/ is actually a file, not a directory. So here using %dir is wrong. ! Note: As I said above, this file should be replaced with a symlink ! Note2: You may say "but actually rpmbuild succeeds with this %files entry". Well, I don't know why (I commented on bug 505995 comment 3) Also. the %files entry --------------------------------------------------- %dir %{_datadir}/gource/* --------------------------------------------------- also contains some files and this entry should also be fixed ( again due to a bug in rpm currently rpmbuild succeeds... )
Not sure if this is stalled. Symlinking to another font is not the right way to solve this. The right way to fix the bundled font problem: * Add a Requires: gnu-free-sans-fonts * In the %build section, use %configure --enable-ttf-font-dir=%{_datadir}/fonts/gnu-free/ * In the %install section, rm -rf %{buildroot}/%{_datadir}/%{name}/fonts * Remove the line in %files referring to the bundled fonts
(In reply to comment #15) > Not sure if this is stalled. Symlinking to another font is not the right way > to solve this. The right way to fix the bundled font problem: > > * Add a Requires: gnu-free-sans-fonts > * In the %build section, use %configure > --enable-ttf-font-dir=%{_datadir}/fonts/gnu-free/ > * In the %install section, rm -rf %{buildroot}/%{_datadir}/%{name}/fonts > * Remove the line in %files referring to the bundled fonts Well, actually it seems that configure accepts --enable-ttf-font-dir, however as I said in my comment 14 we use dejavu fonts by default so unless there is some special desire please use dejavu fonts.
I don't agree, because that would require much more intrusive patching to the application code itself. That makes no sense when we already package the requisite font. Also, keep in mind that the font might be chosen for aesthetic reasons (out of a variety of free fonts available) by the author.
(In reply to comment #17) > I don't agree, because that would require much more intrusive patching to the > application code itself. - As I already said, creating symlink is enough (and many other packages on Fedora simply do this) > That makes no sense when we already package the > requisite font. Also, keep in mind that the font might be chosen for aesthetic > reasons (out of a variety of free fonts available) by the author. - This is packager's choise. Note that as I already said in the comment 14 as "If you choose to use /usr/share/fonts/dejavu/DejaVuSans.ttf", I am not strictly objecting to use gnu-free-sans-fonts (one of the packages I maintain explicitly uses gnu-free fonts)
Thanks Mamoru san, Paul. Here are the updated spec file and source rpm: http://www.siddhesh.in/hacks/gource.spec http://www.siddhesh.in/hacks/gource-0.23-2.fc13.src.rpm > ! For ftgl, F-11 ftgl is still 2.1.2, so you may want to > write this version specific BuildRequires. I already have this in place. IIUC I don't have to change anything for this right? -- Fedora Bugzappers volunteer triage team https://fedoraproject.org/wiki/BugZappers
(In reply to comment #19) > > ! For ftgl, F-11 ftgl is still 2.1.2, so you may want to > > write this version specific BuildRequires. > > I already have this in place. IIUC I don't have to change anything for this > right? Yes. Now this package itself seems good, so: ------------------------------------------------------------- NOTE: Before being sponsored: This package will be accepted with another few (or no) work. But before I accept this package, someone (I am a candidate) must sponsor you. Once you are sponsored, you have the right to review other submitters' review requests and approve the packages formally. For this reason, the person who want to be sponsored (like you) are required to "show that you have an understanding of the process and of the packaging guidelines" as is described on : http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored Usually there are two ways to show this. A. submit other review requests with enough quality. B. Do a "pre-review" of other person's review request (at the time you are not sponsored, you cannot do a formal review) When you have submitted a new review request or have pre-reviewed other person's review request, please write the bug number on this bug report so that I can check your comments or review request. Fedora package collection review requests which are waiting for someone to review can be checked on my wiki page: http://fedoraproject.org/wiki/User:Mtasaka#B._Review_request_tickets (Check "No one is reviewing") Review guidelines are described mainly on: http://fedoraproject.org/wiki/Packaging/ReviewGuidelines http://fedoraproject.org/wiki/Packaging/Guidelines http://fedoraproject.org/wiki/Packaging/ScriptletSnippets ------------------------------------------------------------
ping?
Sorry, I haven't had the time to look into this lately. Will get something out soon and link it here, hopefully before the end of this week. -- Fedora Bugzappers volunteer triage team https://fedoraproject.org/wiki/BugZappers
I've put my review comments as comment 1 on bug 533167.
(In reply to comment #23) > I've put my review comments as comment 1 on bug 533167. For /sbin/install-info, please check my comment on the bug. I will approve this package. ------------------------------------------------------------ This package (gource) is APPROVED by mtasaka ------------------------------------------------------------ Please follow the procedure written on: http://fedoraproject.org/wiki/PackageMaintainers/Join from "Install the Client Tools (Koji)". Now I am sponsoring you. If you want to import this package into Fedora 11/12, you also have to look at http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT (after once you rebuilt this package on koji Fedora rebuilding system). If you have questions, please ask me. Removing NEEDSPONSOR.
Sorry about the delay, here's the koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1978970 Do I request for cvs module for my package now?
(In reply to comment #26) > Do I request for cvs module for my package now? Yes, please.
New Package CVS Request ======================= Package Name: gource Short Description: OpenGL-based 3D visualisation tool for source control repositories Owners: siddhesh Branches: F-12 InitialCC:
CVS done (by process-cvs-requests.py).
*** Bug 565838 has been marked as a duplicate of this bug. ***
Ops, sorry for duplicate but I've just checked F12 yum search to find if the package exists. What's the current status? There are no builds yet. Any problem I can help with?
Sorry for duplicate review, anyway - this one imho should add and ship at least %doc COPYING README THANKS ChangeLog
(In reply to comment #32) > Sorry for duplicate review, anyway - this one imho should add and ship at least > %doc COPYING README THANKS ChangeLog Oops, sorry for overlooking this. Siddhesh, please add this when importing this package into Fedora CVS.
Sure, I'll do that. Also rebasing with the latest upstream version.
gource-0.24-1.fc12 has been submitted as an update for Fedora 12. http://admin.fedoraproject.org/updates/gource-0.24-1.fc12
gource-0.24-1.fc13 has been submitted as an update for Fedora 13. http://admin.fedoraproject.org/updates/gource-0.24-1.fc13
Closing. Thank you, everyone.
gource-0.24-1.fc13 has been pushed to the Fedora 13 stable repository. If problems still persist, please make note of it in this bug report.
gource-0.24-1.fc12 has been pushed to the Fedora 12 stable repository. If problems still persist, please make note of it in this bug report.