Spec URL: http://hedayat.fedorapeople.org/simspark_review/0.1-1/simspark.spec SRPM URL: http://hedayat.fedorapeople.org/simspark_review/0.1-1/simspark-0.1-1.fc10.src.rpm Description: Spark is a physical simulation system. The primary purpose of this system is to provide a *generic* simulator for different kinds of simulations. In these simulations, agents can participate as external processes. Koji build: https://koji.fedoraproject.org/koji/taskinfo?taskID=1139976 Rpmlint output: Doesn't report any errors for src.rpm and spec files. But for rpm packages: simspark.x86_64: W: dangling-symlink /usr/share/simspark/fonts/VeraMono.ttf /usr/share/fonts/dejavu/DejaVuSansMono.ttf simspark.x86_64: W: symlink-should-be-relative /usr/share/simspark/fonts/VeraMono.ttf /usr/share/fonts/dejavu/DejaVuSansMono.ttf simspark-devel.x86_64: E: only-non-binary-in-usr-lib Due to the new font packaging guidelines, this package doesn't contain any font files and it points to a font from dejavu-fonts-sans-mono package instead. And this package requires dejavu-fonts-sans-mono because of this. Extra comments: In the latest version of rcssserver3d package (rcssserver3d-0.6 package is already in Fedora), this package is split into some sub-packages by upstream. Two main sub-packages are simspark and rcssserver3d. simspark package provides core libraries which were previously part of rcssserver3d package. But since simspark is a generic simulation framework and not soccer specific, it is provided as a separate package now. So, the next version of rcssserver3d package (version 0.6.1 and later versions) will depend on this package. A question: Because of this separation, this package have a few files in /usr/bin which conflicts with rcssserver3d-0.6. (But rcssserver3d-0.6.1 won't have any conflicting files with simspark package). Therefore, users which simply update rcssserver3d will not face any problems, but if someone which has rcssserver3d-0.6 decides to install simspark, he can't do that since these packages have some conflicting files. Should I add a Conflicts: section to simspark spec file?!
OK, I'll fix the issue with DevIL-1.7.8 with a patch. Please continue the review.
Some notes: * BR - Would you explain where "BR: ImageMagick" is used? - "BR: gcc-c++" is redundant. * Requires - Unfortunately, on F-11 dejavu-fonts-sans-mono is renamed to dejavu-sans-mono-fonts for some reason... - Is it what you expect that simspark main package always install ruby? I always have ruby installed on my system so I don't care about ruby dependency, however it does not seem that this package should always require ruby. Maybe splitting files depending on ruby into seperate subpackage is preferable. * Build - As you already saw, this package won't build with DevIL 1.7.8 (i.e. rawhide) * Cflags - build.log shows some compilation information: ------------------------------------------------------------------ [ 1%] Building CXX object utility/tinyxml/CMakeFiles/tinyxml_ex.dir/tinyxml.cpp.o [ 1%] Building C object utility/sfsexp/CMakeFiles/sexp.dir/io.c.o ------------------------------------------------------------------ which is not useful. Please make build.log more verbose (ref: https://fedoraproject.org/wiki/Packaging/cmake ) - And "make VERBOSE=1" actually shows Fedora specific compilation flags are not correctly honored, ref: http://koji.fedoraproject.org/koji/taskinfo?taskID=1241115 - Also, Fedora's default optimization level is -O2, while this package use -O3. * ldconfig call - Calling /sbin/ldconfig on scriptlets is not needed for this package because no libraries are installed under default ldconfig search paths. ! By the way, please check if it is as you expect that no libraries are installed under default ldconfig search paths. (In reply to comment #0) > Should I add a Conflicts: section to simspark spec file?! - I would write Explicit conflicts "Conflicts: rcssserver3d < 0.6.1", however other reviewers may say that it is not needed.
Thank you for reviewing the package. * BR: - Latex related cmake files look for "convert" utility of ImageMagic and will fail if it does not exist. I'm not sure if it is actually used during documentation generation. - OK, will be removed * Requires: - Yes, each distro (F9, F10 and F11) use a different name! I'll either add conditional statements for each distro or a file level dependency. I think package dependency is preferred so I'll add that. (any suggestions?) - Yes. Ruby is highly integrated into simspark and is used to glue different plugins and subsystems together to create a functional part. So, I think there is no need to create subpackages for it. (to be more specific, zeitgeist library requires ruby, and other main libraries(oxygen and kerosin) require zeitgeist). * Build: - Fixed. * Cflags: - sorry for that :( these issues will be fixed. * ldconfig: - Yes, it is the default behavior of the upstream package. But, while most of the libraries are plugins, some of them are libraries which executable files will be linked to. Should I create a config file in /etc/ld.so.conf.d for /usr/lib{64}/simspark ?! * OK, I'll add the conflicts statement.
(In reply to comment #3) > Thank you for reviewing the package. > * BR: > - Latex related cmake files look for "convert" utility of ImageMagic and will > fail if it does not exist. I'm not sure if it is actually used during > documentation generation. - Then okay. > * Requires: > - Yes, each distro (F9, F10 and F11) use a different name! I'll either add > conditional statements for each distro or a file level dependency. I think > package dependency is preferred so I'll add that. (any suggestions?) - Package dependency is preferred than file dependency. > - Yes. Ruby is highly integrated into simspark and is used to glue different > plugins and subsystems together to create a functional part. So, I think there > is no need to create subpackages for it. (to be more specific, zeitgeist > library requires ruby, and other main libraries(oxygen and kerosin) require > zeitgeist). - Thank you for explanation. > * ldconfig: > - Yes, it is the default behavior of the upstream package. But, while most of > the libraries are plugins, some of them are libraries which executable files > will be linked to. Should I create a config file in /etc/ld.so.conf.d for > /usr/lib{64}/simspark ?! - Theoretically if the binaries trying to link against libraries under %_libdir/simspark use rpath, there is no need to use ld.so.conf method. However if some of the libraries under %_libdir/simspark are really _system wide_ libraries, I would suggest that such libraries should be moved under %_libdir.
New files: SPEC:http://hedayat.fedorapeople.org/simspark_review/0.1-2/simspark.spec SRPM:http://hedayat.fedorapeople.org/simspark_review/0.1-2/simspark-0.1-2.fc10.src.rpm
Well, for -2: * BR - build.log shows: --------------------------------------------------------------- 99 -- Found the following Boost libraries: 100 -- thread 101 -- regex 102 -- Could NOT find wxWidgets (missing: wxWidgets_FOUND) --------------------------------------------------------------- It seems that this can be enabled by BR: wxGTK-devel: http://koji.fedoraproject.org/koji/taskinfo?taskID=1246607 * Path incorrectness - %_bindir/*-config (in -devel package) contains some wrong path information. e.g. /usr/bin/oxygen-config on i386: --------------------------------------------------------------- 95 fi 96 echo -L/usr//usr/lib/simspark $libs 97 fi 104 fi 105 echo /usr//usr/lib/simspark/$convlib 106 fi ---------------------------------------------------------------
* BR: Yes, that's intentional. Currently, wx related plugins are not used (they are used by a program named rsgedit, but it is still not ready for production). I also didn't include them in rcssserver3d-0.6 package. And since wxGTK dependency is not desired for people who don't need wx related plugins, I think I would create a subpackage for them. Anyway, if you think that it is better to include them now, I will create a sub-package for them. * OK, will be fixed. Thanks.
SRPM: http://hedayat.fedorapeople.org/simspark_review/0.1-3/simspark-0.1-3.fc10.src.rpm SPEC: http://hedayat.fedorapeople.org/simspark_review/0.1-3/simspark.spec
(In reply to comment #7) > * BR: > Yes, that's intentional. Currently, wx related plugins are not used (they are > used by a program named rsgedit, but it is still not ready for production). I > also didn't include them in rcssserver3d-0.6 package. And since wxGTK > dependency is not desired for people who don't need wx related plugins, I think > I would create a subpackage for them. In such case, explicitly disabling wxGTK support is recommended. Are there some option for cmake like "--disable-wx"??
No, there is no such options currently. It just considers wxWidgets as an optional part, so it won't build wx plugins when it doesn't exists. Do you think it is required?
Then, okay. ------------------------------------------------------------- This package (simspark) is APPROVED by mtasaka -------------------------------------------------------------
New Package CVS Request ======================= Package Name: simspark Short Description: Spark physical simulation system Owners: hedayat Branches: F-9 F-10
cvs done.
simspark-0.1-3.fc9 has been submitted as an update for Fedora 9. http://admin.fedoraproject.org/updates/simspark-0.1-3.fc9
simspark-0.1-3.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/simspark-0.1-3.fc10
Can't F-10/9 rcssserver3d be upgraded so as not to get conflict with simspark before simspark is pushed into F-10/9 repositories? Otherwise after simspark is pushed into repositories I am not sure if "$ yum -y install rcssserver3d" still works.
new version of rcssserver3d requires simspark, so I wonder if it is possible to build the new version of rcssserver3d for F-10/9 before simspark appears in the repositories. Anyway, there should be no problem installing the current version of rcssserver3d if the user don't install simspark. BTW, I will update rcssserver3d today, so it should not be a big problem.
(In reply to comment #17) > new version of rcssserver3d requires simspark, so I wonder if it is possible to > build the new version of rcssserver3d for F-10/9 before simspark appears in the > repositories. Well, actually it is possible - Ask rel-eng team (i.e. create a ticket on https://fedorahosted.org/rel-eng/ ) to get simspark tagged as dist-f{9,10}-override Example: https://fedorahosted.org/rel-eng/ticket/1065 - After tagging is done by rel-eng team, simspark is added to koji buildroot, however still it is not pushed into stable repository - After rebuilding new rcssserver3d against simspark is done, you can submit a push request on bodhi for simspark and rcssserver3d as "one set". - After simspark and new rcssserver3d are tagged as dist-f{9,10}-updates, you should ask rel-eng team to remove dist-f{9,10}-override tag.
simspark-0.1-3.fc9 has been pushed to the Fedora 9 stable repository. If problems still persist, please make note of it in this bug report.
simspark-0.1-3.fc10 has been pushed to the Fedora 10 stable repository. If problems still persist, please make note of it in this bug report.
(In reply to comment #18) > Well, actually it is possible > - Ask rel-eng team (i.e. create a ticket on > https://fedorahosted.org/rel-eng/ ) to get simspark tagged as > dist-f{9,10}-override > Example: > https://fedorahosted.org/rel-eng/ticket/1065 > - After tagging is done by rel-eng team, simspark is added to koji buildroot, > however still it is not pushed into stable repository > - After rebuilding new rcssserver3d against simspark is done, > you can submit a push request on bodhi for simspark and rcssserver3d > as "one set". > - After simspark and new rcssserver3d are tagged as dist-f{9,10}-updates, > you should ask rel-eng team to remove dist-f{9,10}-override tag. Thank you very much. I should remember that the process could be more flexible sometimes! :) simspark is in the repositories now, but I will remember what you said for future. Thanks a lot