[pacman-dev] [PATCH 1/3] Add pmtest.rootjoin() and clean up some duplicate joins.
Andrew Gregory
andrew.gregory.8 at gmail.com
Fri Oct 4 11:20:11 EDT 2013
On 10/02/13 at 11:48am, Jeremy Heiner wrote:
> Many calls to os.path.join(root, _) were sprinkled around, in some
> cases redundantly calculating the same exact path. The rootjoin method
> was added to do this specific join and the "sprinkles" redirected. The
> parameters to util.mkfile were tweaked to discourage any more of these
> redundancies from creeping in.
>
> The inverse calculation was being done in two places in two different
> ways, so pmtest.rootremove() was added to unify them.
>
> Signed-off-by: Jeremy Heiner <ScalaProtractor at gmail.com>
> ---
> test/pacman/pmdb.py | 2 +-
> test/pacman/pmpkg.py | 2 +-
> test/pacman/pmtest.py | 52 +++++++++++++++++++++++++++++++--------------------
> test/pacman/util.py | 9 ++++-----
> 4 files changed, 38 insertions(+), 27 deletions(-)
>
> diff --git a/test/pacman/pmdb.py b/test/pacman/pmdb.py
> index b7b3522..c3290e3 100644
> --- a/test/pacman/pmdb.py
> +++ b/test/pacman/pmdb.py
> @@ -237,7 +237,7 @@ def generate(self):
> path = os.path.join(self.dbdir, pkg.fullname())
> util.mkdir(path)
> for name, data in entry.iteritems():
> - util.mkfile(path, name, data)
> + util.mkfile(os.path.join(path, name), data)
>
> if self.dbfile:
> tar = tarfile.open(self.dbfile, "w:gz")
> diff --git a/test/pacman/pmpkg.py b/test/pacman/pmpkg.py
> index 9b3147a..9c9e447 100644
> --- a/test/pacman/pmpkg.py
> +++ b/test/pacman/pmpkg.py
> @@ -180,8 +180,8 @@ def makepkg(self, path):
> def install_package(self, root):
> """Install the package in the given root."""
> for f in self.files:
> - util.mkfile(root, f, f)
> path = os.path.join(root, f)
> + util.mkfile(path, f)
> if os.path.isfile(path):
> os.utime(path, (355, 355))
>
> diff --git a/test/pacman/pmtest.py b/test/pacman/pmtest.py
> index b343d55..4101546 100644
> --- a/test/pacman/pmtest.py
> +++ b/test/pacman/pmtest.py
> @@ -74,6 +74,17 @@ def addrule(self, rulename):
> rule = pmrule.pmrule(rulename)
> self.rules.append(rule)
>
> + def rootjoin(self, path):
> + return os.path.join(self.root, path)
> +
> + def rootremove(self, path):
> + end = len(self.root)
> + if ( end < len(path)
> + and os.sep == path[end]
> + and path.startswith(self.root) ):
> + return path[end+1:]
> + raise ValueError("path not under root: %s" % path)
> +
I'm not sure about the utility of these functions. The checks in
rootremove are unnecessary because both places it's used are
guaranteed to start with root. If those are removed, both of these
are basically just aliases.
> def load(self):
> # Reset test parameters
> self.result = {
> @@ -114,27 +125,27 @@ def generate(self, pacman):
>
> # Create directory structure
> vprint(" Creating directory structure:")
> - dbdir = os.path.join(self.root, util.PM_SYNCDBPATH)
> - cachedir = os.path.join(self.root, util.PM_CACHEDIR)
> - syncdir = os.path.join(self.root, util.SYNCREPO)
> - tmpdir = os.path.join(self.root, util.TMPDIR)
> - logdir = os.path.join(self.root, os.path.dirname(util.LOGFILE))
> - etcdir = os.path.join(self.root, os.path.dirname(util.PACCONF))
> - bindir = os.path.join(self.root, "bin")
> + dbdir = self.rootjoin(util.PM_SYNCDBPATH)
> + cachedir = self.rootjoin(util.PM_CACHEDIR)
> + syncdir = self.rootjoin(util.SYNCREPO)
> + tmpdir = self.rootjoin(util.TMPDIR)
> + logdir = self.rootjoin(os.path.dirname(util.LOGFILE))
> + etcdir = self.rootjoin(os.path.dirname(util.PACCONF))
> + bindir = self.rootjoin("bin")
> ldconfig = os.path.basename(pacman["ldconfig"])
> - ldconfigdir = os.path.join(self.root, os.path.dirname(pacman["ldconfig"][1:]))
> + ldconfigdir = self.rootjoin(os.path.dirname(pacman["ldconfig"][1:]))
> shell = pacman["scriptlet-shell"][1:]
> - shelldir = os.path.join(self.root, os.path.dirname(shell))
> + shelldir = self.rootjoin(os.path.dirname(shell))
> sys_dirs = [dbdir, cachedir, syncdir, tmpdir, logdir, etcdir, bindir,
> ldconfigdir, shelldir]
> for sys_dir in sys_dirs:
> if not os.path.isdir(sys_dir):
> - vprint("\t%s" % sys_dir[len(self.root)+1:])
> + vprint("\t%s" % self.rootremove(sys_dir))
> os.makedirs(sys_dir, 0755)
> # Only the dynamically linked binary is needed for fakechroot
> shutil.copy("/bin/sh", bindir)
> if shell != "bin/sh":
> - shutil.copy("/bin/sh", os.path.join(self.root, shell))
> + shutil.copy("/bin/sh", self.rootjoin(shell))
> shutil.copy(os.path.join(util.SELFPATH, "ldconfig.stub"),
> os.path.join(ldconfigdir, ldconfig))
> ld_so_conf = open(os.path.join(etcdir, "ld.so.conf"), "w")
> @@ -174,8 +185,8 @@ def generate(self, pacman):
> vprint(" Populating file system")
> for f in self.filesystem:
> vprint("\t%s" % f)
> - util.mkfile(self.root, f, f)
> - path = os.path.join(self.root, f)
> + path = self.rootjoin(f)
> + util.mkfile(path, f)
> if os.path.isfile(path):
> os.utime(path, (355, 355))
> for pkg in self.db["local"].pkgs:
> @@ -187,7 +198,7 @@ def generate(self, pacman):
> for roots, dirs, files in os.walk(self.root):
> for i in files:
> filename = os.path.join(roots, i)
> - f = pmfile.PacmanFile(self.root, filename.replace(self.root + "/", ""))
> + f = pmfile.PacmanFile(self.root, self.rootremove(filename))
> self.files.append(f)
> vprint("\t%s" % f.name)
>
> @@ -223,26 +234,27 @@ def run(self, pacman):
> "--show-reachable=yes",
> "--suppressions=%s" % suppfile])
> cmd.extend([pacman["bin"],
> - "--config", os.path.join(self.root, util.PACCONF),
> + "--config", self.rootjoin(util.PACCONF),
> "--root", self.root,
> - "--dbpath", os.path.join(self.root, util.PM_DBPATH),
> - "--cachedir", os.path.join(self.root, util.PM_CACHEDIR)])
> + "--dbpath", self.rootjoin(util.PM_DBPATH),
> + "--cachedir", self.rootjoin(util.PM_CACHEDIR)])
> if not pacman["manual-confirm"]:
> cmd.append("--noconfirm")
> if pacman["debug"]:
> cmd.append("--debug=%s" % pacman["debug"])
> cmd.extend(shlex.split(self.args))
> if not (pacman["gdb"] or pacman["valgrind"] or pacman["nolog"]):
> - output = open(os.path.join(self.root, util.LOGFILE), 'w')
> + output = open(self.rootjoin(util.LOGFILE), 'w')
> else:
> output = None
> vprint("\trunning: %s" % " ".join(cmd))
>
> # Change to the tmp dir before running pacman, so that local package
> # archives are made available more easily.
> + tmpdir = self.rootjoin(util.TMPDIR)
> time_start = time.time()
> self.retcode = subprocess.call(cmd, stdout=output, stderr=output,
> - cwd=os.path.join(self.root, util.TMPDIR), env={'LC_ALL': 'C'})
> + cwd=tmpdir, env={'LC_ALL': 'C'})
> time_end = time.time()
> vprint("\ttime elapsed: %.2fs" % (time_end - time_start))
>
> @@ -256,7 +268,7 @@ def run(self, pacman):
> tap.diag("\tERROR: %s not removed" % util.PM_LOCK)
> os.unlink(util.PM_LOCK)
> # Look for a core file
> - if os.path.isfile(os.path.join(self.root, util.TMPDIR, "core")):
> + if os.path.isfile(os.path.join(tmpdir, "core")):
> tap.diag("\tERROR: pacman dumped a core file")
>
> def check(self):
> diff --git a/test/pacman/util.py b/test/pacman/util.py
> index 14035d7..5c9a0c0 100644
> --- a/test/pacman/util.py
> +++ b/test/pacman/util.py
> @@ -77,11 +77,10 @@ def getfileinfo(filename):
> data["filename"] = filename
> return data
>
> -def mkfile(base, name, data=""):
> - info = getfileinfo(name)
> - filename = info["filename"]
> +def mkfile(path, data=""):
> + info = getfileinfo(path)
> + path = info["filename"]
>
> - path = os.path.join(base, filename)
> if info["isdir"]:
> if not os.path.isdir(path):
> os.makedirs(path, 0755)
> @@ -129,7 +128,7 @@ def mkcfgfile(filename, root, option, db):
> for optkey, optval in value.option.iteritems():
> data.extend(["%s = %s" % (optkey, j) for j in optval])
>
> - mkfile(root, filename, "\n".join(data))
> + mkfile(os.path.join(root, filename), "\n".join(data))
>
>
> #
> --
> 1.8.4
More information about the pacman-dev
mailing list