Bug 556346

Stage was submitted previously at https://bugzilla.redhat.com/show_bug.cgi?id=452486 and was closed as a dead review.

*** Bug 452486 has been marked as a duplicate of this bug. ***

A few comments:

- IMHO there's no sense in using macros for patch names. Normally you can use the same patch for a bunch of minor releases; name the patch after the version you wrote it against.

- Source URL doesn't accord to guidelines, mesh.dl. should be downloads.

- Drop the explicit Requires: fltk. It should be automatically picked up by rpm.

- I doubt -devel really needs cmake. Drop the requirement.

- Isn't SMP make working? If not, make a comment. Using "make -j1" is silly, why not use just plain "make"?

- Please make %files section more explicit.

I see FE-NEEDSPONSOR is flagged. Be sure to go through the Packaging and Review guidelines at
 http://fedoraproject.org/wiki/Packaging/Guidelines
 http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
Additionally to the Packaging Guidelines, there are a bunch of language / application specific guidelines that are linked to in the Packaging Guidelines.

Here are some tricks of the trade:
http://fedoraproject.org/wiki/Packaging_tricks
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets
http://fedoraproject.org/wiki/Common_Rpmlint_issue

(In reply to comment #3)
> A few comments:
> 
> - IMHO there's no sense in using macros for patch names. Normally you can use
> the same patch for a bunch of minor releases; name the patch after the version
> you wrote it against. 
I got rid of the macros.
>
> - Source URL doesn't accord to guidelines, mesh.dl. should be downloads. 
Fixed
>
> - Drop the explicit Requires: fltk. It should be automatically picked up by
> rpm. 
Done
>
> - I doubt -devel really needs cmake. Drop the requirement. 
You're right, it doesn't install any cmake modules.  Moved it to BuildRequires.
>
> - Isn't SMP make working? If not, make a comment. Using "make -j1" is silly,
> why not use just plain "make"?
The SMP seems to create some strange racing conditions and fails at random spots.  It builds fine without the smp flags, so I added a comment to the .spec
> 
> - Please make %files section more explicit.    
I changed the %files to make folder names and file extensions explicit, so I'm not catching everything with globs. 

The updated package is here:
Spec URL: http://www.richmattes.com/rpms/stage/stage.spec
SRPM URL: http://www.richmattes.com/rpms/stage/stage-3.2.2-2.fc12.src.rpm

$ rpmlint stage.spec ../RPMS/i686/stage-* ../RPMS/noarch/stage* ../SRPMS/stage*
stage.i686: W: shared-lib-calls-exit /usr/lib/libstage.so.3.2.2 exit
stage-devel.i686: W: no-documentation
stage-playerplugin.i686: W: no-documentation
6 packages and 1 specfiles checked; 0 errors, 3 warnings.

koji dist-f12
http://koji.fedoraproject.org/koji/taskinfo?taskID=1930806


Thanks for the links!  I've been over all of the guidelines, but the Packaging Tricks and rpmlint issues pages helped me out a lot.  I've been doing some informal reviews on other review requests, and helping out with updating the "player" package as I learn the ropes.

All of the patches were submitted to the upstream patch tracker. I will add the links to the specfile comments on the next version bump.  I'll also remove the errant %name macro from the -devel file's summary.

Bump with some specfile cleanup, built against F11 and F12

Spec URL: http://www.richmattes.com/rpms/stage/stage.spec
SRPM URL: http://www.richmattes.com/rpms/stage/stage-3.2.2-3.fc12.src.rpm

koji dist-f11
http://koji.fedoraproject.org/koji/taskinfo?taskID=1964063

koji dist-f12
http://koji.fedoraproject.org/koji/taskinfo?taskID=1964068

$ rpmlint stage.spec ../RPMS/i686/stage-* ../SRPMS/stage-3.2.2-3.fc12.src.rpm ../RPMS/noarch/stage-doc-3.2.2-3.fc12.noarch.rpm 
stage.i686: W: shared-lib-calls-exit /usr/lib/libstage.so.3.2.2 exit
stage-devel.i686: W: no-documentation
stage-playerplugin.i686: W: no-documentation
6 packages and 1 specfiles checked; 0 errors, 3 warnings.

And here my quick review for Stage.

MUST
* OK: rpmlint
  You shold report the shared-lib-calls-exit to upstream. The library is calling exit all over the place which could be avoided.
* OK: package name
* OK: package version and release
* OK: spec file name
* OK: package guideline-compliant
* OK: license complies with guidelines
* OK: license field accurate
* OK: license file not deleted
* OK: spec in US English
* OK: spec legible
* OK: source matches upstream
  sha256sum 90fb5160655b0c692f6a607d8cd680daacaf4093855937d436abc671b756ab13
* OK: builds under >= 1 archs, others excluded
* OK: dependencies (requires)
* OK: build dependencies complete
* N/A: locales handled using %find_lang, no %{_datadir}/locale
* OK: library -> ldconfig
* N/A: relocatable: give reason
* OK: own all directories
* OK: no dupes in %files
* OK: permission
* OK: %clean RPM_BUILD_ROOT
* OK: macros used consistently
* OK: Package contains code
* OK: large docs => -doc
* OK: doc not runtime dependent
* OK: headers in -devel
* N/A: static in -static
* OK: if contains *.pc, req pkgconfig
* OK: if libfiles are suffixed, the non-suffixed goes to devel
* OK: devel requires versioned base package
* N/A: desktop file uses desktop-file-install
* OK: clean buildroot before install
* OK: filenames UTF-8

SHOULD
* N/A: if license text missing, ask upstream to include it
* N/A: desc and summary contain translations if available
* OK: package build in mock on all architectures
  Koji builds given by packager
* OK: package functioned as described
  Works during short test. Used standalone (stage /usr/share/stage/worlds/simple.world) and in Player (player /usr/share/stage/worlds/simple.cfg).
* OK: scriplets are sane
* N/A: other subpackages should require versioned base
* N/A: if main pkg is development-wise, pkgconfig can go in main package
* OK: require package not files

Checked -3.

Some notes:

* SourceURL
  - For sourceforge hosted tarball, please follow
    https://fedoraproject.org/wiki/Packaging/SourceURL#Sourceforge.net

* BR (BuildRequires) for mesa related packages
  - For historical reason we prefer to use virtually provided dependency
    to (Build)Requires like "BuildRequires: libGL(U)-devel" instead of
    using rpm name directly.

* Requires for -devel subpackage
  - Installed stage.pc contains:
-----------------------------------------------------------------------
     7  Libs: -L${prefix}/lib -lstage -lfltk_images -lpng -lz -ljpeg -lfltk_gl -lGLU -lGL -lfltk
     8  Cflags: -I${prefix}/include/Stage-3.2 -I/usr/include/freetype2 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -D_THREAD_SAFE 
-----------------------------------------------------------------------
    This means -devel subpackage should have 
    "Requires: fltk-devel libpng-devel libjpeg-devel libGL-devel libGLU-devel"
    ( zlib-devel is pulled in by libpng-devel", freetype-devel is pulled in by
      fltk-devel )

! BuildRequires in one place
  - This is not a blocker, however it is more readable to write all
    BuildRequires in one place instead of seperating them into some
    subpackages.

* pkgconfig Requires
  - Explicitly writing "Requires: pkgconfig" is no longer needed on
    Fedora 11 and above.

* Macros
  - Use macros correctly. /usr/share should be %{_datadir}:
    https://fedoraproject.org/wiki/Packaging/RPMMacros

* Build failure
  - Your srpm fails to build on F-13:
    http://koji.fedoraproject.org/koji/taskinfo?taskID=2017917
    This is because Fedora 13 changed the behavior of linker:
    http://fedoraproject.org/wiki/UnderstandingDSOLinkChange

    To check F-13 linker behavior on F-12/11, you can do this by adding
-------------------------------------------------------------
export LDFLAGS="-Wl,--no-add-needed"
-------------------------------------------------------------
  before %cmake line.

(In reply to comment #9)
> Checked -3.
> 
> Some notes:
> 
> * SourceURL
>   - For sourceforge hosted tarball, please follow
>     https://fedoraproject.org/wiki/Packaging/SourceURL#Sourceforge.net

Fixed this, and tested with spectool -g
> 
> * BR (BuildRequires) for mesa related packages
>   - For historical reason we prefer to use virtually provided dependency
>     to (Build)Requires like "BuildRequires: libGL(U)-devel" instead of
>     using rpm name directly.
> 
Changed the BuildRequires to libGL-devel and libGLU-devel

> * Requires for -devel subpackage
>   - Installed stage.pc contains:
> -----------------------------------------------------------------------
>      7  Libs: -L${prefix}/lib -lstage -lfltk_images -lpng -lz -ljpeg -lfltk_gl
> -lGLU -lGL -lfltk
>      8  Cflags: -I${prefix}/include/Stage-3.2 -I/usr/include/freetype2
> -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -D_THREAD_SAFE 
> -----------------------------------------------------------------------
>     This means -devel subpackage should have 
>     "Requires: fltk-devel libpng-devel libjpeg-devel libGL-devel libGLU-devel"
>     ( zlib-devel is pulled in by libpng-devel", freetype-devel is pulled in by
>       fltk-devel )
> 
Added these packages to stage-devel's Requires

> ! BuildRequires in one place
>   - This is not a blocker, however it is more readable to write all
>     BuildRequires in one place instead of seperating them into some
>     subpackages.
> 
No problem, I put them all together

> * pkgconfig Requires
>   - Explicitly writing "Requires: pkgconfig" is no longer needed on
>     Fedora 11 and above.
> 
Removed

> * Macros
>   - Use macros correctly. /usr/share should be %{_datadir}:
>     https://fedoraproject.org/wiki/Packaging/RPMMacros
> 
Missed that one...I changed it to use the macro

> * Build failure
>   - Your srpm fails to build on F-13:
>     http://koji.fedoraproject.org/koji/taskinfo?taskID=2017917
>     This is because Fedora 13 changed the behavior of linker:
>     http://fedoraproject.org/wiki/UnderstandingDSOLinkChange
> 
>     To check F-13 linker behavior on F-12/11, you can do this by adding
> -------------------------------------------------------------
> export LDFLAGS="-Wl,--no-add-needed"
> -------------------------------------------------------------
>   before %cmake line.    

This is where it gets tricky.  I added the ldflags you provided above to my spec file, and I was able to patch the CMake build files to satisfy the new DSO link behavior.  I confirmed my changes worked in Mock, with the ldflags enabled in a fedora-12 build.  When I submitted the new SRPM to Koji, the build failed because Player has a broken dependency (needs to be rebuilt because OpenCV changed some sonames).  Player was rebuilt this morning, and once it gets into F13 Stage should build too.

In the meantime, I'll post my updated spec and SRPM
Spec URL: http://www.richmattes.com/rpms/stage/stage.spec
SRPM URL: http://www.richmattes.com/rpms/stage/stage-3.2.2-4.fc12.src.rpm

rpmlint: 
rpmlint stage.spec ../SRPMS/stage-3.2.2-4.fc12.src.rpm ../RPMS/i686/stage* ../RPMS/noarch/stage*
stage.src: W: spelling-error Summary(en_US) multi -> mulch, mufti
stage.src: W: spelling-error %description -l en_US scalable -> salable, scalawag, scalar
stage.i686: W: spelling-error Summary(en_US) multi -> mulch, mufti
stage.i686: W: spelling-error %description -l en_US scalable -> salable, scalawag, scalar
stage.i686: W: shared-lib-calls-exit /usr/lib/libstage.so.3.2.2 exit
stage-devel.i686: W: spelling-error %description -l en_US libstage -> lib stage, lib-stage, multistage
stage-devel.i686: W: no-documentation
stage-playerplugin.i686: W: no-documentation
6 packages and 1 specfiles checked; 0 errors, 8 warnings.

Scalable is a word, libstage is the name of the library, and I think multi-* is acceptable shorthand for multiple *.  I will investigate the shared lib calls exit further.

Well, while I don't know how to use this package, however when
I try:
- Execute: $ stage /usr/share/stage/worlds/uoa_robotics_lab.world
- After a while, choose "File -> Exit"
- choose "Quit without saving"
Then stage observes segfault, as below:

(gdb) where 
#0  0x004b7416 in __kernel_vsyscall ()
#1  0x00a1ccb1 in raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
#2  0x00a1e57a in abort () at abort.c:92
#3  0x00a5b0bd in __libc_message (do_abort=2, fmt=0xb32240 "*** glibc detected *** %s: %s: 0x%s ***\n")
    at ../sysdeps/unix/sysv/linux/libc_fatal.c:186
#4  0x00a615c1 in malloc_printerr (action=<value optimized out>, str=<value optimized out>, ptr=0x9210ec0) at malloc.c:6264
#5  0x05c44362 in operator delete(void*) () from /usr/lib/libstdc++.so.6
#6  0x00e41f65 in std::_Rb_tree<void*, std::pair<void* const, std::set<Stg::Model::stg_cb_t, std::less<Stg::Model::stg_cb_t>, std::allocator<Stg::Model::stg_cb_t> > >, std::_Select1st<std::pair<void* const, std::set<Stg::Model::stg_cb_t, std::less<Stg::Model::stg_cb_t>, std::allocator<Stg::Model::stg_cb_t> > > >, std::less<void*>, std::allocator<std::pair<void* const, std::set<Stg::Model::stg_cb_t, std::less<Stg::Model::stg_cb_t>, std::allocator<Stg::Model::stg_cb_t> > > > >::_M_erase(std::_Rb_tree_node<std::pair<void* const, std::set<Stg::Model::stg_cb_t, std::less<Stg::Model::stg_cb_t>, std::allocator<Stg::Model::stg_cb_t> > > >*) () from /usr/lib/libstage.so.3.2.2
#7  0x00e41fc5 in std::map<void*, std::set<Stg::Model::stg_cb_t, std::less<Stg::Model::stg_cb_t>, std::allocator<Stg::Model::stg_cb_t> >, std::less<void*>, std::allocator<std::pair<void* const, std::set<Stg::Model::stg_cb_t, std::less<Stg::Model::stg_cb_t>, std::allocator<Stg::Model::stg_cb_t> > > > >::~map() () from /usr/lib/libstage.so.3.2.2
#8  0x00a20798 in __cxa_finalize (d=0xe86d8c) at cxa_finalize.c:56
#9  0x00e342f5 in __do_global_dtors_aux () from /usr/lib/libstage.so.3.2.2
#10 0x00e76c60 in _fini () from /usr/lib/libstage.so.3.2.2
#11 0x00874eee in _dl_fini () at dl-fini.c:248
#12 0x00a203cf in __run_exit_handlers (status=0) at exit.c:78
#13 exit (status=0) at exit.c:100
#14 0x00e73243 in Stg::WorldGui::fileExitCb (w=0x8b7cb90, wg=0x8b7ba30) at /usr/src/debug/Stage-3.2.2-Source/libstage/worldgui.cc:511
#15 0x00cf9404 in do_callback (this=0x8b7cb90, v=0x8b7d390) at ../FL/Fl_Menu_Item.H:133
#16 Fl_Menu_::picked (this=0x8b7cb90, v=0x8b7d390) at Fl_Menu_.cxx:145
#17 0x00cf9915 in Fl_Menu_Bar::handle (this=0x8b7cb90, event=1) at Fl_Menu_Bar.cxx:64
#18 0x00ce591c in send (o=0x8b7cb90, event=1) at Fl_Group.cxx:67
#19 0x00ce5a44 in Fl_Group::handle (this=0x8b7bc70, event=1) at Fl_Group.cxx:195
#20 0x00ccf9ae in Fl_Window::handle (this=0x8b7bc70, ev=1) at Fl.cxx:1074
#21 0x00ccef14 in send (event=1, to=<value optimized out>, window=0x8b7bc70) at Fl.cxx:702
#22 0x00ccff6f in Fl::handle (e=1, window=0x8b7bc70) at Fl.cxx:740
#23 0x00d1b3a3 in fl_handle (thisevent=...) at Fl_x.cxx:1019
#24 0x00d1c6ce in do_queued_events () at Fl_x.cxx:176
#25 0x00d1cb2b in fl_wait (time_to_wait=0.03988600000000006) at Fl_x.cxx:242
#26 0x00cd0b8b in Fl::wait (time_to_wait=0.03988600000000006) at Fl.cxx:376
---Type <return> to continue, or q <return> to quit---
#27 0x00cd0cf4 in Fl::run () at Fl.cxx:384
#28 0x08049086 in main (argc=2, argv=0xbf8afb84) at /usr/src/debug/Stage-3.2.2-Source/libstage/main.cc:100

It seems something wrong on exit handlers is happening. Would
you check this?

I was able to observe this as well.  It looks like if the robot isn't moved during a simulation, the application crashes after it calls exit(0).  However, if you use a configuration file like simple.world, fasr.world, or everything.world that uses a controller (i.e. the ctrl "wander" line in the robot model), Stage is able to exit successfully.  Most of the people using stage are using controllers to implement robot behaviors, which is why I haven't encountered this particular error before.  

I recompiled the project in debug mode and submitted a backtrace upstream.  In the meantime, I'm also going to search for the root cause.

I tested all of the included .world files, the uoa_robotics_lab.world file is the only one that exhibits this particular behavior.  There are a few other .world files that don't work as well, but for the most part none of them are causing segfaults.  I've removed the offending world files for now, while the bug is tracked down.  Here's the new SRPM and spec reflecting the changes:

Spec URL: http://www.richmattes.com/rpms/stage/stage.spec
SRPM URL: http://www.richmattes.com/rpms/stage/stage-3.2.2-5.fc12.src.rpm

koji dist-f13:
http://koji.fedoraproject.org/koji/taskinfo?taskID=2043084

rpmlint:
$ rpmlint stage.spec ../RPMS/i686/stage* ../RPMS/noarch/stage* ../SRPMS/stage*
stage.i686: W: spelling-error Summary(en_US) multi -> mulch, mufti
stage.i686: W: spelling-error %description -l en_US scalable -> salable, scalawag, scalar
stage.i686: W: shared-lib-calls-exit /usr/lib/libstage.so.3.2.2 exit
stage-devel.i686: W: spelling-error %description -l en_US libstage -> lib stage, lib-stage, multistage
stage-devel.i686: W: no-documentation
stage-playerplugin.i686: W: no-documentation
stage.src: W: spelling-error Summary(en_US) multi -> mulch, mufti
stage.src: W: spelling-error %description -l en_US scalable -> salable, scalawag, scalar
6 packages and 1 specfiles checked; 0 errors, 8 warnings.

MUSTFIX: Building doesn't honor CFLAGS:
From build.log:
...
cd /builddir/build/BUILD/Stage-3.2.2-Source/libstage && /usr/lib64/ccache/c++   -Dstage_EXPORTS -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic
   -O3 -DNDEBUG -Wall   -fPIC -I/builddir/build/BUILD/Stage-3.2.2-Source/. -I/builddir/build/BUILD/Stage-3.2.2-Source/libstage -I/builddir/build/BUILD/Stage-3.2.2-Source/replace
 -I/usr/include/libpng12 -I/builddir/build/BUILD/Stage-3.2.2-Source   -I/usr/include/freetype2 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_THREAD_SAFE -D_REENTRANT 
-o CMakeFiles/stage.dir/block.o -c /builddir/build/BUILD/Stage-3.2.2-Source/libstage/block.cc

Note there is a -O3 overriding the -O2 from RPM_OPT_FLAGS.

I updated one of the patches to remove -O3 from the project's cflags.

Spec URL: http://www.richmattes.com/rpms/stage/stage.spec
SRPM URL: http://www.richmattes.com/rpms/stage/stage-3.2.2-6.fc12.src.rpm

koji build:
dist-f13 http://koji.fedoraproject.org/koji/taskinfo?taskID=2045239
dist-f12 http://koji.fedoraproject.org/koji/taskinfo?taskID=2045250

rpmlint:
$ rpmlint stage.spec ../RPMS/i686/stage* ../RPMS/noarch/stage* ../SRPMS/stage*
stage.i686: W: spelling-error Summary(en_US) multi -> mulch, mufti
stage.i686: W: spelling-error %description -l en_US scalable -> salable, scalawag, scalar
stage.i686: W: shared-lib-calls-exit /usr/lib/libstage.so.3.2.2 exit
stage-devel.i686: W: spelling-error %description -l en_US libstage -> lib stage, lib-stage, multistage
stage-devel.i686: W: no-documentation
stage-playerplugin.i686: W: no-documentation
stage.src: W: spelling-error Summary(en_US) multi -> mulch, mufti
stage.src: W: spelling-error %description -l en_US scalable -> salable, scalawag, scalar
6 packages and 1 specfiles checked; 0 errors, 8 warnings.

I think this package itself is in good shape, however as this
is needsponsor ticket, I will check another review request by you
(bug 530251) following:
http://fedoraproject.org/wiki/Extras/HowToGetSponsored

--------------------------------------------------------
  This package (stage) is APPROVED by mtasaka
--------------------------------------------------------

(In reply to comment #17)
> --------------------------------------------------------
>   This package (stage) is APPROVED by mtasaka

Not so hasty, please - This package still has several issues:

* stage.pc is improperly implemented:
It contains expanded references to libraries, instead of
Requires: to other libraries.

* The doc clearly are devel docs.
=> IMO, this *-doc package should be abandoned and be merged into the package it documents: "*-devel".

(In reply to comment #18)
 
> * stage.pc is improperly implemented:
> It contains expanded references to libraries, instead of
> Requires: to other libraries.

- Unfortunately some -devel packages which stage-devel depends
  on don't have .pc files.

> * The doc clearly are devel docs.
> => IMO, this *-doc package should be abandoned and be merged into the package
> it documents: "*-devel".    

- I don't think splitting such many document files (non man-3 files)
  into -doc is bad idea.

Still approved.

New Package CVS Request
=======================
Package Name: stage
Short Description: A 2.5D multi-robot simulator
Owners: rmattes
Branches: F-11 F-12 F-13
InitialCC: timn makghosh

(In reply to comment #19)
> (In reply to comment #18)
> 
> > * stage.pc is improperly implemented:
> > It contains expanded references to libraries, instead of
> > Requires: to other libraries.
> 
> - Unfortunately some -devel packages which stage-devel depends
>   on don't have .pc files.
And? Add appropriate Requires: where applicable.

In it's present shape this package's *.pc's are simply plain defective.


> > * The doc clearly are devel docs.
> > => IMO, this *-doc package should be abandoned and be merged into the package
> > it documents: "*-devel".    
> 
> - I don't think splitting such many document files (non man-3 files)
>   into -doc is bad idea.    
I disagree - The number one rule wrt. to docs in Fedora is to put docs into those packages they are documenting. doxygen generated developer docs clearly belong into *-devel.

(In reply to comment #22)
> (In reply to comment #19)
> > (In reply to comment #18)
> > 
> > > * stage.pc is improperly implemented:
> > > It contains expanded references to libraries, instead of
> > > Requires: to other libraries.
> > 
> > - Unfortunately some -devel packages which stage-devel depends
> >   on don't have .pc files.
> And? Add appropriate Requires: where applicable.
> 
> In it's present shape this package's *.pc's are simply plain defective

- Then in other words, almost not applicable for this package and of
  little use to stick to it (stage.pc is still usable)


> > > * The doc clearly are devel docs.
> > > => IMO, this *-doc package should be abandoned and be merged into the package
> > > it documents: "*-devel".    
> > 
> > - I don't think splitting such many document files (non man-3 files)
> >   into -doc is bad idea.    
> I disagree - The number one rule wrt. to docs in Fedora is to put docs into
> those packages they are documenting. doxygen generated developer docs clearly
> belong into *-devel. 

- So it is not a blocker.

Still approved, cvs admin, please take a look at request on
comment 21.

(In reply to comment #23)

> > And? Add appropriate Requires: where applicable.
> > 
> > In it's present shape this package's *.pc's are simply plain defective
> 
> - Then in other words, almost not applicable for this package
Rubbish ... All that needs to be done would be to replace those hardcoded libs from stage.pc this package erroniously pulls in with corresponding "Requires:".


> > > > * The doc clearly are devel docs.
> > > > => IMO, this *-doc package should be abandoned and be merged into the package
> > > > it documents: "*-devel".    
> > > 
> > > - I don't think splitting such many document files (non man-3 files)
> > >   into -doc is bad idea.    
> > I disagree - The number one rule wrt. to docs in Fedora is to put docs into
> > those packages they are documenting. doxygen generated developer docs clearly
> > belong into *-devel. 
> 
> - So it is not a blocker.
Yes ... nevertheless this package is badly packaged, IMNSHO due to its reviewer having done a poor job.

I will file bugs against this package as soon as it is available in bugzilla and (provided FESCO's ongoing absurdidities) will cast negative bodji votes.

Okay, discussion ended.

Ralf, it's amazing how hard you try to make Fedora an unpleasant place for packagers.

Mamoru, I agree, in particular to the doc subpackage. boost-doc, gtkmm24-docs, qt-doc are precedents. For example for gtkmm the generated documentation is also split. Since doxygen documentation can become quite large, it's often useful to have a separate package for it. As noarch package it can save on the mirrors, and it can save space and download time on machines where you need stage as a devel dependency, but are not actively developing against it.
For the pkgconfig files: they are working. Yes, requires would be a nicer option, and filing a bug to add them after the package is added is fine. But I take this with the "worse is better" mentality: it's better to have the package in the repository, especially since this is working and just not perfect, and then improve it. But not reason for the unnecessary fuss about minor issues!

(In reply to comment #26)
> Ralf, it's amazing how hard you try to make Fedora an unpleasant place for
> packagers.

Well,
* It's amazing how low Fedora's packaging QA is.

* It's amazing to watch people resorting to rude and hostile attacks, when they don't understand their mistakes.


a) This package's *.pc are simply plain broken.

> Mamoru, I agree, in particular to the doc subpackage. boost-doc, gtkmm24-docs,
> qt-doc are precedents.

b) The are 100s of counter-examples (Not splitting out *-doc packages is the norm!)

* In general *-doc packages don't make any sense on rpm based systems.
It's a Debianism often inherited from Deb packages, because Debian's packaging doesn't have a counter part to rpm -U --excludedocs.
* Exceptions are those cases a package contains "huge documents" where spitting them out into a separate noarch-package would provide "big savings" to a repository.

Finally: MUSTFIX:

c) This *-docs particular package is mere devel docs,
=> Requires: by *-devel.

a) again not broken
b) and also other pkgs on Fedora in this way
c) -devel does not require this -doc strictly

-> No blockers.

CVS done (by process-cvs-requests.py).

I created the -doc subpackage because the guidelines mention that if you have a large amount of documentation, you may want to consider splitting it all out.  Stage generates about 5MB of documentation, which is large compared to the 300k devel package, but it's also not like it's 100's of MB.  I'm not against the idea of merging -doc with -devel.

The pkg-config Libs line is generated using FLTK.  FLTK doesn't come with a .pc file, it comes with its own fltk-config program.  The build system runs fltk-config with all of the necessary settings, and collects the libraries necessary to link libstage.  It then passes these libraries to its pkg-config generator script.  I took a look, I'll only be able to get rid of -lGL, -lGLU, and -lpng by adding "Requires: glu libpng".  There's no pretty way to do this though, I have to go in and delete each offending library individually, and manually add "glu" and "libpng".  The resulting patch is not something I'd ever submit upstream, as i think letting fltk-config handle things is a better approach in this case, but it can be done.

(In reply to comment #30)
> I created the -doc subpackage because the guidelines mention that if you have a
> large amount of documentation, you may want to consider splitting it all out. 
> Stage generates about 5MB of documentation, which is large compared to the 300k
> devel package, but it's also not like it's 100's of MB.
Correct, whether to ship a separate *-doc package needs to balance the trade-offs. 

The intention of this section of the FPG had been
a) to spare disk-space on yum repos, because noarch'ed *-doc packages can be hardlinked within the repos on yum servers.

b) to enable packagers to ship different/older versions of *-docs to avoid package updates, in cases a binary package update only slightly changed the package. This typically is helpful in cases, upstreams ship separate documentation tarballs. It's hardly applicable to doxygen generated *-devel docs.

c) to support cases, package suites ship documentation suites/sets which depend on each other but don't necessarily need the corresponding binaries installed.
(I am not aware of any such use case in Fedora).

Apart of these consideration, in general, shipping separate *-doc packages doesn't make much difference to end-users. To the contrary, in many cases it has negative side-effects.

E.g. in your case, the devel docs end up in /usr/share/doc/stage-doc-* and not in /usr/share/doc/stage-devel-* as users normally expect.
Also, your *-doc package ends up as "an empty installed package" when a user installs it by rpm --exclude-docs.


>  I'm not against the idea of merging -doc with -devel.
The bug in your packaging is your *-doc package containing devel docs only, 
but the devel package not pulling-in its accompanying documentation - This is clearly a bug, and not helpful to end-users.

2 potential approaches to this issues would be eiher to let *-devel "Require: *-doc" or to merge the current *-doc package into *-devel.

> The pkg-config Libs line is generated using FLTK.  FLTK doesn't come with a .pc
> file, it comes with its own fltk-config program.  The build system runs
> fltk-config with all of the necessary settings, and collects the libraries
> necessary to link libstage.  It then passes these libraries to its pkg-config
> generator script.
That's the bug. Instead of adding these libraries to *.pc it should add Requires: <pkg> for those packages it finds a *.pc for.

P.S: Now that this package has been pushed into CVS, I'll BZ these two issues against this package.

(In reply to comment #31)
> P.S: Now that this package has been pushed into CVS, I'll BZ these two issues
> against this package.    

By the way:

To spot:
It seems that all new pkgs you processed around 2010-03-15 UTC 21:00-22:00
(including this package) still don't have the entries on redhat bugzilla.
Would you examine what are happening on these pkgs?

Anyway closing.

In the packagedb update on Monday, the bugzilla sync script wasn't updated.  Now fixed.

Thanks.