Being this a core package, I think it should be of an exceptional quality. Let me see: > # Copyright 1999-2016 Gentoo Foundation > # Distributed under the terms of the GNU General Public License v2 > # $Id$ > EAPI="5" 6 > WANT_LIBTOOL="none" > > inherit autotools eutils flag-o-matic multilib pax-utils python-utils-r1 toolchain-funcs > > MY_P="Python-${PV/_/}" > PATCHSET_VERSION="3.5.1-0" > > DESCRIPTION="An interpreted, interactive, object-oriented programming language" > HOMEPAGE="http://www.python.org/" > SRC_URI="http://www.python.org/ftp/python/${PV%_rc*}/${MY_P}.tar.xz https > https://dev.gentoo.org/~floppym/python/python-gentoo-patches-${PATCHSET_VERSION}.tar.xz" The patchset contains ~10 files having in total 50kB in size. Is it really necessary to have it separated? > > LICENSE="PSF-2" > SLOT="3.5/3.5m" > KEYWORDS="~alpha ~amd64 ~arm ~arm64 ~hppa ~ia64 ~m68k ~mips ~ppc ~ppc64 ~s390 ~sh ~sparc ~x86 ~amd64-fbsd ~sparc-fbsd ~x86-fbsd" > IUSE="build elibc_uclibc examples gdbm hardened ipv6 libressl +ncurses +readline sqlite +ssl +threads tk wininst +xml" > > # Do not add a dependency on dev-lang/python to this ebuild. > # If you need to apply a patch which requires python for bootstrapping, please > # run the bootstrap code on your dev box and include the results in the > # patchset. See bug 447752. > > RDEPEND="app-arch/bzip2:0= > app-arch/xz-utils:0= > >=sys-libs/zlib-1.1.3:0= > virtual/libffi > virtual/libintl > gdbm? ( sys-libs/gdbm:0=[berkdb] ) > ncurses? ( > >=sys-libs/ncurses-5.2:0= > readline? ( >=sys-libs/readline-4.1:0= ) > ) > sqlite? ( >=dev-db/sqlite-3.3.8:3= ) > ssl? ( > !libressl? ( dev-libs/openssl:0= ) > libressl? ( dev-libs/libressl:= ) > ) > tk? ( > >=dev-lang/tcl-8.0:0= > >=dev-lang/tk-8.0:0= > dev-tcltk/blt:0= > dev-tcltk/tix > ) > xml? ( >=dev-libs/expat-2.1:0= ) > !!<sys-apps/sandbox-2.6-r1" > DEPEND="${RDEPEND} > virtual/pkgconfig > !sys-devel/gcc[libffi(-)]" > RDEPEND+=" !build? ( app-misc/mime-types )" Use a separate variable like CDEPEND instead of these clever tricks. > PDEPEND=">=app-eselect/eselect-python-20140125-r1" > > S="${WORKDIR}/${MY_P}" > > PYVER=${SLOT%/*} > > src_prepare() { > # Ensure that internal copies of expat, libffi and zlib are not used. > rm -fr Modules/expat > rm -fr Modules/_ctypes/libffi* > rm -fr Modules/zlib No die()? > > if tc-is-cross-compiler; then > # Invokes BUILDPYTHON, which is built for the host arch > local EPATCH_EXCLUDE="*_regenerate_platform-specific_modules.patch" > fi > > EPATCH_SUFFIX="patch" epatch "${WORKDIR}/patches" > epatch "${FILESDIR}/${PN}-3.4.3-ncurses-pkg-config.patch" > epatch "${FILESDIR}/3.5.1-cross-compile.patch" > > epatch_user > After EAPI6 and external patchset removal, this section needs to be revamped. > sed -i -e "s:@@GENTOO_LIBDIR@@:$(get_libdir):g" \ > configure.ac \ > Lib/distutils/command/install.py \ > Lib/distutils/sysconfig.py \ > Lib/site.py \ > Lib/sysconfig.py \ > Lib/test/test_site.py \ > Makefile.pre.in \ > Modules/getpath.c \ > Modules/Setup.dist \ > setup.py || die "sed failed to replace @@GENTOO_LIBDIR@@" > Maybe something like `grep -l '@@GENTOO_LIBDIR@@' | xargs sed ...` would be better instead of infinite listings. > eautoreconf > } > > src_configure() { > local disable > use gdbm || disable+=" gdbm" > use ncurses || disable+=" _curses _curses_panel" > use readline || disable+=" readline" > use sqlite || disable+=" _sqlite3" > use ssl || export PYTHON_DISABLE_SSL="1" > use tk || disable+=" _tkinter" > use xml || disable+=" _elementtree pyexpat" # _elementtree uses pyexpat. This section would really make use of an array. > export PYTHON_DISABLE_MODULES="${disable}" > > if ! use xml; then > ewarn "You have configured Python without XML support." > ewarn "This is NOT a recommended configuration as you" > ewarn "may face problems parsing any XML documents." > fi Oh, you don't say! > > if [[ -n "${PYTHON_DISABLE_MODULES}" ]]; then > einfo "Disabled modules: ${PYTHON_DISABLE_MODULES}" > fi > > if [[ "$(gcc-major-version)" -ge 4 ]]; then > append-flags -fwrapv > fi What is this? > > filter-flags -malign-double What is this? > > [[ "${ARCH}" == "alpha" ]] && append-flags -fPIC Why? > > # https://bugs.gentoo.org/show_bug.cgi?id=50309 > if is-flagq -O3; then > is-flagq -fstack-protector-all && replace-flags -O3 -O2 This code and the bug report is from 2004-2005, how is this relavant to python3 and year 2016? > use hardened && replace-flags -O3 -O2 So, is this the only purpose of the hardened USE-flag? > fi > > # Export CXX so it ends up in /usr/lib/python3.X/config/Makefile. > tc-export CXX > > # The configure script fails to use pkg-config correctly. > # http://bugs.python.org/issue15506 > export ac_cv_path_PKG_CONFIG=$(tc-getPKG_CONFIG) > > # Set LDFLAGS so we link modules with -lpython3.2 correctly. > # Needed on FreeBSD unless Python 3.2 is already installed. > # Please query BSD team before removing this! > append-ldflags "-L." Being this BSD-only workaround with no scope limitation, how does it affect other platforms? > > local dbmliborder > if use gdbm; then > dbmliborder+="${dbmliborder:+:}gdbm" > fi What is this code trying to do? And why doesn't is use usex()? > > BUILD_DIR="${WORKDIR}/${CHOST}" > mkdir -p "${BUILD_DIR}" || die > cd "${BUILD_DIR}" || die > > local myeconfargs=( > --with-fpectl > --enable-shared > $(use_enable ipv6) > $(use_with threads) > --infodir='${prefix}/share/info' > --mandir='${prefix}/share/man' > --with-computed-gotos > --with-dbmliborder="${dbmliborder}" > --with-libc= > --enable-loadable-sqlite-extensions > --without-ensurepip > --with-system-expat > --with-system-ffi This semi-random order could be improved > ) > > ECONF_SOURCE="${S}" OPT="" econf "${myeconfargs[@]}" What is that OPT for? > > if use threads && grep -q "#define POSIX_SEMAPHORES_NOT_ENABLED 1" pyconfig.h; then > eerror "configure has detected that the sem_open function is broken." > eerror "Please ensure that /dev/shm is mounted as a tmpfs with mode 1777." > die "Broken sem_open function (bug 496328)" > fi > } > > src_compile() { > cd "${BUILD_DIR}" || die > > emake CPPFLAGS= CFLAGS= LDFLAGS= What is this FLAGS reset for? > > # Work around bug 329499. See also bug 413751 and 457194. > if has_version dev-libs/libffi[pax_kernel]; then > pax-mark E python > else > pax-mark m python > fi > } > > src_test() { > # Tests will not work when cross compiling. > if tc-is-cross-compiler; then > elog "Disabling tests due to crosscompiling." > return > fi > > cd "${BUILD_DIR}" || die > > # Skip failing tests. > local skipped_tests="gdb" > > for test in ${skipped_tests}; do > mv "${S}"/Lib/test/test_${test}.py "${T}" > done > > local -x PYTHONDONTWRITEBYTECODE= > emake test EXTRATESTOPTS="-u-network" CPPFLAGS= CFLAGS= LDFLAGS= < /dev/tty > local result=$? > > for test in ${skipped_tests}; do > mv "${T}/test_${test}.py" "${S}"/Lib/test > done > > elog "The following tests have been skipped:" > for test in ${skipped_tests}; do > elog "test_${test}.py" > done > > elog "If you would like to run them, you may:" > elog "cd '${EPREFIX}/usr/$(get_libdir)/python${PYVER}/test'" > elog "and run the tests separately." > > if [[ ${result} -ne 0 ]]; then > die "emake test failed" > fi > } > > src_install() { > local libdir=${ED}/usr/$(get_libdir)/python${PYVER} > > cd "${BUILD_DIR}" || die > > emake DESTDIR="${D}" altinstall Some special reason why not ED? > > sed \ > -e "s/\(CONFIGURE_LDFLAGS=\).*/\1/" \ > -e "s/\(PY_LDFLAGS=\).*/\1/" \ > -i "${libdir}/config-${PYVER}"*/Makefile || die "sed failed" > > # Fix collisions between different slots of Python. > rm -f "${ED}usr/$(get_libdir)/libpython3.so" Is rm somehow excluded from the die() everywhere rule? > > # Cheap hack to get version with ABIFLAGS > local abiver=$(cd "${ED}usr/include"; echo python*) > if [[ ${abiver} != python${PYVER} ]]; then > # Replace python3.X with a symlink to python3.Xm > rm "${ED}usr/bin/python${PYVER}" || die Wow, first rm with die(), so maybe not... > dosym "${abiver}" "/usr/bin/python${PYVER}" > # Create python3.X-config symlink > dosym "${abiver}-config" "/usr/bin/python${PYVER}-config" > # Create python-3.5m.pc symlink > dosym "python-${PYVER}.pc" "/usr/$(get_libdir)/pkgconfig/${abiver/${PYVER}/-${PYVER}}.pc" > fi > > # python seems to get rebuilt in src_install (bug 569908) > # Work around it for now. > if has_version dev-libs/libffi[pax_kernel]; then > pax-mark E "${ED}usr/bin/${abiver}" > else > pax-mark m "${ED}usr/bin/${abiver}" > fi > > use elibc_uclibc && rm -fr "${libdir}/test" > use sqlite || rm -fr "${libdir}/"{sqlite3,test/test_sqlite*} > use tk || rm -fr "${ED}usr/bin/idle${PYVER}" "${libdir}/"{idlelib,tkinter,test/test_tk*} > > use threads || rm -fr "${libdir}/multiprocessing" > use wininst || rm -f "${libdir}/distutils/command/"wininst-*.exe Ehmm, yeah, rm is definitely excluded. > > dodoc "${S}"/Misc/{ACKS,HISTORY,NEWS} > > if use examples; then > insinto /usr/share/doc/${PF}/examples > find "${S}"/Tools -name __pycache__ -print0 | xargs -0 rm -fr > doins -r "${S}"/Tools > fi > insinto /usr/share/gdb/auto-load/usr/$(get_libdir) #443510 > local libname=$(printf 'e:\n\t@echo $(INSTSONAME)\ninclude Makefile\n' | \ > emake --no-print-directory -s -f - 2>/dev/null) > newins "${S}"/Tools/gdb/libpython.py "${libname}"-gdb.py This hack is really squeezed, it wouldn't be too bad to expand it vertically a bit. > > newconfd "${FILESDIR}/pydoc.conf" pydoc-${PYVER} > newinitd "${FILESDIR}/pydoc.init" pydoc-${PYVER} > sed \ > -e "s:@PYDOC_PORT_VARIABLE@:PYDOC${PYVER/./_}_PORT:" \ > -e "s:@PYDOC@:pydoc${PYVER}:" \ > -i "${ED}etc/conf.d/pydoc-${PYVER}" "${ED}etc/init.d/pydoc-${PYVER}" || die "sed failed" > > # for python-exec > local vars=( EPYTHON PYTHON_SITEDIR PYTHON_SCRIPTDIR ) > > # if not using a cross-compiler, use the fresh binary > if ! tc-is-cross-compiler; then > local -x PYTHON=./python > local -x LD_LIBRARY_PATH=${LD_LIBRARY_PATH+${LD_LIBRARY_PATH}:}. > else > vars=( PYTHON "${vars[@]}" ) `vars+=( blah )` is not used because the order matters or what? > fi > > python_export "python${PYVER}" "${vars[@]}" > echo "EPYTHON='${EPYTHON}'" > epython.py || die > python_domodule epython.py > > # python-exec wrapping support > local pymajor=${PYVER%.*} > mkdir -p "${D}${PYTHON_SCRIPTDIR}" || die > # python and pythonX > ln -s "../../../bin/${abiver}" \ > "${D}${PYTHON_SCRIPTDIR}/python${pymajor}" || die What is this trying to point to? > ln -s "python${pymajor}" \ > "${D}${PYTHON_SCRIPTDIR}/python" || die > # python-config and pythonX-config > # note: we need to create a wrapper rather than symlinking it due > # to some random dirname(argv[0]) magic performed by python-config > cat > "${D}${PYTHON_SCRIPTDIR}/python${pymajor}-config" <<-EOF || die > #!/bin/sh > exec "${abiver}-config" "\${@}" > EOF > chmod +x "${D}${PYTHON_SCRIPTDIR}/python${pymajor}-config" || die fperms()? > ln -s "python${pymajor}-config" \ > "${D}${PYTHON_SCRIPTDIR}/python-config" || die > # 2to3, pydoc, pyvenv > ln -s "../../../bin/2to3-${PYVER}" \ > "${D}${PYTHON_SCRIPTDIR}/2to3" || die > ln -s "../../../bin/pydoc${PYVER}" \ > "${D}${PYTHON_SCRIPTDIR}/pydoc" || die > ln -s "../../../bin/pyvenv-${PYVER}" \ > "${D}${PYTHON_SCRIPTDIR}/pyvenv" || die > # idle > if use tk; then > ln -s "../../../bin/idle${PYVER}" \ > "${D}${PYTHON_SCRIPTDIR}/idle" || die > fi Some special reason why not ED? > } > > pkg_preinst() { > if has_version "<${CATEGORY}/${PN}-${PYVER}" && ! has_version ">=${CATEGORY}/${PN}-${PYVER}_alpha"; then > python_updater_warning="1" > fi > } > > eselect_python_update() { > if [[ -z "$(eselect python show)" || ! -f "${EROOT}usr/bin/$(eselect python show)" ]]; then > eselect python update > fi > > if [[ -z "$(eselect python show --python${PV%%.*})" || ! -f "${EROOT}usr/bin/$(eselect python show --python${PV%%.*})" ]]; then I have an idea on how to make this line even longer > eselect python update --python${PV%%.*} > fi > } > > pkg_postinst() { > eselect_python_update > > if [[ "${python_updater_warning}" == "1" ]]; then > ewarn "You have just upgraded from an older version of Python." > ewarn > ewarn "Please adjust PYTHON_TARGETS (if so desired), and run emerge with the --newuse or --changed-use option to rebuild packages installing python modules." Pushing limits. Yeah, I like it. > fi > } > > pkg_postrm() { > eselect_python_update > } > And I'm sure there is more, eg. I completely skipped src_test(). I reviewed the 3.5.1-r2.ebuild as it should the most up-to-date one, but the others are almost the same.
(In reply to Jan Chren (rindeal) from comment #0) Thanks for your feedback (minus the rude remarks). One thing to note is that this package is very old and has had several maintainers over the years. It has therefore grown a few barnacles that nobody really wants to touch without some reason for doing so. If you would like to submit some patches, we can certainly look at applying the changes that make sense. > > EAPI="5" > > 6 > EAPI 6 does not provide any must-have feature for this ebuild. As well, as a critical system package, keeping it at EAPI 5 is a conservative approach for maintenance. Moving to EAPI 6 will require the a full audit of the ebuild, and additional testing. > > SRC_URI="http://www.python.org/ftp/python/${PV%_rc*}/${MY_P}.tar.xz > > https That seems like a good improvement. > > https://dev.gentoo.org/~floppym/python/python-gentoo-patches-${PATCHSET_VERSION}.tar.xz" > > The patchset contains ~10 files having in total 50kB in size. Is it really > necessary to have it separated? The devmanual mentions 20k as a soft limit. This is kind of a maintainer judgement call, and it has historically been done as a tarball. > > RDEPEND+=" !build? ( app-misc/mime-types )" > > Use a separate variable like CDEPEND instead of these clever tricks. This is personal preference really. I don't feel strongly about it. > > sed -i -e "s:@@GENTOO_LIBDIR@@:$(get_libdir):g" \ > > configure.ac \ > > Lib/distutils/command/install.py \ > > Lib/distutils/sysconfig.py \ > > Lib/site.py \ > > Lib/sysconfig.py \ > > Lib/test/test_site.py \ > > Makefile.pre.in \ > > Modules/getpath.c \ > > Modules/Setup.dist \ > > setup.py || die "sed failed to replace @@GENTOO_LIBDIR@@" > > > > Maybe something like `grep -l '@@GENTOO_LIBDIR@@' | xargs sed ...` would be > better instead of infinite listings. There is nothing "infinite" about it; we know exactly which files need to be adjusted, so calling grep and xargs is just overhead that would allow for some laziness by the maintainer. > > if ! use xml; then > > ewarn "You have configured Python without XML support." > > ewarn "This is NOT a recommended configuration as you" > > ewarn "may face problems parsing any XML documents." > > fi > > Oh, you don't say! Don't be a dick please. > > > > if [[ -n "${PYTHON_DISABLE_MODULES}" ]]; then > > einfo "Disabled modules: ${PYTHON_DISABLE_MODULES}" > > fi > > > > if [[ "$(gcc-major-version)" -ge 4 ]]; then > > append-flags -fwrapv > > fi > > What is this? > > > > filter-flags -malign-double > > What is this? > > > > > [[ "${ARCH}" == "alpha" ]] && append-flags -fPIC > > Why? > I'm not sure; whoever wrote this ebuild originally left no citation. You could review the commit history for a clue. > > > > # https://bugs.gentoo.org/show_bug.cgi?id=50309 > > if is-flagq -O3; then > > is-flagq -fstack-protector-all && replace-flags -O3 -O2 > > This code and the bug report is from 2004-2005, how is this relavant to > python3 and year 2016? Do you have any evidence to indicate it is no longer necessary? > > use hardened && replace-flags -O3 -O2 > > So, is this the only purpose of the hardened USE-flag? That appears to be the case. > > # Set LDFLAGS so we link modules with -lpython3.2 correctly. > > # Needed on FreeBSD unless Python 3.2 is already installed. > > # Please query BSD team before removing this! > > append-ldflags "-L." > > Being this BSD-only workaround with no scope limitation, how does it affect > other platforms? I'm not sure. Perhaps you could research it and provide some advice. > > local dbmliborder > > if use gdbm; then > > dbmliborder+="${dbmliborder:+:}gdbm" > > fi > > What is this code trying to do? And why doesn't is use usex()? It builds a list of dbm modules to be passed to configure. For python2.7, there are a couple of dbm modules that get (optionally) built, so the code makes a bit more sense there. The code likely predates usex. > > > > BUILD_DIR="${WORKDIR}/${CHOST}" > > mkdir -p "${BUILD_DIR}" || die > > cd "${BUILD_DIR}" || die > > > > local myeconfargs=( > > --with-fpectl > > --enable-shared > > $(use_enable ipv6) > > $(use_with threads) > > --infodir='${prefix}/share/info' > > --mandir='${prefix}/share/man' > > --with-computed-gotos > > --with-dbmliborder="${dbmliborder}" > > --with-libc= > > --enable-loadable-sqlite-extensions > > --without-ensurepip > > --with-system-expat > > --with-system-ffi > > This semi-random order could be improved Cosmetic; if someone feels strongly about it they can sort it. > > ) > > > > ECONF_SOURCE="${S}" OPT="" econf "${myeconfargs[@]}" > > What is that OPT for? It prevents upstream optimization flags from being added to CFLAGS by the build system. > > emake CPPFLAGS= CFLAGS= LDFLAGS= > > What is this FLAGS reset for? configure populates Makefile variables called CONFIGURE_CFLAGS, CONFIGURE_CPPFLAGS, and CONFIGURE_LDFLAGS. Forcing the non-configure variables to empty on the make command line ensure that the flags do not appear twice when invoking the compiler. > > emake DESTDIR="${D}" altinstall > > Some special reason why not ED? EPREFIX has already been accounted for by passing it to configure. Passing ED to emake install is almost always incorrect for an autotools-based package. > > > > sed \ > > -e "s/\(CONFIGURE_LDFLAGS=\).*/\1/" \ > > -e "s/\(PY_LDFLAGS=\).*/\1/" \ > > -i "${libdir}/config-${PYVER}"*/Makefile || die "sed failed" > > > > # Fix collisions between different slots of Python. > > rm -f "${ED}usr/$(get_libdir)/libpython3.so" > > Is rm somehow excluded from the die() everywhere rule? This should probably be looked at. It's possible someone just got lazy about it. > > use threads || rm -fr "${libdir}/multiprocessing" > > use wininst || rm -f "${libdir}/distutils/command/"wininst-*.exe > > Ehmm, yeah, rm is definitely excluded. I think this is probably left over from when the ebuild had a "build" use flag that trumped a lot of the option feature flags. Doing a non-fatal rm -f was easier than putting use foo || use !build || ... all over the place. It could probably be cleaned up at this point. > > vars=( PYTHON "${vars[@]}" ) > > `vars+=( blah )` is not used because the order matters or what? That seems likely. > > ln -s "../../../bin/${abiver}" \ > > "${D}${PYTHON_SCRIPTDIR}/python${pymajor}" || die > > What is this trying to point to? It's creating a symlink for use by python-exec. Talk to mgorny for more details; I'm fuzzy on how it works myself. > > chmod +x "${D}${PYTHON_SCRIPTDIR}/python${pymajor}-config" || die > > fperms()? PYTHON_SCRIPTDIR contains EPREFIX already; calling fperms would duplicate EPREFIX. > > > ln -s "python${pymajor}-config" \ > > "${D}${PYTHON_SCRIPTDIR}/python-config" || die > > # 2to3, pydoc, pyvenv > > ln -s "../../../bin/2to3-${PYVER}" \ > > "${D}${PYTHON_SCRIPTDIR}/2to3" || die > > ln -s "../../../bin/pydoc${PYVER}" \ > > "${D}${PYTHON_SCRIPTDIR}/pydoc" || die > > ln -s "../../../bin/pyvenv-${PYVER}" \ > > "${D}${PYTHON_SCRIPTDIR}/pyvenv" || die > > # idle > > if use tk; then > > ln -s "../../../bin/idle${PYVER}" \ > > "${D}${PYTHON_SCRIPTDIR}/idle" || die > > fi > > Some special reason why not ED? PYTHON_SCRIPTDIR already contians EPREFIX. > > if [[ -z "$(eselect python show --python${PV%%.*})" || ! -f "${EROOT}usr/bin/$(eselect python show --python${PV%%.*})" ]]; then > > I have an idea on how to make this line even longer Yeah, this is ugly, but it does the job. Suggestions that don't break things are welcome.
(In reply to Mike Gilbert from comment #1) > (In reply to Jan Chren (rindeal) from comment #0) > > Thanks for your feedback (minus the rude remarks). One thing to note is that > this package is very old and has had several maintainers over the years. It > has therefore grown a few barnacles that nobody really wants to touch > without some reason for doing so. On every major python release such changes are possible as everyone expects some hiccups to occur. > If you would like to submit some patches, we can certainly look at applying > the changes that make sense. I'd expect that we split the work, I've brought up the issue and done the review, now it's time for @gentoo/python. If I see that things are moving, I'll happily help with patching and testing. > > > EAPI="5" > > > > 6 > > > > EAPI 6 does not provide any must-have feature for this ebuild. As well, as a > critical system package, keeping it at EAPI 5 is a conservative approach for > maintenance. > > Moving to EAPI 6 will require the a full audit of the ebuild, and additional > testing. EAPI6 is a major code cleanup tool so it's a necessary condition for the code cleanup of this package. "full audit of the ebuild, and additional testing" are also beneficial things for a package like this. > > > > SRC_URI="http://www.python.org/ftp/python/${PV%_rc*}/${MY_P}.tar.xz > > > > https > > That seems like a good improvement. > > > > https://dev.gentoo.org/~floppym/python/python-gentoo-patches-${PATCHSET_VERSION}.tar.xz" > > > > The patchset contains ~10 files having in total 50kB in size. Is it really > > necessary to have it separated? > > The devmanual mentions 20k as a soft limit. This is kind of a maintainer > judgement call, and it has historically been done as a tarball. > > > > RDEPEND+=" !build? ( app-misc/mime-types )" > > > > Use a separate variable like CDEPEND instead of these clever tricks. > > This is personal preference really. I don't feel strongly about it. If you look at the ebuild as a whole, you will see or rather not see this line, because it's well hidden. `*DEPEND+=` should be a generally forbidden construct as there is no objectively valid reason in using it. > > > sed -i -e "s:@@GENTOO_LIBDIR@@:$(get_libdir):g" \ > > > configure.ac \ > > > Lib/distutils/command/install.py \ > > > Lib/distutils/sysconfig.py \ > > > Lib/site.py \ > > > Lib/sysconfig.py \ > > > Lib/test/test_site.py \ > > > Makefile.pre.in \ > > > Modules/getpath.c \ > > > Modules/Setup.dist \ > > > setup.py || die "sed failed to replace @@GENTOO_LIBDIR@@" > > > > > > > Maybe something like `grep -l '@@GENTOO_LIBDIR@@' | xargs sed ...` would be > > better instead of infinite listings. > > There is nothing "infinite" about it; we know exactly which files need to be > adjusted, so calling grep and xargs is just overhead that would allow for > some laziness by the maintainer. I haven't created this issue because maintainers were overly active. My suggestion would only reduce the burden of maintaining this long and error prone list. > > > if ! use xml; then > > > ewarn "You have configured Python without XML support." > > > ewarn "This is NOT a recommended configuration as you" > > > ewarn "may face problems parsing any XML documents." > > > fi > > > > Oh, you don't say! > > Don't be a dick please. I tried to point out the fact that it's a pure spam, not to mention that it's completely out of place (pkg_setup is for things like this) and even yells using ewarn(). Terrible piece. And please, don't bring that kind of language to this thread. > > > > > > if [[ "$(gcc-major-version)" -ge 4 ]]; then > > > append-flags -fwrapv > > > fi > > > > What is this? > > > > > > filter-flags -malign-double > > > > What is this? > > > > > > > > [[ "${ARCH}" == "alpha" ]] && append-flags -fPIC > > > > Why? > > > > I'm not sure; whoever wrote this ebuild originally left no citation. You > could review the commit history for a clue. The problem is that it's likely to be ancient, maybe even from the time before git was created by Torvalds. Also standard git-blame doesn't work for ebuilds, only global searching. > > > > > > # https://bugs.gentoo.org/show_bug.cgi?id=50309 > > > if is-flagq -O3; then > > > is-flagq -fstack-protector-all && replace-flags -O3 -O2 > > > > This code and the bug report is from 2004-2005, how is this relavant to > > python3 and year 2016? > > Do you have any evidence to indicate it is no longer necessary? Its age. I guess this workaround stopped being relevant around the time v2.7 came out as a hot stuff. > > > use hardened && replace-flags -O3 -O2 > > > > So, is this the only purpose of the hardened USE-flag? > > That appears to be the case. Then it's very obsolete > > > # Set LDFLAGS so we link modules with -lpython3.2 correctly. > > > # Needed on FreeBSD unless Python 3.2 is already installed. > > > # Please query BSD team before removing this! > > > append-ldflags "-L." > > > > Being this BSD-only workaround with no scope limitation, how does it affect > > other platforms? > > I'm not sure. Perhaps you could research it and provide some advice. My advice is to query BSD team if it's still relevant to 3.5 and hopefully get rid of it. > > > > local dbmliborder > > > if use gdbm; then > > > dbmliborder+="${dbmliborder:+:}gdbm" > > > fi > > > > What is this code trying to do? And why doesn't is use usex()? > > It builds a list of dbm modules to be passed to configure. For python2.7, > there are a couple of dbm modules that get (optionally) built, so the code > makes a bit more sense there. > > The code likely predates usex. > > > > > > > local myeconfargs=( > > > --with-fpectl > > > --enable-shared > > > $(use_enable ipv6) > > > $(use_with threads) > > > --infodir='${prefix}/share/info' > > > --mandir='${prefix}/share/man' > > > --with-computed-gotos > > > --with-dbmliborder="${dbmliborder}" > > > --with-libc= > > > --enable-loadable-sqlite-extensions > > > --without-ensurepip > > > --with-system-expat > > > --with-system-ffi > > > > This semi-random order could be improved > > Cosmetic; if someone feels strongly about it they can sort it. > > > > ) > > > > > > ECONF_SOURCE="${S}" OPT="" econf "${myeconfargs[@]}" > > > > What is that OPT for? > > It prevents upstream optimization flags from being added to CFLAGS by the > build system. > > > > emake CPPFLAGS= CFLAGS= LDFLAGS= > > > > What is this FLAGS reset for? > > configure populates Makefile variables called CONFIGURE_CFLAGS, > CONFIGURE_CPPFLAGS, and CONFIGURE_LDFLAGS. Forcing the non-configure > variables to empty on the make command line ensure that the flags do not > appear twice when invoking the compiler. So its omission wouldn't really break anything, it's just optimization. > > > > emake DESTDIR="${D}" altinstall > > > > Some special reason why not ED? > > EPREFIX has already been accounted for by passing it to configure. > > Passing ED to emake install is almost always incorrect for an > autotools-based package. So I re-read Prefix docs and now it (hopefully) finally makes sense to me as to what to use where. Initially, I thought that s/(D|ROOT)/E\1/ will take care of everything, but now I see that it's a tedious work requiring a good understanding. > > > > > > # Fix collisions between different slots of Python. > > > rm -f "${ED}usr/$(get_libdir)/libpython3.so" > > > > Is rm somehow excluded from the die() everywhere rule? > > This should probably be looked at. It's possible someone just got lazy about > it. > > > > use threads || rm -fr "${libdir}/multiprocessing" > > > use wininst || rm -f "${libdir}/distutils/command/"wininst-*.exe > > > > Ehmm, yeah, rm is definitely excluded. > > I think this is probably left over from when the ebuild had a "build" use > flag that trumped a lot of the option feature flags. Doing a non-fatal rm -f > was easier than putting use foo || use !build || ... all over the place. > > It could probably be cleaned up at this point. > > > > vars=( PYTHON "${vars[@]}" ) > > > > `vars+=( blah )` is not used because the order matters or what? > > That seems likely. "sure" would be better so that a proper comment could be added. > > > > ln -s "../../../bin/${abiver}" \ > > > "${D}${PYTHON_SCRIPTDIR}/python${pymajor}" || die > > > > What is this trying to point to? > > It's creating a symlink for use by python-exec. Talk to mgorny for more > details; I'm fuzzy on how it works myself. As mgorny is a member of python he should have received a mail when you assigned this bug to the python project, or should I CC him explicitly? > > > chmod +x "${D}${PYTHON_SCRIPTDIR}/python${pymajor}-config" || die > > > > fperms()? > > PYTHON_SCRIPTDIR contains EPREFIX already; calling fperms would duplicate > EPREFIX. That's tricky > > > > > ln -s "python${pymajor}-config" \ > > > "${D}${PYTHON_SCRIPTDIR}/python-config" || die > > > # 2to3, pydoc, pyvenv > > > ln -s "../../../bin/2to3-${PYVER}" \ > > > "${D}${PYTHON_SCRIPTDIR}/2to3" || die > > > ln -s "../../../bin/pydoc${PYVER}" \ > > > "${D}${PYTHON_SCRIPTDIR}/pydoc" || die > > > ln -s "../../../bin/pyvenv-${PYVER}" \ > > > "${D}${PYTHON_SCRIPTDIR}/pyvenv" || die > > > # idle > > > if use tk; then > > > ln -s "../../../bin/idle${PYVER}" \ > > > "${D}${PYTHON_SCRIPTDIR}/idle" || die > > > fi > > > > Some special reason why not ED? > > PYTHON_SCRIPTDIR already contians EPREFIX. > > > > if [[ -z "$(eselect python show --python${PV%%.*})" || ! -f "${EROOT}usr/bin/$(eselect python show --python${PV%%.*})" ]]; then > > > > I have an idea on how to make this line even longer > > Yeah, this is ugly, but it does the job. Suggestions that don't break things > are welcome. \ would only break the long line. --- All in all, thanks for your quick response.
>> Don't be a dick please. > I tried to point out the fact that it's a pure spam, not to mention that it's completely out of place (pkg_setup is for things like this) and even yells using ewarn(). Terrible piece. Then just point it out instead of making a sarcastic/aggressive comment about it. That's quite rude, and does not encourage me to take your contribution seriously. Anyway, I do not anticipate taking any action myself until the next version bump for each python slot. I will keep your suggestions in mind when that time comes.
This is rude, inappropriate and not the first kind of your contribution that does make us unwilling to work with you. The long list of complaints is nowhere close to helpful, especially that it's completely unreadable. Some of your suggestions are correct, most of them are not changing anything or are even wrong. Furthermore, I should point out that providing aggressive critics of someone's code with such a major mistakes as misunderstanding EPREFIX is quite unprofessional. People make mistakes but there is a major difference between attempting to make a useful though mistaken suggestion, and aggressively criticizing correct code. If you'd like to contribute, then please submit a pull request or a patch (via the mailing list, since Bugzilla is not a review platform), changing the ebuilds as appropriate. However, please note that we are not going to accept removing code that was added in the past only because nobody knows why it was exactly necessary. If you'd like to help, please try to figure that out (you've got historical git repository to help you, you don't even have to fight CVS) and check if they are actually unnecessary. That certainly will be helpful. Please also try to provide 'matching' ebuilds for all versions of Python so that it will be possible to semi-reasonably diff between them.
Yeah, now we're on the right track.
(In reply to Michał Górny from comment #4) > This is rude, inappropriate Maybe I could do better next time as I'm still learning what people find offensive (way too many things IMO). > and not the first kind of your contribution Please be more specific. What other contributions offended you? I'll add them to my offensive offence-list. > that does make us unwilling to work with you. On whose behalf do you speak? > The long list of complaints is nowhere close to helpful, It's not my problem that the list is so long. I find it spammy to split it into multiple issues prematurely. > especially that it's completely unreadable. That's an issue of BugZilla. I could only suggest you to c&p it to an editor of your choice to return the readability back to its original form. > Some of your suggestions are correct, most of them are not changing anything or > are even wrong. Please notice that not all sentences, I wrote, are suggestions, some are questions. And please point me to a wrong **suggestion**. > > Furthermore, I should point out that providing aggressive critics of > someone's code with such a major mistakes as misunderstanding EPREFIX is > quite unprofessional. I have never criticized EPREFIX related things, I simply asked what lead to the mix up of D and ED, which looked like a semi-random mix up at first glance. But I've completed my knowledge of EPREFIX since then. The reason why I lived happily without knowing the details is that any EPREFIX code changes (even plain wrong ones) are completely invisible to people not using Prefix project, which is a majority of them. > People make mistakes but there is a major difference > between attempting to make a useful though mistaken suggestion, and > aggressively criticizing correct code. This is a repeat of some of the text above. > If you'd like to contribute, then please submit a pull request or a patch > (via the mailing list, since Bugzilla is not a review platform), changing > the ebuilds as appropriate. Discussion should predate any major changes. I understand that BugZilla is not a review platform, but I'm not aware of any other tool closer to it. GitHub/Gerrit would be amazing if Gentoo used it, but it's a largely abandoned platform for Gentoo people used to use mailing lists. I've tried to use the mailing list in the past weeks, but I've came to the decision that it brings in much more problems than it solves. > > However, please note that we are not going to accept removing code that was > added in the past only because nobody knows why it was exactly necessary. That is scary. > If > you'd like to help, please try to figure that out (you've got historical git > repository to help you, you don't even have to fight CVS) and check if they > are actually unnecessary. That certainly will be helpful. Could you be please be so kind to point me to the "historical git repository"? > > Please also try to provide 'matching' ebuilds for all versions of Python so > that it will be possible to semi-reasonably diff between them.
(In reply to Jan Chren (rindeal) from comment #6) > Maybe I could do better next time as I'm still learning what people find > offensive (way too many things IMO). Next time, please don't start by labeling things "bad smell". I think most of found that somewhat offensive. Then, your first sentence in comment 0 sets the tone of the entire bug by declaring some exceptional quality standard to which you are going to hold the code (and its maintainers). You then proceed to point out what you perceive to be mistakes, be it via suggestions or questions. The assumption being that if you are asking a question you think something is wrong with it. That may not have been your intent, but that's how it was perceived. Frankly, you come across as a bit of a know-it-all who is trying to school us in how to write an ebuild. I recognize that there is some valid feedback there. If you can make some adjustments to the way in which you provide criticism, people will be less likely to misinterpret it.
> Could you be please be so kind to point me to the "historical git repository"? Sure. https://github.com/gentoo/gentoo-gitmig-20150809-draft.git You can also graft it onto the live gentoo.git using instructions on the wiki. https://wiki.gentoo.org/wiki/Gentoo_git_workflow#Grafting_Gentoo_history_onto_the_active_repository
(In reply to Mike Gilbert from comment #7) > (In reply to Jan Chren (rindeal) from comment #6) > > Maybe I could do better next time as I'm still learning what people find > > offensive (way too many things IMO). > > Next time, please don't start by labeling things "bad smell". I think most > of found that somewhat offensive. Ok > Then, your first sentence in comment 0 sets the tone of the entire bug by > declaring some exceptional quality standard to which you are going to hold > the code (and its maintainers). Almost every external contribution is criticized until it's shaped to some form which largely coincides with devmanual and which I call a Gentoo QA standard. Most of the internal contributions, however, do not get any review unless they break something really badly (and that happens much more often than at other distros). What I tried to do by reporting this issue is to get the code inside these ebuilds some attention. > You then proceed to point out what you perceive to be mistakes, be it via > suggestions or questions. The assumption being that if you are asking a > question you think something is wrong with it. That may not have been your > intent, but that's how it was perceived. By asking a question, I state that the code is unclear, not plain wrong. Code that is unclear might be a sign of a <notoffensive>code-smell</notoffensive>, or whatever you want to call it. > > Frankly, you come across as a bit of a know-it-all who is trying to school > us in how to write an ebuild. I recognize that there is some valid feedback > there. If you can make some adjustments to the way in which you provide > criticism, people will be less likely to misinterpret it. Yeah, that is true. I've learned that I have to be very careful about the form so that people can focus on the content. Next time I'll add some rainbows and unicorns. (In reply to Mike Gilbert from comment #8) > > Could you be please be so kind to point me to the "historical git repository"? > > Sure. > > https://github.com/gentoo/gentoo-gitmig-20150809-draft.git > > You can also graft it onto the live gentoo.git using instructions on the > wiki. > > https://wiki.gentoo.org/wiki/ > Gentoo_git_workflow#Grafting_Gentoo_history_onto_the_active_repository Thanks, that will certainly help to unveil the history.
So, what is the official python project stance on this? Keep WONTFIX as "do not cross my lawn" signal, or reopen, or post a new censored issue?
I think the best way to contribute the changes would be to rely on github pull requests as pointed in comment #4, that way it will be easier to review each part and approve/discard them
