Spec URL: http://people.atrpms.net/~hdegoede/PyOpenGL.spec SRPM URL: http://people.atrpms.net/~hdegoede/PyOpenGL-3.0.0-0.1.a6.fc7.src.rpm Description: Python bindings for OpenGL --- Notice to reviewers, yes this is an alpha version and the website also has a shiny 2.x stable version, unfortunately the 2.x version contains parts under the SGI Free B license, which is not a free license. 3.x is a rewrite without any license issues (not the reason for the rewrite, but a nice side effect!). Spot has audited 3.x and its fine license wise.
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.