Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 714543 - Review Request: maze5 - A program for generating mazes of miscellaneous styles and sizes
Summary: Review Request: maze5 - A program for generating mazes of miscellaneous style...
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard: Stalled review
Depends On:
Blocks: FE-DEADREVIEW
TreeView+ depends on / blocked
 
Reported: 2011-06-20 02:18 UTC by W. Michael Petullo
Modified: 2011-12-09 20:39 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-12-09 20:39:59 UTC


Attachments (Terms of Use)

Description W. Michael Petullo 2011-06-20 02:18:47 UTC
Spec URL: http://www.flyn.org/SRPMS/maze5.spec
SRPM URL: http://www.flyn.org/SRPMS/maze5-0.8.1-1.fc15.src.rpm
Description:
This package contains the sources for a standalone program 'maze5'
and a plugin 'maze5gimp' for GIMP (the GNU Image Manipulation Program).
Both are for generating mazes of miscellaneous styles and sizes.

Comment 1 Veeti Paananen 2011-06-20 21:25:24 UTC
Just commenting: wouldn't it make sense to split the GIMP plugin into a subpackage?

Comment 2 Veeti Paananen 2011-06-20 21:27:37 UTC
Oh, and you need to add scons as a build requirement.

Comment 3 Yanchuan Nian 2011-06-22 12:21:53 UTC
1.xmltoman is another missed build requiresment
2.maze5.1 should goes to a subdir of %{_mandir},maybe man/man1/maze5.1
3.INSTALL file can be removed from %doc as it is a irrelevant documentation. 
https://fedoraproject.org/wiki/Packaging/Guidelines#Documentation

Comment 4 W. Michael Petullo 2011-06-22 15:39:29 UTC
Spec URL: http://www.flyn.org/SRPMS/maze5.spec
SRPM URL: http://www.flyn.org/SRPMS/maze5-0.8.1-2.fc15.src.rpm

This release addresses comments #1, #2 and #3.

Comment 5 Volker Fröhlich 2011-06-26 23:03:53 UTC
License is GPLv3+ -- not GPLv3, according to the files.

You can replace the "maze5" in Source0 with the name macro.

The Requires doesn't make sense: The main-package doesn't require itself.

You can drop "rm -rf $RPM_BUILD_ROOT". It's not needed.

The files section should be aggregated just before the changelog. %doc is usually in the first line.

The GIMP plug-in directory is called "plug-ins", not "plugins", as far as I can see. The suffix "gimp", the plug-in has, seems somewhat strange to me. This sub-package should also require GIMP, I guess.

Your sub-packages requires the main package, I suppose: http://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package

Put the BR for gimp-devel-tools next to the other BRs. It doesn't make sense to split that. I think putting each BR on a separate line is more legible.

"%{_mandir}/man1/maze5.1.gz" -- Rather make that "%{_mandir}/man1/maze5.1.*"

Use -p with "install" to preserve timestamps.

You're not using the proper compiler flags: http://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags

Besides that:

[makerpm@fedora15 SRPMS]$ rpmlint ../RPMS/x86_64/maze5-*
maze5.x86_64: W: manual-page-warning /usr/share/man/man1/maze5.1.gz 80: a space character is not allowed in an escape name
maze5.x86_64: W: manual-page-warning /usr/share/man/man1/maze5.1.gz 83: a space character is not allowed in an escape name
maze5.x86_64: W: manual-page-warning /usr/share/man/man1/maze5.1.gz 83: a space character is not allowed in an escape name
maze5.x86_64: W: manual-page-warning /usr/share/man/man1/maze5.1.gz 83: a space character is not allowed in an escape name
maze5-gimp.x86_64: W: unstripped-binary-or-object /usr/lib64/gimp/2.0/plugins/maze5gimp

Comment 6 W. Michael Petullo 2011-06-27 18:29:08 UTC
Spec URL: http://www.flyn.org/SRPMS/maze5.spec
SRPM URL: http://www.flyn.org/SRPMS/maze5-0.8.1-3.fc15.src.rpm

Changes:

- License is GPLv3+
- Use name macro in Source0 definition
- GIMP package requires main package
- Don't remove RPM_BUILD_ROOT
- plugin-ins, not plugins
- Consolidate build requirements
- Use -p with install to preserve timestamps

I have not yet implemented the compiler flags; I am trying to figure out the best way to do this due to the project's use of scons.

Comment 7 Volker Fröhlich 2011-06-27 19:44:35 UTC
Try "env" in SConstrust!

You forgot to require GIMP for the Plug-in.

Please put each BuildRequires on a separate line.

"%{_mandir}/man1/maze5.1.gz" -- Rather make that "%{_mandir}/man1/maze5.1.*"

The rpmlint issues are still there, I suppose. chmod +x on the plug-in .so may solve the stripping problem.

Comment 8 Veeti Paananen 2011-07-27 15:47:57 UTC
You could take a look at how the Blender package handles (or handled, they're using CMake now) scons:

- You can actually add %{?_smp_mflags} after the scons command since scons uses the same syntax as make for the number of jobs.

- You'll probably have to patch the SConstruct file to read the RPM environment variable "RPM_OPT_FLAGS" and append its contents to CCFLAGS and CXXFLAGS in the scons "Environment" object.

Comment 9 Veeti Paananen 2011-07-27 15:49:43 UTC
Oh, and the require for the base package has to be in the format "%{name}%{?_isa} = %{version}-%{release}".

Comment 10 Volker Fröhlich 2011-08-13 06:20:22 UTC
Are you still interested in this package?

Comment 11 Volker Fröhlich 2011-10-08 07:17:22 UTC
I guess this review is stalled. If you are still interested, please respond!

Otherwise I'll close this bug as described in http://fedoraproject.org/wiki/Policy_for_stalled_package_reviews

Feel free to re-open it any time though.


Note You need to log in before you can comment on or make changes to this bug.