Bug 1398949 - Review Request: bear - Game engine and editors dedicated to creating great 2D games
Summary: Review Request: bear - Game engine and editors dedicated to creating great 2D...
Keywords:
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:
Depends On:
Blocks: 1176273 1403607 1406787
TreeView+ depends on / blocked
 
Reported: 2016-11-27 14:41 UTC by MartinKG
Modified: 2017-01-16 21:30 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2016-12-27 08:24:08 UTC
Type: ---
alexjnewt: fedora-review+


Attachments (Terms of Use)

Description MartinKG 2016-11-27 14:41:04 UTC
Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/bear.spec
SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/bear-0.7.0-0.1gitac6be8b.fc25.src.rpm
Description: Game engine and editors dedicated to creating great 2D games
Fedora Account System Username: martinkg

%changelog
* Sun Nov 27 2016 Martin Gansser <martinkg@fedoraproject.org> - 0.7.0-0.1gitac6be8b
- imported package bear


rpmlint -i bear.spec /home/martin/rpmbuild/SRPMS/bear-0.7.0-0.1gitac6be8b.fc25.src.rpm /home/martin/rpmbuild/RPMS/x86_64/bear-engine-0.7.0-0.1gitac6be8b.fc25.x86_64.rpm /home/martin/rpmbuild/RPMS/x86_64/bear-factory-0.7.0-0.1gitac6be8b.fc25.x86_64.rpm /home/martin/rpmbuild/RPMS/x86_64/bear-debuginfo-0.7.0-0.1gitac6be8b.fc25.x86_64.rpm
bear.src: W: spelling-error %description -l en_US plee -> peel, pee, lee
The value of this tag appears to be misspelled. Please double-check.

bear.src: W: spelling-error %description -l en_US andy -> Andy, and, any
The value of this tag appears to be misspelled. Please double-check.

bear-engine.x86_64: W: spelling-error Summary(en_US) Runtime -> Run time, Run-time, Rudiment
The value of this tag appears to be misspelled. Please double-check.

bear-engine.x86_64: W: spelling-error %description -l en_US plee -> peel, pee, lee
The value of this tag appears to be misspelled. Please double-check.

bear-engine.x86_64: W: spelling-error %description -l en_US andy -> Andy, and, any
The value of this tag appears to be misspelled. Please double-check.

bear-engine.x86_64: W: spelling-error %description -l en_US runtime -> run time, run-time, rudiment
The value of this tag appears to be misspelled. Please double-check.

bear-engine.x86_64: W: manual-page-warning /usr/share/man/man6/running-bear.6.gz 5: warning: macro `RS' not defined
This man page may contain problems that can cause it not to be formatted as
intended.

bear-engine.x86_64: W: manual-page-warning /usr/share/man/man6/running-bear.6.gz 12: warning: macro `RE' not defined
This man page may contain problems that can cause it not to be formatted as
intended.

bear-engine.x86_64: W: non-conffile-in-etc /etc/ld.so.conf.d/bear-engine-x86_64.conf
A non-executable file in your package is being installed in /etc, but is not a
configuration file. All non-executable files in /etc should be configuration
files. Mark the file as %config in the spec file.

bear-factory.x86_64: W: manual-page-warning /usr/share/man/man1/bf-animation-editor.1.gz 5: warning: macro `RS' not defined
This man page may contain problems that can cause it not to be formatted as
intended.

bear-factory.x86_64: W: manual-page-warning /usr/share/man/man1/bf-animation-editor.1.gz 12: warning: macro `RE' not defined
This man page may contain problems that can cause it not to be formatted as
intended.

bear-factory.x86_64: W: manual-page-warning /usr/share/man/man1/bf-level-editor.1.gz 5: warning: macro `RS' not defined
This man page may contain problems that can cause it not to be formatted as
intended.

bear-factory.x86_64: W: manual-page-warning /usr/share/man/man1/bf-level-editor.1.gz 12: warning: macro `RE' not defined
This man page may contain problems that can cause it not to be formatted as
intended.

bear-factory.x86_64: W: manual-page-warning /usr/share/man/man1/bf-model-editor.1.gz 5: warning: macro `RS' not defined
This man page may contain problems that can cause it not to be formatted as
intended.

bear-factory.x86_64: W: manual-page-warning /usr/share/man/man1/bf-model-editor.1.gz 12: warning: macro `RE' not defined
This man page may contain problems that can cause it not to be formatted as
intended.

bear-factory.x86_64: W: non-conffile-in-etc /etc/ld.so.conf.d/bear-factory-x86_64.conf
A non-executable file in your package is being installed in /etc, but is not a
configuration file. All non-executable files in /etc should be configuration
files. Mark the file as %config in the spec file.

bear-factory.x86_64: W: no-manual-page-for-binary image-cutter
Each executable in standard binary directories should have a man page.

bear-factory.x86_64: W: no-manual-page-for-binary bend-image
Each executable in standard binary directories should have a man page.

bear-factory.x86_64: W: desktopfile-without-binary /usr/share/applications/desc2img.desktop desc2img
the .desktop file is for a file not present in the package. You should check
the requires or see if this is not a error

4 packages and 1 specfiles checked; 0 errors, 19 warnings.

