Bug 714543 - Review Request: maze5 - A program for generating mazes of miscellaneous styles and sizes
Review Request: maze5 - A program for generating mazes of miscellaneous style...
Status: CLOSED NOTABUG
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Nobody's working on this, feel free to take it
Fedora Extras Quality Assurance
Stalled review
:
Depends On:
Blocks: FE-DEADREVIEW
  Show dependency treegraph
 
Reported: 2011-06-19 22:18 EDT by W. Michael Petullo
Modified: 2011-12-09 15:39 EST (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2011-12-09 15:39:59 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description W. Michael Petullo 2011-06-19 22:18:47 EDT
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 17:25:24 EDT
Just commenting: wouldn't it make sense to split the GIMP plugin into a subpackage?
Comment 2 Veeti Paananen 2011-06-20 17:27:37 EDT
Oh, and you need to add scons as a build requirement.
Comment 3 Yanchuan Nian 2011-06-22 08:21:53 EDT
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 11:39:29 EDT
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 19:03:53 EDT
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 14:29:08 EDT
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 15:44:35 EDT
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 11:47:57 EDT
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 11:49:43 EDT
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 02:20:22 EDT
Are you still interested in this package?
Comment 11 Volker Fröhlich 2011-10-08 03:17:22 EDT
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.