Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/andy-super-great-park.spec SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/andy-super-great-park-1.0.8-1.fc21.src.rpm Description: Andy's Super Great Park is an original game with an inventive and easy to learn gameplay, where every action can be simply done with a mouse. Challenge your reflexes and your dexterity in this adventure of 25 to 43 levels, punctuated by amazing boss fights. Through 25 main levels, you will have to retrieve a minimum number of balloons using your plunger gun while avoiding collisions with the obstacles dropped on the path (explosive zeppelins, crates, TNT…). Try to get the best scores and unlock up to 18 extra levels! Grabbing balloons with a plunger gun, shooting birds with a cannon and exploding everything else, all while riding a roller coaster: this is Andy's Super Great Park! Fedora Account System Username: martinkg %changelog * Fri Dec 19 2014 Martin Gansser <martinkg> - 1.0.8-1 - Initial build. rpmlint andy-super-great-park andy-super-great-park-data bear-factory andy-super-great-park.x86_64: W: spelling-error %description -l en_US gameplay -> game play, game-play, nameplate andy-super-great-park.x86_64: W: no-manual-page-for-binary andy-super-great-park andy-super-great-park-data.noarch: W: spelling-error %description -l en_US gameplay -> game play, game-play, nameplate andy-super-great-park-data.noarch: W: no-documentation bear-factory.x86_64: E: invalid-soname /usr/lib64/libbear-editor.so libbear-editor.so bear-factory.x86_64: W: no-manual-page-for-binary bf-level-editor bear-factory.x86_64: W: no-manual-page-for-binary bf-model-editor bear-factory.x86_64: W: no-manual-page-for-binary bf-animation-editor bear-factory.x86_64: W: no-manual-page-for-binary bend-image bear-factory.x86_64: W: desktopfile-without-binary /usr/share/applications/desc2img.desktop desc2img 3 packages and 0 specfiles checked; 1 errors, 9 warnings.
Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/andy-super-great-park.spec SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/andy-super-great-park-1.0.8-2.fc21.src.rpm %changelog * Sat Dec 20 2014 Martin Gansser <martinkg> - 1.0.8-2 - dropped -DCMAKE_SKIP_RPATH:BOOL=ON from cmake - added BR chrpath - using chrpath to get rid of --rpath in bear-factory
Please make the build verbose and make Source0 a URL.
Hi Martin, there are some hints for your spec file. Maybe I can do the official review as well. * SHOULD consider to name the package 'asqp' as upstream does partly for the project name? Well, I can see that the source tarball is using that full name like in the subject. https://fedoraproject.org/wiki/Packaging:NamingGuidelines#General_Naming * SHOULD remove the Group tag cause it's obosolete. Note: This tag is deprecated since Fedora 17. https://fedoraproject.org/wiki/How_to_create_an_RPM_package#SPEC_file_overview * MUST use a valid Source URL, or add a comment how to build the tarball. https://fedoraproject.org/wiki/Packaging:SourceURL?rd=Packaging/SourceURL This? http://www.stuff-o-matic.com/asgp/download/download.php?platform=source * MUST build with mock and koji (scratch) to ensure all BR are correct. https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRequires https://fedoraproject.org/wiki/Using_the_Koji_build_system#Scratch_Builds * SHOULD fix/patch CMakeLists.txt to not enforce docbook2x as it seems to create conflicts. And please send your patch to upstream, do they know about that issue? https://fedoraproject.org/wiki/Packaging:Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment > # Cmake suggests it but "parser error" will be got. > BuildConflicts: docbook2x * SHOULD what is "Plee the Bear"? How is it related to this package? You won't be able to create two individual subpackges for that 'bear' and 'bear-factory' stuff. So consider to package that separately and unbundle. https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries > of the Bear Engine for Plee the Bear & Andy's Super Great Park. * MUST 'Require: hicolor-icon-theme' cause of the folder ownership. https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#File_and_Directory_Ownership > %files > … > %{_datadir}/icons/hicolor/*/apps/asgp.png * MUST (when possible) use the %cmake macro to avoid relisting of all those parameters like RPATH etc. https://fedoraproject.org/wiki/Packaging:Cmake
Hi Raphael, thanks for your reply, (In reply to Raphael Groner from comment #3) > Hi Martin, > > there are some hints for your spec file. Maybe I can do the official review > as well. > > * SHOULD consider to name the package 'asqp' as upstream does partly for the > project name? Well, I can see that the source tarball is using that full > name like in the subject. > https://fedoraproject.org/wiki/Packaging:NamingGuidelines#General_Naming > ok, i will name the package/spec file asgp/asgp.spec > * SHOULD remove the Group tag cause it's obosolete. Note: This tag is > deprecated since Fedora 17. > https://fedoraproject.org/wiki/ > How_to_create_an_RPM_package#SPEC_file_overview > done > * MUST use a valid Source URL, or add a comment how to build the tarball. > https://fedoraproject.org/wiki/Packaging:SourceURL?rd=Packaging/SourceURL > > This? > http://www.stuff-o-matic.com/asgp/download/download.php?platform=source > yes this is a valid link for asgp-1.0.8 but there is a recent version 1.0.12 on https://github.com/j-jorge/asgp/ build instruction: mkdir asgp-build cd asgp-build git clone https://github.com/j-jorge/bear.git git clone https://github.com/j-jorge/asgp.git [root@fc21 asgp-build]# du -sk * 1140264 asgp 37616 bear approx. 1,17 GByte should we use this version ? > > * MUST build with mock and koji (scratch) to ensure all BR are correct. > https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRequires > https://fedoraproject.org/wiki/Using_the_Koji_build_system#Scratch_Builds > already done, but no additional build requires are detected. > * SHOULD fix/patch CMakeLists.txt to not enforce docbook2x as it seems to > create conflicts. And please send your patch to upstream, do they know about > that issue? > https://fedoraproject.org/wiki/Packaging: > Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment > > # Cmake suggests it but "parser error" will be got. > > BuildConflicts: docbook2x > can you send me a patch ? > * SHOULD what is "Plee the Bear"? How is it related to this package? You > won't be able to create two individual subpackges for that 'bear' and > 'bear-factory' stuff. So consider to package that separately and unbundle. > https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries > > of the Bear Engine for Plee the Bear & Andy's Super Great Park. > not done, because not clear. > * MUST 'Require: hicolor-icon-theme' cause of the folder ownership. > https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/ > Guidelines#File_and_Directory_Ownership > > %files > > … > > %{_datadir}/icons/hicolor/*/apps/asgp.png > added Require: hicolor-icon-theme is this ok ? > * MUST (when possible) use the %cmake macro to avoid relisting of all those > parameters like RPATH etc. > https://fedoraproject.org/wiki/Packaging:Cmake - done, replaced cmake with %cmake Spec URL: https://martinkg.fedorapeople.org/Review/asgp-1.0.8-3/asgp.spec %changelog * Sat Dec 20 2014 Martin Gansser <martinkg> - 1.0.8-3 - changed package name from andy-super-great-park to asgp - added valid Source URL - added download instruction - removed group tag - added CMAKE_VERBOSE_MAKEFILE=TRUE to %%make build verbose - added Require hicolor-icon-theme - used macro %%cmake
(In reply to Raphael Groner from comment #3) > > * SHOULD what is "Plee the Bear"? How is it related to this package? You > won't be able to create two individual subpackges for that 'bear' and > 'bear-factory' stuff. So consider to package that separately and unbundle. > https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries > > of the Bear Engine for Plee the Bear & Andy's Super Great Park. > there is already a package plee-the-bear for Fedora and only needs to be unbundled. https://apps.fedoraproject.org/packages/plee-the-bear
(In reply to MartinKG from comment #5) > (In reply to Raphael Groner from comment #3) > > > > * SHOULD what is "Plee the Bear"? How is it related to this package? You > > won't be able to create two individual subpackges for that 'bear' and > > 'bear-factory' stuff. So consider to package that separately and unbundle. > > https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries > > > of the Bear Engine for Plee the Bear & Andy's Super Great Park. > > > > there is already a package plee-the-bear for Fedora and only needs to be > unbundled. > https://apps.fedoraproject.org/packages/plee-the-bear Any status on this? Are you still interested in packaging this? I can review this if you're interested in a review swap. > yes this is a valid link for asgp-1.0.8 > > but there is a recent version 1.0.12 on https://github.com/j-jorge/asgp/ > build instruction: > > mkdir asgp-build > cd asgp-build > git clone https://github.com/j-jorge/bear.git > git clone https://github.com/j-jorge/asgp.git > > [root@fc21 asgp-build]# du -sk * > 1140264 asgp > 37616 bear > > approx. 1,17 GByte > > should we use this version ? There is a new version available. Also your URL comment doesn't correctly explain how to get the sources, but fortunately enough, it should be sufficient to update the URL to the following: https://github.com/j-jorge/%{name}/archive/%{version}.tar.gz#/%{name}_%{version}.tar.gz At a quick glance, a few things will need to be updated (for example, "%setup -qn %{oname}-%{version}" can be replaced with "%setup -q"), but it's pretty minor.
(In reply to Jeremy Newton from comment #6) > > Any status on this? Are you still interested in packaging this? > I can review this if you're interested in a review swap. ok lets us swap the review.
(In reply to MartinKG from comment #7) > (In reply to Jeremy Newton from comment #6) > > > > Any status on this? Are you still interested in packaging this? > > I can review this if you're interested in a review swap. > > ok lets us swap the review. Sounds good, I have #1379798 available to review. As for this review, please update the spec to the latest version (1.0.16) and update the source url to something like this: https://github.com/j-jorge/%{name}/archive/%{version}.tar.gz#/%{name}_%{version}.tar.gz As well, please unbundle plee-the-bear if possible.
More notes: Please make use of the make macros as well: make %{?_smp_mflags} -> %make_build make install DESTDIR=%{buildroot} INSTALL="install -p" -> %make_install As well, please clean up %cmake, as some of the parameters are already defined by this macro. Specifically, the following should be removed: -DCMAKE_VERBOSE_MAKEFILE=TRUE -DCMAKE_C_FLAGS="-DNDEBUG %{optflags}" -DCMAKE_CXX_FLAGS="-DNDEBUG %{optflags}" -DCMAKE_BUILD_WITH_INSTALL_RPATH=OFF -DCMAKE_INSTALL_PREFIX=%{_prefix} Furthermore, I don't think these are necessary: -DCMAKE_NO_BUILTIN_CHRPATH=ON -DCMAKE_BUILD_WITH_INSTALL_RPATH=OFF I think this is why you were having rpath issues, this should be ON not OFF: -DCMAKE_SKIP_RPATH:BOOL=OFF You can replace it with a macro instead, for example: %cmake . %_cmake_skip_rpath Note that the oname variable can be effectively removed with updates I suggested in comment#8.
I still testing asgp to compile, but it fails with this error message: [ 96%] Linking CXX shared library ../../../bin/librp.so cd /home/martin/rpmbuild/BUILD/asgp-1.0.16/asgp/lib/src/rp && /usr/bin/cmake -E cmake_link_script CMakeFiles/rp.dir/link.txt --verbose=1 /usr/bin/c++ -fPIC -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic -g -Wl,-z,relro -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -shared -Wl,-soname,librp.so -o ../../../bin/librp.so CMakeFiles/rp.dir/code/add_ingame_layers.cpp.o CMakeFiles/rp.dir/code/attractable_item.cpp.o CMakeFiles/rp.dir/code/balloon.cpp.o CMakeFiles/rp.dir/code/bird.cpp.o CMakeFiles/rp.dir/code/bird_support.cpp.o CMakeFiles/rp.dir/code/bomb.cpp.o CMakeFiles/rp.dir/code/bonus.cpp.o CMakeFiles/rp.dir/code/boss.cpp.o CMakeFiles/rp.dir/code/boss_controller.cpp.o CMakeFiles/rp.dir/code/boss_teleport.cpp.o CMakeFiles/rp.dir/code/cable.cpp.o CMakeFiles/rp.dir/code/cannonball.cpp.o CMakeFiles/rp.dir/code/cart_controller.cpp.o CMakeFiles/rp.dir/code/cart.cpp.o CMakeFiles/rp.dir/code/config_file.cpp.o CMakeFiles/rp.dir/code/config_save.cpp.o CMakeFiles/rp.dir/code/crate.cpp.o CMakeFiles/rp.dir/code/cursor.cpp.o CMakeFiles/rp.dir/code/decorative_balloon.cpp.o CMakeFiles/rp.dir/code/end.cpp.o CMakeFiles/rp.dir/code/entity.cpp.o CMakeFiles/rp.dir/code/explosion.cpp.o CMakeFiles/rp.dir/code/game_key.cpp.o CMakeFiles/rp.dir/code/game_variables.cpp.o CMakeFiles/rp.dir/code/help_button.cpp.o CMakeFiles/rp.dir/code/hole.cpp.o CMakeFiles/rp.dir/code/http_request.cpp.o CMakeFiles/rp.dir/code/init.cpp.o CMakeFiles/rp.dir/code/interactive_item.cpp.o CMakeFiles/rp.dir/code/level_exit.cpp.o CMakeFiles/rp.dir/code/level_selector.cpp.o CMakeFiles/rp.dir/code/level_settings.cpp.o CMakeFiles/rp.dir/code/level_variables.cpp.o CMakeFiles/rp.dir/code/obstacle.cpp.o CMakeFiles/rp.dir/code/pause_game.cpp.o CMakeFiles/rp.dir/code/plank.cpp.o CMakeFiles/rp.dir/code/plunger.cpp.o CMakeFiles/rp.dir/code/serial_switcher.cpp.o CMakeFiles/rp.dir/code/show_key_layer.cpp.o CMakeFiles/rp.dir/code/switching.cpp.o CMakeFiles/rp.dir/code/tar.cpp.o CMakeFiles/rp.dir/code/tnt.cpp.o CMakeFiles/rp.dir/code/util.cpp.o CMakeFiles/rp.dir/code/wall.cpp.o CMakeFiles/rp.dir/code/zeppelin.cpp.o CMakeFiles/rp.dir/android/code/back_button_home_item.cpp.o CMakeFiles/rp.dir/android/code/java_activity.cpp.o CMakeFiles/rp.dir/layer/code/help_layer.cpp.o CMakeFiles/rp.dir/layer/code/key_layer.cpp.o CMakeFiles/rp.dir/layer/code/misc_layer.cpp.o CMakeFiles/rp.dir/layer/code/pause_layer.cpp.o CMakeFiles/rp.dir/layer/code/status_layer.cpp.o CMakeFiles/rp.dir/layer/status/code/background_component.cpp.o CMakeFiles/rp.dir/layer/status/code/balloon_component.cpp.o CMakeFiles/rp.dir/layer/status/code/bonus_component.cpp.o CMakeFiles/rp.dir/layer/status/code/boss_component.cpp.o CMakeFiles/rp.dir/layer/status/code/cannonball_component.cpp.o CMakeFiles/rp.dir/layer/status/code/lives_component.cpp.o CMakeFiles/rp.dir/layer/status/code/floating_score_component.cpp.o CMakeFiles/rp.dir/layer/status/code/plunger_component.cpp.o CMakeFiles/rp.dir/layer/status/code/score_component.cpp.o CMakeFiles/rp.dir/layer/status/code/status_component.cpp.o CMakeFiles/rp.dir/layer/status/code/time_component.cpp.o CMakeFiles/rp.dir/message/code/help_layer_starting_message.cpp.o CMakeFiles/rp.dir/message/code/key_layer_starting_message.cpp.o CMakeFiles/rp.dir/message/code/pause_message.cpp.o CMakeFiles/rp.dir/transition_effect/code/level_ending_effect.cpp.o CMakeFiles/rp.dir/transition_effect/code/level_starting_effect.cpp.o -L/usr/lib64/libjpeg.so -L/usr/lib64/libpng.so -L/home/martin/rpmbuild/BUILD/asgp-1.0.16/asgp/bin -L/home/martin/rpmbuild/BUILD/asgp-1.0.16/asgp/../bear/bear-engine/bin -lbear_engine -lbear_gui -lbear_generic_items -lclaw_configuration_file /usr/bin/ld: cannot find -lbear_engine /usr/bin/ld: cannot find -lbear_gui /usr/bin/ld: cannot find -lbear_generic_items collect2: error: ld returned 1 exit status lib/src/rp/CMakeFiles/rp.dir/build.make:1840: recipe for target 'bin/librp.so' failed make[2]: *** [bin/librp.so] Error 1 make[2]: Leaving directory '/home/martin/rpmbuild/BUILD/asgp-1.0.16/asgp' CMakeFiles/Makefile2:254: recipe for target 'lib/src/rp/CMakeFiles/rp.dir/all' failed make[1]: *** [lib/src/rp/CMakeFiles/rp.dir/all] Error 2 Makefile:130: recipe for target 'all' failed make: *** [all] Error 2 the build section looks like this: %build cp %{SOURCE1} . cd asgp %cmake -DCMAKE_BUILD_TYPE=release \ -DCMAKE_SKIP_RPATH:BOOL=ON \ -DCMAKE_BUILD_TYPE=debug %make_build
(In reply to MartinKG from comment #10) > I still testing asgp to compile, but it fails with this error message: > > [ 96%] Linking CXX shared library ../../../bin/librp.so > cd /home/martin/rpmbuild/BUILD/asgp-1.0.16/asgp/lib/src/rp && /usr/bin/cmake > -E cmake_link_script CMakeFiles/rp.dir/link.txt --verbose=1 > /usr/bin/c++ -fPIC -O2 -g -pipe -Wall -Werror=format-security > -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong > --param=ssp-buffer-size=4 -grecord-gcc-switches > -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic -g > -Wl,-z,relro -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -shared > -Wl,-soname,librp.so -o ../../../bin/librp.so > CMakeFiles/rp.dir/code/add_ingame_layers.cpp.o > CMakeFiles/rp.dir/code/attractable_item.cpp.o > CMakeFiles/rp.dir/code/balloon.cpp.o CMakeFiles/rp.dir/code/bird.cpp.o > CMakeFiles/rp.dir/code/bird_support.cpp.o CMakeFiles/rp.dir/code/bomb.cpp.o > CMakeFiles/rp.dir/code/bonus.cpp.o CMakeFiles/rp.dir/code/boss.cpp.o > CMakeFiles/rp.dir/code/boss_controller.cpp.o > CMakeFiles/rp.dir/code/boss_teleport.cpp.o > CMakeFiles/rp.dir/code/cable.cpp.o CMakeFiles/rp.dir/code/cannonball.cpp.o > CMakeFiles/rp.dir/code/cart_controller.cpp.o > CMakeFiles/rp.dir/code/cart.cpp.o CMakeFiles/rp.dir/code/config_file.cpp.o > CMakeFiles/rp.dir/code/config_save.cpp.o CMakeFiles/rp.dir/code/crate.cpp.o > CMakeFiles/rp.dir/code/cursor.cpp.o > CMakeFiles/rp.dir/code/decorative_balloon.cpp.o > CMakeFiles/rp.dir/code/end.cpp.o CMakeFiles/rp.dir/code/entity.cpp.o > CMakeFiles/rp.dir/code/explosion.cpp.o CMakeFiles/rp.dir/code/game_key.cpp.o > CMakeFiles/rp.dir/code/game_variables.cpp.o > CMakeFiles/rp.dir/code/help_button.cpp.o CMakeFiles/rp.dir/code/hole.cpp.o > CMakeFiles/rp.dir/code/http_request.cpp.o CMakeFiles/rp.dir/code/init.cpp.o > CMakeFiles/rp.dir/code/interactive_item.cpp.o > CMakeFiles/rp.dir/code/level_exit.cpp.o > CMakeFiles/rp.dir/code/level_selector.cpp.o > CMakeFiles/rp.dir/code/level_settings.cpp.o > CMakeFiles/rp.dir/code/level_variables.cpp.o > CMakeFiles/rp.dir/code/obstacle.cpp.o > CMakeFiles/rp.dir/code/pause_game.cpp.o CMakeFiles/rp.dir/code/plank.cpp.o > CMakeFiles/rp.dir/code/plunger.cpp.o > CMakeFiles/rp.dir/code/serial_switcher.cpp.o > CMakeFiles/rp.dir/code/show_key_layer.cpp.o > CMakeFiles/rp.dir/code/switching.cpp.o CMakeFiles/rp.dir/code/tar.cpp.o > CMakeFiles/rp.dir/code/tnt.cpp.o CMakeFiles/rp.dir/code/util.cpp.o > CMakeFiles/rp.dir/code/wall.cpp.o CMakeFiles/rp.dir/code/zeppelin.cpp.o > CMakeFiles/rp.dir/android/code/back_button_home_item.cpp.o > CMakeFiles/rp.dir/android/code/java_activity.cpp.o > CMakeFiles/rp.dir/layer/code/help_layer.cpp.o > CMakeFiles/rp.dir/layer/code/key_layer.cpp.o > CMakeFiles/rp.dir/layer/code/misc_layer.cpp.o > CMakeFiles/rp.dir/layer/code/pause_layer.cpp.o > CMakeFiles/rp.dir/layer/code/status_layer.cpp.o > CMakeFiles/rp.dir/layer/status/code/background_component.cpp.o > CMakeFiles/rp.dir/layer/status/code/balloon_component.cpp.o > CMakeFiles/rp.dir/layer/status/code/bonus_component.cpp.o > CMakeFiles/rp.dir/layer/status/code/boss_component.cpp.o > CMakeFiles/rp.dir/layer/status/code/cannonball_component.cpp.o > CMakeFiles/rp.dir/layer/status/code/lives_component.cpp.o > CMakeFiles/rp.dir/layer/status/code/floating_score_component.cpp.o > CMakeFiles/rp.dir/layer/status/code/plunger_component.cpp.o > CMakeFiles/rp.dir/layer/status/code/score_component.cpp.o > CMakeFiles/rp.dir/layer/status/code/status_component.cpp.o > CMakeFiles/rp.dir/layer/status/code/time_component.cpp.o > CMakeFiles/rp.dir/message/code/help_layer_starting_message.cpp.o > CMakeFiles/rp.dir/message/code/key_layer_starting_message.cpp.o > CMakeFiles/rp.dir/message/code/pause_message.cpp.o > CMakeFiles/rp.dir/transition_effect/code/level_ending_effect.cpp.o > CMakeFiles/rp.dir/transition_effect/code/level_starting_effect.cpp.o > -L/usr/lib64/libjpeg.so -L/usr/lib64/libpng.so > -L/home/martin/rpmbuild/BUILD/asgp-1.0.16/asgp/bin > -L/home/martin/rpmbuild/BUILD/asgp-1.0.16/asgp/../bear/bear-engine/bin > -lbear_engine -lbear_gui -lbear_generic_items -lclaw_configuration_file > /usr/bin/ld: cannot find -lbear_engine > /usr/bin/ld: cannot find -lbear_gui > /usr/bin/ld: cannot find -lbear_generic_items > collect2: error: ld returned 1 exit status > lib/src/rp/CMakeFiles/rp.dir/build.make:1840: recipe for target > 'bin/librp.so' failed > make[2]: *** [bin/librp.so] Error 1 > make[2]: Leaving directory '/home/martin/rpmbuild/BUILD/asgp-1.0.16/asgp' > CMakeFiles/Makefile2:254: recipe for target > 'lib/src/rp/CMakeFiles/rp.dir/all' failed > make[1]: *** [lib/src/rp/CMakeFiles/rp.dir/all] Error 2 > Makefile:130: recipe for target 'all' failed > make: *** [all] Error 2 > > the build section looks like this: > > %build > cp %{SOURCE1} . > cd asgp > %cmake -DCMAKE_BUILD_TYPE=release \ > -DCMAKE_SKIP_RPATH:BOOL=ON \ > -DCMAKE_BUILD_TYPE=debug > %make_build I'll have some time to look at it tomorrow. I'll let you know if I figure it out. Also, you're defining DCMAKE_BUILD_TYPE twice. Is this a typo?
Ok, so I've looked into it. Looks like the maintainer for plee the bear has moved the library files to %{_libdir}/plee-the-bear, so you'll have to add the following to cmake: -DBEAR_ENGINE_LIBRARY_DIRECTORY=%{_libdir}/plee-the-bear But this is problematic if this is a run-time dependency for asgp (is it?). I would assume that one of the packagers for plee-the-bear should move this into a "lib" sub package and make a devel package to provide the include files. It seems the include files can be specified with: -DBEAR_ENGINE_INCLUDE_DIRECTORY=LOCATION and -DBEAR_ITEMS_INCLUDE_DIRECTORY=LOCATION The packagers also removed the editor binaries, so if you would like those to be included too, I would inquire to the maintainers to add them or ask to become a co-maintainer for the package. Either way, it looks like you'll need to talk to them to see how to move forward. Feel free to add me as a CC if you email them or make another bug report.
Actually, it looks like the "bear engine" (https://github.com/j-jorge/bear) is being bundled with plee-the-bear. I'm not 100% familiar with the code, but it looks like it should probably be made a separate package altogether so it can be unbundled. This would also make packaging "Tunnel" easier as well (https://github.com/j-jorge/tunnel)
created bear package https://bugzilla.redhat.com/show_bug.cgi?id=1398949
@Jeremy, Now I have managed to build a running asgp, but with a bundled bear-engine package. Can you have a look at it. Spec URL: https://martinkg.fedorapeople.org/Review/asgp-1.0.16/test1/asgp.spec SRPM URL: https://martinkg.fedorapeople.org/Review/asgp-1.0.16/test1/asgp-1.0.16-1.fc25.src.rpm changelog * Tue Nov 22 2016 Martin Gansser <martinkg> - 1.0.16-1 - Update to 1.0.16 - Update Source URL
(In reply to MartinKG from comment #15) > @Jeremy, > Now I have managed to build a running asgp, but with a bundled bear-engine > package. Can you have a look at it. > > Spec URL: > https://martinkg.fedorapeople.org/Review/asgp-1.0.16/test1/asgp.spec > SRPM URL: > https://martinkg.fedorapeople.org/Review/asgp-1.0.16/test1/asgp-1.0.16-1. > fc25.src.rpm > > changelog > * Tue Nov 22 2016 Martin Gansser <martinkg> - 1.0.16-1 > - Update to 1.0.16 > - Update Source URL I gave it a shot, but it seems cmake is having trouble finding libclaw (I'm using f25 with mock): CMake Error at /usr/share/cmake/libclaw/libclaw-config.cmake:377 (MESSAGE): Could not find Claw library Also, why are you using git clone to get the sources, when you can just use the two following download links? for bear, where longhash is the bear git commit hash: https://github.com/j-jorge/bear/archive/%{longhash}.tar.gz and for asgp: https://github.com/j-jorge/%{name}/archive/%{version}.tar.gz As well, I've noticed that asgp 1.18 was released 3 days ago. I would encourage you to target that if possible.
(In reply to Jeremy Newton from comment #16) > (In reply to MartinKG from comment #15) > for bear, where longhash is the bear git commit hash: > https://github.com/j-jorge/bear/archive/%{longhash}.tar.gz In other words, use something similar to what you did with the bear package you have in review. The sources should be valid URLs where possible. Obviously CMakeLists.txt would be exempt from this.
Spec URL: https://martinkg.fedorapeople.org/Review/asgp-1.0.18/test1/asgp.spec SRPM URL: https://martinkg.fedorapeople.org/Review/asgp-1.0.18/test1/asgp-1.0.18-1.fc25.src.rpm %changelog * Fri Dec 09 2016 Martin Gansser <martinkg> - 1.0.18-1 - Update to 1.0.18 - Update Source URL - Add Requires bear-engine >= %%{bear_version} - Add Requires bear-factory >= %%{bear_version}
I still get the claw error when building in mock.
I have only uploaded the spec file due my thin internet bandwidth. Spec URL: https://martinkg.fedorapeople.org/Review/asgp-1.0.18/test2/asgp.spec there's a problem with asgp/CMakeLists.txt see below: + /usr/bin/cmake -DCMAKE_C_FLAGS_RELEASE:STRING=-DNDEBUG -DCMAKE_CXX_FLAGS_RELEASE:STRING=-DNDEBUG -DCMAKE_Fortran_FLAGS_RELEASE:STRING=-DNDEBUG -DCMAKE_VERBOSE_MAKEFILE:BOOL=ON -DCMAKE_INSTALL_PREFIX:PATH=/usr -DINCLUDE_INSTALL_DIR:PATH=/usr/include -DLIB_INSTALL_DIR:PATH=/usr/lib64 -DSYSCONF_INSTALL_DIR:PATH=/etc -DSHARE_INSTALL_PREFIX:PATH=/usr/share -DLIB_SUFFIX=64 -DBUILD_SHARED_LIBS:BOOL=ON -DBEAR_ENGINE_INSTALL_LIBRARY_DIR=lib64/bear -DBEAR_EDITORS_ENABLED=OFF -DRP_INSTALL_CUSTOM_LIBRARY_DIR=lib64/bear -DBEAR_ROOT_DIRECTORY=/usr/include/bear -DRP_INSTALL_DATA_DIR=share/asgp -DRP_BEAR_FACTORY_ENABLED=ON -- The C compiler identification is GNU 6.2.1 -- The CXX compiler identification is GNU 6.2.1 -- Check for working C compiler: /usr/bin/cc -- Check for working C compiler: /usr/bin/cc -- works -- Detecting C compiler ABI info -- Detecting C compiler ABI info - done -- Detecting C compile features -- Detecting C compile features - done -- Check for working CXX compiler: /usr/bin/c++ -- Check for working CXX compiler: /usr/bin/c++ -- works -- Detecting CXX compiler ABI info -- Detecting CXX compiler ABI info - done -- Detecting CXX compile features -- Detecting CXX compile features - done CMake Error at CMakeLists.txt:9 (include): include could not find load file: uninstall CMake Error at CMakeLists.txt:11 (subdirs): subdirs Incorrect SUBDIRS command. Directory: bear does not exist. CMake Error at asgp/CMakeLists.txt:19 (include): include could not find load file: gettext CMake Error at asgp/CMakeLists.txt:20 (include): include could not find load file: compiler-defaults CMake Error at asgp/CMakeLists.txt:21 (include): include could not find load file: docbook-to-man CMake Error at asgp/CMakeLists.txt:58 (include): include could not find load file: FindSDL2 CMake Error at asgp/CMakeLists.txt:61 (message): SDL2 library must be installed. -- Configuring incomplete, errors occurred!
(In reply to MartinKG from comment #20) > I have only uploaded the spec file due my thin internet bandwidth. > Spec URL: > https://martinkg.fedorapeople.org/Review/asgp-1.0.18/test2/asgp.spec > > there's a problem with asgp/CMakeLists.txt see below: > > + /usr/bin/cmake -DCMAKE_C_FLAGS_RELEASE:STRING=-DNDEBUG > -DCMAKE_CXX_FLAGS_RELEASE:STRING=-DNDEBUG > -DCMAKE_Fortran_FLAGS_RELEASE:STRING=-DNDEBUG > -DCMAKE_VERBOSE_MAKEFILE:BOOL=ON -DCMAKE_INSTALL_PREFIX:PATH=/usr > -DINCLUDE_INSTALL_DIR:PATH=/usr/include -DLIB_INSTALL_DIR:PATH=/usr/lib64 > -DSYSCONF_INSTALL_DIR:PATH=/etc -DSHARE_INSTALL_PREFIX:PATH=/usr/share > -DLIB_SUFFIX=64 -DBUILD_SHARED_LIBS:BOOL=ON > -DBEAR_ENGINE_INSTALL_LIBRARY_DIR=lib64/bear -DBEAR_EDITORS_ENABLED=OFF > -DRP_INSTALL_CUSTOM_LIBRARY_DIR=lib64/bear > -DBEAR_ROOT_DIRECTORY=/usr/include/bear -DRP_INSTALL_DATA_DIR=share/asgp > -DRP_BEAR_FACTORY_ENABLED=ON > -- The C compiler identification is GNU 6.2.1 > -- The CXX compiler identification is GNU 6.2.1 > -- Check for working C compiler: /usr/bin/cc > -- Check for working C compiler: /usr/bin/cc -- works > -- Detecting C compiler ABI info > -- Detecting C compiler ABI info - done > -- Detecting C compile features > -- Detecting C compile features - done > -- Check for working CXX compiler: /usr/bin/c++ > -- Check for working CXX compiler: /usr/bin/c++ -- works > -- Detecting CXX compiler ABI info > -- Detecting CXX compiler ABI info - done > -- Detecting CXX compile features > -- Detecting CXX compile features - done > CMake Error at CMakeLists.txt:9 (include): > include could not find load file: > > uninstall > > > CMake Error at CMakeLists.txt:11 (subdirs): > subdirs Incorrect SUBDIRS command. Directory: bear does not exist. > > > CMake Error at asgp/CMakeLists.txt:19 (include): > include could not find load file: > > gettext > > > CMake Error at asgp/CMakeLists.txt:20 (include): > include could not find load file: > > compiler-defaults > > > CMake Error at asgp/CMakeLists.txt:21 (include): > include could not find load file: > > docbook-to-man > > > CMake Error at asgp/CMakeLists.txt:58 (include): > include could not find load file: > > FindSDL2 > > > CMake Error at asgp/CMakeLists.txt:61 (message): > SDL2 library must be installed. > > > -- Configuring incomplete, errors occurred! Indeed, this is due to missing files cmake files in the bear-devel package, I'll post a fix over there.
Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/asgp.spec SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/asgp-1.0.18-2.fc25.src.rpm %changelog * Tue Dec 13 2016 Martin Gansser <martinkg> - 1.0.18-2 - Add BR bear-devel - Add BR cmake option -DBEAR_ROOT_DIRECTORY=%%{_includedir}/bear - Remove bear Sources - Add LINK_DIRECTORIES(/usr/lib64/bear/) to CMakeLists.txt - Delete subdirs bear in CMakeLists.txt CMakeLists file looks now: diff -Naur CMakeLists.txt.orig CMakeLists.txt --- CMakeLists.txt.orig 2016-12-13 21:25:53.137347804 +0100 +++ CMakeLists.txt 2016-12-13 21:39:11.399075740 +0100 @@ -6,6 +6,8 @@ set( CMAKE_MODULE_PATH "${BEAR_ROOT_DIRECTORY}/cmake-helper" ) +LINK_DIRECTORIES(/usr/lib64/bear/) + include( uninstall ) -subdirs( bear asgp ) +subdirs( asgp ) asgp compiles fine, but when i run the program, i get this error message: martin@fc25 ~]$ asgp Exception: /usr/include/bear/bear-engine/bin/libbear_generic_items.so: cannot open shared object file: No such file or directory asgp [-h] [--log-concise=integer] [--log-file=file] [--log-level=string] [--log-uniq] engine_options
asgp runs now, new rpm packages: Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/asgp.spec SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/asgp-1.0.18-2.fc25.src.rpm %changelog * Tue Dec 13 2016 Martin Gansser <martinkg> - 1.0.18-2 - Add BR bear-devel - Add BR cmake option -DBEAR_ROOT_DIRECTORY=%%{_includedir}/bear - Remove bear Sources - Add LINK_DIRECTORIES(/usr/lib64/bear/) to CMakeLists.txt - Delete subdirs bear in CMakeLists.txt
Just from a quick glance. Source1 shouldn't be necessary, please remove it. I have some additional comments, but I need to look into it further to give you better feedback.
new rpms: Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/asgp.spec SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/asgp-1.0.18-3.fc25.src.rpm %changelog * Fri Dec 23 2016 Martin Gansser <martinkg> - 1.0.18-3 - remove Source1 - convert docbook2man filename taken from .sgml file to lowercase - add macor %%license
Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated ===== MUST items ===== C/C++: [x]: Package does not contain kernel modules. [x]: Package contains no static executables. [-]: Development (unversioned) .so files in -devel subpackage, if present. Note: Unversioned so-files in private %_libdir subdirectory (see attachment). Verify they are not in ld path. > This is fine, no need to change this. [x]: Package does not contain any libtool archives (.la) [x]: Rpath absent or only used for internal libs. Generic: [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [!]: License field in the package spec file matches the actual license. Note: Checking patched sources after %prep for licenses. Licenses found: "Apache (v2.0)", "*No copyright* CC by-sa", "GPL (v2 or later)", "*No copyright* CC by-sa (v3.0)", "Unknown or generated". 1555 files have unknown license. > License is GPLv3 not GPLv3+. See LICENSE file [x]: License file installed when any subpackage combination is installed. [!]: If the package is under multiple licenses, the licensing breakdown must be documented in the spec. > You should really add a breakdown if possible [!]: Package must own all directories that it creates. Note: Directories without known owners: /usr/share/icons/hicolor/24x24, /usr/share/icons/hicolor/64x64/apps, /usr/share/icons/hicolor/48x48/apps, /usr/share/icons/hicolor/32x32/apps, /usr/share/icons/hicolor/24x24/apps, /usr/share/icons/hicolor/16x16/apps, /usr/share/icons/hicolor/16x16, /usr/share/icons/hicolor/128x128/apps, /usr/share/icons/hicolor/128x128, /usr/share/icons/hicolor/64x64, /usr/share/icons/hicolor/48x48, /usr/share/icons/hicolor/32x32, /usr/share/icons/hicolor, /usr/share/bear-factory > This can be fixed by adding the following to the main package: BuildRequires: hicolor-icon-theme Requires: hicolor-icon-theme > Which is something I missed when I was reviewing bear. Please fix this. [x]: %build honors applicable compiler flags or justifies otherwise. [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: Development files must be in a -devel package [x]: Package uses nothing in %doc for runtime. [x]: The spec file handles locales properly. [x]: Package consistently uses macros (instead of hard-coded directory names). [x]: Package is named according to the Package Naming Guidelines. [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. [x]: gtk-update-icon-cache is invoked in %postun and %posttrans if package contains icons. Note: icons in asgp [x]: Useful -debuginfo package or justification otherwise. [x]: Package is not known to require an ExcludeArch tag. > ExcludeArch is required, but this is fine until bear is fixed on ppc64le... This arch is largely broken for a lot of stuff, so it's not a big deal. [x]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 20480 bytes in 2 files. [x]: Package complies to the Packaging Guidelines [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: No rpmlint messages. [x]: 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 is included in %license. [x]: Package requires other packages for directories it uses. [x]: Package does not own files or directories owned by other packages. [x]: All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. [x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Package contains desktop file if it is a GUI application. [x]: Package installs a %{name}.desktop using desktop-file-install or desktop-file-validate if there is such a file. [x]: Dist tag is present. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [x]: Package use %makeinstall only when make install DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: Package does not use a name that already exists. [x]: Package is not relocatable. [x]: Sources used to build the package match the upstream source, as provided in the spec URL. [x]: Spec file name must match the spec package %{name}, in the format %{name}.spec. [x]: File names are valid UTF-8. [x]: Packages must not store files under /srv, /opt or /usr/local ===== SHOULD items ===== Generic: [-]: Avoid bundling fonts in non-fonts packages. Note: Package contains font files [-]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x]: Final provides and requires are sane (see attachments). [-]: Fully versioned dependency in subpackages if applicable. Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in asgp- data , asgp-debuginfo [?]: Package functions as described. [?]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [-]: Patches link to upstream bugs/comments/lists or are otherwise justified. > If this patch is upstreamable, I would send it, elsewise it doesn't matter too much. [-]: Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [?]: Package should compile and build into binary rpms on all supported architectures. [x]: %check is present and all tests pass. [?]: Packages should try to preserve timestamps of original installed files. [x]: Reviewer should test that the package builds in mock. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: Sources can be downloaded from URI in Source: tag [x]: SourceX is a working URL. [x]: Spec use %global instead of %define unless justified. ===== EXTRA items ===== Generic: [x]: Rpmlint is run on debuginfo package(s). Note: No rpmlint messages. [x]: Rpmlint is run on all installed packages. Note: No rpmlint messages. [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. [x]: Spec file according to URL is the same as in SRPM. Rpmlint ------- Checking: asgp-1.0.18-3.fc26.x86_64.rpm asgp-data-1.0.18-3.fc26.noarch.rpm asgp-debuginfo-1.0.18-3.fc26.x86_64.rpm asgp-1.0.18-3.fc26.src.rpm 4 packages and 0 specfiles checked; 0 errors, 0 warnings. Rpmlint (debuginfo) ------------------- Checking: asgp-debuginfo-1.0.18-3.fc26.x86_64.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings. Rpmlint (installed packages) ---------------------------- sh: /usr/bin/python: No such file or directory 3 packages and 0 specfiles checked; 0 errors, 0 warnings. Requires -------- asgp (rpmlib, GLIBC filtered): /bin/sh asgp-data bear-engine bear-factory libbear_engine.so()(64bit) libbear_generic_items.so()(64bit) libbear_gui.so()(64bit) libc.so.6()(64bit) libclaw_application.so.1()(64bit) libclaw_configuration_file.so.1()(64bit) libclaw_logger.so.1()(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.0)(64bit) libm.so.6()(64bit) libstdc++.so.6()(64bit) libstdc++.so.6(CXXABI_1.3)(64bit) libstdc++.so.6(CXXABI_1.3.8)(64bit) rtld(GNU_HASH) asgp-data (rpmlib, GLIBC filtered): asgp-debuginfo (rpmlib, GLIBC filtered): Provides -------- asgp: application() application(asgp.desktop) asgp asgp(x86-64) librp.so()(64bit) asgp-data: asgp-data asgp-debuginfo: asgp-debuginfo asgp-debuginfo(x86-64) Unversioned so-files -------------------- asgp: /usr/lib64/bear/librp.so > This is fine because it's in a subfolder Source checksums ---------------- https://github.com/j-jorge/asgp/archive/90d6d90e3196d387dc58f028a04e75af2281e513/asgp-90d6d90e3196d387dc58f028a04e75af2281e513.tar.gz#/asgp-90d6d90.tar.gz : CHECKSUM(SHA256) this package : 1cd9c4c7d52226e1e7498d3d0b2d681a416d8d61db8f88ff0fc18a4b713cf5b9 CHECKSUM(SHA256) upstream package : 1cd9c4c7d52226e1e7498d3d0b2d681a416d8d61db8f88ff0fc18a4b713cf5b9 Generated by fedora-review 0.6.1 (f03e4e7) last change: 2016-05-02 Command line :/usr/bin/fedora-review -b 1176273 -m fedora-rawhide-x86_64 Buildroot used: fedora-rawhide-x86_64 Active plugins: Generic, Shell-api, C/C++ Disabled plugins: Java, Python, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP Disabled flags: EXARCH, DISTTAG, EPEL5, BATCH, EPEL6
(In reply to Jeremy Newton from comment #26) > Package Review > ============== > > Legend: > [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated > > ===== MUST items ===== > > C/C++: > [x]: Package does not contain kernel modules. > [x]: Package contains no static executables. > [-]: Development (unversioned) .so files in -devel subpackage, if present. > Note: Unversioned so-files in private %_libdir subdirectory (see > attachment). Verify they are not in ld path. > > This is fine, no need to change this. ok > > [x]: Package does not contain any libtool archives (.la) > [x]: Rpath absent or only used for internal libs. > > Generic: > [x]: Package is licensed with an open-source compatible license and meets > other legal requirements as defined in the legal section of Packaging > Guidelines. > [!]: License field in the package spec file matches the actual license. > Note: Checking patched sources after %prep for licenses. Licenses > found: "Apache (v2.0)", "*No copyright* CC by-sa", "GPL (v2 or > later)", "*No copyright* CC by-sa (v3.0)", "Unknown or generated". > 1555 files have unknown license. > > License is GPLv3 not GPLv3+. See LICENSE file done > > [x]: License file installed when any subpackage combination is installed. > [!]: If the package is under multiple licenses, the licensing breakdown > must be documented in the spec. > > You should really add a breakdown if possible done > > [!]: Package must own all directories that it creates. > Note: Directories without known owners: > /usr/share/icons/hicolor/24x24, /usr/share/icons/hicolor/64x64/apps, > /usr/share/icons/hicolor/48x48/apps, > /usr/share/icons/hicolor/32x32/apps, > /usr/share/icons/hicolor/24x24/apps, > /usr/share/icons/hicolor/16x16/apps, /usr/share/icons/hicolor/16x16, > /usr/share/icons/hicolor/128x128/apps, > /usr/share/icons/hicolor/128x128, /usr/share/icons/hicolor/64x64, > /usr/share/icons/hicolor/48x48, /usr/share/icons/hicolor/32x32, > /usr/share/icons/hicolor, /usr/share/bear-factory > > This can be fixed by adding the following to the main package: > BuildRequires: hicolor-icon-theme > Requires: hicolor-icon-theme > > Which is something I missed when I was reviewing bear. Please fix this. - add RR hicolor-icon-theme in both packages, I think BR is not required. > [x]: %build honors applicable compiler flags or justifies otherwise. > [x]: Package contains no bundled libraries without FPC exception. > [x]: Changelog in prescribed format. > [x]: Sources contain only permissible code or content. > [-]: Development files must be in a -devel package > [x]: Package uses nothing in %doc for runtime. > [x]: The spec file handles locales properly. > [x]: Package consistently uses macros (instead of hard-coded directory > names). > [x]: Package is named according to the Package Naming Guidelines. > [x]: Package does not generate any conflict. > [x]: Package obeys FHS, except libexecdir and /usr/target. > [-]: If the package is a rename of another package, proper Obsoletes and > Provides are present. > [x]: Requires correct, justified where necessary. > [x]: Spec file is legible and written in American English. > [-]: Package contains systemd file(s) if in need. > [x]: gtk-update-icon-cache is invoked in %postun and %posttrans if package > contains icons. > Note: icons in asgp > [x]: Useful -debuginfo package or justification otherwise. > [x]: Package is not known to require an ExcludeArch tag. > > ExcludeArch is required, but this is fine until bear is fixed on ppc64le... This arch is largely broken for a lot of stuff, so it's not a big deal. ok > > ===== SHOULD items ===== > > Generic: > [-]: Avoid bundling fonts in non-fonts packages. > Note: Package contains font files > [-]: If the source package does not include license text(s) as a separate > file from upstream, the packager SHOULD query upstream to include it. > [x]: Final provides and requires are sane (see attachments). > [-]: Fully versioned dependency in subpackages if applicable. > Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in asgp- > data , asgp-debuginfo > [?]: Package functions as described. > [?]: Latest version is packaged. > [x]: Package does not include license text files separate from upstream. > [-]: Patches link to upstream bugs/comments/lists or are otherwise > justified. > > If this patch is upstreamable, I would send it, elsewise it doesn't matter too much. done new rpms: Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/asgp.spec SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/asgp-1.0.18-4.fc25.src.rpm %changelog * Mon Jan 02 2017 Martin Gansser <martinkg> - 1.0.18-4 - correct license to GPLv3 - add license breakdown - add BR desktop-file-utils - add RR hicolor-icon-theme
(In reply to MartinKG from comment #27) > (In reply to Jeremy Newton from comment #26) > > [!]: License field in the package spec file matches the actual license. > > Note: Checking patched sources after %prep for licenses. Licenses > > found: "Apache (v2.0)", "*No copyright* CC by-sa", "GPL (v2 or > > later)", "*No copyright* CC by-sa (v3.0)", "Unknown or generated". > > 1555 files have unknown license. > > > License is GPLv3 not GPLv3+. See LICENSE file > > done > > > > [x]: License file installed when any subpackage combination is installed. > > [!]: If the package is under multiple licenses, the licensing breakdown > > must be documented in the spec. > > > You should really add a breakdown if possible > > done I think you misunderstood. There are two licenses provided: CCPL (CC by-sa v3.0) and GPL. According you the readme, all code is GPL v3 (excluding data/common.scm, which is actually GPLv2+) and all multimedia is CC by-sa v3.0. You can change the breakdown to the following: > # All code is GPLv3 except: > # asgp/data/common.scm is GPL (v2 or later) > # All multimedia (pictures, sounds, levels, etc.) is CC BY-SA V3.0 As well, asgp/android can be deleted in prep as you shouldn't need these files for Fedora (thus you should also remove ASL 2.0). Also please include the full licenses in the data file %license macro like so: > %license asgp/LICENSE asgp/license/* Note the %license macro for the main package is not necessary, since the main package requires the data package, but you may include it if you so please. > > [!]: Package must own all directories that it creates. > > Note: Directories without known owners: > > /usr/share/icons/hicolor/24x24, /usr/share/icons/hicolor/64x64/apps, > > /usr/share/icons/hicolor/48x48/apps, > > /usr/share/icons/hicolor/32x32/apps, > > /usr/share/icons/hicolor/24x24/apps, > > /usr/share/icons/hicolor/16x16/apps, /usr/share/icons/hicolor/16x16, > > /usr/share/icons/hicolor/128x128/apps, > > /usr/share/icons/hicolor/128x128, /usr/share/icons/hicolor/64x64, > > /usr/share/icons/hicolor/48x48, /usr/share/icons/hicolor/32x32, > > /usr/share/icons/hicolor, /usr/share/bear-factory > > > This can be fixed by adding the following to the main package: > > BuildRequires: hicolor-icon-theme > > Requires: hicolor-icon-theme > > > Which is something I missed when I was reviewing bear. Please fix this. > > - add RR hicolor-icon-theme in both packages, I think BR is not required. Indeed, the BR is not required, but it should silence the fedora-review warning. Adding only the RR is fine by me. The license issue is the only thing that needs to be fixed. You don't have to upload another srpm if you don't want to, but please post the new spec with the suggested changes before I can approve.
(In reply to Jeremy Newton from comment #28) > (In reply to MartinKG from comment #27) > > (In reply to Jeremy Newton from comment #26) > > > [!]: License field in the package spec file matches the actual license. > > > Note: Checking patched sources after %prep for licenses. Licenses > > > found: "Apache (v2.0)", "*No copyright* CC by-sa", "GPL (v2 or > > > later)", "*No copyright* CC by-sa (v3.0)", "Unknown or generated". > > > 1555 files have unknown license. > > > > License is GPLv3 not GPLv3+. See LICENSE file > > > > done > > > > > > [x]: License file installed when any subpackage combination is installed. > > > [!]: If the package is under multiple licenses, the licensing breakdown > > > must be documented in the spec. > > > > You should really add a breakdown if possible > > > > done > > I think you misunderstood. There are two licenses provided: CCPL (CC by-sa > v3.0) and GPL. > > According you the readme, all code is GPL v3 (excluding data/common.scm, > which is actually GPLv2+) and all multimedia is CC by-sa v3.0. You can > change the breakdown to the following: > > > # All code is GPLv3 except: > > # asgp/data/common.scm is GPL (v2 or later) > > # All multimedia (pictures, sounds, levels, etc.) is CC BY-SA V3.0 done > As well, asgp/android can be deleted in prep as you shouldn't need these > files for Fedora (thus you should also remove ASL 2.0). > done, delete android and removed ASL 2.0 license > Also please include the full licenses in the data file %license macro like > so: > > > %license asgp/LICENSE asgp/license/* done > Note the %license macro for the main package is not necessary, since the > main package requires the data package, but you may include it if you so > please. > > > > [!]: Package must own all directories that it creates. > > > Note: Directories without known owners: > > > /usr/share/icons/hicolor/24x24, /usr/share/icons/hicolor/64x64/apps, > > > /usr/share/icons/hicolor/48x48/apps, > > > /usr/share/icons/hicolor/32x32/apps, > > > /usr/share/icons/hicolor/24x24/apps, > > > /usr/share/icons/hicolor/16x16/apps, /usr/share/icons/hicolor/16x16, > > > /usr/share/icons/hicolor/128x128/apps, > > > /usr/share/icons/hicolor/128x128, /usr/share/icons/hicolor/64x64, > > > /usr/share/icons/hicolor/48x48, /usr/share/icons/hicolor/32x32, > > > /usr/share/icons/hicolor, /usr/share/bear-factory > > > > This can be fixed by adding the following to the main package: > > > BuildRequires: hicolor-icon-theme > > > Requires: hicolor-icon-theme > > > > Which is something I missed when I was reviewing bear. Please fix this. > > > > - add RR hicolor-icon-theme in both packages, I think BR is not required. > > Indeed, the BR is not required, but it should silence the fedora-review > warning. Adding only the RR is fine by me. > > The license issue is the only thing that needs to be fixed. You don't have > to upload another srpm if you don't want to, but please post the new spec > with the suggested changes before I can approve. all suggestions hopefully changed. new spec file only: Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/asgp.spec
Looks good, approved!
@Jeremy many thanks for your assistance !
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/andy-super-great-park
Hi Jeremy, can we rerequest the package as asgp ?
please use asgp, not andy-super-great-park
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/asgp
package has been built successfully on fc24, fc25 and rawhide.