[Bug 770740] New: Review Request: Morse - Simulates robots using the Blender Game Engine
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: Morse - Simulates robots using the Blender Game Engine
https://bugzilla.redhat.com/show_bug.cgi?id=770740
Summary: Review Request: Morse - Simulates robots using the Blender Game Engine Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: unspecified Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: spencerandrewjackson@yahoo.com QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, package-review@lists.fedoraproject.org Classification: Fedora Story Points: --- Type: --- Regression: --- Mount Type: --- Documentation: ---
Spec URL: https://dl-web.dropbox.com/get/Public/morse.spec?w=5c5ce0b5 SRPM URL: http://dl.dropbox.com/u/4517312/morse-0.4.1-0.fc16.src.rpm Description:
Hi!
I've been using Morse for a university project team, and thought I'd package it up for Fedora. A review would be much appreciated, especially as I had to modify some of the installation paths. I sent the changes I made to Morse's upstream.
Morse is a 3D robotics simulator, using the Blender Game Engine. A roboticist may create a 3D model, then attach actuators and sensors. Morse then allows them to control the model and interact with the simulated environment. A variety of middleware protocols are supported, but many of these don't seem to be packaged yet for Fedora.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=770740
--- Comment #1 from Spencer Jackson spencerandrewjackson@yahoo.com 2011-12-28 15:06:29 EST --- I forgot to mention, this is my first package, and I believe I need a sponsor... Any help in that regard would be much appreciated.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=770740
Spencer Jackson spencerandrewjackson@yahoo.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |177841(FE-NEEDSPONSOR)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=770740
--- Comment #2 from Spencer Jackson spencerandrewjackson@yahoo.com 2011-12-30 15:26:23 EST --- New spec file. Spec URL: http://dl.dropbox.com/u/4517312/morse.spec SRPM URL: http://dl.dropbox.com/u/4517312/morse-0.4.1-1.fc16.src.rpm
Builds and installs documentation, correctly forces the use of python3 while bytecode compiling, as well as miscellaneous other cleanups.
rpmlint output:
[makerpm@khezef rpmbuild]$ rpmlint SPECS/morse.spec /var/lib/mock/fedora-16-x86_64/result/morse-* morse morse-debuginfo SPECS/morse.spec: W: invalid-url Source0: laas-morse-0.4.1-0-g15dc857.tar.gz morse.src: W: spelling-error %description -l en_US middleware -> middle ware, middle-ware, middleweight morse.src: W: invalid-url Source0: laas-morse-0.4.1-0-g15dc857.tar.gz morse.x86_64: W: no-manual-page-for-binary morseexec morse.x86_64: W: no-manual-page-for-binary morse morse.x86_64: W: no-manual-page-for-binary morseexec morse.x86_64: W: no-manual-page-for-binary morse 5 packages and 1 specfiles checked; 0 errors, 7 warnings.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=770740
adev adev88@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |adev88@gmail.com
--- Comment #3 from adev adev88@gmail.com 2012-01-06 11:25:57 EST --- Hi,
Quick comments about your spec file :
- use the %{_isa} macro for your "Requires" dependencies on binary packages.
- "-DCMAKE_INSTALL_PREFIX=/usr" is useless, it is already included in the %cmake macro
- %{_libdir} could be used instead of /usr/lib64/ with your cmake command
- same with %{_includedir} instead of /usr/include
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=770740
--- Comment #4 from Spencer Jackson spencerandrewjackson@yahoo.com 2012-01-08 17:32:16 EST --- Thanks, adev! I've applied your recommendations to the spec.
New spec URL: http://dl.dropbox.com/u/4517312/morse.spec New SRPM URL: http://dl.dropbox.com/u/4517312/morse-0.4.1-2.fc16.src.rpm
rpmlint morse morse-debuginfo RPMS/x86_64/morse-0.4.1-2.fc16.x86_64.rpm RPMS/x86_64/morse-debuginfo-0.4.1-2.fc16.x86_64.rpm SRPMS/morse-0.4.1-2.fc16.src.rpm SPECS/morse.spec morse.x86_64: W: no-manual-page-for-binary morseexec morse.x86_64: W: no-manual-page-for-binary morse morse.x86_64: W: no-manual-page-for-binary morseexec morse.x86_64: W: no-manual-page-for-binary morse morse.src: W: spelling-error %description -l en_US middleware -> middle ware, middle-ware, middleweight morse.src: W: invalid-url Source0: laas-morse-0.4.1-0-g15dc857.tar.gz SPECS/morse.spec: W: invalid-url Source0: laas-morse-0.4.1-0-g15dc857.tar.gz 5 packages and 1 specfiles checked; 0 errors, 7 warnings.
I just sent an email to the packaging list, ( http://lists.fedoraproject.org/pipermail/packaging/2012-January/008067.html ) basically asking what to do about the middleware protocols which are supported by Morse, but not packaged in Fedora. Some of these, like ROS, might take a while to be supported. Any thoughts on how to handle that would be appreciated.
Upstream also recommended that I create subpackages for each middleware. That makes a lot of sense, but I should probably figure out what's the best way to handle the unsupported middlewares before I do that...
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=770740
adev adev88@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |pingou@pingoured.fr
--- Comment #5 from adev adev88@gmail.com 2012-01-09 05:49:39 EST --- Hi, This is an informal review, I am not a sponsor.
[PASS] MUST: rpmlint must be run on the source rpm and all binary rpms the build produces. The output should be posted in the review.
morse.src: I: enchant-dictionary-not-found en_US morse.src: W: invalid-url Source0: laas-morse-0.4.1-0-g15dc857.tar.gz morse.x86_64: W: no-manual-page-for-binary morseexec morse.x86_64: W: no-manual-page-for-binary morse 3 packages and 0 specfiles checked; 0 errors, 4 warnings.
-> no man pages
[PASS] MUST: The package must be named according to the Package Naming Guidelines . [PASS] MUST: The spec file name must match the base package %{name}, in the format %{name}.spec unless your package has an exemption. [PASS] MUST: The package must meet the Packaging Guidelines.
[PASS] MUST: The package must be licensed with a Fedora approved license and meet the Licensing Guidelines . [PASS] MUST: The License field in the package spec file must match the actual license. [PASS] 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. [PASS] MUST: The spec file must be written in American English. [PASS] MUST: The spec file for the package MUST be legible. [PASS] MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. Reviewers should use md5sum for this task. If no upstream URL can be specified for this package, please see the Source URL Guidelines for how to deal with this.
rpmdev-md5 morse-0.4.1-2.fc16.src.rpm 293849e6e24b0b3adf2dbd9ae0064300 morse-0.4.1-2.fc16.src.rpm 0e9c30808c82062d284a528c052e16f2 morse.desktop 794f77239d535d61861426d450b66850 morse-0.4.1-lsbpathfix.patch a2b9c70415653665ad5d90504ffebc5e morse-0.4.1.usepythonsitearch.patch b57e3f3dd1eed67ce201ddfa14b920ad laas-morse-0.4.1-0-g15dc857.tar.gz 5ced6d1eec3f7a66e6f0e41bc64de92d morse.spec
md5sum 0.4.1 b57e3f3dd1eed67ce201ddfa14b920ad 0.4.1
[PASS] MUST: The package MUST successfully compile and build into binary rpms on at least one primary architecture. -> see koji [PASS] MUST: If the package does not successfully compile, build or work on an architecture, then those architectures should be listed in the spec in ExcludeArch. Each architecture listed in ExcludeArch MUST have a bug filed in bugzilla, describing the reason that the package does not compile/build/work on that architecture. The bug number MUST be placed in a comment, next to the corresponding ExcludeArch line. [PASS] MUST: All build dependencies must be listed in BuildRequires, except for any that are listed in the exceptions section of the Packaging Guidelines ; inclusion of those as BuildRequires is optional. Apply common sense. [PASS] MUST: The spec file MUST handle locales properly. This is done by using the %find_lang macro. Using %{_datadir}/locale/* is strictly forbidden. [PASS] MUST: Every binary RPM package (or subpackage) which stores shared library files (not just symlinks) in any of the dynamic linker's default paths, must call ldconfig in %post and %postun. [PASS] MUST: Packages must NOT bundle copies of system libraries. [PASS] MUST: If the package is designed to be relocatable, the packager must state this fact in the request for review, along with the rationalization for relocation of that specific package. Without this, use of Prefix: /usr is considered a blocker. [PASS] MUST: A package must own all directories that it creates. If it does not create a directory that it uses, then it should require a package which does create that directory. [PASS] MUST: A Fedora package must not list a file more than once in the spec file's %files listings. (Notable exception: license texts in specific situations) [PASS] MUST: Permissions on files must be set properly. Executables should be set with executable permissions, for example. [PASS] MUST: Each package must consistently use macros. [PASS] MUST: The package must contain code, or permissable content. [PASS] MUST: Large documentation files must go in a -doc subpackage. (The definition of large is left up to the packager's best judgement, but is not restricted to size. Large can refer to either size or quantity). [PASS] MUST: If a package includes something as %doc, it must not affect the runtime of the application. To summarize: If it is in %doc, the program must run properly if it is not present. [PASS] MUST: Header files must be in a -devel package. [PASS] MUST: Static libraries must be in a -static package. [PASS] MUST: If a package contains library files with a suffix (e.g. libfoo.so.1.1), then library files that end in .so (without suffix) must go in a -devel package. [PASS] MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency: Requires: %{name}%{?_isa} = %{version}-%{release} [PASS] MUST: Packages must NOT contain any .la libtool archives, these must be removed in the spec if they are built. [PASS] MUST: Packages containing GUI applications must include a %{name}.desktop file, and that file must be properly installed with desktop-file-install in the %install section. If you feel that your packaged GUI application does not need a .desktop file, you must put a comment in the spec file with your explanation. [PASS] MUST: Packages must not own files or directories already owned by other packages. The rule of thumb here is that the first package to be installed should own the files or directories that other packages may rely upon. This means, for example, that no package in Fedora should ever share ownership with any of the files or directories owned by the filesystem or man package. If you feel that you have a good reason to own a file or directory that another package owns, then please present that at package review time. [PASS] MUST: All filenames in rpm packages must be valid UTF-8.
[PASS] 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. [PASS] SHOULD: The description and summary sections in the package spec file should contain translations for supported Non-English languages, if available. [PASS] SHOULD: The reviewer should test that the package builds in mock. [PASS] SHOULD: The package should compile and build into binary rpms on all supported architectures.
koji build --arch-override=x86_64 --scratch f16-candidate morse-0.4.1-2.fc16.src.rpm Uploading srpm: morse-0.4.1-2.fc16.src.rpm [====================================] 100% 00:05:49 75.06 MiB 219.88 KiB/sec Created task: 3632889 Task info: http://koji.fedoraproject.org/koji/taskinfo?taskID=3632889 Watching tasks (this may be safely interrupted)... 3632889 build (f16-candidate, morse-0.4.1-2.fc16.src.rpm): free 3632889 build (f16-candidate, morse-0.4.1-2.fc16.src.rpm): free -> open (ppc12.phx2.fedoraproject.org) 3632890 buildArch (morse-0.4.1-2.fc16.src.rpm, x86_64): open (x86-03.phx2.fedoraproject.org) 3632890 buildArch (morse-0.4.1-2.fc16.src.rpm, x86_64): open (x86-03.phx2.fedoraproject.org) -> closed 0 free 1 open 1 done 0 failed 3632889 build (f16-candidate, morse-0.4.1-2.fc16.src.rpm): open (ppc12.phx2.fedoraproject.org) -> closed 0 free 0 open 2 done 0 failed
koji build --arch-override=i386 --scratch f16-candidate morse-0.4.1-2.fc16.src.rpm Uploading srpm: morse-0.4.1-2.fc16.src.rpm [====================================] 100% 00:05:52 75.06 MiB 217.76 KiB/sec Created task: 3632894 Task info: http://koji.fedoraproject.org/koji/taskinfo?taskID=3632894 Watching tasks (this may be safely interrupted)... 3632894 build (f16-candidate, morse-0.4.1-2.fc16.src.rpm): open (x86-16.phx2.fedoraproject.org) 3632895 buildArch (morse-0.4.1-2.fc16.src.rpm, i386): open (x86-13.phx2.fedoraproject.org) 3632895 buildArch (morse-0.4.1-2.fc16.src.rpm, i386): open (x86-13.phx2.fedoraproject.org) -> closed 0 free 1 open 1 done 0 failed 3632894 build (f16-candidate, morse-0.4.1-2.fc16.src.rpm): open (x86-16.phx2.fedoraproject.org) -> closed 0 free 0 open 2 done 0 failed
3632894 build (f16-candidate, morse-0.4.1-2.fc16.src.rpm) completed successfully
[PASS] SHOULD: The reviewer should test that the package functions as described. A package should not segfault instead of running, for example. [PASS] SHOULD: If scriptlets are used, those scriptlets must be sane. This is vague, and left up to the reviewers judgement to determine sanity. [PASS] SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency. [PASS] SHOULD: The placement of pkgconfig(.pc) files depends on their usecase, and this is usually for development purposes, so should be placed in a -devel pkg. A reasonable exception is that the main pkg itself is a devel tool not installed in a user runtime, e.g. gcc or gdb. [PASS] SHOULD: If the package has file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin consider requiring the package which provides the file instead of the file itself. [FAIL] SHOULD: your package should contain man pages for binaries/scripts. If it doesn't, work with upstream to add them where they make sense.
-> add man pages
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=770740
--- Comment #6 from Spencer Jackson spencerandrewjackson@yahoo.com 2012-01-12 15:35:43 EST --- Just uploaded a new spec Spec URL: http://dl.dropbox.com/u/4517312/morse.spec SRPM URL: http://dl.dropbox.com/u/4517312/morse-0.4.1-3.fc16.src.rpm
I've added the missing man pages, as well as properly generate documentation which is placed into a doc subpackage. Further, I've added optional build time dependencies on features which have unpackaged dependencies.
[makerpm@khezef rpmbuild]$ rpmlint morse morse-debuginfo morse-doc SPECS/morse.spec /var/lib/mock/fedora-16-x86_64/result/morse-0.4.1-3.fc16.src.rpm morse.x86_64: W: spelling-error %description -l en_US ros -> rps, rod, rs morse.x86_64: W: spelling-error %description -l en_US Middleware -> Middle ware, Middle-ware, Middleweight morse.x86_64: W: spelling-error %description -l en_US subpackages -> sub packages, sub-packages, prepackages SPECS/morse.spec: W: invalid-url Source0: laas-morse-0.4.1-0-g15dc857.tar.gz morse.src: W: spelling-error %description -l en_US middleware -> middle ware, middle-ware, middleweight morse.src: W: spelling-error %description -l en_US ros -> rps, rod, rs morse.src: W: spelling-error %description -l en_US Middleware -> Middle ware, Middle-ware, Middleweight morse.src: W: spelling-error %description -l en_US subpackages -> sub packages, sub-packages, prepackages morse.src: W: invalid-url Source0: laas-morse-0.4.1-0-g15dc857.tar.gz 4 packages and 1 specfiles checked; 0 errors, 9 warnings.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=770740
--- Comment #7 from Spencer Jackson spencerandrewjackson@yahoo.com 2012-01-17 16:13:38 EST --- New spec Spec URL: http://dl.dropbox.com/u/4517312/morse.spec SRPM URL: http://dl.dropbox.com/u/4517312/morse-0.4.1-4.fc16.src.rpm
[makerpm@khezef rpmbuild]$ rpmlint /var/lib/mock/fedora-16-x86_64/result/morse-* morse morse-doc morse-debuginfo SPECS/morse.spec morse.src: W: spelling-error %description -l en_US middleware -> middle ware, middle-ware, middleweight morse.src: W: invalid-url Source0: laas-morse-0.4.1-0-g15dc857.tar.gz SPECS/morse.spec: W: invalid-url Source0: laas-morse-0.4.1-0-g15dc857.tar.gz 7 packages and 1 specfiles checked; 0 errors, 3 warnings.
https://bugzilla.redhat.com/show_bug.cgi?id=770740
--- Comment #8 from Pierre-YvesChibon pingou@pingoured.fr --- @Adrien, now that you are an official packager, what about finishing this review ?
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=770740
Jason Tibbitts tibbs@math.uh.edu changed:
What |Removed |Added ---------------------------------------------------------------------------- Summary|Review Request: Morse - |Review Request: morse - |Simulates robots using the |Simulates robots using the |Blender Game Engine |Blender Game Engine Flags| |needinfo?(spencerandrewjack | |son@yahoo.com)
--- Comment #9 from Jason Tibbitts tibbs@math.uh.edu --- I am triaging old review tickets. I can't promise a review if you reply, but by closing out the stale tickets we can devote extra attention to the ones which aren't stale.
This fails to build for me; here's a koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=5322961
In addition, we do have guidelines for dealing with github which you might want to look over: http://fedoraproject.org/wiki/Packaging:SourceURL
https://bugzilla.redhat.com/show_bug.cgi?id=770740
Raphael Groner projects.rg@smart.ms changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |spencerandrewjackson@yahoo. | |com Flags| |needinfo?(spencerandrewjack | |son@yahoo.com)
--- Comment #10 from Raphael Groner projects.rg@smart.ms --- Any news here? I could take the review process if you may like.
https://bugzilla.redhat.com/show_bug.cgi?id=770740
Raphael Groner projects.rg@smart.ms changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|nobody@fedoraproject.org |projects.rg@smart.ms
--- Comment #11 from Raphael Groner projects.rg@smart.ms --- Your spec file is not ready for a formal review. Please fix following listed points. After done that, I could have a deeper look.
# Tarballs can be obtained from GitHub at https://github.com/laas/morse/tarball/0.4.1 Source0: laas-morse-0.4.1-0-g15dc857.tar.gz
%global name morse %global commit g15dc857 %global version 0.4.1 Source0: https://github.com/laas/%%7Bname%7D/tarball/%%7Bversion%7D#/laas-name-%%7Bve...
Analogously, you should give the individual URL directly into PatchX.
%{?with_pocolibs:
What's that? I would not suggest conditional packaging. What is the purpose?
Requires: %{name}%{?_isa} = %{version}-%{release}
Remove %{?_isa} completely from Requires. Otherwise, you won't be able to build cross-arch.
BuildRequires: swig BuildRequires: pocolibs-devel
Move those on top to the BR for the main package. There's only one %build allowed for all sub-packages.
%build:
You should use %cmake macro instead of direct cmake call with tons of standard parameters. https://fedoraproject.org/wiki/Packaging:Cmake?rd=Packaging/cmake
pushd doc make popd
cd doc ; make
%install desktop-file-install --vendor="fedora" \
The Vendor tag should not be used. It is set automatically by the build system. https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#...
# Move …
Use install -d instead of mkdir -p to preserve timestamps etc.
https://bugzilla.redhat.com/show_bug.cgi?id=770740
--- Comment #12 from Raphael Groner projects.rg@smart.ms --- https://fedoraproject.org/wiki/Packaging/SourceURL
https://fedoraproject.org/wiki/Packaging:Guidelines#Timestamps
https://bugzilla.redhat.com/show_bug.cgi?id=770740
Raphael Groner projects.rg@smart.ms changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(spencerandrewjack | |son@yahoo.com)
--- Comment #13 from Raphael Groner projects.rg@smart.ms --- ping? Are you still interested in maintaining this package?
https://bugzilla.redhat.com/show_bug.cgi?id=770740
Raphael Groner projects.rg@smart.ms changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |CLOSED Blocks| |201449 (FE-DEADREVIEW) Resolution|--- |NOTABUG Last Closed| |2015-01-26 11:39:57
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=201449 [Bug 201449] FE-DEADREVIEW -- Reviews stalled due to lack of submitter response should be blocking this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=770740
Miroslav Suchý msuchy@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks|177841 (FE-NEEDSPONSOR) |
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=177841 [Bug 177841] Tracker: Review requests from new Fedora packagers who need a sponsor
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla