Re: [gentoo-dev] [OT] pkgcore bikeshed (was Portage team)
On Monday 13 January 2014 09:53:45 Tom Wijsman wrote: On Mon, 13 Jan 2014 16:15:37 +0700 C. Bergström wrote: At the end of the day we have one codebase which is engineered and another which has evolved. Too broad generalization, too much assumption; both can be held as meaning nothing compared to what engineered and evolved could really be, but as with doing that, it gets a subjective nature. In other words, the lack of context makes this statement meaningless. anyone who has spent serious time in the portage code base knows it sucks. i'm not blaming anyone -- it's no one's fault. portage started as prototyped idea that has since had more and more stuff piled onto it over the years by each successive maintainer. devs i've talked to agree that it sucks to work with. it's why pkgcore was born in the first place. i'd like to see portage pkgcore merge, but it'd take quite a bit of work on the portage side to migrate step by step. we generally haven't had leads who have enough time sorting out the existing bugs/feature requests to try and also restructure/reshape things. maybe by trying to get new interest in the project means we can find some people willing to rip off some sizable chunks. the fact that the public API is pretty much non-existent is nice because it means we're free to change/break whatever we want. note though that the let's rewrite everything in a branch and then merge later approach doesn't work. it's been done a few times in portage land and aborted each time. it's rare for this to work for other projects either. small steps are much easier to review/merge. -mike signature.asc Description: This is a digitally signed message part.
Re: [gentoo-dev] rfc: revisiting our stabilization policy
On Friday 17 January 2014 02:02:51 gro...@gentoo.org wrote: Maybe, a good solution is to introduce a special arch, noarch, for such packages (similar to what's done in the rpm world). Then, if a package is ~noarch, it is automatically considered ~arch for all arches. Similar for stable. The maintainer should be able to keyword ~noarch and to stabilize noarch. Comments? you mean * ? this already works today (at least with portage): KEYWORDS=~* KEYWORDS=* in fact, i was planning on converting Chromium OS over to use this instead of a list of arches. but that's because we run a simpler system of there really only being two sets of ebuilds in the tree -- stable for all and unstable for all. for the ebuilds that are truly arch-specific (or otherwise need restricting), then we'll do: KEYWORDS=-* ~arm -mike signature.asc Description: This is a digitally signed message part.
Add a KEYWORD representing any arch (was: Re: [gentoo-dev] rfc: revisiting our stabilization policy)
El dom, 19-01-2014 a las 03:36 -0500, Mike Frysinger escribió: On Friday 17 January 2014 02:02:51 gro...@gentoo.org wrote: Maybe, a good solution is to introduce a special arch, noarch, for such packages (similar to what's done in the rpm world). Then, if a package is ~noarch, it is automatically considered ~arch for all arches. Similar for stable. The maintainer should be able to keyword ~noarch and to stabilize noarch. Comments? you mean * ? this already works today (at least with portage): KEYWORDS=~* KEYWORDS=* in fact, i was planning on converting Chromium OS over to use this instead of a list of arches. but that's because we run a simpler system of there really only being two sets of ebuilds in the tree -- stable for all and unstable for all. for the ebuilds that are truly arch-specific (or otherwise need restricting), then we'll do: KEYWORDS=-* ~arm -mike I had no idea that existed :O, I guess something related with specification is missing? :/
[gentoo-dev] Re: Add a KEYWORD representing any arch
On Sun, 19 Jan 2014, Pacho Ramos wrote: El dom, 19-01-2014 a las 03:36 -0500, Mike Frysinger escribió: you mean * ? this already works today (at least with portage): KEYWORDS=~* KEYWORDS=* Currently not allowed by policy: http://devmanual.gentoo.org/keywording/index.html I had no idea that existed :O, I guess something related with specification is missing? :/ Now what problem are we trying to solve? As I see it, it is mainly one of manpower, namely that some arch teams cannot keep up with stable requests, and I doubt that any technical solution will help to solve this. Introducing a noarch keyword or allowing * will potentially cause problems with dependency resolution. Instead, we should come up with a clear set of rules under what circumstances package maintainers are allowed to stabilise ebuilds themselves on all architectures. Ulrich pgpiU02gcnyG5.pgp Description: PGP signature
Re: Add a KEYWORD representing any arch (was: Re: [gentoo-dev] rfc: revisiting our stabilization policy)
On Sunday 19 January 2014 04:28:33 Pacho Ramos wrote: El dom, 19-01-2014 a las 03:36 -0500, Mike Frysinger escribió: On Friday 17 January 2014 02:02:51 gro...@gentoo.org wrote: Maybe, a good solution is to introduce a special arch, noarch, for such packages (similar to what's done in the rpm world). Then, if a package is ~noarch, it is automatically considered ~arch for all arches. Similar for stable. The maintainer should be able to keyword ~noarch and to stabilize noarch. Comments? you mean * ? this already works today (at least with portage): KEYWORDS=~* KEYWORDS=* in fact, i was planning on converting Chromium OS over to use this instead of a list of arches. but that's because we run a simpler system of there really only being two sets of ebuilds in the tree -- stable for all and unstable for all. for the ebuilds that are truly arch-specific (or otherwise need restricting), then we'll do: KEYWORDS=-* ~arm I had no idea that existed :O, I guess something related with specification is missing? :/ specs are for chumps! although PMS documents -* already, so * and ~* is a logical extension. i suspect on the portage side the history is related to package.keywords support. but i'm just guessing. -mike signature.asc Description: This is a digitally signed message part.
Re: [gentoo-dev] Last rites: sys-apps/usleep
Dion Moult mo...@gentoo.org writes: # Dion Moult mo...@gentoo.org (19 Jan 2014) # Mask for removal in 30 days. usleep is actually provided part of # app-admin/killproc (bug #467212) sys-apps/usleep Veto. app-admin/killproc isn't even keyworded, yet. (Bug #494254). -- Amadeusz Żołnowski pgpv1V6GgHkQg.pgp Description: PGP signature
Re: [gentoo-dev] Last rites: sys-apps/usleep
Reverted. Amadeusz Żołnowski aide...@gentoo.org wrote: Dion Moult mo...@gentoo.org writes: # Dion Moult mo...@gentoo.org (19 Jan 2014) # Mask for removal in 30 days. usleep is actually provided part of # app-admin/killproc (bug #467212) sys-apps/usleep Veto. app-admin/killproc isn't even keyworded, yet. (Bug #494254). -- Amadeusz Żołnowski
Re: [gentoo-dev] Re: Add a KEYWORD representing any arch
El dom, 19-01-2014 a las 10:46 +0100, Ulrich Mueller escribió: On Sun, 19 Jan 2014, Pacho Ramos wrote: El dom, 19-01-2014 a las 03:36 -0500, Mike Frysinger escribió: you mean * ? this already works today (at least with portage): KEYWORDS=~* KEYWORDS=* Currently not allowed by policy: http://devmanual.gentoo.org/keywording/index.html I had no idea that existed :O, I guess something related with specification is missing? :/ Now what problem are we trying to solve? As I see it, it is mainly one of manpower, namely that some arch teams cannot keep up with stable requests, and I doubt that any technical solution will help to solve this. Introducing a noarch keyword or allowing * will potentially cause problems with dependency resolution. Instead, we should come up with a clear set of rules under what circumstances package maintainers are allowed to stabilise ebuilds themselves on all architectures. Ulrich Yeah, the problem is manpower and, then, we are thinking in cases like wallpapers, changes in the installation of some files (that are not arch specific)... But, how to indicate a concrete package can be handled in this special noarch way? It's easy for some cases like I posted, but there are others that are more difficult to handle (perl modules for example?) If we could agree on the kind of packages we could handle in this way (stabilizing for all arches) would be nice
[gentoo-dev] glibc-2.18 heading to ~arch in next ~week
with glibc-2.17 in stable now and glibc-2.19 release in like ~2 weeks, glibc 2.18 is heading to ~arch. there's been very little reported breakage reported thus far ... i hope it's because there isn't any vs people aren't using it. so if people want to try it out ahead of time, that'd be nice. -mike signature.asc Description: This is a digitally signed message part.
Re: [gentoo-dev] rfc: revisiting our stabilization policy
William Hubbs schrieb: When you say drop keywords do you mean: 1) revert the old version back to ~arch or 2) remove the old version. As a maintainer, I would rather do 2, because I do not want to backport fixes to the old version. William With 1) users would still be using newer versions with ~arch keyword except with explicit mask on newer versions, so keeping the old versions doesnt make much sense. With 2), there may be additional one-time cost for the maintainer (since he should check with reserve dependencies first to avoid broken dependency trees), but afterwards this solution should mean an adjusted amount of stable packages for each arch and no permanent additional work for the maintainer. -- Thomas Sachau Gentoo Linux Developer signature.asc Description: OpenPGP digital signature
[gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in dev-lang/ocaml/files: ocaml-4.01.0-pkg-config-ncurses.patch ocaml-3.11.2-Fix-ocamlopt-w.r.t.-binutils-2.21.patch
please check with maintainers, ie in this case me, before jumping in and pushing, straight to stable, patches to the known to be fragile core part of a whole herd... reviews below On Sat, 18 Jan 2014 13:12:48 + (UTC) Mark Wright (gienah) gie...@gentoo.org wrote: gienah 14/01/18 13:12:48 Added:ocaml-4.01.0-pkg-config-ncurses.patch ocaml-3.11.2-Fix-ocamlopt-w.r.t.-binutils-2.21.patch in case you hadn't noticed there is a patchset held in gentoo cvs repo, please remove those files and make them fit in the workflow Log: Fix bug #459512 - dev-lang/ocaml with sys-libs/ncurses[tinfo] - .../work/ocaml-4.00.1/byterun/terminfo.c:54: undefined reference to 'tgetent', thanks to Reinis Danne for reporting (patch by me). Add virtual/pkg-config to DEPEND as the ocaml configure script calls pkg-config. upstream link ? patch lgtm but i would have liked to have time to have a word to say... I doubt that this tinfo thing has suddenly become ultra urgent Apply patch from upstream for upstream bug http://caml.inria.fr/mantis/view.php?id=5237 to ocaml-3.11.2. ocaml 3.11 is still there only because of a slacking arch and has most likely other issues, you're wasting your time there :)
Re: [gentoo-dev] rfc: formally allow qa to suspend commit rights
either the QA lead or two members of the QA team can require the Infra team to temporarily suspend commit access for the developer -1 to that part That sounds like you are able to make non-trivial decisions without the approval of the lead.
Re: [gentoo-dev] glibc-2.18 heading to ~arch in next ~week
On Sun, Jan 19, 2014 at 05:52:48AM -0500, Mike Frysinger wrote: with glibc-2.17 in stable now and glibc-2.19 release in like ~2 weeks, glibc 2.18 is heading to ~arch. there's been very little reported breakage reported thus far ... i hope it's because there isn't any vs people aren't using it. so if people want to try it out ahead of time, that'd be nice. What ever happened with the rpc/libtirpc changes? Will packages that used RPC headers from glibc need more changes, and if so, what? -- Robin Hugh Johnson Gentoo Linux: Developer, Infrastructure Lead E-Mail : robb...@gentoo.org GnuPG FP : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85
Re: [gentoo-dev] rfc: formally allow qa to suspend commit rights
On Sat, Jan 18, 2014 at 10:02 PM, William Hubbs willi...@gentoo.org wrote: This is nothing new; the qa team has requested that commit rights be suspended before. I am just proposing that we actually add the parts of the old patch to the glep that spell out when and how this can happen. Thoughts? Yes, thoughts, absolutely. Asking for QA to be at the same time judge, party and executioner. Need I say more? Denis.
Re: [gentoo-dev] rfc: revisiting our stabilization policy
On Thu, 16 Jan 2014 23:44:42 +0100 Tom Wijsman tom...@gentoo.org wrote: If I don’t, why do I care if the package is a year old? I lose none of my time to use the old version, since it does all I want; This is under the assumption that the old version has no further implications, which is a false assumption; because the older a stable ebuild get, the higher the chance is that it becomes incompatible with its libraries and/or causes blockers. Even further, a security bug for an old version of a library it depends on could cause its removal. Right, of course things can become incompatible—but the distro handles that by either leaving old enough version of e.g. libraries around that the latest stable versions of their reverse dependencies don’t break, or, in exceptional cases (e.g. security), by breaking things intentionally if necessary, thus telling me that there’s a problem. I lose a nonzero amount of time if I get a version which breaks things (even if the only time I lose is the time it takes to downgrade), Depends on whether the stable version is as perfect as one thinks it is; an upgrade can go two ways, it improves or regresses. (Well, three ways as it can stay the same, but that wouldn't demonstrate the point) Well, yes. I was considering the case of a stable version that does work well. If the stable version has a bug that affects me, I won’t be using it—I’ll pull in an unstable version that fixes the bug, and then get back to stable ASAP after that. so it’s in my best interest to use the stable versions of such packages, even if they’re a month, a year, or three years old. Based on what you know, what you need and that you can resist change; yes, but this doesn't take into account what you don't know, you don't know you need and the improvements that change can bring. While it doesn't happen often, some people will say if I knew this earlier, I would have already upgraded a long while ago; either because the new version brings something good, or the old version has a regression they were not aware of yet or came due to incompatibility... Sure, it was wrong to say it takes *no* time, but I think less time than sticking to the bleeding edge and having things break from time to time. My experience with stable has been that bugs (at least bugs that I encounter) are rare and, most importantly, bugs are *very* rare in the important packages that are hard to fix (glibc, openrc, gentoo-sources, openssh for servers, etc.). I understand they’re fairly rare in unstable as well, but I would expect a bit more common than in stable. If stable really is falling behind and the backlog is always growing, obviously something has to be done. I just don’t want “something” to mean “don’t have a stable tree”. The stable tree provides me with a benefit. If standards have to slip a bit to maintain timeliness, then I’d prefer a stable tree that’s as stable as practical, accepting reality—perhaps where users are able to submit reports of working packages, or where we let platform-agnostic packages be stabilized after one arch has tested, or various of the other suggestions in this thread. Just not no stable tree at all. -- Christopher Head signature.asc Description: PGP signature
Re: [gentoo-dev] RFC: Hosting daily gx86 squashfs images and deltas
On Fri, 17 Jan 2014 19:53:44 +0100 Michał Górny mgo...@gentoo.org wrote: Dnia 2014-01-17, o godz. 10:18:58 Alec Warner anta...@gentoo.org napisał(a): On Fri, Jan 17, 2014 at 8:27 AM, Michał Górny mgo...@gentoo.org wrote: Hello, all. I'm using squashfs to hold my Gentoo repositories on all of my systems for some time. As you probably know, this allows me to save space while keeping portage fast. However, it makes updating the tree quite burdensome and time-consuming. Yes, full metadata with md5-cache. That's the same thing you get via 'emerge --sync'. And that's why the deltas are so big -- I recall three big cache updates this week. I would absolutely use this on my machines. -- Christopher Head signature.asc Description: PGP signature
[gentoo-dev] Automated Package Removal and Addition Tracker, for the week ending 2014-01-19 23h59 UTC
The attached list notes all of the packages that were added or removed from the tree, for the week ending 2014-01-19 23h59 UTC. Removals: app-office/rabbit 2014-01-13 02:33:48 mrueg app-i18n/rskkserv 2014-01-13 02:35:05 mrueg dev-ruby/postgres 2014-01-13 02:36:45 mrueg dev-ruby/radiant2014-01-17 22:20:31 mrueg dev-ruby/actionwebservice 2014-01-18 09:56:05 graaff dev-ruby/gettext_activerecord 2014-01-18 09:57:48 graaff dev-ruby/gettext_rails 2014-01-18 09:57:58 graaff Additions: sci-libs/chemkit2014-01-13 14:26:00 jlec dev-libs/rapidxml 2014-01-13 17:42:49 jlec dev-java/cal10n 2014-01-13 18:03:51 ercpe dev-java/slf4j-ext 2014-01-13 18:13:18 ercpe sci-libs/lemon 2014-01-13 18:15:25 jlec sci-libs/coinor-sample 2014-01-14 17:48:58 bicatali sci-libs/coinor-utils 2014-01-14 17:51:59 bicatali sci-libs/coinor-osi 2014-01-14 17:58:51 bicatali sci-libs/coinor-vol 2014-01-14 18:00:42 bicatali sci-libs/coinor-dylp2014-01-14 18:03:16 bicatali sci-libs/scalapack 2014-01-14 18:40:05 bicatali sci-libs/mumps 2014-01-14 18:47:26 bicatali sci-libs/coinor-clp 2014-01-14 18:52:06 bicatali sci-libs/coinor-cgl 2014-01-14 18:54:51 bicatali sci-libs/coinor-cbc 2014-01-14 18:58:08 bicatali sci-libs/coinor-alps2014-01-14 19:01:58 bicatali sci-libs/coinor-netlib 2014-01-14 19:34:46 bicatali sci-libs/coinor-bcp 2014-01-14 21:47:05 bicatali sci-libs/coinor-bcps2014-01-14 21:49:28 bicatali sci-libs/coinor-blis2014-01-14 21:51:36 bicatali sci-libs/coinor-csdp2014-01-14 21:54:44 bicatali sci-libs/coinor-dip 2014-01-14 21:56:48 bicatali sci-libs/coinor-flopcpp 2014-01-14 21:59:55 bicatali sci-libs/coinor-mp 2014-01-14 22:02:05 bicatali sci-libs/coinor-smi 2014-01-14 22:06:00 bicatali sci-libs/coinor-symphony2014-01-14 22:09:00 bicatali sci-libs/coinor-bonmin 2014-01-15 19:31:56 bicatali sci-libs/coinor-couenne 2014-01-15 19:40:37 bicatali sci-libs/coinhsl2014-01-15 19:44:11 bicatali sci-libs/ipopt 2014-01-15 19:45:36 bicatali sci-libs/coinor-cppad 2014-01-15 19:47:45 bicatali sci-libs/coinor-os 2014-01-15 20:11:02 bicatali sci-libs/avogadrolibs 2014-01-16 18:47:35 jlec sys-cluster/libcircle 2014-01-18 02:23:17 ottxor app-emulation/armv8-fast-model 2014-01-18 09:21:49 vapier dev-db/lmdb 2014-01-18 13:46:09 eras www-client/google-chrome-beta 2014-01-19 00:30:15 floppym www-client/google-chrome-unstable 2014-01-19 00:30:38 floppym dev-ml/pa_bench 2014-01-19 13:53:06 aballier dev-ml/typerep 2014-01-19 13:57:44 aballier dev-ml/pa_test 2014-01-19 15:34:42 aballier dev-ml/re2 2014-01-19 16:06:19 aballier dev-ml/async_kernel 2014-01-19 16:15:33 aballier dev-ml/faillib 2014-01-19 16:29:46 aballier sec-policy/selinux-cachefilesd 2014-01-19 19:23:44 swift -- Robin Hugh Johnson Gentoo Linux Developer E-Mail : robb...@gentoo.org GnuPG FP : 11AC BA4F 4778 E3F6 E4ED F38E B27B 944E 3488 4E85 Removed Packages: app-office/rabbit,removed,mrueg,2014-01-13 02:33:48 app-i18n/rskkserv,removed,mrueg,2014-01-13 02:35:05 dev-ruby/postgres,removed,mrueg,2014-01-13 02:36:45 dev-ruby/radiant,removed,mrueg,2014-01-17 22:20:31 dev-ruby/actionwebservice,removed,graaff,2014-01-18 09:56:05 dev-ruby/gettext_activerecord,removed,graaff,2014-01-18 09:57:48 dev-ruby/gettext_rails,removed,graaff,2014-01-18 09:57:58 Added Packages: sci-libs/chemkit,added,jlec,2014-01-13 14:26:00 dev-libs/rapidxml,added,jlec,2014-01-13 17:42:49 dev-java/cal10n,added,ercpe,2014-01-13 18:03:51 dev-java/slf4j-ext,added,ercpe,2014-01-13 18:13:18 sci-libs/lemon,added,jlec,2014-01-13 18:15:25 sci-libs/coinor-sample,added,bicatali,2014-01-14 17:48:58 sci-libs/coinor-utils,added,bicatali,2014-01-14 17:51:59 sci-libs/coinor-osi,added,bicatali,2014-01-14 17:58:51 sci-libs/coinor-vol,added,bicatali,2014-01-14 18:00:42 sci-libs/coinor-dylp,added,bicatali,2014-01-14 18:03:16
Re: [gentoo-dev] rfc: revisiting our stabilization policy
On Sun, 19 Jan 2014 14:31:57 -0800 Christopher Head ch...@chead.ca wrote: Right, of course things can become incompatible—but the distro handles that by either leaving old enough version of e.g. libraries around that the latest stable versions of their reverse dependencies don’t break, or, in exceptional cases (e.g. security), by breaking things intentionally if necessary, thus telling me that there’s a problem. True, note that upper limits on the dependencies (cat/pkg-ver) or similar blockers are not always in place; which can make this problematic if the maintainer doesn't catch the incompatible regression, especially since a lot of us run testing and don't look after older or stable packages as much as we would want. If stable really is falling behind and the backlog is always growing, obviously something has to be done. I just don’t want “something” to mean “don’t have a stable tree”. The stable tree provides me with a benefit. If standards have to slip a bit to maintain timeliness, then I’d prefer a stable tree that’s as stable as practical, accepting reality—perhaps where users are able to submit reports of working packages, or where we let platform-agnostic packages be stabilized after one arch has tested, or various of the other suggestions in this thread. Just not no stable tree at all. +1 as long as we can find effort and ways to keep it around. -- With kind regards, Tom Wijsman (TomWij) Gentoo Developer E-mail address : tom...@gentoo.org GPG Public Key : 6D34E57D GPG Fingerprint : C165 AF18 AB4C 400B C3D2 ABF0 95B2 1FCD 6D34 E57D signature.asc Description: PGP signature
Re: [gentoo-dev] rfc: formally allow qa to suspend commit rights
On Sun, 19 Jan 2014 13:22:07 -0700 Denis Dupeyron calc...@gentoo.org wrote: On Sat, Jan 18, 2014 at 10:02 PM, William Hubbs willi...@gentoo.org wrote: This is nothing new; the qa team has requested that commit rights be suspended before. I am just proposing that we actually add the parts of the old patch to the glep that spell out when and how this can happen. Thoughts? Yes, thoughts, absolutely. Asking for QA to be at the same time judge, party and executioner. Need I say more? That is under the assumption that such suspension is permanent as well as that QA is the only judge. However, most mentioned suspensions there are temporary and QA needs to bring forward reasoning as to why QA has requested the temporary suspension; the final judge here is the Gentoo Council just like with ComRel's suspensions. QA really is just a party here and has nearly no final power when it comes to judging or execution; the goal here is to deal with the rather unusual bigger breakages, but if this doesn't go through QA can just forward the request to ComRel and have them consider and do it. This patch just came up by a hypothetical discussion where QA was given the impression that QA has the power to request this; some of the Council meetings back in history seem to approve this patch, others do not. It's a rather odd history, and hence we set things straight here. It is more of a Do we want QA to delegate this through ComRel or not?. -- With kind regards, Tom Wijsman (TomWij) Gentoo Developer E-mail address : tom...@gentoo.org GPG Public Key : 6D34E57D GPG Fingerprint : C165 AF18 AB4C 400B C3D2 ABF0 95B2 1FCD 6D34 E57D signature.asc Description: PGP signature
Re: [gentoo-dev] rfc: formally allow qa to suspend commit rights
On Sun, 19 Jan 2014 13:46:01 +0100 hasufell hasuf...@gentoo.org wrote: either the QA lead or two members of the QA team can require the Infra team to temporarily suspend commit access for the developer -1 to that part That sounds like you are able to make non-trivial decisions without the approval of the lead. For non-trivial decisions, two members is indeed a bit low; it is probably made in the time of a smaller team, as increasing the amount would beat its purpose (for in case lead is absent) it sounds like or two members of the QA team should just be removed. (For trivial decisions, this is in place to deal with extreme breakage.) -- With kind regards, Tom Wijsman (TomWij) Gentoo Developer E-mail address : tom...@gentoo.org GPG Public Key : 6D34E57D GPG Fingerprint : C165 AF18 AB4C 400B C3D2 ABF0 95B2 1FCD 6D34 E57D signature.asc Description: PGP signature
Re: [gentoo-dev] rfc: formally allow qa to suspend commit rights
On Sun, Jan 19, 2014 at 6:01 PM, Tom Wijsman tom...@gentoo.org wrote: It is more of a Do we want QA to delegate this through ComRel or not?. Actually, no. What it is is a Subject was thoroughly discussed in the past, and a decision was made. More than once, in fact. What basis do you have that would warrant more bilkeshedding on this subject? It may sound crazy, but it isn't entirely impossible that decisions made in the past were not made lightly. It's also not entirely impossible that one of the reasons such decisions are made is so that people can stop rehashing the same topics over and over again and focus on more useful and fun topics. Denis.
Re: [gentoo-dev] rfc: formally allow qa to suspend commit rights
On Sat, Jan 18, 2014 at 9:02 PM, William Hubbs willi...@gentoo.org wrote: All, I would like to bring back for discussion an old patch to glep 48 [1] which was suggested by Jorge [2]. That patch evolved into this one [3], and in the council meeting back then [4], parts of it made their way into glep 48, but the rest seemed to be forgotten. Attached you will find an updated version of this patch taking into account the current version of glep 48. This is nothing new; the qa team has requested that commit rights be suspended before. I am just proposing that we actually add the parts of the old patch to the glep that spell out when and how this can happen. Thoughts? We almost never suspend commit rights. I'm not really finding a situation where this is necessary. Certainly not in the streamlined fashion proposed here. -A William [1] http://wiki.gentoo.org/wiki/GLEP:48 [2] http://archives.gentoo.org/gentoo-project/msg_ac161677a6e06a8647e16420eeae8d47.xml [3] http://www.mail-archive.com/gentoo-qa@lists.gentoo.org/msg00144.html [4] http://www.gentoo.org/proj/en/council/meeting-logs/20110608-summary.txt
Re: [gentoo-dev] rfc: formally allow qa to suspend commit rights
On Sun, 19 Jan 2014 18:22:39 -0700 Denis Dupeyron calc...@gentoo.org wrote: On Sun, Jan 19, 2014 at 6:01 PM, Tom Wijsman tom...@gentoo.org wrote: It is more of a Do we want QA to delegate this through ComRel or not?. Actually, no. What it is is a Subject was thoroughly discussed in the past, and a decision was made. More than once, in fact. What basis do you have that would warrant more bilkeshedding on this subject? The basis that it has once been accepted as well as another time invited more discussion, clearly indicates that it needs further bikeshedding: http://www.gentoo.org/proj/en/council/meeting-logs/20110308-summary.txt * GLEP 48 (QA) After a long discussion and a review of the final proposal text, the result is the following: - vote: in favor: scarabeus, ferringb, wired, jmbsvicetto didn't state (abstain): betelgeuse, patrick, a3li - Given the result, the GLEP update is accepted and can proceed, albeit Peteri raised a question how Devrel is going to work out the resolution after the process is handled over from QA. It was agreed that the part of the text (last sentence of the diff) will be updated with string based on what those two teams agree with without more council involvment (unless required otherwise).. http://www.gentoo.org/proj/en/council/meeting-logs/20110608-summary.txt * GLEP48 review Jorge submitted a proposal to the ml to update GLEP48[4]. After some initial debate over the power granted to the QA team, the timeline in case of an escalation to DevRel, the relation with DevRel and whether QA should only enforce policies or also take part in creating policies, after the request by Patrick, Jorge - suggested pushing this back to the mls. Petteri then asked the council to at least vote to commit the non suspension related parts of the proposal. The diff[5] was approved with 6 yes votes. Alec during this discussion presented some thoughts about the QA team[6]. [4] - http://archives.gentoo.org/gentoo-project/msg_ac161677a6e06a8647e16420eeae8d47.xml [5] - http://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo/xml/htdocs/proj/en/glep/glep-0048.txt?r1=1.3r2=1.4 [6] - http://pastebin.com/C1jGF1DJ It may sound crazy, but it isn't entirely impossible that decisions made in the past were not made lightly. This assumes that the decisions have voted against the matter; however, they voted for this matter on the basis of a small change to be made to it (20110308-summary.txt) but that never happened and seems forgotten. Some developers even refer to Diego having used this power in the past. It's also not entirely impossible that one of the reasons such decisions are made is so that people can stop rehashing the same topics over and over again and focus on more useful and fun topics. This assumes the topic to be useless or boring; however, that's personal opinion and there is an useful need for this from the QA, Council and ComRel perspective. Sometimes we need to deal with a more serious topic. This is one of those days. -- With kind regards, Tom Wijsman (TomWij) Gentoo Developer E-mail address : tom...@gentoo.org GPG Public Key : 6D34E57D GPG Fingerprint : C165 AF18 AB4C 400B C3D2 ABF0 95B2 1FCD 6D34 E57D signature.asc Description: PGP signature
Re: [gentoo-dev] rfc: formally allow qa to suspend commit rights
On Sun, 19 Jan 2014 17:24:30 -0800 Alec Warner anta...@gentoo.org wrote: We almost never suspend commit rights. I'm not really finding a situation where this is necessary. Certainly not in the streamlined fashion proposed here. Well, the QA team has been inactive for a while; so, I guess this might have been a while back. We for example have: #gentoo-qa | * WilliamH has seen flameeys ask infra to lock people out for a time before But also: #gentoo-qa | @hwoarang: pretty sure diego had the powerzz to suspend people Whether this has actually happened is something that is questionable; but yes, one would have to agree that this was definitely streamlined. The idea was on people's minds, probably because that patch was accepted in one of the Council meetings; but streamlining of it never happened. Hence the topic is brought up again here to make a decision final. -- With kind regards, Tom Wijsman (TomWij) Gentoo Developer E-mail address : tom...@gentoo.org GPG Public Key : 6D34E57D GPG Fingerprint : C165 AF18 AB4C 400B C3D2 ABF0 95B2 1FCD 6D34 E57D signature.asc Description: PGP signature
Re: [gentoo-dev] glibc-2.18 heading to ~arch in next ~week
On Sunday 19 January 2014 13:07:24 Robin H. Johnson wrote: On Sun, Jan 19, 2014 at 05:52:48AM -0500, Mike Frysinger wrote: with glibc-2.17 in stable now and glibc-2.19 release in like ~2 weeks, glibc 2.18 is heading to ~arch. there's been very little reported breakage reported thus far ... i hope it's because there isn't any vs people aren't using it. so if people want to try it out ahead of time, that'd be nice. What ever happened with the rpc/libtirpc changes? unfortunately libtirpc isn't feature complete with glibc. so upstream glibc reverted the rpc removal and made it into a configure time flag which Gentoo has enabled (i.e. we ship rpc headers/linkable symbols). which is why the bug onslaught died down. this doesn't preclude anyone from switching their package today to libtirpc; there are a good number of packages where the functionality it provides is sufficient. in fact, you generally want to do that: - you'll get IPv6 support (which glibc refuses to add) - it'll make packages work on other C libraries (like uClibc) - will let us stop enabling rpc functionality in glibc (we want) - other extensions being added to libtirpc? we still have the tracker open: https://bugs.gentoo.org/381391 Will packages that used RPC headers from glibc need more changes, and if so, what? the idea was to make libtirpc have general feature parity with glibc. there's very few people working on this upstream though (the few maintainers are common NFS peeps so they tend to focus more on that). still, some functionality is being added slowly. -mike signature.asc Description: This is a digitally signed message part.
[gentoo-dev] ekeyword written in python from scratch
i finally got annoyed with the perl version (and its output and bugs and limitations) and wrote a new version. it's fairly modular (and has pretty good unittest coverage!?), so if we wanted to look at integrating it into portage or other tools, that should be pretty easy now. at any rate, if other devs who use this want to give it a crack, that'd be great. iron out bugs before the next release. http://git.overlays.gentoo.org/gitweb/?p=proj/gentoolkit.git;a=blob_plain;f=src/ekeyword/ekeyword.py;hb=gentoolkit- dev -mike signature.asc Description: This is a digitally signed message part.
[gentoo-dev] updated s390 VMs
the awesome folks at Marist College in conjunction with the Linux Foundation have upgraded our s390 VMs to newer hardware. faster/more CPUs, more RAM, and more disk space. it's at the point where we aren't hurting to simply build packages. so if devs are interested in ssh access, feel free to ask s390@. -mike signature.asc Description: This is a digitally signed message part.
[gentoo-dev] new profiles.desc header documenting profile/keyword policy
this has all been fairly ad-hoc in the past, so formalize it in the one place that impacts everyone -- profiles.desc. -mike ### # This is a list of valid profiles for each architecture. This file is used by # repoman when doing a repoman scan or repoman full. # # DO NOT ADD PROFILES WITH A die or exit IN THEM OR IT KILLS REPOMAN. # # Policy statement; updates should be posted to gentoo-dev mailing list. # # The meaning of the status field: # stable - Fully supported profile; needs dedicated arch team to keep up with #stabilization (if using stable KEYWORDS) and keyword requests. # dev- Ideally on track to becoming stable; KEYWORDS breakage should be #kept to a minimum and bug reports are recommended. # exp- Developers may ignore breakage in these profiles. It is up to the #profile maintainer to keep things viable, but it is not a hard #requirement that the deptree stay error free. # Stable keywords may be used with any of these types. If an arch only has exp # profiles, then other developers may ignore that keyword when doing upgrades # and cleaning out old versions (dropping of stable/unstable KEYWORDS). # # Note: Please do not mix tabs spaces. Look at surrounding lines first. # # File Layout (for exact format, see the portage(5) man page): #arch profile_directory status signature.asc Description: This is a digitally signed message part.
Re: [gentoo-portage-dev] [PATCH 1/3] emerge: Deprecate --autounmask
El dom, 19-01-2014 a las 01:21 +0100, Alexander Berntsen escribió: Remove the --autounmask option from emerge. Please note that removing the option does not mean that the variable used for keeping track of autounmasking is not removed from depgraph.py. If I understand the change correctly (I don't know much about python but, but per the diff, looks like you are dropping the option and making the code behave like it's always 'True'), seems that you are forcing autounmask to be on always. Even if I use this in all my systems (passing it in by default emerge opts), I think we still need a way to disable it sometimes. For example, I need that when portage shows me really strange error messages. I remember this was an old bug related with backtracking, but can't find it just now (it should contains quite a few duplicates) :S I am referring to that kind of errors that reports the wrong package as being the culprit of some conflict
Re: [gentoo-portage-dev] Signing off patches
On Saturday 18 January 2014 17:57:38 Tom Wijsman wrote: On Sat, 18 Jan 2014 08:43:12 -0800 W. Trevor King wrote: On Sat, Jan 18, 2014 at 04:02:02PM +0100, Tom Wijsman wrote: I think the idea is that you shouldn't need to refer to an external resource like the mailing list to understand the idea behind the patch, Either someone cares about the background of a patch or he/she doesn't. full details to understand the change must be in the commit message. saying go find it in the mailing list is not a workable solution. this doesn't mean you have to copy paste the entire discussion, but it does mean you have to distill things down. for topics that go on for a while, adding a tag linking to the mailing list archive is certainly OK. when it comes to tags, i only copy in what other people have bothered posting. so if someone posts their Reviewed-by or Acked-by, i'll use them. -mike signature.asc Description: This is a digitally signed message part.
Re: [gentoo-portage-dev] Bugzilla workflow
On Tuesday 14 January 2014 21:58:38 Tom Wijsman wrote: On Sun, 05 Jan 2014 15:42:48 -0800 Brian Dolbec wrote: 2) start working on a solution, a) if you have significant progress, but need more time, mark it accordingly, assign it to yourself, leave a comment, etc. Assigning it to oneself is a quite good idea as it allows to easily keep track of the bugs you are working on, otherwise you have to rely on techniques that aren't optimal; which are unfortunate. In the lists of all bugs, that can be obtained by checking out the product and/or categories; this gives a quite clear overview of who is working on what, as well as which bugs are still free. As this is still able to be done, there seems no need to assign the bug to Portage team. i disagree. dev-portage@ get's cc-ed on bugs when they're being kept abreast of developments (like PMS), or someone just wants an opinion/feedback on an issue. so there's no way to differentiate between bugs that are assigned to the portage team and bugs where the portage team's opinion is being requested. i want a query for the former and i just rely on generated bugzilla e-mails for the latter. what's wrong with using the whiteboard ? it's a free text field and you can easily construct a query that produces exactly what you want. just stick in your username in there. 3) commit the fix. Mark it as InVCS, if not already, set status to IN_PROGRESS InVCS becomes redundant; other than that, good. i don't see how it's redundant. there is no other flag that indicates things have been fixed in the git tree and the only reason the bug remains open is that a release has not yet been cut. -mike signature.asc Description: This is a digitally signed message part.
Re: [gentoo-portage-dev] [PATCH v3] Test for read-only filesystems, bail out during preinst if there are any which will be written to and display a useful error message. Fixes bug 378869.
On Saturday 18 January 2014 21:00:48 Chris Reffett wrote: --- /dev/null +++ b/pym/portage/util/rochecker.py @@ -0,0 +1,81 @@ +#-*- coding:utf-8 -*- +# Copyright 2014 Gentoo Foundation +# Distributed under the terms of the GNU General Public License v2 + +Methods to check whether Portage is going to write to read-only filesystems. +Since the methods are not portable across different OSes, each OS needs its +own method. To expand RO checking for different OSes, add a method which +accepts a list of directories and returns a list of mounts which need to be +remounted RW, then add elif ostype == (the ostype value for your OS) to prefer OSes - OS's +get_ro_checker() + period at the end +def get_ro_checker(): + + Uses the system type to find an appropriate method for testing whether Portage + is going to write to any read-only filesystems + + @return: + 1. A method for testing for RO filesystems appropriate to the current system + not a new issue, but we really need to get away from this style and use PEP257. someone feel like fixing that in the code base ? :) + if ostype == Linux: + return linux_ro_checker + else: + return empty_ro_checker isn't this a glorified dict ? _CHECKERS = { 'Linux': linux_ro_checker, } def get_ro_checker() return _CHECKERS.get(ostype, empty_ro_checker) -mike signature.asc Description: This is a digitally signed message part.
Re: [gentoo-portage-dev] [PATCH 2/3] emerge: Rename --autounmask-write to --autounmask
On Saturday 18 January 2014 19:21:11 Alexander Berntsen wrote: Rename --autounmask-write to --autounmask. typically when we delete/rename an option, we give users a heads up. that means a cycle where the old option exists but emits a warning before switching to the new one. then we can delete it. -mike signature.asc Description: This is a digitally signed message part.
Re: [gentoo-portage-dev] [PATCH v4] Add repoman check to warn if src_prepare/src_configure are used in EAPI 0/1 and if pkg_pretend is used in EAPI 4. Fixes bug 379491.
Acked-by: Mike Frysinger vap...@gentoo.org btw you should keep the patch summary short. if it goes long, better to move it to the main body. -mike signature.asc Description: This is a digitally signed message part.
Re: [gentoo-portage-dev] [PATCH 3/3] Have repoman deprecate G2CONF for the GNOME team. (bug #482084).
On Wednesday 15 January 2014 19:07:20 Tom Wijsman wrote: +class DeprecateG2CONF(LineCheck): + repoman_check_name = 'G2CONF.deprecated' + re = re.compile(r'.*G2CONF.*') + + def check(self, num, line): + Run the check on line and return error if there is one + m = self.re.match(line) use re.search instead of re.match so you can drop the redundant .*. further, i think you want to use word boundaries. so: re = re.compile(r'\bG2CONF\b') -mike signature.asc Description: This is a digitally signed message part.
Re: [gentoo-portage-dev] [PATCH 3/3] Have repoman deprecate G2CONF for the GNOME team. (bug #482084).
btw, suggestion for commit summary/message: repoman: deprecate G2CONF Deprecate the G2CONF variable as requested by the GNOME team. URL: http://bugs.gentoo.org/482084 -mike signature.asc Description: This is a digitally signed message part.
Re: [gentoo-portage-dev] [PATCH 1/3 v2] Have repoman check if the packages to unpack rare archive formats from SRC_URI are present in DEPEND (bug #205909).
On Friday 17 January 2014 18:03:57 Tom Wijsman wrote: --- please shorten your commit summary and move more content to the body +getNonSystemArchiveDepends.archivers = { it is super weird to attach to the object like this. some might even say wrong. move it into the class definition. def getNonSystemArchiveDepends(fetchlist, eapi): ... ARCHIVERS = { ... } + re.compile('.*\.7[zZ]$'):app-arch/p7zip, regexes should always use raw strings. there should also be a space after the colon in dicts. so you want: re.compile(r'.*\.7[zZ]$'): 'app-arch/p7zip', + re.compile('.*\.lzma$'):app-arch/lzma-utils, xz-utils, not lzma-utils + re.compile('.*\.(bz2?|tbz2)$'):app-arch/bzip2, + re.compile('.*\.(tar(\.(bz2?|gz|Z))?|tbz2|t[bg]z)?$'):app-arch/tar, + re.compile('.*\.(gz|tar\.Z|t[bg]z|[zZ])$'):app-arch/gzip, + re.compile('.*\.tar.xz$'):app-arch/tar, requiring people list gzip, tar, and bzip2 is a significant policy change (which i'm inclined to say is wrong). it needs discussion on the dev mailing list first. +def checkArchiveDepends(atoms, catpkg, relative_path, \ + system_set_atoms, needed_unpack_depends, stats, fails): you don't need the \ there because you have paren already to gather things. + for entry in needed_unpack_depends[catpkg]: + if entry not in [atom.cp for atom in atoms if atom != ||]: + stats['unpack.' + mytype + '.missing'] += 1 + fails['unpack.' + mytype + '.missing'].append( \ + relative_path + : %s is missing in %s % \ + (entry, mytype)) i know existing style doesn't follow it, but imo we should avoid string concatenation. it makes the code harder to read imo. key = 'unpack.%s.missing' % mytype stats[key] += 1 fails[key].append(...) +def eapi_has_xz_utils(eapi): + return eapi not in (0, 1, 2) i would use eapi_supports_xz_archives unless there's a pre-existing style -mike signature.asc Description: This is a digitally signed message part.
Re: [gentoo-portage-dev] [PATCH] Add repoman check to warn if src_prepare/src_configure are used in EAPI 0/1
On Tuesday 14 January 2014 16:15:48 Tom Wijsman wrote: On Mon, 13 Jan 2014 23:59:11 -0500 Mike Frysinger wrote: we probably should just use dev branches in the main repo, at least for people who have write access to the repo dev/$USERNAME/whatever you want To be more clear, which one? g.o.g.o, GitHub or is one of both fine? g.o.g.o r'\s*src_(configure|prepare)\s*\(\)' You can then proceed further and move the re outside: the idea was to walk a balance between simplicity and maintainability. imo, the fixed version above is the best. What about the latter improvements about the parentheses? seems fine as long as portage supports an EAPI, i see no reason to omit useful checks like this. Repeating my original question in different words: Why is it useful? people run repoman outside of the main tree. we don't really know their desire for EAPI compatibility. legacy/old portage/who knows. Chromium OS for a long time was restricted to EAPI 4 for two reasons -- it had an old portage version (and upgrading to a newer one regressed performance significantly, so we held off until we could figure out why), and it was using a really old stage3 to build the SDK (which means we needed to support upgrading an old system too). -mike signature.asc Description: This is a digitally signed message part.
Re: [gentoo-portage-dev] [PATCH 1/3] emerge: Deprecate --autounmask
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 On 19/01/14 11:23, Alexander Berntsen wrote: emerge --ask foo # This won't -write emerge --autounmask --pretend foo # Same as the above Sorry, the comments here are imprecise. The first-mentioned will *prompt* the user for writing the changes. The second-mentioned will merely print out the suggestions. - -- Alexander alexan...@plaimi.net http://plaimi.net/~alexander -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.22 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iF4EAREIAAYFAlLbqD4ACgkQRtClrXBQc7Xq2gEAnZm6ZFdSykzpv6sOO/Wg6KFh cbbijfaQXGmmrXM2gJQA/AklPbjJGgJ/TxfSydymu3gTo7UutkLwawpea+dCcAEf =7Q4V -END PGP SIGNATURE-
Re: [gentoo-portage-dev] [PATCH 1/3] emerge: Deprecate --autounmask
El dom, 19-01-2014 a las 11:23 +0100, Alexander Berntsen escribió: On 19/01/14 09:01, Pacho Ramos wrote: If I understand the change correctly (I don't know much about python but, but per the diff, looks like you are dropping the option and making the code behave like it's always 'True'), seems that you are forcing autounmask to be on always. You do not understand it correctly. It makes --ask imply --autounmask-write. Ah, nice :) Even if I use this in all my systems (passing it in by default emerge opts), I think we still need a way to disable it sometimes. There is. Regardless of whether you mean (current) --autounmask or (current) --autounmask-write behaviour. emerge --ask foo # This won't -write emerge --autounmask --pretend foo # Same as the above emerge --ask --pretend foo # This won't even offer the suggestions Please see [0] for more information. [0] https://bugs.gentoo.org/show_bug.cgi?id=481578#c10 Then, I guess -ap would be the equivalent of --autounmask=n and should behave in the same way, right? In that case, no problem (even if I think we should document this since using --ask --pretend at the same time doesn't look so intuitive to me :( )
Re: [gentoo-portage-dev] [PATCH v3] Test for read-only filesystems, bail out during preinst if there are any which will be written to and display a useful error message. Fixes bug 378869.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 On 19/01/14 10:17, Mike Frysinger wrote: prefer OSes - OS's That's just not proper English. It makes no sense. Please don't prefer that. If for nothing else, then to prevent me from sighing whenever I have to read it. not a new issue, but we really need to get away from this style and use PEP257. someone feel like fixing that in the code base ? :) I agree. If you give me at least a week (won't have time until next weekend), I can look into it. - -- Alexander alexan...@plaimi.net http://plaimi.net/~alexander -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.22 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iF4EAREIAAYFAlLbq10ACgkQRtClrXBQc7U7MAD/Yl2pd0ood3UKmvNMg9gdWMsw hQqnReHPRwLpDNRccOcA/jUODDanycVmNLpD/rjkYnKyllnbUSIxduYuMcJFqXme =vaDs -END PGP SIGNATURE-
Re: [gentoo-portage-dev] [PATCH 2/3] emerge: Rename --autounmask-write to --autounmask
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 On 19/01/14 10:20, Mike Frysinger wrote: typically when we delete/rename an option, we give users a heads up. that means a cycle where the old option exists but emits a warning before switching to the new one. then we can delete it. We should have a news item. [0] https://bugs.gentoo.org/show_bug.cgi?id=481578#c14 - -- Alexander alexan...@plaimi.net http://plaimi.net/~alexander -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.22 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iF4EAREIAAYFAlLbrYEACgkQRtClrXBQc7WUcAEAosYXsLWlWJj1r7vAoVjOD2CG qncNxjMh2qzfAOPjbPIA/3E6DZ7WztAdNweVLpRdxeeg/LKA8TlUBqjniH8Nvt6B =5oJv -END PGP SIGNATURE-
Re: [gentoo-portage-dev] [PATCH] Add repoman check to warn if src_prepare/src_configure are used in EAPI 0/1
On Sun, Jan 19, 2014 at 4:44 AM, Mike Frysinger vap...@gentoo.org wrote: Chromium OS for a long time was restricted to EAPI 4 for two reasons -- it had an old portage version (and upgrading to a newer one regressed performance significantly, so we held off until we could figure out why) I am curious to know more about the performance regression if you can share. Is that something that got fixed, or did you disable some features (like the slot-operator stuff)?
Re: [gentoo-portage-dev] [PATCH 1/3] emerge: Deprecate --autounmask
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 On 19/01/14 17:49, Mike Gilbert wrote: Please give me a way to shut off autounmask entirely no mater what other options I pass to emerge. Tying it to --ask with no way to disable it is not acceptable. - --autounmask=n should do this. I messed up something. Sorry. I wrote this months ago, and don't have it all in my head right now. I will fix this next weekend. - -- Alexander alexan...@plaimi.net http://plaimi.net/~alexander -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.22 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iF4EAREIAAYFAlLcGSUACgkQRtClrXBQc7VehAEAhPIv0r7WZMOQSzzctvsDjkEF CwmLlLNb8tzb52E/Ng0A/AxQueneyo2uK1HktnU9YPOAYmi6ziAImKS4SNYS5bF6 =ysNQ -END PGP SIGNATURE-
[gentoo-portage-dev] [PATCH] Implement FEATURES=mirror for emerge (bug 498498)
This was only implemented for the ebuild command before. --- pym/_emerge/Scheduler.py | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pym/_emerge/Scheduler.py b/pym/_emerge/Scheduler.py index d663e97..2fd4d7e 100644 --- a/pym/_emerge/Scheduler.py +++ b/pym/_emerge/Scheduler.py @@ -165,6 +165,8 @@ class Scheduler(PollScheduler): self._build_opts.buildpkg_exclude = InternalPackageSet( \ initial_atoms= .join(myopts.get(--buildpkg-exclude, [])).split(), \ allow_wildcard=True, allow_repo=True) + if mirror in self.settings.features: + self._build_opts.fetch_all_uri = True self._binpkg_opts = self._binpkg_opts_class() for k in self._binpkg_opts.__slots__: @@ -752,11 +754,11 @@ class Scheduler(PollScheduler): pass elif pkg.type_name == ebuild: - prefetcher = EbuildFetcher(background=True, config_pool=self._ConfigPool(pkg.root, self._allocate_config, self._deallocate_config), - fetchonly=1, logfile=self._fetch_log, + fetchonly=1, fetchall=self._build_opts.fetch_all_uri, + logfile=self._fetch_log, pkg=pkg, prefetch=True, scheduler=self._sched_iface) elif pkg.type_name == binary and \ -- 1.8.3.2
Re: [gentoo-portage-dev] [PATCH] Implement FEATURES=mirror for emerge (bug 498498)
The removed white space was unintentional. Please remove that change before committing that patch.
Re: [gentoo-portage-dev] [PATCH 0/3] Initial fetch() refactoring
Am 19.01.2014 04:07, schrieb W. Trevor King: I felt like I should actually make a useful contribution to balance the less useful commit-message discussion ;). Browsing through the open Portage bugs, #175612 looked interesting. After I looked at pym/portage/package/ebuild/fetch.py, I decided to take a step back and just try and refactor fetch(), which was pushing 1k lines. Here are three paches pulling fairly self-contained blocks out of fetch(). I thought I'd float them to the list to see if this was a productive avenue, or if this is going to be too much work to review. I tried to avoid making too many changes other than the function-extraction, but in some places I couldn't help myself ;). Good idea. The patches aren't particularly well tested yet. I ran the test suite and got some errors, but they seemed to be related to my non-root invocation, and not due to these changes themselves. I thought I'd post my work so far, before digging deeper into the test suite. The tests can be run as non-root user. Make sure all the files in the git repo are +rw for that user. You should check the result using pyflakes. Do you have these patches in the publicly accessible git repo? Cheers, Trevor [1]: https://bugs.gentoo.org/show_bug.cgi?id=175612 W. Trevor King (3): pym/portage/package/ebuild/fetch.py: Factor out _get_checksum_failure_max_tries pym/portage/package/ebuild/fetch.py: Factor out _get_fetch_resume_size pym/portage/package/ebuild/fetch.py: Factor out _get_uris pym/portage/package/ebuild/fetch.py | 305 +--- 1 file changed, 177 insertions(+), 128 deletions(-)
Re: [gentoo-portage-dev] [PATCH 0/3] Initial fetch() refactoring
On Sun, Jan 19, 2014 at 09:05:41PM +0100, Sebastian Luther wrote: Am 19.01.2014 04:07, schrieb W. Trevor King: The patches aren't particularly well tested yet. I ran the test suite and got some errors, but they seemed to be related to my non-root invocation, and not due to these changes themselves. I thought I'd post my work so far, before digging deeper into the test suite. The tests can be run as non-root user. Make sure all the files in the git repo are +rw for that user. Ok. I'll go back and check the results on master so I can compare. You should check the result using pyflakes. Thanks. This picked up a few errors in the third patch. I'll post v2 shortly. Do you have these patches in the publicly accessible git repo? I do now: git://tremily.us/portage.git fetch-refactor Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [gentoo-portage-dev] [PATCH 3/3] pym/portage/package/ebuild/fetch.py: Factor out _get_uris
Am 19.01.2014 04:07, schrieb W. Trevor King: +def _get_uris(uris, settings, custom_mirrors=(), locations=()): missing doc string
Re: [gentoo-portage-dev] [PATCH 0/3] Initial fetch() refactoring
Am 19.01.2014 04:07, schrieb W. Trevor King: The patches aren't particularly well tested yet. I ran the test suite and got some errors, but they seemed to be related to my non-root invocation, and not due to these changes themselves. I thought I'd post my work so far, before digging deeper into the test suite. Since I doubt there's currently any test or any way to write one, I'd suggest to try and run emerge and analyze coverage with dev-python/coverage or a similar tool.
[gentoo-portage-dev] [PATCH v2 2/3] pym/portage/package/ebuild/fetch.py: Factor out _get_fetch_resume_size
The current fetch() function is quite long, which makes it hard to know what I can change without adverse side effects. By pulling this logic out of the main function, we get clearer logic in fetch() and more explicit input for the config extraction. --- pym/portage/package/ebuild/fetch.py | 50 - 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/pym/portage/package/ebuild/fetch.py b/pym/portage/package/ebuild/fetch.py index 6f46572..de5bf00 100644 --- a/pym/portage/package/ebuild/fetch.py +++ b/pym/portage/package/ebuild/fetch.py @@ -272,6 +272,32 @@ def _get_checksum_failure_max_tries(settings, default=5): return v +def _get_fetch_resume_size(settings, default='350K'): + v = settings.get(PORTAGE_FETCH_RESUME_MIN_SIZE) + if v is not None: + v = .join(v.split()) + if not v: + # If it's empty, silently use the default. + v = default + match = _fetch_resume_size_re.match(v) + if (match is None or + match.group(2).upper() not in _size_suffix_map): + writemsg(_(!!! Variable PORTAGE_FETCH_RESUME_MIN_SIZE +contains an unrecognized format: '%s'\n) % \ + settings[PORTAGE_FETCH_RESUME_MIN_SIZE], + noiselevel=-1) + writemsg(_(!!! Using PORTAGE_FETCH_RESUME_MIN_SIZE + default value: %s\n) % default, + noiselevel=-1) + v = None + if v is None: + v = default + match = _fetch_resume_size_re.match(v) + v = int(match.group(1)) * \ + 2 ** _size_suffix_map[match.group(2).upper()] + return v + + def fetch(myuris, mysettings, listonly=0, fetchonly=0, locks_in_subdir=.locks, use_locks=1, try_mirrors=1, digests=None, allow_missing_digests=True): @@ -297,29 +323,7 @@ def fetch(myuris, mysettings, listonly=0, fetchonly=0, checksum_failure_max_tries = _get_checksum_failure_max_tries( settings=mysettings) - - fetch_resume_size_default = 350K - fetch_resume_size = mysettings.get(PORTAGE_FETCH_RESUME_MIN_SIZE) - if fetch_resume_size is not None: - fetch_resume_size = .join(fetch_resume_size.split()) - if not fetch_resume_size: - # If it's undefined or empty, silently use the default. - fetch_resume_size = fetch_resume_size_default - match = _fetch_resume_size_re.match(fetch_resume_size) - if match is None or \ - (match.group(2).upper() not in _size_suffix_map): - writemsg(_(!!! Variable PORTAGE_FETCH_RESUME_MIN_SIZE -contains an unrecognized format: '%s'\n) % \ - mysettings[PORTAGE_FETCH_RESUME_MIN_SIZE], noiselevel=-1) - writemsg(_(!!! Using PORTAGE_FETCH_RESUME_MIN_SIZE - default value: %s\n) % fetch_resume_size_default, - noiselevel=-1) - fetch_resume_size = None - if fetch_resume_size is None: - fetch_resume_size = fetch_resume_size_default - match = _fetch_resume_size_re.match(fetch_resume_size) - fetch_resume_size = int(match.group(1)) * \ - 2 ** _size_suffix_map[match.group(2).upper()] + fetch_resume_size = _get_fetch_resume_size(settings=mysettings) # Behave like the package has RESTRICT=primaryuri after a # couple of checksum failures, to increase the probablility -- 1.8.5.2.8.g0f6c0d1
[gentoo-portage-dev] [PATCH v2 0/3] Initial fetch() refactoring
Following Sebastian's pyflakes and testing suggestions turned up a few bugs in the _get_uris patch. Changes since v1: Patch #1: * Fixed: summary to: summary and converted some from tabs to spaces where I'd missed them in v1. Patch #3: * Fixed docstrings as for patch #1. * Fix 'myuri' → 'uri' in _get_file_uri_tuples. * Fix 'cmirr' → 'm_uri' in _expand_mirror's urlunparse calls. * Fix 'cmirr' → 'locmirr' in _expand_mirror's third_party_mirrors branch. * Move “No known mirror” error from _get_uris to _expand_mirror. * Calculate 'restrict', 'restrict_fetch', 'restrict_mirror', and 'force_mirror' in _get_uris. * Remove 'force_mirror' from fetch, because it's only used in _get_uris. * Fix 'custom_mirrors' → 'custommirrors' in _get_uris invocation. After these changes, runtests.sh passes with only TODO messages on Python 2.7, 3.2, and 3.3, although I'm not sure how thoroughly the test suite covers fetch. There are also a few unrelated: /usr/lib64/python3.3/xml/etree/ElementTree.py:1726: DeprecationWarning: This method of XMLParser is deprecated. Define doctype() method on the TreeBuilder target. parser.feed(data) warnings in the 3.3 results. For folks who prefer Git fetches to the emailed patches, my current series is at: git://tremily.us/portage.git fetch-refactor For those who prefer Git diffs to the above “changes since v1”, the v1 version of this series is tagged in my repository as fetch-refactor-v1. Sebastian, I'd normally cc you directly on v2, but I'm not sure what the Portage team's conventions are here. Let me know if there is a convention, or if you have a personal preference on direct mail vs the list. I think projects that encourage cc-ing tend to have reasonable numbers of NNTP readers, and I don't know where the gentoo-portage-dev@ population falls on that issue. W. Trevor King (3): pym/portage/package/ebuild/fetch.py: Factor out _get_checksum_failure_max_tries pym/portage/package/ebuild/fetch.py: Factor out _get_fetch_resume_size pym/portage/package/ebuild/fetch.py: Factor out _get_uris pym/portage/package/ebuild/fetch.py | 318 +--- 1 file changed, 189 insertions(+), 129 deletions(-) -- 1.8.5.2.8.g0f6c0d1
[gentoo-portage-dev] [PATCH v2 3/3] pym/portage/package/ebuild/fetch.py: Factor out _get_uris
The current fetch() function is quite long, which makes it hard to know what I can change without adverse side effects. By pulling this logic out of the main function, we get clearer logic in fetch() and more explicit input for the config extraction. This block was especially complicated, so I also created the helper functions _get_file_uri_tuples and _expand_mirror. I'd prefer if _expand_mirror iterated through URIs instead of (group, URI) tuples, but we need a distinct marker for third-party URIs to build third_party_mirror_uris which is used to build primaryuri_dict which is used way down in fetch(): if checksum_failure_count == \ checksum_failure_primaryuri: # Switch to primaryuri mode in order # to increase the probablility of # of success. primaryuris = \ primaryuri_dict.get(myfile) if primaryuris: uri_list.extend( reversed(primaryuris)) I don't know if this is useful enough to motivate the uglier _expandmirror return values, but I'll kick that can down the road for now. --- pym/portage/package/ebuild/fetch.py | 209 ++-- 1 file changed, 128 insertions(+), 81 deletions(-) diff --git a/pym/portage/package/ebuild/fetch.py b/pym/portage/package/ebuild/fetch.py index de5bf00..a1940f4 100644 --- a/pym/portage/package/ebuild/fetch.py +++ b/pym/portage/package/ebuild/fetch.py @@ -15,9 +15,9 @@ import sys import tempfile try: - from urllib.parse import urlparse + from urllib.parse import urlparse, urlunparse except ImportError: - from urlparse import urlparse + from urlparse import urlparse, urlunparse import portage portage.proxy.lazyimport.lazyimport(globals(), @@ -298,6 +298,129 @@ def _get_fetch_resume_size(settings, default='350K'): return v +def _get_file_uri_tuples(uris): + + Return a list of (filename, uri) tuples + + file_uri_tuples = [] + # Check for 'items' attribute since OrderedDict is not a dict. + if hasattr(uris, 'items'): + for filename, uri_set in uris.items(): + for uri in uri_set: + file_uri_tuples.append((filename, uri)) + if not uri_set: + file_uri_tuples.append((filename, None)) + else: + for uri in uris: + if urlparse(uri).scheme: + file_uri_tuples.append( + (os.path.basename(uri), uri)) + else: + file_uri_tuples.append( + (os.path.basename(uri), None)) + return file_uri_tuples + + +def _expand_mirror(uri, custom_mirrors=(), third_party_mirrors=()): + + Replace the 'mirror://' scheme in the uri + + Returns an iterable listing expanded (group, URI) tuples, + where the group is either 'custom' or 'third-party'. + + parsed = urlparse(uri) + mirror = parsed.netloc + path = parsed.path + if path: + # Try user-defined mirrors first + if mirror in custom_mirrors: + for cmirr in custom_mirrors[mirror]: + m_uri = urlparse(cmirr) + yield ('custom', urlunparse(( + m_uri.scheme, m_uri.netloc, path) + + parsed[3:])) + + # now try the official mirrors + if mirror in third_party_mirrors: + uris = [] + for locmirr in third_party_mirrors[mirror]: + m_uri = urlparse(locmirr) + uris.append(urlunparse(( + m_uri.scheme, m_uri.netloc, path) + + parsed[3:])) + random.shuffle(uris) + for uri in uris: + yield ('third-party', uri) + + if (not custom_mirrors.get(mirror, []) and + not third_party_mirrors.get(mirror, [])): + writemsg( + _(No known mirror by the name: %s\n) + % (mirror,)) + else: + writemsg(_(Invalid mirror definition in SRC_URI:\n), +noiselevel=-1) + writemsg( %s\n % (uri), noiselevel=-1) + + +def _get_uris(uris, settings, custom_mirrors=(), locations=()): + restrict = settings.get(PORTAGE_RESTRICT, ).split() + restrict_fetch = fetch in restrict + restrict_mirror = mirror in restrict or nomirror in restrict + force_mirror = ( + force-mirror in settings.features and + not restrict_mirror) + + third_party_mirrors =
[gentoo-portage-dev] [PATCH v2 1/3] pym/portage/package/ebuild/fetch.py: Factor out _get_checksum_failure_max_tries
The current fetch() function is quite long, which makes it hard to know what I can change without adverse side effects. By pulling this logic out of the main function, we get clearer logic in fetch() and more explicit input for the config extraction. --- pym/portage/package/ebuild/fetch.py | 59 + 1 file changed, 34 insertions(+), 25 deletions(-) diff --git a/pym/portage/package/ebuild/fetch.py b/pym/portage/package/ebuild/fetch.py index 5316f03..6f46572 100644 --- a/pym/portage/package/ebuild/fetch.py +++ b/pym/portage/package/ebuild/fetch.py @@ -240,6 +240,38 @@ _size_suffix_map = { 'Y' : 80, } + +def _get_checksum_failure_max_tries(settings, default=5): + + Get the maximum number of failed download attempts + + Generally, downloading the same file repeatedly from + every single available mirror is a waste of bandwidth + and time, so there needs to be a cap. + + v = default + try: + v = int(settings.get(PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS, + default)) + except (ValueError, OverflowError): + writemsg(_(!!! Variable PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS +contains non-integer value: '%s'\n) % \ + settings[PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS], + noiselevel=-1) + writemsg(_(!!! Using PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS + default value: %s\n) % default, + noiselevel=-1) + v = default + if v 1: + writemsg(_(!!! Variable PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS +contains value less than 1: '%s'\n) % v, noiselevel=-1) + writemsg(_(!!! Using PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS + default value: %s\n) % default, + noiselevel=-1) + v = default + return v + + def fetch(myuris, mysettings, listonly=0, fetchonly=0, locks_in_subdir=.locks, use_locks=1, try_mirrors=1, digests=None, allow_missing_digests=True): @@ -263,31 +295,8 @@ def fetch(myuris, mysettings, listonly=0, fetchonly=0, print(_( \mirror\ mode desired and \mirror\ restriction found; skipping fetch.)) return 1 - # Generally, downloading the same file repeatedly from - # every single available mirror is a waste of bandwidth - # and time, so there needs to be a cap. - checksum_failure_max_tries = 5 - v = checksum_failure_max_tries - try: - v = int(mysettings.get(PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS, - checksum_failure_max_tries)) - except (ValueError, OverflowError): - writemsg(_(!!! Variable PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS -contains non-integer value: '%s'\n) % \ - mysettings[PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS], noiselevel=-1) - writemsg(_(!!! Using PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS - default value: %s\n) % checksum_failure_max_tries, - noiselevel=-1) - v = checksum_failure_max_tries - if v 1: - writemsg(_(!!! Variable PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS -contains value less than 1: '%s'\n) % v, noiselevel=-1) - writemsg(_(!!! Using PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS - default value: %s\n) % checksum_failure_max_tries, - noiselevel=-1) - v = checksum_failure_max_tries - checksum_failure_max_tries = v - del v + checksum_failure_max_tries = _get_checksum_failure_max_tries( + settings=mysettings) fetch_resume_size_default = 350K fetch_resume_size = mysettings.get(PORTAGE_FETCH_RESUME_MIN_SIZE) -- 1.8.5.2.8.g0f6c0d1
Re: [gentoo-portage-dev] [PATCH 3/3] pym/portage/package/ebuild/fetch.py: Factor out _get_uris
On Sat, Jan 18, 2014 at 7:07 PM, W. Trevor King wk...@tremily.us wrote: The current fetch() function is quite long, which makes it hard to know what I can change without adverse side effects. By pulling this logic out of the main function, we get clearer logic in fetch() and more explicit input for the config extraction. This block was especially complicated, so I also created the helper functions _get_file_uri_tuples and _expand_mirror. I'd prefer if _expand_mirror iterated through URIs instead of (group, URI) tuples, but we need a distinct marker for third-party URIs to build third_party_mirror_uris which is used to build primaryuri_dict which is used way down in fetch(): if checksum_failure_count == \ checksum_failure_primaryuri: # Switch to primaryuri mode in order # to increase the probablility of # of success. primaryuris = \ primaryuri_dict.get(myfile) if primaryuris: uri_list.extend( reversed(primaryuris)) I don't know if this is useful enough to motivate the uglier _expandmirror return values, but I'll kick that can down the road for now. --- pym/portage/package/ebuild/fetch.py | 197 +--- 1 file changed, 117 insertions(+), 80 deletions(-) diff --git a/pym/portage/package/ebuild/fetch.py b/pym/portage/package/ebuild/fetch.py index bd572fa..a617945 100644 --- a/pym/portage/package/ebuild/fetch.py +++ b/pym/portage/package/ebuild/fetch.py @@ -15,9 +15,9 @@ import sys import tempfile try: - from urllib.parse import urlparse + from urllib.parse import urlparse, urlunparse except ImportError: - from urlparse import urlparse + from urlparse import urlparse, urlunparse import portage portage.proxy.lazyimport.lazyimport(globals(), @@ -297,6 +297,118 @@ def _get_fetch_resume_size(settings, default='350K'): return v +def _get_file_uri_tuples(uris): + Return a list of (filename, uri) tuples + As mike noted on another thread: Return a list of (filename, uri) tuples. + file_uri_tuples = [] + # Check for 'items' attribute since OrderedDict is not a dict. + if hasattr(uris, 'items'): + for filename, uri_set in uris.items(): + for uri in uri_set: + file_uri_tuples.append((filename, uri)) + if not uri_set: + file_uri_tuples.append((filename, None)) + else: + for uri in uris: + if urlparse(uri).scheme: + file_uri_tuples.append( + (os.path.basename(myuri), myuri)) + else: + file_uri_tuples.append( + (os.path.basename(myuri), None)) + return file_uri_tuples + + +def _expand_mirror(uri, custom_mirrors=(), third_party_mirrors=()): + Replace the 'mirror://' scheme in the uri Sentence should end in a period. + + Returns an iterable listing expanded (group, URI) tuples, + where the group is either 'custom' or 'third-party'. + + parsed = urlparse(uri) + mirror = parsed.netloc + path = parsed.path + if path: + # Try user-defined mirrors first + if mirror in custom_mirrors: + for cmirr in custom_mirrors[mirror]: + m_uri = urlparse(cmirr) + yield ('custom', urlunparse(( + cmirr.scheme, cmirr.netloc, path) + + parsed[3:])) + + # now try the official mirrors + if mirror in third_party_mirrors: + uris = [] + for locmirr in third_party_mirrors[mirror]: + m_uri = urlparse(cmirr) + uris.append(urlunparse(( + cmirr.scheme, cmirr.netloc, path) + + parsed[3:])) + random.shuffle(uris) + for uri in uris: + yield ('third-party', uri) + else: + writemsg(_(Invalid mirror definition in SRC_URI:\n), +noiselevel=-1) + writemsg( %s\n % (uri), noiselevel=-1) Is this a py3k thing? You should be able to write: writemsg( %s\n % uri, noiselevel=-1) The tuple is only needed for multiple arguments in py2k. + + +def _get_uris(uris, settings, custom_mirrors=(), locations=()): I want you to be careful here. This is bad style when you are constructing mutable objects in parameter defaults: def func(a=[]): ... a.append(5) ... del func def func(a=[]): ...
Re: [gentoo-portage-dev] [PATCH v2 1/3] pym/portage/package/ebuild/fetch.py: Factor out _get_checksum_failure_max_tries
On Sun, Jan 19, 2014 at 2:44 PM, Alec Warner anta...@gentoo.org wrote: On Sun, Jan 19, 2014 at 2:14 PM, W. Trevor King wk...@tremily.us wrote: The current fetch() function is quite long, which makes it hard to know what I can change without adverse side effects. By pulling this logic out of the main function, we get clearer logic in fetch() and more explicit input for the config extraction. --- pym/portage/package/ebuild/fetch.py | 59 + 1 file changed, 34 insertions(+), 25 deletions(-) diff --git a/pym/portage/package/ebuild/fetch.py b/pym/portage/package/ebuild/fetch.py index 5316f03..6f46572 100644 --- a/pym/portage/package/ebuild/fetch.py +++ b/pym/portage/package/ebuild/fetch.py @@ -240,6 +240,38 @@ _size_suffix_map = { 'Y' : 80, } + +def _get_checksum_failure_max_tries(settings, default=5): + + Get the maximum number of failed download attempts + + Generally, downloading the same file repeatedly from + every single available mirror is a waste of bandwidth + and time, so there needs to be a cap. + + v = default + try: + v = int(settings.get(PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS, + default)) + except (ValueError, OverflowError): + writemsg(_(!!! Variable PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS +contains non-integer value: '%s'\n) % \ + settings[PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS], + noiselevel=-1) + writemsg(_(!!! Using PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS + default value: %s\n) % default, + noiselevel=-1) + v = default + if v 1: + writemsg(_(!!! Variable PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS +contains value less than 1: '%s'\n) % v, noiselevel=-1) + writemsg(_(!!! Using PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS + default value: %s\n) % default, + noiselevel=-1) + v = default + return v + + This function and the next function you wrote are identical. How about making a single function? def getValueFromSettings(settings, key, default, validator): try: return validator(settings.get(key, default)) except ValidationError as e: # writemsg the exception, it will contain the juicy bits.) def getIntValueFromSettings(settings, key, default): def validator(v): try: v = int(v) except (ValueError, OverflowError) as e: raise ValidationError( bla bla bla not an int, we picked the default.) if v 1: v = default raise ValidationError(bla bla bla less than one we used the defualt.) return getValueFromSettings(settings, key, default, validator) Note, don't actually use these function names, they are terrible. -A Then we are not necessarily writing a bunch of the same functions over and over. The defaults we can stick in a config file, or in python, or at the call site. -A def fetch(myuris, mysettings, listonly=0, fetchonly=0, locks_in_subdir=.locks, use_locks=1, try_mirrors=1, digests=None, allow_missing_digests=True): @@ -263,31 +295,8 @@ def fetch(myuris, mysettings, listonly=0, fetchonly=0, print(_( \mirror\ mode desired and \mirror\ restriction found; skipping fetch.)) return 1 - # Generally, downloading the same file repeatedly from - # every single available mirror is a waste of bandwidth - # and time, so there needs to be a cap. - checksum_failure_max_tries = 5 - v = checksum_failure_max_tries - try: - v = int(mysettings.get(PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS, - checksum_failure_max_tries)) - except (ValueError, OverflowError): - writemsg(_(!!! Variable PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS -contains non-integer value: '%s'\n) % \ - mysettings[PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS], noiselevel=-1) - writemsg(_(!!! Using PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS - default value: %s\n) % checksum_failure_max_tries, - noiselevel=-1) - v = checksum_failure_max_tries - if v 1: - writemsg(_(!!! Variable PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS -contains value less than 1: '%s'\n) % v, noiselevel=-1) - writemsg(_(!!! Using PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS - default value: %s\n) % checksum_failure_max_tries, - noiselevel=-1) - v = checksum_failure_max_tries - checksum_failure_max_tries = v - del v + checksum_failure_max_tries =
Re: [gentoo-portage-dev] [PATCH 0/3] Initial fetch() refactoring
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 On 19/01/14 22:22, Sebastian Luther wrote: The usual doc string style used in portage is: text Please use that for new functions. Also make sure you don't use spaces to indent the last . As mentioned by Mike in another thread, we should use PEP 257[0]. I will convert old code to conform to this... sometime... soon... (I promise!) So if new patches could just do that right away, that would be neat. [0] http://www.python.org/dev/peps/pep-0257/ - -- Alexander alexan...@plaimi.net http://plaimi.net/~alexander -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.22 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iF4EAREIAAYFAlLcVaAACgkQRtClrXBQc7XjTwD/UsCQo+SJdiCX/QsMC8jFDPic k0jEAN6DA5xML6/nJYQA/36FszMfVMZ/vzg5VF9FS6BDWwGm/Qy2whyqLiF3FOrX =+5Yy -END PGP SIGNATURE-
Re: [gentoo-portage-dev] [PATCH 0/3] Initial fetch() refactoring
On Sun, Jan 19, 2014 at 2:45 PM, Alexander Berntsen alexan...@plaimi.netwrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA256 On 19/01/14 22:22, Sebastian Luther wrote: The usual doc string style used in portage is: text Please use that for new functions. Also make sure you don't use spaces to indent the last . As mentioned by Mike in another thread, we should use PEP 257[0]. I will convert old code to conform to this... sometime... soon... (I promise!) So if new patches could just do that right away, that would be neat. Does pylint or pyflakes point out if you mess it up? Automation for the win. -A [0] http://www.python.org/dev/peps/pep-0257/ - -- Alexander alexan...@plaimi.net http://plaimi.net/~alexander -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.22 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iF4EAREIAAYFAlLcVaAACgkQRtClrXBQc7XjTwD/UsCQo+SJdiCX/QsMC8jFDPic k0jEAN6DA5xML6/nJYQA/36FszMfVMZ/vzg5VF9FS6BDWwGm/Qy2whyqLiF3FOrX =+5Yy -END PGP SIGNATURE-
Re: [gentoo-portage-dev] [PATCH 0/3] Initial fetch() refactoring
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 On 19/01/14 23:46, Alec Warner wrote: Does pylint or pyflakes point out if you mess it up? Not very successfully in my experience. - -- Alexander alexan...@plaimi.net http://plaimi.net/~alexander -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.22 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iF4EAREIAAYFAlLcVrsACgkQRtClrXBQc7XZcQD9GD8lV8HpZbQjpqu8ggNh4vOo /fvFsCI2PaWiEsC4ISUA/iYRiXAYD7SEF4a3R/t6vOdmEQSHyvZcxH9NDwI4/biQ =RMJ0 -END PGP SIGNATURE-
Re: [gentoo-portage-dev] [PATCH v2 1/3] pym/portage/package/ebuild/fetch.py: Factor out _get_checksum_failure_max_tries
On Sun, Jan 19, 2014 at 02:45:24PM -0800, Alec Warner wrote: On Sun, Jan 19, 2014 at 2:44 PM, Alec Warner anta...@gentoo.org wrote: This function and the next function you wrote are identical. How about making a single function? … def getIntValueFromSettings(settings, key, default): … Note, don't actually use these function names, they are terrible. A good name would be getint [1]. If we used ConfigParser, we wouldn't have to write any Portage-specific code at all ;). I don't think there's a shortage of things that could be streamlined here, and was trying to minimize rewriting until fetch() had been reduced to something I can comprehend ;). Cheers, Trevor [1]: http://docs.python.org/3/library/configparser.html#configparser.ConfigParser.getint -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [gentoo-portage-dev] [PATCH 0/3] Initial fetch() refactoring
On Sun, Jan 19, 2014 at 2:50 PM, Alexander Berntsen alexan...@plaimi.netwrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA256 On 19/01/14 23:46, Alec Warner wrote: Does pylint or pyflakes point out if you mess it up? Not very successfully in my experience. I'm very against add a bunch of extra rules that have to be enforced by hand. I want to make it easy to contribute, not more difficult. If bob can run a tool that tells him all the things that are wrong with his patch, that avoids us having like 1/3rd of the conversations on list ;) -A - -- Alexander alexan...@plaimi.net http://plaimi.net/~alexander -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.22 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iF4EAREIAAYFAlLcVrsACgkQRtClrXBQc7XZcQD9GD8lV8HpZbQjpqu8ggNh4vOo /fvFsCI2PaWiEsC4ISUA/iYRiXAYD7SEF4a3R/t6vOdmEQSHyvZcxH9NDwI4/biQ =RMJ0 -END PGP SIGNATURE-
Re: [gentoo-portage-dev] [PATCH 3/3] pym/portage/package/ebuild/fetch.py: Factor out _get_uris
On Sun, Jan 19, 2014 at 02:36:43PM -0800, Alec Warner wrote: On Sat, Jan 18, 2014 at 7:07 PM, W. Trevor King wk...@tremily.us wrote: +def _get_file_uri_tuples(uris): + Return a list of (filename, uri) tuples + As mike noted on another thread: Return a list of (filename, uri) tuples. I'd replaced this with: Return a list of (filename, uri) tuples in v2, but I'll queue the one-line form in v3. +def _expand_mirror(uri, custom_mirrors=(), third_party_mirrors=()): + Replace the 'mirror://' scheme in the uri Sentence should end in a period. Ok. I'll do that for the other summaries as well, and uppercase URI, in v3. + writemsg(_(Invalid mirror definition in SRC_URI:\n), +noiselevel=-1) + writemsg( %s\n % (uri), noiselevel=-1) Is this a py3k thing? You should be able to write: writemsg( %s\n % uri, noiselevel=-1) The tuple is only needed for multiple arguments in py2k. 1. I think I just copied and pasted it ;). 2. (uri) is not actually a tuple: type(('hello')) type 'str' To get a tuple, you need (uri,). I'm fine removing the parens in v3. +def _get_uris(uris, settings, custom_mirrors=(), locations=()): I want you to be careful here. This is bad style when you are constructing mutable objects in parameter defaults: I used tuples instead of lists because they are not mutable (as you point out later on). I'm not sure if I appreciate that more or less than the other form def _get_uris(uris, settings, custom_mirrors=None, locations=None): if not custom_mirrors: custom_mirrors = () if not locations: locations = () If you want me to do it that way, I'll queue it for v3. I think the default tuples are more readable. They are certainly shorter. Another advisable way to write it is to simply not have default arguments, and to force the caller to sync in an empty iterable: Meh. I don't have strong feelings about this, but custom_mirrors struck me as something that would not always be required ;). + third_party_mirrors = settings.thirdpartymirrors() Why pass in all of settings? We need other stuff too, see v2. The less settings parsing I need to do in fetch() itself, the happier I am. If that makes the helper functions a bit uglier, that's fine. Fewer people will have to read those. Think about testing this function. Working on it for v3. Do you really want to try to construct some sort of 'testing' settings object, or simply construct a list? I was going to use a dict for testing, and stub out a thirdpartymirrors mock for _get_uris. + third_party_mirror_uris = {} + filedict = OrderedDict() + primaryuri_dict = {} + for filename, uri in _get_file_uri_tuples(uris=uris): + if filename not in filedict: + filedict[filename] = [ + os.path.join(location, 'distfiles', filename) + for location in locations] + if uri is None: + continue + if uri.startswith('mirror://'): + uris = _expand_mirror( too many uri / uris variables... can we called this 'expanded_uris'? Queued for v3. I'm really having trouble tracking what is what. Remember that 'uris' is already a parameter to this function. Are you shadowing it here, or trying to overwrite it? Clobbering it. The original value is only used for the _get_file_uri_tuples call. + uri=uri, custom_mirrors=custom_mirrors, + third_party_mirrors=third_party_mirrors) + filedict[filename].extend(uri for group, uri in uris) group appears unused in your implicit iterable here. perhaps: filedict[filename].extend(uri for _, uri in uris) Queued for v3. + third_party_mirror_uris.setdefault(filename, []).extend( + uri for group, uri in uris + if group == 'third-party') I'm curious if this matters. We are iterator over 'uris' twice. Is it cheaper to do it once and build the iterables once? third_party_uris = [] for group, uri in uris: if group == 'third_party': third_party_uris.append(uri) filedict[filename].append(uri) I'm guessing the iterable is so small it doesn't matter. My guess is that the speed gain from list comprehension outweighs the loss of double iterations, but I agree that it is probably doesn't matter ;). + if not filedict[filename]: + writemsg( + _(No known mirror by the name: %s\n) + % (mirror,)) + else: + if restrict_fetch or force_mirror: Are these
Re: [gentoo-portage-dev] [PATCH 3/3] pym/portage/package/ebuild/fetch.py: Factor out _get_uris
On Sun, Jan 19, 2014 at 03:06:29PM -0800, W. Trevor King wrote: On Sun, Jan 19, 2014 at 02:36:43PM -0800, Alec Warner wrote: On Sat, Jan 18, 2014 at 7:07 PM, W. Trevor King wk...@tremily.us wrote: + # Order primaryuri_dict values to match that in SRC_URI. + for uris in primaryuri_dict.values(): + uris.reverse() Is there any guaranteed ordering for dict.values()? No. I thought dict order was random (and they seriously make it random in modern python versions, to detect bugs.) How does this uris.reverse() match the SRC_URI ordering? No idea (copy-pasted code). I'd be happy to cut this out in v3. Ah, we're not reversing the ordering of values(), we're reversing the ordering of each individual value. Keeping it in. Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [gentoo-portage-dev] [PATCH 0/3] Initial fetch() refactoring
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 On 19/01/14 23:54, Alec Warner wrote: I'm very against add a bunch of extra rules that have to be enforced by hand. I want to make it easy to contribute, not more difficult. If bob can run a tool that tells him all the things that are wrong with his patch, that avoids us having like 1/3rd of the conversations on list ;) Feel free to write a tool for this, or to contribute to any of the numerous linters and/or editor plug-ins. It would be much appreciated. As for the difficulty of PEP 257... I have higher hopes for Portage contributors than getting stuck at that. If I write a patch that makes most of the docstrings follow it, then they can infer 99% of the extra rules by just looking at the other functions and methods. If they fail to comply, we can just mention it. If the docstring formatting is the biggest issue with their patch, I doubt they'll have a hard time fixing it. - -- Alexander alexan...@plaimi.net http://plaimi.net/~alexander -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.22 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iF4EAREIAAYFAlLcZPQACgkQRtClrXBQc7XizAD/Y/Gxc7N6VkgNFWRgP5lmQ84r UwSne2xaqJYphY9x1TcBAIRpjBHB580edLz/8zpT14lqhW3oOmeMz0pNMB8ssW5d =+zp5 -END PGP SIGNATURE-
Re: [gentoo-portage-dev] [PATCH 0/3] Initial fetch() refactoring
On Sun, Jan 19, 2014 at 3:51 PM, Alexander Berntsen alexan...@plaimi.netwrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA256 On 19/01/14 23:54, Alec Warner wrote: I'm very against add a bunch of extra rules that have to be enforced by hand. I want to make it easy to contribute, not more difficult. If bob can run a tool that tells him all the things that are wrong with his patch, that avoids us having like 1/3rd of the conversations on list ;) Feel free to write a tool for this, or to contribute to any of the numerous linters and/or editor plug-ins. It would be much appreciated. I already prefer pylint, and I think it does cover most of what I want. I am working a pylintrc patch. As for the difficulty of PEP 257... I have higher hopes for Portage contributors than getting stuck at that. If I write a patch that makes most of the docstrings follow it, then they can infer 99% of the extra rules by just looking at the other functions and methods. If they fail to comply, we can just mention it. If the docstring formatting is the biggest issue with their patch, I doubt they'll have a hard time fixing it. I'm not saying its hard, I'm saying it is a giant waste of time for the list to tell people 'hey your docstrings are wrong' when they can just run a tool to do it ;) -A - -- Alexander alexan...@plaimi.net http://plaimi.net/~alexander -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.22 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iF4EAREIAAYFAlLcZPQACgkQRtClrXBQc7XizAD/Y/Gxc7N6VkgNFWRgP5lmQ84r UwSne2xaqJYphY9x1TcBAIRpjBHB580edLz/8zpT14lqhW3oOmeMz0pNMB8ssW5d =+zp5 -END PGP SIGNATURE-
Re: [gentoo-portage-dev] [PATCH 1/3] pym/portage/package/ebuild/fetch.py: Factor out _get_checksum_failure_max_tries
On Sat, 18 Jan 2014 19:07:45 -0800 W. Trevor King wk...@tremily.us wrote: [...SNIP...] + v = int(settings.get(PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS, + default)) + except (ValueError, OverflowError): + writemsg(_(!!! Variable PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS + contains non-integer value: '%s'\n) % \ + settings[PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS], + noiselevel=-1) + writemsg(_(!!! Using PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS + default value: %s\n) % default, + noiselevel=-1) + v = default + if v 1: + writemsg(_(!!! Variable PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS + contains value less than 1: '%s'\n) % v, noiselevel=-1) + writemsg(_(!!! Using PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS [...SNIP...] The code screams PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS to me which makes it hard to read, I would suggest assigning it to a variable in advance as to not have to repeat it this often. Some code snippets for ideas: key = PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS v = int(settings.get(key), default) !!! Variable %s contains non-integer value: '%s % (key, ...) If needed, add a word to key to make the variable name slightly more meaningful; but avoid the full length if possible. eg. try_mirrors_key -- With kind regards, Tom Wijsman (TomWij) Gentoo Developer E-mail address : tom...@gentoo.org GPG Public Key : 6D34E57D GPG Fingerprint : C165 AF18 AB4C 400B C3D2 ABF0 95B2 1FCD 6D34 E57D signature.asc Description: PGP signature
Re: [gentoo-portage-dev] [PATCH 2/3] pym/portage/package/ebuild/fetch.py: Factor out _get_fetch_resume_size
On Sat, 18 Jan 2014 19:07:46 -0800 W. Trevor King wk...@tremily.us wrote: +def _get_fetch_resume_size(settings, default='350K'): + v = settings.get(PORTAGE_FETCH_RESUME_MIN_SIZE) + if v is not None: + v = .join(v.split()) + if not v: + # If it's empty, silently use the default. + v = default + match = _fetch_resume_size_re.match(v) + if (match is None or + match.group(2).upper() not in _size_suffix_map): + writemsg(_(!!! Variable PORTAGE_FETCH_RESUME_MIN_SIZE + contains an unrecognized format: '%s'\n) % \ + settings[PORTAGE_FETCH_RESUME_MIN_SIZE], + noiselevel=-1) + writemsg(_(!!! Using PORTAGE_FETCH_RESUME_MIN_SIZE + default value: %s\n) % default, + noiselevel=-1) + v = None + if v is None: + v = default + match = _fetch_resume_size_re.match(v) + v = int(match.group(1)) * \ + 2 ** _size_suffix_map[match.group(2).upper()] + return v There is some duplicate code here, I think the conditions can be rewritten in such way that the duplicate code doesn't take place. Move everything from the first 'if' except for the first line out of that 'if', that way get a bare: if v is not None: v = .join(v.split()) Outside that, you now have 'if not v:' whereas you later have 'if v is None:'; you can optimize this to 'if not v or v is None:' (maybe that can be optimized further, dunno) and just have one condition set the default here. This gives you: if not v or v is None: # If it's empty, silently use the default. v = default Afterwards, you have 'match = _fetch_resume_size_re.match(v)' in both places, you can again just have this once. Can the two remaining code blocks just be placed after that? -- With kind regards, Tom Wijsman (TomWij) Gentoo Developer E-mail address : tom...@gentoo.org GPG Public Key : 6D34E57D GPG Fingerprint : C165 AF18 AB4C 400B C3D2 ABF0 95B2 1FCD 6D34 E57D signature.asc Description: PGP signature
Re: [gentoo-portage-dev] [PATCH 1/3] pym/portage/package/ebuild/fetch.py: Factor out _get_checksum_failure_max_tries
On Mon, Jan 20, 2014 at 02:26:59AM +0100, Tom Wijsman wrote: On Sat, 18 Jan 2014 19:07:45 -0800 W. Trevor King wk...@tremily.us wrote: [...SNIP...] + v = int(settings.get(PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS, + default)) … The code screams PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS to me which makes it hard to read, … That's all copy-pasted out of fetch() under the assumption that the fewer changes I made (besides creating _get_checksum_failure_max_tries at all), the easier it would be to review those changes. Perhaps I was mistaken ;). I would suggest assigning it to a variable in advance as to not have to repeat it this often. Some code snippets for ideas: key = PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS Will queue to v3. Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [gentoo-portage-dev] [PATCH 2/3] pym/portage/package/ebuild/fetch.py: Factor out _get_fetch_resume_size
On Mon, Jan 20, 2014 at 02:41:41AM +0100, Tom Wijsman wrote: There is some duplicate code here, I think the conditions can be rewritten in such way that the duplicate code doesn't take place. Do you want a rewrite squashed into this commit, or as a follow-on commit after this one (which gets a test suite in v3)? Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [gentoo-portage-dev] Signing off patches
On Sat, 18 Jan 2014 20:15:57 -0800 W. Trevor King wk...@tremily.us wrote: On Sun, Jan 19, 2014 at 02:33:06AM +0100, Tom Wijsman wrote: On Sat, 18 Jan 2014 15:24:59 -0800 W. Trevor King wk...@tremily.us wrote: If it doesn't need to get updated, then it probably already started out explaining the consensus ;). That is a guess, you can look this up in past patches. Sure. Will you? If I want to touch some code, and it looks confusing, I'll use blame to see who wrote it and whay they were thinking about. If the commit message is not informative, I usually give up. I have a hard time imaging folks tracking down the thread that spawned that patch, assuming such a thread even exists, for each troublesome line they'd like to touch. It's much easier to summarize any issues the list resolved (because they're likely the same questions the new dev is asking) in the commit message, where future developers can find them. How does this make the consensus written after the patch part of it? The person whom commits can be different than the person whom wrote the patch; and hence, that person commits without writing down consensus. If that person were to write it down, it would change the authorship. Hence you made a guess, because I see pushed commits without consensus. You spend time if you want to spend time and add whoever you feel moved to add. We discuss whether to make it policy to add involved people. But “involved” can be hard to pin down, especially by someone who may be applying v5 of a patch that hasn't been carefully following the whole discussion in earlier versions. If this would be formalized, then v5 would include all tags until then. The Linux kernel docs say [1]: If this patch fixes a problem reported by somebody else, consider adding a Reported-by: tag to credit the reporter for their contribution. Note the “consider” wiggle word. They are a bit more formal about Reviewed-by, but only because it's signing off on their Reviewer's statement of oversight. In general, if you're not signing some statement with the tag, formalizing “involved” is hard. Here I like vapier's approach from the other reply in this sub thread, that is to add it whenever people make the effort of providing the tag; which is an approach the Linux kernel upstream takes as well, if you want to be seen as a contributor you need to provide the tags. If you are submitting v2 of a patch, and feel a desire to credit reviewers / testers with this syntax, I think that's considerate of you. If you are committing someone else's patch to master, and want to record the folks who acked it on the list to distribute responsibility, that's fine too. If you want to use another syntax, or not do any of this at all, it's still fine by me ;). However, if a consistent syntax already exists, I see no reason not to use it when it suits your purpose. We discuss here whether to make it policy to use the same syntax. I don't understand the distinction between “here are some guidelines, apply as and if you see fit” and “make it a policy to …”. Say you have a situation like this: When the thread starts with words like ensure, exercised, followed, should, proposals then I rather get the impression that this is rather working towards making it part of policy than some random guidelines; even if these words are not mentioned, it is for us to discuss whether we should make this more than just guidelines. As I understand it, 6 should be: 6a. Everyone gets on with their lives. I could see a situation where: 6b. Charlie reminds Dan that he could have used the tags. Everyone gets on with their lives. Is there another alternative step 6 implied by the “policy” keyword? Or is the policy workflow even more different somehow? This discussion is based on that second situation 6b, as I think everyone is fine with 6a; but, some will want this to not become policy, others might want to see this to become policy. Hence this leads to a discussion. At the moment people seem fine with them being used optional, thus I agree with what was said here regarding it is a respectful suggestion to consider; at least nobody has strongly opposed to using them at all. -- With kind regards, Tom Wijsman (TomWij) Gentoo Developer E-mail address : tom...@gentoo.org GPG Public Key : 6D34E57D GPG Fingerprint : C165 AF18 AB4C 400B C3D2 ABF0 95B2 1FCD 6D34 E57D signature.asc Description: PGP signature
Re: [gentoo-portage-dev] [PATCH 2/3] pym/portage/package/ebuild/fetch.py: Factor out _get_fetch_resume_size
On Sun, 19 Jan 2014 18:01:23 -0800 W. Trevor King wk...@tremily.us wrote: On Mon, Jan 20, 2014 at 02:41:41AM +0100, Tom Wijsman wrote: There is some duplicate code here, I think the conditions can be rewritten in such way that the duplicate code doesn't take place. Do you want a rewrite squashed into this commit, or as a follow-on commit after this one (which gets a test suite in v3)? Sound more sane to do in a follow-up commit. While writing this review I didn't note that you were just moving most code, now you have ideas for further refactoring I guess. :) -- With kind regards, Tom Wijsman (TomWij) Gentoo Developer E-mail address : tom...@gentoo.org GPG Public Key : 6D34E57D GPG Fingerprint : C165 AF18 AB4C 400B C3D2 ABF0 95B2 1FCD 6D34 E57D signature.asc Description: PGP signature
Re: [gentoo-portage-dev] Signing off patches
On Mon, Jan 20, 2014 at 03:09:14AM +0100, Tom Wijsman wrote: On Sat, 18 Jan 2014 20:15:57 -0800 W. Trevor King wrote: On Sun, Jan 19, 2014 at 02:33:06AM +0100, Tom Wijsman wrote: On Sat, 18 Jan 2014 15:24:59 -0800 W. Trevor King wrote: If it doesn't need to get updated, then it probably already started out explaining the consensus ;). That is a guess, you can look this up in past patches. Sure. Will you? If I want to touch some code, and it looks confusing, I'll use blame to see who wrote it and whay they were thinking about. If the commit message is not informative, I usually give up. I have a hard time imaging folks tracking down the thread that spawned that patch, assuming such a thread even exists, for each troublesome line they'd like to touch. It's much easier to summarize any issues the list resolved (because they're likely the same questions the new dev is asking) in the commit message, where future developers can find them. How does this make the consensus written after the patch part of it? The person whom commits can be different than the person whom wrote the patch; and hence, that person commits without writing down consensus. If that person were to write it down, it would change the authorship. If the initial submission does not express the consensus, you can either ask for a resubmission that does, or say “Alice, is it ok if I change your commit message to read ‘…’? when I commit it?”. Hence you made a guess, because I see pushed commits without consensus. No policy/suggestion/goal is going to be followed 100% of the time. If most commits, especially those that were contentious enough to go through a few rounds of revision, have a commit message with a good summary, then the future developer using blame has good odds of finding something useful. I think that's a good goal to shoot for, even if you don't hit it all the time. Even if you only consider the present, improving your commit message to address tricky implementation details or unclear motivation before submitting the next revision on a patch series will help reviewers understand your patch better in the first place. Here I like vapier's approach from the other reply in this sub thread, that is to add it whenever people make the effort of providing the tag; which is an approach the Linux kernel upstream takes as well, if you want to be seen as a contributor you need to provide the tags. Sounds fair to me. Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [gentoo-portage-dev] [PATCH 1/3 v2] Have repoman check if the packages to unpack rare archive formats from SRC_URI are present in DEPEND (bug #205909).
On Sun, Jan 19, 2014 at 6:23 PM, Tom Wijsman tom...@gentoo.org wrote: On Sun, 19 Jan 2014 04:38:31 -0500 Mike Frysinger vap...@gentoo.org wrote: On Friday 17 January 2014 18:03:57 Tom Wijsman wrote: --- please shorten your commit summary and move more content to the body Right, I'm used to writing this on the command line; I'll try to look into setting up an editor or a GUI (gitg, ...) to do my commit messages. +getNonSystemArchiveDepends.archivers = { it is super weird to attach to the object like this. some might even say wrong. It a function, this makes it a static function variable. What is weird here? It works properly, what would be wrong? It is certainly weird (as we discussed on IRC.) I've never seen anyone do it in any codebase I liked. One of the problems is that it isn't immutable, so that earlier callers can mess with later callers. That is not possible in vapier's proposal, as the attributes are hidden in the function code and are not visible to callers. move it into the class definition. def getNonSystemArchiveDepends(fetchlist, eapi): ... ARCHIVERS = { ... } That makes it a non-static function variable? This is a regression. I guess I am not seeing why it must be a static function variable. Can you explain? + re.compile('.*\.7[zZ]$'):app-arch/p7zip, regexes should always use raw strings. there should also be a space after the colon in dicts. so you want: re.compile(r'.*\.7[zZ]$'): 'app-arch/p7zip', Raw strings are a solution when \ forms a problem, which is not the case here. The colon has no space after it in the rest of repoman near it, hence I left it out; is there a coding standard we're trying to respect here? Can you refer it to me? For the colon's in dicts thing: http://google-styleguide.googlecode.com/svn/trunk/pyguide.html#Whitespace + re.compile('.*\.lzma$'):app-arch/lzma-utils, xz-utils, not lzma-utils http://dev.gentoo.org/~ulm/pms/5/pms.html Please see unpack in PMS 11.3.3.13 Misc Commands, quote: lzma-compressed files (*.lzma). Ebuilds must ensure that LZMA Utils is installed. The TODO comment that had a PMS reference was near it before, but with the refactor it has since moved; I'll put back the reference to it. + re.compile('.*\.(bz2?|tbz2)$'):app-arch/bzip2, + re.compile('.*\.(tar(\.(bz2?|gz|Z))?|tbz2|t[bg]z)?$'):app-arch/tar, + re.compile('.*\.(gz|tar\.Z|t[bg]z|[zZ])$'):app-arch/gzip, + re.compile('.*\.tar.xz$'):app-arch/tar, requiring people list gzip, tar, and bzip2 is a significant policy change (which i'm inclined to say is wrong). it needs discussion on the dev mailing list first. Please see unpack in PMS 11.3.3.13 Misc Commands, example quote: gzip-compressed tar files (*.tar.gz, *.tgz, *.tar.Z, *.tbz). Ebuilds must ensure that GNU gzip and GNU tar are installed. To spare on mail length, I don't quote the rest of that enumeration. The @system set in gentoo will ensure these are installed. I understand the wording of PMS (as the dependencies should be expressed somewhere) but in general we prefer to do that in @system. For the same reason all packages do not depend on glibc, or the compiler, or anything else. +def checkArchiveDepends(atoms, catpkg, relative_path, \ + system_set_atoms, needed_unpack_depends, stats, fails): you don't need the \ there because you have paren already to gather things. + for entry in needed_unpack_depends[catpkg]: + if entry not in [atom.cp for atom in atoms if atom != ||]: + stats['unpack.' + mytype + '.missing'] += 1 + fails['unpack.' + mytype + '.missing'].append( \ + relative_path + : %s is missing in %s % \ + (entry, mytype)) i know existing style doesn't follow it, but imo we should avoid string concatenation. it makes the code harder to read imo. key = 'unpack.%s.missing' % mytype stats[key] += 1 fails[key].append(...) +def eapi_has_xz_utils(eapi): + return eapi not in (0, 1, 2) i would use eapi_supports_xz_archives unless there's a pre-existing style -mike Good idea, I'll take these last ones into account in the next version. Thank you everyone for the reviews so far. -- With kind regards, Tom Wijsman (TomWij) Gentoo Developer E-mail address : tom...@gentoo.org GPG Public Key : 6D34E57D GPG Fingerprint : C165 AF18 AB4C 400B C3D2 ABF0 95B2 1FCD 6D34 E57D
[gentoo-portage-dev] [PATCH v3 1/4] pym/portage/package/ebuild/fetch.py: Factor out _get_checksum_failure_max_tries
The current fetch() function is quite long, which makes it hard to know what I can change without adverse side effects. By pulling this logic out of the main function, we get clearer logic in fetch() and more explicit input for the config extraction. Following a suggestion by Tom Wijsman, I put the setting name in a new 'key' variable to cut down on the PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS noise. --- pym/portage/package/ebuild/fetch.py| 61 -- pym/portage/tests/ebuild/test_fetch.py | 45 + 2 files changed, 81 insertions(+), 25 deletions(-) create mode 100644 pym/portage/tests/ebuild/test_fetch.py diff --git a/pym/portage/package/ebuild/fetch.py b/pym/portage/package/ebuild/fetch.py index 5316f03..911500a 100644 --- a/pym/portage/package/ebuild/fetch.py +++ b/pym/portage/package/ebuild/fetch.py @@ -240,6 +240,40 @@ _size_suffix_map = { 'Y' : 80, } + +def _get_checksum_failure_max_tries(settings, default=5): + + Get the maximum number of failed download attempts. + + Generally, downloading the same file repeatedly from + every single available mirror is a waste of bandwidth + and time, so there needs to be a cap. + + key = 'PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS' + v = default + try: + v = int(settings.get(key, default)) + except (ValueError, OverflowError): + writemsg(_(!!! Variable %s contains + non-integer value: '%s'\n) + % (key, settings[key]), + noiselevel=-1) + writemsg(_(!!! Using %s default value: %s\n) + % (key, default), + noiselevel=-1) + v = default + if v 1: + writemsg(_(!!! Variable %s contains + value less than 1: '%s'\n) + % (key, v), + noiselevel=-1) + writemsg(_(!!! Using %s default value: %s\n) + % (key, default), + noiselevel=-1) + v = default + return v + + def fetch(myuris, mysettings, listonly=0, fetchonly=0, locks_in_subdir=.locks, use_locks=1, try_mirrors=1, digests=None, allow_missing_digests=True): @@ -263,31 +297,8 @@ def fetch(myuris, mysettings, listonly=0, fetchonly=0, print(_( \mirror\ mode desired and \mirror\ restriction found; skipping fetch.)) return 1 - # Generally, downloading the same file repeatedly from - # every single available mirror is a waste of bandwidth - # and time, so there needs to be a cap. - checksum_failure_max_tries = 5 - v = checksum_failure_max_tries - try: - v = int(mysettings.get(PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS, - checksum_failure_max_tries)) - except (ValueError, OverflowError): - writemsg(_(!!! Variable PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS -contains non-integer value: '%s'\n) % \ - mysettings[PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS], noiselevel=-1) - writemsg(_(!!! Using PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS - default value: %s\n) % checksum_failure_max_tries, - noiselevel=-1) - v = checksum_failure_max_tries - if v 1: - writemsg(_(!!! Variable PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS -contains value less than 1: '%s'\n) % v, noiselevel=-1) - writemsg(_(!!! Using PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS - default value: %s\n) % checksum_failure_max_tries, - noiselevel=-1) - v = checksum_failure_max_tries - checksum_failure_max_tries = v - del v + checksum_failure_max_tries = _get_checksum_failure_max_tries( + settings=mysettings) fetch_resume_size_default = 350K fetch_resume_size = mysettings.get(PORTAGE_FETCH_RESUME_MIN_SIZE) diff --git a/pym/portage/tests/ebuild/test_fetch.py b/pym/portage/tests/ebuild/test_fetch.py new file mode 100644 index 000..26e0349 --- /dev/null +++ b/pym/portage/tests/ebuild/test_fetch.py @@ -0,0 +1,45 @@ +# Copyright 1998-2013 Gentoo Foundation +# Distributed under the terms of the GNU General Public License v2 + +from portage.package.ebuild.fetch import ( + _get_checksum_failure_max_tries, + ) +from portage.tests import TestCase + + +class FetchTestCase(TestCase): + + Test fetch and it's helper functions. + + The fetch function, as it stands, is too complicated to test + on its own. However, the new helper functions are much more + limited and easier to test. Despite these tests, the helper + functions are internal implementation details, and their + presence and interface may
[gentoo-portage-dev] [PATCH v3 4/4] pym/portage/package/ebuild/fetch.py: Flatten conditionals in _get_fetch_resume_size
Make this easier to read by avoiding nested conditionals [1]. [1]: http://article.gmane.org/gmane.linux.gentoo.portage.devel/4058 Reported-by: Tom Wijsman tom...@gentoo.org --- pym/portage/package/ebuild/fetch.py | 30 ++ 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/pym/portage/package/ebuild/fetch.py b/pym/portage/package/ebuild/fetch.py index 0093a6e..2bf88d8 100644 --- a/pym/portage/package/ebuild/fetch.py +++ b/pym/portage/package/ebuild/fetch.py @@ -276,24 +276,22 @@ def _get_checksum_failure_max_tries(settings, default=5): def _get_fetch_resume_size(settings, default='350K'): key = 'PORTAGE_FETCH_RESUME_MIN_SIZE' - v = settings.get(key) + v = settings.get(key, default) if v is not None: v = .join(v.split()) - if not v: - # If it's empty, silently use the default. - v = default - match = _fetch_resume_size_re.match(v) - if (match is None or - match.group(2).upper() not in _size_suffix_map): - writemsg(_(!!! Variable %s contains an - unrecognized format: '%s'\n) - % (key, settings[key]), - noiselevel=-1) - writemsg(_(!!! Using %s default value: %s\n) - % (key, default), - noiselevel=-1) - v = None - if v is None: + if not v: + # If it's empty, silently use the default. + v = default + match = _fetch_resume_size_re.match(v) + if (match is None or + match.group(2).upper() not in _size_suffix_map): + writemsg(_(!!! Variable %s contains + an unrecognized format: '%s'\n) + % (key, settings[key]), + noiselevel=-1) + writemsg(_(!!! Using %s default value: %s\n) + % (key, default), + noiselevel=-1) v = default match = _fetch_resume_size_re.match(v) v = int(match.group(1)) * \ -- 1.8.5.2.8.g0f6c0d1
Re: [gentoo-portage-dev] [PATCH 2/3] emerge: Rename --autounmask-write to --autounmask
On Sunday 19 January 2014 05:48:33 Alexander Berntsen wrote: On 19/01/14 10:20, Mike Frysinger wrote: typically when we delete/rename an option, we give users a heads up. that means a cycle where the old option exists but emits a warning before switching to the new one. then we can delete it. We should have a news item. i don't think so. i doubt this flag is in common use enough to warrant spamming every user of Gentoo. especially when the people who do use it can easily be migrated per my recommendation. similarly, people who do use this flag but miss the news would be broken. generally speaking, i think it makes a lot more sense to put the notice at the point where it matters. -mike signature.asc Description: This is a digitally signed message part.
Re: [gentoo-portage-dev] [PATCH] Add repoman check to warn if src_prepare/src_configure are used in EAPI 0/1
On Sunday 19 January 2014 11:59:36 Mike Gilbert wrote: On Sun, Jan 19, 2014 at 4:44 AM, Mike Frysinger vap...@gentoo.org wrote: Chromium OS for a long time was restricted to EAPI 4 for two reasons -- it had an old portage version (and upgrading to a newer one regressed performance significantly, so we held off until we could figure out why) I am curious to know more about the performance regression if you can share. Is that something that got fixed, or did you disable some features (like the slot-operator stuff)? we finally tracked it down (was due to new the new FEATURES=merge-sync option. when you're installing 200 to 500 binary packages (with like 32 in parallel), that can easily choke your throughput. some systems saw really excessive latencies (which i would guess was due to their drive taking longer to process dirty blocks). but it took us some time to figure that out as we were making a large version jump and we didn't have too much time to dedicate to tracking it down. lots o bugs to fix. -mike signature.asc Description: This is a digitally signed message part.
Re: [gentoo-portage-dev] [PATCH] Add repoman check to warn if src_prepare/src_configure are used in EAPI 0/1
On Sun, Jan 19, 2014 at 9:47 PM, Mike Frysinger vap...@gentoo.org wrote: On Sunday 19 January 2014 11:59:36 Mike Gilbert wrote: On Sun, Jan 19, 2014 at 4:44 AM, Mike Frysinger vap...@gentoo.org wrote: Chromium OS for a long time was restricted to EAPI 4 for two reasons -- it had an old portage version (and upgrading to a newer one regressed performance significantly, so we held off until we could figure out why) I am curious to know more about the performance regression if you can share. Is that something that got fixed, or did you disable some features (like the slot-operator stuff)? we finally tracked it down (was due to new the new FEATURES=merge-sync option. when you're installing 200 to 500 binary packages (with like 32 in parallel), that can easily choke your throughput. some systems saw really excessive latencies (which i would guess was due to their drive taking longer to process dirty blocks). but it took us some time to figure that out as we were making a large version jump and we didn't have too much time to dedicate to tracking it down. lots o bugs to fix. -mike Are you still doing crazy crap like disabling all of the portage locking? ;p -A
Re: [gentoo-portage-dev] [PATCH] Add repoman check to warn if src_prepare/src_configure are used in EAPI 0/1
On Monday 20 January 2014 01:02:31 Alec Warner wrote: Are you still doing crazy crap like disabling all of the portage locking? ;p yes no. we make sure we hold the right locks and don't do operations that otherwise cause problems. then we disable the core lock grabbing :P. the current locking logic is pessimistic and causes way too much of a slow down for us to do anything else. -mike signature.asc Description: This is a digitally signed message part.