Bug 468517

Last time I checked it build fine in el5 mock an RPMlint was silent.

Created attachment 321519 [details]
Comment for Patch2

Some notes for 1.35.1-1:

* License
  - Well as far as I checked the whole code I only find codes
    under
    * license written in "COPYING" (Copyright only)
    * LGPLv2+ (e.g. fitsfile.c)
    So the license tag should be "LGPLv2+".
    Also add some files to %doc which indicates LGPL license.

* Source origin
  - Would you write where you obtained %SOURCE3?

* About saoimage-1.35.1-mkstemp.patch
  - See attached.

* BuildRequires
  - In your srpm "BuildRequires: desktop-file-utils" is missing
    (while the spec file on the URL you posted here has it)

* %install script
  - What is the line below for?
-------------------------------------------------------------
install -d $RPM_BUILD_ROOT/dev
-------------------------------------------------------------

* XML file
  - I guess this should be installed under %_datadir/mime/packages

* Directory ownership issue
  - The directory %_datadir/mime must not be owned by this
    package.


By the way:
[tasaka1@localhost saoimage]$ gdb saoimage
GNU gdb Fedora (6.8-15.fc10)
Copyright (C) 2008 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "i386-redhat-linux-gnu"...
(gdb) run
Starting program: /usr/bin/saoimage 
open error: No such file or directory
ERROR: unable to open /dev/imt1o
Error: No remote input possible.

 Smithsonian Astrophysical Observatory image utility
 SAOimage 1.35.1 1 December 2003, Doug Mink, dmink.edu

Reserved 512 x 512 image data buffer.

Program received signal SIGSEGV, Segmentation fault.
0x0807bf89 in init_window_basics (border_pixel=0) at wndwinit.c:201
201       (void)XDestroyImage(ximage);
Missing separate debuginfos, use: debuginfo-install libXau.i386 libXcursor.i386 libXdmcp.i386 libXfixes.i386 libXrender.i386
(gdb) thread apply all bt
(gdb) bt
#0  0x0807bf89 in init_window_basics (border_pixel=0) at wndwinit.c:201
#1  0x0807bfd1 in init_windows2 () at wndwinit.c:97
#2  0x0806bded in init_packages (argc=1, argv=0xbffff3e4) at maininit.c:206
#3  0x0806bf44 in main (argc=1, argv=0xbffff3e4) at maininit.c:118
(gdb) qui
The program is running.  Exit anyway? (y or n) y
[tasaka1@localhost saoimage]$ exit

By the way it may be good to check implicit function declaration
by adding -Werror-implicit-function-declaration:

http://www.redhat.com/archives/rhl-devel-list/2008-March/msg02036.html

Thanks for the comments Mamoru!

(In reply to comment #2)
> Created an attachment (id=321519) [details]
> Comment for Patch2

Good catch. Will fix.

> Some notes for 1.35.1-1:
> 
> * License
>   - Well as far as I checked the whole code I only find codes
>     under
>     * license written in "COPYING" (Copyright only)
>     * LGPLv2+ (e.g. fitsfile.c)
>     So the license tag should be "LGPLv2+".
>     Also add some files to %doc which indicates LGPL license.

Thanks, you seem to be right. Will be fixed in next revision.

> * Source origin
>   - Would you write where you obtained %SOURCE3?

I stole it :)
Actually -- the logo is present in the source code (being the default picture to display). I grabbed this one from some random site and converted from gif, assuming it's no problem to use it given it's already included in the source code anyways.

> * BuildRequires
>   - In your srpm "BuildRequires: desktop-file-utils" is missing
>     (while the spec file on the URL you posted here has it)

I must have uploaded some older version by mistake. I will ensure it is present in next revision.

> * %install script
>   - What is the line below for?
> -------------------------------------------------------------
> install -d $RPM_BUILD_ROOT/dev

Will remove it. It was used in on of earlier revisions, since the make install creates some links in /dev by default.

> * XML file
>   - I guess this should be installed under %_datadir/mime/packages

Right. Thanks for noticing that.

> * Directory ownership issue
>   - The directory %_datadir/mime must not be owned by this
>     package.

Why? It does not include shared-mime-info or any other package that would require it in its dependency chain.

> By the way:
> open error: No such file or directory
> ERROR: unable to open /dev/imt1o
> Error: No remote input possible.

This is OK since we do not ship IRAF.

> Program received signal SIGSEGV, Segmentation fault.

This does not happen in my el5. Could you please tell me which Fedora version do you use? Thanks!

