Spec URL: http://members.iinet.net.au/~timmsy/glglobe/glglobe.spec SRPM URL: http://members.iinet.net.au/~timmsy/glglobe/glglobe-0.2-1.fc7.src.rpm Description: GLGlobe is an OpenGL - globe simulation for Linux. It was inspired by XGlobe or XEarth and can use the marker-files of these programs. The simulation includes day light and night time rendering, and the globe can be rotated and scaled interactively, or automatically rotated based on the current time of day
I do have some queries about the compile process: gcc -Wall -O3 -I/usr/X11/include -c -o vmath.o vmath.c gcc -Wall -O3 -I/usr/X11/include -c -o globe.o globe.c globe.c: In function 'make_globe_old2': globe.c:507: warning: dereferencing type-punned pointer will break strict-aliasing rules globe.c:511: warning: pointer targets in assignment differ in signedness globe.c: In function 'make_globe': globe.c:663: warning: dereferencing type-punned pointer will break strict-aliasing rules globe.c:665: warning: dereferencing type-punned pointer will break strict-aliasing rules Should I be concerned about these warnings ? Should I attempt to patch around them ? Thanks, DaveT.
I wouldn't worry overmuch about the warnings, but I would worry quite a bit about the fact that your package doesn't seem to honor %{optflags}. You'll need to figure out how to pass the proper compiler flags. Also, to answer your question about commenting out %configure, % macros are still expanded if the line begins with '#'. Either double the '%' or remove it.
Jason, is that dis-honouring caused by removing the %configure ? Did you determine that it doesn't by the params passed to gcc ? {Hoping for a hint on where to look} Will it be the source's make file ? Can the package patch the make file to solve it ? commented macros: Is it because the macros expanded out are multi-line, and perhaps even start with a new line that the expanded part does not stay commented ? An offside: The upstream developer hasn't touched the source in years, from what I can tell. I remember both Hans and yourself mentioned on fedora-devel that non-activity is not necessarily a blocker. In any case, I have requested of Florian whether he has/can make some nicer icons. ps. This is my first package up for review, and I'll eventually need a sponsor.
I did not investigate how this package builds, so at this point I can't tell you how to pass the proper compiler flags to it. I don't see any configure script in the package, so I'm not sure what good a call to %configure could possibly do. If there's just a plain Makefile, you probably just need check that it will allow you to pass CFLAGS on the make line, or patch it to allow that. > Is it because the macros expanded out are multi-line, and perhaps even start > with a new line that the expanded part does not stay commented ? Perhaps; I'm not intimately familiar with the details of how RPM expands macros. > ps. This is my first package up for review, and I'll eventually need a > sponsor. This should block FE-NEEDSPONSOR, then.
Updated spec: http://members.iinet.net.au/~timmsy/glglobe/glglobe.spec Updated src.rpm: http://members.iinet.net.au/~timmsy/glglobe/glglobe-0.2-2.fc7.src.rpm - del commenting about non-existent %configure. - add export CFLAGS="$RPM_OPT_FLAGS" so that rpm flags are used during build. - add makefile patch to use environment CFLAGS if they are defined.
Current spec: http://members.iinet.net.au/~timmsy/glglobe/glglobe.spec Updated src.rpm: http://members.iinet.net.au/~timmsy/glglobe/glglobe-0.2-3.fc7.src.rpm It didn't build in mock due to desktop-file-install not being available: - add BuildRequires: desktop-file-utils so that it build in mock
Well, for 0.2-3: * Source1-4 - How did these created? If you downloaded from some URLs, please write the full URLs like Source1. Otherwise please write comments how you created these files. * Desktop file - Categories "X-Red-Hat-Extra X-Red-Hat-Base" are both deprecated and should be removed. * Requires for scriptlets -------------------------------------------------- Requires(post): desktop-file-utils Requires(postun): desktop-file-utils -------------------------------------------------- - are not needed (actually no scriptlets require desktop-file-utils ). * Documents -------------------------------------------------- %install .... mkdir -p %{buildroot}%{_docdir}/glglobe install -p -m 0644 ChangeLog COPYING README TODO %{buildroot}%{_docdir}/glglobe .... %files %doc %{_datadir}/doc/* -------------------------------------------------- - Usually documents must be version specific. Just use: -------------------------------------------------- %doc ChangeLog COPYING README TODO -------------------------------------------------- (and check what actually happens). * desktop-file-utils usage -------------------------------------------------- install -p -m 0644 %{SOURCE4} %{buildroot}%{_datadir}/applications desktop-file-install --vendor fedora --delete-original \ --dir %{buildroot}%{_datadir}/applications \ --add-category X-Red-Hat-Base \ %{buildroot}%{_datadir}/applications/glglobe.desktop ---------------------------------------------------- - This can be replaced by ---------------------------------------------------- desktop-file-install --vendor fedora \ --dir %{buildroot}%{_datadir}/applications \ %{SOURCE4} ---------------------------------------------------- (and remove deprecated categories) * Directory ownership - The directory %{_datadir}/glglobe/ is not owned by any packages - The directories -------------------------------------------------- %{_datadir}/icons/hicolor %{_datadir}/icons/hicolor/* %{_datadir}/icons/hicolor/*/apps -------------------------------------------------- are already owned by hicolor-icon-theme and this package must not own these.
One more issue * License - License tag policy is changed and now you (we) must follow: http://fedoraproject.org/wiki/Packaging/LicensingGuidelines and http://fedoraproject.org/wiki/Licensing As far as I checked the source code, this is licensed under GPL version 2 only. So please use "GPLv2" as license tag.
Mamoru, thanks for taking a look: (In reply to comment #7) Updated spec: http://members.iinet.net.au/~timmsy/glglobe/glglobe.spec SRPM: http://members.iinet.net.au/~timmsy/glglobe/glglobe-0.2-4.fc7.src.rpm - add comment re Source1..4 creation - mod .desktop category to Education, although some or all of the {Education, Graphics, DataVisualization, Geography, Amusement, ImageProcessing } might fit ? - del .desktop category deprecated items - del extraneous BR and Requires(post/postun) for desktop-file-utils > desktop-file-install --vendor fedora --delete-original \ When I followed other packages as an example, it seems that --delete-original was normal. Is this a changed packaging standard ? - mod desktop-file-install to simpler command, using SOURCEx - add ownership of app data directory - mod ownership of icon data to glglobe specific filenames (In reply to comment #8) - mod License to meet new Licensing Guidelines Changed but didn't work out rpmlint's unhappiness until I realized I hadn't updated rpmlint on that machine ;) Current spec: http://members.iinet.net.au/~timmsy/glglobe/glglobe.spec Updated SRPM: http://members.iinet.net.au/~timmsy/glglobe/glglobe-0.2-4.fc7.src.rpm A query: is it worth testing in fc6 and fdevel {my testing has been in f7}?
I have not checked -4 spec/srpm, for now just answering your comment: (In reply to comment #9) > - mod .desktop category to Education, although some or all of the {Education, > Graphics, DataVisualization, Geography, Amusement, ImageProcessing } might fit ? - IMO only one Category "Education" is enough. > > desktop-file-install --vendor fedora --delete-original \ > When I followed other packages as an example, it seems that --delete-original > was normal. Is this a changed packaging standard ? - You don't have to (and must not) delete the original SOURCEx. The reason you had to use --delete-original was if you didn't use it, i.e. -------------------------------------------------- install -p -m 0644 %{SOURCE4} %{buildroot}%{_datadir}/applications desktop-file-install --vendor fedora \ --dir %{buildroot}%{_datadir}/applications \ --add-category X-Red-Hat-Base \ %{buildroot}%{_datadir}/applications/glglobe.desktop ---------------------------------------------------- This result in leaving 2 desktop files named "glglobe.desktop" and "fedora-glglobe.desktop" under %{buildroot}%{_datadir}/applications . So in this usage --delete-original was actually needed. By the new usage (written below:) ---------------------------------------------------- desktop-file-install --vendor fedora \ --dir %{buildroot}%{_datadir}/applications %{SOURCE4} ---------------------------------------------------- Only one desktop file named "fedora-glglobe.desktop" is created under %{buildroot}%{_datadir}/applications so it is already okay. And you don't have to (and must not) delete the original SOURCE4 file. > A query: is it worth testing in fc6 and fdevel {my testing has been in f7}? - If you have FC6 or rawhide machine and you can test on them, of course it is appreciated.
Well, for -4: * Desktop category Category "X-Red-Hat-Extra" is still there, which is not needed. Then: ------------------------------------------------------------- NOTE: Before being sponsored: This package will be accepted with another few 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: http://fedoraproject.org/PackageReviewStatus/NEW.html (NOTE: please don't choose "Merge Review") Review guidelines are described mainly on: http://fedoraproject.org/wiki/Packaging/ReviewGuidelines http://fedoraproject.org/wiki/Packaging/Guidelines http://fedoraproject.org/wiki/Packaging/ScriptletSnippets ------------------------------------------------------------
(In reply to comment #11) > Well, for -4: > * Desktop category > Category "X-Red-Hat-Extra" is still there, which is not needed. Oops, I forgot to cp the updated .desktop to the SOURCES directory, so it built with the earlier version. fix noted in spec: http://members.iinet.net.au/~timmsy/glglobe/glglobe.spec Updated SRPM: http://members.iinet.net.au/~timmsy/glglobe/glglobe-0.2-5.fc7.src.rpm It builds in f7 mock. It installs and runs on a minimal noX f7 install {forwarding the X connection to my normal gui desktop}, so it seems rpm is picking up the correct runtime libs - but is that a reasonable way to check Requires ? ===== Currently I am pre-reviewing: https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=234326 and initial looks at: https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=251019 with a more complete pre-review to come.
(In reply to comment #12) > It builds in f7 mock. It installs and runs on a minimal noX f7 install > {forwarding the X connection to my normal gui desktop}, so it seems rpm is > picking up the correct runtime libs - but is that a reasonable way to check > Requires ? It depends on cases. This (glglobe) is a single package with no subpackages and dependency check is rather easy. If one srpm creates some subpackages the dependencies between those subpackages should be carefully examined. Also there are some dependencies which rpmbuild does not detect automatically.. So actually it depends on cases... > ===== > Currently I am pre-reviewing: > https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=234326 - Please check my comment. * If you do a (pre)review, please write clearly what you think is blocking the review However I will accept this review ------------------------------------------------------------- This package (glglobe) is APPROVED by me ------------------------------------------------------------- Please follow the procedure according to: http://fedoraproject.org/wiki/PackageMaintainers/Join from "Get a Fedora Account". At a point a mail should be sent to sponsor members which notifies that you need a sponsor (at the stage, please also write on this bug for confirmation that you requested for sponsorship) Then I will sponsor you. If you want to import this package into Fedora 7, you also have to look at http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT (after once you rebuilt this package on Fedora rebuilding system). If you have questions, please ask me.
As described in the /Join link, I am confirming that I have requested membership in the cvsextras and fedorabugs groups as described in the /Join link.
Now I am sponsoring you.
Thanks for the review and sponsoring me, Mamoru. New Package CVS Request ======================= Package Name: glglobe Short Description: OpenGl Globe - Earth simulation for linux Owners: dtimms Branches: FC-6 F-7 InitialCC: Cvsextras Commits: yes
cvs done.
Thanks Kevin. 101193 build (dist-f8, devel:glglobe-0_2-5_fc8) completed successfully Closing.
Package Change Request ====================== Package Name: glglobe New Branches: EL-5 EL-6 Owners: dtimms
CVS done (by process-cvs-requests.py).