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: > > > - > > > "portdir":self.settings["snapshot_cache_path"]+"/portage",\ > > > - > > > "distdir":self.settings["distdir"],"port_tmpdir":"tmpfs"} > > > + > > > "portdir":normpath(self.settings["snapshot_cache_path"]+"/" + > > > self.settings["repo_name"]), > > > + > > > "distdir":self.settings["distdir"],"port_tmpdir":"tmpfs"} > > > > Can we do this for the mountmap defaults too, and just override the > > mountmap special cases in the SNAPCACHE branch? I think this would be > > a good commit for that (or we can do it in a commit after this). > > > > I don't think the 'portage' → self.settings["repo_name"] replacement > > should go in this commit. > > repo_name variable is already defined. This one got missed in the > change. You want it in the next commit by itself? Can do.
That's what I want ;). Just to avoid making this one more complicated
than it needs to be.
> > > - 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 don't know where we stand on 'x=y' vs 'x = y', but I'd prefer the
> > latter here.
>
> I do too, but I thought it was agreed to fix all that separately
> later :/
Matt had me work some ' = ' spacing into the last series, but later
works for me.
> > I also think that the PORT_LOGDIR environment variable
> > should be:
> >
> > self.env["PORT_LOGDIR"] = self.target_mounts["port_logdir"]
>
> meh, but I also prefer some uniformity. I prefer all-caps for
> constants. In this case, the end result later in a new defaults.py
> file... since it is internal, why mix case in keys. They are also
> pushed into the bash environment for the chroot scripts to use. So,
> again, uniformity can be your friend when making changes.
> Especially with a large complex app like catalyst.
I don't care about caps, I was trying to fix settings → target_mounts,
since settings holds the mount source ;).
> > Nothing in the chroot should care where the source comes from ;).
> > I'd considering renaming mountmap → source_mounts for clarity,
> > using existing settings to override source_mounts at
> > initialization, and using source_mounts thereafter.
>
> mounts and mountmap were existing variable names. With some of the
> flack I've gotten over my choice of variable name already, now you
> want me to go through even more??? Do it in a separate commit.
> (after the rewrite patches are done for.)
Fair enough ;).
> > > + #print "bind(); cmd =", cmd
> >
> > If it's not useful enough to print, I don't think we should commit it
> > ;).
>
> There are various of these added to the code. They are EXTREMELY
> helpful and even necessary to debug major code changes. Of which the
> mounts, mountmap changes were one of the toughest to debug.
>
> Both you and I have already stated that we want to convert to using
> python's logging. These are just a first step towards that goal. All
> that is needed is to search out the #print's and replace them with teh
> proper logging statement.
Ok.
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
