Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.

Bug 234122

Summary: Review Request: pygtkglext - Python bindings for GtkGLExt
Product: [Fedora] Fedora Reporter: Hans de Goede <hdegoede>
Component: Package ReviewAssignee: Mamoru TASAKA <mtasaka>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideFlags: mtasaka: fedora-review+
wtogami: fedora-cvs+
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2007-04-02 16:25:15 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Description Hans de Goede 2007-03-27 08:09:03 UTC
Spec URL: http://people.atrpms.net/~hdegoede/pygtkglext.spec
SRPM URL: http://people.atrpms.net/~hdegoede/pygtkglext-1.1.0-1.fc7.src.rpm
Description:
Python bindings for GtkGLExt

Comment 1 Mamoru TASAKA 2007-03-29 08:32:20 UTC
I will review this after I return to home.

Comment 2 Mamoru TASAKA 2007-03-29 18:57:40 UTC
Well, for 1.1.0-1

* %makeinstall
-------------------------------------
# Note: make install PREFIX=$RPM_BUILD_ROOT doesnot work :(
-------------------------------------
  - Perhaps you want to use the following?
-------------------------------------
make install DESTDIR=$RPM_BUILD_ROOT INSTALL="%{__install} -p"
-------------------------------------
    The above works for me. And INSTALL option should be
    added to keep timestamps on python scripts installed.

* Documentation
-------------------------------------
sed -i 's/\r//g' README.win32
-------------------------------------
  - Simply remove this line. README._win32_ should
    not be for linux.

* %files
  - By the way, why does file entry has the same line?
    (perhaps you just did copy & paste)

* libtool archives
  - Perhaps .la files should be removed.

* Executable permission vs shebang
-------------------------------------
chmod +x $RPM_BUILD_ROOT%{python_sitearch}/gtk-2.0/gtk/gtkgl/apputils.py
-------------------------------------
  - Again please check if this is correct

* Requires
-------------------------------------
Requires:       gtkglext
-------------------------------------
  - Is this needed? Libraries' dependency should automatically
    pull this.

* python_sitelib vs python_sitearch
-------------------------------------
if [ %{python_sitelib} != %{python_sitearch} ]; then
<skip>
fi
-------------------------------------
  - While this should work, IMO it is better to configure
    configure says:
-------------------------------------
 19151  if test "${am_cv_python_pythondir+set}" = set; then
 19152    echo $ECHO_N "(cached) $ECHO_C" >&6
 19153  else
 19154    am_cv_python_pythondir=`$PYTHON -c "from distutils import sysconfig;
print sysconfig.get_python_lib(0,0,prefix='$PYTHON_PREFIX')" 2>/dev/null ||
 19155       echo "$PYTHON_PREFIX/lib/python$PYTHON_VERSION/site-packages"`
 19156  fi
-------------------------------------
    From my impression, this should be okay with changing to
    get_python_lib(1,0,prefix='$PYTHON_PREFIX')

* License
  - Well, the included files are GPL and LGPL, and actually this
    is licensed under LGPL (also declared on
    http://www.k-3d.org/gtkglext/Main_Page#Licensing )

* Encoding
  - The following documents are not encoded with UTF-8. Please
    fix.
--------------------------------------
<file>          <encoded by>
AUTHORS         EUC-JP
README          EUC-JP
(README.win32   SJIS)
--------------------------------------
    (Just a note: usually Japanese documents are not encoded
     with UTF-8 and normally encoded with either EUC-JP, ISO-2022-JP
     <both are for UNIX file> or SJIS <for Windows file>)

Comment 3 Hans de Goede 2007-03-29 20:59:25 UTC
(In reply to comment #2)
> * %files
>   - By the way, why does file entry has the same line?
>     (perhaps you just did copy & paste)
> 
Those lines are not identical (gdkgl versus gtkgl)

> * Executable permission vs shebang
> -------------------------------------
> chmod +x $RPM_BUILD_ROOT%{python_sitearch}/gtk-2.0/gtk/gtkgl/apputils.py
> -------------------------------------
>   - Again please check if this is correct
> 

Actually this file contains a main with test routines and thus can be executed
stand alone, I've added a comment about this.

> * python_sitelib vs python_sitearch
> -------------------------------------
> if [ %{python_sitelib} != %{python_sitearch} ]; then
> <skip>
> fi
> -------------------------------------
>   - While this should work, IMO it is better to configure
>     configure says:
> -------------------------------------
>  19151  if test "${am_cv_python_pythondir+set}" = set; then
>  19152    echo $ECHO_N "(cached) $ECHO_C" >&6
>  19153  else
>  19154    am_cv_python_pythondir=`$PYTHON -c "from distutils import sysconfig;
> print sysconfig.get_python_lib(0,0,prefix='$PYTHON_PREFIX')" 2>/dev/null ||
>  19155       echo "$PYTHON_PREFIX/lib/python$PYTHON_VERSION/site-packages"`
>  19156  fi
> -------------------------------------
>     From my impression, this should be okay with changing to
>     get_python_lib(1,0,prefix='$PYTHON_PREFIX')
> 

Erm the above solution gives me a headache, and as said my own (simple,
understandable) solution works fine.

All other items fixed, here is a new version:
Spec URL: http://people.atrpms.net/~hdegoede/pygtkglext.spec
SRPM URL: http://people.atrpms.net/~hdegoede/pygtkglext-1.1.0-2.fc7.src.rpm


Comment 4 Mamoru TASAKA 2007-03-30 16:55:23 UTC
Okay.

--------------------------------------------
  This package (pygtkglext) is APPROVED by me
--------------------------------------------

(In reply to comment #3)
> (In reply to comment #2)
> > * %files
> >   - By the way, why does file entry has the same line?
> >     (perhaps you just did copy & paste)
> > 
> Those lines are not identical (gdkgl versus gtkgl)

Oops.....

Comment 5 Hans de Goede 2007-03-30 18:28:13 UTC
New Package CVS Request
=======================
Package Name:      pygtkglext
Short Description: Python bindings for GtkGLExt
Owners:            j.w.r.degoede@hhs.nl
Branches:          FC-6 devel
InitialCC:         <empty>


Comment 6 Hans de Goede 2007-04-02 06:55:55 UTC
Many thanks for the review!

Imported and build, closing.


Comment 7 Mamoru TASAKA 2007-04-02 16:25:15 UTC
(Actually closing)