Comment 1 Jeremy Newton 2016-11-27 16:20:21 UTC
(In reply to MartinKG from comment #0)
> Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/bear.spec
> SRPM URL:
> https://martinkg.fedorapeople.org/Review/SRPMS/bear-0.7.0-0.1gitac6be8b.fc25.
> src.rpm
> Description: Game engine and editors dedicated to creating great 2D games
> Fedora Account System Username: martinkg
> 
> %changelog
> * Sun Nov 27 2016 Martin Gansser <martinkg@fedoraproject.org> -
> 0.7.0-0.1gitac6be8b
> - imported package bear
> 
> 
> rpmlint -i bear.spec
> /home/martin/rpmbuild/SRPMS/bear-0.7.0-0.1gitac6be8b.fc25.src.rpm
> /home/martin/rpmbuild/RPMS/x86_64/bear-engine-0.7.0-0.1gitac6be8b.fc25.
> x86_64.rpm
> /home/martin/rpmbuild/RPMS/x86_64/bear-factory-0.7.0-0.1gitac6be8b.fc25.
> x86_64.rpm
> /home/martin/rpmbuild/RPMS/x86_64/bear-debuginfo-0.7.0-0.1gitac6be8b.fc25.
> x86_64.rpm
> bear.src: W: spelling-error %description -l en_US plee -> peel, pee, lee
> The value of this tag appears to be misspelled. Please double-check.
> 
> bear.src: W: spelling-error %description -l en_US andy -> Andy, and, any
> The value of this tag appears to be misspelled. Please double-check.
> 
> bear-engine.x86_64: W: spelling-error Summary(en_US) Runtime -> Run time,
> Run-time, Rudiment
> The value of this tag appears to be misspelled. Please double-check.
> 
> bear-engine.x86_64: W: spelling-error %description -l en_US plee -> peel,
> pee, lee
> The value of this tag appears to be misspelled. Please double-check.
> 
> bear-engine.x86_64: W: spelling-error %description -l en_US andy -> Andy,
> and, any
> The value of this tag appears to be misspelled. Please double-check.
> 
> bear-engine.x86_64: W: spelling-error %description -l en_US runtime -> run
> time, run-time, rudiment
> The value of this tag appears to be misspelled. Please double-check.
> 
> bear-engine.x86_64: W: manual-page-warning
> /usr/share/man/man6/running-bear.6.gz 5: warning: macro `RS' not defined
> This man page may contain problems that can cause it not to be formatted as
> intended.
> 
> bear-engine.x86_64: W: manual-page-warning
> /usr/share/man/man6/running-bear.6.gz 12: warning: macro `RE' not defined
> This man page may contain problems that can cause it not to be formatted as
> intended.
> 
> bear-engine.x86_64: W: non-conffile-in-etc
> /etc/ld.so.conf.d/bear-engine-x86_64.conf
> A non-executable file in your package is being installed in /etc, but is not
> a
> configuration file. All non-executable files in /etc should be configuration
> files. Mark the file as %config in the spec file.
> 
> bear-factory.x86_64: W: manual-page-warning
> /usr/share/man/man1/bf-animation-editor.1.gz 5: warning: macro `RS' not
> defined
> This man page may contain problems that can cause it not to be formatted as
> intended.
> 
> bear-factory.x86_64: W: manual-page-warning
> /usr/share/man/man1/bf-animation-editor.1.gz 12: warning: macro `RE' not
> defined
> This man page may contain problems that can cause it not to be formatted as
> intended.
> 
> bear-factory.x86_64: W: manual-page-warning
> /usr/share/man/man1/bf-level-editor.1.gz 5: warning: macro `RS' not defined
> This man page may contain problems that can cause it not to be formatted as
> intended.
> 
> bear-factory.x86_64: W: manual-page-warning
> /usr/share/man/man1/bf-level-editor.1.gz 12: warning: macro `RE' not defined
> This man page may contain problems that can cause it not to be formatted as
> intended.
> 
> bear-factory.x86_64: W: manual-page-warning
> /usr/share/man/man1/bf-model-editor.1.gz 5: warning: macro `RS' not defined
> This man page may contain problems that can cause it not to be formatted as
> intended.
> 
> bear-factory.x86_64: W: manual-page-warning
> /usr/share/man/man1/bf-model-editor.1.gz 12: warning: macro `RE' not defined
> This man page may contain problems that can cause it not to be formatted as
> intended.
> 
> bear-factory.x86_64: W: non-conffile-in-etc
> /etc/ld.so.conf.d/bear-factory-x86_64.conf
> A non-executable file in your package is being installed in /etc, but is not
> a
> configuration file. All non-executable files in /etc should be configuration
> files. Mark the file as %config in the spec file.
> 
> bear-factory.x86_64: W: no-manual-page-for-binary image-cutter
> Each executable in standard binary directories should have a man page.
> 
> bear-factory.x86_64: W: no-manual-page-for-binary bend-image
> Each executable in standard binary directories should have a man page.
> 
> bear-factory.x86_64: W: desktopfile-without-binary
> /usr/share/applications/desc2img.desktop desc2img
> the .desktop file is for a file not present in the package. You should check
> the requires or see if this is not a error
> 
> 4 packages and 1 specfiles checked; 0 errors, 19 warnings.

Some comments:
- I would remove "Conflicts:      wxGTK3-devel"
- The sed commands can be compressed into:
# change docbook_to_man to docbook2man
sed -i -e 's|docbook-to-man|docbook2man|g' cmake-helper/docbook-to-man.cmake
#sed -i -e 's|docbook-to-man|docbook-utils|' CMakeLists.txt
sed -i -e 's|docbook-to-man|docbook2man|g' bear-*/desktop/man/*.sgml
sed -i -e 's|docbook-to-man|docbook2man|g' bear-*/desktop/man/CMakeLists.txt
- I would replace (non packaged) with (tunnel) from the descriptions, as this is the upstream git name and likely the future package name.
- andy-super-great-park should be asgp (the upstream git name)
- run-time is the correct spelling, not runtime
- Add "%config" to fix the non-conffile-in-etc warnings
- Is the desc2img binary missing? Or is this a typo in the .desktop?
- I would think you should patch the man pages to fix the warnings, but I haven't looked into it

Comment 2 MartinKG 2016-11-27 19:10:20 UTC
Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/bear.spec
SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/bear-0.7.0-0.2gitac6be8b.fc25.src.rpm

%changelog
* Sun Nov 27 2016 Martin Gansser <martinkg@fedoraproject.org> - 0.7.0-0.2gitac6be8b
- Remove Conflicts: wxGTK3-devel"
- Compressed sed command
- replace (non packaged) with (tunnel) from the descriptions
- replace (andy-super-great-park) with (asgp) from the descriptions
- run-time is the correct spelling, not runtime
- Add %%config to fix the non-conffile-in-etc warnings
- Remove desc2img.desktop due desc2img binary missing

Comment 3 MartinKG 2016-11-28 07:18:28 UTC
(In reply to Jeremy Newton from comment #1)

> 
> Some comments:
> - I would remove "Conflicts:      wxGTK3-devel"

the build fails if wxGTK3-devel is installed !

/home/martin/rpmbuild/BUILD/bear-ac6be8bebf35cd1a4d4151773707c9ee313b154e/bear-factory/bear-editor/src/bf/code/accelerator_table.cpp:102:45: error: 'virtual bool wxEvtHandler::ProcessEvent(wxEvent&)' is inaccessible within this context
       m_event_handler.ProcessEvent( command );
                                             ^
In file included from /usr/include/wx-3.0/wx/window.h:18:0,
                 from /home/martin/rpmbuild/BUILD/bear-ac6be8bebf35cd1a4d4151773707c9ee313b154e/bear-factory/bear-editor/src/bf/../bf/accelerator_table.hpp:19,
                 from /home/martin/rpmbuild/BUILD/bear-ac6be8bebf35cd1a4d4151773707c9ee313b154e/bear-factory/bear-editor/src/bf/code/accelerator_table.cpp:14:
/usr/include/wx-3.0/wx/event.h:3355:18: note: declared here
     virtual bool ProcessEvent(wxEvent& event);
                  ^~~~~~~~~~~~
bear-factory/bear-editor/src/bf/CMakeFiles/bear-editor.dir/build.make:65: recipe for target 'bear-factory/bear-editor/src/bf/CMakeFiles/bear-editor.dir/code/accelerator_table.cpp.o' failed

Comment 4 MartinKG 2016-11-28 08:08:54 UTC
there is another issue:
bear checks the presence of libclaw, it is only available for Fedora 24.

# rpm -qa |grep claw
claws-mail-3.14.0-1.fc25.x86_64
libclaw-1.7.4-13.fc24.x86_64
libclaw-devel-1.7.4-13.fc24.x86_64

bear-config.cmake:  message( FATAL_ERROR "The Bear Engine needs libclaw." )

Comment 5 MartinKG 2016-11-28 13:34:43 UTC
there is another issue:

the package plee-the-bear already contains the folder
%{_datadir}/bear-factory

Comment 6 Jeremy Newton 2016-11-28 13:46:31 UTC
You need to use BuildConflicts not Conflicts.

As for libclaw, it definitely does exist for f25/26:
https://apps.fedoraproject.org/packages/libclaw
It was just never rebuilt for f25.

As for plee-the-bear, bear needs to be unbundled after this package is accepted. I would contact the maintainers and open a bugreport (make it depend on this one).

Comment 7 MartinKG 2016-11-28 15:44:19 UTC
(In reply to Jeremy Newton from comment #6)
> You need to use BuildConflicts not Conflicts.
> 
> As for libclaw, it definitely does exist for f25/26:
> https://apps.fedoraproject.org/packages/libclaw
> It was just never rebuilt for f25.
> 
> As for plee-the-bear, bear needs to be unbundled after this package is
> accepted. I would contact the maintainers and open a bugreport (make it
> depend on this one).

I had today contact with the maintainer of libclaw, he built the package for rawhide.


Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/bear.spec
SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/bear-0.7.0-0.3gitac6be8b.fc25.src.rpm

%changelog
* Mon Nov 28 2016 Martin Gansser <martinkg@fedoraproject.org> - 0.7.0-0.3gitac6be8b
- Add BR chrpath
- Add BR libjpeg-turbo-devel
- Add BuildConflicts wxGTK3-devel

Comment 8 Jeremy Newton 2016-12-08 01:23:40 UTC
(In reply to MartinKG from comment #7)
> (In reply to Jeremy Newton from comment #6)
> > You need to use BuildConflicts not Conflicts.
> > 
> > As for libclaw, it definitely does exist for f25/26:
> > https://apps.fedoraproject.org/packages/libclaw
> > It was just never rebuilt for f25.
> > 
> > As for plee-the-bear, bear needs to be unbundled after this package is
> > accepted. I would contact the maintainers and open a bugreport (make it
> > depend on this one).
> 
> I had today contact with the maintainer of libclaw, he built the package for
> rawhide.
> 
> 
> Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/bear.spec
> SRPM URL:
> https://martinkg.fedorapeople.org/Review/SRPMS/bear-0.7.0-0.3gitac6be8b.fc25.
> src.rpm
> 
> %changelog
> * Mon Nov 28 2016 Martin Gansser <martinkg@fedoraproject.org> -
> 0.7.0-0.3gitac6be8b
> - Add BR chrpath
> - Add BR libjpeg-turbo-devel
> - Add BuildConflicts wxGTK3-devel

Have you spoke to plee the bear maintainers?

Taking this review.

Comment 9 Jeremy Newton 2016-12-09 05:19:45 UTC
I get a build error when attempting to build in mock (F25):

bear-factory/bear-editor/src/bf/CMakeFiles/bear-editor.dir/build.make:65: recipe for target 'bear-factory/bear-editor/src/bf/CMakeFiles/bear-editor.dir/code/accelerator_table.cpp.o' failed
make[2]: Leaving directory '/builddir/build/BUILD/bear-ac6be8bebf35cd1a4d4151773707c9ee313b154e'
/builddir/build/BUILD/bear-ac6be8bebf35cd1a4d4151773707c9ee313b154e/bear-factory/bear-editor/src/bf/code/accelerator_table.cpp: In member function 'void bf::accelerator_table::on_key_pressed(wxKeyEvent&)':
/builddir/build/BUILD/bear-ac6be8bebf35cd1a4d4151773707c9ee313b154e/bear-factory/bear-editor/src/bf/code/accelerator_table.cpp:102:45: error: 'virtual bool wxEvtHandler::ProcessEvent(wxEvent&)' is inaccessible within this context
       m_event_handler.ProcessEvent( command );
                                             ^
In file included from /usr/include/wx-3.0/wx/window.h:18:0,
                 from /builddir/build/BUILD/bear-ac6be8bebf35cd1a4d4151773707c9ee313b154e/bear-factory/bear-editor/src/bf/../bf/accelerator_table.hpp:19,
                 from /builddir/build/BUILD/bear-ac6be8bebf35cd1a4d4151773707c9ee313b154e/bear-factory/bear-editor/src/bf/code/accelerator_table.cpp:14:
/usr/include/wx-3.0/wx/event.h:3355:18: note: declared here
     virtual bool ProcessEvent(wxEvent& event);
                  ^~~~~~~~~~~~
make[2]: *** [bear-factory/bear-editor/src/bf/CMakeFiles/bear-editor.dir/code/accelerator_table.cpp.o] Error 1
make[1]: *** [bear-factory/bear-editor/src/bf/CMakeFiles/bear-editor.dir/all] Error 2

Comment 10 MartinKG 2016-12-09 14:54:18 UTC
(In reply to Jeremy Newton from comment #9)
> I get a build error when attempting to build in mock (F25):
> 
> bear-factory/bear-editor/src/bf/CMakeFiles/bear-editor.dir/build.make:65:
> recipe for target
> 'bear-factory/bear-editor/src/bf/CMakeFiles/bear-editor.dir/code/
> accelerator_table.cpp.o' failed
> make[2]: Leaving directory
> '/builddir/build/BUILD/bear-ac6be8bebf35cd1a4d4151773707c9ee313b154e'
> /builddir/build/BUILD/bear-ac6be8bebf35cd1a4d4151773707c9ee313b154e/bear-
> factory/bear-editor/src/bf/code/accelerator_table.cpp: In member function
> 'void bf::accelerator_table::on_key_pressed(wxKeyEvent&)':
> /builddir/build/BUILD/bear-ac6be8bebf35cd1a4d4151773707c9ee313b154e/bear-
> factory/bear-editor/src/bf/code/accelerator_table.cpp:102:45: error:
> 'virtual bool wxEvtHandler::ProcessEvent(wxEvent&)' is inaccessible within
> this context
>        m_event_handler.ProcessEvent( command );
>                                              ^
> In file included from /usr/include/wx-3.0/wx/window.h:18:0,
>                  from
> /builddir/build/BUILD/bear-ac6be8bebf35cd1a4d4151773707c9ee313b154e/bear-
> factory/bear-editor/src/bf/../bf/accelerator_table.hpp:19,
>                  from
> /builddir/build/BUILD/bear-ac6be8bebf35cd1a4d4151773707c9ee313b154e/bear-
> factory/bear-editor/src/bf/code/accelerator_table.cpp:14:
> /usr/include/wx-3.0/wx/event.h:3355:18: note: declared here
>      virtual bool ProcessEvent(wxEvent& event);
>                   ^~~~~~~~~~~~

because the conflicting package wxGTK3-devel-3.0.2-30.fc25.x86_64 is installed.


rpm -qf /usr/include/wx-3.0/wx/event.h
wxGTK3-devel-3.0.2-30.fc25.x86_64

Comment 11 Jeremy Newton 2016-12-09 16:40:05 UTC
(In reply to MartinKG from comment #10)
> (In reply to Jeremy Newton from comment #9)
> > I get a build error when attempting to build in mock (F25):
> > 
> > bear-factory/bear-editor/src/bf/CMakeFiles/bear-editor.dir/build.make:65:
> > recipe for target
> > 'bear-factory/bear-editor/src/bf/CMakeFiles/bear-editor.dir/code/
> > accelerator_table.cpp.o' failed
> > make[2]: Leaving directory
> > '/builddir/build/BUILD/bear-ac6be8bebf35cd1a4d4151773707c9ee313b154e'
> > /builddir/build/BUILD/bear-ac6be8bebf35cd1a4d4151773707c9ee313b154e/bear-
> > factory/bear-editor/src/bf/code/accelerator_table.cpp: In member function
> > 'void bf::accelerator_table::on_key_pressed(wxKeyEvent&)':
> > /builddir/build/BUILD/bear-ac6be8bebf35cd1a4d4151773707c9ee313b154e/bear-
> > factory/bear-editor/src/bf/code/accelerator_table.cpp:102:45: error:
> > 'virtual bool wxEvtHandler::ProcessEvent(wxEvent&)' is inaccessible within
> > this context
> >        m_event_handler.ProcessEvent( command );
> >                                              ^
> > In file included from /usr/include/wx-3.0/wx/window.h:18:0,
> >                  from
> > /builddir/build/BUILD/bear-ac6be8bebf35cd1a4d4151773707c9ee313b154e/bear-
> > factory/bear-editor/src/bf/../bf/accelerator_table.hpp:19,
> >                  from
> > /builddir/build/BUILD/bear-ac6be8bebf35cd1a4d4151773707c9ee313b154e/bear-
> > factory/bear-editor/src/bf/code/accelerator_table.cpp:14:
> > /usr/include/wx-3.0/wx/event.h:3355:18: note: declared here
> >      virtual bool ProcessEvent(wxEvent& event);
> >                   ^~~~~~~~~~~~
> 
> because the conflicting package wxGTK3-devel-3.0.2-30.fc25.x86_64 is
> installed.
> 
> 
> rpm -qf /usr/include/wx-3.0/wx/event.h
> wxGTK3-devel-3.0.2-30.fc25.x86_64

Hmmm, I'll look into it this weekend, but the "BuildConflicts" should prevent that because I'm using mock. I need to look at the root.log more carefully.

Comment 12 MartinKG 2016-12-11 18:23:38 UTC
(In reply to Jeremy Newton from comment #6)
> You need to use BuildConflicts not Conflicts.
> 
> As for libclaw, it definitely does exist for f25/26:
> https://apps.fedoraproject.org/packages/libclaw
> It was just never rebuilt for f25.
> 
> As for plee-the-bear, bear needs to be unbundled after this package is
> accepted. I would contact the maintainers and open a bugreport (make it
> depend on this one).

Bug report opened:
https://github.com/j-jorge/plee-the-bear/issues/5

Comment 13 Jeremy Newton 2016-12-11 19:11:24 UTC
In the short term, we can just exclude the duplicate files from the two games and have them require the bear packages. I'm looking into the review right now.

Comment 14 Jeremy Newton 2016-12-11 20:44:17 UTC
Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed


Issues:
=======
- gtk-update-icon-cache is invoked in %postun and %posttrans if package
  contains icons.
  Note: icons in bear-factory
  See: http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache
- update-desktop-database is invoked in %post and %postun if package
  contains desktop file(s) with a MimeType: entry.
  Note: desktop file(s) with MimeType entry in bear-factory
  See: http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#desktop-
  database


===== 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.
> See additional comments below

[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.
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "BSD (2 clause)", "BSD (2 clause) MIT/X11 (BSD like)", "GPL (v2
     or later)", "*No copyright* CC by-sa (v3.0)", "Unknown or generated".
     1903 files have unknown license.
> The glew code is what's causing it to pick up BSD. I believe this code
> SHOULD be removed in prep to make sure it's not compiled or included in
> the debug package. This is more of a SHOULD than a MUST though.

[!]: License file installed when any subpackage combination is installed.
> This is due to a missing require, see "Requires correct" comment to fix it

[x]: If the package is under multiple licenses, the licensing breakdown
     must be documented in the spec.
[x]: 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/cmake,
     /usr/share/icons/hicolor/16x16/apps, /usr/lib64/bear,
     /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
[!]: Package does not own files or directories owned by other packages.
     Note: Dirs in package are owned also by: /usr/share/bear-factory/item-
     description/generic/link(plee-the-bear), /usr/share/bear-factory/item-
     description/generic/system(plee-the-bear), /usr/share/bear-
     factory/images(plee-the-bear), /usr/share/bear-factory/item-
     description/generic/expr(plee-the-bear), /usr/share/bear-factory/item-
     description/generic/forced_movement(plee-the-bear), /usr/share/bear-
     factory/item-description/generic/script(plee-the-bear), /usr/share
     /bear-factory/item-description(plee-the-bear), /usr/share/bear-factory
     /item-description/generic(plee-the-bear), /usr/share/bear-factory
     /item-description/generic/level_variable(plee-the-bear), /usr/share
     /bear-factory/item-description/generic/item_brick(plee-the-bear),
     /usr/share/bear-factory/item-description/generic/game_variable(plee-
     the-bear), /usr/share/bear-factory(plee-the-bear), /usr/share/bear-
     factory/item-description/generic/shader(plee-the-bear)
> See additional comments below

[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.
[x]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[-]: 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.
[!]: Package does not generate any conflict.
     Note: Package contains Conflicts: tag(s) needing fix or justification.
> See additional comments below

[x]: Package obeys FHS, except libexecdir and /usr/target.
[x]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[!]: Requires correct, justified where necessary.
> You need to add the following to bear-factory:
> Requires: %{name}-engine%{?_isa} = %{version}-%{release}

[x]: Spec file is legible and written in American English.
[x]: Package contains systemd file(s) if in need.
[x]: Useful -debuginfo package or justification otherwise.
[x]: Package is not known to require an ExcludeArch tag.
[x]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 10240 bytes in 1 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: There are rpmlint messages (see attachment).
> See comments inline

[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]: 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]: %config files are marked noreplace or the reason is justified.
[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]: No %config files under /usr.
[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:
[x]: 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 bear-
     engine , bear-factory , bear-debuginfo
> I've already mentioned this in the "Requires correct" comment

[x]: Package functions as described.
> I believe so

[x]: Latest version is packaged.
> Close enough, please use your own discretion here.

[x]: Package does not include license text files separate from upstream.
> I don't think so, but glew should be removed in prep just in case.

[-]: 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: There are rpmlint messages (see attachment).
[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: bear-engine-0.7.0-0.3gitac6be8b.fc25.x86_64.rpm
          bear-factory-0.7.0-0.3gitac6be8b.fc25.x86_64.rpm
          bear-debuginfo-0.7.0-0.3gitac6be8b.fc25.x86_64.rpm
          bear-0.7.0-0.3gitac6be8b.fc25.src.rpm
bear-engine.x86_64: W: spelling-error Summary(en_US) Runtime -> Run time, Run-time, Rudiment
> Run-time is the correct spelling and should be used

bear-engine.x86_64: W: spelling-error %description -l en_US plee -> peel, pee, lee
bear-engine.x86_64: W: spelling-error %description -l en_US asgp -> asp, gasp, asap
bear-engine.x86_64: W: manual-page-warning /usr/share/man/man6/running-bear.6.gz 5: warning: macro `RS' not defined
bear-engine.x86_64: W: manual-page-warning /usr/share/man/man6/running-bear.6.gz 12: warning: macro `RE' not defined
bear-factory.x86_64: W: manual-page-warning /usr/share/man/man1/bf-model-editor.1.gz 5: warning: macro `RS' not defined
bear-factory.x86_64: W: manual-page-warning /usr/share/man/man1/bf-model-editor.1.gz 12: warning: macro `RE' not defined
bear-factory.x86_64: W: manual-page-warning /usr/share/man/man1/bf-level-editor.1.gz 5: warning: macro `RS' not defined
bear-factory.x86_64: W: manual-page-warning /usr/share/man/man1/bf-level-editor.1.gz 12: warning: macro `RE' not defined
bear-factory.x86_64: W: manual-page-warning /usr/share/man/man1/bf-animation-editor.1.gz 5: warning: macro `RS' not defined
bear-factory.x86_64: W: manual-page-warning /usr/share/man/man1/bf-animation-editor.1.gz 12: warning: macro `RE' not defined
bear-factory.x86_64: W: no-manual-page-for-binary image-cutter
bear-factory.x86_64: W: no-manual-page-for-binary bend-image
> I would advice to querying upstream to fix the manpage issues, but this is a "SHOULD"

bear.src: W: spelling-error %description -l en_US plee -> peel, pee, lee
bear.src: W: spelling-error %description -l en_US asgp -> asp, gasp, asap
4 packages and 0 specfiles checked; 0 errors, 15 warnings.



Rpmlint (debuginfo)
-------------------
Checking: bear-debuginfo-0.7.0-0.3gitac6be8b.fc25.x86_64.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.





Rpmlint (installed packages)
----------------------------
bear-engine.x86_64: W: spelling-error Summary(en_US) Runtime -> Run time, Run-time, Rudiment
> Already mentioned above

bear-engine.x86_64: W: spelling-error %description -l en_US plee -> peel, pee, lee
bear-engine.x86_64: W: spelling-error %description -l en_US asgp -> asp, gasp, asap
bear-engine.x86_64: W: manual-page-warning /usr/share/man/man6/running-bear.6.gz 5: warning: macro `RS' not defined
bear-engine.x86_64: W: manual-page-warning /usr/share/man/man6/running-bear.6.gz 12: warning: macro `RE' not defined
bear-factory.x86_64: W: manual-page-warning /usr/share/man/man1/bf-animation-editor.1.gz 5: warning: macro `RS' not defined
bear-factory.x86_64: W: manual-page-warning /usr/share/man/man1/bf-animation-editor.1.gz 12: warning: macro `RE' not defined
bear-factory.x86_64: W: manual-page-warning /usr/share/man/man1/bf-level-editor.1.gz 5: warning: macro `RS' not defined
bear-factory.x86_64: W: manual-page-warning /usr/share/man/man1/bf-level-editor.1.gz 12: warning: macro `RE' not defined
bear-factory.x86_64: W: manual-page-warning /usr/share/man/man1/bf-model-editor.1.gz 5: warning: macro `RS' not defined
bear-factory.x86_64: W: manual-page-warning /usr/share/man/man1/bf-model-editor.1.gz 12: warning: macro `RE' not defined
bear-factory.x86_64: W: no-manual-page-for-binary image-cutter
bear-factory.x86_64: W: no-manual-page-for-binary bend-image
> Already mentioned above

3 packages and 0 specfiles checked; 0 errors, 13 warnings.



Requires
--------
bear-debuginfo (rpmlib, GLIBC filtered):

bear-engine (rpmlib, GLIBC filtered):
    config(bear-engine)
    libGL.so.1()(64bit)
    libGLU.so.1()(64bit)
    libSDL2-2.0.so.0()(64bit)
    libSDL2_mixer-2.0.so.0()(64bit)
    libbear_audio.so()(64bit)
    libbear_communication.so()(64bit)
    libbear_debug.so()(64bit)
    libbear_engine.so()(64bit)
    libbear_expr.so()(64bit)
    libbear_gui.so()(64bit)
    libbear_input.so()(64bit)
    libbear_net.so()(64bit)
    libbear_text_interface.so()(64bit)
    libbear_time.so()(64bit)
    libbear_universe.so()(64bit)
    libbear_visual.so()(64bit)
    libboost_filesystem.so.1.60.0()(64bit)
    libboost_regex.so.1.60.0()(64bit)
    libboost_system.so.1.60.0()(64bit)
    libboost_thread.so.1.60.0()(64bit)
    libc.so.6()(64bit)
    libclaw_application.so.1()(64bit)
    libclaw_dynamic_library.so.1()(64bit)
    libclaw_graphic.so.1()(64bit)
    libclaw_logger.so.1()(64bit)
    libclaw_net.so.1()(64bit)
    libclaw_tween.so.1()(64bit)
    libdl.so.2()(64bit)
    libfreetype.so.6()(64bit)
    libgcc_s.so.1()(64bit)
    libgcc_s.so.1(GCC_3.0)(64bit)
    libjpeg.so.62()(64bit)
    libm.so.6()(64bit)
    libpng16.so.16()(64bit)
    libpthread.so.0()(64bit)
    libstdc++.so.6()(64bit)
    libstdc++.so.6(CXXABI_1.3)(64bit)
    libstdc++.so.6(CXXABI_1.3.8)(64bit)
    libz.so.1()(64bit)
    rtld(GNU_HASH)

bear-factory (rpmlib, GLIBC filtered):
    config(bear-factory)
    libbear-editor.so()(64bit)
    libboost_filesystem.so.1.60.0()(64bit)
    libboost_system.so.1.60.0()(64bit)
    libc.so.6()(64bit)
    libclaw_application.so.1()(64bit)
    libclaw_configuration_file.so.1()(64bit)
    libclaw_graphic.so.1()(64bit)
    libclaw_logger.so.1()(64bit)
    libclaw_tween.so.1()(64bit)
    libgcc_s.so.1()(64bit)
    libgcc_s.so.1(GCC_3.0)(64bit)
    libjpeg.so.62()(64bit)
    libm.so.6()(64bit)
    libpng16.so.16()(64bit)
    libpthread.so.0()(64bit)
    libstdc++.so.6()(64bit)
    libstdc++.so.6(CXXABI_1.3)(64bit)
    libstdc++.so.6(CXXABI_1.3.8)(64bit)
    libwx_baseu-2.8.so.0()(64bit)
    libwx_baseu-2.8.so.0(WXU_2.8)(64bit)
    libwx_baseu_xml-2.8.so.0()(64bit)
    libwx_baseu_xml-2.8.so.0(WXU_2.8)(64bit)
    libwx_gtk2u_adv-2.8.so.0()(64bit)
    libwx_gtk2u_adv-2.8.so.0(WXU_2.8)(64bit)
    libwx_gtk2u_core-2.8.so.0()(64bit)
    libwx_gtk2u_core-2.8.so.0(WXU_2.8)(64bit)
    libwx_gtk2u_core-2.8.so.0(WXU_2.8.1)(64bit)
    libz.so.1()(64bit)
    rtld(GNU_HASH)



Provides
--------
bear-debuginfo:
    bear-debuginfo
    bear-debuginfo(x86-64)

bear-engine:
    bear-engine
    bear-engine(x86-64)
    config(bear-engine)
    libbear_audio.so()(64bit)
    libbear_communication.so()(64bit)
    libbear_debug.so()(64bit)
    libbear_engine.so()(64bit)
    libbear_expr.so()(64bit)
    libbear_generic_items.so()(64bit)
    libbear_gui.so()(64bit)
    libbear_input.so()(64bit)
    libbear_net.so()(64bit)
    libbear_text_interface.so()(64bit)
    libbear_time.so()(64bit)
    libbear_universe.so()(64bit)
    libbear_visual.so()(64bit)

bear-factory:
    application()
    application(bf-animation-editor.desktop)
    application(bf-level-editor.desktop)
    application(bf-model-editor.desktop)
    bear-factory
    bear-factory(x86-64)
    config(bear-factory)
    libbear-editor.so()(64bit)
    mimehandler(application/x-bear-animation)
    mimehandler(application/x-bear-level)
    mimehandler(application/x-bear-model)



Unversioned so-files
--------------------
bear-engine: /usr/lib64/bear/libbear_audio.so
bear-engine: /usr/lib64/bear/libbear_communication.so
bear-engine: /usr/lib64/bear/libbear_debug.so
bear-engine: /usr/lib64/bear/libbear_engine.so
bear-engine: /usr/lib64/bear/libbear_expr.so
bear-engine: /usr/lib64/bear/libbear_generic_items.so
bear-engine: /usr/lib64/bear/libbear_gui.so
bear-engine: /usr/lib64/bear/libbear_input.so
bear-engine: /usr/lib64/bear/libbear_net.so
bear-engine: /usr/lib64/bear/libbear_text_interface.so
bear-engine: /usr/lib64/bear/libbear_time.so
bear-engine: /usr/lib64/bear/libbear_universe.so
bear-engine: /usr/lib64/bear/libbear_visual.so
bear-factory: /usr/lib64/bear/libbear-editor.so

Source checksums
----------------
https://github.com/j-jorge/bear/archive/ac6be8bebf35cd1a4d4151773707c9ee313b154e/bear-ac6be8bebf35cd1a4d4151773707c9ee313b154e.tar.gz#/bear-ac6be8b.tar.gz :
  CHECKSUM(SHA256) this package     : 20e95af49ecd65e4d8511e7fd965782d80cbcfdbdd54f345965ef9fa55246b95
  CHECKSUM(SHA256) upstream package : 20e95af49ecd65e4d8511e7fd965782d80cbcfdbdd54f345965ef9fa55246b95


Generated by fedora-review 0.6.1 (f03e4e7) last change: 2016-05-02
Command line :/usr/bin/fedora-review -b 1398949
Buildroot used: fedora-25-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


Addition Comments
-----------------
I'm not opposed to how this is packaged and this should be fine until upstream fixes it, but it's critical that plee-the-bear's maintainers also agree with this, as they need to unbundle it. Furthermore, if everything goes well, you need to make sure that plee-the-bear and bear-* packages are pushed all in one bodhi update, which you will need to collaborate with them on as well. Please see bug #1403607.

Also, I can't reproduce the build conflicts issue I was having before, so I'm assuming this was just a mock bug/glitch. I'll make a bug with them if I can figure out how to reproduce it again.

Comment 15 MartinKG 2016-12-12 15:15:15 UTC
(In reply to Jeremy Newton from comment #14)
> Package Review
> ==============
> 
> Legend:
> [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
> [ ] = Manual review needed
> 
> 
> Issues:
> =======
> - gtk-update-icon-cache is invoked in %postun and %posttrans if package
>   contains icons.
>   Note: icons in bear-factory
>   See: http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache
> - update-desktop-database is invoked in %post and %postun if package
>   contains desktop file(s) with a MimeType: entry.
>   Note: desktop file(s) with MimeType entry in bear-factory
>   See: http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#desktop-
>   database
> 

done

> ===== 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.
> > See additional comments below

done, created subpkg devel

> 
> [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.
> [x]: License field in the package spec file matches the actual license.
>      Note: Checking patched sources after %prep for licenses. Licenses
>      found: "BSD (2 clause)", "BSD (2 clause) MIT/X11 (BSD like)", "GPL (v2
>      or later)", "*No copyright* CC by-sa (v3.0)", "Unknown or generated".
>      1903 files have unknown license.
> > The glew code is what's causing it to pick up BSD. I believe this code
> > SHOULD be removed in prep to make sure it's not compiled or included in
> > the debug package. This is more of a SHOULD than a MUST though.
> 
done

> [!]: License file installed when any subpackage combination is installed.
> > This is due to a missing require, see "Requires correct" comment to fix it
> 
> [x]: If the package is under multiple licenses, the licensing breakdown
>      must be documented in the spec.
> [x]: 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/cmake,
>      /usr/share/icons/hicolor/16x16/apps, /usr/lib64/bear,
>      /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
> [!]: Package does not own files or directories owned by other packages.
>      Note: Dirs in package are owned also by: /usr/share/bear-factory/item-
>      description/generic/link(plee-the-bear), /usr/share/bear-factory/item-
>      description/generic/system(plee-the-bear), /usr/share/bear-
>      factory/images(plee-the-bear), /usr/share/bear-factory/item-
>      description/generic/expr(plee-the-bear), /usr/share/bear-factory/item-
>      description/generic/forced_movement(plee-the-bear), /usr/share/bear-
>      factory/item-description/generic/script(plee-the-bear), /usr/share
>      /bear-factory/item-description(plee-the-bear), /usr/share/bear-factory
>      /item-description/generic(plee-the-bear), /usr/share/bear-factory
>      /item-description/generic/level_variable(plee-the-bear), /usr/share
>      /bear-factory/item-description/generic/item_brick(plee-the-bear),
>      /usr/share/bear-factory/item-description/generic/game_variable(plee-
>      the-bear), /usr/share/bear-factory(plee-the-bear), /usr/share/bear-
>      factory/item-description/generic/shader(plee-the-bear)
> > See additional comments below

done
> 
> [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.
> [x]: Development files must be in a -devel package
> [x]: Package uses nothing in %doc for runtime.
> [-]: 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.
> [!]: Package does not generate any conflict.
>      Note: Package contains Conflicts: tag(s) needing fix or justification.
> > See additional comments below
> 
> [x]: Package obeys FHS, except libexecdir and /usr/target.
> [x]: If the package is a rename of another package, proper Obsoletes and
>      Provides are present.
> [!]: Requires correct, justified where necessary.
> > You need to add the following to bear-factory:
> > Requires: %{name}-engine%{?_isa} = %{version}-%{release}
> 

done
 
> ===== SHOULD items =====
> 
> Generic:
> [x]: 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 bear-
>      engine , bear-factory , bear-debuginfo
> > I've already mentioned this in the "Requires correct" comment
> 

done

> [x]: Package functions as described.
> > I believe so
> 
> [x]: Latest version is packaged.
> > Close enough, please use your own discretion here.
> 
> [x]: Package does not include license text files separate from upstream.
> > I don't think so, but glew should be removed in prep just in case.
> 
> [-]: 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: There are rpmlint messages (see attachment).
> [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: bear-engine-0.7.0-0.3gitac6be8b.fc25.x86_64.rpm
>           bear-factory-0.7.0-0.3gitac6be8b.fc25.x86_64.rpm
>           bear-debuginfo-0.7.0-0.3gitac6be8b.fc25.x86_64.rpm
>           bear-0.7.0-0.3gitac6be8b.fc25.src.rpm
> bear-engine.x86_64: W: spelling-error Summary(en_US) Runtime -> Run time,
> Run-time, Rudiment
> > Run-time is the correct spelling and should be used
> 

done

> bear-engine.x86_64: W: spelling-error %description -l en_US plee -> peel,
> pee, lee
> bear-engine.x86_64: W: spelling-error %description -l en_US asgp -> asp,
> gasp, asap
> bear-engine.x86_64: W: manual-page-warning
> /usr/share/man/man6/running-bear.6.gz 5: warning: macro `RS' not defined
> bear-engine.x86_64: W: manual-page-warning
> /usr/share/man/man6/running-bear.6.gz 12: warning: macro `RE' not defined
> bear-factory.x86_64: W: manual-page-warning
> /usr/share/man/man1/bf-model-editor.1.gz 5: warning: macro `RS' not defined
> bear-factory.x86_64: W: manual-page-warning
> /usr/share/man/man1/bf-model-editor.1.gz 12: warning: macro `RE' not defined
> bear-factory.x86_64: W: manual-page-warning
> /usr/share/man/man1/bf-level-editor.1.gz 5: warning: macro `RS' not defined
> bear-factory.x86_64: W: manual-page-warning
> /usr/share/man/man1/bf-level-editor.1.gz 12: warning: macro `RE' not defined
> bear-factory.x86_64: W: manual-page-warning
> /usr/share/man/man1/bf-animation-editor.1.gz 5: warning: macro `RS' not
> defined
> bear-factory.x86_64: W: manual-page-warning
> /usr/share/man/man1/bf-animation-editor.1.gz 12: warning: macro `RE' not
> defined
> bear-factory.x86_64: W: no-manual-page-for-binary image-cutter
> bear-factory.x86_64: W: no-manual-page-for-binary bend-image
> > I would advice to querying upstream to fix the manpage issues, but this is a "SHOULD"
> 
upstream bug report
https://github.com/j-jorge/bear/issues/7


> Addition Comments
> -----------------
> I'm not opposed to how this is packaged and this should be fine until
> upstream fixes it, but it's critical that plee-the-bear's maintainers also
> agree with this, as they need to unbundle it. Furthermore, if everything
> goes well, you need to make sure that plee-the-bear and bear-* packages are
> pushed all in one bodhi update, which you will need to collaborate with them
> on as well. Please see bug #1403607.

I have some doubts that the developer of plee-the-bear changes this.
> 
> Also, I can't reproduce the build conflicts issue I was having before, so
> I'm assuming this was just a mock bug/glitch. I'll make a bug with them if I
> can figure out how to reproduce it again.
ok, many thanks.

New Package:
Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/bear.spec
SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/bear-0.7.0-0.4gitac6be8b.fc25.src.rpm

Comment 16 Tom "spot" Callaway 2016-12-12 17:20:13 UTC
Do you have another package (besides plee-the-bear) that depends on the bear library?

Comment 17 Tom "spot" Callaway 2016-12-12 17:22:30 UTC
(In reply to Tom "spot" Callaway from comment #16)
> Do you have another package (besides plee-the-bear) that depends on the bear
> library?

Replying to myself, I see that andy-super-great-park does. I guess the question is whether it is simpler to separate this out like this, or have plee-the-bear spit out a library package (bear-libs & bear-libs-devel).

Comment 18 Jeremy Newton 2016-12-12 17:41:50 UTC
(In reply to Tom "spot" Callaway from comment #17)
> (In reply to Tom "spot" Callaway from comment #16)
> > Do you have another package (besides plee-the-bear) that depends on the bear
> > library?
> 
> Replying to myself, I see that andy-super-great-park does. I guess the
> question is whether it is simpler to separate this out like this, or have
> plee-the-bear spit out a library package (bear-libs & bear-libs-devel).

I would say split it out, but it's up to you how you want to do it. I figure it would be cleaner, and we'll have one source per package.

I made some comments upstream, but packaging the private libraries should be ok for now:
https://github.com/j-jorge/plee-the-bear/issues/5

We can just install the headers manually for now. I'm assuming agsp and plee-the-bear just need headers from bear.

(In reply to MartinKG from comment #15)
> (In reply to Jeremy Newton from comment #14)
> > Package Review
> > ==============
> > 
> > Legend:
> > [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
> > [ ] = Manual review needed
> > 
> > 
> > Issues:
> > =======
> > - gtk-update-icon-cache is invoked in %postun and %posttrans if package
> >   contains icons.
> >   Note: icons in bear-factory
> >   See: http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache
> > - update-desktop-database is invoked in %post and %postun if package
> >   contains desktop file(s) with a MimeType: entry.
> >   Note: desktop file(s) with MimeType entry in bear-factory
> >   See: http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#desktop-
> >   database
> > 
> 
> done
> 
> > ===== 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.
> > > See additional comments below
> 
> done, created subpkg devel

Well any libraries needed for run-time should be placed into the main package, and any libraries or header/source files needed for build time should be placed into the devel package. (you may have to install the devel headers manually).

The unversioned libraries should realistically be fixed upstream IMHO. plee-the-bear and asgp should just BuildRequire the bear-engine-devel package and build without requiring two SOURCE tarballs.

Comment 19 Tom "spot" Callaway 2016-12-12 19:13:29 UTC
(In reply to Jeremy Newton from comment #18)

> Well any libraries needed for run-time should be placed into the main
> package, and any libraries or header/source files needed for build time
> should be placed into the devel package. (you may have to install the devel
> headers manually).
> 
> The unversioned libraries should realistically be fixed upstream IMHO.
> plee-the-bear and asgp should just BuildRequire the bear-engine-devel
> package and build without requiring two SOURCE tarballs.

Well put. The packages in this review are not setup like this, and they'd need to be adjusted to have the .so files in the main package (they're all needed at runtime).

Once these packages are fixed, I can see how much work it will be to rewire plee-the-bear to use them instead of its own bundled copy of bear.

Comment 20 Jeremy Newton 2016-12-12 21:23:02 UTC
(In reply to Tom "spot" Callaway from comment #19)
> (In reply to Jeremy Newton from comment #18)
> 
> > Well any libraries needed for run-time should be placed into the main
> > package, and any libraries or header/source files needed for build time
> > should be placed into the devel package. (you may have to install the devel
> > headers manually).
> > 
> > The unversioned libraries should realistically be fixed upstream IMHO.
> > plee-the-bear and asgp should just BuildRequire the bear-engine-devel
> > package and build without requiring two SOURCE tarballs.
> 
> Well put. The packages in this review are not setup like this, and they'd
> need to be adjusted to have the .so files in the main package (they're all
> needed at runtime).
> 
> Once these packages are fixed, I can see how much work it will be to rewire
> plee-the-bear to use them instead of its own bundled copy of bear.

Sounds good.

@MartinKG

You should be able to just manually copy all the needed devel files like so:

install -D cmake-helper/bear-config.cmake %{buildroot}%{_includedir}/%{name}/cmake-helper/
for file in $(find bear-engine/{core,lib}/src -name *.hpp);
do
    install -D $file %{buildroot}%{_includedir}/%{name}/$file
done

Make a devel subpackage (bear-devel) for these files, and all the lib's should move back to the respective engine or factory packages.

In asgp/plee-the-bear, all you need to do is remove bear, add bear-devel as a build require, and then add the following cmake parameter: -DBEAR_ROOT_DIRECTORY=%{_includedir}/bear

I'm not 100% sure if this will work, so please test this.

Comment 21 MartinKG 2016-12-13 16:29:48 UTC
New bear Package:
Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/bear.spec
SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/bear-0.7.0-0.5gitac6be8b.fc25.src.rpm

%changelog
* Tue Dec 13 2016 Martin Gansser <martinkg@fedoraproject.org> - 0.7.0-0.5gitac6be8b
- Dropped subpkg engine/factory-devel because unversioned files needed at runtime
- Add subpkg %%{name}-devel

Comment 22 Jeremy Newton 2016-12-13 18:26:19 UTC
(In reply to MartinKG from comment #21)
> New bear Package:
> Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/bear.spec
> SRPM URL:
> https://martinkg.fedorapeople.org/Review/SRPMS/bear-0.7.0-0.5gitac6be8b.fc25.
> src.rpm
> 
> %changelog
> * Tue Dec 13 2016 Martin Gansser <martinkg@fedoraproject.org> -
> 0.7.0-0.5gitac6be8b
> - Dropped subpkg engine/factory-devel because unversioned files needed at
> runtime
> - Add subpkg %%{name}-devel

To fix asgp, change:
> install -D cmake-helper/bear-config.cmake %{buildroot}%{_includedir}/%{name}/cmake-helper/

into

> install -D cmake-helper/*.cmake %{buildroot}%{_includedir}/%{name}/cmake-helper/

(Does install allow wildcards?)
As all of these cmake files seem to be required.

As well, the following is incorrect:
> %files devel
> %{_includedir}/%{name}/*

should be
> %files devel
> %{_includedir}/%{name}

That way the devel package owns this folder (omitting %dir is recursive).

Furthermore, some cleanup suggestions:
-The following
> %dir %{_libdir}/%{name}
> %{_libdir}/%{name}/lib%{name}*.so
> %exclude %{_libdir}/%{name}/lib%{name}-editor.so

can be changed to:
> %{_libdir}/%{name}
> %exclude %{_libdir}/%{name}/lib%{name}-editor.so

As this should be functionally the same

- The following:
> %dir %{_datadir}/%{name}-factory/
> %dir %{_datadir}/%{name}-factory/images/
> %dir %{_datadir}/%{name}-factory/item-description/
> %dir %{_datadir}/%{name}-factory/item-description/generic
> %{_datadir}/%{name}-factory/images/
> %{_datadir}/%{name}-factory/item-description/

can be changed to:
> %{_datadir}/%{name}-factory

As "factory" owns everything in %{_datadir}/%{name}-factory anyway.

Side note, the following should probably be moved to devel as well:
> %{_datadir}/cmake/%{name}-engine

Comment 23 Tom "spot" Callaway 2016-12-13 18:31:46 UTC
Okay, I've been poking at this today. A few comments:

* You'll need to scoop up all the .tpp files as well, same logic as the .hpp files.
* The version of bear that you're using is newer and incompatible with the current version of plee-the-bear.

Now, I know that the upstream for bear is the same as the upstream for plee-the-bear, would you be willing to ask if he plans to update plee-the-bear to be compatible with git head for bear?

The only other path forward here (other than extensive patching of plee-the-bear) is for plee-the-bear to use its bundled copy in a way that doesn't conflict with your bear package.

Comment 24 Jeremy Newton 2016-12-13 19:34:31 UTC
(In reply to Tom "spot" Callaway from comment #23)
> Okay, I've been poking at this today. A few comments:
> 
> * You'll need to scoop up all the .tpp files as well, same logic as the .hpp
> files.
> * The version of bear that you're using is newer and incompatible with the
> current version of plee-the-bear.
> 
> Now, I know that the upstream for bear is the same as the upstream for
> plee-the-bear, would you be willing to ask if he plans to update
> plee-the-bear to be compatible with git head for bear?
> 
> The only other path forward here (other than extensive patching of
> plee-the-bear) is for plee-the-bear to use its bundled copy in a way that
> doesn't conflict with your bear package.

Hmmm, that is problematic. I assume plee-the-bear git head also fails?

For the time being you'll need to bundle it, but can you rework plee-the-bear to use another folder instead of %{_datadir}/bear-factory? That way we can avoid conflicts with this package, as I assuming upstream will eventually update plee to work with this.

Comment 25 Tom "spot" Callaway 2016-12-13 19:57:22 UTC
(In reply to Jeremy Newton from comment #24)

> Hmmm, that is problematic. I assume plee-the-bear git head also fails?
> 
> For the time being you'll need to bundle it, but can you rework
> plee-the-bear to use another folder instead of %{_datadir}/bear-factory?
> That way we can avoid conflicts with this package, as I assuming upstream
> will eventually update plee to work with this.

Yeah, that was the first thing I checked, but upstream hasn't touched plee-the-bear git in over a year.

I think it won't be too hard to avoid conflicts, since plee-the-bear really only uses %{_datadir}/bear-factory/plee-the-bear/. I've got a local build going that removes the other files from %{_datadir}/bear-factory/* just to make sure. If that succeeds, I'll push it to rawhide (and other targets as needed).

Comment 26 Tom "spot" Callaway 2016-12-13 20:35:48 UTC
plee-the-bear-0.7.0-10 (now building in rawhide) removes all the conflicts. I think that is sufficient for bear to proceed, though, it would be nice to be able to use a consistent system copy of bear for all the apps.

Comment 27 MartinKG 2016-12-14 08:07:08 UTC
https://martinkg.fedorapeople.org/Review/SRPMS/bear-0.7.0-0.6gitac6be8b.fc25.src.rpm(In reply to Tom "spot" Callaway from comment #23)
> Okay, I've been poking at this today. A few comments:
> 
> * You'll need to scoop up all the .tpp files as well, same logic as the .hpp
> files.
> * The version of bear that you're using is newer and incompatible with the
> current version of plee-the-bear.
> 
> Now, I know that the upstream for bear is the same as the upstream for
> plee-the-bear, would you be willing to ask if he plans to update
> plee-the-bear to be compatible with git head for bear?
> 

That's the answer from the developer of plee-the-bear:
Hello,

I have pushed some commits tonight to fix the compilation of plee the bear with the head of bear. Actually, there's no plan to maintain nor to update plee anymore so i hope these changes will be enough.

By the way, it's good to see that some people are still interested by these games. Thanks you for maintaining the packages :)

Best regards,

Julien

Comment 28 MartinKG 2016-12-14 08:09:19 UTC
New bear Package:
Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/bear.spec
SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/bear-0.7.0-0.6gitac6be8b.fc25.src.rpm

%changelog
* Tue Dec 13 2016 Martin Gansser <martinkg@fedoraproject.org> - 0.7.0-0.6gitac6be8b
- use wildcard to copy all cmake and cmake.in files for subpkg bear-devel
- copy also *.tpp files for subpkg bear-devel
- specfile cleanup

Comment 29 Jeremy Newton 2016-12-23 04:47:42 UTC
Looks good, but the one issue remains. You need to set DCMAKE_SKIP_RPATH to ON. You need this ON to skip the build step that adds rpath to the binaries. The use of chrpath can be avoided by turning this on.

Please fix this prior to importing the package.

Approved.

Comment 30 MartinKG 2016-12-23 08:42:44 UTC
(In reply to Jeremy Newton from comment #29)
> Looks good, but the one issue remains. You need to set DCMAKE_SKIP_RPATH to
> ON. You need this ON to skip the build step that adds rpath to the binaries.
> The use of chrpath can be avoided by turning this on.
> 
> Please fix this prior to importing the package.
> 
> Approved.

done
Many thanks for the review.

New bear Package:
Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/bear.spec
SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/bear-0.7.0-0.7gitac6be8b.fc25.src.rpm

%changelog
* Fri Dec 23 2016 Martin Gansser <martinkg@fedoraproject.org> - 0.7.0-0.7gitac6be8b
- change to -DCMAKE_SKIP_RPATH:BOOL=ON
- obsolete chrpath command
- convert docbook2man filename taken from .sgml file to lowercase

Comment 31 Jeremy Newton 2016-12-23 13:33:26 UTC
Np, remember to remove BuildRequires:  chrpath

Comment 32 Gwyn Ciesla 2016-12-23 13:49:12 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/bear

Comment 33 MartinKG 2016-12-27 08:24:08 UTC
package has been built successfully on fc24, fc25 and rawhide.

Comment 34 Michael Schwendt 2017-01-11 16:16:34 UTC
There are two things that seem to have slipped through during package review. At least they have not been commented on:

> %build
> %cmake -DBEAR_ENGINE_INSTALL_LIBRARY_DIR=%{_lib}/%{name} \
>        -DBEAR_FACTORY_INSTALL_LIBRARY_DIR=%{_lib}/%{name}  \

What's the goal of relocating the libs into that directory?
Where would they be installed by default?
Where do dependencies search for the bear libs by default?
Do they CMake files as included within the Fedora bear packages find the libs by default?

> %config(noreplace) %{_sysconfdir}/ld.so.conf.d/%{name}-engine-%{_arch}.conf     

This ld.so config file adds the same directory to runtime linker's search path. Of course! Else they wouldn't be found in that path. But that makes them available globally again, just as if they were put into %_libdir directly. And the ld.so cache contents even are searched before the system's libdirs.

So, there is no gain from relocating them.

Not running "ldconfig" in the scriptlets of the -engine and -factory subpackages is a related mistake, because of the added ld.so config files that extend the search path. Anything else (a package or the admin) that runs ldconfig, would also find the bear libs and add them to the cache. And then uninstalling the package, the cache ought to be updated again to remove the lib entries again.

$ ldconfig -p|grep bear
	libbear_visual.so (libc6,x86-64) => /usr/lib64/bear/libbear_visual.so
	libbear_universe.so (libc6,x86-64) => /usr/lib64/bear/libbear_universe.so
	libbear_time.so (libc6,x86-64) => /usr/lib64/bear/libbear_time.so
	libbear_text_interface.so (libc6,x86-64) => /usr/lib64/bear/libbear_text_interface.so
	libbear_net.so (libc6,x86-64) => /usr/lib64/bear/libbear_net.so
	libbear_input.so (libc6,x86-64) => /usr/lib64/bear/libbear_input.so
	libbear_gui.so (libc6,x86-64) => /usr/lib64/bear/libbear_gui.so
	libbear_generic_items.so (libc6,x86-64) => /usr/lib64/bear/libbear_generic_items.so
	libbear_expr.so (libc6,x86-64) => /usr/lib64/bear/libbear_expr.so
	libbear_engine.so (libc6,x86-64) => /usr/lib64/bear/libbear_engine.so
	libbear_debug.so (libc6,x86-64) => /usr/lib64/bear/libbear_debug.so
	libbear_communication.so (libc6,x86-64) => /usr/lib64/bear/libbear_communication.so
	libbear_audio.so (libc6,x86-64) => /usr/lib64/bear/libbear_audio.so


> %{_mandir}/man6/running-%n{ame}.6*

This refers to a missing executable.

Comment 35 MartinKG 2017-01-16 21:30:38 UTC
Have implemented your suggestions.

bear-0.7.0-0.12.20161230git781ec80.fc24 has been submitted as an update to Fedora 24. 
https://bodhi.fedoraproject.org/updates/FEDORA-2017-01992a871e

bear-0.7.0-0.12.20161230git781ec80.fc25 has been submitted as an update to Fedora 25.
https://bodhi.fedoraproject.org/updates/FEDORA-2017-f0e0b41d10


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