On Sun, Aug 29, 2010 at 12:46:52PM +0100, Roger Leigh wrote: > On Sun, Aug 29, 2010 at 12:07:22PM +0200, Marc 'HE' Brockschmidt wrote: > > Roger Leigh <rle...@codelibre.net> writes: > > > On Sat, Aug 28, 2010 at 01:33:10PM +0200, Marc 'HE' Brockschmidt wrote: > > >> Roger Leigh <rle...@codelibre.net> writes: > > >>> Please could you unblock schroot 1.4.10-2? > > >> 127 files changed, 6132 insertions(+), 6896 deletions(-) > > >> > > >> *cough* This is a bit much and far away from properly reviewable - even > > >> if we filter out all autoconf crop and documentation updates. Are all of > > >> these changes really needed? > > > Yes. If you look at the diffstat (below), > > > > We always do that when reviewing changes. In this case, this ends up with > > h...@franck:~$ filterdiff -i *.cc -i *.h < schroot-diff | diffstat > > [...] > > 79 files changed, 1255 insertions(+), 726 deletions(-) > > > > As schroot is an important part of the Debian infrastructure, I'm even > > more reluctant to just accept these changes into stable. How have these > > changes been tested? > > I've done hand testing of all changes myself in addition to testing > with the included testsuite. […]
> I've also tested it with sbuild, and all the releases are all > known to function correctly when run with sbuild. > > Releases later than 1.4.7 also deprecate features for removal in > squeeze+1. If we don't have an update in squeeze, I'll need to > postpone some changes until squeeze+2 which I would really prefer > not to do. I'm afraid I've been ill for the last few days, but what I have done is to separate the big diff into separate sets of changes, which means you can look at things with most of the "noise" taken out. The patches are available from http://people.debian.org/~rleigh/schroot-1.4.11-patches/ Summary of each patch: race-fix.diff Work around a kernel bug Fixes bugs: #593516 Changes • Use an flock file lock when reading /proc/mounts to reduce the change of racing reading /proc/mounts, which could potentially lead to catastrophic dataloss if we rm -rf a mounted filesystem the kernel skipped when reading /proc/mounts. This isn't a true fix, since programs mounting/unmounting without taking the same lock will still trigger this, but the chances are vastly reduced. Required: Yes. This is a very serious bug, and needs fixing for squeeze. Chance of triggering is low, but if running many parallel schroot instances, the chance of beginning/ending a session at the same time is greatly increased. Independently tested, and tested by me. session-name-check.diff Add strict checking for chroot names Fixes bugs: #589889 Changes • Add is_valid_sessionname function; based on is_valid_filename Required: Yes. Closes potential security hole by preventing session names from containing relative pathnames which could be abused to overwrite files with root privileges. 1.4.7 is not vulnerable, but uses a rather stricter check which means many valid session names are not permitted. Tested. environment.diff Correctly set up user environment in chroot Fixes bugs: #589830, #589917 Changes: • sbuild::auth::get_minimal_environment unconditionally preserves TERM and SHELL in the environment. LOGNAME and USER are also unconditionally preserved. Severity: very important (bad environment under some circumstances, fixes a regression introduced in 1.4.7) Required for squeeze: Yes (1.4.7 is buggy) chroot-namespace.diff Fixes bugs: #512131 Addition of support for chroot namespaces. Changes • sbuild::chroot_config methods now include a "namespace" argument • Validate session names (for security; see above) • New functions to search namespaces (find*/lookup*) • Chroot listing/info functions also print namespaces • Add compatibility aliases to preserve full backward compatibility with earlier releases Required: No. However, this feature has been long requested by several buildd admins (to allow all source chroots to be updated using the accompanying --all-source-chroots option (below) which this enables). If it's not in squeeze, we won't be able to update dependent tools and deprecate the compatibility aliases until squeeze+1, which is IMO too long for a relatively simple change. Most changes here are simply adding a chroot_namespace parameter which touches most functions, and adjusting existing code to use the new find_*/lookup_* functions. This has been extensively tested. namespace-session.diff Session support for chroot namespaces Fixes bugs: N/A Changes • Don't use a copy of the chroot_config; pass in chroot objects directly. This means a number of failure modes are no longer possible, increasing robustness and security. Replace string_list with chroot_list • Remove a number of static assertions and checks which are no longer required. • Changes to dchroot session classes; update to use updated interfaces Required: It's a prerequisite for namespace support. Tested. schroot-options.diff Addition of --all-source-chroots option for schroot Fixes bugs: N/A Changes • Add all_source_chroots to schroot options and --all-source-chroots command-line option to set it; update option validation to work with the new option • When loading schroot.conf, use namespaces. • Use new chroot config search methods when setting up chroots for use • Use new chroot config print methods when displaying chroot info • Use new schroot chroot list interface Required: It's a prerequisite for namespace support. Tested. test.diff Testsuite updates Fixes bugs: N/A Changes • Updates to tests for chroot namespaces Required: It's a prerequisite for namespace support dchroot-deprecated.diff Don't use deprecated keyfile names. Fixes bugs: N/A Changes • Use "directory" rather than "location" when creating schroot configuration from dchroot.conf Required: No. However, this removes lots of annoying deprecation warnings which the user can do nothing to correct. Trivial one- liner change which is demonstrably correct (and has been tested to be so). device-type-refactor.diff Move virtual method into a different file Fixes bugs: N/A Changes • Move get_chroot-type method from sbuild::chroot_block_device_base to sbuild::chroot_block_device, which is where it belongs since sbuild::chroot_block_device_base is not an instatiatable type. Required: No. Not strictly a bug, but was not good practice. Tested. namespace-remove.diff Code cleanup Fixes bugs: N/A Changes • Removal of unnecessary use of sbuild:: namespace Required: No. Changes are simple search-and-replace which gives a big diff but isn't actually changing anything. Tested. debian.diff Updates to Debian packaging Fixes bugs: N/A Changes • Separate doxygen documentation building so that it's not build redundantly for all arches by only building in a build-indep rule. Required for squeeze: Possibly not, but schroot 1.4.7 will not autobuild currently due to doxygen bugs this works around. Tested successfully from 1.4.10-1 to 1.4.11. session-name-unify.diff Unify chroot/session/keyfile names Fixes bugs: #593256 Changes • Unify chroot get_name/get_session_name/get_keyfile_name methods with single get_name method. • All functions using get_session_name/get_keyfile_name updated to use get_name • Remove get_active and use get_facet<chroot_facet_session> to check for presence of session facet [to check if chroot is session chroot] Required: No. This is straightforward search-and-replace code refactoring, so should be a safe change. Makes the code far more clean and robust. Tested. po.diff Translation updates Fixes bugs: #594024 #594239 #593622 #588734 #588963 #589079 Changes • Updates for various translations • Removal of file/line numbers to make translation updates more diff-friendly in the future. Required for squeeze: Yes. build.diff Changes to build intrastructure Fixes bugs: #589658 Changes • Addition to doc rule to top-level Makefile for building doxygen docs and updates to improve documentation building • Bump of version number • Autodetection of doxygen • Correct autodetection of LVM, Btrfs and losetup • Correct Boost program_options autodetection Required for squeeze: Yes. The configure changes are required for correct feature autodetection, which can otherwise fail and result in a misbuild. Tested. doc.diff Changes to documentation (no code changes) Fixes bugs: N/A Changes • Update NEWS (chroot namespaces and feature deprecations) • Update README (document configure options) • Doxygen template update • ChangeLog update • schrootl.conf template update Required for squeeze: N/A. This is just documentation accompanying the rest of the changes. doxygen.diff Updates to API reference (no code changes) Fixes bugs: N/A Changes • Updates to inline doxygen code comments Required for squeeze: N/A. This is just documentation accompanying the rest of the changes. This updates the API reference to give the documentation 100% code coverage. #595647 is the only outstanding issue (and it's not a regression, just behaviour not matching documentation), and is just a two-liner to fix. Regards, Roger -- .''`. Roger Leigh : :' : Debian GNU/Linux http://people.debian.org/~rleigh/ `. `' Printing on GNU/Linux? http://gutenprint.sourceforge.net/ `- GPG Public Key: 0x25BFB848 Please GPG sign your mail.
signature.asc
Description: Digital signature