Bug 234121

Would you discuss with bug 221031 and bug 225118 ?

(In reply to comment #1)
> Would you discuss with bug 221031 and bug 225118 ?

Woops, done. I'm leaving this open for now. I will close it once its clear with
which package we will move forward, and in which review bug that will get reviewed.

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

The other 2 reviews have both been closed by their respective reporters in favor
of this one. So anyone interested please review this.

Well, for 3.0.0-0.1.a6:

* Executable permission vs shebang
---------------------------------------------------
chmod +x $RPM_BUILD_ROOT%{python_sitelib}/OpenGL/Tk/__init__.py \
  $RPM_BUILD_ROOT%{python_sitelib}/OpenGL/tests/test_glgetfloat_leak.py \
  $RPM_BUILD_ROOT%{python_sitelib}/OpenGL/tests/tests.py \
  $RPM_BUILD_ROOT%{python_sitelib}/OpenGL/tests/test_glutinit_0args.py \
  $RPM_BUILD_ROOT%{python_sitelib}/OpenGL/tests/test_glut_function_predef.py
---------------------------------------------------
  - Well, please make it sure:
    * If these scripts are expected to be called from other python
      scripts and are not expected to be called "directly" by user
      (well, "directly" used herein may be ambiguous), then
      having shebang is wrong and these files should _not_ have
      executable permission.
    * If opposite, i.e. these scripts are meant to be called directly
      by user, then these scripts should have executable permission.

    Usually the case is former.

* Python related dependency
  - Well, would you explain why the Requires for this package is
    enough by the following?
---------------------------------------------------
Requires:       python-numeric python-setuptools
---------------------------------------------------

    I usually check python related dependency by following. For this
    package, it returns many dependency, such as
    wxPython tkinter python-imaging pygame ... 

---------------------------------------------------
$ grep 'import ' `rpm -ql PyOpenGL | grep py$` | sed -e 's|^.*:||' | sed -e
's|^[ \t][ \t]*||' | sort | uniq
---------------------------------------------------


----------------------------------------------------
  Again, I would appreciate it if you would review
  either of my review request (from bug 233423 to bug 233426), thanks.

(In reply to comment #5)
> Well, for 3.0.0-0.1.a6:
> 
> * Executable permission vs shebang
> ---------------------------------------------------
> chmod +x $RPM_BUILD_ROOT%{python_sitelib}/OpenGL/Tk/__init__.py \
>   $RPM_BUILD_ROOT%{python_sitelib}/OpenGL/tests/test_glgetfloat_leak.py \
>   $RPM_BUILD_ROOT%{python_sitelib}/OpenGL/tests/tests.py \
>   $RPM_BUILD_ROOT%{python_sitelib}/OpenGL/tests/test_glutinit_0args.py \
>   $RPM_BUILD_ROOT%{python_sitelib}/OpenGL/tests/test_glut_function_predef.py
> ---------------------------------------------------
>   - Well, please make it sure:
>     * If these scripts are expected to be called from other python
>       scripts and are not expected to be called "directly" by user
>       (well, "directly" used herein may be ambiguous), then
>       having shebang is wrong and these files should _not_ have
>       executable permission.
>     * If opposite, i.e. these scripts are meant to be called directly
>       by user, then these scripts should have executable permission.
> 
>     Usually the case is former.
> 

I will look into this.

> * Python related dependency
>   - Well, would you explain why the Requires for this package is
>     enough by the following?
> ---------------------------------------------------
> Requires:       python-numeric python-setuptools
> ---------------------------------------------------
> 
>     I usually check python related dependency by following. For this
>     package, it returns many dependency, such as
>     wxPython tkinter python-imaging pygame ... 
> 
> ---------------------------------------------------
> $ grep 'import ' `rpm -ql PyOpenGL | grep py$` | sed -e 's|^.*:||' | sed -e
> 's|^[ \t][ \t]*||' | sort | uniq
> ---------------------------------------------------
> 

Thanks, good tip, okay, so that gives me the following list of (non standard /
not already required) modules:
Tkinter (+ Dialog)
pygame
wxPython
Image

Correct?

Going through them one by one:
Image -> python-imaging, not needed, I've packaged up PyOpenGL for use with
  glchess, and that works fine without python-imaging, some texture loading 
  utility functions may need this, but I think its best to them make the 
  packages using those functions require python-imaging, to me PyOpenGL is an
  opengl wrapper and as such should require those bits which are absolutely 
  necessary todo that task.

pygame:
[hans@localhost glchess]$ grep -rl pygame /usr/lib/python2.5/site-packages/OpenGL
/usr/lib/python2.5/site-packages/OpenGL/tests/test_glgetfloat_leak.py
/usr/lib/python2.5/site-packages/OpenGL/tests/test_glgetfloat_leak.pyc
/usr/lib/python2.5/site-packages/OpenGL/tests/test_glgetfloat_leak.pyo
/usr/lib/python2.5/site-packages/OpenGL/tests/tests.py
/usr/lib/python2.5/site-packages/OpenGL/tests/testing_context.py
/usr/lib/python2.5/site-packages/OpenGL/tests/testing_context.pyc
/usr/lib/python2.5/site-packages/OpenGL/tests/testing_context.pyo
/usr/lib/python2.5/site-packages/OpenGL/tests/tests.pyc
/usr/lib/python2.5/site-packages/OpenGL/tests/tests.pyo

I've tried nuking the tests dir, but then using PyOpenGL fails with an import
error, so some bits are needed. Since pygame is only needed for tests I don't
want to require it.


tkinter:
[hans@localhost glchess]$ grep -irl Tkinter /usr/lib/python2.5/site-packages/OpenGL
/usr/lib/python2.5/site-packages/OpenGL/Tk/__init__.pyc
/usr/lib/python2.5/site-packages/OpenGL/Tk/__init__.py
/usr/lib/python2.5/site-packages/OpenGL/Tk/__init__.pyo

IOW only needed by applications which actually want to use the TK integration,
those should require tkinter themselves


wxpython:
[hans@localhost glchess]$ grep -irl wxPython /usr/lib/python2.5/site-packages/OpenGL
/usr/lib/python2.5/site-packages/OpenGL/tests/test_nolights.py
/usr/lib/python2.5/site-packages/OpenGL/tests/test_nolights.pyc
/usr/lib/python2.5/site-packages/OpenGL/tests/test_nolights.pyo
/usr/lib/python2.5/site-packages/OpenGL/tests/test_reported_leak.py
/usr/lib/python2.5/site-packages/OpenGL/tests/test_reported_leak.pyc
/usr/lib/python2.5/site-packages/OpenGL/tests/test_reported_leak.pyo

Idem as pygame


---

Before continuing I would like to hear from you what you think about this. 
I've removed all 4 of the above from my system and glchess still works fine, IOW
these really are optional, and thus in my vision should not be required.

Well, what happens if you completely drop
all files under Tk/ and tests/ temporally?

If no problems ocurr, it does make sense to split such a
usually-unneeded scripts (as python splits tkinter part).

If some problems may arise, would you tell us?
If it can be fixed with easy, then we can think
the case above.

(In reply to comment #7)
> Well, what happens if you completely drop
> all files under Tk/ and tests/ temporally?
> 

As said I tried removing tests before, but that caused an error, however I tried
again and now it works, so I guess I messed up last time :)

Removing Tk also gives no problems.

Suggested solution:
Remove the tests dir, removing the need for python-imaging, pygame and wxPython

Put the tk support in its own subpackage and make that require tkinter.

Agreed?

Well, IMO:

* If you think tests scripts are somewhat useful, then
  you can create test subpackage (postgresql seems to
  have test package). 
  If you think that these scripts are not needed, then
  remove them.
  I leave this as how you judge.

* I agree with creating Tk subpackage.

I've chosen to not package to tests. All mentioned issues are fixed, new version
here:
Spec URL: http://people.atrpms.net/~hdegoede/PyOpenGL.spec
SRPM URL: http://people.atrpms.net/~hdegoede/PyOpenGL-3.0.0-0.2.a6.fc7.src.rpm

I'll review the python bindings for mecab as time allows. I'm afraid I can't
help with the ruby bindings, as I know absolutely nothing about Ruby, it might
be best to post to the list for this, or maybe there is a Ruby SIG?

(In reply to comment #10)
> I've chosen to not package to tests. All mentioned issues are fixed, new version
> here:
> Spec URL: http://people.atrpms.net/~hdegoede/PyOpenGL.spec
> SRPM URL: http://people.atrpms.net/~hdegoede/PyOpenGL-3.0.0-0.2.a6.fc7.src.rpm
> 

I check this after I once take a rest...

> I'll review the python bindings for mecab as time allows. I'm afraid I can't
> help with the ruby bindings, as I know absolutely nothing about Ruby, it might
> be best to post to the list for this, or maybe there is a Ruby SIG?

"Ruby SIG" means the "guideline for ruby package"? If so,
there is.

http://fedoraproject.org/wiki/Packaging/Ruby

Well, for -0.2:

* Again python related dependency.
  - Please check the dependency for python-numarray

Other things are okay.

(In reply to comment #12)
> Well, for -0.2:
> 
> * Again python related dependency.
>   - Please check the dependency for python-numarray
> 
> Other things are okay.

First of all, thanks for all the hard work!

I'm not such a python expert, anything specific I can grab for to see which
parts need it? For the record I don't have it installed:
[hans@shalem ~]$ rpm -q python-numarray
package python-numarray is not installed

And PyOpenGL works fine.

Well, this script.

/usr/lib/python2.5/site-packages/OpenGL/arrays/numarrays.py
--------------------------------------------
     1  """NumArray implementation of the OpenGL-ctypes array interfaces
     2  """
     3  REGISTRY_NAME = 'numarray'
     4  try:
     5          import numarray
     6  except ImportError, err:
     7          raise ImportError( """No numarray module present: %s"""%(err))
     8  import operator
     9  
---------------------------------------------

However, it seems that this script is not called by
any other scripts in PyOpenGL.

Since I think it is too strict to create subpackages only
for this script and I don't think this script should be
removed (perhaps someone may use this), so I think
it is okay to leave this.

----------------------------------------------
   This package (PyOpenGL) is APPROVED by me.

New Package CVS Request
=======================
Package Name:      PyOpenGL
Short Description: Python bindings for OpenGL
Owners:            j.w.r.degoede
Branches:          FC-6 devel
InitialCC:         <empty>

Many thanks for the review!

Imported and build, closing.