Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226342
Summary: Merge Review: python Product: Fedora Extras Version: devel Platform: All OS/Version: Linux Status: NEW Severity: normal Priority: normal Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: nobody@fedoraproject.org QAContact: fedora-package-review@redhat.com CC: katzj@redhat.com
Fedora Merge Review: python
http://cvs.fedora.redhat.com/viewcvs/devel/python/ Initial Owner: katzj@redhat.com
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: python
https://bugzilla.redhat.com/show_bug.cgi?id=226342
bugzilla@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |medium Priority|normal |medium Product|Fedora Extras |Fedora
opensource@till.name changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |opensource@till.name
------- Additional Comments From opensource@till.name 2007-11-09 10:03 EST ------- The version in the changelog entries does not match the real version of the package, e.g. for python-2.5-14.fc7 the latest changelog entry is "2.5.3-14" which is very confusing.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: python
https://bugzilla.redhat.com/show_bug.cgi?id=226342
bugzilla@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Version|devel |rawhide
tibbs@math.uh.edu changed:
What |Removed |Added ---------------------------------------------------------------------------- OtherBugsDependingO| |426387 nThis| |
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=226342
Jon Ciesla limb@jcomserv.net changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |james.antill@redhat.com, | |limb@jcomserv.net
--- Comment #2 from Jon Ciesla limb@jcomserv.net 2008-09-18 15:01:19 EDT --- Adding current owner.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=226342
James Antill james.antill@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
--- Comment #3 from James Antill james.antill@redhat.com 2008-10-03 10:12:29 EDT --- Package Change Request ====================== Package Name: python New Branches: F-10 Owners: james katzj
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=226342
--- Comment #4 from Jon Ciesla limb@jcomserv.net 2008-10-03 10:18:51 EDT --- ??? Shouldn't ownership be changed in pkgdb?
https://admin.fedoraproject.org/pkgdb/packages/name/python
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=226342
--- Comment #5 from James Antill james.antill@redhat.com 2008-10-03 12:16:54 EDT --- The owner in pkgdb is james (me) ... what's the problem?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=226342
--- Comment #6 from Jon Ciesla limb@jcomserv.net 2008-10-03 12:30:00 EDT --- I saw that. I misread the request. I thought it was an ownership change or some stripe, I totally missed the New Branch for F-10.
Ignore. :)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=226342
Kevin Fenzi kevin@tummy.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-cvs? |fedora-cvs+
--- Comment #7 from Kevin Fenzi kevin@tummy.com 2008-10-03 12:46:36 EDT --- cvs done.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=226342
Michal Nowak mnowak@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |mnowak@redhat.com AssignedTo|nobody@fedoraproject.org |mnowak@redhat.com Flag| |fedora-review?, | |needinfo?(james.antill@redh | |at.com)
--- Comment #8 from Michal Nowak mnowak@redhat.com 2009-04-09 07:32:25 EDT --- * mostly missing state of the patches w.r.t. upstream
Patch0: python-2.6-config.patch Patch1: Python-2.2.1-pydocnogui.patch
https://fedoraproject.org/wiki/Packaging:Guidelines#All_patches_should_have_...
* inconsistency in sub-packages "Requires" field:
- python-libs : Requires: %{python} = %{version}-%{release} - python-tools: Requires: %{name} = %{version}-%{release}
in case it's expected to be %{python} -eq %{name} I'd prefer to land on %{name} here.
* inconsistency in patching
%patch6 -p1 -b .plural %patch7 -p1
* old-school exec, back-ticks are BASH specific and non-POSIX, even in Python are now discouraged
topdir=`pwd` -> topdir=$(pwd)
* missing _"_ should cover LD_LIBRARY_PATH's value
- LD_LIBRARY_PATH=$topdir $topdir/python Tools/scripts/pathfix.py -i "%{_bindir}/env python%{pybasever}" .
- LD_LIBRARY_PATH=$topdir PATH=$PATH:$topdir make -s OPT="$CFLAGS" %{?_smp_mflags}
- make install DESTDIR=$RPM_BUILD_ROOT
* from what is this preventing? Considering that this is the only occurrence.
[ -d $RPM_BUILD_ROOT ] && rm -fr $RPM_BUILD_ROOT ^^^^^^^^^^^^^^^^^^^^^^
* using path-based BR is discouraged /usr/bin/find, use "findutils" better, but note, that are listed in packages not necessary to be pulled to BR's https://fedoraproject.org/wiki/Packaging:Guidelines#Exceptions and thus always expected to be present.
* BuildRoot is non-standard, use one of this ones: https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag
BuildRoot: %{_tmppath}/%{name}-%{version}-root
* is this hard-coded "/usr" necessary?
mkdir -p $RPM_BUILD_ROOT/usr $RPM_BUILD_ROOT%{_mandir}
->
mkdir -p $RPM_BUILD_ROOT%{_prefix} $RPM_BUILD_ROOT%{_mandir}
* don't mix variable and macro style (e.g. $RPM_BUILD_ROOT -> %{buildroot})
https://fedoraproject.org/wiki/Packaging:Guidelines#Using_.25.7Bbuildroot.7D...
* old-school defattr
%defattr(-, root, root) -> %defattr(-,root,root,-)
* be consistent with
%dir %{_libdir}/python%{pybasever} %dir %{_libdir}/python%{pybasever}/site-packages
where when %{_prefix} is changed then %{_libdir} is changed too:
""" %if "%{_lib}" == "lib64" %attr(0755,root,root) %dir /usr/lib/python%{pybasever} %attr(0755,root,root) %dir /usr/lib/python%{pybasever}/site-packages %endif """
->
""" %if "%{_lib}" == "lib64" %dir %{_prefix}/lib/python%{pybasever} %dir %{_prefix}/lib/python%{pybasever}/site-packages %endif """
* hard-coded includedir
/usr/include/* -> %{_includedir}/%{name}%{pybasever}
* %defattr(-,root,root,755) -> %defattr(-,root,root,-)
* RPMLINT:
- python.x86_64: W: obsolete-not-provided python-elementtree
python-elementtree still present in F-9 and devel
- python.x86_64: E: binary-or-shlib-defines-rpath /usr/lib64/python 2.6/lib-dynload/_sqlite3.so ['/usr/lib64']
https://fedoraproject.org/wiki/Packaging:Guidelines#Beware_of_Rpath
- python.x86_64: E: non-standard-executable-perm /usr/lib64/python2 .6/lib-dynload/_ssl.so 0555
[and a lot of others] should be 0755?
- python.x86_64: E: script-without-shebang /usr/lib64/python2.6/run py.py
Add shebang, or remove exec perm.
- python-devel.x86_64: W: summary-ended-with-dot The libraries and header files needed for Python development. - python-tools.x86_64: W: summary-ended-with-dot A collection of de velopment tools included with Python.
Fix, please.
- python-test.x86_64: W: spelling-error-in-description pacakge pack age
ditto
- python-test.x86_64: W: no-documentation
Ignore, perhaps...
- python-test.x86_64: E: non-executable-script /usr/lib64/python2.6 /test/test_multibytecodec_support.py 0644
Add 0755 perms, or remove shebang.
- python-test.x86_64: E: zero-length /usr/lib64/python2.6/test/null cert.pem
Is it necessary for the test?
- python-test.x86_64: E: wrong-script-interpreter /usr/lib64/python 2.6/test/test_pep263.py "-*-"
rpmlint is confused here because of the leading _!_, which is hardly necessary there, MacCVS should be fixed, not that file.
- python-test.x86_64: E: wrong-script-end-of-line-encoding /usr/lib 64/python2.6/test/test_pep263.py
- python-tools.x86_64: W: devel-file-in-non-devel-package /usr/lib6 4/python2.6/Demo/embed/demo.c
Probably not necessary to create python-tools-devel for two tests/4 .c files.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=226342
--- Comment #9 from Michal Nowak mnowak@redhat.com 2009-08-11 09:22:43 EDT --- ping
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=226342
Dave Malcolm dmalcolm@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |526126
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=226342
--- Comment #10 from Dave Malcolm dmalcolm@redhat.com 2010-01-26 18:53:18 EST --- (In reply to comment #8)
Thanks for your work on this.
I did a big reworking of the devel/python.spec last night; mostly to eliminate embedded copies of library sources; to ensure safety doing this I replaced the "dynfiles" code with an explicit list of the compiled modules in the payload (as a "safety net" to trap if modules went away).
I'm sorry about my lack of response on the review so far; I'm somewhat doomed with another issue right now :(
2.6.4-12.fc13 is building as: http://koji.fedoraproject.org/koji/taskinfo?taskID=1946933 and contains some fixes as follows:
* Tue Jan 26 2010 David Malcolm dmalcolm@redhat.com - 2.6.4-12 - Address some of the issues identified in package review (bug 226342): - update libs requirement on base package to use %%{name} for consistency's sake - convert from backticks to $() syntax throughout - wrap value of LD_LIBRARY_PATH in quotes - convert "/usr/bin/find" requirement to "findutils" - remove trailing periods from summaries of -devel and -tools subpackages - fix spelling mistake in description of -test subpackage - convert usage of $$RPM_BUILD_ROOT to %%{buildroot} throughout, for stylistic consistency - supply dirmode arguments to defattr directives
Further comments inline:
- mostly missing state of the patches w.r.t. upstream
Patch0: python-2.6-config.patch Patch1: Python-2.2.1-pydocnogui.patch
https://fedoraproject.org/wiki/Packaging:Guidelines#All_patches_should_have_...
Agreed; this is the biggest issue for me with the spec as it stands. I hope to address this when other things calm down.
- inconsistency in sub-packages "Requires" field:
- python-libs : Requires: %{python} = %{version}-%{release}
- python-tools: Requires: %{name} = %{version}-%{release}
in case it's expected to be %{python} -eq %{name} I'd prefer to land on %{name} here.
Fixed in 2.6.4-12.fc13
- inconsistency in patching
%patch6 -p1 -b .plural %patch7 -p1
Still TODO, I'm afraid. My preference is to use -b for C code, and omit it for .py code (to avoid accidentally shipping the backups in the payload).
- old-school exec, back-ticks are BASH specific and non-POSIX, even in Python
are now discouraged
topdir=`pwd` -> topdir=$(pwd)
I've updated these throughout the specfile to use $() in 2.6.4-12.fc13
- missing _"_ should cover LD_LIBRARY_PATH's value
- LD_LIBRARY_PATH=$topdir $topdir/python Tools/scripts/pathfix.py -i
"%{_bindir}/env python%{pybasever}" .
- LD_LIBRARY_PATH=$topdir PATH=$PATH:$topdir make -s OPT="$CFLAGS"
%{?_smp_mflags}
I've updated these two so to: LD_LIBRARY_PATH="$topdir" in 2.6.4-12.fc13 - is this correct?
- make install DESTDIR=$RPM_BUILD_ROOT
What's the issue here?
- from what is this preventing? Considering that this is the only occurrence.
[ -d $RPM_BUILD_ROOT ] && rm -fr $RPM_BUILD_ROOT ^^^^^^^^^^^^^^^^^^^^^^
Changed to "rm -rf $RPM_BUILD_ROOT" in 2.6.4-12.fc13 as per packaging guidelines.
- using path-based BR is discouraged /usr/bin/find, use "findutils" better, but
note, that are listed in packages not necessary to be pulled to BR's https://fedoraproject.org/wiki/Packaging:Guidelines#Exceptions and thus always expected to be present.
Changed to "findutils" in 2.6.4-12.fc13
- BuildRoot is non-standard, use one of this ones:
https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag
BuildRoot: %{_tmppath}/%{name}-%{version}-root
Already changed to be the 2nd one on the list
- is this hard-coded "/usr" necessary?
mkdir -p $RPM_BUILD_ROOT/usr $RPM_BUILD_ROOT%{_mandir}
->
mkdir -p $RPM_BUILD_ROOT%{_prefix} $RPM_BUILD_ROOT%{_mandir}
Fixed in 2.6.4-12.fc13
- don't mix variable and macro style (e.g. $RPM_BUILD_ROOT -> %{buildroot})
https://fedoraproject.org/wiki/Packaging:Guidelines#Using_.25.7Bbuildroot.7D...
Changed $RPM_BUILD_ROOT -> %{buildroot} in 2.6.4-12.fc13
- old-school defattr
%defattr(-, root, root) -> %defattr(-,root,root,-)
Fixed in 2.6.4-12.fc13
be consistent with
%dir %{_libdir}/python%{pybasever} %dir %{_libdir}/python%{pybasever}/site-packages
where when %{_prefix} is changed then %{_libdir} is changed too:
""" %if "%{_lib}" == "lib64" %attr(0755,root,root) %dir /usr/lib/python%{pybasever} %attr(0755,root,root) %dir /usr/lib/python%{pybasever}/site-packages %endif """
->
""" %if "%{_lib}" == "lib64" %dir %{_prefix}/lib/python%{pybasever} %dir %{_prefix}/lib/python%{pybasever}/site-packages %endif """
Already fixed
- hard-coded includedir
/usr/include/* -> %{_includedir}/%{name}%{pybasever}
Already fixed
- %defattr(-,root,root,755) -> %defattr(-,root,root,-)
Which subpackages?
- RPMLINT:
- python.x86_64: W: obsolete-not-provided python-elementtree
python-elementtree still present in F-9 and devel
- python.x86_64: E: binary-or-shlib-defines-rpath /usr/lib64/python
2.6/lib-dynload/_sqlite3.so ['/usr/lib64']
https://fedoraproject.org/wiki/Packaging:Guidelines#Beware_of_Rpath
- python.x86_64: E: non-standard-executable-perm /usr/lib64/python2
.6/lib-dynload/_ssl.so 0555
[and a lot of others] should be 0755?
- python.x86_64: E: script-without-shebang /usr/lib64/python2.6/run
py.py
Add shebang, or remove exec perm.
I haven't yet looked into the ones above. RPath issue is possibly already fixed.
- python-devel.x86_64: W: summary-ended-with-dot The libraries and
header files needed for Python development.
- python-tools.x86_64: W: summary-ended-with-dot A collection of de
velopment tools included with Python.
Fix, please.
Fixed in 2.6.4-12.fc13
- python-test.x86_64: W: spelling-error-in-description pacakge pack
age
ditto
Fixed in 2.6.4-12.fc13
- python-test.x86_64: W: no-documentation
Ignore, perhaps...
- python-test.x86_64: E: non-executable-script /usr/lib64/python2.6
/test/test_multibytecodec_support.py 0644
Add 0755 perms, or remove shebang.
- python-test.x86_64: E: zero-length /usr/lib64/python2.6/test/null
cert.pem
Is it necessary for the test?
- python-test.x86_64: E: wrong-script-interpreter /usr/lib64/python
2.6/test/test_pep263.py "-*-"
rpmlint is confused here because of the leading _!_, which is hardly necessary there, MacCVS should be fixed, not that file.
- python-test.x86_64: E: wrong-script-end-of-line-encoding /usr/lib
64/python2.6/test/test_pep263.py
- python-tools.x86_64: W: devel-file-in-non-devel-package /usr/lib6
4/python2.6/Demo/embed/demo.c
Probably not necessary to create python-tools-devel for two tests/4 .c files.
I haven't yet looked at the ones above.
Thanks again for your review work; sorry for delay in resolving the issues you identified.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=226342
Michal Nowak mnowak@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|needinfo?(james.antill@redh | |at.com) |
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=226342
--- Comment #11 from Dave Malcolm dmalcolm@redhat.com 2010-01-29 19:21:30 EST --- I've gone through the patches, adding comments explaining what they all do, and upstream status, as far as possible, in tag "python-2_6_4-13_fc13". I also removed the various commented-out patches.
See: http://cvs.fedoraproject.org/viewvc/rpms/python/devel/python.spec?r1=1.164&a...
There appear to be some places where patches need to be upstreamed.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=226342
--- Comment #12 from Jon Ciesla limb@jcomserv.net 2011-10-18 11:55:29 EDT --- Where are we on this?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=226342
Michal Nowak mnowak@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|mnowak@redhat.com |nobody@fedoraproject.org
package-review@lists.fedoraproject.org