From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lists.gentoo.org (pigeon.gentoo.org [208.92.234.80]) by finch.gentoo.org (Postfix) with ESMTP id E2704138A1A for ; Tue, 17 Feb 2015 18:42:45 +0000 (UTC) Received: from pigeon.gentoo.org (localhost [127.0.0.1]) by pigeon.gentoo.org (Postfix) with SMTP id 8DB6EE0850; Tue, 17 Feb 2015 18:42:44 +0000 (UTC) Received: from smtp.gentoo.org (smtp.gentoo.org [140.211.166.183]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by pigeon.gentoo.org (Postfix) with ESMTPS id F0564E07FA for ; Tue, 17 Feb 2015 18:42:43 +0000 (UTC) Received: from big_daddy.dol-sen.ca (S010634bdfa9ecf80.vc.shawcable.net [96.49.31.57]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: dolsen) by smtp.gentoo.org (Postfix) with ESMTPSA id 04F8E34083F for ; Tue, 17 Feb 2015 18:42:42 +0000 (UTC) Date: Tue, 17 Feb 2015 10:42:40 -0800 From: Brian Dolbec To: gentoo-portage-dev@lists.gentoo.org Subject: Re: [gentoo-portage-dev] [PATCH 1/2] Add FEATURES=binpkg-multi-instance (bug 150031) Message-ID: <20150217104240.1db9be28.dolsen@gentoo.org> In-Reply-To: <1424162233-25071-1-git-send-email-zmedico@gentoo.org> References: <1424162233-25071-1-git-send-email-zmedico@gentoo.org> Organization: Gentoo Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-Id: Gentoo Linux mail X-BeenThere: gentoo-portage-dev@lists.gentoo.org Reply-to: gentoo-portage-dev@lists.gentoo.org MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Archives-Salt: c70b3643-f143-4940-becc-23638d7afe5b X-Archives-Hash: 397a05d785776af1ccb1e01dc70a09b5 On Tue, 17 Feb 2015 00:37:12 -0800 Zac Medico wrote: > FEATURES=binpkg-multi-instance causes an integer build-id to be > associated with each binary package instance. Inclusion of the > build-id in the file name of the binary package file makes it > possible to store an arbitrary number of binary packages built from > the same ebuild. > > Having multiple instances is useful for a number of purposes, such as > retaining builds that were built with different USE flags or linked > against different versions of libraries. The location of any > particular package within PKGDIR can be expressed as follows: > > ${PKGDIR}/${CATEGORY}/${PN}/${PF}-${BUILD_ID}.xpak > > The build-id starts at 1 for the first build of a particular ebuild, > and is incremented by 1 for each new build. It is possible to share a > writable PKGDIR over NFS, and locking ensures that each package added > to PKGDIR will have a unique build-id. It is not necessary to migrate > an existing PKGDIR to the new layout, since portage is capable of > working with a mixed PKGDIR layout, where packages using the old > layout are allowed to remain in place. > > The new PKGDIR layout is backward-compatible with binhost clients > running older portage, since the file format is identical, the > per-package PATH attribute in the 'Packages' index directs them to > download the file from the correct URI, and they automatically use > BUILD_TIME metadata to select the latest builds. > > There is currently no automated way to prune old builds from PKGDIR, > although it is possible to remove packages manually, and then run > 'emaint --fix binhost' to update the ${PKGDIR}/Packages index. > > It is not necessary to migrate an existing PKGDIR to the new layout, > since portage is capable of working with a mixed PKGDIR layout, where > packages using the old layout are allowed to remain in-place. > > There is currently no automated way to prune old builds from PKGDIR, > although it is possible to remove packages manually, and then run > 'emaint --fix binhost' update the ${PKGDIR}/Packages index. Support > for FEATURES=binpkg-multi-instance is planned for eclean-pkg. > > X-Gentoo-Bug: 150031 > X-Gentoo-Bug-URL: https://bugs.gentoo.org/show_bug.cgi?id=150031 > --- > bin/quickpkg | 1 - > man/make.conf.5 | 27 + > pym/_emerge/Binpkg.py | 33 +- > pym/_emerge/BinpkgFetcher.py | 13 +- > pym/_emerge/BinpkgVerifier.py | 6 +- > pym/_emerge/EbuildBinpkg.py | 9 +- > pym/_emerge/EbuildBuild.py | 36 +- > pym/_emerge/Package.py | 67 +- > pym/_emerge/Scheduler.py | 6 +- > pym/_emerge/clear_caches.py | 1 - > pym/_emerge/resolver/output.py | 21 +- > pym/portage/const.py | 2 + > pym/portage/dbapi/__init__.py | 10 +- > pym/portage/dbapi/bintree.py | 842 > +++++++++++---------- > pym/portage/dbapi/vartree.py | 8 +- > pym/portage/dbapi/virtual.py | 113 ++- > pym/portage/emaint/modules/binhost/binhost.py | 47 +- > pym/portage/package/ebuild/config.py | 3 +- > pym/portage/tests/resolver/ResolverPlayground.py | 26 > +- .../resolver/binpkg_multi_instance/__init__.py | 2 > + .../resolver/binpkg_multi_instance/__test__.py | 2 > + .../binpkg_multi_instance/test_rebuilt_binaries.py | 101 +++ > pym/portage/versions.py | 48 +- 23 files > changed, 932 insertions(+), 492 deletions(-) create mode 100644 > > overall, there is no way I know the code well enough to know if you screwed up. But the code looks decent, so... My only questions are: pym/portage/dbapi/bintree.py: You removed several functions from the binarytree class and essentially reduced prevent_collision to a warning message. Can you briefly say why they are not needed please. _populate() ==> it is some 500+ LOC nasty. From what I can see I think you added slightly more loc than you deleted/changed. While I don't expect a breakup of this function to be directly a part of this commit. IT IS BADLY NEEDED. I'd like to see this function properly split into pieces and act as a driver for the smaller tasks. Currently it is a mess of nested if/else, for,... that is extremely difficult to keep straight and make logic changes to. It's almost as bad as repomans 1000+ LOC main loop. Why "if TRUE:" <== just fix the indent... At the same time, I noticed _ensure_dirs, _file_permissions functions defined in that class. Shouldn't these just be util functions available everywhere. pym/portage/versions.py class _pkg_str(_unicode) in __init__() if build_time is not None: try: build_time = long(build_time) except ValueError: if build_time: build_time = -1 else: build_time = 0 if build_id is not None: try: build_id = long(build_id) except ValueError: if build_id: build_id = -1 else: build_id = None if file_size is not None: try: file_size = long(file_size) except ValueError: if file_size: file_size = -1 else: file_size = None if mtime is not None: try: mtime = long(mtime) except ValueError: if mtime: mtime = -1 else: mtime = None I hate repeating code. This can be done using one universal small function and will clean up the __init__() @static_method def _long(var, default): if var is not None: try: var = long(var) except ValueError: if var: var = -1 else: var = default return var -- Brian Dolbec