[PATCH] add --copyin and --copyout to mock.
Michael E Brown
Michael_E_Brown at dell.com
Thu Dec 13 22:10:06 UTC 2007
On Thu, Dec 13, 2007 at 02:52:07PM -0600, Clark Williams wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Attached is a patch that adds the --copyin and --copyout options to mock. These
> options allow people to add to a chroot and to pull files out of a chroot, without
> having to know the path to the chroot.
>
> Usage:
>
> mock -r <config> --copyin foo bar baz /tmp
>
> copies the files foo, bar and baz to the chroot /tmp directory for <config>
>
> mock -r <config> --copyout /tmp/result.txt /opt/mydir/something ./results
>
> copies the files <chroot>/tmp/result.txt and <chroot>/opt/mydir/something into the
> local dir ./results.
>
> In the process of doing this I was bitten by os.path.join()'s "intelligent behavior",
> in that when using it to do a copyin, it would refuse to join two absolute paths.
> While coming up with a utility function to do what I wanted, Michael saw that we were
> manipulating the self.rootdir a bit haphazardly and we arrived at makeChrootPath() as
> a method in the Root object. I then did a first cut at replacing ad hoc usage of
> self.rootdir with self.makeChrootPath().
>
> Unless someone objects strenuously, I'll commit this tonight.
>
> Clark
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.7 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
>
> iD8DBQFHYZt3Hyuj/+TTEp0RAq6hAKDkVtmPrWH3NmKGcjEI46HhM6GpCQCfY8oe
> lJAtUdxKXWDS3ilnYdJR1fY=
> =SNv7
> -----END PGP SIGNATURE-----
> diff --git a/py/mock.py b/py/mock.py
> index 5661dff..8fa0bd9 100755
> --- a/py/mock.py
> +++ b/py/mock.py
> @@ -65,6 +65,8 @@ import mock.util
> def command_parse(config_opts):
> """return options and args from parsing the command line"""
> parser = OptionParser(usage=__doc__, version=__VERSION__)
> +
> + # modes (basic commands)
nice touch.
> parser.add_option("--rebuild", action="store_const", const="rebuild",
> dest="mode", default='rebuild',
> help="rebuild the specified SRPM(s)")
> @@ -526,7 +537,37 @@ def main(ret):
>
> elif options.mode == 'orphanskill':
> mock.util.orphansKill(chroot.rootdir)
> -
> + elif options.mode == 'copyin':
> + chroot.tryLockBuildRoot()
It would be a good idea to log these actions to the root log. Until we
get a better way to do it, the best way to do that is to call
chroot._resetLogging() right here.
> + uidManager.dropPrivsForever()
> + if len(args) < 2:
> + log.critical("Must have source and destinations for copyin")
> + sys.exit(50)
> + dest = chroot.makeChrootPath(args[-1])
> + if len(args) > 2 and not os.path.isdir(dest):
> + log.critical("multiple source files and %s is not a directory!" % dest)
> + sys.exit(50)
Nice error handling.
> + args = args[:-1]
> + import shutil
> + for f in args:
> + print "copying %s to %s" % (f, dest)
> + shutil.copy(f, dest)
Looking good.
One small tweak: use log.info() instead of print. Print doesnt go to the
logs properly. You should not see any print statements anywhere in the
mock codebase.
Additionally, if you are adding a debugging statement, if you use
log.debug(), you never need to remove the logging statement as long as
it is useful. ( "mock -v -v ..." will show log.debug() statements.)
nb. mock.py uses log.XXX(), other places may have their own loggers.
> + elif options.mode == 'copyout':
> + chroot.tryLockBuildRoot()
ditto.
> + uidManager.dropPrivsForever()
> + if len(args) < 2:
> + log.critical("Must have source and destinations for copyout")
> + sys.exit(50)
> + dest = args[-1]
> + if len(args) > 2 and not os.path.isdir(dest):
> + log.critical("multiple source files and %s is not a directory!" % dest)
> + sys.exit(50)
ditto.
> + args = args[:-1]
> + import shutil
> + for f in args:
> + src = chroot.makeChrootPath(f)
> + print "copying %s to %s" % (src, dest)
> + shutil.copy(src, dest)
ditto.
>
> if __name__ == '__main__':
> # fix for python 2.4 logging module bug:
> diff --git a/py/mock/backend.py b/py/mock/backend.py
> index 4593357..ed283c9 100644
> --- a/py/mock/backend.py
> +++ b/py/mock/backend.py
> @@ -142,6 +142,11 @@ class Root(object):
> return 1
>
> decorate(traceLog())
> + def makeChrootPath(self, *args):
Comment here would be good.
"For safety reasons, self.rootdir should not be used directly. Instead
use this handy helper function anytime you want to reference a path in
relation to the chroot."
or something similar
> + tmp = self.rootdir + "/" + "/".join(args)
> + return tmp.replace("//", "/")
> +
> + decorate(traceLog())
> def init(self):
> self.state("init")
>
> @@ -188,41 +193,41 @@ class Root(object):
> 'proc',
> 'sys',
> ]:
> - mock.util.mkdirIfAbsent(os.path.join(self.rootdir, item))
> + mock.util.mkdirIfAbsent(self.makeChrootPath(item))
>
> # touch files
> self.root_log.debug('touch required files')
> - for item in [os.path.join(self.rootdir, 'etc', 'mtab'),
> - os.path.join(self.rootdir, 'etc', 'fstab'),
> - os.path.join(self.rootdir, 'var', 'log', 'yum.log')]:
> + for item in [self.makeChrootPath('etc', 'mtab'),
> + self.makeChrootPath('etc', 'fstab'),
> + self.makeChrootPath('var', 'log', 'yum.log')]:
> mock.util.touch(item)
>
> # write in yum.conf into chroot
> # always truncate and overwrite (w+)
> self.root_log.debug('configure yum')
> - yumconf = os.path.join(self.rootdir, 'etc', 'yum', 'yum.conf')
> + yumconf = self.makeChrootPath('etc', 'yum', 'yum.conf')
> yumconf_fo = open(yumconf, 'w+')
> yumconf_fo.write(self.yum_conf_content)
> yumconf_fo.close()
>
> # symlink /etc/yum.conf to /etc/yum/yum.conf (FC6 requires)
> try:
> - os.unlink(os.path.join(self.rootdir, "etc", "yum.conf"))
> + os.unlink(self.makeChrootPath("etc", "yum.conf"))
> except OSError:
> pass
> - os.symlink('yum/yum.conf', os.path.join(self.rootdir, "etc", "yum.conf"))
> + os.symlink('yum/yum.conf', self.makeChrootPath("etc", "yum.conf"))
>
> # set up resolv.conf
> if self.use_host_resolv:
> - resolvdir = os.path.join(self.rootdir, 'etc')
> - resolvpath = os.path.join(self.rootdir, 'etc', 'resolv.conf')
> + resolvdir = self.makeChrootPath('etc')
> + resolvpath = self.makeChrootPath('etc', 'resolv.conf')
> if os.path.exists(resolvpath):
> os.remove(resolvpath)
> shutil.copy2('/etc/resolv.conf', resolvdir)
>
> # files in /etc that need doing
> for key in self.chroot_file_contents:
> - p = os.path.join(self.rootdir, *key.split('/'))
> + p = self.makeChrootPath(*key.split('/'))
This original *key.split("/") was to work around the os.path.join
"intelligence". You could make this easier to read by getting rid of
this and just directly using key.
> if not os.path.exists(p):
> # write file
> fo = open(p, 'w+')
> @@ -254,8 +259,8 @@ class Root(object):
> decorate(traceLog())
> def _setupDev(self):
> # files in /dev
> - mock.util.rmtree(os.path.join(self.rootdir, "dev"))
> - mock.util.mkdirIfAbsent(os.path.join(self.rootdir, "dev", "pts"))
> + mock.util.rmtree(self.makeChrootPath("dev"))
> + mock.util.mkdirIfAbsent(self.makeChrootPath("dev", "pts"))
> prevMask = os.umask(0000)
> devFiles = (
> (stat.S_IFCHR | 0666, os.makedev(1, 3), "dev/null"),
> @@ -268,23 +273,23 @@ class Root(object):
> )
> for i in devFiles:
> # create node
> - os.mknod( os.path.join(self.rootdir, i[2]), i[0], i[1] )
> + os.mknod( self.makeChrootPath(i[2]), i[0], i[1])
> # set context. (only necessary if host running selinux enabled.)
> # fails gracefully if chcon not installed.
> mock.util.do("chcon --reference=/%s %s" %
> - (i[2], os.path.join(self.rootdir, i[2])), raiseExc=0)
> + (i[2], self.makeChrootPath(i[2])), raiseExc=0)
>
> - os.symlink("/proc/self/fd/0", os.path.join(self.rootdir, "dev/stdin"))
> - os.symlink("/proc/self/fd/1", os.path.join(self.rootdir, "dev/stdout"))
> - os.symlink("/proc/self/fd/2", os.path.join(self.rootdir, "dev/stderr"))
> + os.symlink("/proc/self/fd/0", self.makeChrootPath("dev/stdin"))
> + os.symlink("/proc/self/fd/1", self.makeChrootPath("dev/stdout"))
> + os.symlink("/proc/self/fd/2", self.makeChrootPath("dev/stderr"))
> os.umask(prevMask)
>
> # mount/umount
> - umntCmd = 'umount -n %s/dev/pts' % self.rootdir
> + umntCmd = 'umount -n %s' % self.makeChrootPath('/dev/pts')
> if umntCmd not in self.umountCmds:
> self.umountCmds.append(umntCmd)
>
> - mntCmd = 'mount -n -t devpts mock_chroot_devpts %s/dev/pts' % self.rootdir
> + mntCmd = 'mount -n -t devpts mock_chroot_devpts %s' % self.makeChrootPath('/dev/pts')
> if mntCmd not in self.mountCmds:
> self.mountCmds.append(mntCmd)
>
> @@ -405,7 +410,7 @@ class Root(object):
> gid=self.chrootgid,
> )
>
> - bd_out = self.rootdir + self.builddir
> + bd_out = self.makeChrootPath(self.builddir)
> rpms = glob.glob(bd_out + '/RPMS/*.rpm')
> srpms = glob.glob(bd_out + '/SRPMS/*.rpm')
> packages = rpms + srpms
> @@ -482,11 +487,11 @@ class Root(object):
>
> decorate(traceLog())
> def _makeBuildUser(self):
> - if not os.path.exists(os.path.join(self.rootdir, 'usr/sbin/useradd')):
> + if not os.path.exists(self.makeChrootPath('usr/sbin/useradd')):
> raise mock.exception.RootError, "Could not find useradd in chroot, maybe the install failed?"
>
> # safe and easy. blow away existing /builddir and completely re-create.
> - mock.util.rmtree(os.path.join(self.rootdir, self.homedir))
> + mock.util.rmtree(self.makeChrootPath(self.homedir))
> dets = { 'uid': self.chrootuid, 'gid': self.chrootgid, 'user': self.chrootuser, 'group': self.chrootgroup, 'home': self.homedir }
>
> self.doChroot('/usr/sbin/userdel -r %(user)s' % dets, raiseExc=False)
> @@ -533,7 +538,7 @@ class Root(object):
> self.uidManager.becomeUser(self.chrootuid, self.chrootgid)
> try:
> # create dir structure
> - for subdir in ["%s/%s/%s" % (self.rootdir, self.builddir, s) for s in ('RPMS', 'SRPMS', 'SOURCES', 'SPECS', 'BUILD', 'originals')]:
> + for subdir in ["%s" % self.makeChrootPath(self.builddir, s) for s in ('RPMS', 'SRPMS', 'SOURCES', 'SPECS', 'BUILD', 'originals')]:
No longer a need for "%s" % ... here. Just get rid of it and directly
use self.makeChrootPath().
> mock.util.mkdirIfAbsent(subdir)
>
> # change ownership so we can write to build home dir
> @@ -543,7 +548,7 @@ class Root(object):
> os.chmod(os.path.join(dirpath, path), 0755)
>
> # rpmmacros default
> - macrofile_out = '%s%s/.rpmmacros' % (self.rootdir, self.homedir)
> + macrofile_out = '%s/.rpmmacros' % self.makeChrootPath(self.homedir)
Probably dont need string interpolation here, either.
> rpmmacros = open(macrofile_out, 'w+')
> for key, value in self.macros.items():
> rpmmacros.write( "%s %s\n" % (key, value) )
> @@ -559,7 +564,7 @@ class Root(object):
> decorate(traceLog())
> def _copySrpmIntoChroot(self, srpm):
> srpmFilename = os.path.basename(srpm)
> - dest = self.rootdir + '/' + self.builddir + '/' + 'originals'
> + dest = self.makeChrootPath(self.builddir, 'originals')
> shutil.copy2(srpm, dest)
> return os.path.join(self.builddir, 'originals', srpmFilename)
>
> diff --git a/docs/releasetests.sh b/docs/releasetests.sh
> index df82523..3043a60 100755
> --- a/docs/releasetests.sh
> +++ b/docs/releasetests.sh
> @@ -53,6 +53,23 @@ if [ ! -e $CHROOT/usr/include/python* ]; then
> exit 1
> fi
>
> +# test the copyin command
> +time $MOCKCMD --copyin docs/release-instructions.txt /tmp
> +if [ ! -f $(CHROOT)/tmp/release-instructions.txt ]; then
> + echo "copyin test FAILED! could not find /tmp/release-instructions.txt"
> + exit 1
> +fi
> +
> +# test the copyout command
> +time $MOCKCMD --copyout /etc/passwd /tmp/my-copyout-passwd
> +if [ ! -f /tmp/my-copyout-passwd ]; then
> + echo "copyout test FAILED! could not find /tmp/my-copyout-passwd"
> +fi
> +mkdir /tmp/$uniqueext
> +time $MOCKCMD --copyout /etc/passwd /etc/fstab /tmp/$uniqueext
> +if [ ! -f /tmp/$uniqueext/passwd -o ! -f /tmp/$uniqueext/fstab ]; then
> + echo "multiple copyout failed!"
> +fi
You could convert the daemontest test to use --copyin and kill two birds
with one stone. (change the comment to say that it is testing both.) ==
shorter unit test runs...
Really excellent overall. Thanks.
--
Michael
More information about the Fedora-buildsys-list
mailing list