Bug 229676

Assigning to me.

Well, for gle-4.0.12-2:

* Encoding
  - Well, you tried to change LICENSE.txt to UNIX format by dos2unix,
    however, it is not enough.
---------------------------------------------------
[tasaka1@localhost gle]$ file /usr/share/doc/gle-4.0.12/LICENSE.txt
/usr/share/doc/gle-4.0.12/LICENSE.txt: data
---------------------------------------------------
    Well, emacs with hexl-mode shows that this document has
    a strange '0x13' code (note: Windows CR code is '0x0d').
    Please remove both.

* Conditional dependency
  - Mockbuild log says:
---------------------------------------------------
checking for XOpenDisplay in -lX11... no
.....
   ** X11 preview support: no
---------------------------------------------------
    Please fix configure (NOTE: not configure.in but
    configure. Please don't use autotool as much as
    possible) (around the line 2709).

* Timestamps
  - This package tries to install some files from original
    zip file without any modification, such as
---------------------------------------------------
./src/manip/manip.hlp
./src/fonts/font.dat
./src/fonts/psfont.dat
./src/lib/*.gle
./src/TeX/init.tex
---------------------------------------------------
   , and keeping timestamps on these files are recommended.
   Please fix the line of build log such as
---------------------------------------------------
mkdir -p ../../build/font
cp *.fve *.fmt ../../build/font
cp font.dat ../../build/font
cp psfont.dat ../../build/font
touch movefonts
make[1]: Leaving directory `/builddir/build/BUILD/gle4/src/fonts'
make -C src/lib -f Makefile.gcc
make[1]: Entering directory `/builddir/build/BUILD/gle4/src/lib'
mkdir -p ../../build/lib
cp *.gle ../../build/lib/
make[1]: Leaving directory `/builddir/build/BUILD/gle4/src/lib'
make -C src/TeX
make[1]: Entering directory `/builddir/build/BUILD/gle4/src/TeX'
cp init.tex ../../build/
../../build/bin/gle -mkinittex
---------------------------------------------------
   to:
---------------------------------------------------
cp -p *.fve *.fmt ../../build/font
cp -p font.dat ../../build/font
cp -p psfont.dat ../../build/font
.....
---------------------------------------------------
  for example.

* rpmlint
  - not silent.
---------------------------------------------------
E: gle-debuginfo script-without-shebang /usr/src/debug/gle4/src/manip/unix_extra.cpp
W: gle-debuginfo spurious-executable-perm
/usr/src/debug/gle4/src/gle/tokens/CharBitMap.h
W: gle-debuginfo spurious-executable-perm
/usr/src/debug/gle4/src/gle/surface/gsurface.h
W: gle-debuginfo spurious-executable-perm /usr/src/debug/gle4/src/gle/surface/f2c.h
E: gle-debuginfo script-without-shebang /usr/src/debug/gle4/src/gle/letzfitz/let.cpp
W: gle-debuginfo spurious-executable-perm /usr/src/debug/gle4/src/manip/ffblk_def.h
E: gle-debuginfo script-without-shebang
/usr/src/debug/gle4/src/gle/tokens/Tokenizer.cpp
W: gle-debuginfo spurious-executable-perm
/usr/src/debug/gle4/src/gle/tokens/Tokenizer.h
E: gle-debuginfo script-without-shebang
/usr/src/debug/gle4/src/gle/surface/fcontour.cpp
E: gle-debuginfo script-without-shebang
/usr/src/debug/gle4/src/gle/surface/gsurface.cpp
E: gle-debuginfo script-without-shebang
/usr/src/debug/gle4/src/gle/surface/ffitcontour.cpp
E: gle-debuginfo script-without-shebang /usr/src/debug/gle4/src/manip/unixscr.cpp
E: gle-debuginfo script-without-shebang
/usr/src/debug/gle4/src/gle/surface/vdevice.cpp
E: gle-debuginfo script-without-shebang /usr/src/debug/gle4/src/gle/surface/hide.cpp
E: gle-debuginfo script-without-shebang /usr/src/debug/gle4/src/gle/tokens/BinIO.cpp
W: gle-debuginfo spurious-executable-perm
/usr/src/debug/gle4/src/gle/tokens/StringKeyHash.h
W: gle-debuginfo spurious-executable-perm /usr/src/debug/gle4/src/gle/tokens/BinIO.h
E: gle-debuginfo script-without-shebang
/usr/src/debug/gle4/src/gle/tokens/StringKeyHash.cpp
E: gle-debuginfo script-without-shebang
/usr/src/debug/gle4/src/gle/surface/gcontour.cpp
E: gle-debuginfo script-without-shebang
/usr/src/debug/gle4/src/gle/tokens/Tokenizer.i
E: gle-debuginfo script-without-shebang /usr/src/debug/gle4/src/manip/unixinkey.cpp
E: gle-debuginfo script-without-shebang /usr/src/debug/gle4/src/manip/varargs.cpp
-----------------------------------------------------
    - Perhaps all permission issue. Please fix the permission to 0644.

* License
  - Some files are not licensed under BSD.
    A. GPL
------------------------------------------------------
./src/gui/about.*
./src/gui/arc.*
( and many other files under ./src/gui )
------------------------------------------------------
   B. assumely GPL (however, would you contact upstream?)
------------------------------------------------------
./src/gui/gsinc/gdevdsp.h
( and some other files under ./src/gui/gsinc )
------------------------------------------------------

For sourceURL, please check:
http://fedoraproject.org/wiki/Packaging/SourceURL

> * Encoding
>   - Well, you tried to change LICENSE.txt to UNIX format by dos2unix,
>     however, it is not enough.

Fixed.

> * Conditional dependency
>   - Mockbuild log says:
> ---------------------------------------------------
> checking for XOpenDisplay in -lX11... no
> .....
>    ** X11 preview support: no
> ---------------------------------------------------
>     Please fix configure (NOTE: not configure.in but
>     configure. Please don't use autotool as much as
>     possible) (around the line 2709).

Unable to reproduce, qt4-devel should bring in all needed X11 libs?
 
> * Timestamps
>   - This package tries to install some files from original
>     zip file without any modification, such as

Fixed.


> * rpmlint
>   - not silent.

Fixed.


> * License
>   - Some files are not licensed under BSD.
>     A. GPL
> ------------------------------------------------------
> ./src/gui/about.*
> ./src/gui/arc.*
> ( and many other files under ./src/gui )
> ------------------------------------------------------
>    B. assumely GPL (however, would you contact upstream?)
> ------------------------------------------------------
> ./src/gui/gsinc/gdevdsp.h
> ( and some other files under ./src/gui/gsinc )
> ------------------------------------------------------

Fixed by creating subpackage for gui with GPL license, ok?

 
> For sourceURL, please check:
> http://fedoraproject.org/wiki/Packaging/SourceURL

Fixed.

Updated spec:  
  http://web.phys.ntnu.no/~terjeros/gle/gle.spec
srpm:
  http://web.phys.ntnu.no/~terjeros/gle/gle-4.0.12-3.fc6.src.rpm

Created attachment 149830 [details]
mock build log of gle-4.0.12-3 on FC-devel i386

Mock build log of gle-4.0.12-3 on FC-7 devel i386 shows:
* Timestamps
  - Still some "cp" command does not keep timestamps
    Perhaps in some cases you have to make a patch against
    Makefile.in, not Makefile

* Conditional dependency
  - Build log still says:
--------------------------------------
   ** X11 preview support: no
--------------------------------------

* For license:
  - Well, GPL part splitting seems okay, however, I still wonder
    if the license of ./src/gui/gsinc/gdevdsp.h and so on
    is okay.
--------------------------------------
     1	/* Copyright (C) 2001-2004, Ghostgum Software Pty Ltd.	All rights
reserved.
     2
     3	   This software is provided AS-IS with no warranty, either express or
     4	   implied.
     5
     6	   This software is distributed under license and may not be copied,
     7	   modified or distributed except as expressly authorized under the
terms
     8	   of the license contained in the file LICENSE in this distribution.
     9
    10	   For more information about licensing, please refer to
    11	   http://www.ghostscript.com/licensing/. For information on
    12	   commercial licensing, go to http://www.artifex.com/licensing/ or
    13	   contact Artifex Software, Inc., 101 Lucas Valley Road #110,
    14	   San Rafael, CA  94903, U.S.A., +1(415)492-9861.
    15	 */