(In reply to comment #3)
> By the way it may be good to check implicit function declaration
> by adding -Werror-implicit-function-declaration:

This sounds worthwhile. Due to the big amount of functions without prototypes (there's at least one in vast majority of files) this would require substantial effort. For now I included headers for functions covered by FORTIFY_SOURCE where they were missing and I'll add prototypes for the rest if this becomes mandatory part of packaging guidelines. (I encourage you to advocate for that)

New package:

SPEC: http://netbsd.sk/~lkundrak/SPECS/saoimage.spec
SRPM: http://netbsd.sk/~lkundrak/SRPMS/saoimage-1.35.1-2.el5.src.rpm

(In reply to comment #4)
> Thanks for the comments Mamoru!
> > * Source origin
> >   - Would you write where you obtained %SOURCE3?
> 
> I stole it :)
> Actually -- the logo is present in the source code (being the default picture
> to display). I grabbed this one from some random site and converted from gif,
> assuming it's no problem to use it given it's already included in the source
> code anyways.

Okay, I trust you then.

> > * Directory ownership issue
> >   - The directory %_datadir/mime must not be owned by this
> >     package.
> 
> Why? It does not include shared-mime-info or any other package that would
> require it in its dependency chain.

Actually currently only shared-mime-info package  owns this directory
on my system. The same discussion can also be applied to
%_datadir/icons/hicolor.

> > Program received signal SIGSEGV, Segmentation fault.
> 
> This does not happen in my el5. Could you please tell me which Fedora version
> do you use? Thanks!

I use F-10 i386. segv still occurs with -2 srpm.

(In reply to comment #6)
> (In reply to comment #4)

> > > * Directory ownership issue
> > >   - The directory %_datadir/mime must not be owned by this
> > >     package.
> > 
> > Why? It does not include shared-mime-info or any other package that would
> > require it in its dependency chain.
> 
> Actually currently only shared-mime-info package  owns this directory
> on my system. The same discussion can also be applied to
> %_datadir/icons/hicolor.

I quite don't understand if you still insist on not owning the package. In case yes, I don't see what would we gain here, except for having a slight possibility of the package leaving a stale unowned directory.

> > > Program received signal SIGSEGV, Segmentation fault.
> > 
> > This does not happen in my el5. Could you please tell me which Fedora version
> > do you use? Thanks!
> 
> I use F-10 i386. segv still occurs with -2 srpm.

Thanks. I reproduced it in a mock chroot and think I have fixed it in new package:

SPEC: http://netbsd.sk/~lkundrak/SPECS/saoimage.spec
SRPM: http://netbsd.sk/~lkundrak/SRPMS/saoimage-1.35.1-3.el5.src.rpm

(In reply to comment #7)
> I quite don't understand if you still insist on not owning the package. In case
> yes, I don't see what would we gain here, except for having a slight
> possibility of the package leaving a stale unowned directory.

* Please be consistent with other packages
  - Currently only 1 (binary) package (shared-mime-info) owns /usr/share/mime
  - While 81 packages have files under /usr/share/mime/packages (including shared-mime-info)

If you want to make it sure that /usr/share/mime is owned by some packages
when shared-mime-info is not installed (current packaging guidelines is against
this), this is for packaging guidelines issue (you can suggest that this 
directory must be in filesystem package or so).

ref:
  - 18 packages (??) own /usr/share/icons/hicolor
  - While 637 packages have files under /usr/share/icons/hicolor/*/apps

* And I don't think that XML files under the directory are useful
  when shared-mime-info is not installed.

> > > > Program received signal SIGSEGV, Segmentation fault.
I don't see this SEGV on -3, thanks.

Thank you for the package.

The package looks sane, please take into consideration Mamoru's comment and fix small cosmetic issue around desktop file and this package is APPROVED.

  ./saoimage.desktop: warning: key "Encoding" in group "Desktop Entry" is deprecated

(In reply to comment #9)
> Thank you for the package.
> 
> The package looks sane, please take into consideration Mamoru's comment and fix
> small cosmetic issue around desktop file and this package is APPROVED.

Will do. Thanks.

>   ./saoimage.desktop: warning: key "Encoding" in group "Desktop Entry" is
> deprecated

Encoding used to be mandatory, and including it is not an error yet. Will exclude it from recent Fedora branches and devel.

New Package CVS Request
=======================
Package Name: saoimage
Short Description: Utility for displaying astronomical images
Owners: lkundrak
Branches: EL-5 F-9 F-10

CVS Done

Thanks a lot Ma{moru,rek}, Dennis.
Imported and build.