Re: [gentoo-catalyst] [PATCH 2/2] modules/generic_stage_target.py, modules/stage1_target.py: Add a target_mounts dictionary

[W. Trevor King]

On Wed, Dec 18, 2013 at 11:47:25PM -0800, Brian Dolbec wrote:
> On Wed, 2013-12-18 at 23:34 -0800, Brian Dolbec wrote:
> > On Tue, 2013-12-17 at 21:35 -0800, W. Trevor King wrote:
> > > On Tue, Dec 17, 2013 at 09:18:04PM -0800, Brian Dolbec wrote:
> > > > On Tue, 2013-12-17 at 19:28 -0800, W. Trevor King wrote:
> > > > > On Tue, Dec 17, 2013 at 05:07:27PM -0800, Brian Dolbec wrote:
> > > > > > -			self.mounts.append("/var/log/portage")
> > > > > > -			self.mountmap["/var/log/portage"]=self.settings["port_logdir"]
> > > > > > -			self.env["PORT_LOGDIR"]="/var/log/portage"
> > > > > > +			self.mounts.append("port_logdir")
> > > > > > +			self.mountmap["port_logdir"]=self.settings["port_logdir"]
> > > > > > +			self.env["PORT_LOGDIR"]=self.settings["port_logdir"]
> > > > >
> > > > >  I also think that the PORT_LOGDIR environment variable
> > > > > should be:
> > > > >
> > > > >   self.env["PORT_LOGDIR"] = self.target_mounts["port_logdir"]
> > >
> > > I don't care about caps, I was trying to fix settings → target_mounts,
> > > since settings holds the mount source ;).
> >
> > No, this code block is adding it to the environment if defined in
> > settings.  This patch was just removing the hard coded paths.
> >
> > This is the full if block:
> >
> > 		if "port_logdir" in self.settings:
> > 			self.mounts.append("port_logdir")
> > 			self.mountmap["port_logdir"]=self.settings["port_logdir"]
> > 			self.env["PORT_LOGDIR"]=self.settings["port_logdir"]
> > 			self.env["PORT_LOGDIR_CLEAN"]='find "${PORT_LOGDIR}" -type f ! -name "summary.log*" -mtime +30 -delete'
> >
> > So far it is not used from the new TARGET_MOUNTS_DEFAULTS.
> > It is so far only enabled if uncommented and defined in catalyst.conf or
> > from the cli.
> >
> > Should I remove it from the TARGET_MOUNTS_DEFAULTS for the time being?
>
> What we could do is add a "PortageLog" to the options setting which
> would turn on and use the defaults unless otherwise overridden by
> setting the port_logdir config option.  But that would be something to
> add later.
>
> This patch is to fix/complete the mounts, mountmap changes from the
> hard-coded paths and paths as keys.

Who is consuming the env dict?  I thought it was for scripts running
in the chroot, in which case it can't possibly care about the mount
source, but might conceivably care about the mount target.  In any
case, current usage in your pending branch is inconsistent:

  $ git grep 'self\.env\[.*\] *=' origin/pending --
  origin/pending:modules/generic_stage_target.py:    self.env["CCACHE_DIR"] = self.target_mounts["ccache"]
  origin/pending:modules/generic_stage_target.py:    self.env["PATH"]="/usr/lib/icecc/bin:"+self.env["PATH"]
  origin/pending:modules/generic_stage_target.py:    self.env["PORT_LOGDIR"]=self.settings["port_logdir"]
  origin/pending:modules/generic_stage_target.py:    self.env["PORT_LOGDIR_CLEAN"]='find "${PORT_LOGDIR}" -type f ! -name "summary.log*" -mtime +30 -delete'
  origin/pending:modules/generic_stage_target.py:    self.env[varname]=self.settings[x]
  origin/pending:modules/generic_stage_target.py:    self.env[varname]=string.join(self.settings[x])
  origin/pending:modules/generic_stage_target.py:    self.env[varname]="true"
  origin/pending:modules/generic_stage_target.py:    self.env[varname]="false"
  origin/pending:modules/generic_stage_target.py:    self.env["MAKEOPTS"]=self.settings["makeopts"]
  origin/pending:modules/generic_stage_target.py:    self.env[kname+"_kernelopts"]=myopts
  origin/pending:modules/generic_stage_target.py:    self.env[kname+"_kernelopts"]=""
  origin/pending:modules/generic_stage_target.py:    self.env["clst_kextraversion"]=\
  origin/pending:modules/generic_target.py:    self.env["PATH"]="/bin:/sbin:/usr/bin:/usr/sbin"

CCACHE_DIR is set using target_mounts (which makes sense to me), but
PORT_LOGDIR is set using settings (which does not make sense to me).
For example, if I decide to configure port_logdir (via my Catalyst
settings) to /some/wonky/path/on/the/host/system, I want Catalyst to
bind-mount that to /var/tmp/portage
(TARGET_MOUNTS_DEFAULTS['port_logdir']) and then have emerges in the
chroot use /var/tmp/portage.  If emerges in the chroot try to use
/some/wonky/path/on/the/host/system thinks are not going to work.

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