--------------------------------------
    --- Does this refer to GPL? Would you contact upstream?

Created attachment 149831 [details]
config.log on 4.0.12-3

config.log says:
-------------------------------
   235	configure:2478: checking for X
   236	configure:2584: g++ -E	conftest.cc
   237	configure:2590: $? = 0
   238	configure:2640: g++ -o conftest -O2 -g -pipe -Wall
-Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=
ssp-buffer-size=4 -m32 -march=i386 -mtune=generic -fasynchronous-unwind-tables 
 conftest.cc -lXt  >&5
   239	configure:2643: $? = 0
   240	configure:2646: test -s conftest
   241	configure:2649: $? = 0
   242	configure:2698: result: libraries , headers 
   243	configure:2703: checking for XOpenDisplay in -lX11
   244	configure:2734: g++ -o conftest -O2 -g -pipe -Wall
-Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=
ssp-buffer-size=4 -m32 -march=i386 -mtune=generic -fasynchronous-unwind-tables 
 conftest.cc -lX11 -lXt -L  >&5
   245	g++: argument to '-L' missing
   246	
   247	configure:2737: $? = 1
   248	configure: failed program was:
-----------------------------------
   The part of configure corresponding to this error is:
-----------------------------------
  2702	if test "$with_x" != no; then
  2703	    echo "$as_me:$LINENO: checking for XOpenDisplay in -lX11" >&5
  2704	echo $ECHO_N "checking for XOpenDisplay in -lX11... $ECHO_C" >&6
  2705	if test "${ac_cv_lib_X11_XOpenDisplay+set}" = set; then
  2706	  echo $ECHO_N "(cached) $ECHO_C" >&6
  2707	else
  2708	  ac_check_lib_save_LIBS=$LIBS
  2709	LIBS="-lX11 -lXt -L$x_libraries $LIBS"
  2710	cat >conftest.$ac_ext <<_ACEOF
  2711	#line $LINENO "configure"
  2712	/* confdefs.h.	*/
  2713	_ACEOF
  2714	cat confdefs.h >>conftest.$ac_ext
  2715	cat >>conftest.$ac_ext <<_ACEOF
  2716	/* end confdefs.h.  */
  2717	
  2718	/* Override any gcc2 internal prototype to avoid an error.  */
  2719	#ifdef __cplusplus
  2720	extern "C"
  2721	#endif
  2722	/* We use char because int might match the return type of a gcc2
  2723	   builtin and then its argument prototype would still apply.  */
  2724	char XOpenDisplay ();
  2725	int
  2726	main ()
  2727	{
  2728	XOpenDisplay ();
  2729	  ;
  2730	  return 0;
  2731	}
  2732	_ACEOF
  2733	rm -f conftest.$ac_objext conftest$ac_exeext
  2734	if { (eval echo "$as_me:$LINENO: \"$ac_link\"") >&5
  2735	  (eval $ac_link) 2>&5
  2736	  ac_status=$?
  2737	  echo "$as_me:$LINENO: \$? = $ac_status" >&5
  2738	  (exit $ac_status); } &&
  2739		 { ac_try='test -s conftest$ac_exeext'
------------------------------------

> * Timestamps
>   - Still some "cp" command does not keep timestamps
>     Perhaps in some cases you have to make a patch against
>     Makefile.in, not Makefile

This seems a bit off for me, most of these files are generated during 
build. I am dropping the preserve timestamps patch for now,
I can't see this as a blocker. Where is this policy coming from anyway?

> * For license

I will send a email upstream.

> * Conditional dependency
>   - Build log still says:
> --------------------------------------
>    ** X11 preview support: no
>
> config.log says:
>-------------------------------
>   235	configure:2478: checking for X
>   245	g++: argument to '-L' missing

Seems like configure want explicit x flags, I tried to fix by adding:

 --x-includes=%{_includedir}/X11 --x-libraries=%{_libdir}

Better now?

Spec:
  http://web.phys.ntnu.no/~terjeros/gle/gle.spec
SRPM:
  http://web.phys.ntnu.no/~terjeros/gle/gle-4.0.12-3.fc6.src.rpm

>This seems a bit off for me, most of these files are generated during 
>build. I am dropping the preserve timestamps patch for now,
>I can't see this as a blocker. Where is this policy coming from anyway?

http://fedoraproject.org/wiki/Packaging/Guidelines#head-0239576e441f9ef53d175c4aec8c12868dffb5ab

>http://fedoraproject.org/wiki/Packaging/Guidelines#head-0239576e441f9ef53d175c4aec8c12868dffb5ab

 When adding file copying commands in the spec file, consider using a command  
 that preserves the files' timestamps, eg. cp -p or install -p.

 When downloading sources, patches etc, consider using a client that preserves     
 the upstream timestamps. For example wget -N or curl -R. To make the change 
 global for wget, add this to your ~/.wgetrc: timestamping = on, and for curl, 
 add to your ~/.curlrc: -R.

As far as I can see this don't cover the issue discussed here. 

Well, the reason why the guideline refers to keeping timestamps
is that keeping timestamps on the installed files is recommended
because
* it shows when the files are actually created.
* it shows if the vendor or something else (like you) have
  modified the files.  

  Changing timestamps even if the content of the files are not
  modified are confusing, unless there is a special reason
  it should be done. For example, the installed file
  "./src/TeX/init.tex" are not modified since "2006-09-01" and
  there is no reason to change the timestamp of this file
  every time you upgrade or modify your spec/srpm.

It is important to show clearly as much as possible what you
changed and what you _didn't_ change each time you modify
spec file.
Keeping timestamps becomes more important when the package
contains
* many text files such as header files
* many image files
for example. In short, trying to keep timestamps is very important
IMO.

> I will send a email upstream.

Upstream maintainer Jan Struyf was very helpful and fixed a lot of problems and
release
a updated package.

Please have a look:

spec:
 http://web.phys.ntnu.no/~terjeros/gle/gle.spec
srpms:
 http://web.phys.ntnu.no/~terjeros/gle/gle-4.0.12a-1.fc6.src.rpm

Created attachment 150073 [details]
mock build log of gle-4.0.12a-1 on FC-devel i386

mockbuild failed on FC-devel i386.

NOTE: on rawhide qt4 is upgraded to 4.2.3-1, which may
relate to the failure.

(In reply to comment #12)
> NOTE: on rawhide qt4 is upgraded to 4.2.3-1, which may
> relate to the failure.

Perhaps this is due to qt4-4.2.3-1

One comment:
* Please change the license from BSD to GPL.

Other things are okay.
-------------------------------------------------
   This package (gle) is APPROVED by me.
-------------------------------------------------

>    This package (gle) is APPROVED by me.

Thanks for the review!

 - Terje

New Package CVS Request
=======================
Package Name: gle
Short Description: Graphics Layout Engine
Owners: terjeros.no
Branches: FC-6

New Package SCM Request
=======================
Package Name: gle
Short Description: Graphics Layout Engine
Upstream URL: http://www.gle-graphics.org/
Owners: jussilehtola
Branches: el5 el6
InitialCC:

Git done (by process-git-requests).

Crap, sorry, I messed up. Funny that the scripts didn't catch that.

Package Change Request
======================
Package Name: gle
New Branches: el5 el6 epel7
Owners: jussilehtola

Git done (by process-git-requests).

gle-4.2.4c-14.el5 has been submitted as an update for Fedora EPEL 5.
https://admin.fedoraproject.org/updates/gle-4.2.4c-14.el5

gle-4.2.4c-14.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/gle-4.2.4c-14.el6

gle-4.2.4c-14.el5 has been pushed to the Fedora EPEL 5 stable repository.

gle-4.2.4c-14.el6 has been pushed to the Fedora EPEL 6 stable repository.