I really like the direction this is going in. It so far looks very
clean, though I think that is likely to change as the realities of
package installation start to creep in.
One overriding comment I have is that I'd prefer code for different
payloads to be in different files. These are going to grow in a hurry.
> +from pykickstart.version import makeVersion
This import appears to only get used in testing code down at the bottom,
so it should be moved there.
> + def postInstall(self):
> + """ Perform post-installation tasks. """
> + pass
> +
> + # set default runlevel/target (?)
> + # write out static config (storage, modprobe, keyboard, ??)
> + # kickstart should handle this before we get here
> + # copy firmware
> + # recreate initrd
> + # postInstall or bootloader.install
> + # copy dd rpms (yum/rpm only?)
> + # kickstart
> + # copy dd modules and firmware (yum/rpm only?)
> + # kickstart
> + # write escrow packets
> + # stop logger
I think a lot of this can also be done via %post scripts. We'll have to
see, though. Some of it looks hopelessly tangled together.
> +class LiveImagePayload(ImagePayload):
> + """ A LivePayload copies the source image onto the target system. """
> + def setup(self, storage):
> + super(LiveImagePayload, self).setup()
> + if not stat.S_ISBLK(os.stat(self.image_file)[stat.ST_MODE]):
> + raise PayloadSetupError("unable to find image")
> +
> + def install(self):
> + """ Install the payload. """
> + cmd = "rsync"
> + args = ["-rlptgoDHAXv", self.os_image, ROOT_PATH]
> + try:
> + rc = iutil.execWithRedirect(cmd, args,
> + stderr="/dev/tty5", stdout="/dev/tty5")
> + except (OSError, RuntimeError) as e:
> + err = str(e)
> + else:
> + err = None
> + if rc != 0:
> + err = "%s exited with code %d" % (cmd, rc)
> +
> + if err:
> + exn = PayloadInstallError(err)
> + if errorHandler(exn) == ERROR_RAISE:
> + raise exn
If we are clever, we can progress report this, too. I'm not yet clever
enough to do so, however.
> +class TarPayload(ArchivePayload):
> + """ A TarPayload unpacks tar archives onto the target system. """
According to the setup method, this method looks to really only cover
unpacking a single archive onto the system, not multiple as the above
comment says. I think multiple would be more correct, though.
> + def install(self):
> + try:
> + selfarchive.extractall(path=ROOT_PATH)
> + except (tarfile.ExtractError, tarfile.CompressionError) as e:
> + log.error("extracting tar archive %s: %s" % (self.image_file, e))
Missing a dot between "self" and "archive".
> + def preInstall(self):
> + """ Perform pre-installation tasks. """
> + super(YumPayload, self).preInstall()
> +
> + # doPreInstall
> + # create a bunch of directories like /var, /var/lib/rpm, /root, &c (?)
> + # create mountpoints for protected device mountpoints (?)
> + # initialize the backend logger
> + # write static configs (storage, modprobe.d/anaconda.conf, network, keyboard)
> + # on upgrade, just make sure /etc/mtab is a symlink to /proc/self/mounts
I hope some of this can either move to %post scripts or go away
entirely. I don't think we need to be creating those directories
ourselves any more. It looks like the only reason we currently do so is
because we then write some files out into some of those directories,
which we don't really need to do until after packages are written out,
at which point the directories will exist.
Anyway that's just one example.
> + def postInstall(self):
> + """ Perform post-installation tasks. """
> + self._yum.close()
> +
> + # clean up repo tmpdirs
> + self._yum.cleanPackages()
> + self._yum.cleanHeaders()
> +
> + # remove cache dirs of install-specific repos
> + for repo in self._yum.repos.listEnabled():
> + if repo.name == BASE_REPO_NAME or repo.id.startswith("anaconda-"):
> + shutil.rmtree(repo.cachedir)
> +
> + # clean the yum cache on upgrade
> + if self.data.upgrade.upgrade:
> + self._yum.cleanMetadata()
> +
> + # TODO: on preupgrade, remove the preupgrade dir
> +
> + self._removeTxSaveFile()
Don't forget to call the superclass's method.
> + def _get_txmbr(self, key):
> + """ Return a (name, TransactionMember) tuple from cb key. """
> + if hasattr(key, "po"):
> + # New-style callback, key is a TransactionMember
> + txmbr = key.po
> + name = key.name
> + elif isinstance(key, tuple):
> + # Old-style (hdr, path) callback
> + h = key[0]
> + name = h['name']
> + epoch = '0'
> + if h['epoch'] is not None:
> + epoch = str(h['epoch'])
> + pkgtup = (h['name'], h['arch'], epoch, h['version'], h['release'])
> + txmbrs = self._yum.tsInfo.getMembers(pkgtup=pkgtup)
> + if len(txmbrs) != 1:
> + log.error("unable to find package %s" % pkgtup)
> + exn = PayloadInstallError("failed to find package")
> + if errorHandler(exn, pkgtup) == ERROR_RAISE:
> + raise exn
> +
> + txmbr = txmbrs[0]
> + else:
> + # cleanup/remove error
> + name = key
> + txmbr = None
> +
> + return (name, txmbr)
> +
> + def callback(self, what, amount, total, h, user):
We talked about this in IRC, but for everyone else's benefit I'll sum up
here. First, the old-style code can all go. That was introduced almost
a year ago and the new yum should have propagated by now. We know what
distributions this new anaconda will run on, and we can enforce a yum
via Requires.
We'll also have to ask for using that new version of the callback since
yum sets up for the old style one by default.
Finally, we should refer to "key" throughout the callback instead of
"h", due to largely historical reasons that involve us not wanting to do
anything with RPM headers or even give the appearance that we are doing
so.
> +class YumDepsolveCallback(object):
> + def __init__(self):
> + pass
> +
> + def transactionPopulation(self):
> + pass
> +
> + def pkgAdded(self, pkgtup, state):
> + pass
> +
> + def tscheck(self):
> + pass
> +
> + def downloadHeader(self, name):
> + pass
> +
> + def procReq(self, name, need):
> + pass
> +
> + def procConflict(self, name, need):
> + pass
> +
> + def end(self):
> + pass
We talked a bit about this class in IRC. It is likely only needed for
bouncing the progress bar while deps are being solved, but we'll be
doing that entirely as a background process. Thus, I would propose
first checking if yum absolutely needs the callback and if so, just have
a class like so:
class YumDepsolveCallback(object):
def getattr(self, attr):
return fake
def fake(self, *args, **kwargs):
pass
- Chris
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
