Bug 1176273 - Review Request: asgp - 2D arcade game
Summary: Review Request: asgp - 2D arcade game
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jeremy Newton
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Keywords:
Depends On: 1398949
Blocks: FE-GAMESIG
TreeView+ depends on / blocked
 
Reported: 2014-12-19 22:35 UTC by MartinKG
Modified: 2017-01-10 14:59 UTC (History)
3 users (show)

(edit)
Clone Of:
(edit)
Last Closed: 2017-01-10 14:59:52 UTC
alexjnewt: fedora-review+


Attachments (Terms of Use)

Description MartinKG 2014-12-19 22:35:29 UTC
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@fedoraproject.org> - 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.

Comment 1 MartinKG 2014-12-20 10:47:06 UTC
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@fedoraproject.org> - 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

Comment 2 Volker Fröhlich 2014-12-20 12:41:37 UTC
Please make the build verbose and make Source0 a URL.

Comment 3 Raphael Groner 2014-12-20 12:57:02 UTC
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

Comment 4 MartinKG 2014-12-20 16:42:55 UTC
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@fedoraproject.org> - 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

Comment 5 MartinKG 2014-12-21 09:05:50 UTC
(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

Comment 6 Jeremy Newton 2016-11-21 19:14:39 UTC
(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.

Comment 7 MartinKG 2016-11-21 19:49:53 UTC
(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.

Comment 8 Jeremy Newton 2016-11-22 03:16:50 UTC
(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.

Comment 9 Jeremy Newton 2016-11-23 17:45:44 UTC
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.

Comment 10 MartinKG 2016-11-25 08:47:50 UTC
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

Comment 11 Jeremy Newton 2016-11-26 02:19:20 UTC
(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?

Comment 12 Jeremy Newton 2016-11-26 19:49:44 UTC
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.

Comment 13 Jeremy Newton 2016-11-26 20:31:41 UTC
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)

Comment 14 MartinKG 2016-11-27 14:43:20 UTC
created bear package https://bugzilla.redhat.com/show_bug.cgi?id=1398949

Comment 15 MartinKG 2016-12-08 15:04:41 UTC
@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@fedoraproject.org> - 1.0.16-1
- Update to 1.0.16
- Update Source URL

Comment 16 Jeremy Newton 2016-12-09 05:02:57 UTC
(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@fedoraproject.org> - 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.

Comment 17 Jeremy Newton 2016-12-09 05:19:30 UTC
(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.

Comment 18 MartinKG 2016-12-09 13:53:51 UTC
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@fedoraproject.org> - 1.0.18-1
- Update to 1.0.18
- Update Source URL
- Add Requires bear-engine >= %%{bear_version}
- Add Requires bear-factory >= %%{bear_version}

Comment 19 Jeremy Newton 2016-12-11 21:05:42 UTC
I still get the claw error when building in mock.

Comment 20 MartinKG 2016-12-13 16:47:59 UTC
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!

Comment 21 Jeremy Newton 2016-12-13 18:05:09 UTC
(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.

Comment 22 MartinKG 2016-12-14 09:16:04 UTC
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@fedoraproject.org> - 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

Comment 23 MartinKG 2016-12-21 13:54:49 UTC
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@fedoraproject.org> - 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

Comment 24 Jeremy Newton 2016-12-23 05:04:09 UTC
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.

Comment 25 MartinKG 2016-12-23 10:07:29 UTC
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@fedoraproject.org> - 1.0.18-3
- remove Source1
- convert docbook2man filename taken from .sgml file to lowercase
- add macor %%license

Comment 26 Jeremy Newton 2016-12-31 04:50:33 UTC
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

Comment 27 MartinKG 2017-01-02 21:05:38 UTC
(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@fedoraproject.org> - 1.0.18-4
- correct license to GPLv3
- add license breakdown
- add BR  desktop-file-utils
- add RR hicolor-icon-theme

Comment 28 Jeremy Newton 2017-01-03 23:07:20 UTC
(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.

Comment 29 MartinKG 2017-01-04 10:27:20 UTC
(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

Comment 30 Jeremy Newton 2017-01-04 13:43:02 UTC
Looks good, approved!

Comment 31 MartinKG 2017-01-04 15:16:12 UTC
@Jeremy
many thanks for your assistance !

Comment 32 Gwyn Ciesla 2017-01-04 16:00:36 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/andy-super-great-park

Comment 33 MartinKG 2017-01-04 18:50:38 UTC
Hi Jeremy,

can we rerequest the package as asgp ?

Comment 34 Jeremy Newton 2017-01-04 20:16:44 UTC
please use asgp, not andy-super-great-park

Comment 35 Gwyn Ciesla 2017-01-04 22:44:14 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/asgp

Comment 36 MartinKG 2017-01-10 14:59:52 UTC
package has been built successfully on fc24, fc25 and rawhide.


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