https://bugzilla.redhat.com/show_bug.cgi?id=1219411 --- Comment #3 from Matej Stuchlik <mstuchli@xxxxxxxxxx> --- (In reply to Aurelien Bompard from comment #2) > Since there's already a python3 package in Fedora, I'll comment on the diff > between the current master and this package. > My comments are inlined in the diff below, and start with "--> " > > @@ -16,3 +16,3 @@ > > -%global with_rewheel 1 > +%global with_rewheel 0 > > @@ -23,2 +23,10 @@ > > +# is this the EPEL 7 main Python 3? > +%if "%python3_pkgversion" == "%pyshortver" > +%global main_python3 1 > +%else > +%global main_python3 0 > +%endif > + > + > %global pylibdir %{_libdir}/python%{pybasever} > > --> The with_rewheel flag should be set to 1 in the final package, as it is > in the current master. Correct, in the final package it should be and will be, first however one needs to resolve the dependency cycle between Python, pip, setuptools and wheel. There's more about this at the top of the .spec file in the comment. > --> I don't understand this main_python3 package flag. This package will > never be the main Python3 package, because it's from EPEL and RHEL will > always ship the main python3 package. The purpose of the main_python3 flag is somewhat explained in [0], in "Lifecycle of python3X stacks, rebuilding". Let me know if it's not clear. :) I'll add a comment to the .spec explaining the purpose of the flag. Note that Python 3 is actually *not* in RHEL 7, hence why there's a need for it in EPEL. [0] https://fedoraproject.org/wiki/User:Bkabrda/EPEL7_Python3 > @@ -1121,3 +1140,4 @@ > %endif > - false > + false \ > + -O1 > %endif # with_debug_build > > --> What's the reason for adding the O1 flag? The point of that flag is to make debugging the debug build easier, but now that you mention it, it should be set to -O0 or -Og. Will fix! > @@ -1424,2 +1469,20 @@ > > +%if ! 0%{?main_python3} > +# make altinstall doesn't create python3.X-config, but we want it > +# (we don't want to have just python3.Xm-config, that's a bit confusing) > +ln -s \ > + %{_bindir}/python%{LDVERSION_optimized}-config \ > + %{buildroot}%{_bindir}/python%{pybasever}-config > +# make altinstall doesn't create python-3.4m.pc, only python-3.4.pc, but we > want both > +ln -s \ > + %{_libdir}/pkgconfig/python-%{pybasever}.pc \ > + %{buildroot}%{_libdir}/pkgconfig/python-%{LDVERSION_optimized}.pc > +%endif > + > +# remove libpython3.so in EPEL python to not cause collision between > python3X and > +# python3X+1 stacks... I don't see any way in which this would be useful > +# Gentoo does this, as well... TODO: there's a sysconfig variable pointing > +# to this, maybe we should do something about it? > +rm -f %{buildroot}%{_libdir}/libpython3.so > + > # ====================================================== > > --> Looks good to me, I don't know about the sysconfig variable, can you > elaborate on that? I'm frankly not sure on that, the comment is Slavek's. Will find out. > @@ -1456,2 +1519,3 @@ > --verbose --findleaks \ > + -x test_distutils \ > %ifarch ppc64le aarch64 > > --> What's the reason for that? One of the tests in distutils test suite checks for a precise pip version, since we unbundle pip, it may happen that it is of a different version that the test expects. This was a temporary "solution", will fix. :) > END OF DIFF > > I tried to review the other changes in the sources but the SRPM file seems > corrupt, I get: > error: unpacking of archive failed on file > /home/abompard/devel/rpms/RPMS/python34/Python-3.4.3.tar.xz;55658a3b: cpio: > read > error: python34-3.4.3-1.fc21.src.rpm cannot be installed > Please upload a fixed version so I can continue the review, and answer the > few questions I have asked here in the meantime. > Thanks! SRPM URL: https://mstuchli.fedorapeople.org/python34-3.4.3-1.fc21.src.rpm -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review