https://bugzilla.redhat.com/show_bug.cgi?id=843678
Ryan Curtin <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |[email protected] --- Comment #1 from Ryan Curtin <[email protected]> --- Hello there, I am an unsponsored reviewer, so this is an unofficial review, but I've done my best. Any of the MUST/SHOULD guidelines which passed I haven't included here (for the sake of space) but I did test them. >>> MUST: rpmlint must be run on the source rpm and all binary rpms the build >>> produces. The output should be posted in the review. Runs just fine: $ rpmlint -v sugar-castle.spec sugar-castle.spec: I: checking-url http://mirror.aarnet.edu.au/pub/sugarlabs/activities/4397/castle-23.xo (timeout 10 seconds) 0 packages and 1 specfiles checked; 0 errors, 0 warnings. >>> MUST: The License field in the package spec file must match the actual >>> license. [3] >>> 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 must be included in %doc.[4] The spec file lists GPLv3+, but nowhere in the source is a license mentioned, nor is a license included with the package. Perhaps upstream should be contacted for clarity? >>> MUST: The package MUST successfully compile and build into binary rpms on >>> at least one primary architecture. Builds okay: http://koji.fedoraproject.org/koji/taskinfo?taskID=4343387 >>> MUST: Permissions on files must be set properly. Executables should be set >>> with executable permissions, for example. All of the resource files (data/ and activity/) are marked executable unnecessarily. It seems as though activity.svg must be executable, though, because '#!/usr/bin/python' is being put into it. >>> MUST: Each package must consistently use macros. %{buildroot} and $RPM_BUILD_ROOT are used interchangeably; just pick one of the two and use it consistently: > %install > rm -rf $RPM_BUILD_ROOT > %{__python} ./setup.py install --prefix=%{buildroot}/%{_prefix} Also, where you use 'sed -i -e '1i#!/usr/bin/python', maybe it would be good to use %{__python} instead of /usr/bin/python. >>> SHOULD: If the source package does not include license text(s) as a >>> separate file from upstream, the packager SHOULD query upstream to include >>> it. License information from upstream does not seem to be available (this is mentioned earlier). >>> SHOULD: The reviewer should test that the package builds in mock. Builds okay. http://koji.fedoraproject.org/koji/taskinfo?taskID=4343387 >>> SHOULD: The package should compile and build into binary rpms on all >>> supported architectures. This is noarch so I assume that is not strictly necessary. >>> SHOULD: The reviewer should test that the package functions as described. A >>> package should not segfault instead of running, for example. I attempted to call the executable with: $ /usr/share/sugar/activities/Castle.activity/Castle.py which may not be correct, so I apologize if that was the wrong way to call it. I found that 'pygame' is an unlisted dependency. Once that was installed, I seemed to run into a path issue: ---- Peter says: Can't find data/pointer.png Traceback (most recent call last): File "/usr/share/sugar/activities/Castle.activity/Castle.py", line 219, in <module> game.run() File "/usr/share/sugar/activities/Castle.activity/Castle.py", line 145, in run g.init() File "/usr/share/sugar/activities/Castle.activity/g.py", line 78, in init pointer=utils.load_image('pointer.png',True) File "/usr/share/sugar/activities/Castle.activity/utils.py", line 56, in load_image print "Peter says: Can't find "+fname; exit() File "/usr/share/sugar/activities/Castle.activity/utils.py", line 10, in exit save() File "/usr/share/sugar/activities/Castle.activity/utils.py", line 20, in save f=open(fname, 'w') IOError: [Errno 2] No such file or directory: 'data/castle.dat' ---- It is possible that my invocation of the program is incorrect, and when done properly some path-like variable is set correctly and this is not a problem. >>> SHOULD: If scriptlets are used, those scriptlets must be sane. This is >>> vague, and left up to the reviewers judgement to determine sanity. I think that maybe a patch file should be used instead of sed to get the '#!/usr/bin/python' in there. If upstream changes how things work, the patch will then probably fail while the sed expressions would continue happily along when maybe they shouldn't. >>> SHOULD: your package should contain man pages for binaries/scripts. If it >>> doesn't, work with upstream to add them where they make sense. I am not sure how applicable man pages are in this context, but none are provided. Hopefully my review is helpful. I apologize in advance for any errors I've made. -- You are receiving this mail because: You are on the CC list for the bug. _______________________________________________ package-review mailing list [email protected] https://admin.fedoraproject.org/mailman/listinfo/package-review
