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.