[buildd-tools-devel] Bug#718127: schroot: CHROOT_SESSION_PURGE and tar-based source chroots

[Aurelien Jarno]

On Thu, Jul 10, 2014 at 09:07:57PM +0100, Roger Leigh wrote:
> On Wed, Jul 09, 2014 at 10:15:19PM +0200, Aurelien Jarno wrote:
> > On Sun, Jul 28, 2013 at 09:52:22AM +0100, Roger Leigh wrote:
> > > Package: schroot
> > > Version: 1.7.0-1
> > > Severity: normal
> > > 
> > > Check if CHROOT_SESSION_PURGE is being set for file source chroots.
> > > If it is, should this be changed?
> > > 
> > > We are purging the chroot.  But if we're repacking, then we are
> > > preserving its state.
> > > 
> > > We should have an additional environment variable so that setup
> > > scripts can determine if they are working with a source chroot.
> > 
> > Indeed this buildds are using this variable to determine if the current
> > chroot is a source chroot or not. Since we switched to tar based chroots
> > instead of lvm one, this variable is always defined. This has broken at
> > least one security update on some buildds.
> 
> What was the reason for the breakage?  Is this an issue with schroot
> itself, or the assumptions being made in the 99builddsourceslist
> script?  Could you provide some more detail please?  I didn't fully
> see how the details you provided related to this.

The problem is due to the assumption being made in the
99builddsourceslist script. It assumes that CHROOT_SESSION_PURGE is not
set for a source chroot. It means we ended up with additional entries in
sources.list for source chroot, and after an upgrade of the chroot, we
ended up with packages which should not have been there.

> > I therefore think we should get this fixed asap, either by not setting
> > this variable for source tar based chroots, or by providing a new
> > variable. In the meantime, is there another (even complex) way to know
> > if a chroot is a source one or not?
> 
> As a really hacky temporary workaround, check for "(source chroot)"
> in CHROOT_DESCRIPTION.  This will not be changed for 1.6.x, but it's
> obviously not something to rely on long term since it's purely a
> text string.  And it's also localised so will vary depending on the
> locale, so will only be reliable for the C locale.  And there's
> nothing stopping this being in the initial description so it might
> cause false positives.

Thanks for the hint, I'll try to implement something based on that.

> I think the correct solution will be to add a
> 
>   CHROOT_SESSION_SOURCE=true|false
> 
> setup environment variable, with corresponding
> 
>   SESSION_SOURCE  = 1 << 3  ///< The chroot is a source chroot.
> 
> setup flag to be set by the chroot facets.  The reason for this is that
> SESSION_PURGE is indicating that the session will be purged, and
> nothing more than that.  It does not make any implied statement about
> whether or not it's a source chroot or not.  For most chroot types,
> this is only set for non-source chroots since it uses a different
> chroot type for the source chroot, but this isn't the case for file
> chroots where both the source and non-source chroots are both
> purged.  I'd rather not overload this flag with unwarranted meaning,
> so I think adding an additional flag is the way to go here.
> 

That's looks quite reasonable to me.

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien at aurel32.net                 http://www.aurel32.net