On Tue, Dec 17, 2013 at 05:07:27PM -0800, Brian Dolbec wrote:
> +target_mounts = {
> + "proc": "/proc",
> + …
> + "port_logdir": "/var/log/portage",
> + }
> …
> @@ -173,11 +188,12 @@ class generic_stage_target(generic_target):
> file_locate(self.settings,["portage_confdir"],expand=0)
>
> """ Setup our mount points """
> + self.target_mounts = target_mounts.copy()
> if "SNAPCACHE" in self.settings:
> self.mounts=["proc", "dev", "portdir", "distdir",
> "port_tmpdir"]
> self.mountmap={"proc":"/proc", "dev":"/dev",
> "devpts":"/dev/pts",
> -
> "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. It also looks like there is some leading
non-tab whitespace at the beginning of your new distdir line, which
was otherwise unchanged.
> - 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 also think that the PORT_LOGDIR environment variable
should be:
self.env["PORT_LOGDIR"] = self.target_mounts["port_logdir"]
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.
> def bind(self):
> for x in self.mounts:
> - if not os.path.exists(self.settings["chroot_path"] +
> self.mountmap[x]):
> - os.makedirs(self.settings["chroot_path"]+x,0755)
> + #print "bind(); x =", x
> + target = normpath(self.settings["chroot_path"] +
> self.target_mounts[x])
> + if not os.path.exists(target):
> + os.makedirs(target, 0755)
>
> if not os.path.exists(self.mountmap[x]):
> if not self.mountmap[x] == "tmpfs":
> - os.makedirs(self.mountmap[x],0755)
> + os.makedirs(self.mountmap[x], 0755)
>
> src=self.mountmap[x]
Might as well shift this src declaration up and use it instead of
self.mountmap[x] earlier. Not exactly required for this commit, but
you are defining target, so it's not too much of a stretch ;).
> - if "SNAPCACHE" in self.settings and x == "/usr/portage":
> + #print "bind(); src =", src
> + if "SNAPCACHE" in self.settings and x ==
> self.settings["portdir"]:
Oops. x is the mountmap key, so I think this should be 'portdir'.
> + #print "bind(); cmd =", cmd
If it's not useful enough to print, I don't think we should commit it
;). I'll be happy once we're using subprocess :p.
A few typos, but I think this makes mount handling much more sane :).
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
