Created attachment 326724 [details] Version 1 patch to port getbinpkg to use urllib2 for proxy-awareness Steps to Reproduce: ------------------- () Reside behind a typical corporate firewall that supplies proxies for the common protocols: rsync, ftp, http, https () export appropriate proxy-settings environment variables RSYNC_PROXY, all_proxy, ftp_proxy, http_proxy, https_proxy, and no_proxy. () set the appropriate settings for http-proxy-host, http-proxy-port, and http-proxy-exceptions in your .subversion/servers/[global] section () repo-init and repo-sync google-chromium source-tree, see http://dev.chromium.org/chromium-os/developer-guide The problem is not specific to that project tree, however that project tree uses 500-plus ebuild projects with a wide variety of repository URLs, so it exercises portage heavily. () attempt "chromite/bin/cros_sdk --replace --bootstrap" () attempt "cros_sdk --enter: () attempt "./setup_board --board=$amd64-corei7 --force --default" () attempt "./build_packages --showoutput" Expected Result: ---------------- () The "bootstrap", "setup_board", and "build_packages" steps that use portage should successfully access any repository URLs through the proxies. () In cases where optional units such as files or package metadata is NOT actually present (e.g., a proxy-capable browser reports these as 404-not-present) the steps that use portage should promptly determine that these are not-present. Observed Result: ---------------- () packages that are actually present and use the old, deprecated package-info-as directory protocol fail to find these packages completely. () Optional packages that are NOT actually present get an error attempting the new package-info-as-index protocol properly (since this goes through "wget" or "curl" which properly handle proxy settings) but then fall back to using the old protocol code which isn't proxy-aware so that access attempts time-out. Since there are many such timeouts and the timeouts have to remain long due to slow repos, this can add many hours to the overall build time even if it ultimate succeeds since these things are optional. A streamlined unit-test that exhibits the problem: cd <portage-installation-dir>/portage/pym python >>> import portage.getbinpkg >>> portage.getbinpkg.dir_get_list("https://commondatastorage.googleapis.com/chromeos-prebuilt/host/amd64/amd64-generic/paladin-R24-3041.0.0-rc8/packages/") ...and then observe that it takes basically forever to time-out and notice that it thinks the network is unreachable. Additional Notes: ----------------- I will include as "Revision For Comment" a patch that fixes this problem. Patch is applied on commit 65e69242814553b826b9277495a5f62b79569269 dated Tue Oct 16 01:33:54 2012 (about 8-ish patches after the v2.2.0_alpha138 tag). My root-cause analysis indicates that the basic problem is in getbinpkg in the five procedures that use httplib directly: dir_get_list() file_get_metadata() file_get() file_get_lib() dir_get_metadata() Only two of these, file_get() and dir_get_metadata() are called outside of getbinpkg, by dbapi/bintree. The patch ports these five functions to use urllib2, which is proxy-aware. Due the call-graph relationships between all these procedures, calling file_get() and dir_get_list() cover the patched code. The chromeos-build scenario described above could be worked around by simply altering the dbapi/bintree code so that added a proxy-aware url-reachability check in the old-protocol fallback code. However, in general portage can be used in a wider variety of situations that exercise any of the httplib-using procedures in getbinpkg. So this patch is more in the form of a "correct fix" making all procedures proxy-aware than that workaround would be. The patch passes unit-tests that basically boil down to: cd <portage-installation-dir>/portage/pym python >>> import portage.getbinpkg >>> file_get(<variety-of-urls, sys.stdout) >>> dir_get_list(<variety-of-directory-urls>) Unfortunately I am as yet unable to do an end-to-end test with the chromeos build using this version of portage. Chromeos currently uses an older portage version (2.1.10.11-r9), and the current version of portage does not work as a drop-in replacement. The chromeos call from "parallel_emerge.py" down into portage gets a "invalid file descriptor" error down in the guts of SpawnProcess. Unclear yet whether the fault is in portage or chromeos. I will post a patch and/or bug-report to the correct project when I get that issue root-caused. The patch has some known problems at this point: () Needs to import urllib3 names and fallback to importing urllib2 names () directory listings through sftp urls are not proxy-enabled, although I retained the old non-proxy-aware equivalent code for these. () Any sftp code would probably use paramiko, which is not (yet?) a standard part of python, so if paramiko is used the code should gracefully catch paramiko import exceptions and fall back to not supporting sftp () Needs to have an end-to-end test with chromeos build I will be proceeding with these modifications, however, I wanted to get the discussion process started with this "RFC" version of the patch. emerge --info output: --------------------- Portage 2.1.10.11-r9 (!/usr/local/portage/chromiumos/profiles/default/linux/amd64/10.0, gcc-4.6.3, glibc-2.15-r4, 3.5.2-1.fc17.x86_64 x86_64) ================================================================= System uname: Linux-3.5.2-1.fc17.x86_64-x86_64-Intel-R-_Core-TM-_i7-3930K_CPU_@_3.20GHz-with-glibc2.2.5 Timestamp of tree: Unknown ccache version 3.1.6 [disabled] app-shells/bash: 4.2_p20 dev-java/java-config: 2.1.10 dev-lang/python: 2.6.8::chromiumos dev-util/ccache: 3.1.6 dev-util/cmake: 2.8.6-r4 sys-apps/baselayout: 2.0.1-r228::chromiumos sys-apps/sandbox: 2.4 sys-devel/autoconf: 2.68 sys-devel/automake: 1.10.3::gentoo, 1.11.1 sys-devel/binutils: 2.21.1::chromiumos sys-devel/gcc: 4.6.3-r29::chromiumos sys-devel/gcc-config: 1.7.3 sys-devel/libtool: 2.4-r1 sys-kernel/linux-headers: 3.4-r1::chromiumos (virtual/os-headers) Repositories: portage-stable x-crossdev chromiumos private-overlays-chromeos-overlay ACCEPT_KEYWORDS="amd64" ACCEPT_LICENSE="* -@EULA" CBUILD="x86_64-pc-linux-gnu" CFLAGS="-O2 -pipe" CHOST="x86_64-pc-linux-gnu" CONFIG_PROTECT="/etc/hosts /etc/locale.gen /etc/localtime /etc/make.conf" CONFIG_PROTECT_MASK="/etc/ca-certificates.conf /etc/env.d /etc/env.d/java/ /etc/fonts/fonts.conf /etc/gconf /etc/gentoo-release /etc/revdep-rebuild /etc/sandbox.d /etc/terminfo" CXXFLAGS="-O2 -pipe" DISTDIR="/var/lib/portage/distfiles" EMERGE_DEFAULT_OPTS="--oneshot" FEATURES="allow-missing-manifests assume-digests binpkg-logs buildpkg clean-logs distlocks fixlafiles fixpackages news parallel-fetch parallel-install protect-owned sandbox sfperms unmerge-logs unmerge-orphans userfetch userpriv usersandbox" FFLAGS="" GENTOO_MIRRORS="https://commondatastorage.googleapis.com/chromeos-localmirror https://commondatastorage.googleapis.com/chromeos-mirror/gentoo" INSTALL_MASK=" /usr/lib*/*.la /etc/init.d /etc/conf.d /etc/logrotate.d " LANG="en_US.UTF-8" LDFLAGS="-Wl,-O1" MAKEOPTS="-j12" PKGDIR="/var/lib/portage/pkgs" PORTAGE_BUNZIP2_COMMAND="pbunzip2 --ignore-trailing-garbage=1" PORTAGE_BZIP2_COMMAND="pbzip2" PORTAGE_CONFIGROOT="/" PORTAGE_RSYNC_OPTS="--recursive --links --safe-links --perms --times --compress --force --whole-file --delete --stats --timeout=180 --exclude=/distfiles --exclude=/local --exclude=/packages" PORTAGE_TMPDIR="/var/tmp" PORTDIR="/usr/local/portage/stable" PORTDIR_OVERLAY="/usr/local/portage/crossdev /usr/local/portage/chromiumos /home/pcdrews/trunk/src/private-overlays/chromeos-overlay" SYNC="rsync://rsync.gentoo.org/gentoo-portage" USE="acl amd64 bzip2 cracklib cros_host crypt cxx dri expat fortran hardened iconv ipv6 mmx modules mudflap multilib ncurses nls nptl nptlonly openmp pic pie python readline sse sse2 ssl sysfs unicode xorg zlib" ALSA_CARDS="ali5451 als4000 atiixp atiixp-modem bt87x ca0106 cmipci emu10k1x ens1370 ens1371 es1938 es1968 fm801 hda-intel intel8x0 intel8x0m maestro3 trident usb-audio via82xx via82xx-modem ymfpci" ALSA_PCM_PLUGINS="adpcm alaw asym copy dmix dshare dsnoop empty extplug file hooks iec958 ioplug ladspa lfloat linear meter mmap_emul mulaw multi null plug rate route share shm softvol" APACHE2_MODULES="actions alias auth_basic authn_alias authn_anon authn_dbm authn_default authn_file authz_dbm authz_default authz_groupfile authz_host authz_owner authz_user autoindex cache dav dav_fs dav_lock deflate dir disk_cache env expires ext_filter file_cache filter headers include info log_config logio mem_cache mime mime_magic negotiation rewrite setenvif speling status unique_id userdir usertrack vhost_alias" ELIBC="glibc" INPUT_DEVICES="keyboard mouse evdev" KERNEL="linux" LCD_DEVICES="bayrad cfontz cfontz633 glk hd44780 lb216 lcdm001 mtxorb ncurses text" USERLAND="GNU" VIDEO_CARDS="fbdev glint intel mach64 mga neomagic nv r128 radeon savage sis tdfx trident vesa vga via vmware dummy v4l" Unset: CPPFLAGS, CTARGET, LC_ALL, LINGUAS, PORTAGE_COMPRESS, PORTAGE_COMPRESS_FLAGS, PORTAGE_RSYNC_EXTRA_OPTS
For cross-reference, the original report/patch regarding this topic in chromeos is: https://gerrit.chromium.org/gerrit/#/c/34294/
(In reply to comment #1) > For cross-reference, the original report/patch regarding this topic in > chromeos is: > > https://gerrit.chromium.org/gerrit/#/c/34294/ As mentioned in my comment there, none of the functions that your patch modifies (in the getbinpkg module) should be called unless it falls back to the deprecated protocol inside bintree.py. I would really like to avoid making any modifications to the deprecated code, since it should be entirely irrelevant these days. In fact, I'd like to remove the deprecated code.
Removing the deprecated code (in dbapi/bintree?) would actually be wonderful. All the new-protocol code is based ultimately on getbinpkg.file_get, which uses wget and curl, which are proxy-capable, so there is no problem. The actual symptom I observe is that the new-protocol code, when it encounters a not-there binpkg, falls back to the old-protocol code. The old-protocol code is doomed to fail also because the binpkg is not-there. But it takes outrageously long to do so, since it times-out due to not being proxy-capable. Has the old-protocol code been deprecated long enough that it is time to remove it and let legacy projects that have neglected to upgrade die of their own neglect? This would be MUCH simpler and less-invasive than my current patch. And would solve my problem neatly. Say the word, I'll re-work this patch as that approach instead. Basically boils down to deleting a big chunk of dbapi.bintree.binarytree.populate(), probably lines 1492-1090. Which would leave some dead-code in getbinpkg that could also be removed.
(In reply to comment #3) > Has the old-protocol code been deprecated long enough that it is time to > remove it and let legacy projects that have neglected to upgrade die of > their own neglect? Yes, I've gone ahead and removed it: http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=d159be545ecb431addc3e7bb31a8ed8cfd9984af Is there anything else needed?
(In reply to comment #4) > (In reply to comment #3) > > Has the old-protocol code been deprecated long enough that it is time to > > remove it and let legacy projects that have neglected to upgrade die of > > their own neglect? > > Yes, I've gone ahead and removed it: > > http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit; > h=d159be545ecb431addc3e7bb31a8ed8cfd9984af > > Is there anything else needed? This is released in portage-2.1.11.30 and 2.2.0_alpha141.
Excellent! I will try this out in the context of chromeos builds and confirm the fix here. Thanks for the prompt attention on this.
I back-ported Zac's patch to the 2.1.10.11-r9 version of portage that the chromeos build uses, and am happy to report that it works like a charm. I'll mark this as "resolved - worksforme"
